feat: preserve login params in forgot password screen#819
Merged
Conversation
dosubot
Bot
added
the
size:S
Contributor
📝 WalkthroughWalkthroughThe changes modify the authentication flow to preserve URL query parameters when navigating between login and forgot-password pages. LoginPage now passes query parameters to LoginForm, which uses them to construct the forgot-password link destination. ForgotPasswordPage similarly preserves parameters when redirecting back to login. Translation keys are added for UI consistency. Changes
Sequence DiagramsequenceDiagram
actor User
participant LoginPage
participant LoginForm
participant ForgotPasswordPage
User->>LoginPage: Navigate with ?ref=xyz
LoginPage->>LoginPage: Extract searchParams
LoginPage->>LoginForm: Pass params prop (?ref=xyz)
LoginForm->>User: Render forgot password link
User->>LoginForm: Click forgot password
LoginForm->>ForgotPasswordPage: Navigate to /forgot-password?ref=xyz
ForgotPasswordPage->>ForgotPasswordPage: useLocation captures ?ref=xyz
ForgotPasswordPage->>User: Render back to login button
User->>ForgotPasswordPage: Click back to login
ForgotPasswordPage->>LoginPage: window.location.replace(/login?ref=xyz)
LoginPage->>User: Restored with original params
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
frontend/src/pages/forgot-password-page.tsx (1)
18-20: You can simplify redirect construction by usingsearchdirectly.
searchalready contains either""or"?...", so re-parsing/re-serializing is unnecessary here.♻️ Suggested simplification
const { t } = useTranslation(); const { search } = useLocation(); - const searchParams = new URLSearchParams(search); ... onClick={() => { - const eparams = searchParams.toString(); - window.location.replace( - `/login${eparams.length > 0 ? `?${eparams}` : ""}`, - ); + window.location.replace(`/login${search}`); }}Also applies to: 40-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/forgot-password-page.tsx` around lines 18 - 20, The code re-parses the location query string by creating a URLSearchParams from useLocation().search (variables search and searchParams) even though search already contains the raw query string ("" or "?..."); remove the URLSearchParams creation and any re-serialization and instead use search directly when constructing redirects/links (e.g., where you currently use searchParams.toString() or build "?"+searchParams), update the occurrences around the redirect construction and the similar block at lines 40-43 to reference search directly so you preserve the leading "?" when present and avoid unnecessary parsing.frontend/src/components/auth/login-form.tsx (1)
74-80: Keephrefaligned with the click redirect target.Right now the rendered link target omits params while
onClickincludes them. Making both use the same computed URL avoids inconsistent behavior.♻️ Suggested update
<a - href="/forgot-password" + href={`/forgot-password${props.params ?? ""}`} onClick={(e) => { e.preventDefault(); - window.location.replace( - `/forgot-password${props.params ? `${props.params}` : ""}`, - ); + window.location.replace(e.currentTarget.href); }} className="text-muted-foreground hover:text-muted-foreground/80 text-sm absolute right-0 bottom-[2.565rem]" // 2.565 is *just* perfect >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/auth/login-form.tsx` around lines 74 - 80, The href and onClick redirect are out of sync; compute a single target URL once (e.g., const target = `/forgot-password${props.params ? props.params : ""}`) in the LoginForm component and use that same target for both the href attribute and inside the onClick handler (replace the current inline string in window.location.replace). Update references in the onClick closure and href to use the shared target so the rendered link matches the programmatic redirect.frontend/src/pages/login-page.tsx (1)
267-270: Consider removing the inline IIFE from JSX.This works, but deriving the query suffix once above the return improves readability and avoids repeating URL-construction logic across pages.
♻️ Suggested refactor
+ const querySuffix = search ? search : ""; ... <LoginForm onSubmit={(values) => loginMutate(values)} loading={loginIsPending || oauthIsPending} formId={formId} - params={(() => { - const eparams = searchParams.toString(); - return eparams.length > 0 ? `?${eparams}` : ""; - })()} + params={querySuffix} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/pages/login-page.tsx` around lines 267 - 270, The JSX currently computes the query suffix inline with an IIFE for the params prop; pull that logic out above the component's return by creating a single const (e.g., querySuffix or paramsSuffix) that builds `searchParams.toString()` and sets it to either `'?'+eparams` or `''`, then replace the IIFE in the params prop with that const; update usages in this file (login-page component, the params prop) so URL-construction logic is centralized and not repeated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/auth/login-form.tsx`:
- Around line 74-80: The href and onClick redirect are out of sync; compute a
single target URL once (e.g., const target = `/forgot-password${props.params ?
props.params : ""}`) in the LoginForm component and use that same target for
both the href attribute and inside the onClick handler (replace the current
inline string in window.location.replace). Update references in the onClick
closure and href to use the shared target so the rendered link matches the
programmatic redirect.
In `@frontend/src/pages/forgot-password-page.tsx`:
- Around line 18-20: The code re-parses the location query string by creating a
URLSearchParams from useLocation().search (variables search and searchParams)
even though search already contains the raw query string ("" or "?..."); remove
the URLSearchParams creation and any re-serialization and instead use search
directly when constructing redirects/links (e.g., where you currently use
searchParams.toString() or build "?"+searchParams), update the occurrences
around the redirect construction and the similar block at lines 40-43 to
reference search directly so you preserve the leading "?" when present and avoid
unnecessary parsing.
In `@frontend/src/pages/login-page.tsx`:
- Around line 267-270: The JSX currently computes the query suffix inline with
an IIFE for the params prop; pull that logic out above the component's return by
creating a single const (e.g., querySuffix or paramsSuffix) that builds
`searchParams.toString()` and sets it to either `'?'+eparams` or `''`, then
replace the IIFE in the params prop with that const; update usages in this file
(login-page component, the params prop) so URL-construction logic is centralized
and not repeated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b01900b1-2db9-4fc1-ae72-e4349a50fa56
📒 Files selected for processing (5)
frontend/src/components/auth/login-form.tsxfrontend/src/lib/i18n/locales/en-US.jsonfrontend/src/lib/i18n/locales/en.jsonfrontend/src/pages/forgot-password-page.tsxfrontend/src/pages/login-page.tsx
Rycochet
approved these changes
Apr 26, 2026
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.
Fixes #803
Summary by CodeRabbit
New Features
Refactor