Conversation
Yemyam
requested review from
JustinTan-1,
Lujarios,
burtonjong,
jason-duong4509,
promatty,
tanvimahal,
themaxboucher and
wesleylui
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a three-part invite flow (type-selection → Invite User modal → Create Agency modal), new InviteAgencyForm component, expanded InviteUserForm with role-based logic and validations, new createAgency and inviteUser mutations wiring success/error handling, modal lifecycle handlers, and adds Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant Dashboard
participant Modal
participant Form
participant Mutation
participant API
Admin->>Dashboard: Click "Invite"
Dashboard->>Modal: Open type-selection modal
Admin->>Modal: Choose "User" or "Agency"
Modal->>Form: Render InviteUserForm or InviteAgencyForm
Admin->>Form: Fill fields and submit
Form->>Mutation: Trigger inviteUserMutation / createAgencyMutation
Mutation->>API: Send HTTP request (inviteUser / createAgency)
API-->>Mutation: Return success/error
Mutation->>Dashboard: onSuccess -> refetch orgs, close modal
Dashboard->>Admin: UI updated (modal closed, org list refreshed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
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
| <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
There was a problem hiding this comment.
can we use colors from globals?
|
@Yemyam I think once you address matthews comments and then change the "invite agency" to "create agency" this looks good |
Yemyam
force-pushed
the
SANC-64-update-the-invitations-frontend
branch
from
aff620b to
5b88285
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/app/_components/admincomponents/invite-user-form.tsx (2)
69-89: Redundant inlinebackgroundColoron filled-variant buttons.When
variant="filled"is active, Mantine'scolorprop already controls the background. The inlinestyle={{ backgroundColor: ... }}duplicates this. It's harmless but unnecessary.Proposed cleanup
onClick={() => handleRoleChange(role)} style={{ flex: 1, - backgroundColor: selectedRole === role ? "#8B2635" : undefined, }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/_components/admincomponents/invite-user-form.tsx` around lines 69 - 89, Remove the redundant inline backgroundColor in the Button style: inside the ALL_ROLES.map render where Button components are created (using props variant, color, leftSection, onClick calling handleRoleChange, and children ROLE_LABELS[role]), delete the backgroundColor entry from the style object but keep other style properties like flex: 1; rely on Mantine's variant="filled" and the color prop (which already sets the filled background when selectedRole === role) to control the button background.
8-8: Consider extractingSYSTEM_ORG_SLUGSto a shared constant.This same list (
"admins","drivers") is defined ininvite-user-form.tsx(line 8) but the same slugs are hard-coded as string literals inadmin-dashboard.tsx(lines 105 and 112). Creating a shared constant would prevent divergence and ensure these slugs stay in sync if they ever need to change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/_components/admincomponents/invite-user-form.tsx` at line 8, Extract the array constant SYSTEM_ORG_SLUGS (["admins","drivers"]) into a shared module (e.g., a new export from a central constants file) and replace the duplicate string literals in admin-dashboard.tsx with an imported reference to that shared SYSTEM_ORG_SLUGS; update invite-user-form.tsx to import the shared SYSTEM_ORG_SLUGS instead of defining it locally and ensure both files import the same symbol so the slugs remain in sync.src/app/_components/admincomponents/admin-dashboard.tsx (1)
158-199: No way to navigate back from a form modal to the type-selection modal.Once a user selects "Invite User" or "Create Agency," the only escape is closing the modal entirely. A "Back" button or breadcrumb to return to type selection would improve UX. This could be deferred if not in the current Figma spec.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/_components/admincomponents/admin-dashboard.tsx` around lines 158 - 199, The modal flow lacks a way to return from the form view to the initial type-selection view; add a "Back" control in the form modal (the Modal rendered when inviteType !== null) that calls a handler (e.g., handleBackToTypeSelection) to set inviteType back to null while keeping showInviteModal true, so the Modal re-renders the type-selection UI; update the Modal footer or header in the branch that uses inviteType (and functions like handleInviteTypeSelect, handleCloseModal) to include this Back action and ensure any form state is reset when toggling back.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/_components/admincomponents/admin-dashboard.tsx`:
- Around line 144-149: handleCloseModal currently only clears validation
messages (userForm.clearErrors(), agencyForm.clearErrors()) so previous input
remains when reopening; update handleCloseModal to also reset form values by
calling userForm.reset() and agencyForm.reset() (or userForm.reset(defaults) /
agencyForm.reset(defaults) if you rely on initial values), then continue to
setShowInviteModal(false) and setInviteType(null) so both cancel/close and
onSuccess paths produce the same cleared state.
- Around line 36-43: In createAgencyMutation's onSuccess handler, don't silently
return when data is falsy; instead notify the user (e.g., call notify.warn or
notify.error with a message like "No agency data returned from server") and
optionally log the unexpected response, and only perform agencyForm.reset(),
setShowInviteModal(false), and organizations.refetch() when data is present;
update the guard in onSuccess to show the warning and exit without closing the
modal or resetting the form so the user receives feedback.
In `@src/app/_components/admincomponents/invite-user-form.tsx`:
- Around line 92-102: The Organization Permission Level Select (symbols: Select,
OrganizationRole, form.getInputProps("organizationRole")) is always shown but
should be hidden when the chosen invitee role maps to the system org (as handled
in admin-dashboard.tsx); update the component to conditionally render the Select
only when the selected role is organization-scoped (e.g., check
form.getInputProps("role") or form.values.role and render the organizationRole
Select only if role !== OrganizationRole.ADMIN && role !== "Driver" (or whatever
admin-dashboard.tsx maps to system org)). Ensure you reference the same form
field name used for the invitee role and keep the existing key and input props
for organizationRole when rendering.
---
Nitpick comments:
In `@src/app/_components/admincomponents/admin-dashboard.tsx`:
- Around line 158-199: The modal flow lacks a way to return from the form view
to the initial type-selection view; add a "Back" control in the form modal (the
Modal rendered when inviteType !== null) that calls a handler (e.g.,
handleBackToTypeSelection) to set inviteType back to null while keeping
showInviteModal true, so the Modal re-renders the type-selection UI; update the
Modal footer or header in the branch that uses inviteType (and functions like
handleInviteTypeSelect, handleCloseModal) to include this Back action and ensure
any form state is reset when toggling back.
In `@src/app/_components/admincomponents/invite-user-form.tsx`:
- Around line 69-89: Remove the redundant inline backgroundColor in the Button
style: inside the ALL_ROLES.map render where Button components are created
(using props variant, color, leftSection, onClick calling handleRoleChange, and
children ROLE_LABELS[role]), delete the backgroundColor entry from the style
object but keep other style properties like flex: 1; rely on Mantine's
variant="filled" and the color prop (which already sets the filled background
when selectedRole === role) to control the button background.
- Line 8: Extract the array constant SYSTEM_ORG_SLUGS (["admins","drivers"])
into a shared module (e.g., a new export from a central constants file) and
replace the duplicate string literals in admin-dashboard.tsx with an imported
reference to that shared SYSTEM_ORG_SLUGS; update invite-user-form.tsx to import
the shared SYSTEM_ORG_SLUGS instead of defining it locally and ensure both files
import the same symbol so the slugs remain in sync.
| const createAgencyMutation = api.organization.createOrganization.useMutation({ | ||
| onSuccess: (data) => { | ||
| if (!data) return; | ||
| notify.success(`Agency "${data.name}" created successfully`); | ||
| agencyForm.reset(); | ||
| setShowInviteModal(false); | ||
| void organizations.refetch(); | ||
| }, |
There was a problem hiding this comment.
Silent early return on null data — user gets no feedback.
The if (!data) return; guard silently swallows the case. The mutation's onSuccess fired, so the UI won't show a loading state, but the modal stays open with no notification. Consider at least a warning.
Proposed fix
onSuccess: (data) => {
- if (!data) return;
+ if (!data) {
+ notify.error("Unexpected error: no data returned");
+ return;
+ }
notify.success(`Agency "${data.name}" created successfully`);📝 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.
| const createAgencyMutation = api.organization.createOrganization.useMutation({ | |
| onSuccess: (data) => { | |
| if (!data) return; | |
| notify.success(`Agency "${data.name}" created successfully`); | |
| agencyForm.reset(); | |
| setShowInviteModal(false); | |
| void organizations.refetch(); | |
| }, | |
| const createAgencyMutation = api.organization.createOrganization.useMutation({ | |
| onSuccess: (data) => { | |
| if (!data) { | |
| notify.error("Unexpected error: no data returned"); | |
| return; | |
| } | |
| notify.success(`Agency "${data.name}" created successfully`); | |
| agencyForm.reset(); | |
| setShowInviteModal(false); | |
| void organizations.refetch(); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/_components/admincomponents/admin-dashboard.tsx` around lines 36 -
43, In createAgencyMutation's onSuccess handler, don't silently return when data
is falsy; instead notify the user (e.g., call notify.warn or notify.error with a
message like "No agency data returned from server") and optionally log the
unexpected response, and only perform agencyForm.reset(),
setShowInviteModal(false), and organizations.refetch() when data is present;
update the guard in onSuccess to show the warning and exit without closing the
modal or resetting the form so the user receives feedback.
✅ Confirmed as addressed by @promatty
There was a problem hiding this comment.
i mean i dont see this happening since we return the org object, but to be safe lets add error handling
There was a problem hiding this comment.
@promatty, that makes sense! Even though the API should always return the organization object, adding the error handling is good defensive programming. It ensures users get proper feedback in the unlikely edge case where something unexpected happens.
Thanks for addressing this! 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
promatty
left a comment
promatty
left a comment
There was a problem hiding this comment.
small changes, approving now so you can merge after fixing them
| <Stack gap="md"> | ||
| <Box fw={500} fz="lg"> | ||
| User Information | ||
| <Box fw={600} fz="lg" c="#8B2635"> |
There was a problem hiding this comment.
can we use colors from globals?
| const createAgencyMutation = api.organization.createOrganization.useMutation({ | ||
| onSuccess: (data) => { | ||
| if (!data) return; | ||
| notify.success(`Agency "${data.name}" created successfully`); | ||
| agencyForm.reset(); | ||
| setShowInviteModal(false); | ||
| void organizations.refetch(); | ||
| }, |
There was a problem hiding this comment.
i mean i dont see this happening since we return the org object, but to be safe lets add error handling
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