Sanc 64 update the invitations frontend #47
Conversation
Yemyam
requested review from
JustinTan-1,
Lujarios,
burtonjong,
jason-duong4509,
promatty,
tanvimahal,
themaxboucher and
wesleylui
as code owners
📝 WalkthroughWalkthroughImplements a two-step invite flow (select invite type → show corresponding modal form), adds Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant Dashboard
participant Modal
participant Form
participant Mutation
participant API
Admin->>Dashboard: Click "Invite New"
Dashboard->>Modal: Open type-selection modal
Admin->>Modal: Select "User" or "Agency"
Modal->>Form: Render InviteUserForm or InviteAgencyForm
Admin->>Form: Fill fields and submit
Form->>Mutation: call inviteUserMutation / createOrganization
Mutation->>API: HTTP request (inviteUser / createOrganization)
API-->>Mutation: Response (success/error)
Mutation->>Dashboard: onSuccess -> refetch orgs, close modal
Dashboard->>Admin: Updated UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
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 |
Yemyam
force-pushed
the
SANC-64-update-the-invitations-frontend
branch
from
bb0a87e to
80fc82c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/app/_components/admincomponents/admin-dashboard.tsx`:
- Line 115: Remove the debug console.log by deleting the line
console.log("submit user", submitData); in the admin-dashboard component (where
submitData is handled) inside admin-dashboard.tsx; if persistent logging is
needed replace it with the app's structured logger call (e.g., using
processLogger or a provided logging utility) or handle the data silently by
returning/setting state, but do not leave console.log in the production
component.
- Line 127: Remove the debug console.log call that prints agencyForm.values in
the submit handler; locate the line console.log("submit agency",
agencyForm.values) inside the admin dashboard submit routine (the agency form
submit function in admin-dashboard.tsx) and delete it (or replace with a proper
structured logger if required by project logging conventions), ensuring no
leftover debug output remains.
- Around line 35-45: The onSuccess handler for createAgencyMutation assumes its
parameter `data` is non-null; add a null-check or non-null assertion before
accessing `data.name` in the onSuccess callback of
api.organization.createOrganization.useMutation (the createAgencyMutation
onSuccess handler) — e.g., return early or guard if `!data`, or assert `data!`
if you’re certain success always provides data, then keep the notify.success,
agencyForm.reset(), setShowInviteModal(false) and organizations.refetch() calls
only when data is present.
In `@src/app/_components/admincomponents/invite-user-form.tsx`:
- Line 100: The fallback is applied after mapping so it never runs; change the
expression to apply the fallback to organizations before mapping, e.g. replace
data={organizations.map(... ) ?? NO_ORGS_DATA} with data={(organizations ??
NO_ORGS_DATA).map(org => ({ value: org.id, label: org.name }))} so the
NO_ORGS_DATA fallback is used when organizations is null/undefined.
🧹 Nitpick comments (4)
src/server/api/routers/organizations.ts (1)
100-106: Consider validatingorganizationRoleconditionally withorganizationId.When
organizationIdis not provided,organizationRoleis still required but ignored (lines 141-149). Consider using Zod's.refine()or discriminated unions to enforce thatorganizationRoleis only required whenorganizationIdis present, or makeorganizationRoleoptional as well.♻️ Suggested schema refinement
.input( z.object({ name: z.string().min(1), email: z.string().email(), - organizationRole: z.nativeEnum(OrganizationRole), + organizationRole: z.nativeEnum(OrganizationRole).optional(), organizationId: z.string().optional(), - }), + }).refine( + (data) => !data.organizationId || data.organizationRole, + { message: "organizationRole is required when organizationId is provided", path: ["organizationRole"] } + ), )Then update the
addMembercall:if (input.organizationId) { + if (!input.organizationRole) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "organizationRole is required when organizationId is provided", + }); + } await auth.api.addMember({src/app/_components/admincomponents/invite-agency-form.tsx (1)
5-12: Consider renaming the interface to avoid shadowing the component name.Both the interface (
InviteAgencyForm) and the component (InviteAgencyForm) share the same name, which can cause confusion and potential import issues. Consider renaming the interface toInviteAgencyFormValuesorAgencyFormData.♻️ Suggested rename
-interface InviteAgencyForm { +interface InviteAgencyFormValues { name: string; slug: string; } interface InviteAgencyFormProps { - form: UseFormReturnType<InviteAgencyForm>; + form: UseFormReturnType<InviteAgencyFormValues>; }src/app/_components/admincomponents/admin-dashboard.tsx (2)
57-62: Consider importing the shared email regex.The email regex is already defined in
src/types/validation.ts. Importing it would reduce duplication and ensure consistency.Proposed fix
+import { emailRegex } from "@/types/validation"; ... email: (value) => { if (value.trim().length === 0) return "Email is required"; - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; if (!emailRegex.test(value)) return "Invalid email address"; return null; },
131-136: Consider resetting form values on modal close.Currently only errors are cleared, but form values persist. If a user partially fills the form and closes the modal, stale data will remain when reopening. Consider also calling
form.reset()or resettinginviteTypeto ensure a clean state.Proposed fix
const handleCloseModal = () => { - userForm.clearErrors(); - agencyForm.clearErrors(); + userForm.reset(); + agencyForm.reset(); setShowInviteModal(false); setInviteType(null); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/_components/admincomponents/admin-dashboard.tsxsrc/app/_components/admincomponents/invite-agency-form.tsxsrc/app/_components/admincomponents/invite-user-form.tsxsrc/app/_components/common/modal/modal.tsxsrc/assets/icons/home.tsxsrc/assets/icons/user.tsxsrc/server/api/routers/organizations.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-23T05:44:37.925Z
Learnt from: tanvimahal
Repo: Code-the-Change-YYC/salvationarmy PR: 25
File: src/server/api/routers/organizations.ts:38-38
Timestamp: 2025-12-23T05:44:37.925Z
Learning: In this codebase's TRPC protectedProcedure context, ctx.session has a nested structure containing both a session object and a user object. To access activeOrganizationId, use ctx.session.session.activeOrganizationId (not ctx.session.activeOrganizationId).
Applied to files:
src/server/api/routers/organizations.ts
🧬 Code graph analysis (2)
src/app/_components/common/modal/modal.tsx (1)
src/app/_components/common/modal/modaltests.tsx (3)
setCustomFooterOpen(153-153)setCustomFooterOpen(158-158)simpleOpen(19-306)
src/app/_components/admincomponents/admin-dashboard.tsx (7)
src/trpc/react.tsx (1)
api(25-25)src/lib/notifications.ts (1)
notify(13-58)src/types/validation.ts (1)
emailRegex(10-10)src/app/_components/common/button/Button.tsx (1)
Button(41-80)src/app/_components/common/modal/modal.tsx (1)
Modal(24-70)src/app/_components/admincomponents/invite-user-form.tsx (1)
InviteUserForm(29-108)src/app/_components/admincomponents/invite-agency-form.tsx (1)
InviteAgencyForm(14-38)
🪛 GitHub Actions: CI
src/app/_components/admincomponents/admin-dashboard.tsx
[error] 37-37: yarn tsc --noEmit: TypeScript error TS18047: 'data' is possibly 'null'.
🪛 GitHub Check: ci
src/app/_components/admincomponents/admin-dashboard.tsx
[failure] 37-37:
'data' is possibly 'null'.
🔇 Additional comments (10)
src/app/_components/common/modal/modal.tsx (1)
42-51: LGTM!The increased margin (
mt="xl") provides better visual separation between modal content and footer actions, aligning with the updated Figma designs.src/assets/icons/user.tsx (1)
4-35: LGTM!The
strokeprop addition enables flexible customization while maintaining backward compatibility with the default#434343color. The consolidated path structure is cleaner than the previous implementation with duplicate paths.src/assets/icons/home.tsx (1)
4-35: LGTM!Consistent implementation with the User icon. The
strokeprop customization pattern across icons enables unified styling in the modal invitation flow.src/server/api/routers/organizations.ts (2)
125-131: LGTM on the name field integration.The
namefield is properly validated at input and correctly passed tosignUpEmail. This ensures invited users have a name set from the start.
141-149: Behavior is intentional: non-MEMBER roles are created without organization association.The code correctly implements the design where only users with the MEMBER role are associated with an organization. Frontend validation enforces this by requiring
organizationIdonly for MEMBER role and clearing it for non-member roles (ADMIN, DRIVER). WhenorganizationIdis omitted or not provided for other roles, the user is created successfully without organization membership, which is the expected behavior.src/app/_components/admincomponents/invite-agency-form.tsx (2)
14-37: LGTM on component structure.The form component follows Mantine's recommended patterns with
form.key()for React keys andgetInputProps()for field binding. The layout is clean and accessible with proper labels.
28-34: Slug validation and format guidance already exist in the parent form.The form validation is implemented in
admin-dashboard.tsx(lines 81-84) with a regex pattern/^[a-z0-9-]+$/that enforces lowercase letters, numbers, and hyphens. The validation error message clearly communicates the required format: "Slug must only contain lowercase letters, numbers, and hyphens". Additionally, the field label already provides a format example ("Agency Slug (ex: my-org)"). No changes are needed.Likely an incorrect or invalid review comment.
src/app/_components/admincomponents/invite-user-form.tsx (1)
23-27: Verify:OWNERrole labeled as "Driver" is intentional.The mapping of
OrganizationRole.OWNERto the label"Driver"appears semantically unusual. Please confirm this aligns with the business logic and Figma designs. If "Driver" is indeed a distinct role concept, consider renaming the enum value for clarity.src/app/_components/admincomponents/admin-dashboard.tsx (2)
147-186: Approve: Two-step invitation flow is well-structured.The type selection modal cleanly separates concerns with clear UX. The
showDefaultFooter={false}correctly hides the confirm/cancel buttons since selection is done via the type buttons. TheonConfirm={() => {}}no-op is acceptable here since confirmation isn't needed for this step.
188-228: LGTM: Form modals properly integrated.The conditional rendering of user/agency modals based on
inviteType, with appropriate loading states and form passing, is correctly implemented.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| submitData.organizationId = userForm.values.organizationId; | ||
| } | ||
|
|
||
| console.log("submit user", submitData); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statement.
This appears to be a debug artifact that should be removed before merging.
Proposed fix
- console.log("submit user", submitData);
inviteUserMutation.mutate(submitData);📝 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.
| console.log("submit user", submitData); | |
| inviteUserMutation.mutate(submitData); |
🤖 Prompt for AI Agents
In `@src/app/_components/admincomponents/admin-dashboard.tsx` at line 115, Remove
the debug console.log by deleting the line console.log("submit user",
submitData); in the admin-dashboard component (where submitData is handled)
inside admin-dashboard.tsx; if persistent logging is needed replace it with the
app's structured logger call (e.g., using processLogger or a provided logging
utility) or handle the data silently by returning/setting state, but do not
leave console.log in the production component.
There was a problem hiding this comment.
@promatty Thanks for confirming! 👍
|
|
||
| // todo: handle case where admin permissions are given within the organization | ||
| console.log("submit", form.values); | ||
| console.log("submit agency", agencyForm.values); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statement.
Same as above - debug statement should be removed.
Proposed fix
- console.log("submit agency", agencyForm.values);
createAgencyMutation.mutate(agencyForm.values);📝 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.
| console.log("submit agency", agencyForm.values); | |
| createAgencyMutation.mutate(agencyForm.values); |
🤖 Prompt for AI Agents
In `@src/app/_components/admincomponents/admin-dashboard.tsx` at line 127, Remove
the debug console.log call that prints agencyForm.values in the submit handler;
locate the line console.log("submit agency", agencyForm.values) inside the admin
dashboard submit routine (the agency form submit function in
admin-dashboard.tsx) and delete it (or replace with a proper structured logger
if required by project logging conventions), ensuring no leftover debug output
remains.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/server/api/routers/organizations.ts`:
- Around line 99-106: The inviteUser adminProcedure input schema currently
allows any organizationRole with an optional organizationId; update the Zod
schema for inviteUser to enforce the frontend contract by adding a .refine() on
the object: require organizationId when organizationRole ===
OrganizationRole.MEMBER (and optionally reject organizationId for non-MEMBER
roles if you prefer strictness). Reference the inviteUser input object and
OrganizationRole enum when adding the refine so validation fails if MEMBER is
missing organizationId (or if non-MEMBER includes one, depending on chosen
strictness).
♻️ Duplicate comments (2)
src/app/_components/admincomponents/admin-dashboard.tsx (2)
115-116: Remove debugconsole.logstatement.This debug statement should be removed before merging.
127-128: Remove debugconsole.logstatement.This debug statement should be removed before merging.
🧹 Nitpick comments (4)
src/server/api/routers/organizations.ts (1)
110-119: Consider removing redundant authorization check.This manual role check duplicates the protection already provided by
adminProcedure. While defense-in-depth is valid, consider whether this adds value or just maintenance overhead.src/app/_components/admincomponents/admin-dashboard.tsx (3)
57-62: Import shared email regex from validation module.The email regex is duplicated here. Consider importing from
src/types/validation.tswhich already exportsemailRegexto maintain a single source of truth.♻️ Proposed fix
+import { emailRegex } from "@/types/validation";Then in the validation:
email: (value) => { if (value.trim().length === 0) return "Email is required"; - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; if (!emailRegex.test(value)) return "Invalid email address"; return null; },
147-159: Remove unnecessaryonConfirmprop.Since
showDefaultFooter={false}, theonConfirm={() => {}}is never invoked. Consider removing it for clarity, unless the Modal component requires it.♻️ Proposed fix
<Modal opened={showInviteModal} onClose={handleCloseModal} - onConfirm={() => {}} title={
209-228: Clarify modal title: "Create" vs "Invite" an Agency.The modal title says "Invite an Agency" but the action creates an organization via
createOrganization. Consider changing the title to "Create an Agency" or "Add an Agency" to more accurately reflect the action.♻️ Proposed fix
title={ <Box fw={600} fz="xl"> - Invite an Agency + Create an Agency </Box> }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/_components/admincomponents/admin-dashboard.tsxsrc/app/_components/admincomponents/invite-agency-form.tsxsrc/assets/icons/home.tsxsrc/assets/icons/user.tsxsrc/server/api/routers/organizations.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/assets/icons/home.tsx
- src/app/_components/admincomponents/invite-agency-form.tsx
- src/assets/icons/user.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-23T05:44:37.925Z
Learnt from: tanvimahal
Repo: Code-the-Change-YYC/salvationarmy PR: 25
File: src/server/api/routers/organizations.ts:38-38
Timestamp: 2025-12-23T05:44:37.925Z
Learning: In this codebase's TRPC protectedProcedure context, ctx.session has a nested structure containing both a session object and a user object. To access activeOrganizationId, use ctx.session.session.activeOrganizationId (not ctx.session.activeOrganizationId).
Applied to files:
src/server/api/routers/organizations.ts
🧬 Code graph analysis (2)
src/server/api/routers/organizations.ts (1)
src/lib/auth.ts (1)
auth(10-116)
src/app/_components/admincomponents/admin-dashboard.tsx (7)
src/trpc/react.tsx (1)
api(25-25)src/lib/notifications.ts (1)
notify(13-58)src/types/validation.ts (1)
emailRegex(10-10)src/app/_components/common/button/Button.tsx (1)
Button(41-80)src/app/_components/common/modal/modal.tsx (1)
Modal(24-70)src/app/_components/admincomponents/invite-user-form.tsx (1)
InviteUserForm(29-108)src/app/_components/admincomponents/invite-agency-form.tsx (1)
InviteAgencyForm(14-38)
🔇 Additional comments (4)
src/server/api/routers/organizations.ts (1)
141-149: LGTM! Conditional organization membership is correctly implemented.The logic correctly gates
addMemberon the presence oforganizationId, allowing user creation without immediate organization membership when appropriate.src/app/_components/admincomponents/admin-dashboard.tsx (3)
35-45: LGTM! Null-safety addressed.The optional chaining
data?.name ?? "Agency"correctly handles the potentially nulldataresponse.
73-88: LGTM! Agency form validation is well-implemented.The slug validation correctly enforces URL-safe format with lowercase letters, numbers, and hyphens.
90-117: LGTM! Submission data correctly prepared.The conditional inclusion of
organizationIdonly forMEMBERrole aligns well with the backend changes that gateaddMemberon the presence oforganizationId.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| inviteUser: adminProcedure | ||
| .input( | ||
| z.object({ | ||
| name: z.string().min(1), | ||
| email: z.string().email(), | ||
| organizationRole: z.nativeEnum(OrganizationRole), | ||
| organizationId: z.string(), | ||
| organizationId: z.string().optional(), | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/server/api/routers && wc -l organizations.tsRepository: Code-the-Change-YYC/salvationarmy
Length of output: 97
🏁 Script executed:
cd src/server/api/routers && sed -n '90,160p' organizations.tsRepository: Code-the-Change-YYC/salvationarmy
Length of output: 2216
🏁 Script executed:
rg "organizationRole" --type=ts -B 3 -A 3 | head -100Repository: Code-the-Change-YYC/salvationarmy
Length of output: 7374
Enforce the relationship between organizationRole and organizationId in the schema.
The frontend shows that organizationId is conditionally required only when organizationRole is MEMBER. However, the schema accepts any combination, allowing organizationRole (required) without organizationId when the role is non-MEMBER, even though the API won't use it. Use Zod's .refine() to enforce that organizationId is required when organizationRole is MEMBER, or adjust the schema to reflect that non-MEMBER roles should not include organizationId at all.
🤖 Prompt for AI Agents
In `@src/server/api/routers/organizations.ts` around lines 99 - 106, The
inviteUser adminProcedure input schema currently allows any organizationRole
with an optional organizationId; update the Zod schema for inviteUser to enforce
the frontend contract by adding a .refine() on the object: require
organizationId when organizationRole === OrganizationRole.MEMBER (and optionally
reject organizationId for non-MEMBER roles if you prefer strictness). Reference
the inviteUser input object and OrganizationRole enum when adding the refine so
validation fails if MEMBER is missing organizationId (or if non-MEMBER includes
one, depending on chosen strictness).
80fc82c to
4329fdd
Compare
There was a problem hiding this comment.
In this file, what is the distinction between the invite types of "user", "agency" and null?
There was a problem hiding this comment.
I'm using invite type to determine which modal to display. "user" for the user modal, "agency" for the agency modal, and null for the selection modal to select between inviting a user and agency.
burtonjong
left a comment
burtonjong
left a comment
There was a problem hiding this comment.
I think there's some confusion about these organization roles
Maybe we can hop on a call and i can explain it better if you still don't understand from my comments.
We would use these organizations in the future for these example cases:
not everyone in the salvation army navigation center should be able to invite new agencies. This could be done by appointing ADMINs (org role) and MEMBERs (org role) that have different permissions. Admins will be able to send invitations, and members wont be able to
Or even for drivers, ADMIN drivers will be able to see ALL bookings whereas MEMBER drivers won't, and only bookings assigned to them, etc.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Yemyam
force-pushed
the
SANC-64-update-the-invitations-frontend
branch
from
b0bcf0f to
aff620b
Compare
| // todo: handle case where admin permissions are given within the organization | ||
| console.log("submit", form.values); | ||
| const formValues = agencyForm.getValues(); | ||
| console.log("submit agency", formValues); |
| submitData.organizationId = userForm.values.organizationId; | ||
| } | ||
|
|
||
| console.log("submit user", submitData); |
| <Stack gap="md"> | ||
| <Box fw={500} fz="lg"> | ||
| User Information | ||
| <Box fw={600} fz="lg" c="#8B2635"> |
There was a problem hiding this comment.
this color is hardcoded here a few times. can we import from the global scss file instead
| withAsterisk | ||
| label="Email Address (Gmail only)" | ||
| placeholder="Example@gmail.com" | ||
| key={form.key("email")} | ||
| {...form.getInputProps("email")} |
There was a problem hiding this comment.
why do we only accept gmail again?
| email: (value) => { | ||
| if (value.trim().length === 0) return "Email is required"; | ||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| if (!emailRegex.test(value)) return "Invalid email address"; | ||
| return null; |
There was a problem hiding this comment.
we have email regex in a shared function already. also, if we are only allowing gmail then the regex should also adjust for this
|
@Yemyam I think once you address matthews comments and then change the "invite agency" to "create agency" this looks good |
3 participants
Update invite user and agency modals to match Figma designs
Frontend Changes:
NOTE:
createOrganizationinviteUser, every user is being added to the db as a driver.Summary by CodeRabbit
New Features
UI Improvements