fix: jail page stuck on notifications — add 3-step waitlist flow#1726
Merged
fix: jail page stuck on notifications — add 3-step waitlist flow#1726
Conversation
The old JoinWaitlistPage gated the invite code input behind isPermissionGranted, permanently trapping users who denied notifications. New flow: 1. Email collection (required) — saved via updateUserById server action 2. Enable notifications (skippable) — soft prompt with "Not now" link 3. Jail screen — always shows invite code input regardless of notification state Also adds a subtle notification banner on the jail screen for users who skipped step 2.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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:
WalkthroughReworks JoinWaitlistPage into a URL-backed, three-step flow (email → notifications → jail) with persistent step state, email validation and submission, notification handling, invite-code validation/acceptance, and conditional data fetching. Adds isValidEmail utility and replaces an inline regex with it. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/Invites/JoinWaitlistPage.tsx (1)
91-95: The validateInviteCode function doesn't actually risk stranding the loading state.The underlying
invitesApi.validateInviteCode(insrc/services/invites.ts) wraps the server action in a try/catch and always returns{ success: false, username: '' }on error—it never throws. However, for consistency and defensive coding, this function should follow the same error handling pattern ashandleAcceptInvite(which uses try/finally). Consider adding try/finally or try/catch to handle any unexpected errors:const validateInviteCode = async (code: string): Promise<boolean> => { setIsLoading(true) try { const res = await invitesApi.validateInviteCode(code) return res.success } finally { setIsLoading(false) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Invites/JoinWaitlistPage.tsx` around lines 91 - 95, The validateInviteCode function sets isLoading but can leave it true on unexpected errors; update validateInviteCode to wrap the API call in try/finally (or try/catch/finally) so setIsLoading(false) always runs, e.g., setIsLoading(true) then try { await invitesApi.validateInviteCode(code); return res.success } finally { setIsLoading(false) }; reference the existing validateInviteCode function and mirror the defensive pattern used in handleAcceptInvite.
🤖 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/components/Invites/JoinWaitlistPage.tsx`:
- Around line 151-163: The email input in JoinWaitlistPage lacks an accessible
name; add one by either giving the input an id (e.g., id="join-email") and
adding a corresponding <label htmlFor="join-email">Email address</label> placed
with the descriptive text above, or add aria-label="Email address" directly on
the input. Ensure the change is applied to the input that uses
value={emailValue}, onChange which calls setEmailValue/setEmailError, and
preserves the onKeyDown handler that calls handleEmailSubmit when Enter is
pressed.
- Around line 28-37: The current logic computes initialStep with useMemo and
stores it in local state (initialStep, useMemo, step, setStep, useState), which
freezes the flow to the pre-auth render; replace this with URL-backed state
using useQueryStates from nuqs so step reacts to changes in user and
isPermissionGranted and persists across navigation/refresh. Remove the useMemo +
useState pair and initialize step via useQueryStates with a typed step enum
(1|2|3), update any setters to use the query-state setter, and ensure initial
value logic mirrors: if user?.user.email => isPermissionGranted ? 3 : 2 else 1
so auth hydration updates the URL state.
- Around line 115-119: The handleLogout function sets isLoggingOut true but only
clears it after logoutUser succeeds; if logoutUser rejects the flag remains
true. Wrap the await logoutUser()/router.push('/setup') sequence in try/finally
(or use .finally) so that setIsLoggingOut(false) always runs, keeping references
to handleLogout, setIsLoggingOut, logoutUser and router.push to locate the
change.
---
Nitpick comments:
In `@src/components/Invites/JoinWaitlistPage.tsx`:
- Around line 91-95: The validateInviteCode function sets isLoading but can
leave it true on unexpected errors; update validateInviteCode to wrap the API
call in try/finally (or try/catch/finally) so setIsLoading(false) always runs,
e.g., setIsLoading(true) then try { await invitesApi.validateInviteCode(code);
return res.success } finally { setIsLoading(false) }; reference the existing
validateInviteCode function and mirror the defensive pattern used in
handleAcceptInvite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4703ae5b-478b-42ea-afdc-75a010e4a791
📒 Files selected for processing (1)
src/components/Invites/JoinWaitlistPage.tsx
- validateInviteCode: wrap in try/finally so isLoading always clears - handleLogout: wrap in try/finally so isLoggingOut always clears - Email input: add aria-label for accessibility
kushagrasarathe
previously approved these changes
Mar 10, 2026
Contributor
kushagrasarathe
left a comment
kushagrasarathe
left a comment
There was a problem hiding this comment.
lgtm, approved, check comments pliz
| } | ||
| return 1 | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []) |
Contributor
There was a problem hiding this comment.
suggestion: add user and isPermissionGranted to dep array
Contributor
Author
There was a problem hiding this comment.
Fixed — useEffect now includes isFetchingUser in deps and also guards against URL bypass (forces email step when no email on file).
| import { useNotifications } from '@/hooks/useNotifications' | ||
| import { updateUserById } from '@/app/actions/users' | ||
|
|
||
| const isValidEmail = (email: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email) |
Contributor
There was a problem hiding this comment.
nit: regex should go in utils
Contributor
Author
There was a problem hiding this comment.
Done — moved to src/utils/format.utils.ts as shared isValidEmail, also deduplicated from withdraw.utils.ts.
|
|
||
| {isPermissionGranted && ( | ||
| {/* Step 2: Enable Notifications (skippable) */} | ||
| {step === 2 && ( |
Contributor
There was a problem hiding this comment.
nit: no back navigation in steps, i might want to have it if i want to change my email after reaching step 2
Contributor
Author
There was a problem hiding this comment.
Intentionally omitted for now — once email is submitted it's saved server-side, so going back to change it would need a different UX (edit vs re-enter). Can add in a follow-up if needed.
| {step === 3 && ( | ||
| <div className="flex h-full flex-col justify-between gap-4 md:gap-10 md:pt-5"> | ||
| {!isPermissionGranted && ( | ||
| <div className="flex items-center justify-between rounded-sm border border-n-2 px-3 py-2"> |
Contributor
There was a problem hiding this comment.
nit: iirc we dont use rounded-sm, we have hard edges, not sure how this looks in ui, but please check
Contributor
Author
There was a problem hiding this comment.
rounded-sm is the design system standard per CLAUDE.md (2px radius). It's used across the app on inputs and cards. The ValidatedInput here inherits it from the existing jail screen — no change from before.
- Step 1: email collection (required, server action) - Step 2: enable notifications (skippable) - Step 3: jail screen with invite code input (always accessible) Uses useQueryState from nuqs for step management, matching the codebase convention for multi-step flows. Step survives refresh, supports deep-linking, and reacts to auth state changes via useEffect. Fixes bug where users got permanently stuck on "Enable notifications" screen when denying permission.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/components/Invites/JoinWaitlistPage.tsx`:
- Around line 71-90: handleEmailSubmit currently calls updateUserById but never
refreshes the user context, causing stale user.user.email; after a successful
update (i.e., result.error is falsy) call and await fetchUser() (the context
refresh function used elsewhere) before calling setStep so the local user state
reflects the new email; handle any fetchUser errors (setEmailError) and ensure
isSubmittingEmail is still toggled in the finally block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb6fa601-59fd-4353-afbb-b5521187a251
📒 Files selected for processing (2)
src/components/Invites/JoinWaitlistPage.tsxsrc/content
✅ Files skipped from review due to trivial changes (1)
- src/content
- Call fetchUser() after email save to keep user context in sync - Move isValidEmail to format.utils.ts for reuse (Kushagra review) - Fix border-n-2 → border-n-1 to match design system convention - Add back navigation from notifications step to email step
…x, optimize query - Use BaseInput component instead of raw <input> for design system consistency - Extract nextStepAfterEmail helper to deduplicate step advancement logic - Reuse isValidEmail from format.utils in withdraw.utils (remove duplicate regex) - Gate waitlist-position query on step === 'jail' to avoid unnecessary API calls - Move loading spinner inside step 3 so steps 1-2 render immediately - Simplify onClick handler (remove unnecessary arrow wrapper)
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/Invites/JoinWaitlistPage.tsx (2)
106-115: SharedisLoadingstate between validation and acceptance could cause subtle UX confusion.
isLoadingis set by bothvalidateInviteCode(debounced validation) andhandleAcceptInvite(submission). While the button is disabled during loading which prevents overlap, the shared state means:
- The "Next" button shows a spinner during background validation (not just submission)
- If validation completes while user is deciding to click, state changes unexpectedly
Consider using separate flags (e.g.,
isValidatingandisAccepting) for clearer semantics, or keep as-is if the current UX is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Invites/JoinWaitlistPage.tsx` around lines 106 - 115, The shared isLoading flag causes background validation (validateInviteCode) to trigger the same UI state as submission (handleAcceptInvite); introduce two booleans (isValidating and isAccepting) in the component, update validateInviteCode to set isValidating true/false, update handleAcceptInvite to set isAccepting true/false, and change any UI bindings (button disabled state and spinner rendering) to use the combined state (disabled when isValidating || isAccepting) but show the spinner only when isAccepting so validation doesn't display submission loading.
34-43: Default step computed at hook initialization — may not reflect late-arriving auth state.The
withDefaultcallback is evaluated once whenuseQueryStateinitializes. IfuserorisPermissionGrantedare not yet hydrated (e.g., SSR → client transition), the default will be computed with stale values and persisted to the URL. While theuseEffecton lines 64-71 corrects this after hydration, users may briefly see the wrong step or experience a flash.This is likely acceptable given the
useEffectauto-correction, but worth noting if you observe UI flicker during initial load.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Invites/JoinWaitlistPage.tsx` around lines 34 - 43, The default step is being computed inside the useQueryState withDefault initializer using potentially stale user/isPermissionGranted values; instead, stop computing the final default during hook initialization (remove the dynamic withDefault computation) and initialize step to a neutral default (e.g., 'email' or leave unset) and rely on the existing effect that runs after auth hydrates to call setStep(nextStepAfterEmail(isPermissionGranted)) when user and isPermissionGranted are available; update the code around useQueryState, step, setStep, and the useEffect that currently corrects the step so the URL-driven state is only adjusted after reliable auth state (user) and permission (isPermissionGranted) are present, preventing the early incorrect default from being persisted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/Invites/JoinWaitlistPage.tsx`:
- Around line 106-115: The shared isLoading flag causes background validation
(validateInviteCode) to trigger the same UI state as submission
(handleAcceptInvite); introduce two booleans (isValidating and isAccepting) in
the component, update validateInviteCode to set isValidating true/false, update
handleAcceptInvite to set isAccepting true/false, and change any UI bindings
(button disabled state and spinner rendering) to use the combined state
(disabled when isValidating || isAccepting) but show the spinner only when
isAccepting so validation doesn't display submission loading.
- Around line 34-43: The default step is being computed inside the useQueryState
withDefault initializer using potentially stale user/isPermissionGranted values;
instead, stop computing the final default during hook initialization (remove the
dynamic withDefault computation) and initialize step to a neutral default (e.g.,
'email' or leave unset) and rely on the existing effect that runs after auth
hydrates to call setStep(nextStepAfterEmail(isPermissionGranted)) when user and
isPermissionGranted are available; update the code around useQueryState, step,
setStep, and the useEffect that currently corrects the step so the URL-driven
state is only adjusted after reliable auth state (user) and permission
(isPermissionGranted) are present, preventing the early incorrect default from
being persisted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3cdf7a2-d135-4e7f-ab51-9eadf908ff7b
📒 Files selected for processing (3)
src/components/Invites/JoinWaitlistPage.tsxsrc/utils/format.utils.tssrc/utils/withdraw.utils.ts
User feedback: the notification hint on the jail screen was out of design system and visually distracting. Notifications are already offered in step 2 — no need to re-prompt on the jail screen.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/components/Invites/JoinWaitlistPage.tsx`:
- Around line 64-71: The effect that advances steps (useEffect in
JoinWaitlistPage) only fast-forwards and allows a URL-backed step like
?step=jail to skip the required email step; update the effect to also enforce
the email requirement by checking if user?.user.email is falsy and step !==
'email' then calling setStep('email') so users without an email are always sent
back to the email step, while keeping the existing forward logic (use the same
variables: step, user?.user.email, isPermissionGranted, setStep,
nextStepAfterEmail).
- Around line 106-114: The validateInviteCode function is incorrectly sharing
the global isLoading state with acceptInvite, which lets a finishing validation
clear the accept spinner; change validation to use a separate loading state
(e.g., isValidating) instead of setIsLoading inside validateInviteCode, and
ensure acceptInvite continues to use isLoading (or isAccepting) so their
lifecycles don't interfere; also after invitesApi.acceptInvite() succeeds, call
fetchUser() to refresh user state (replace any direct user state mutation in
acceptInvite with fetchUser())—update references in validateInviteCode,
acceptInvite, and any other invite-related handlers (lines around 117-130 and
249-255) to use the new distinct flags and fetchUser().
- Around line 58-62: The waitlist-position queryKey should include the current
user's id for consistency and cache isolation; update the useQuery call
(queryKey currently ['waitlist-position']) to include user?.user.userId (e.g.,
['waitlist-position', user?.user.userId]) while leaving queryFn as
invitesApi.getWaitlistQueuePosition() and the enabled condition (enabled:
!!user?.user.userId && step === 'jail') unchanged so the cache key matches other
user-specific queries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 053c5f15-95ac-4668-9b9d-86a5853818b2
📒 Files selected for processing (1)
src/components/Invites/JoinWaitlistPage.tsx
- Prevent URL bypass: enforce email step when no email on file - Split shared isLoading into isValidating/isAccepting to prevent race condition between background validation and invite acceptance - Add userId to waitlist-position queryKey for proper cache isolation - Clear error state on logout
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/Invites/JoinWaitlistPage.tsx (1)
121-135:⚠️ Potential issue | 🟠 MajorKeep invite acceptance locked until
fetchUser()finishes.Line 127 starts
fetchUser()and Line 134 immediately clearsisAccepting. If the auth refresh is slow, the jail screen stays mounted and the user can tap Next again, sending a secondacceptInvite()request for the same code. Await the refresh before dropping the loading state.Based on learnings: In the JoinWaitlistPage component, after successfully accepting an invite via invitesApi.acceptInvite(), calling fetchUser() is sufficient to update the user state and automatically display the app.🩹 Suggested fix
const handleAcceptInvite = async () => { setIsAccepting(true) try { const res = await invitesApi.acceptInvite(inviteCode, inviteType) if (res.success) { sessionStorage.setItem('showNoMoreJailModal', 'true') - fetchUser() + await fetchUser() } else { setError('Something went wrong. Please try again or contact support.') } } catch { setError('Something went wrong. Please try again or contact support.')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Invites/JoinWaitlistPage.tsx` around lines 121 - 135, handleAcceptInvite currently calls fetchUser() but does not await it before clearing the loading flag, allowing duplicate acceptInvite calls; modify handleAcceptInvite so that after invitesApi.acceptInvite(inviteCode, inviteType) returns success you await fetchUser() (e.g., await fetchUser()) before calling setIsAccepting(false), keeping the loading state locked until the auth refresh finishes; ensure error paths (res.success false and the catch block) still set isAccepting(false) and setError appropriately.
🤖 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/components/Invites/JoinWaitlistPage.tsx`:
- Around line 78-79: handleEmailSubmit silently returns if user.user.userId is
missing, and other entry points (the Enter key handler and the submit button)
can bypass loading/validation guards; make all entry points use the same guard
logic: inside handleEmailSubmit, validate isValidEmail(emailValue), ensure
user?.user.userId exists, and return if isSubmittingEmail is true; mirror that
exact guard in the Enter/onKeyDown handler and the button onClick path (and
ensure the button's disabled prop uses the same conditions:
!isValidEmail(emailValue) || isSubmittingEmail || !user?.user.userId); also
preventDefault in the form submit/Enter handler before checking guards to avoid
duplicate requests.
- Around line 99-107: The jail view never knows the user skipped/denied
notifications; update the flow to record that and render the reminder when step
=== 'jail'. Add a boolean state like notificationsSkipped via useState, set it
to true inside the catch block of handleEnableNotifications (and also set it
true from the "Skip" click handler used in the Step 2 branch), and set it false
when permission is successfully granted (e.g., afterPermissionAttempt resolves).
Then update the jail rendering (the component branch that checks step ===
'jail') to conditionally show the skipped-notifications banner/CTA when
notificationsSkipped is true. Ensure afterPermissionAttempt does not overwrite
notificationsSkipped unless permission succeeded.
---
Duplicate comments:
In `@src/components/Invites/JoinWaitlistPage.tsx`:
- Around line 121-135: handleAcceptInvite currently calls fetchUser() but does
not await it before clearing the loading flag, allowing duplicate acceptInvite
calls; modify handleAcceptInvite so that after
invitesApi.acceptInvite(inviteCode, inviteType) returns success you await
fetchUser() (e.g., await fetchUser()) before calling setIsAccepting(false),
keeping the loading state locked until the auth refresh finishes; ensure error
paths (res.success false and the catch block) still set isAccepting(false) and
setError appropriately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb50d6a8-c51d-40ab-9b37-16fd70edc1b2
📒 Files selected for processing (1)
src/components/Invites/JoinWaitlistPage.tsx
Add isSubmittingEmail check to the function itself so Enter key can't bypass the loading guard and fire duplicate requests.
The email submit had silent failure paths where no error was shown to the user. Now every failure path sets an error message, and a "Skip for now" link appears when the email step fails so users aren't permanently stuck.
After email submit succeeds, setStep('notifications') fires but the
useEffect sees stale user data (no email yet) and resets to 'email'.
Fix: track emailStepDone flag set synchronously before setStep, so
the useEffect doesn't override the step transition.
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
2 participants
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
Companion PR
Backend: peanutprotocol/peanut-api-ts#599 — auto-unlock service
Test plan