feat: guest user merge#8717
Draft
csiyang wants to merge 34 commits intosiyangcao/nes-1272-create-guest-userfrom
Draft
feat: guest user merge#8717csiyang wants to merge 34 commits intosiyangcao/nes-1272-create-guest-userfrom
csiyang wants to merge 34 commits intosiyangcao/nes-1272-create-guest-userfrom
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request implements anonymous user account conversion and redirection logic. It adds server-side and client-side redirects for authenticated non-anonymous users, enables converting anonymous accounts to permanent accounts via email/password or social providers, and introduces journey ID extraction and sign-up button components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RegisterPage
participant Firebase as Firebase Auth
participant Apollo as Apollo Client
participant Router
User->>RegisterPage: Click "Create Account"
RegisterPage->>RegisterPage: Check if user.isAnonymous
alt Anonymous User
RegisterPage->>Apollo: Call UPDATE_ME mutation
Apollo->>Apollo: Update profile (name, email)
RegisterPage->>Firebase: Call linkWithCredential
Firebase->>Firebase: Link email/password to account
RegisterPage->>Firebase: Update display name
RegisterPage->>RegisterPage: Check for journeyId
alt Journey ID Present
RegisterPage->>Apollo: Call JOURNEY_PUBLISH mutation
Apollo->>Apollo: Publish journey
end
RegisterPage->>Firebase: Re-sign in with new email/password
else Non-Anonymous User
RegisterPage->>Firebase: Standard createAccountAndSignIn
end
Firebase->>Router: User authenticated
Router->>User: Redirect or render dashboard
sequenceDiagram
participant User
participant SignInServiceButton
participant Firebase as Firebase Auth
participant Apollo as Apollo Client
User->>SignInServiceButton: Click social provider button
SignInServiceButton->>SignInServiceButton: Get current user
alt Anonymous User
SignInServiceButton->>Firebase: Call linkWithPopup(provider)
Firebase->>User: Open provider popup
User->>Firebase: Authenticate with provider
Firebase->>Firebase: Link provider to anonymous account
SignInServiceButton->>Apollo: Call UPDATE_ME mutation
Apollo->>Apollo: Update profile with provider data
SignInServiceButton->>SignInServiceButton: Check for journeyId
alt Journey ID Present
SignInServiceButton->>Apollo: Call JOURNEY_PUBLISH mutation
Apollo->>Apollo: Publish journey
end
else Standard User
SignInServiceButton->>Firebase: Call signInWithPopup(provider)
Firebase->>User: Open provider popup
User->>Firebase: Authenticate with provider
Firebase->>Firebase: Sign in / link account
end
Firebase->>User: User authenticated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
mikeallisonJS
approved these changes
Feb 11, 2026
csiyang
force-pushed
the
siyangcao/nes-1272-create-guest-user
branch
from
5e19b9e to
221398e
Compare
…s-1271-merge-guest-user
csiyang
force-pushed
the
siyangcao/nes-1271-merge-guest-user
branch
from
a5a41f4 to
2eb476c
Compare
…s-1271-merge-guest-user
…s-1271-merge-guest-user
Contributor
…s-1271-merge-guest-user
…s-1271-merge-guest-user
…s-1271-merge-guest-user
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 5
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/src/components/SignIn/RegisterPage/RegisterPage.spec.tsx (1)
53-61:⚠️ Potential issue | 🟠 MajorIsolate tests by clearing shared mocks in
beforeEach.Several assertions use
.toHaveBeenCalled(), but shared mocks are not cleared per test. This can hide regressions.✅ Suggested fix
beforeEach(() => { + jest.clearAllMocks() mockUseRouter.mockReturnValue({ back, push: jest.fn(), query: { redirect: null } } as unknown as NextRouter) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/SignIn/RegisterPage/RegisterPage.spec.tsx` around lines 53 - 61, Tests share mocked functions causing cross-test interference; update the beforeEach in RegisterPage.spec.tsx to clear or reset mocks at the start (e.g., call jest.clearAllMocks() and/or mockUseRouter.mockClear()) before calling mockUseRouter.mockReturnValue so each test starts with fresh mock state; reference the existing beforeEach, mockUseRouter, back, push, and query to locate the block to modify.
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsx (1)
34-34: Use the barrel export for consistent import paths.The import directly references
SignUpButton.tsxinstead of using the index re-export. This is inconsistent with the project's pattern of exporting components throughindex.tsfiles.♻️ Suggested fix
-import { SignUpButton } from '../../CustomizeFlowNextButton/SignUpButton/SignUpButton' +import { SignUpButton } from '../../CustomizeFlowNextButton/SignUpButton'Based on learnings: "Export all components through index.ts files."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsx` at line 34, The import in LinksScreen.tsx uses a direct path to SignUpButton; change it to use the barrel export so the component is imported from the parent index instead (replace the direct import of SignUpButton with an import from the CustomizeFlowNextButton directory's index export), ensuring you still reference the SignUpButton symbol where used in LinksScreen.
🤖 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/pages/users/sign-in.tsx`:
- Around line 42-45: Wrap the redirect parsing in a try-catch so malformed input
doesn't crash SSR: in getAppRedirectDestination (initAuth.ts) catch errors from
decodeURIComponent and from creating new URL(redirectUrl) and return a safe
fallback destination (e.g., homepage or sign-in) on error; then in sign-in.tsx
(where getAppRedirectDestination is called) ensure you call it inside a
try-catch or rely on its safe fallback so the returned redirect object is always
valid and never throws.
In `@apps/journeys-admin/src/components/SignIn/RegisterPage/RegisterPage.tsx`:
- Around line 114-126: The GraphQL mutation updateMe is currently run before
linking the Firebase account (linkWithCredential), which can leave persisted
profile data if linking fails; move the updateMe call to after successful
linkWithCredential and updateProfile so the database is only changed once
EmailAuthProvider.credential + linkWithCredential(currentUser, credential)
succeeds and userCredential.user has been updated; specifically, perform
EmailAuthProvider.credential(email, password), await
linkWithCredential(currentUser, credential), await
updateProfile(userCredential.user, { displayName: name }), then call
updateMe({...}) to persist firstName/lastName/email.
In
`@apps/journeys-admin/src/components/SignIn/SignInServiceButton/SignInServiceButton.spec.tsx`:
- Around line 51-123: The tests share mockSignInWithPopup and mockLinkWithPopup
but never clear their call history, causing inter-test leakage; update the spec
to reset these mocks between tests (e.g., add a beforeEach or afterEach that
calls jest.clearAllMocks() or individually calls mockSignInWithPopup.mockClear()
and mockLinkWithPopup.mockClear()) so each it block starts with a fresh mock
state; ensure this reset sits alongside the existing beforeEach/afterEach that
set mockGetAuth and mockUseRouter so SignInServiceButton tests for "new user"
and "guest user" are isolated.
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/CustomizeFlowNextButton/SignUpButton/SignUpButton.tsx`:
- Around line 12-28: The handleClick function builds an absolute domain but
passes it as pathname to router.push; change the router.push call to use a
relative pathname ('/users/sign-in') and keep the absolute redirectUrl only in
the query. Update the router.push invocation in handleClick to pass { pathname:
'/users/sign-in', query: { redirect: redirectUrl } } and remove the unnecessary
{ shallow: true } option so navigation works correctly.
In `@apps/journeys-admin/src/libs/firebaseClient/initAuth.ts`:
- Around line 20-31: The code calls new URL(redirectUrl) which throws for
relative paths; wrap the URL construction in a try-catch and handle invalid URL
parse errors in initAuth: when redirectUrl is present attempt const parsed = new
URL(redirectUrl) in a try block, call allowedHost(parsed.host,
ALLOWED_REDIRECT_HOSTS) as before, and in the catch treat redirectUrl as a safe
relative path (or reject/return null per existing flow) so the function doesn't
crash on inputs like "/templates/abc/customize"; reference redirectUrl,
ALLOWED_REDIRECT_HOSTS, allowedHost, and the new URL(...) call when making the
change.
---
Outside diff comments:
In
`@apps/journeys-admin/src/components/SignIn/RegisterPage/RegisterPage.spec.tsx`:
- Around line 53-61: Tests share mocked functions causing cross-test
interference; update the beforeEach in RegisterPage.spec.tsx to clear or reset
mocks at the start (e.g., call jest.clearAllMocks() and/or
mockUseRouter.mockClear()) before calling mockUseRouter.mockReturnValue so each
test starts with fresh mock state; reference the existing beforeEach,
mockUseRouter, back, push, and query to locate the block to modify.
---
Nitpick comments:
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsx`:
- Line 34: The import in LinksScreen.tsx uses a direct path to SignUpButton;
change it to use the barrel export so the component is imported from the parent
index instead (replace the direct import of SignUpButton with an import from the
CustomizeFlowNextButton directory's index export), ensuring you still reference
the SignUpButton symbol where used in LinksScreen.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/journeys-admin/__generated__/JourneyPublish.tsis excluded by!**/__generated__/**
📒 Files selected for processing (16)
apps/journeys-admin/pages/users/sign-in.tsxapps/journeys-admin/src/components/SignIn/RegisterPage/RegisterPage.spec.tsxapps/journeys-admin/src/components/SignIn/RegisterPage/RegisterPage.tsxapps/journeys-admin/src/components/SignIn/SignIn.spec.tsxapps/journeys-admin/src/components/SignIn/SignIn.tsxapps/journeys-admin/src/components/SignIn/SignInServiceButton/SignInServiceButton.spec.tsxapps/journeys-admin/src/components/SignIn/SignInServiceButton/SignInServiceButton.tsxapps/journeys-admin/src/components/SignIn/utils/getJourneyIdFromRedirect/getJourneyIdFromRedirect.spec.tsapps/journeys-admin/src/components/SignIn/utils/getJourneyIdFromRedirect/getJourneyIdFromRedirect.tsapps/journeys-admin/src/components/SignIn/utils/getJourneyIdFromRedirect/index.tsapps/journeys-admin/src/components/SignIn/utils/index.tsapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/CustomizeFlowNextButton/SignUpButton/SignUpButton.spec.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/CustomizeFlowNextButton/SignUpButton/SignUpButton.tsxapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/CustomizeFlowNextButton/SignUpButton/index.tsapps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/Screens/LinksScreen/LinksScreen.tsxapps/journeys-admin/src/libs/firebaseClient/initAuth.ts
apps/journeys-admin/src/components/SignIn/RegisterPage/RegisterPage.tsx
Outdated
Show resolved
Hide resolved
apps/journeys-admin/src/components/SignIn/SignInServiceButton/SignInServiceButton.spec.tsx
Show resolved
Hide resolved
Comment on lines
+12
to
+28
| function handleClick(): void { | ||
| const domain = | ||
| process.env.NEXT_PUBLIC_JOURNEYS_ADMIN_URL ?? window.location.origin | ||
| const redirectUrl = | ||
| journeyId != null | ||
| ? `${domain}/templates/${journeyId}/customize` | ||
| : `${domain}/` | ||
|
|
||
| void router.push( | ||
| { | ||
| pathname: `${domain}/users/sign-in`, | ||
| query: { redirect: redirectUrl } | ||
| }, | ||
| undefined, | ||
| { shallow: true } | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how router.push is used with sign-in pages
rg -n -A5 "router\.push.*sign-in" --type=ts --type=tsx -g '!*.spec.*'Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Check the full SignUpButton.tsx file context
cat -n apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/CustomizeFlowNextButton/SignUpButton/SignUpButton.tsxRepository: JesusFilm/core
Length of output: 1276
🏁 Script executed:
#!/bin/bash
# Search for router.push patterns with absolute URLs or domains
rg -n "pathname.*domain\|pathname.*process\.env" --type=ts --type=tsx | head -30Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Check for any router.push patterns with shallow: true option
rg -n "shallow\s*:\s*true" --type=ts --type=tsx -B3 -A1Repository: JesusFilm/core
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Search for router.push patterns without strict type checking
rg "router\.push" -A3 --type=ts | grep -A3 "pathname.*domain\|pathname.*http\|shallow" | head -50Repository: JesusFilm/core
Length of output: 4094
🏁 Script executed:
#!/bin/bash
# Search for any router.push calls to understand the pattern
rg "router\.push" -B2 -A4 --type=ts | head -100Repository: JesusFilm/core
Length of output: 9786
🏁 Script executed:
#!/bin/bash
# Look for how sign-in navigation is handled elsewhere
rg "sign-in\|sign_in" -B3 -A3 --type=ts | head -80Repository: JesusFilm/core
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check if there are other uses of shallow: true in the codebase
rg "shallow\s*:" --type=ts -B2 -A2 | head -50Repository: JesusFilm/core
Length of output: 4120
Fix invalid absolute URL in router.push pathname.
The pathname property in router.push must be a relative path (e.g., /users/sign-in), not an absolute URL. Using ${domain}/users/sign-in will cause navigation to fail. The domain variable should only be used to construct the redirectUrl query parameter.
Additionally, shallow: true is unnecessary for this cross-page navigation and should be removed.
Fix
function handleClick(): void {
const domain =
process.env.NEXT_PUBLIC_JOURNEYS_ADMIN_URL ?? window.location.origin
const redirectUrl =
journeyId != null
? `${domain}/templates/${journeyId}/customize`
: `${domain}/`
- void router.push(
- {
- pathname: `${domain}/users/sign-in`,
- query: { redirect: redirectUrl }
- },
- undefined,
- { shallow: true }
- )
+ void router.push({
+ pathname: '/users/sign-in',
+ query: { redirect: redirectUrl }
+ })
}📝 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.
Suggested change
| function handleClick(): void { | |
| const domain = | |
| process.env.NEXT_PUBLIC_JOURNEYS_ADMIN_URL ?? window.location.origin | |
| const redirectUrl = | |
| journeyId != null | |
| ? `${domain}/templates/${journeyId}/customize` | |
| : `${domain}/` | |
| void router.push( | |
| { | |
| pathname: `${domain}/users/sign-in`, | |
| query: { redirect: redirectUrl } | |
| }, | |
| undefined, | |
| { shallow: true } | |
| ) | |
| } | |
| function handleClick(): void { | |
| const domain = | |
| process.env.NEXT_PUBLIC_JOURNEYS_ADMIN_URL ?? window.location.origin | |
| const redirectUrl = | |
| journeyId != null | |
| ? `${domain}/templates/${journeyId}/customize` | |
| : `${domain}/` | |
| void router.push({ | |
| pathname: '/users/sign-in', | |
| query: { redirect: redirectUrl } | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/journeys-admin/src/components/TemplateCustomization/MultiStepForm/CustomizeFlowNextButton/SignUpButton/SignUpButton.tsx`
around lines 12 - 28, The handleClick function builds an absolute domain but
passes it as pathname to router.push; change the router.push call to use a
relative pathname ('/users/sign-in') and keep the absolute redirectUrl only in
the query. Update the router.push invocation in handleClick to pass { pathname:
'/users/sign-in', query: { redirect: redirectUrl } } and remove the unnecessary
{ shallow: true } option so navigation works correctly.
…s-1271-merge-guest-user
…s-1271-merge-guest-user
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
Release Notes
New Features
Bug Fixes