Distinguish 403 Forbidden from generic errors in UI#429
Conversation
Greptile SummaryThis PR introduces first-class 403 Forbidden handling across the frontend: a shared
Confidence Score: 5/5Safe to merge; the 403 distinction works correctly across all major surfaces and the new type guard is correctly implemented The core implementation — src/components/ui/members/member-profile-container.tsx and src/components/ui/coaching-sessions/coaching-session-title.tsx have minor UX inconsistencies in how the permission message is presented relative to the rest of the PR Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[API Request] --> B{Response}
B -->|200 OK| C[Render Data]
B -->|403 Forbidden| D{Request Type}
B -->|Other Error| E[Generic Error Path]
D -->|Page-level fetch| F[ForbiddenError Component]
F --> G["Actions: 'Actions Access Denied'"]
F --> H["Members: 'Members Access Denied'"]
D -->|Mutation| I[toast.error - PERMISSION_DENIED_MESSAGE]
I --> J[actions mutationErrorMessage]
I --> K[add-member-dialog]
I --> L[member-card resend/assign]
I --> M[coaching-session-form]
I --> N[reschedule-series-dialog]
I --> O[coaching-preferences-section]
I --> P["member-profile-container: uses toast() not toast.error()"]
E -->|Page-level| Q[Generic error block]
E -->|Mutation| R[Generic error message]
subgraph guard["isForbiddenError guard"]
S["error instanceof EntityApiError AND error.status === 403"]
end
D -.->|checked via| S
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[API Request] --> B{Response}
B -->|200 OK| C[Render Data]
B -->|403 Forbidden| D{Request Type}
B -->|Other Error| E[Generic Error Path]
D -->|Page-level fetch| F[ForbiddenError Component]
F --> G["Actions: 'Actions Access Denied'"]
F --> H["Members: 'Members Access Denied'"]
D -->|Mutation| I[toast.error - PERMISSION_DENIED_MESSAGE]
I --> J[actions mutationErrorMessage]
I --> K[add-member-dialog]
I --> L[member-card resend/assign]
I --> M[coaching-session-form]
I --> N[reschedule-series-dialog]
I --> O[coaching-preferences-section]
I --> P["member-profile-container: uses toast() not toast.error()"]
E -->|Page-level| Q[Generic error block]
E -->|Mutation| R[Generic error message]
subgraph guard["isForbiddenError guard"]
S["error instanceof EntityApiError AND error.status === 403"]
end
D -.->|checked via| S
Reviews (5): Last reviewed commit: "switch `error.isForbidden()` for `isForb..." | Re-trigger Greptile |
jhodapp
left a comment
There was a problem hiding this comment.
Thanks for this Raymond. A few inline changes plus one general one here.
The PR description mentions page-level and mutation-level 403 tests, but only actions-page-container.test.tsx is updated. A few 403 paths added here are untested:
- Members page page-level
ForbiddenErrorrender (src/app/organizations/[id]/members/page.tsx) - Mutation toast sites: members add/resend/assign, dashboard session form, reschedule series, goals overview card, coaching preferences
At minimum, a Members-page render test mirroring the new Actions one would lock in the second page-level path.
| } else { | ||
| let message: string; | ||
| if (isDurationValidationError(error)) { | ||
| if (error.isForbidden()) { |
There was a problem hiding this comment.
error.isForbidden() could read as isForbiddenError(error) for consistency with the other call sites.
There was a problem hiding this comment.
Nice suggestion.
isForbiddenError's type predicate narrows the else branches to never here since error is already EntityApiError; it breaks the sibling error.status / error.isNetworkError() checks.
Happy to iterate further on this one.
There was a problem hiding this comment.
@rnambaale so you're saying you can't use my suggested shape instead?
There was a problem hiding this comment.
I have switched to isForbiddenError(error) here dfb6a25.
|
I have add more tests for the 403 path |
Description
The UI previously collapsed all backend failures into generic error messages, so a 403 Forbidden (user is authenticated but lacks permission) was indistinguishable from a network outage or a 5xx. This change adds a first-class 403 distinction so users see a clear "you don't have permission" message instead of a misleading generic error.
GitHub Issue: Closes #refactor-group/refactor-platform-rs#358
Changes
isForbidden()(403 check) toEntityApiError, plus a sharedPERMISSION_DENIED_MESSAGEconstant and anisForbiddenError(error: unknown)type guard insrc/types/entity-api-error.ts, re-exported through@/types/generaland@/lib/api/entity-api.ForbiddenErrorcomponent (Actions page, Members page) instead of a generic error block.mutationErrorMessage()helper.Screenshots / Videos Showing UI Changes (if applicable)
Helpful captures:
ForbiddenErrorscreen on a 403 data fetch.ForbiddenErrorscreen.Testing Strategy
npx vitest run __tests__/components/ui/actions/actions-page-container.test.tsx— all 18 pass, including the two new 403 cases.npx tsc --noEmit— clean.Concerns
The backend PR that introduces the 401/403 split is refactor-group/refactor-platform-rs#367