Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions forms_pro/api/team.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import frappe

from forms_pro.utils.teams import GetTeamFormsResponseSchema
from forms_pro.utils.teams import get_team_forms as get_team_forms_utils


@frappe.whitelist()
def get_team_forms(team_id: str) -> list[GetTeamFormsResponseSchema]:
"""
Get the list of forms for the current team

Args:
team_id: ID of the team

Returns:
list[GetTeamFormsResponseSchema] - List of forms for the team
"""
forms = get_team_forms_utils(team_id=team_id)
return forms
Comment thread
harshtandiya marked this conversation as resolved.
61 changes: 61 additions & 0 deletions forms_pro/api/user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import frappe
from frappe.core.doctype.has_role.has_role import HasRole
from pydantic import BaseModel, Field, field_validator

from forms_pro.utils.teams import get_user_teams as get_user_teams_utils


class GetUserTeamsResponseSchema(BaseModel):
name: str = Field(description="ID of the team")
team_name: str = Field(description="The name of the team")
is_current: bool = Field(description="Whether this is the current team")


class GetUserResponseSchema(BaseModel):
email: str
first_name: str
last_name: str | None = None
full_name: str
username: str
desk_theme: str
roles: list[str]
has_desk_access: bool

@field_validator("roles", mode="before")
@classmethod
def extract_roles(cls, v: list[HasRole]) -> list[str]:
if not v:
return []

return [role.role for role in v]


@frappe.whitelist()
def get_user() -> GetUserResponseSchema:
"""
Get Current User Data
"""

user_id = frappe.session.user
user_doc = frappe.get_doc("User", user_id)
data = user_doc.as_dict()
data["roles"] = user_doc.get("roles")
data["has_desk_access"] = user_doc.has_desk_access()

return GetUserResponseSchema.model_validate(data).model_dump()
Comment thread
harshtandiya marked this conversation as resolved.


@frappe.whitelist()
def get_user_teams() -> list[GetUserTeamsResponseSchema]:
"""
Get the list of teams for the current user
"""

user = frappe.session.user

if user == "Guest":
return []

teams = get_user_teams_utils(user)

return [GetUserTeamsResponseSchema.model_validate(team).model_dump() for team in teams]
Comment thread
harshtandiya marked this conversation as resolved.
10 changes: 9 additions & 1 deletion forms_pro/forms_pro/doctype/form/form.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"column_break_pqae",
"title",
"linked_doctype",
"linked_team_id",
"settings_section",
"login_required",
"column_break_xocw",
Expand Down Expand Up @@ -107,12 +108,19 @@
"fieldname": "allow_incomplete",
"fieldtype": "Check",
"label": "Allow Incomplete Forms"
},
{
"fieldname": "linked_team_id",
"fieldtype": "Link",
"label": "Linked Team",
"options": "FP Team",
"reqd": 1
}
],
"grid_page_length": 50,
"index_web_pages_for_search": 1,
"links": [],
"modified": "2025-10-07 12:16:18.825427",
"modified": "2025-12-20 20:40:24.219032",
"modified_by": "Administrator",
"module": "Forms Pro",
"name": "Form",
Expand Down
1 change: 1 addition & 0 deletions forms_pro/forms_pro/doctype/form/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class Form(Document):
fields: DF.Table[FormField]
is_published: DF.Check
linked_doctype: DF.Link
linked_team_id: DF.Link
login_required: DF.Check
metadata: DF.Code | None
route: DF.Data | None
Expand Down
13 changes: 12 additions & 1 deletion forms_pro/forms_pro/doctype/form/test_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import frappe
from frappe.tests import IntegrationTestCase

from forms_pro.utils.teams import get_user_teams

# On IntegrationTestCase, the doctype test records and all
# link-field test record dependencies are recursively loaded
# Use these module variables to add/remove to/from that list
Expand All @@ -18,9 +20,18 @@ def setUp(self):
self.test_doctype_name = "Test Form DocType"
self.create_test_doctype()

self.test_user = "test_forms_pro_user@example.com"
self.test_team = get_user_teams(self.test_user)[0]["name"]

Comment on lines +23 to +25
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "install.py" -o -name "*fixture*" -o -name "*setup*" | head -20

Repository: BuildWithHussain/forms_pro

Length of output: 113


🏁 Script executed:

rg -n "test_forms_pro_user@example.com" -A 5 -B 5

Repository: BuildWithHussain/forms_pro

Length of output: 2308


🏁 Script executed:

rg -n "def get_user_teams" -A 15

Repository: BuildWithHussain/forms_pro

Length of output: 1637


🏁 Script executed:

rg -n "def setUp|def setUpClass" -B 2 -A 10 forms_pro/forms_pro/doctype/form/test_form.py

Repository: BuildWithHussain/forms_pro

Length of output: 564


🏁 Script executed:

cat -n forms_pro/forms_pro/doctype/form/test_form.py | head -40

Repository: BuildWithHussain/forms_pro

Length of output: 1679


🏁 Script executed:

cat -n forms_pro/install.py

Repository: BuildWithHussain/forms_pro

Length of output: 1019


🏁 Script executed:

rg -n "FP Team" -B 3 -A 10 forms_pro/install.py

Repository: BuildWithHussain/forms_pro

Length of output: 52


🏁 Script executed:

cat -n forms_pro/utils/test_form_generator.py | head -30

Repository: BuildWithHussain/forms_pro

Length of output: 1331


🏁 Script executed:

rg -n "FP Team" forms_pro/ -A 5 | head -50

Repository: BuildWithHussain/forms_pro

Length of output: 3097


🏁 Script executed:

ls -la forms_pro/fixtures/

Repository: BuildWithHussain/forms_pro

Length of output: 239


🏁 Script executed:

cat forms_pro/fixtures/*.json 2>/dev/null | head -100

Repository: BuildWithHussain/forms_pro

Length of output: 352


🏁 Script executed:

cat -n forms_pro/overrides/roles.py

Repository: BuildWithHussain/forms_pro

Length of output: 1820


🏁 Script executed:

rg -n "def.*after_insert|def.*on_update" forms_pro/overrides/roles.py -B 2 -A 15

Repository: BuildWithHussain/forms_pro

Length of output: 52


Guard against IndexError when retrieving test team.

The test user is created with the "Forms Pro User" role during test setup, which should automatically create a default team through the handle_forms_pro_role_change hook in forms_pro/overrides/roles.py. However, accessing [0] on get_user_teams() without bounds checking creates an unnecessary risk—if team creation fails or is skipped, an IndexError will crash the test. Add defensive checks to ensure the user has at least one team before accessing it.

Suggested fix:

 self.test_user = "test_forms_pro_user@example.com"
-self.test_team = get_user_teams(self.test_user)[0]["name"]
+user_teams = get_user_teams(self.test_user)
+self.assertGreater(len(user_teams), 0, f"User {self.test_user} has no teams")
+self.test_team = user_teams[0]["name"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
self.test_user = "test_forms_pro_user@example.com"
self.test_team = get_user_teams(self.test_user)[0]["name"]
self.test_user = "test_forms_pro_user@example.com"
user_teams = get_user_teams(self.test_user)
self.assertGreater(len(user_teams), 0, f"User {self.test_user} has no teams")
self.test_team = user_teams[0]["name"]
🤖 Prompt for AI Agents
In forms_pro/forms_pro/doctype/form/test_form.py around lines 23 to 25, guard
against IndexError when retrieving the test user's team: call
get_user_teams(self.test_user) into a variable, check that the returned list is
non-empty before indexing, and if it is empty either (a) create or assign a
default team for the test user (e.g., invoke the team-creation path used by the
Forms Pro role hook) or (b) raise an explicit AssertionError with a clear
message so the test fails fast and clearly; then set self.test_team to the first
team's name only after this defensive check.

# Create a test Form
self.test_form = frappe.get_doc(
{"doctype": "Form", "title": "Test Form", "linked_doctype": self.test_doctype_name, "fields": []}
{
"doctype": "Form",
"title": "Test Form",
"linked_doctype": self.test_doctype_name,
"fields": [],
"linked_team_id": self.test_team,
}
)
self.test_form.insert()

Expand Down
22 changes: 21 additions & 1 deletion forms_pro/forms_pro/doctype/fp_team/fp_team.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,24 @@ class FPTeam(Document):
users: DF.TableMultiSelect[FPTeamMember]
# end: auto-generated types

pass
@property
def team_members(self) -> list[str]:
"""
Get the list of team members

Returns:
list[str] - List of team member email addresses
"""
return [member.user for member in self.users] if self.users else []

def is_team_member(self, user: str) -> bool:
"""
Check if a user is a member of the team

Args:
user: The user email address

Returns:
bool - True if the user is a member of the team, False otherwise
"""
return user in self.team_members
2 changes: 1 addition & 1 deletion forms_pro/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
# Testing
# -------

# before_tests = "forms_pro.install.before_tests"
before_tests = "forms_pro.install.before_tests"

# Overriding Methods
# ------------------------------
Expand Down
30 changes: 30 additions & 0 deletions forms_pro/install.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import frappe
from frappe.core.doctype.user.user import User

from forms_pro.roles import FORMS_PRO_ROLE


def before_tests():
give_admin_forms_pro_role()
create_test_user()


def give_admin_forms_pro_role():
admin = frappe.get_doc("User", "Administrator")
admin.append("roles", {"role": FORMS_PRO_ROLE})
admin.save()
Comment on lines +12 to +15
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Check for existing role before appending.

Line 14 appends the Forms Pro role without checking if it already exists. If before_tests is called multiple times, this will create duplicate role entries for the Administrator user.

🔎 Recommended fix to prevent duplicate roles
 def give_admin_forms_pro_role():
     admin = frappe.get_doc("User", "Administrator")
-    admin.append("roles", {"role": FORMS_PRO_ROLE})
+    if not any(r.role == FORMS_PRO_ROLE for r in admin.roles):
+        admin.append("roles", {"role": FORMS_PRO_ROLE})
     admin.save()
🤖 Prompt for AI Agents
In forms_pro/install.py around lines 12 to 15, the code appends the
FORMS_PRO_ROLE to the Administrator user unconditionally which can create
duplicate role entries; change it to first check whether the role already exists
on the admin document (e.g., inspect admin.get("roles") or admin.roles) and only
append and save if the role is not present, ensuring admin.save() is called only
when a new role was actually added.



def create_test_user():
test_user = "test_forms_pro_user@example.com"

if frappe.db.exists("User", test_user):
return

user: User = frappe.new_doc("User")
user.email = test_user
user.first_name = "Test"
user.last_name = "Forms Pro User"
user.insert(ignore_permissions=True)
user.append("roles", {"role": FORMS_PRO_ROLE})
user.save(ignore_permissions=True)
Comment on lines +18 to +30
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Check for existing role before appending.

Line 29 appends the Forms Pro role without checking if it already exists. While this is less critical than in give_admin_forms_pro_role due to the existence check on line 21, it could still cause issues if the user document is partially created in a previous failed run.

🔎 Recommended fix to prevent duplicate roles
     user: User = frappe.new_doc("User")
     user.email = test_user
     user.first_name = "Test"
     user.last_name = "Forms Pro User"
     user.insert(ignore_permissions=True)
-    user.append("roles", {"role": FORMS_PRO_ROLE})
+    if not any(r.role == FORMS_PRO_ROLE for r in user.roles):
+        user.append("roles", {"role": FORMS_PRO_ROLE})
     user.save(ignore_permissions=True)
🤖 Prompt for AI Agents
In forms_pro/install.py around lines 18 to 30, the code appends the
FORMS_PRO_ROLE to the new user unconditionally, which can create a duplicate
role entry if the user doc was partially created in a prior run; before
appending, check whether the user already has a role entry for FORMS_PRO_ROLE
(e.g., inspect user.get("roles") or query existing UserRole) and only append/add
the role when it's not present, then save the document as before.

5 changes: 3 additions & 2 deletions forms_pro/overrides/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from frappe.core.doctype.user.user import User

from forms_pro.roles import FORMS_PRO_ROLE
from forms_pro.utils.teams import get_user_teams
from forms_pro.utils.teams import get_user_teams, set_current_team


def handle_forms_pro_role_change(doc, method) -> None:
Expand All @@ -27,7 +27,7 @@ def handle_forms_pro_role_change(doc, method) -> None:
create_default_team_for_user(user)


def create_default_team_for_user(user: User):
def create_default_team_for_user(user: User) -> None:
from forms_pro.forms_pro.doctype.fp_team.fp_team import FPTeam

team: FPTeam = frappe.new_doc("FP Team")
Expand All @@ -40,3 +40,4 @@ def create_default_team_for_user(user: User):
},
)
team.save(ignore_permissions=True)
set_current_team(team.name, user.name)
16 changes: 11 additions & 5 deletions forms_pro/utils/form_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@


@frappe.whitelist()
def create_form_with_doctype(doctype: str):
def create_form_with_doctype(team_id: str, doctype: str):
roles = frappe.get_roles(frappe.session.user)
if FORMS_PRO_ROLE not in roles:
frappe.throw("You are not authorized to create a form")
Comment thread
harshtandiya marked this conversation as resolved.

form_generator = FormGenerator(linked_doctype=doctype)
form_generator = FormGenerator(team_id=team_id, linked_doctype=doctype)
form_generator.generate()

return {
Expand All @@ -19,12 +19,12 @@ def create_form_with_doctype(doctype: str):


@frappe.whitelist()
def create_form():
def create_form(team_id: str):
roles = frappe.get_roles(frappe.session.user)
if FORMS_PRO_ROLE not in roles:
frappe.throw("You are not authorized to create a form")

form_generator = FormGenerator()
form_generator = FormGenerator(team_id=team_id)
form_generator.generate()

return {
Expand All @@ -34,8 +34,13 @@ def create_form():


class FormGenerator:
def __init__(self, linked_doctype: str | None = None) -> None:
def __init__(
self,
team_id: str,
linked_doctype: str | None = None,
) -> None:
self.doctype = None
self.team_id = team_id
Comment on lines +37 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing validation for team_id parameter.

The team_id parameter is required but there's no validation. If a caller passes None (e.g., when user.currentTeam?.name is undefined in the frontend), the form will be created with linked_team_id = None, which may cause issues when querying team forms later.

Consider adding validation:

🔎 Suggested fix
     def __init__(
         self,
         team_id: str,
         linked_doctype: str | None = None,
     ) -> None:
+        if not team_id:
+            frappe.throw("Team ID is required to create a form")
         self.doctype = None
         self.team_id = team_id
🤖 Prompt for AI Agents
In forms_pro/utils/form_generator.py around lines 37 to 43, the __init__ method
accepts a required team_id but performs no validation; add a guard to ensure
team_id is provided and is a non-empty string (e.g., if team_id is None or
team_id == "" then raise ValueError with a clear message), optionally
coerce/check type and document the expectation in the method docstring so
callers (and tests) must pass a valid team_id rather than None.

if linked_doctype:
self.doctype = frappe.get_doc("DocType", linked_doctype)

Expand Down Expand Up @@ -80,6 +85,7 @@ def _initialize_form_document(self) -> None:
form_document = frappe.new_doc("Form")
form_document.linked_doctype = self.doctype.name
form_document.title = "Untitled Form"
form_document.linked_team_id = self.team_id
form_document.insert(ignore_permissions=True)
self.form_document = form_document

Expand Down
Loading