feat: team management pages#46
Merged
Merged
Conversation
- Introduced a new home dashboard layout with a dedicated Dashboard.vue component. - Migrated existing dashboard functionality from the previous Dashboard.vue to the new home structure. - Added sidebar items management through a new sidebarItems.ts file for better organization. - Updated TeamSwitcher component to conditionally render based on the current team. - Removed unused code and components to streamline the application.
…ents - Wrapped the slot in BaseLayout with a div for improved layout consistency. - Removed unnecessary padding class from the Dashboard component for a cleaner design. - Ensured consistent formatting in watch options for better readability.
- Added functionality to remove members from a team and toggle their edit permissions. - Introduced `remove_member_from_team` and `toggle_can_edit_team` API endpoints. - Created `RemoveMemberDialog` component for user confirmation before removal. - Updated `TeamMemberList` to include removal actions and permission toggling. - Enhanced `FPTeam` model to manage team member permissions effectively.
- Implemented a new API endpoint `save` to update team fields, allowing modifications to `team_name` and `logo`. - Created `ManageTeamHeader` component for editing team name and logo upload functionality. - Integrated the new save functionality into the team store for seamless updates. - Updated `ManageTeam` page to include the new header component for enhanced team management.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds email-based team invitations with invitation lifecycle hooks, member permission toggles, safe team field updates, and member removal. Extends FPTeam to include per-member metadata and revocation on removal. Adds backend API endpoints, frontend team management UI and store methods, integration tests, and routing/layout adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant InviteUI as Invite Dialog
participant API as forms_pro Team API
participant Frappe as Frappe (DB / Doc)
participant Hook as Invitation Hook
participant Email as Email Service
User->>InviteUI: enter emails & send
InviteUI->>API: POST invite_team_members(team_id, emails)
API->>API: validate permissions
API->>Frappe: create UserInvitation(s) with redirect containing team_id
Frappe->>Hook: after_insert invoked
Hook->>Frappe: rewrite redirect_to_path to include invite_id (API route)
Frappe->>Email: send invitation emails
Email-->>User: invitation link
Note over User,Email: Invited user clicks link
User->>API: GET add_member_to_team_via_invitation?invite_id=...
API->>API: validate invitation accepted & user exists
API->>Frappe: add user to FP Team (FPTeam.add_to_team)
API->>Hook: after_accept invoked
Hook->>Frappe: set current team for user, adjust redirect_to_path
API-->>User: redirect to /forms
sequenceDiagram
participant Manager as Team Manager UI
participant Store as Team Store
participant API as forms_pro Team API
participant DB as Database / DocShare
Manager->>Store: initialize() / load members
Store->>API: GET get_team_members(team_id)
API->>DB: fetch members + DocShare permissions
DB-->>API: members with write/share flags
API-->>Store: TeamMember[] (includes can_edit_team, is_owner)
Store-->>Manager: render list
Manager->>Store: toggleEditPermissionForMember(email)
Store->>API: POST toggle_can_edit_team(team_id, email)
API->>DB: update DocShare write flag
DB-->>API: success
API-->>Store: success
Store->>API: refresh get_team_members
Store-->>Manager: updated UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Removed unused `call` import from the team store file to streamline dependencies.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (9)
frontend/src/layouts/BaseLayout.vue (1)
50-52: Potential redundant layout constraints with nested routes.The new wrapper adds
p-4padding andmax-w-screen-lgconstraints to all slot content. However,ManageForm.vue(atfrontend/src/pages/manage/ManageForm.vue:3-4) already applies its ownmax-w-screen-lg mx-autocontainer. This creates:
- Redundant
max-w-screen-lg(harmless but unnecessary)- Additional
p-4padding around ManageForm's content that may not be intendedConsider whether ManageForm needs to adjust its own container styling, or whether BaseLayout should conditionally apply these constraints.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/layouts/BaseLayout.vue` around lines 50 - 52, BaseLayout.vue currently wraps all page slots with a div using "p-4 flex-1 max-w-screen-lg mx-auto", which duplicates the container styling and adds unwanted padding for pages like ManageForm.vue that already define "max-w-screen-lg mx-auto"; update BaseLayout.vue to avoid forcing both max-width and padding onto all children — either remove "max-w-screen-lg mx-auto" from the wrapper and keep only page-agnostic spacing (e.g., "flex-1") or make the wrapper conditional (e.g., add a prop or use a class binding like hasContainer) so pages such as ManageForm.vue can opt out and avoid redundant max-width and p-4 padding while preserving layout for other pages.frontend/src/components/team/RemoveMemberDialog.vue (1)
10-13: Consider awaiting async operation before closing dialog.If
removeMemberFromTeamis async, the dialog closes immediately without confirming success. Consider awaiting the call and handling potential errors to provide user feedback.♻️ Suggested improvement
-function removeMember() { - teamStore.removeMemberFromTeam(memberEmail.value.email); - open.value = false; +async function removeMember() { + try { + await teamStore.removeMemberFromTeam(memberEmail.value.email); + open.value = false; + } catch (error) { + // Handle error - show toast notification + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/team/RemoveMemberDialog.vue` around lines 10 - 13, The removeMember function currently calls teamStore.removeMemberFromTeam(memberEmail.value.email) and immediately closes the dialog (open.value = false); if removeMemberFromTeam is async, await it and only close the dialog after success, handling errors to show feedback. Update removeMember to be async, await teamStore.removeMemberFromTeam(memberEmail.value.email), set open.value = false on success, and handle/rethrow errors (e.g., set an error state or show a notification) so failures don’t silently close the dialog; also consider disabling the dialog controls or using a loading flag while the call is in flight.frontend/src/pages/home/Home.vue (1)
5-6: RedundantuserStore.initialize()call.
App.vuealready callsuserStore.initialize()at the top level (seefrontend/src/App.vue:6-7), which executes for all routes. This duplicate call in Home.vue is unnecessary and triggers redundant API requests sinceinitialize()doesn't guard against multiple invocations.♻️ Remove redundant initialization
<script setup lang="ts"> import BaseLayout from "@/layouts/BaseLayout.vue"; -import { useUser } from "@/stores/user"; import { useSidebarItems } from "./sidebarItems"; -const userStore = useUser(); -userStore.initialize(); const sidebarItems = useSidebarItems(); </script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/home/Home.vue` around lines 5 - 6, The Home.vue component calls userStore.initialize() redundantly; since App.vue already invokes useUser().initialize() globally, remove the duplicate call in Home.vue (the lines referencing useUser and initialize) to prevent repeated API requests; simply rely on the existing initialization in App.vue and delete the userStore.initialize() invocation (keep any local useUser() usage only if you still need the store reference without re-initializing).forms_pro/forms_pro/doctype/fp_team/fp_team.py (1)
54-56: Handle case when DocShare doesn't exist.If
get_share_namereturnsNone(no share exists),frappe.db.get_value("DocShare", None, "write")will returnNone. While this is safe (evaluates to falsy forcan_edit_team), it's inconsistent with the defensive pattern used informs_pro/api/team.py:169-171which explicitly checks for None.♻️ Suggested defensive check
share_name = get_share_name(doctype="FP Team", name=self.name, user=member.user, everyone=0) - _user["can_edit_team"] = frappe.db.get_value("DocShare", share_name, "write") + _user["can_edit_team"] = bool(share_name and frappe.db.get_value("DocShare", share_name, "write")) _user["is_owner"] = self.owner == member.user🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forms_pro/forms_pro/doctype/fp_team/fp_team.py` around lines 54 - 56, The current code calls frappe.db.get_value("DocShare", share_name, "write") without checking whether share_name (from get_share_name(doctype="FP Team", name=self.name, user=member.user, everyone=0)) is None; update the block in the FP Team code so you first check if share_name is truthy and only then call frappe.db.get_value to set _user["can_edit_team"], otherwise set _user["can_edit_team"] = False; keep the existing _user["is_owner"] = self.owner == member.user assignment unchanged.frontend/src/components/team/TeamSwitcher.vue (1)
59-64: Inconsistent null-safety patterns within the guarded block.Line 59 uses optional chaining (
?.) while line 64 uses non-null assertion (!). Both are safe since they're inside thev-if="userStore.currentTeam"guard, but the inconsistency reduces readability. Consider using either pattern consistently.♻️ Suggested fix for consistency
<TeamLogo v-if="isSidebarCollapsed" - :team-name="userStore.currentTeam?.team_name" + :team-name="userStore.currentTeam!.team_name" :logo-url="userStore.currentTeam?.logo ?? null" />Or use optional chaining on line 64 as well:
<TeamSwitcherItem - :label="userStore.currentTeam!.team_name" + :label="userStore.currentTeam?.team_name ?? ''" :logo-url="userStore.currentTeam?.logo ?? null" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/team/TeamSwitcher.vue` around lines 59 - 64, The code mixes optional chaining and non-null assertion for the same guarded value—userStore.currentTeam—inside the v-if block (used in TeamSwitcherItem props like :team-name/:logo-url and :label). Make the null-safety style consistent: update the TeamSwitcherItem prop that uses userStore.currentTeam! (the :label prop) to use optional chaining (userStore.currentTeam?.team_name) so all accesses inside the v-if guard use the same pattern and improve readability.forms_pro/overrides/invitations.py (1)
44-46: Exact role list comparison may be overly restrictive.The check
role_names != ["Forms Pro User"]will skip processing if the invitation has additional roles beyond "Forms Pro User". If this is intentional (only single-role invitations should be processed), consider adding a comment explaining the design decision.💡 Alternative: Check if role is included instead of exact match
If invitations with additional roles should also be processed:
- if role_names != ["Forms Pro User"]: + if "Forms Pro User" not in role_names: return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forms_pro/overrides/invitations.py` around lines 44 - 46, The current exact equality check using role_names and ["Forms Pro User"] is too strict and will skip invites that include "Forms Pro User" plus other roles; update the logic in the invitations handling (where role_names is derived from doc.roles) to either check membership (ensure "Forms Pro User" is in role_names) if multi-role invites should be processed, or add a clear explanatory comment next to the existing check to document that only single-role invitations are intended to be handled; modify the condition around role_names (and adjust any early return) accordingly so behavior matches the intended design.forms_pro/api/team.py (1)
8-14: Consolidate duplicate import blocks.The imports from
forms_pro.utils.teamsare split across two blocks. Merge them for cleaner code.♻️ Proposed fix
from forms_pro.forms_pro.doctype.fp_team.fp_team import FPTeam, GetTeamMembersResponse -from forms_pro.utils.teams import ( - GetTeamFormsResponseSchema, - set_current_team, -) -from forms_pro.utils.teams import ( - get_team_forms as get_team_forms_utils, -) +from forms_pro.utils.teams import ( + GetTeamFormsResponseSchema, + get_team_forms as get_team_forms_utils, + set_current_team, +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forms_pro/api/team.py` around lines 8 - 14, The two separate import blocks from forms_pro.utils.teams should be merged into a single import statement: replace the two imports that reference GetTeamFormsResponseSchema, set_current_team and get_team_forms (aliased as get_team_forms_utils) with one consolidated import line that imports GetTeamFormsResponseSchema, set_current_team, and get_team_forms as get_team_forms_utils together to remove duplication and tidy the top of the module.frontend/src/stores/team.ts (2)
79-98:asynckeyword is misleading since the function doesn't await or return a Promise.The
savefunction is declaredasyncbut callscreateResource(...).submit()without awaiting. The function effectively returnsvoidimmediately while the request executes in the background. Either removeasyncto match the actual behavior, or properly await the submission if callers need to know when the save completes.♻️ Option 1: Remove async (match current behavior)
- async function save(fields: { team_name?: string; logo?: string }) { + function save(fields: { team_name?: string; logo?: string }) { createResource({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/team.ts` around lines 79 - 98, The save function is marked async but doesn't await or return the createResource(...).submit() Promise, so either remove the async keyword from save to reflect the fire-and-forget behavior, or make save return/await the submission by returning the Promise from createResource(...).submit() (and keep async if you await) so callers can await completion; update the save declaration and ensure createResource(...).submit() is returned/awaited accordingly (refer to the save function and the createResource(...).submit() call).
68-70: Inconsistent use ofreload()vsfetch()for refreshing team members.
removeMemberFromTeamusesteamMembersResource.reload()(line 69) whiletoggleEditPermissionForMemberandsaveuseteamMembersResource.fetch()(lines 48, 91). For consistency and predictability, use the same method throughout.♻️ Proposed fix for consistency
onSuccess() { - teamMembersResource.reload(); + teamMembersResource.fetch(); toast.success("Member removed from team"); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/team.ts` around lines 68 - 70, The onSuccess handler in removeMemberFromTeam currently calls teamMembersResource.reload(), which is inconsistent with toggleEditPermissionForMember and save that call teamMembersResource.fetch(); update removeMemberFromTeam's onSuccess to call teamMembersResource.fetch() instead of reload() so all refreshes use the same method (ensure any returned promise handling remains the same).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@forms_pro/api/team.py`:
- Around line 204-219: Prevent accidental removal of a team owner by adding an
ownership guard: in the API function remove_member_from_team and/or the
FPTeam.remove_from_team method, detect if the target member_email corresponds to
a member with is_owner (or equivalent owner flag) on the FPTeam and if so raise
a permission/validation error instead of removing them; update
remove_member_from_team to check team.get("members") or call a new
FPTeam.is_owner(member_email) helper before invoking team.remove_from_team, and
add the same owner-check inside FPTeam.remove_from_team to enforce safety at the
model level.
- Around line 153-178: The toggle_can_edit_team function currently flips a
member's write/share on the DocShare even if that member is the team owner;
fetch the FP Team doc (frappe.get_doc("FP Team", team_id)) and check its owner
field, and if team.owner == member_email then prevent the toggle by raising a
frappe.PermissionError (or return a clear error) before calling
get_share_name/frappe.get_doc("DocShare", ...); ensure this check occurs after
the initial permission check and before mutating the DocShare so the owner
always retains full permissions.
- Around line 118-150: add_member_to_team_via_invitation can raise
DuplicateEntryError if the after_accept hook already added the user; update this
function to guard or handle that race by either checking membership before
calling team.add_to_team (use FPTeam methods/fields to see if invite.email is
already in team) or wrap team.add_to_team(invite.email) in a try/except that
catches DuplicateEntryError and treats it as success, then continue with
team.save(ignore_permissions=True) and set_current_team; reference
add_member_to_team_via_invitation, team.add_to_team, DuplicateEntryError and
FPTeam when making the change.
In `@forms_pro/forms_pro/doctype/fp_team/fp_team.py`:
- Around line 105-120: Add a guard to prevent removing the team owner: in the
doctype method remove_from_team check whether the passed user equals the team
document owner (self.owner or the owner field on the FP Team doc) and raise a
validation/permission error instead of removing them; mirror the same check in
the API function remove_member_from_team (validate the user against the
team.owner before calling remove_from_team) and return an appropriate
HTTP/validation error (e.g., raise a ValidationError or return 400) so owner
removal is blocked at both the model and API layers.
In `@forms_pro/tests/test_invitations.py`:
- Around line 85-100: The _make_accepted_invitation function declares an unused
team_id parameter; remove team_id from the function signature and all call sites
so the method only accepts (invitee_email, owner), or alternatively (if team
handling is required) set "redirect_to_path" using the provided team_id (e.g.,
f"/forms?team={team_id}") inside _make_accepted_invitation; pick one approach
and update the function signature and every call to _make_accepted_invitation
accordingly to keep caller/callee signatures consistent.
In `@frontend/src/components/team/InviteMemberDialog.vue`:
- Around line 49-51: The params object construction uses team.currentTeam
without guarding for null, so ensure the submit/action handler in
InviteMemberDialog.vue (the place building params with team_id:
team.currentTeam?.name and emails: inviteEmails.value) checks that
team.currentTeam is non-null before calling the API: if null, return early or
surface a user-facing error/validation and disable the submit flow; only build
params and proceed with the request when team.currentTeam exists (or provide a
fallback identifier) to avoid sending undefined team_id.
- Around line 44-64: sendInvitationEmails currently only creates the useCall
resource but never executes it, so the API is never invoked; update
sendInvitationEmails to trigger the request after creating the resource (e.g.
call the returned resource's .submit() or .fetch() method) or pass immediate:
true to useCall if supported, ensuring the call uses the same options (baseUrl,
url, method, params with team.currentTeam?.name and inviteEmails.value) and that
onSuccess/onError handlers remain unchanged.
In `@frontend/src/components/team/ManageTeamHeader.vue`:
- Around line 32-36: The Upload/Change Button currently has no click handler or
upload flow; wire it to a hidden file input and implement an upload sequence
that reads the selected file, uploads it to your storage service, obtains the
resulting file URL, and then calls teamStore.save({ logo: fileUrl }) to persist
it; specifically, add a ref'd <input type="file"> triggered by Button's click,
implement an onFileSelected handler to upload the file (or call existing upload
utility), await the returned URL, call teamStore.save({ logo: fileUrl }) and
handle success/failure (show loading state and error feedback) while keeping the
Button label logic that uses teamStore.currentTeam?.logo.
- Around line 27-31: The code uses a non-null assertion on
teamStore.currentTeam.team_name which can throw if currentTeam is null; change
the usage in the TeamLogo props to safely handle a missing team by replacing the
non-null assertion with optional chaining and a fallback (e.g., use
teamStore.currentTeam?.team_name ?? '' for the team-name prop) and similarly
ensure logo uses teamStore.currentTeam?.logo ?? null so both props are safe when
teamStore.currentTeam is null; update references to TeamLogo,
teamStore.currentTeam, team_name, and logo accordingly.
In `@frontend/src/components/team/RemoveMemberDialog.vue`:
- Around line 34-36: The onClick handler in RemoveMemberDialog.vue is
reassigning the ref variable (open = false) instead of updating its value;
change the handler to set open.value = false so the dialog actually closes
(match the usage on line 12), and scan other click handlers in this component to
ensure any ref assignments use .value (e.g., the onClick callback that currently
uses open = false).
In `@frontend/src/components/team/TeamMemberList.vue`:
- Around line 39-42: The template currently passes memberToRemove with a wrong
type assertion and a misleading prop name; update the binding to use a clear
prop name and remove the unsafe assertion: change the component usage to bind
the full object (e.g. v-model:member="memberToRemove") instead of
v-model:member-email="memberToRemove as TeamMember" and keep
v-model="openRemoveMemberDialog". Then update RemoveMemberDialog's prop
definition (prop name: member) to accept TeamMember | null (or make it optional
and not required) and handle null checks inside the component before accessing
properties like full_name so opening the dialog with a null value is safe.
In `@frontend/src/pages/home/Dashboard.vue`:
- Around line 101-113: Wrap the async call in handleCreateDraftFormWithDoctype
with a try-catch to surface API failures from createNewFormWithDoctype; in the
try block await createNewFormWithDoctype(...) and keep the existing router.push
to "Edit Form" using data.form_document, and in the catch call toast.error with
the actual error message. Also fix the typo and broaden the pre-check error
message: when !selectedDoctype.value || !user.currentTeam?.name, change
toast.error to a clear sentence (e.g., "Error occurred while creating form.
Ensure a DocType is selected and a team is available.") so it references both
DocType and currentTeam. Ensure you reference the existing symbols
handleCreateDraftFormWithDoctype, selectedDoctype.value, user.currentTeam?.name,
createNewFormWithDoctype, toast.error, and router.push.
- Around line 115-127: handleCreateDraftForm currently lacks error handling and
has a typo in the toast message; wrap the body of handleCreateDraftForm in a
try-catch, correct the message from "Error occured..." to "Error occurred...",
await createNewForm(user.currentTeam.name) inside the try, validate that the
returned data has data.form_document before calling router.push (otherwise show
a toast.error with the caught error or a useful fallback message), and log or
include the caught error details in the toast to aid debugging; reference
handleCreateDraftForm, createNewForm, router.push and toast.error when applying
these changes.
In `@frontend/src/stores/team.ts`:
- Line 4: The import statement currently pulls in an unused symbol `call` from
"frappe-ui" which causes a TypeScript build error; edit the import in this
module (the line importing useCall, createResource, call) to remove `call` so it
only imports the used symbols (`useCall` and `createResource`), then run the
type check/build to confirm the unused-import error is resolved.
---
Nitpick comments:
In `@forms_pro/api/team.py`:
- Around line 8-14: The two separate import blocks from forms_pro.utils.teams
should be merged into a single import statement: replace the two imports that
reference GetTeamFormsResponseSchema, set_current_team and get_team_forms
(aliased as get_team_forms_utils) with one consolidated import line that imports
GetTeamFormsResponseSchema, set_current_team, and get_team_forms as
get_team_forms_utils together to remove duplication and tidy the top of the
module.
In `@forms_pro/forms_pro/doctype/fp_team/fp_team.py`:
- Around line 54-56: The current code calls frappe.db.get_value("DocShare",
share_name, "write") without checking whether share_name (from
get_share_name(doctype="FP Team", name=self.name, user=member.user, everyone=0))
is None; update the block in the FP Team code so you first check if share_name
is truthy and only then call frappe.db.get_value to set _user["can_edit_team"],
otherwise set _user["can_edit_team"] = False; keep the existing
_user["is_owner"] = self.owner == member.user assignment unchanged.
In `@forms_pro/overrides/invitations.py`:
- Around line 44-46: The current exact equality check using role_names and
["Forms Pro User"] is too strict and will skip invites that include "Forms Pro
User" plus other roles; update the logic in the invitations handling (where
role_names is derived from doc.roles) to either check membership (ensure "Forms
Pro User" is in role_names) if multi-role invites should be processed, or add a
clear explanatory comment next to the existing check to document that only
single-role invitations are intended to be handled; modify the condition around
role_names (and adjust any early return) accordingly so behavior matches the
intended design.
In `@frontend/src/components/team/RemoveMemberDialog.vue`:
- Around line 10-13: The removeMember function currently calls
teamStore.removeMemberFromTeam(memberEmail.value.email) and immediately closes
the dialog (open.value = false); if removeMemberFromTeam is async, await it and
only close the dialog after success, handling errors to show feedback. Update
removeMember to be async, await
teamStore.removeMemberFromTeam(memberEmail.value.email), set open.value = false
on success, and handle/rethrow errors (e.g., set an error state or show a
notification) so failures don’t silently close the dialog; also consider
disabling the dialog controls or using a loading flag while the call is in
flight.
In `@frontend/src/components/team/TeamSwitcher.vue`:
- Around line 59-64: The code mixes optional chaining and non-null assertion for
the same guarded value—userStore.currentTeam—inside the v-if block (used in
TeamSwitcherItem props like :team-name/:logo-url and :label). Make the
null-safety style consistent: update the TeamSwitcherItem prop that uses
userStore.currentTeam! (the :label prop) to use optional chaining
(userStore.currentTeam?.team_name) so all accesses inside the v-if guard use the
same pattern and improve readability.
In `@frontend/src/layouts/BaseLayout.vue`:
- Around line 50-52: BaseLayout.vue currently wraps all page slots with a div
using "p-4 flex-1 max-w-screen-lg mx-auto", which duplicates the container
styling and adds unwanted padding for pages like ManageForm.vue that already
define "max-w-screen-lg mx-auto"; update BaseLayout.vue to avoid forcing both
max-width and padding onto all children — either remove "max-w-screen-lg
mx-auto" from the wrapper and keep only page-agnostic spacing (e.g., "flex-1")
or make the wrapper conditional (e.g., add a prop or use a class binding like
hasContainer) so pages such as ManageForm.vue can opt out and avoid redundant
max-width and p-4 padding while preserving layout for other pages.
In `@frontend/src/pages/home/Home.vue`:
- Around line 5-6: The Home.vue component calls userStore.initialize()
redundantly; since App.vue already invokes useUser().initialize() globally,
remove the duplicate call in Home.vue (the lines referencing useUser and
initialize) to prevent repeated API requests; simply rely on the existing
initialization in App.vue and delete the userStore.initialize() invocation (keep
any local useUser() usage only if you still need the store reference without
re-initializing).
In `@frontend/src/stores/team.ts`:
- Around line 79-98: The save function is marked async but doesn't await or
return the createResource(...).submit() Promise, so either remove the async
keyword from save to reflect the fire-and-forget behavior, or make save
return/await the submission by returning the Promise from
createResource(...).submit() (and keep async if you await) so callers can await
completion; update the save declaration and ensure createResource(...).submit()
is returned/awaited accordingly (refer to the save function and the
createResource(...).submit() call).
- Around line 68-70: The onSuccess handler in removeMemberFromTeam currently
calls teamMembersResource.reload(), which is inconsistent with
toggleEditPermissionForMember and save that call teamMembersResource.fetch();
update removeMemberFromTeam's onSuccess to call teamMembersResource.fetch()
instead of reload() so all refreshes use the same method (ensure any returned
promise handling remains the same).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a90b8d64-e06b-4520-bec5-5a5f21162a29
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
forms_pro/api/team.pyforms_pro/forms_pro/doctype/fp_team/fp_team.pyforms_pro/hooks.pyforms_pro/overrides/invitations.pyforms_pro/overrides/roles.pyforms_pro/tests/test_invitations.pyfrontend/package.jsonfrontend/src/components/team/InviteMemberDialog.vuefrontend/src/components/team/ManageTeamHeader.vuefrontend/src/components/team/RemoveMemberDialog.vuefrontend/src/components/team/TeamMemberList.vuefrontend/src/components/team/TeamSwitcher.vuefrontend/src/layouts/BaseLayout.vuefrontend/src/pages/Dashboard.vuefrontend/src/pages/home/Dashboard.vuefrontend/src/pages/home/Home.vuefrontend/src/pages/home/sidebarItems.tsfrontend/src/pages/manage/ManageForm.vuefrontend/src/pages/team/ManageTeam.vuefrontend/src/router.tsfrontend/src/stores/team.ts
💤 Files with no reviewable changes (1)
- frontend/src/pages/Dashboard.vue
Comment on lines
+204
to
+219
| @frappe.whitelist(methods=["POST"]) | ||
| def remove_member_from_team(team_id: str, member_email: str) -> None: | ||
| """ | ||
| Remove a member from a team | ||
| """ | ||
|
|
||
| if not frappe.has_permission( | ||
| doctype="FP Team", | ||
| ptype="write", | ||
| doc=team_id, | ||
| user=frappe.session.user, | ||
| ): | ||
| raise frappe.PermissionError("You do not have permission to remove a member from this team") | ||
|
|
||
| team: FPTeam = frappe.get_doc("FP Team", team_id) | ||
| team.remove_from_team(member_email) |
There was a problem hiding this comment.
No guard prevents removing the team owner.
The remove_from_team method (per fp_team.py:105-120) has no owner check. A member with write permission could remove the team owner, leaving the team in an inconsistent state where is_owner checks would fail for all remaining members.
Add a guard to prevent owner removal:
🛡️ Proposed fix
`@frappe.whitelist`(methods=["POST"])
def remove_member_from_team(team_id: str, member_email: str) -> None:
"""
Remove a member from a team
"""
if not frappe.has_permission(
doctype="FP Team",
ptype="write",
doc=team_id,
user=frappe.session.user,
):
raise frappe.PermissionError("You do not have permission to remove a member from this team")
team: FPTeam = frappe.get_doc("FP Team", team_id)
+ if team.owner == member_email:
+ raise frappe.PermissionError("Cannot remove the team owner")
team.remove_from_team(member_email)🧰 Tools
🪛 Ruff (0.15.4)
[warning] 216-216: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@forms_pro/api/team.py` around lines 204 - 219, Prevent accidental removal of
a team owner by adding an ownership guard: in the API function
remove_member_from_team and/or the FPTeam.remove_from_team method, detect if the
target member_email corresponds to a member with is_owner (or equivalent owner
flag) on the FPTeam and if so raise a permission/validation error instead of
removing them; update remove_member_from_team to check team.get("members") or
call a new FPTeam.is_owner(member_email) helper before invoking
team.remove_from_team, and add the same owner-check inside
FPTeam.remove_from_team to enforce safety at the model level.
Comment on lines
+105
to
+120
| def remove_from_team(self, user: str) -> None: | ||
| """ | ||
| Remove a user from the team | ||
|
|
||
| Args: | ||
| user: The user email address | ||
| """ | ||
|
|
||
| self.users = [member for member in self.users if member.user != user] | ||
| self.save() | ||
| remove_docshare( | ||
| doctype="FP Team", | ||
| name=self.name, | ||
| user=user, | ||
| flags={"ignore_permissions": True}, | ||
| ) |
There was a problem hiding this comment.
Missing guard to prevent owner removal.
The remove_from_team method doesn't prevent removing the team owner. This could orphan the team with no valid owner, breaking ownership-based permission checks. The API endpoint remove_member_from_team (at forms_pro/api/team.py:204-219) also lacks this validation.
🛡️ Proposed fix
def remove_from_team(self, user: str) -> None:
"""
Remove a user from the team
Args:
user: The user email address
"""
+ if user == self.owner:
+ frappe.throw(
+ frappe._("Cannot remove the team owner"),
+ frappe.ValidationError,
+ )
self.users = [member for member in self.users if member.user != user]
self.save()📝 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
| def remove_from_team(self, user: str) -> None: | |
| """ | |
| Remove a user from the team | |
| Args: | |
| user: The user email address | |
| """ | |
| self.users = [member for member in self.users if member.user != user] | |
| self.save() | |
| remove_docshare( | |
| doctype="FP Team", | |
| name=self.name, | |
| user=user, | |
| flags={"ignore_permissions": True}, | |
| ) | |
| def remove_from_team(self, user: str) -> None: | |
| """ | |
| Remove a user from the team | |
| Args: | |
| user: The user email address | |
| """ | |
| if user == self.owner: | |
| frappe.throw( | |
| frappe._("Cannot remove the team owner"), | |
| frappe.ValidationError, | |
| ) | |
| self.users = [member for member in self.users if member.user != user] | |
| self.save() | |
| remove_docshare( | |
| doctype="FP Team", | |
| name=self.name, | |
| user=user, | |
| flags={"ignore_permissions": True}, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@forms_pro/forms_pro/doctype/fp_team/fp_team.py` around lines 105 - 120, Add a
guard to prevent removing the team owner: in the doctype method remove_from_team
check whether the passed user equals the team document owner (self.owner or the
owner field on the FP Team doc) and raise a validation/permission error instead
of removing them; mirror the same check in the API function
remove_member_from_team (validate the user against the team.owner before calling
remove_from_team) and return an appropriate HTTP/validation error (e.g., raise a
ValidationError or return 400) so owner removal is blocked at both the model and
API layers.
Comment on lines
+101
to
+113
| const handleCreateDraftFormWithDoctype = async () => { | ||
| if (!selectedDoctype.value || !user.currentTeam?.name) { | ||
| toast.error("Error occured while creating form. Check DocType Selected."); | ||
| return; | ||
| } | ||
| const data = await createNewFormWithDoctype(selectedDoctype.value, user.currentTeam.name); | ||
| router.push({ | ||
| name: "Edit Form", | ||
| params: { | ||
| id: data.form_document, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Missing error handling and typo in error message.
- The async call to
createNewFormWithDoctypelacks try-catch. If the API call fails, the error won't be displayed to the user. - Typo: "occured" → "occurred".
- The error message only mentions DocType but the guard also checks for
currentTeam.
🐛 Suggested fix with error handling
const handleCreateDraftFormWithDoctype = async () => {
if (!selectedDoctype.value || !user.currentTeam?.name) {
- toast.error("Error occured while creating form. Check DocType Selected.");
+ toast.error("Error occurred while creating form. Please ensure a DocType is selected and a team exists.");
return;
}
- const data = await createNewFormWithDoctype(selectedDoctype.value, user.currentTeam.name);
- router.push({
- name: "Edit Form",
- params: {
- id: data.form_document,
- },
- });
+ try {
+ const data = await createNewFormWithDoctype(selectedDoctype.value, user.currentTeam.name);
+ router.push({
+ name: "Edit Form",
+ params: {
+ id: data.form_document,
+ },
+ });
+ } catch (error) {
+ toast.error("Failed to create form", {
+ description: error instanceof Error ? error.message : "Unknown error",
+ });
+ }
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/home/Dashboard.vue` around lines 101 - 113, Wrap the async
call in handleCreateDraftFormWithDoctype with a try-catch to surface API
failures from createNewFormWithDoctype; in the try block await
createNewFormWithDoctype(...) and keep the existing router.push to "Edit Form"
using data.form_document, and in the catch call toast.error with the actual
error message. Also fix the typo and broaden the pre-check error message: when
!selectedDoctype.value || !user.currentTeam?.name, change toast.error to a clear
sentence (e.g., "Error occurred while creating form. Ensure a DocType is
selected and a team is available.") so it references both DocType and
currentTeam. Ensure you reference the existing symbols
handleCreateDraftFormWithDoctype, selectedDoctype.value, user.currentTeam?.name,
createNewFormWithDoctype, toast.error, and router.push.
Comment on lines
+115
to
+127
| const handleCreateDraftForm = async () => { | ||
| if (!user.currentTeam?.name) { | ||
| toast.error("Error occured while creating form. User has no default team."); | ||
| return; | ||
| } | ||
| const data = await createNewForm(user.currentTeam.name); | ||
| router.push({ | ||
| name: "Edit Form", | ||
| params: { | ||
| id: data.form_document, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Missing error handling and typo in error message.
Same issues as handleCreateDraftFormWithDoctype: no try-catch and typo "occured".
🐛 Suggested fix with error handling
const handleCreateDraftForm = async () => {
if (!user.currentTeam?.name) {
- toast.error("Error occured while creating form. User has no default team.");
+ toast.error("Error occurred while creating form. User has no default team.");
return;
}
- const data = await createNewForm(user.currentTeam.name);
- router.push({
- name: "Edit Form",
- params: {
- id: data.form_document,
- },
- });
+ try {
+ const data = await createNewForm(user.currentTeam.name);
+ router.push({
+ name: "Edit Form",
+ params: {
+ id: data.form_document,
+ },
+ });
+ } catch (error) {
+ toast.error("Failed to create form", {
+ description: error instanceof Error ? error.message : "Unknown error",
+ });
+ }
};📝 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
| const handleCreateDraftForm = async () => { | |
| if (!user.currentTeam?.name) { | |
| toast.error("Error occured while creating form. User has no default team."); | |
| return; | |
| } | |
| const data = await createNewForm(user.currentTeam.name); | |
| router.push({ | |
| name: "Edit Form", | |
| params: { | |
| id: data.form_document, | |
| }, | |
| }); | |
| }; | |
| const handleCreateDraftForm = async () => { | |
| if (!user.currentTeam?.name) { | |
| toast.error("Error occurred while creating form. User has no default team."); | |
| return; | |
| } | |
| try { | |
| const data = await createNewForm(user.currentTeam.name); | |
| router.push({ | |
| name: "Edit Form", | |
| params: { | |
| id: data.form_document, | |
| }, | |
| }); | |
| } catch (error) { | |
| toast.error("Failed to create form", { | |
| description: error instanceof Error ? error.message : "Unknown error", | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/home/Dashboard.vue` around lines 115 - 127,
handleCreateDraftForm currently lacks error handling and has a typo in the toast
message; wrap the body of handleCreateDraftForm in a try-catch, correct the
message from "Error occured..." to "Error occurred...", await
createNewForm(user.currentTeam.name) inside the try, validate that the returned
data has data.form_document before calling router.push (otherwise show a
toast.error with the caught error or a useful fallback message), and log or
include the caught error details in the toast to aid debugging; reference
handleCreateDraftForm, createNewForm, router.push and toast.error when applying
these changes.
- Added checks to prevent duplicate team member invitations and ensure team owners cannot have their permissions toggled. - Updated `FPTeam` model to prevent removal of the team owner from the team. - Modified `RemoveMemberDialog` and `TeamMemberList` components to improve member removal functionality and user experience.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
forms_pro/api/team.py (1)
146-150:⚠️ Potential issue | 🟠 MajorMake invitation completion idempotent.
after_acceptcan already add the invitee to the team. In that case this endpoint throwsDuplicateEntryErrorinstead of treating the second execution as success, so the redirect flow can fail on a harmless replay.Suggested fix
- if team.is_team_member(invite.email): - raise frappe.DuplicateEntryError("User is already a member of the team") - - team.add_to_team(invite.email) - team.save(ignore_permissions=True) + if not team.is_team_member(invite.email): + team.add_to_team(invite.email) + team.save(ignore_permissions=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forms_pro/api/team.py` around lines 146 - 150, The endpoint should be idempotent: if team.is_team_member(invite.email) returns true, do not raise frappe.DuplicateEntryError but treat the request as successful (skip adding and proceed), because after_accept may have already added the user; update the logic around team.is_team_member, team.add_to_team, and team.save so existing members are a no-op and the flow continues normally (e.g., skip add_to_team when is_team_member is true or catch and ignore duplicate-entry errors from add_to_team), ensuring the endpoint returns success instead of raising.
🧹 Nitpick comments (1)
forms_pro/tests/test_invitations.py (1)
232-245: Add regression coverage for the invitation replay paths.The happy-path test doesn't cover the two risky branches in
add_member_to_team_via_invitation: reusing an accepted invite after the user is already on the team, and calling the endpoint with a differentteam_idthan the one originally embedded in the invitation. A negative test for each would lock down the intended idempotency and authorization behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@forms_pro/tests/test_invitations.py` around lines 232 - 245, Add two tests to cover the invitation replay branches for add_member_to_team_via_invitation: (1) create an accepted invitation via _make_accepted_invitation, call add_member_to_team_via_invitation once to add the user, then call it again with the same invite_id and assert the user is not duplicated in the "FP Team" users list and that frappe.local.response still indicates a redirect to "/forms"; (2) create an accepted invitation embedding a specific team_id, then call add_member_to_team_via_invitation with a different team_id and assert the call is rejected (e.g., raises the same exception the endpoint uses or sets an error response) to lock down authorization; use existing helpers (_create_user, _create_team, inv_doc.name) and inspect frappe.get_doc("FP Team", team_id) and frappe.local.response for assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@forms_pro/api/team.py`:
- Around line 124-150: The endpoint currently trusts the caller-supplied team_id
and never verifies it against the invitation; fetch the team id from the
accepted invitation (use the invitation field that encodes the target team,
e.g., invite.team or invite.reference_docname) and either reject if the
caller-supplied team_id mismatches or ignore the caller value and use invite's
team id for all subsequent operations (permission check using invite.invited_by,
team lookup via FPTeam using the invite-derived id, and final
team.add_to_team(invite.email)); if the invitation has no team field, add and
validate one on the User Invitation model and enforce the match before calling
team.add_to_team.
- Around line 100-106: The permission check in invite_team_members currently
uses ptype="read", allowing ordinary members to invite despite "Can Add
Members?" being revoked; update the check to require write access by changing
ptype="read" to ptype="write" in the frappe.has_permission call that checks the
"FP Team" document (the block referencing team_id and frappe.session.user) and
adjust the raised PermissionError message if needed to reflect that write
permission is required.
In `@frontend/src/components/team/TeamMemberList.vue`:
- Around line 79-84: The icon-only trash Button in TeamMemberList.vue lacks an
accessible label, so update the Button usage (the element with
`@click`="handleMemberRemoval(row)") to include an explicit accessible label
(e.g., add aria-label or a visually-hidden text prop) that describes the action
and, when possible, includes the member identity (use row.name or row.email) —
e.g., aria-label="Remove member {name}". Ensure this label is present on the
Button component rather than relying solely on the icon so screen readers
announce the button purpose.
In `@frontend/src/stores/team.ts`:
- Around line 79-97: The save function currently only refetches
teamMembersResource on success, so updated team metadata (team_name/logo) won't
propagate; update the onSuccess handler in the save function (the createResource
call) to also refresh the current team metadata—e.g., call the existing team
metadata resource or method (such as currentTeam.fetch(), teamResource.fetch(),
or whichever resource holds team details) after success so the header and team
switcher reflect the new team_name and logo immediately.
---
Duplicate comments:
In `@forms_pro/api/team.py`:
- Around line 146-150: The endpoint should be idempotent: if
team.is_team_member(invite.email) returns true, do not raise
frappe.DuplicateEntryError but treat the request as successful (skip adding and
proceed), because after_accept may have already added the user; update the logic
around team.is_team_member, team.add_to_team, and team.save so existing members
are a no-op and the flow continues normally (e.g., skip add_to_team when
is_team_member is true or catch and ignore duplicate-entry errors from
add_to_team), ensuring the endpoint returns success instead of raising.
---
Nitpick comments:
In `@forms_pro/tests/test_invitations.py`:
- Around line 232-245: Add two tests to cover the invitation replay branches for
add_member_to_team_via_invitation: (1) create an accepted invitation via
_make_accepted_invitation, call add_member_to_team_via_invitation once to add
the user, then call it again with the same invite_id and assert the user is not
duplicated in the "FP Team" users list and that frappe.local.response still
indicates a redirect to "/forms"; (2) create an accepted invitation embedding a
specific team_id, then call add_member_to_team_via_invitation with a different
team_id and assert the call is rejected (e.g., raises the same exception the
endpoint uses or sets an error response) to lock down authorization; use
existing helpers (_create_user, _create_team, inv_doc.name) and inspect
frappe.get_doc("FP Team", team_id) and frappe.local.response for assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff7d8741-0732-4a0c-9e40-ee1e9c8bae6e
📒 Files selected for processing (6)
forms_pro/api/team.pyforms_pro/forms_pro/doctype/fp_team/fp_team.pyforms_pro/tests/test_invitations.pyfrontend/src/components/team/RemoveMemberDialog.vuefrontend/src/components/team/TeamMemberList.vuefrontend/src/stores/team.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/team/RemoveMemberDialog.vue
Comment on lines
+124
to
+150
| invite_id = invite_id or frappe.form_dict.get("id") | ||
| if not invite_id: | ||
| raise frappe.PermissionError("Invitation id is required") | ||
|
|
||
| invite: UserInvitation = frappe.get_doc("User Invitation", invite_id) | ||
|
|
||
| if invite.status != "Accepted": | ||
| raise frappe.PermissionError("Invitation not accepted") | ||
|
|
||
| if not frappe.has_permission( | ||
| doctype="FP Team", | ||
| ptype="read", | ||
| doc=team_id, | ||
| user=invite.invited_by, | ||
| ): | ||
| raise frappe.PermissionError("You do not have permission to add a member to this team") | ||
|
|
||
| if not frappe.db.exists("User", invite.email): | ||
| raise frappe.PermissionError("User not found") | ||
|
|
||
| team: FPTeam = frappe.get_doc("FP Team", team_id) | ||
|
|
||
| if team.is_team_member(invite.email): | ||
| raise frappe.DuplicateEntryError("User is already a member of the team") | ||
|
|
||
| team.add_to_team(invite.email) | ||
| team.save(ignore_permissions=True) |
There was a problem hiding this comment.
Bind invitation completion to the invited team.
This endpoint trusts the caller-supplied team_id and never verifies that it matches the team encoded in the accepted invitation. If the inviter can read multiple teams, the same accepted invite_id can be replayed against a different team_id and add the user to the wrong team. Derive the team from invitation metadata, or reject mismatches before team.add_to_team(...).
🧰 Tools
🪛 Ruff (0.15.4)
[warning] 126-126: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 131-131: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 139-139: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 142-142: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 147-147: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@forms_pro/api/team.py` around lines 124 - 150, The endpoint currently trusts
the caller-supplied team_id and never verifies it against the invitation; fetch
the team id from the accepted invitation (use the invitation field that encodes
the target team, e.g., invite.team or invite.reference_docname) and either
reject if the caller-supplied team_id mismatches or ignore the caller value and
use invite's team id for all subsequent operations (permission check using
invite.invited_by, team lookup via FPTeam using the invite-derived id, and final
team.add_to_team(invite.email)); if the invitation has no team field, add and
validate one on the User Invitation model and enforce the match before calling
team.add_to_team.
Comment on lines
+79
to
+97
| async function save(fields: { team_name?: string; logo?: string }) { | ||
| createResource({ | ||
| url: "forms_pro.api.team.save", | ||
| method: "POST", | ||
| makeParams() { | ||
| return { | ||
| team_id: currentTeam.value?.name, | ||
| fields, | ||
| }; | ||
| }, | ||
| onSuccess() { | ||
| toast.success("Team Details Updated"); | ||
| teamMembersResource.fetch(); | ||
| }, | ||
| onError(error: Error) { | ||
| console.error(error); | ||
| toast.error("Failed to update team " + error.message); | ||
| }, | ||
| }).submit(); |
There was a problem hiding this comment.
Refresh team metadata after save, not just members.
This success path only refetches teamMembersResource. team_name/logo changes won't propagate to the header or team switcher until some other code reloads the current team state, so the user can get a success toast while still seeing stale team details.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/stores/team.ts` around lines 79 - 97, The save function
currently only refetches teamMembersResource on success, so updated team
metadata (team_name/logo) won't propagate; update the onSuccess handler in the
save function (the createResource call) to also refresh the current team
metadata—e.g., call the existing team metadata resource or method (such as
currentTeam.fetch(), teamResource.fetch(), or whichever resource holds team
details) after success so the header and team switcher reflect the new team_name
and logo immediately.
3 tasks
harshtandiya
added a commit
that referenced
this pull request
Apr 19, 2026
* chore: bump lucide-icons * feat: restructure home dashboard and sidebar - Introduced a new home dashboard layout with a dedicated Dashboard.vue component. - Migrated existing dashboard functionality from the previous Dashboard.vue to the new home structure. - Added sidebar items management through a new sidebarItems.ts file for better organization. - Updated TeamSwitcher component to conditionally render based on the current team. - Removed unused code and components to streamline the application. * refactor: enhance layout structure in BaseLayout and Dashboard components - Wrapped the slot in BaseLayout with a div for improved layout consistency. - Removed unnecessary padding class from the Dashboard component for a cleaner design. - Ensured consistent formatting in watch options for better readability. * fix: manageform page layout * feat: team invitations via User Invitations * feat: implement team member removal and permission toggling - Added functionality to remove members from a team and toggle their edit permissions. - Introduced `remove_member_from_team` and `toggle_can_edit_team` API endpoints. - Created `RemoveMemberDialog` component for user confirmation before removal. - Updated `TeamMemberList` to include removal actions and permission toggling. - Enhanced `FPTeam` model to manage team member permissions effectively. * feat: add team details update functionality - Implemented a new API endpoint `save` to update team fields, allowing modifications to `team_name` and `logo`. - Created `ManageTeamHeader` component for editing team name and logo upload functionality. - Integrated the new save functionality into the team store for seamless updates. - Updated `ManageTeam` page to include the new header component for enhanced team management. * refactor: clean up imports in team store - Removed unused `call` import from the team store file to streamline dependencies. * chore: better code - Added checks to prevent duplicate team member invitations and ensure team owners cannot have their permissions toggled. - Updated `FPTeam` model to prevent removal of the team owner from the team. - Modified `RemoveMemberDialog` and `TeamMemberList` components to improve member removal functionality and user experience. * fix: tests * chore: better code
harshtandiya
added a commit
that referenced
this pull request
Apr 19, 2026
* fix: route handling for /form * feat: child table support v1 (#42) * chore: bump frappe-ui to `0.1.262` * feat: add 'Table' option to fieldtype selection - Updated form_field.json and form_field.py to include 'Table' in the list of selectable field types for enhanced functionality. - Adjusted the modified timestamp in form_field.json to reflect recent changes. * feat: implement Table field component and enhance field options handling - Added a new Table component for rendering tabular data input. - Updated RenderField and FieldRenderer components to support dynamic options for Select and Link fields. - Introduced useFieldOptions composable for improved option management. - Enhanced form_fields utility to include Table as a selectable field type. - Updated auto-imports and TypeScript definitions to accommodate new functionality. * feat: enhance Table component fieldtype mapping - Integrated utility function to map fieldtype for Table component, ensuring proper handling of field types. - Defaulted unmapped fieldtypes to "Data" for improved consistency in table rendering. * fix: update TableField component to use the correct Table component - Replaced ListView with Table in the TableField definition to ensure proper rendering of tabular data. - Adjusted imports in form_fields utility to reflect the change in component usage. * refactor: remove onMounted hook from useFieldOptions - Eliminated the onMounted lifecycle hook from the useFieldOptions composable to streamline the loading process of options. - Adjusted imports to reflect the removal of the unused onMounted function. * fix: child table fields fetch * refactor: update TextEditor component bindings - Changed `:model-value` to `:content` for TextEditor components across multiple files to ensure consistency in prop usage. - Added an empty declaration block in auto-imports.d.ts for improved TypeScript support. * feat: team management pages (#46) * chore: bump lucide-icons * feat: restructure home dashboard and sidebar - Introduced a new home dashboard layout with a dedicated Dashboard.vue component. - Migrated existing dashboard functionality from the previous Dashboard.vue to the new home structure. - Added sidebar items management through a new sidebarItems.ts file for better organization. - Updated TeamSwitcher component to conditionally render based on the current team. - Removed unused code and components to streamline the application. * refactor: enhance layout structure in BaseLayout and Dashboard components - Wrapped the slot in BaseLayout with a div for improved layout consistency. - Removed unnecessary padding class from the Dashboard component for a cleaner design. - Ensured consistent formatting in watch options for better readability. * fix: manageform page layout * feat: team invitations via User Invitations * feat: implement team member removal and permission toggling - Added functionality to remove members from a team and toggle their edit permissions. - Introduced `remove_member_from_team` and `toggle_can_edit_team` API endpoints. - Created `RemoveMemberDialog` component for user confirmation before removal. - Updated `TeamMemberList` to include removal actions and permission toggling. - Enhanced `FPTeam` model to manage team member permissions effectively. * feat: add team details update functionality - Implemented a new API endpoint `save` to update team fields, allowing modifications to `team_name` and `logo`. - Created `ManageTeamHeader` component for editing team name and logo upload functionality. - Integrated the new save functionality into the team store for seamless updates. - Updated `ManageTeam` page to include the new header component for enhanced team management. * refactor: clean up imports in team store - Removed unused `call` import from the team store file to streamline dependencies. * chore: better code - Added checks to prevent duplicate team member invitations and ensure team owners cannot have their permissions toggled. - Updated `FPTeam` model to prevent removal of the team owner from the team. - Modified `RemoveMemberDialog` and `TeamMemberList` components to improve member removal functionality and user experience. * fix: tests * chore: better code * fix: team avatar uploader (#47) * feat: Image uploader component with crop * feat: integrate ImageUploader for team logo management - Replaced the existing logo upload button with an ImageUploader component for enhanced functionality. - Added error handling and upload progress display within the ImageUploader. - Updated the button label dynamically based on the upload state and existing logo presence. * feat: enhance TeamSwitcher with search functionality - Added a search input to the TeamSwitcher component, allowing users to filter teams by name. - Updated the team options computation to include search query handling. - Integrated a new search icon and improved layout for better user experience. - Refactored dropdown item templates to accommodate the search input and maintain consistent styling. * refactor: update CreateTeamDialog to use ImageUploader for logo uploads - Replaced FileUploader with ImageUploader component for improved logo management. - Updated type handling for uploaded files to align with new component. - Enhanced error handling and upload progress display in the logo upload section. - Adjusted button label to reflect upload status dynamically. * refactor: update BaseLayout styling and icon usage - Modified the layout classes for improved design consistency and responsiveness. - Changed the logout button icon from a string to a component reference for better integration with the icon library. * feat: add Breadcrumbs component to Dashboard - Integrated Breadcrumbs component into the Dashboard for improved navigation. - Defined breadcrumb items to enhance user context within the application. * feat: submission list pages (#59) * feat: add submissions page and update sidebar navigation - Introduced a new "Manage Form Submissions" page for viewing form submissions. - Updated the router to include a route for submissions. - Refactored sidebar items to include navigation to the new submissions page. - Enhanced the overview page with breadcrumb navigation for better user context. * feat: submissions page v1 * feat: add Drawer component for UI enhancement - Introduced a new Drawer component to provide a sliding panel interface. - Implemented customizable properties for size, position, title, and actions. - Added keyboard accessibility with Escape key support for closing the drawer. - Included transition effects for smooth opening and closing animations. * feat: extend Drawer component with additional size options - Added "xl" size option to the Drawer component for enhanced customization. - Updated size classes for both horizontal and vertical orientations to include new "xl" dimensions. * feat: add submission details view and field value component - Introduced SubmissionDetails component to display detailed information about a specific submission. - Created SubmissionFieldValue component to render individual field values with appropriate formatting based on field type. - Enhanced SubmissionList component to include a drawer for viewing submission details upon row click. - Updated API integration to fetch submission data securely based on user permissions. * feat: enhance SubmissionFieldValue component with dynamic class handling - Added computed property to determine class names based on field type, improving layout for Switch and Checkbox types. - Updated template structure to utilize dynamic classes for better styling and responsiveness. * fix: validate doctype against linked_doctype in get_submission_response A client could pass an arbitrary doctype to access submissions from a different form. Now we fetch both linked_team_id and linked_doctype from the Form doc and reject requests where the supplied doctype doesn't match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: throw when form not found in get_all_submissions If form_id is invalid, frappe.db.get_value returns None and calling has_permission(doc=None) has undefined behaviour. Explicitly throw DoesNotExistError before the permission check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: show dash placeholder for null textarea value in SubmissionFieldValue Textarea was rendering blank when value is null/undefined, inconsistent with the default case which uses value ?? "–". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove unused isLoading ref in SubmissionList isLoading was declared and set but never bound in the template, so the loading state had no effect and was also never cleared on fetch error. Removed it entirely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: show loading state in drawer when formData not yet available If a row is clicked before formData loads, the drawer was empty. Now shows a Loading... placeholder until linked_doctype is available. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: add aria-labelledby to Drawer dialog element Adds id="drawer-title" to the title heading and wires aria-labelledby on the dialog div so assistive tech can announce the drawer title. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: form sharing methods (#58) * fix: form sharing methods - Introduced `add_form_access` and `set_form_permission` functions to manage user permissions on forms. - Updated the frontend to utilize the new API endpoints for adding access and setting permissions. - Enhanced permission validation to ensure only authorized users can share forms. * fix: validate permission_to allowlist and add docstrings to sharing API - Validate `permission_to` against an explicit allowlist in `set_form_permission` to prevent unexpected kwargs from being forwarded to `add_docshare` - Use `int(bool(value))` for safe coercion of the permission value - Expand docstrings on `add_form_access` and `set_form_permission` with full Args/Raises sections and inline comments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: hygine (#60) * chore: update gitignore and pre-commit config for current project structure Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: replace license.txt with LICENSE Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: add community health files and GitHub templates Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: update README with contributing and security references Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: update Python version references to 3.14 across all configs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: set ruff target-version to py313 (py314 not yet supported) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: move vulnerable dependency check to a separate workflow The vulnerable dependency check has been extracted from the linter workflow into its own dedicated workflow file. This change improves organization and allows for more focused execution of dependency checks during pull requests and manual triggers. * chore: update dependabot configuration to use npm instead of yarn * chore(deps): bump actions/cache from 4 to 5 (#61) Bumps [actions/cache](https://github.com/actions/cache) from 4 to 5. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v4...v5) --- updated-dependencies: - dependency-name: actions/cache dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump actions/setup-node from 3 to 6 (#62) Bumps [actions/setup-node](https://github.com/actions/setup-node) from 3 to 6. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@v3...v6) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump actions/setup-python from 4 to 6 (#63) Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4 to 6. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v4...v6) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump pre-commit/action from 3.0.0 to 3.0.1 (#65) Bumps [pre-commit/action](https://github.com/pre-commit/action) from 3.0.0 to 3.0.1. - [Release notes](https://github.com/pre-commit/action/releases) - [Commits](pre-commit/action@v3.0.0...v3.0.1) --- updated-dependencies: - dependency-name: pre-commit/action dependency-version: 3.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump actions/checkout from 3 to 6 (#66) Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump jsdom from 25.0.1 to 29.0.2 in /frontend (#67) Bumps [jsdom](https://github.com/jsdom/jsdom) from 25.0.1 to 29.0.2. - [Release notes](https://github.com/jsdom/jsdom/releases) - [Commits](jsdom/jsdom@v25.0.1...v29.0.2) --- updated-dependencies: - dependency-name: jsdom dependency-version: 29.0.2 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump vue from 3.5.21 to 3.5.32 in /frontend (#68) Bumps [vue](https://github.com/vuejs/core) from 3.5.21 to 3.5.32. - [Release notes](https://github.com/vuejs/core/releases) - [Changelog](https://github.com/vuejs/core/blob/main/CHANGELOG.md) - [Commits](vuejs/core@v3.5.21...v3.5.32) --- updated-dependencies: - dependency-name: vue dependency-version: 3.5.32 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump zod from 4.1.12 to 4.3.6 in /frontend (#69) Bumps [zod](https://github.com/colinhacks/zod) from 4.1.12 to 4.3.6. - [Release notes](https://github.com/colinhacks/zod/releases) - [Commits](colinhacks/zod@v4.1.12...v4.3.6) --- updated-dependencies: - dependency-name: zod dependency-version: 4.3.6 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump socket.io-client from 4.8.1 to 4.8.3 in /frontend (#70) Bumps [socket.io-client](https://github.com/socketio/socket.io) from 4.8.1 to 4.8.3. - [Release notes](https://github.com/socketio/socket.io/releases) - [Changelog](https://github.com/socketio/socket.io/blob/main/CHANGELOG.md) - [Commits](https://github.com/socketio/socket.io/compare/socket.io-client@4.8.1...socket.io-client@4.8.3) --- updated-dependencies: - dependency-name: socket.io-client dependency-version: 4.8.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump postcss from 8.5.6 to 8.5.8 in /frontend (#71) Bumps [postcss](https://github.com/postcss/postcss) from 8.5.6 to 8.5.8. - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](postcss/postcss@8.5.6...8.5.8) --- updated-dependencies: - dependency-name: postcss dependency-version: 8.5.8 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): update frappe-ui to 0.1.272 * chore(deps): update vite to version 5.4.21 and adjust auto-imports and Vite config * refactor(tests): streamline email handling in TestTeamInvitations Removed the use of patching for the sendmail function and replaced it with a flag to mute emails during tests. This change simplifies the setup and teardown process while ensuring that email notifications are suppressed during test execution. Additionally, cleaned up the tearDown method to delete the Email Queue after tests. * refactor(tests): enhance email handling in TestTeamInvitations Reintroduced patching for the sendmail function in the TestTeamInvitations class to ensure email notifications are suppressed during tests. Updated the setUp and tearDown methods to manage the patching process effectively, ensuring compatibility with different Frappe versions. Removed the deletion of the Email Queue in tearDown as it is no longer necessary. * feat: test framework with test factory (#72) * feat(tests): add UserFactory and FPTeamFactory via frappe_factory_bot Introduces a factories package under forms_pro/tests/factories/ with: - UserFactory: generates random User docs; with_forms_pro_role trait includes the role in the initial doc dict so on_update fires once with the role already set - FPTeamFactory: generates FP Team docs with a random team name default Both factories implement __del_override__ as a safety-net cleanup guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(tests): use UserFactory in test_roles Replaces manual frappe.get_doc + Faker boilerplate with UserFactory.create(). Role assignment is kept as a post-insert step since the test specifically exercises the on_update hook that fires when a role is added after creation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(tests): use factories in TestTeamInvitations Replaces _create_user() and _create_team() helpers and the Faker/string tracking lists with UserFactory and FPTeamFactory. Key changes: - _user(*traits) and _team(owner) thin wrappers call the factories and track Document objects instead of email/name strings - Helper signatures updated to accept Document args throughout - UserFactory.build().email used where only a random email is needed (no DB insert required) - All 11 tests pass unchanged in behaviour Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(hooks): add required_apps for frappe_factory_bot Introduces a new required_apps list in hooks.py to include the frappe_factory_bot repository, enabling its integration into the application. This change enhances the app's capabilities by allowing the use of factory methods for testing and development. * ci: fetch frappe_factory_bot before installing forms_pro bench get-app with a local path skips required_apps resolution, so frappe_factory_bot was never fetched. bench install-app then failed with ModuleNotFoundError when Frappe tried to satisfy the required_apps entry. Explicitly get the app before installing forms_pro. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: update README with local test instructions and installation steps for frappe_factory_bot Added a new section to the README detailing how to run tests locally using frappe_factory_bot. Included installation instructions and commands for running all tests as well as specific test modules. * chore: remove del_override * refactor: tests * chore: refactor test_fp_team --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(deps): update devDependencies for Vite and Vitest (#73) Updated the versions of @vitejs/plugin-vue, @vitest/coverage-v8, vite, and vitest in package.json to their latest compatible releases. Removed deprecated dependencies from yarn.lock and updated @bcoe/v8-coverage to version 1.0.2. * feat(e2e): Playwright E2E test suite (#57) (#79) * feat(e2e): Phase 1 — Playwright foundation Install @playwright/test, add test:e2e scripts, create playwright.config.ts with Chromium project, global-setup for Frappe session auth, and a smoke spec that asserts an authenticated user can reach the forms dashboard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(e2e): Phase 2 — data-testid attributes, POMs, and test data fixture Add data-testid to 7 Vue components (form-card, btn-new-form, field-type-*, btn-save-form, btn-publish, btn-submit-form, submission-success, btn-send-invite, input-invite-email). Restructure InviteMemberDialog to use #actions slot so the send button is directly targetable. Create Page Object Models (dashboard, form-builder, submission, team) and a Playwright fixture with API helpers for form create/publish/teardown. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(e2e): Phase 2+3 — semantic selectors and form-creation spec (5 tests) Remove all data-testid attributes from Vue components (reverted to original). Rewrite POMs with semantic selectors: getByRole/getByLabel/getByText, scoped to pre-existing data-form-builder-component attributes. Add form-creation.spec.ts covering dashboard listing, builder load, field canvas, publish, and unpublish. Tests avoid triggering Frappe validation by never saving a field with an empty label; publish/unpublish uses setValue({is_published}) which is always valid. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(e2e): Phase 3+4 — remaining specs, flake fixes, and CI workflow Specs: - form-submission.spec.ts — guest context submission + success message - submission-view.spec.ts — empty state and post-submission row visibility - team-invite.spec.ts — invite dialog open/fill/send flow Fixture: - submitForm fixture for creating a guest submission via API Reliability fixes: - smoke.spec.ts: reload on Frappe 500 under parallel startup load - TeamPage.goto(): waitForLoadState("networkidle") so Vue API calls complete before assertions run - TeamPage.openInviteDialog(): wait for email input to be visible (not just the dialog shell) before returning - playwright.config.ts: retries=1 everywhere; github reporter added CI: - .github/workflows/ui-tests.yml: full Playwright E2E workflow running against a real Frappe bench on every PR and push to develop Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(ui-tests): log bench serve output on startup timeout Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(ui-tests): fix server startup — switch to bench start, add hostname, fix action versions - Replace bench serve (silent crash) with bench start + Procfile (disable watch/schedule) - Add forms.test to /etc/hosts and set host_name config; rename site test_site → forms.test - Swap npx wait-on for a curl loop (no install overhead, same pattern as buzz CI) - Fix non-existent action versions: checkout@v6→@v4, setup-python@v6→@v5, setup-node@v6→@v4 - Add timeout-minutes: 60, fix concurrency group for push events, add workflow_dispatch - Add Playwright browser cache step and bench logs dump on failure Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(ui-tests): setup E2E test data — give Admin Forms Pro role and default team `before_tests()` already added the role to Administrator, but the `on_update` hook that creates the default team checks `has_value_changed` on a freshly-fetched doc (which is always False), so no team was ever created. `_ensure_admin_has_default_team()` closes that gap explicitly. A new "Setup E2E test data" CI step calls `bench execute forms_pro.install.before_tests` before the server starts, so the test user has a team available when fixtures call `get_user_teams`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(form-generator): replace clear_cache() with targeted doctype cache clear frappe.clear_cache() flushes Redis entirely, including session and CSRF token data. In the E2E test suite this causes all POST API calls after the first form creation to fail with CSRFTokenError, because the token captured during global-setup is invalidated server-side. Replace with frappe.clear_document_cache("DocType", name) which only evicts the specific newly-created DocType from the document cache — sufficient for the new schema to be reloaded on next access, and does not touch sessions or CSRF tokens. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(ui-tests): set ignore_csrf=1 for test site Frappe's DocType.on_update() calls frappe.clear_cache() internally whenever a DocType is saved. This flushes session/CSRF data from Redis, invalidating the CSRF token captured in Playwright's storageState.json. All POST API calls after the first form creation then fail with CSRFTokenError. ignore_csrf=1 is the standard Frappe configuration for CI/test sites and skips CSRF validation entirely, which is correct here. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(ui-tests): mute emails so invite test does not need an email account Sets mute_emails=1 in site config so Frappe returns a dummy outgoing account instead of throwing OutgoingEmailError when no real account is configured, allowing the team-invite E2E test to succeed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(ui-tests): run E2E tests as the shared Forms Pro test user - Switch global-setup login from Administrator to FORMS_PRO_TEST_USER (test_forms_pro_user@example.com / testforms123) so E2E tests run under a least-privileged Forms Pro User, matching real-user conditions - Fix create_test_user() to strip the System User role Frappe auto-assigns on insert, leaving only the Forms Pro User role - Set a known password via update_password() so Playwright can log in - Add frontend/e2e/tsconfig.json with @types/node so process.env references in global-setup.ts type-check correctly - Install @types/node dev dependency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: update .gitignore to include Playwright report and skills-lock.json * feat(multiselect): add Multiselect field with registry-driven builder extras (#85) * feat(form-field): add Multiselect fieldtype to doctype and backend mapping * fix(submission): serialize list values to JSON string before DocType insert * feat(multiselect): wire up Multiselect field across the frontend * docs: add add-field skill and register it in CLAUDE.md * chore: move skills to .claude/skills, gitignore .agents, document npx skills workflow * docs: add userinterface-wiki skill reference to CLAUDE.md * feat(fieldTypes): add optional builderExtras component slot to FieldTypeDefinition * refactor(fields): move Multiselect into multiselect/ subfolder, add MultiselectBuilderExtras * feat(fieldTypes): update Multiselect import path and register MultiselectBuilderExtras * refactor(builder): replace hardcoded Multiselect extras with registry-driven builderExtras * feat(multiselect): enhance option addition with error handling and unique validation * feat(multiselect): change layout to description-first (label → desc → checkboxes) * refactor(builder): wrap template in single root div so builderExtras stacks below field * test(e2e): add Playwright spec for Multiselect field builder and submission flow * test(e2e): wait for canvas render and set label before adding options * test(e2e): fix strict mode violation on No options defined locator * test(e2e): wait for Add Option button to confirm field rendered on canvas * test(e2e): ensure Add Option button is visible before adding options * fix(e2e): ensure form is saved before publishing to enable Publish button * fix(e2e): set form title and wait on Save button state Multiselect spec clicked Save while form title was blank — Frappe rejected the save with MandatoryError, Save button never hid, and the publish helper timed out. Fill a title before save so the request succeeds, and in the helper wait for the Save button to disappear (same render cycle as Publish appearing) instead of polling for Publish. * fix(ci): resolve all CI failures on backport branch - Node 18 → 20 in ci.yml and typecheck.yml (@vitejs/plugin-vue@6 requires ≥20.19) - nosemgrep comment on allow_guest=True in get_doctype_fields (intentional for public form rendering) - Migrate form_fields.ts to re-export FIELD_TYPE_DEFINITIONS, fixing Multiselect missing from sidebar - Add Multiselect to FormFieldTypes enum; fix TS2367 comparison in SubmissionFieldValue.vue * fix(tests): use FrappeTestCase for version-15 compatibility IntegrationTestCase was introduced in Frappe v16; version-15 exposes FrappeTestCase from frappe.tests.utils. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests