[fix] Facilitators can only withdraw an application before it is accepted#2566
Conversation
📝 WalkthroughWalkthroughServer-side router rejects deferral requests when a registration decision is null, forbids facilitator deferrals, and forbids facilitator withdrawals once a decision exists. Client-side introduces getApplicationActionLabel and shows a "Withdraw application" overflow item only for upcoming rows with null decision. DropoutModal now renders a WithdrawConfirm flow for pre-decision registrations and corresponding Storybook story and tests validate the confirm/response/mutation payload. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR tightens facilitator withdrawal permissions so that a facilitator can only withdraw their application while it is still under review (
Confidence Score: 4/5Safe to merge — the core withdrawal restriction works correctly in both the frontend and backend. The backend guard and frontend visibility condition are consistent and correctly implement the intended policy change. The only rough edge is that the FORBIDDEN error message says 'not yet been accepted' but also fires for rejected facilitators (decision = 'Reject'), which is inaccurate if called directly. A test covering the Reject path on the server is also absent. Neither issue affects real users since the UI hides the button for rejected applicants, but they're worth cleaning up. dropout.ts and dropout.test.ts — the error message wording and the missing 'Reject' test case. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Facilitator calls dropoutOrDeferral] --> B{type === 'Deferral'?}
B -- Yes --> C[FORBIDDEN: Facilitators cannot defer]
B -- No --> D{decision !== null?}
D -- Yes
'Accept' or 'Reject' --> E[FORBIDDEN: Can only withdraw
while still under review]
D -- No
decision === null --> F[Insert dropout record ✓]
style C fill:#f88,stroke:#c00
style E fill:#f88,stroke:#c00
style F fill:#8f8,stroke:#060
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/website/src/server/routers/dropout.ts (1)
74-76: ⚡ Quick winConsider clarifying the error message to reflect both rejection and acceptance.
The condition
decision !== nullblocks withdrawal after both acceptance and rejection, but the error message only mentions acceptance. Consider: "Facilitators can only withdraw an application that is still pending." or "...that has not yet been decided."💬 Suggested message improvement
if (courseRegistration.decision !== null) { - throw new TRPCError({ code: 'FORBIDDEN', message: 'Facilitators can only withdraw an application that has not yet been accepted.' }); + throw new TRPCError({ code: 'FORBIDDEN', message: 'Facilitators can only withdraw an application that is still pending.' }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/server/routers/dropout.ts` around lines 74 - 76, The error message thrown when checking courseRegistration.decision !== null is misleading (it only mentions acceptance) — update the TRPCError message created in the dropout router (the throw new TRPCError({...}) that follows the courseRegistration.decision !== null check) to clearly state the real condition, e.g. "Facilitators can only withdraw an application that is still pending." (or "...that has not yet been decided.") so it covers both rejection and acceptance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/website/src/server/routers/dropout.ts`:
- Around line 74-76: The error message thrown when checking
courseRegistration.decision !== null is misleading (it only mentions acceptance)
— update the TRPCError message created in the dropout router (the throw new
TRPCError({...}) that follows the courseRegistration.decision !== null check) to
clearly state the real condition, e.g. "Facilitators can only withdraw an
application that is still pending." (or "...that has not yet been decided.") so
it covers both rejection and acceptance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3d367f0-16ef-4084-af64-7cccc640b2ff
📒 Files selected for processing (4)
apps/website/src/components/my-courses/CourseListRow.test.tsxapps/website/src/components/my-courses/useCourseListRow.tsxapps/website/src/server/routers/dropout.test.tsapps/website/src/server/routers/dropout.ts
Will-Howard
temporarily deployed
to
wh-2551-facilitator-withdraw-application-2026-05 - bluedot-preview PR #2566
Render
Destroyed
Will-Howard
temporarily deployed
to
wh-2551-facilitator-withdraw-application-2026-05 - bluedot-storybook-preview PR #2566
Render
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/website/src/components/courses/DropoutModal.tsx (1)
76-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGate this branch on the registration query finishing.
Until
courseRegistrations.getAllresolves,allowDeferraldefaults totrueand pending applications render the normal drop/defer form before flipping toWithdrawConfirm. On a slow response that can briefly show — and allow interaction with — controls that should never be available for this registration. A short loading state here would avoid the flicker and inconsistent behaviour.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/components/courses/DropoutModal.tsx` around lines 76 - 90, The branch that returns WithdrawConfirm is executing before trpc.courseRegistrations.getAll.useQuery() finishes, causing a flicker; update the component to check the query status (e.g., use the isLoading / isFetching / isFetched boolean returned from trpc.courseRegistrations.getAll.useQuery()) and short-circuit render (null or a loading state) until the registrations result is settled, then compute registration, allowDeferral and isDeferral and evaluate registration?.decision to return WithdrawConfirm; ensure the gating references the existing trpc.courseRegistrations.getAll.useQuery(), registrations/registration, allowDeferral, and WithdrawConfirm symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/website/src/server/routers/dropout.ts`:
- Around line 69-71: The current guard forbids any "Deferral" when
courseRegistration.decision is null, which blocks non-facilitators too; change
the conditional to only apply to facilitators by checking
courseRegistration.role === 'Facilitator' as well (e.g. if (type === 'Deferral'
&& courseRegistration.role === 'Facilitator' && courseRegistration.decision ===
null) throw new TRPCError(...)). Update the guard in the dropout handler where
type and courseRegistration are used.
---
Outside diff comments:
In `@apps/website/src/components/courses/DropoutModal.tsx`:
- Around line 76-90: The branch that returns WithdrawConfirm is executing before
trpc.courseRegistrations.getAll.useQuery() finishes, causing a flicker; update
the component to check the query status (e.g., use the isLoading / isFetching /
isFetched boolean returned from trpc.courseRegistrations.getAll.useQuery()) and
short-circuit render (null or a loading state) until the registrations result is
settled, then compute registration, allowDeferral and isDeferral and evaluate
registration?.decision to return WithdrawConfirm; ensure the gating references
the existing trpc.courseRegistrations.getAll.useQuery(),
registrations/registration, allowDeferral, and WithdrawConfirm symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: feea1056-5d05-4888-b67b-42680c8f6b00
📒 Files selected for processing (8)
apps/website/src/components/courses/DropoutModal.stories.tsxapps/website/src/components/courses/DropoutModal.test.tsxapps/website/src/components/courses/DropoutModal.tsxapps/website/src/components/my-courses/CourseListRow.stories.tsxapps/website/src/components/my-courses/CourseListRow.test.tsxapps/website/src/components/my-courses/useCourseListRow.tsxapps/website/src/server/routers/dropout.test.tsapps/website/src/server/routers/dropout.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/website/src/components/my-courses/CourseListRow.test.tsx
1 participant
Description
Issue
Fixes #2551
Developer checklist
Screenshot
Focusing on the withdraw-before-start case
Note: I am aware the padding is a little ugly on the modal. This is coming from the outer
Modalcomponent so is fiddly to change.