fix: previous teams appear after logging into new user #7729
Conversation
…s and publisher pages
WalkthroughAuthentication-related behaviors were updated: pages now use default withUser options; UserMenu’s logout flow now signs out, clears store, and conditionally redirects via window.location.assign; CreateJourneyButton switches to full-page sign-in redirects with encoded redirect URLs. Corresponding unit tests were updated to mock and assert window.location.assign. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UM as UserMenu
participant Auth as Auth Client
participant Apollo as Apollo Client
participant Win as window.location
U->>UM: Click "Sign out"
UM->>Auth: signOut()
Auth-->>UM: resolved
UM->>Apollo: clearStore()
alt pathname startsWith("/templates")
UM-->>U: No redirect
else
UM->>Win: assign("/users/sign-in?redirect=<encoded current URL>")
end
sequenceDiagram
autonumber
actor U as User
participant CJB as CreateJourneyButton
participant Win as window.location
U->>CJB: Click "Create journey"
CJB->>CJB: Build journey URL (ensure createNew=true)
CJB->>CJB: Encode redirect URL
CJB->>Win: assign("/users/sign-in?redirect=<encoded>&login=<bool>")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
View your CI Pipeline Execution ↗ for commit f512927
☁️ Nx Cloud last updated this comment at |
github-actions
bot
temporarily deployed
to
Preview - journeys-admin
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
…ferent-account-the-discover-page
github-actions
bot
requested a deployment
to
Preview - journeys-admin
github-actions
bot
temporarily deployed
to
Preview - journeys-admin
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsx (2)
103-110: Make intent explicit and capture URL before sign-out.
- Non-awaited client.clearStore(): prefix with void to document intent and avoid unhandled rejections.
- Use a preSignOutUrl snapshot to ensure redirect targets the page the user actually logged out from.
- onClick={async () => { - await user.signOut() - client.clearStore() - if (!router.pathname.startsWith('/templates')) { - const redirectUrl = `/users/sign-in?redirect=${encodeURIComponent(window.location.href)}` - window.location.assign(redirectUrl) - } - }} + onClick={async () => { + const preSignOutUrl = window.location.href + await user.signOut() + void client.clearStore() + if (!router.pathname.startsWith('/templates')) { + const redirectUrl = + `/users/sign-in?redirect=${encodeURIComponent(preSignOutUrl)}` + window.location.assign(redirectUrl) + } + }}
39-44: Remove unused hooks to satisfy lint and reduce bundle.enqueueSnackbar and setActiveTeam are no longer used; drop useSnackbar import/variable and useTeam/setActiveTeam usage.
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx (1)
179-185: Build URLs with URL/URLSearchParams for robustness; drop redundant nullish coalescing.
- String includes('createNew') can false-positive and is brittle for future params; prefer URL APIs.
- login is already boolean in the signature; remove ?? false.
- const redirectUrl = url.includes('createNew') - ? url - : `${url}?createNew=true` - const signInUrl = `${domain}/users/sign-in?redirect=${encodeURIComponent(redirectUrl)}&login=${login ?? false}` - - window.location.assign(signInUrl) + const templateUrl = new URL(`/templates/${journey?.id ?? ''}`, domain) + if (templateUrl.searchParams.get('createNew') !== 'true') { + templateUrl.searchParams.set('createNew', 'true') + } + const signInUrl = new URL('/users/sign-in', domain) + signInUrl.searchParams.set('redirect', templateUrl.toString()) + signInUrl.searchParams.set('login', String(login)) + window.location.assign(signInUrl.toString())libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx (1)
266-276: Solid redirect assertions; add window.location restore to prevent test bleed.The helper is good. To avoid cross-test pollution, snapshot and restore window.location in beforeEach/afterEach.
describe('CreateJourneyButton', () => { const prefetch = jest.fn() const push = jest.fn().mockResolvedValue('') const originalEnv = process.env + // Preserve original window.location + const originalLocation = window.location beforeEach(() => { jest.clearAllMocks() teamResult.mockClear() journeyDuplicateMock.result.mockClear() + Object.defineProperty(window, 'location', { value: originalLocation }) }) + afterEach(() => { + // Restore original window.location after each test + Object.defineProperty(window, 'location', { value: originalLocation }) + })Optionally, extract a small helper to build the expected sign-in URL to keep expectations DRY across tests.
Also applies to: 464-478, 483-497, 523-537, 542-556, 582-596, 601-615
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx (1)
30-41: Great helper; also restore window.location after tests.Like above, snapshot and restore window.location to avoid affecting other tests.
describe('UserMenu', () => { const handleProfileClose = jest.fn() const signOut = jest.fn() + const originalLocation = window.location @@ beforeEach(() => { jest.clearAllMocks() + Object.defineProperty(window, 'location', { value: originalLocation }) }) + afterEach(() => { + Object.defineProperty(window, 'location', { value: originalLocation }) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/journeys-admin/pages/index.tsx(1 hunks)apps/journeys-admin/pages/publisher/index.tsx(1 hunks)apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx(2 hunks)apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsx(1 hunks)libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx(4 hunks)libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsxlibs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsxapps/journeys-admin/pages/index.tsxlibs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsxapps/journeys-admin/pages/publisher/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsxlibs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsxapps/journeys-admin/pages/index.tsxlibs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsxapps/journeys-admin/pages/publisher/index.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsxapps/journeys-admin/pages/index.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsxapps/journeys-admin/pages/publisher/index.tsx
🧬 Code graph analysis (2)
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx (1)
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsx (1)
UserMenu(32-119)
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx (1)
apps/journeys-admin-e2e/src/pages/login-page.ts (1)
login(56-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Preview (videos-admin, 7729/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys-admin, 7729/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys, 7729/merge, pull_request, 22)
- GitHub Check: Deploy Preview (watch, 7729/merge, pull_request, 22)
- GitHub Check: build (22)
- GitHub Check: lint (22)
🔇 Additional comments (3)
apps/journeys-admin/pages/publisher/index.tsx (1)
137-139: Double-check CSR unauthenticated flow on Publisher.Mirrors the Index change; please verify that navigating client-side while signed out still redirects as expected and does not flash content.
You can quickly validate in dev by starting signed-out and hitting this route via Link from another page, ensuring a redirect to /users/sign-in occurs and no content is rendered beforehand.
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx (1)
126-178: Logout flow tests align with new behavior.Assertions for signOut, clearStore, and conditional redirect look correct and match the implementation.
Please also run these in watch mode to ensure no flakiness due to the non-awaited clearStore promise.
Also applies to: 180-229
apps/journeys-admin/pages/index.tsx (1)
99-101: Confirm CSR redirect or re-enable whenUnauthedAfterInitapps/journeys-admin/pages/index.tsx has whenUnauthedAfterInit commented out (≈lines 99–101); getServerSideProps still uses withUserTokenSSR and returns a redirect when user == null, so SSR (and Next's client-side navigation that fetches props) should redirect — either restore whenUnauthedAfterInit here or add a client-side useEffect to router.replace('/users/sign-in') when user is unauthenticated; verify clicking a Link to this page while signed out redirects.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsx (1)
105-109: Harden logout: clear cache even if signOut fails; use early return for readability.Wrap signOut/clearStore in try/finally so the Apollo cache is always cleared, and use an early return to skip redirect on /templates. Optionally consider
location.replaceto avoid the user navigating back into an authed page after logout.- await user.signOut() - await client.clearStore() - if (!router.pathname.startsWith('/templates')) { - const redirectUrl = `/users/sign-in?redirect=${encodeURIComponent(window.location.href)}` - window.location.assign(redirectUrl) - } + try { + await user.signOut() + } finally { + await client.clearStore() + } + if (router.pathname.startsWith('/templates')) return + const redirectUrl = `/users/sign-in?redirect=${encodeURIComponent(window.location.href)}` + window.location.assign(redirectUrl) // or window.location.replace(redirectUrl)libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx (1)
179-185: Build URLs with the URL API to avoid edge-case bugs and ensure proper encoding.Safer with existing/extra query params and clearer intent.
Confirm
NEXT_PUBLIC_JOURNEYS_ADMIN_URLis set in all consuming apps; otherwise the redirect will use the host project’s origin.- const redirectUrl = url.includes('createNew') - ? url - : `${url}?createNew=true` - const signInUrl = `${domain}/users/sign-in?redirect=${encodeURIComponent(redirectUrl)}&login=${login ?? false}` - - window.location.assign(signInUrl) + const target = new URL(url) + if (!target.searchParams.has('createNew')) { + target.searchParams.set('createNew', 'true') + } + const signIn = new URL(`${domain}/users/sign-in`) + signIn.searchParams.set('redirect', target.toString()) + signIn.searchParams.set('login', String(!!login)) + window.location.assign(signIn.toString())libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx (1)
266-277: DRY the window.location.assign mock and ensure cleanup between tests.This helper is duplicated in other specs; extract to a shared test util and restore
window.locationafter each test to avoid cross-test contamination.+// At top-level (near other consts) +const originalLocation = window.location + +afterEach(() => { + Object.defineProperty(window, 'location', { value: originalLocation, writable: true }) +})Consider moving
mockWindowLocationAssignto a shared helper, e.g.test/utils/mockWindow.ts:export function mockWindowLocationAssign(origin: string) { const mockAssign = jest.fn() Object.defineProperty(window, 'location', { value: { ...window.location, origin, assign: mockAssign }, writable: true }) return mockAssign }apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx (2)
30-41: Share the location.assign mock and restore window.location after each test.Mirror the shared helper suggested in CreateJourneyButton.spec and restore the original location to prevent leaks.
+const originalLocation = window.location +afterEach(() => { + Object.defineProperty(window, 'location', { value: originalLocation, writable: true }) +})
126-178: Good: verifies signOut → clearStore → redirect, and no-redirect on /templates.Consider adding one negative-path test where
signOutrejects to ensure we still clear the cache and avoid redirect once you adopt the try/finally change.I can add that test once the logout try/finally refactor lands.
Also applies to: 180-229
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/journeys-admin/pages/index.tsx(1 hunks)apps/journeys-admin/pages/publisher/index.tsx(1 hunks)apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx(2 hunks)apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsx(1 hunks)libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx(4 hunks)libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsxlibs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsxapps/journeys-admin/pages/index.tsxapps/journeys-admin/pages/publisher/index.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsxlibs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsxapps/journeys-admin/pages/index.tsxapps/journeys-admin/pages/publisher/index.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsxapps/journeys-admin/pages/index.tsxapps/journeys-admin/pages/publisher/index.tsxapps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx
🧬 Code graph analysis (1)
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.spec.tsx (1)
apps/journeys-admin/src/components/PageWrapper/NavigationDrawer/UserNavigation/UserMenu/UserMenu.tsx (1)
UserMenu(32-119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Deploy Preview (videos-admin, 7729/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys-admin, 7729/merge, pull_request, 22)
- GitHub Check: Deploy Preview (watch, 7729/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys, 7729/merge, pull_request, 22)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
- GitHub Check: test (22, 1/3)
- GitHub Check: lint (22)
🔇 Additional comments (3)
apps/journeys-admin/pages/index.tsx (1)
99-101: LGTM: client-side auto-redirect removed; SSR guard remains.With
withUserTokenSSR({ whenUnauthed: AuthAction.REDIRECT_TO_LOGIN })still in place, behavior stays protected.Please sanity-check for any client-side flicker of unauthed content on slow networks.
apps/journeys-admin/pages/publisher/index.tsx (1)
137-139: LGTM: aligned with index page; SSR still enforces auth.Consistent removal of
whenUnauthedAfterInitlooks good.libs/journeys/ui/src/components/TemplateView/CreateJourneyButton/CreateJourneyButton.spec.tsx (1)
464-478: Assertions look solid; keep using encoded expectations.Good coverage of both login/create-account and cross-origin cases.
Also applies to: 483-499, 523-537, 542-556, 582-596, 601-615
|
This pull request has been marked stale due to 28 days without activity. It will be closed in 14 days unless updated. |
Summary by CodeRabbit
New Features
Bug Fixes
Changes