-
Notifications
You must be signed in to change notification settings - Fork 12
feat: manage form pages #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9bd7011
81c605e
bbc1bd2
4c088a6
3433fb5
5823e4e
91b062e
435b88b
96c882b
091ba60
ae24b97
eb8eee6
20613e5
d5b632a
b3989ea
8f601f3
374fed7
c87dc09
d57226f
303e6fb
2f1a875
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,20 @@ | ||||||||||||||||||||
| import frappe | ||||||||||||||||||||
| from frappe import _ | ||||||||||||||||||||
| from frappe.share import remove | ||||||||||||||||||||
| from pydantic import BaseModel, Field | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from forms_pro.api.user import get_user | ||||||||||||||||||||
| from forms_pro.forms_pro.doctype.form.form import Form | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class FormSharedWithResponse(BaseModel): | ||||||||||||||||||||
| full_name: str | ||||||||||||||||||||
| user_image: str | None | ||||||||||||||||||||
| email: str = Field(alias="user") | ||||||||||||||||||||
| read: bool | ||||||||||||||||||||
| write: bool | ||||||||||||||||||||
| share: bool | ||||||||||||||||||||
| submit: bool | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @frappe.whitelist(allow_guest=True) | ||||||||||||||||||||
|
|
@@ -9,7 +25,7 @@ def get_form_by_route(route: str) -> dict: | |||||||||||||||||||
|
|
||||||||||||||||||||
| @frappe.whitelist(allow_guest=True) | ||||||||||||||||||||
| def get_form(form_id: str) -> dict: | ||||||||||||||||||||
| form = frappe.get_doc( | ||||||||||||||||||||
| form: Form = frappe.get_doc( | ||||||||||||||||||||
| "Form", | ||||||||||||||||||||
| form_id, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
@@ -24,6 +40,66 @@ def get_form(form_id: str) -> dict: | |||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @frappe.whitelist() | ||||||||||||||||||||
| def get_form_shared_with(form_id: str) -> list[frappe.Any]: | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Get list of users with which a form is shared. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| We validate the current user has read access to the form. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| if not frappe.has_permission( | ||||||||||||||||||||
| "Form", | ||||||||||||||||||||
| "read", | ||||||||||||||||||||
| form_id, | ||||||||||||||||||||
| ): | ||||||||||||||||||||
| frappe.throw(_("You do not have read access to this form")) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| form: Form = frappe.get_doc("Form", form_id) | ||||||||||||||||||||
| shared_with = form.shared_with() | ||||||||||||||||||||
|
|
||||||||||||||||||||
| shared_with_responses = [] | ||||||||||||||||||||
|
|
||||||||||||||||||||
| for user in shared_with: | ||||||||||||||||||||
| _user = get_user(user["user"]) | ||||||||||||||||||||
| user.update(_user) | ||||||||||||||||||||
| shared_with_responses.append(FormSharedWithResponse.model_validate(user).model_dump()) | ||||||||||||||||||||
|
Comment on lines
+61
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle The 🔎 Proposed fix for user in shared_with:
_user = get_user(user["user"])
- user.update(_user)
+ if _user:
+ user.update(_user)
shared_with_responses.append(FormSharedWithResponse.model_validate(user).model_dump())📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| return shared_with_responses | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @frappe.whitelist() | ||||||||||||||||||||
| def remove_form_access(form_id: str, user_email: str) -> None: | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| Remove access to a form for a user. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| We validate the current user has write access to the form. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| args: | ||||||||||||||||||||
| form_id: str - The ID of the form to remove access to. | ||||||||||||||||||||
| user_email: str - The email of the user to remove access to. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| """ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if not frappe.has_permission("Form", "write", form_id): | ||||||||||||||||||||
| frappe.throw(_("You do not have write access to this form")) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return remove(doctype="Form", name=form_id, user=user_email, flags={"ignore_permissions": True}) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @frappe.whitelist() | ||||||||||||||||||||
| def get_doctype_list() -> list[str]: | ||||||||||||||||||||
| if not frappe.has_permission("DocType", "read"): | ||||||||||||||||||||
| frappe.throw(_("You do not have read access to this doctype")) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return frappe.db.get_list( | ||||||||||||||||||||
| "DocType", | ||||||||||||||||||||
| filters={"istable": 0}, | ||||||||||||||||||||
| pluck="name", | ||||||||||||||||||||
| order_by="name", | ||||||||||||||||||||
| limit_page_length=99999, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @frappe.whitelist() | ||||||||||||||||||||
| def get_doctype_fields(doctype: str) -> dict: | ||||||||||||||||||||
| doctype = frappe.get_doc("DocType", doctype) | ||||||||||||||||||||
|
|
@@ -43,14 +119,3 @@ def get_doctype_fields(doctype: str) -> dict: | |||||||||||||||||||
| fields = [field for field in fields if field.fieldtype not in FIELDTYPES_TO_REMOVE] | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return fields | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @frappe.whitelist() | ||||||||||||||||||||
| def get_doctype_list() -> list[str]: | ||||||||||||||||||||
| return frappe.db.get_list( | ||||||||||||||||||||
| "DocType", | ||||||||||||||||||||
| filters={"istable": 0}, | ||||||||||||||||||||
| pluck="name", | ||||||||||||||||||||
| order_by="name", | ||||||||||||||||||||
| limit_page_length=99999, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,8 +30,22 @@ def extract_roles(cls, v: list[HasRole]) -> list[str]: | |
| return [role.role for role in v] | ||
|
|
||
|
|
||
| class GetUserBasicResponse(BaseModel): | ||
| full_name: str | ||
| user_image: str | None = None | ||
|
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical type mismatch: The Python model defines export type GetUserBasicResponse = {
full_name: string;
user_image: string; // ❌ Not nullable
};This mismatch will cause TypeScript type errors when 🔎 Fix the frontend type definitionUpdate export type GetUserBasicResponse = {
full_name: string;
- user_image: string;
+ user_image: string | null;
};🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| @frappe.whitelist() | ||
| def get_user(user: str) -> GetUserBasicResponse | None: | ||
| """Get basic user data for a given user""" | ||
| data = frappe.db.get_value("User", user, ["full_name", "user_image"], as_dict=True) | ||
| if not data: | ||
| return None | ||
| return GetUserBasicResponse.model_validate(data).model_dump() | ||
|
|
||
|
|
||
| @frappe.whitelist() | ||
| def get_user() -> GetUserResponseSchema: | ||
| def get_current_user() -> GetUserResponseSchema: | ||
| """ | ||
| Get Current User Data | ||
| """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,11 @@ | ||
| # Copyright (c) 2025, harsh@buildwithhussain.com and contributors | ||
| # For license information, please see license.txt | ||
|
|
||
| from typing import Any | ||
|
|
||
| import frappe | ||
| from frappe.model.document import Document | ||
| from frappe.share import get_users | ||
|
|
||
|
|
||
| class Form(Document): | ||
|
|
@@ -34,6 +37,14 @@ class Form(Document): | |
| def linked_doctype_doc(self) -> Document: | ||
| return frappe.get_doc("DocType", self.linked_doctype) | ||
|
|
||
| @frappe.whitelist() | ||
| def shared_with(self) -> list[dict[str, Any]]: | ||
| """ | ||
| Get list of users with which this form is shared | ||
| """ | ||
| users_shared_with = get_users(self.doctype, self.name) | ||
| return users_shared_with | ||
|
Comment on lines
+40
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for frappe.share.get_users usage patterns and permission checks
rg -n -A5 -B5 'frappe\.share\.get_users|from frappe\.share import get_users' --type=pyRepository: BuildWithHussain/forms_pro Length of output: 794 🏁 Script executed: # Search for other whitelisted methods that check permissions
rg -n -A10 '@frappe\.whitelist\(\)' --type=py | head -100Repository: BuildWithHussain/forms_pro Length of output: 5439 🏁 Script executed: # Examine the specific file
cat -n forms_pro/forms_pro/doctype/form/form.py | head -60Repository: BuildWithHussain/forms_pro Length of output: 2195 🏁 Script executed: # Search for permission checks in whitelisted methods more broadly
rg -B2 -A8 '@frappe\.whitelist\(\)' --type=py | rg -B8 -A2 'check_perm|has_perm|get_permission' | head -80Repository: BuildWithHussain/forms_pro Length of output: 1617 🏁 Script executed: # Search for frappe.share documentation or get_users usage in similar contexts
rg -n 'get_users' --type=py -B3 -A3 | head -50Repository: BuildWithHussain/forms_pro Length of output: 1023 🏁 Script executed: # Check if there are other document methods with whitelist that don't check permissions
rg -B2 -A8 '@frappe\.whitelist\(\)' --type=py forms_pro/forms_pro/doctype/ -A8Repository: BuildWithHussain/forms_pro Length of output: 881 🏁 Script executed: # Look at the full get_form_shared_with implementation for comparison
sed -n '43,68p' forms_pro/api/form.pyRepository: BuildWithHussain/forms_pro Length of output: 782 🏁 Script executed: # Search for any comments or docstrings about frappe.whitelist() permission handling
rg -B5 -A5 'frappe.whitelist' --type=py | rg -B5 -A5 'permission|auth|check' | head -60Repository: BuildWithHussain/forms_pro Length of output: 2856 🏁 Script executed: # Check if there are any permission decorators or checks in Frappe that might auto-validate
rg -n 'allow_guest|check_permission|require_login' --type=py | head -20Repository: BuildWithHussain/forms_pro Length of output: 257 🏁 Script executed: # Look for the actual Form doctype definition to see if there are document-level permission rules
fd -name '*.json' forms_pro/forms_pro/doctype/formRepository: BuildWithHussain/forms_pro Length of output: 300 🏁 Script executed: # Find the Form doctype JSON definition
fd form.json forms_pro/forms_pro/doctype/form/Repository: BuildWithHussain/forms_pro Length of output: 112 🏁 Script executed: # Check the JSON file if it exists
cat forms_pro/forms_pro/doctype/form/form.json 2>/dev/null | head -100Repository: BuildWithHussain/forms_pro Length of output: 1931 🏁 Script executed: # Check if there are permissions defined in the doctype JSON for the shared_with method
cat forms_pro/forms_pro/doctype/form/form.json | grep -A50 'permission' 2>/dev/null | head -80Repository: BuildWithHussain/forms_pro Length of output: 357 🏁 Script executed: # Examine whether Frappe automatically enforces permissions on document methods
rg -n 'def shared_with' -A20 forms_pro/forms_pro/doctype/form/form.pyRepository: BuildWithHussain/forms_pro Length of output: 849 Add explicit permission validation before returning sensitive sharing information. The @frappe.whitelist()
def shared_with(self) -> list[dict[str, Any]]:
"""
Get list of users with which this form is shared
"""
if not frappe.has_permission("Form", "read", self.name):
frappe.throw(_("You do not have read access to this form"))
users_shared_with = get_users(self.doctype, self.name)
return users_shared_with🤖 Prompt for AI Agents |
||
|
|
||
| def generate_initial_route(self) -> str: | ||
| return "s/forms_pro_" + frappe.utils.random_string(8) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,16 @@ | |
|
|
||
| # import frappe | ||
| from frappe.model.document import Document | ||
| from frappe.utils import cached_property | ||
| from pydantic import BaseModel, EmailStr | ||
|
|
||
| from forms_pro.api.user import get_user | ||
|
|
||
|
|
||
| class GetTeamMembersResponse(BaseModel): | ||
| full_name: str | ||
| user_image: str | None | ||
| email: EmailStr | ||
|
|
||
|
|
||
| class FPTeam(Document): | ||
|
|
@@ -20,15 +30,25 @@ class FPTeam(Document): | |
| users: DF.TableMultiSelect[FPTeamMember] | ||
| # end: auto-generated types | ||
|
|
||
| @property | ||
| def team_members(self) -> list[str]: | ||
| @cached_property | ||
| def team_members(self) -> list[GetTeamMembersResponse]: | ||
| """ | ||
| 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 [] | ||
| if not len(self.users): | ||
| return [] | ||
|
|
||
| members = [] | ||
|
|
||
| for member in self.users: | ||
| _user = get_user(member.user) | ||
| _user["email"] = member.user | ||
| members.append(GetTeamMembersResponse.model_validate(_user).model_dump()) | ||
|
|
||
| return members | ||
|
Comment on lines
+33
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type annotation mismatch: returns list[dict], not list[GetTeamMembersResponse]. Line 34 declares the return type as 🔎 Fix return type annotation @cached_property
-def team_members(self) -> list[GetTeamMembersResponse]:
+def team_members(self) -> list[dict]:
"""
Get the list of team members🤖 Prompt for AI Agents |
||
|
|
||
| def is_team_member(self, user: str) -> bool: | ||
| """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| Forms Pro | ||
| Forms Pro | ||
| User Forms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: BuildWithHussain/forms_pro
Length of output: 1160
🏁 Script executed:
Repository: BuildWithHussain/forms_pro
Length of output: 353
🏁 Script executed:
Repository: BuildWithHussain/forms_pro
Length of output: 1781
Add
tsconfig.jsonbefore enabling TypeScript support in pre-commit hooks.The hook changes themselves are syntactically correct, but verification reveals that
tsconfig.jsonis missing from the project. Adding TypeScript file processing to prettier and eslint without a TypeScript configuration will lead to inconsistent behavior and potential build failures. Create atsconfig.jsonin the repository root before these changes take effect. Alternatively, confirm whether TypeScript is actually intended to be used in this project—if not, these hook changes should not be merged.