Merged
Conversation
|
View your CI Pipeline Execution ↗ for commit 88dce4e
☁️ Nx Cloud last updated this comment at |
Contributor
|
The latest updates on your projects.
|
mikeallisonJS
previously approved these changes
Apr 7, 2026
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:
WalkthroughServer-side verify now validates Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SSR as Server (getServerSideProps)
participant API as GraphQL
Browser->>SSR: GET /users/verify?email=...&token=...(&redirect=...)
SSR->>API: VALIDATE_EMAIL(email, token)
alt VALIDATE_EMAIL succeeds
API-->>SSR: success
SSR-->>Browser: HTTP redirect -> /users/terms-and-conditions (with encoded redirect if provided) or redirectToApp(ctx)
else VALIDATE_EMAIL fails
API-->>SSR: error
SSR-->>Browser: render verify page with props (email, token, initialError: null, initialApolloState)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Merge main to incorporate guest login flow fixes from #8945. Resolved conflict in verify.tsx by taking main's simplified redirectToApp approach. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cts-to-dashboard-instead-of
Contributor
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
stage-branch-merger Bot
added a commit
that referenced
this pull request
Apr 7, 2026
…board-instead-of' (PR #8955) into stage
stage-branch-merger Bot
added a commit
that referenced
this pull request
Apr 7, 2026
…board-instead-of' (PR #8955) into stage
stage-branch-merger Bot
added a commit
that referenced
this pull request
Apr 7, 2026
…board-instead-of' (PR #8955) into stage
…cts-to-dashboard-instead-of
stage-branch-merger Bot
added a commit
that referenced
this pull request
Apr 7, 2026
…board-instead-of' (PR #8955) into stage
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/journeys-admin/pages/users/verify.tsx (1)
265-273:⚠️ Potential issue | 🟠 MajorUse the query email in the fallback render path.
This catch path is the exact stale-cookie fallback, but
ValidateEmailstill ignoresprops.emailand derives the address fromuser?.email. If the SSR cookie belongs to another account, the page can render and retry against the wrong email.🔧 Suggested fix
function ValidateEmail({ + email: initialEmail, token, initialError = null }: ValidateEmailProps): ReactElement { const { t } = useTranslation('apps-journeys-admin') const router = useRouter() const client = useApolloClient() const { user } = useAuth() - const email = user?.email ?? '' + const email = initialEmail ?? user?.email ?? ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/pages/users/verify.tsx` around lines 265 - 273, The fallback/stale-cookie path currently derives the address from user?.email instead of the SSR-provided prop; update the ValidateEmail render logic to use the incoming prop email (props.email) as the source of truth when present and only fall back to user?.email if props.email is missing, so the retry/confirmation flow targets the SSR-provided address rather than an account from a stale cookie. Locate the Verify page component and the ValidateEmail usage/implementation and change the email selection to prefer the email prop, and ensure any validation/errors/messages also reference props.email in that code path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/journeys-admin/src/libs/checkConditionalRedirect/checkConditionalRedirect.ts`:
- Around line 47-52: Reject external/non-app-relative redirect values before
using them: in checkConditionalRedirect (variables currentRedirect, resolvedUrl,
redirect, encodedRedirect) validate that any candidate redirect is an internal
path (e.g., begins with a single "/" and does not start with "//" or contain a
scheme like "http://" or "https://") before assigning redirect or building
encodedRedirect; if validation fails, ignore that candidate and fall back to
resolvedUrl or default behavior. Apply the same validation logic to the other
branch referenced in the file (the block handling lines ~92-105) so both places
only reuse safe internal paths.
---
Outside diff comments:
In `@apps/journeys-admin/pages/users/verify.tsx`:
- Around line 265-273: The fallback/stale-cookie path currently derives the
address from user?.email instead of the SSR-provided prop; update the
ValidateEmail render logic to use the incoming prop email (props.email) as the
source of truth when present and only fall back to user?.email if props.email is
missing, so the retry/confirmation flow targets the SSR-provided address rather
than an account from a stale cookie. Locate the Verify page component and the
ValidateEmail usage/implementation and change the email selection to prefer the
email prop, and ensure any validation/errors/messages also reference props.email
in that code path.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 90f48592-c138-4dd8-b7a8-9b2805217ee3
📒 Files selected for processing (3)
apps/journeys-admin/pages/users/verify.tsxapps/journeys-admin/src/libs/checkConditionalRedirect/checkConditionalRedirect.spec.tsxapps/journeys-admin/src/libs/checkConditionalRedirect/checkConditionalRedirect.ts
stage-branch-merger Bot
added a commit
that referenced
this pull request
Apr 8, 2026
…board-instead-of' (PR #8955) into stage
stage-branch-merger Bot
added a commit
that referenced
this pull request
Apr 8, 2026
…board-instead-of' (PR #8955) into stage
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
`@apps/journeys-admin/src/libs/checkConditionalRedirect/checkConditionalRedirect.ts`:
- Around line 39-42: The redirect check uses the raw query-string encoded
`redirect` value (assigned to requestedRedirect from resolvedUrl) which makes
the `/users/verify` escape hatch fail when query order/encoding differs; instead
parse the redirect into a URL and use its pathname for comparisons. Update the
logic that builds and compares `encodedRedirect`/startsWith checks (the blocks
around the requestedRedirect usage and the `/users/verify${encodedRedirect}`
comparison, including the earlier 50-58 and later 80-81 checks) to derive `const
redirectPath = new URL(requestedRedirect, 'https://admin.nextstep.is').pathname`
(or equivalent) and use `redirectPath` (or pathname-based startsWith) when
matching `/users/verify` and related route checks.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 647c3459-e355-4e4f-9faa-e13d2c52f85c
📒 Files selected for processing (2)
apps/journeys-admin/src/libs/checkConditionalRedirect/checkConditionalRedirect.spec.tsxapps/journeys-admin/src/libs/checkConditionalRedirect/checkConditionalRedirect.ts
✅ Files skipped from review due to trivial changes (1)
- apps/journeys-admin/src/libs/checkConditionalRedirect/checkConditionalRedirect.spec.tsx
Contributor
|
Merge conflict attempting to merge this into stage. Please fix manually. |
…cts-to-dashboard-instead-of
Contributor
|
Merge conflict attempting to merge this into stage. Please fix manually. |
mikeallisonJS
approved these changes
Apr 8, 2026
csiyang
deleted the
siyangcao/nes-1525-email-verification-redirects-to-dashboard-instead-of
branch
tanflem
pushed a commit
that referenced
this pull request
Apr 13, 2026
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 by CodeRabbit
Bug Fixes
Tests