Feature/free trial frontend#348
Conversation
Task 1 of the free-trial frontend plan: foundation for the signup/activate/progress screens.
RHF form (email/org_name/role) posting to TRIAL_SIGNUP_PATH via apiPublicPost, flipping to a "check your email" confirmation state on success. Handles the 409 (account exists) case with an info toast + link to /login, and generic failures with an error toast. Mirrors the login/forgot-password card markup and AnimatedBackgroundSimple wrapper. Test lives at app/free-trial/__tests__/signup.test.tsx (not __tests__/free-trial/) because jest.config.ts's testMatch only globs app/**/__tests__, components/**/__tests__, hooks/**/__tests__ and lib/**/__tests__ — a __tests__/free-trial/ location is silently never picked up (same is true of the pre-existing __tests__/app/charts/ test, which likewise isn't executed by `npm run test`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirrors app/resetpassword: token from URL via useSearchParams inside
Suspense, RHF password+confirm (minLength 8, match validation). On
success, stashes {email, password} in sessionStorage for the progress
screen's auto-login and routes to /free-trial/progress?task_id=...
Adds email to TrialActivateResponse since the backend now returns it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the final screen of the free-trial flow: /free-trial/progress polls
trial/status via SWR, renders the 8-step CloneProgress list, and on
completion auto-logs the user in (apiPost('/api/v2/login/') using the
creds stashed by the activate page) before redirecting to /impact. On
failure it shows a retry link back to /free-trial.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gin fallback) The completed-status auto-login effect had no try/catch around the login POST and just returned if stashed creds were missing, leaving the user stuck forever on the progress spinner with a plaintext password left in sessionStorage. Since the workspace clone itself succeeded in both cases, both paths now clear the stashed creds and show a "workspace is ready — log in" card linking to /login instead of hanging silently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a public free-trial signup, activation, and clone-progress flow with polling, retries, auto-login, analytics, and route integration. It also adds trial countdown UI and preserves organization-user names in alert recipient displays. ChangesFree-trial onboarding
Alert recipient display
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant SignupPage
participant PublicTrialAPI
participant ActivatePage
participant ProgressPage
participant AuthStore
Visitor->>SignupPage: submit trial details
SignupPage->>PublicTrialAPI: create trial signup
Visitor->>ActivatePage: submit activation token and password
ActivatePage->>PublicTrialAPI: activate trial
PublicTrialAPI-->>ActivatePage: return task_id and email
ActivatePage->>ProgressPage: navigate with task_id
ProgressPage->>PublicTrialAPI: poll clone status
PublicTrialAPI-->>ProgressPage: return progress or terminal status
ProgressPage->>AuthStore: authenticate after completion
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
app/free-trial/progress/__tests__/progress.test.tsx (2)
194-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFake timers aren't restored if an assertion throws.
jest.useRealTimers()(218) only runs on the happy path; a failed assertion earlier in the test leaves fake timers active for subsequent tests in the suite.🧪 Suggested fix
+afterEach(() => { + jest.useRealTimers(); +}); + describe('TrialProgressPage', () => {🤖 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 `@app/free-trial/progress/__tests__/progress.test.tsx` around lines 194 - 219, Update the test “flips to the timeout fallback...” to restore real timers in a guaranteed cleanup path, such as a finally block or the suite’s established cleanup hook, so every assertion failure still resets Jest timers. Keep the existing timeout behavior and assertions unchanged.
88-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for the consecutive-poll-failures give-up path.
Only the hard-timeout half of
pollGaveUp(timedOut) is tested; thepollFailures >= TRIAL_MAX_CONSECUTIVE_POLL_FAILURESbranch (page.tsx lines 121, 126) has no test anywhere in this reviewed set.🤖 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 `@app/free-trial/progress/__tests__/progress.test.tsx` around lines 88 - 277, The TrialProgressPage tests cover only the hard-timeout path and omit the consecutive polling-failures give-up branch. Add a test in the TrialProgressPage suite that makes polling fail until TRIAL_MAX_CONSECUTIVE_POLL_FAILURES is reached, then verifies the timeout fallback renders with the login and retry controls and records the poll-timeout event; keep the existing hard-timeout test unchanged.app/free-trial/_components/CloneProgress.tsx (1)
30-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStep state is visual-only — add a text/ARIA equivalent for screen readers.
done/in-progress/pendingare conveyed only through color and icon; nothing informs assistive tech which step is current or completed.♿ Suggested fix
<li key={label} data-testid={`trial-step-${index}`} data-state={state} + aria-current={state === 'in-progress' ? 'step' : undefined} className="flex items-center gap-3" > ... <span className={cn('text-sm', ...)}> {label} + <span className="sr-only"> + {state === 'done' && ' (completed)'} + {state === 'in-progress' && ' (in progress)'} + </span> </span>🤖 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 `@app/free-trial/_components/CloneProgress.tsx` around lines 30 - 62, Update the step rendering in CloneProgress to expose each state textually for assistive technologies: identify completed, current/in-progress, and pending steps through accessible text or ARIA semantics, including which step is current. Keep the existing visual icons and styling unchanged while ensuring the state information is not conveyed solely by color or icons.Source: Path instructions
types/trial.ts (1)
23-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a literal union for
TrialProgressStep.status.
TrialStatusResponse.statususes a strict literal union, but each per-stepstatushere is a loosestring, losing type safety for whatever finite set of step states the backend actually emits (e.g.pending/running/completed/failed).🤖 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 `@types/trial.ts` around lines 23 - 28, Update TrialProgressStep.status to use a literal union matching the backend’s finite step states, such as pending, running, completed, and failed, instead of string; keep the existing TrialProgressStep shape and optional fields unchanged.components/header.tsx (1)
62-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding coverage for the trial badge's tier thresholds.
TrialBadge's day-based tier logic (≤2red /≤5orange / else teal, plus thedays <= 0"Last day today" label) is new conditional UI logic with no accompanying tests in this batch.🤖 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 `@components/header.tsx` around lines 62 - 83, Add tests for TrialBadge covering the day-based thresholds: red tone and “Last day today” for days <= 0, red for days 1–2, orange for days 3–5, and teal for days above 5. Mock the auth state and trialDaysRemaining as needed, and assert the rendered label and tone classes via data-testid="trial-days-badge".
🤖 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 `@app/free-trial/activate/page.tsx`:
- Around line 57-61: Update the activation flow around the
sessionStorage.setItem call to stop persisting data.password or any raw
credentials. Use the short-lived, one-time login token returned by /activate as
the stored handoff value, and update the progress-screen consumer to read and
consume that token for auto-login.
In `@app/free-trial/page.tsx`:
- Around line 28-52: Instrument the trial-entry funnel across all three sites:
in app/free-trial/page.tsx, add a mount-time page-view event alongside the
existing onSubmit tracking; in app/free-trial/activate/page.tsx, add the
corresponding mount-time activation view event alongside its submit tracking;
and in app/login/page.tsx, update the trial-entry CTA’s onClick to call
trackEvent before navigation. Add or reuse the corresponding TRIAL_* analytics
event symbols, including TRIAL_SIGNUP_VIEWED, TRIAL_ACTIVATE_VIEWED, and
TRIAL_ENTRY_CLICKED.
In `@app/free-trial/progress/page.tsx`:
- Around line 105-127: Add a mount-only page-view analytics event for
ProgressCard using the TRIAL_PROGRESS_VIEWED constant, adding that constant to
constants/analytics.ts if absent. Trigger it once when the progress screen
mounts, without coupling it to polling updates or rerenders, and follow the
existing analytics dispatch pattern used by the other trial events.
In `@components/client-layout.tsx`:
- Around line 27-28: Update the public-route pathname check in the client layout
so the free-trial prefix uses a trailing slash boundary, matching the existing
invitation prefix pattern and preventing unrelated paths that merely begin with
“/free-trial” from bypassing AuthGuard.
In `@constants/trial.ts`:
- Line 27: Update the comment associated with TRIAL_STATUS_POLL_INTERVAL and the
fallback logic around it to reflect the actual 5-second polling cadence: 15
failures correspond to approximately 75 seconds, not 45 seconds. Change only the
stale timing documentation and preserve the existing interval and fallback
behavior.
---
Nitpick comments:
In `@app/free-trial/_components/CloneProgress.tsx`:
- Around line 30-62: Update the step rendering in CloneProgress to expose each
state textually for assistive technologies: identify completed,
current/in-progress, and pending steps through accessible text or ARIA
semantics, including which step is current. Keep the existing visual icons and
styling unchanged while ensuring the state information is not conveyed solely by
color or icons.
In `@app/free-trial/progress/__tests__/progress.test.tsx`:
- Around line 194-219: Update the test “flips to the timeout fallback...” to
restore real timers in a guaranteed cleanup path, such as a finally block or the
suite’s established cleanup hook, so every assertion failure still resets Jest
timers. Keep the existing timeout behavior and assertions unchanged.
- Around line 88-277: The TrialProgressPage tests cover only the hard-timeout
path and omit the consecutive polling-failures give-up branch. Add a test in the
TrialProgressPage suite that makes polling fail until
TRIAL_MAX_CONSECUTIVE_POLL_FAILURES is reached, then verifies the timeout
fallback renders with the login and retry controls and records the poll-timeout
event; keep the existing hard-timeout test unchanged.
In `@components/header.tsx`:
- Around line 62-83: Add tests for TrialBadge covering the day-based thresholds:
red tone and “Last day today” for days <= 0, red for days 1–2, orange for days
3–5, and teal for days above 5. Mock the auth state and trialDaysRemaining as
needed, and assert the rendered label and tone classes via
data-testid="trial-days-badge".
In `@types/trial.ts`:
- Around line 23-28: Update TrialProgressStep.status to use a literal union
matching the backend’s finite step states, such as pending, running, completed,
and failed, instead of string; keep the existing TrialProgressStep shape and
optional fields unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c754ba21-66f6-4f3f-a222-2de60067bd3b
📒 Files selected for processing (19)
app/free-trial/__tests__/signup.test.tsxapp/free-trial/_components/CloneProgress.tsxapp/free-trial/activate/__tests__/activate.test.tsxapp/free-trial/activate/page.tsxapp/free-trial/page.tsxapp/free-trial/progress/__tests__/progress.test.tsxapp/free-trial/progress/page.tsxapp/login/page.tsxcomponents/alerts/AlertWizardModal.tsxcomponents/alerts/RecipientCombobox.tsxcomponents/client-layout.tsxcomponents/header.tsxconstants/analytics.tsconstants/trial.tshooks/api/__tests__/useTrialStatus.test.tsxhooks/api/useTrialStatus.tslib/navigation.tstypes/alerts.tstypes/trial.ts
| // Stash creds for the auto-login on the progress screen (cleared after login). | ||
| sessionStorage.setItem( | ||
| TRIAL_CREDS_STORAGE_KEY, | ||
| JSON.stringify({ email: res.email, password: data.password }) | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching app/free-trial =="
git ls-files | grep -E '^app/free-trial/' | sed -n '1,120p' || true
echo
echo "== candidate page file sizes and outline =="
for f in $(git ls-files 'app/free-trial/**/page.tsx'); do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view compact 2>/dev/null | sed -n '1,160p' || true
done
echo
echo "== inspect free-trial activate page relevant content =="
if [ -f app/free-trial/activate/page.tsx ]; then
cat -n app/free-trial/activate/page.tsx | sed -n '1,180p'
fi
echo
echo "== related storage key and progress page references =="
rg -n "TRIAL_CREDS_STORAGE_KEY|free-trial|activate|progress|sessionStorage|login|password" app -S --glob '*.tsx' --glob '*.ts' | sed -n '1,240p' || trueRepository: DalgoT4D/webapp_v2
Length of output: 2023
🌐 Web query:
DalgoT4D webapp_v2 app/free-trial/activate/page.tsx TRIAL_CREDS_STORAGE_KEY sessionStorage password
💡 Result:
The query involves the DalgoT4D platform, an ecosystem for AI-assisted development [1][2]. While webapp_v2 is a known component repository within the DalgoT4D platform's architecture [1][2], TRIAL_CREDS_STORAGE_KEY and the associated app/free-trial/activate/page.tsx appear to be implementation-specific identifiers. There is no authoritative public documentation or codebase reference indicating that TRIAL_CREDS_STORAGE_KEY is a standard component of the Dalgo platform. In broader web development, storing passwords in sessionStorage—even under a key like TRIAL_CREDS_STORAGE_KEY—is generally considered a security risk [3][4][5]. sessionStorage is accessible to any script running within the same origin, making it vulnerable to Cross-Site Scripting (XSS) attacks [6][5]. Industry best practices for authentication persistence involve using secure, HTTP-only, SameSite cookies or session tokens rather than storing raw credentials in client-side web storage [4][5]. If a developer must persist a session, using a non-sensitive session token or ID is preferred over storing a user's password [4][5].
Citations:
- 1: https://github.com/DalgoT4D/dalgo-core
- 2: https://projecttech4dev.org/how-the-dalgo-team-uses-ai-assisted-development-workflows/
- 3: https://stackoverflow.com/questions/56562395/gun-user-recall-stores-password-as-plain-text-in-sessionstorage-without-encryp
- 4: https://security.stackexchange.com/questions/36958/is-it-safe-to-store-password-in-html5-sessionstorage
- 5: https://stackoverflow.com/questions/26431687/password-in-sessionstorage
- 6: marcodellemarche/cubbit-pages@fa34e8c
Avoid storing the raw password in sessionStorage.
The auto-login flow persists user credentials client-side, so XSS on this origin could read the plaintext password from sessionStorage. Move the handoff to a short-lived, one-time login token returned by /activate and consumed by the progress screen instead.
🤖 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 `@app/free-trial/activate/page.tsx` around lines 57 - 61, Update the activation
flow around the sessionStorage.setItem call to stop persisting data.password or
any raw credentials. Use the short-lived, one-time login token returned by
/activate as the stored handoff value, and update the progress-screen consumer
to read and consume that token for auto-login.
| export default function FreeTrialPage() { | ||
| const [submittedEmail, setSubmittedEmail] = useState<string | null>(null); | ||
| const [accountExists, setAccountExists] = useState(false); | ||
| const { | ||
| register, | ||
| handleSubmit, | ||
| control, | ||
| formState: { errors, isSubmitting }, | ||
| } = useForm<TrialSignupRequest>(); | ||
|
|
||
| const onSubmit = async (data: TrialSignupRequest) => { | ||
| try { | ||
| await apiPublicPost(TRIAL_SIGNUP_PATH, data); | ||
| trackEvent(ANALYTICS_EVENTS.TRIAL_SIGNUP_SUBMITTED); | ||
| setSubmittedEmail(data.email); | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : ''; | ||
| if (message.includes(String(HTTP_STATUS_CONFLICT))) { | ||
| setAccountExists(true); | ||
| toastInfo.generic('An account with this email already exists.'); | ||
| } else { | ||
| toastError.api(error, 'Could not start your trial. Please try again.'); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trial-entry funnel has no view/click analytics instrumentation. Each terminal submit action is tracked (TRIAL_SIGNUP_SUBMITTED, TRIAL_ACTIVATED), but nothing tracks a user actually reaching or clicking into these new pages, so funnel drop-off (CTA click → signup view → activate view → submit) can't be measured. Per coding guidelines, "Instrument every new feature with analytics (view + key actions)... This is a review-blocker."
app/free-trial/page.tsx#L28-L52: fire a page-view event (e.g.TRIAL_SIGNUP_VIEWED) on mount, alongside the existing submit tracking.app/free-trial/activate/page.tsx#L30-L83: fire a page-view event (e.g.TRIAL_ACTIVATE_VIEWED) on mount, alongside the existing submit tracking.app/login/page.tsx#L176-L182: add anonClickhandler that callstrackEvent(e.g.TRIAL_ENTRY_CLICKED) before navigating.
📍 Affects 3 files
app/free-trial/page.tsx#L28-L52(this comment)app/free-trial/activate/page.tsx#L30-L83app/login/page.tsx#L176-L182
🤖 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 `@app/free-trial/page.tsx` around lines 28 - 52, Instrument the trial-entry
funnel across all three sites: in app/free-trial/page.tsx, add a mount-time
page-view event alongside the existing onSubmit tracking; in
app/free-trial/activate/page.tsx, add the corresponding mount-time activation
view event alongside its submit tracking; and in app/login/page.tsx, update the
trial-entry CTA’s onClick to call trackEvent before navigation. Add or reuse the
corresponding TRIAL_* analytics event symbols, including TRIAL_SIGNUP_VIEWED,
TRIAL_ACTIVATE_VIEWED, and TRIAL_ENTRY_CLICKED.
Source: Coding guidelines
| function ProgressCard() { | ||
| const router = useRouter(); | ||
| const searchParams = useSearchParams(); | ||
| const taskId = searchParams.get('task_id'); | ||
| const loginAttemptedRef = useRef(false); | ||
| const pollTimeoutTrackedRef = useRef(false); | ||
| const [manualLoginNeeded, setManualLoginNeeded] = useState(false); | ||
| // consecutive failed status polls (reset to 0 on any success) + hard-timeout flag. | ||
| // Either one flips the screen off the infinite spinner onto the fallback card. | ||
| const [pollFailures, setPollFailures] = useState(0); | ||
| const [timedOut, setTimedOut] = useState(false); | ||
| const [retrying, setRetrying] = useState(false); | ||
|
|
||
| // Give up when the backend is unreachable for a sustained stretch, or when we | ||
| // blow past the hard timeout without a terminal status. Once we give up we null | ||
| // the SWR key so polling stops (data keeps its last value). | ||
| const pollGaveUp = pollFailures >= TRIAL_MAX_CONSECUTIVE_POLL_FAILURES || timedOut; | ||
|
|
||
| const { data, mutate } = useTrialStatus(taskId, { | ||
| enabled: !pollGaveUp, | ||
| onSuccess: () => setPollFailures(0), | ||
| onError: () => setPollFailures((n) => n + 1), | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing page-view analytics event for the progress screen.
Key-action events (TRIAL_POLL_TIMEOUT, TRIAL_MANUAL_LOGIN_REQUIRED, TRIAL_CLONE_COMPLETED, TRIAL_CLONE_FAILED) are all instrumented, but there's no "view" event fired when the progress screen itself mounts.
As per coding guidelines, "Instrument every new feature with analytics (view + key actions)... This is a review-blocker."
📊 Suggested fix
+ useEffect(() => {
+ trackEvent(ANALYTICS_EVENTS.TRIAL_PROGRESS_VIEWED);
+ }, []);
+
const { data, mutate } = useTrialStatus(taskId, {(Add the corresponding TRIAL_PROGRESS_VIEWED constant to constants/analytics.ts if it doesn't already exist.)
📝 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.
| function ProgressCard() { | |
| const router = useRouter(); | |
| const searchParams = useSearchParams(); | |
| const taskId = searchParams.get('task_id'); | |
| const loginAttemptedRef = useRef(false); | |
| const pollTimeoutTrackedRef = useRef(false); | |
| const [manualLoginNeeded, setManualLoginNeeded] = useState(false); | |
| // consecutive failed status polls (reset to 0 on any success) + hard-timeout flag. | |
| // Either one flips the screen off the infinite spinner onto the fallback card. | |
| const [pollFailures, setPollFailures] = useState(0); | |
| const [timedOut, setTimedOut] = useState(false); | |
| const [retrying, setRetrying] = useState(false); | |
| // Give up when the backend is unreachable for a sustained stretch, or when we | |
| // blow past the hard timeout without a terminal status. Once we give up we null | |
| // the SWR key so polling stops (data keeps its last value). | |
| const pollGaveUp = pollFailures >= TRIAL_MAX_CONSECUTIVE_POLL_FAILURES || timedOut; | |
| const { data, mutate } = useTrialStatus(taskId, { | |
| enabled: !pollGaveUp, | |
| onSuccess: () => setPollFailures(0), | |
| onError: () => setPollFailures((n) => n + 1), | |
| }); | |
| function ProgressCard() { | |
| const router = useRouter(); | |
| const searchParams = useSearchParams(); | |
| const taskId = searchParams.get('task_id'); | |
| const loginAttemptedRef = useRef(false); | |
| const pollTimeoutTrackedRef = useRef(false); | |
| const [manualLoginNeeded, setManualLoginNeeded] = useState(false); | |
| // consecutive failed status polls (reset to 0 on any success) + hard-timeout flag. | |
| // Either one flips the screen off the infinite spinner onto the fallback card. | |
| const [pollFailures, setPollFailures] = useState(0); | |
| const [timedOut, setTimedOut] = useState(false); | |
| const [retrying, setRetrying] = useState(false); | |
| useEffect(() => { | |
| trackEvent(ANALYTICS_EVENTS.TRIAL_PROGRESS_VIEWED); | |
| }, []); | |
| // Give up when the backend is unreachable for a sustained stretch, or when we | |
| // blow past the hard timeout without a terminal status. Once we give up we null | |
| // the SWR key so polling stops (data keeps its last value). | |
| const pollGaveUp = pollFailures >= TRIAL_MAX_CONSECUTIVE_POLL_FAILURES || timedOut; | |
| const { data, mutate } = useTrialStatus(taskId, { | |
| enabled: !pollGaveUp, | |
| onSuccess: () => setPollFailures(0), | |
| onError: () => setPollFailures((n) => n + 1), | |
| }); | |
| } |
🤖 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 `@app/free-trial/progress/page.tsx` around lines 105 - 127, Add a mount-only
page-view analytics event for ProgressCard using the TRIAL_PROGRESS_VIEWED
constant, adding that constant to constants/analytics.ts if absent. Trigger it
once when the progress screen mounts, without coupling it to polling updates or
rerenders, and follow the existing analytics dispatch pattern used by the other
trial events.
Source: Coding guidelines
| pathname.startsWith('/invitations/') || | ||
| pathname.startsWith('/free-trial'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Bound the /free-trial prefix match like its siblings.
Every other public-route prefix check here uses a trailing / to bound the match; /free-trial doesn't, so any future route merely starting with those characters (no / boundary) would also incorrectly bypass AuthGuard.
🔒 Suggested fix
pathname.startsWith('/invitations/') ||
- pathname.startsWith('/free-trial');
+ pathname === '/free-trial' ||
+ pathname.startsWith('/free-trial/');📝 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.
| pathname.startsWith('/invitations/') || | |
| pathname.startsWith('/free-trial'); | |
| pathname.startsWith('/invitations/') || | |
| pathname === '/free-trial' || | |
| pathname.startsWith('/free-trial/'); |
🤖 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 `@components/client-layout.tsx` around lines 27 - 28, Update the public-route
pathname check in the client layout so the free-trial prefix uses a trailing
slash boundary, matching the existing invitation prefix pattern and preventing
unrelated paths that merely begin with “/free-trial” from bypassing AuthGuard.
| // dedupingInterval (2000ms in lib/swr.tsx). The hook also sets refreshWhenHidden so | ||
| // polling continues even if the tab is backgrounded — this is a provisioning screen | ||
| // the user watches, and it must keep advancing regardless of tab focus/visibility. | ||
| export const TRIAL_STATUS_POLL_INTERVAL = 5000; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale comment: cadence/timeout math doesn't match TRIAL_STATUS_POLL_INTERVAL.
The comment says "at the 3s cadence, 15 failures ≈ 45s" but TRIAL_STATUS_POLL_INTERVAL is 5000 (5s), so 15 failures is actually ≈75s, not 45s. Update the comment to match the real value so the fallback timing is accurately documented.
📝 Suggested fix
-// Number of consecutive failed status polls before we stop spinning and show the
-// "taking too long" fallback card (e.g. backend unreachable / wrong port). At the
-// 3s cadence, 15 failures ≈ 45s of solid errors before we give up.
+// Number of consecutive failed status polls before we stop spinning and show the
+// "taking too long" fallback card (e.g. backend unreachable / wrong port). At the
+// 5s cadence (TRIAL_STATUS_POLL_INTERVAL), 15 failures ≈ 75s of solid errors before we give up.
export const TRIAL_MAX_CONSECUTIVE_POLL_FAILURES = 15;Also applies to: 30-33
🤖 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 `@constants/trial.ts` at line 27, Update the comment associated with
TRIAL_STATUS_POLL_INTERVAL and the fallback logic around it to reflect the
actual 5-second polling cadence: 15 failures correspond to approximately 75
seconds, not 45 seconds. Change only the stale timing documentation and preserve
the existing interval and fallback behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@app/free-trial/page.tsx`:
- Around line 60-71: Update handleResend to instrument the resend attempt with a
dedicated analytics event, following the event naming and payload conventions in
rules/analytics.md. Invoke trackEvent when the resend action begins, while
preserving the existing resending guard, API call, notifications, and cleanup
behavior.
- Around line 110-119: Update the “Start over” handler in the free-trial form to
reset all React Hook Form fields and clear the accountExists state in addition
to setting submittedEmail to null. Ensure reopening the form starts with
empty/default values and no account-exists banner.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 799fb117-2d4e-4e68-863e-fda8c95e1445
📒 Files selected for processing (2)
app/free-trial/__tests__/signup.test.tsxapp/free-trial/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/free-trial/tests/signup.test.tsx
| const handleResend = async () => { | ||
| if (resending) return; | ||
| setResending(true); | ||
| try { | ||
| await apiPublicPost(TRIAL_SIGNUP_PATH, getValues()); | ||
| toastInfo.generic(`Verification link re-sent to ${submittedEmail}.`); | ||
| } catch (error) { | ||
| toastError.api(error, 'Could not resend the link. Please try again.'); | ||
| } finally { | ||
| setResending(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Instrument the resend action.
handleResend introduces a new user action but never calls trackEvent. Add a dedicated analytics event and track the resend attempt, following rules/analytics.md.
As per coding guidelines, “Instrument every new feature with analytics (view + key actions).”
Suggested instrumentation
try {
+ trackEvent(ANALYTICS_EVENTS.TRIAL_SIGNUP_RESEND_CLICKED);
await apiPublicPost(TRIAL_SIGNUP_PATH, getValues());📝 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 handleResend = async () => { | |
| if (resending) return; | |
| setResending(true); | |
| try { | |
| await apiPublicPost(TRIAL_SIGNUP_PATH, getValues()); | |
| toastInfo.generic(`Verification link re-sent to ${submittedEmail}.`); | |
| } catch (error) { | |
| toastError.api(error, 'Could not resend the link. Please try again.'); | |
| } finally { | |
| setResending(false); | |
| } | |
| }; | |
| const handleResend = async () => { | |
| if (resending) return; | |
| setResending(true); | |
| try { | |
| trackEvent(ANALYTICS_EVENTS.TRIAL_SIGNUP_RESEND_CLICKED); | |
| await apiPublicPost(TRIAL_SIGNUP_PATH, getValues()); | |
| toastInfo.generic(`Verification link re-sent to ${submittedEmail}.`); | |
| } catch (error) { | |
| toastError.api(error, 'Could not resend the link. Please try again.'); | |
| } finally { | |
| setResending(false); | |
| } | |
| }; |
🤖 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 `@app/free-trial/page.tsx` around lines 60 - 71, Update handleResend to
instrument the resend attempt with a dedicated analytics event, following the
event naming and payload conventions in rules/analytics.md. Invoke trackEvent
when the resend action begins, while preserving the existing resending guard,
API call, notifications, and cleanup behavior.
Source: Coding guidelines
| <p className="text-sm text-gray-600"> | ||
| Wrong email?{' '} | ||
| <button | ||
| type="button" | ||
| onClick={() => setSubmittedEmail(null)} | ||
| className="text-primary hover:underline font-medium" | ||
| data-testid="trial-change-email" | ||
| > | ||
| Start over | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset form state when starting over.
setSubmittedEmail(null) only changes the rendered branch; React Hook Form retains the old fields, and accountExists remains true. After a conflict, “Start over” reopens the form with stale values and the account-exists banner.
Suggested fix
const {
getValues,
+ reset,
formState: { errors, isSubmitting },
} = useForm<TrialSignupRequest>();
- onClick={() => setSubmittedEmail(null)}
+ onClick={() => {
+ reset();
+ setAccountExists(false);
+ setSubmittedEmail(null);
+ }}📝 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.
| <p className="text-sm text-gray-600"> | |
| Wrong email?{' '} | |
| <button | |
| type="button" | |
| onClick={() => setSubmittedEmail(null)} | |
| className="text-primary hover:underline font-medium" | |
| data-testid="trial-change-email" | |
| > | |
| Start over | |
| </button> | |
| <p className="text-sm text-gray-600"> | |
| Wrong email?{' '} | |
| <button | |
| type="button" | |
| onClick={() => { | |
| reset(); | |
| setAccountExists(false); | |
| setSubmittedEmail(null); | |
| }} | |
| className="text-primary hover:underline font-medium" | |
| data-testid="trial-change-email" | |
| > | |
| Start over | |
| </button> |
🤖 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 `@app/free-trial/page.tsx` around lines 110 - 119, Update the “Start over”
handler in the free-trial form to reset all React Hook Form fields and clear the
accountExists state in addition to setting submittedEmail to null. Ensure
reopening the form starts with empty/default values and no account-exists
banner.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes