feat(web): polish auth and profile flows#405
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughBackend: email verification now issues session tokens and sets auth cookies; user follow creation and follower-read APIs added with in-app NEW_FOLLOWER notifications. Frontend: follow/unfollow UI, social-graph list pages, auth layout/page UI updates, settings and left-nav UX changes, and middleware session cookie detection expanded. ChangesFollow System with Email Verification Tokens & Social UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/domains/users/user/user.service.ts (1)
255-280:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake follow creation atomic to avoid duplicate-follow race failures.
The separate existence check before insert introduces a race window. Two concurrent requests can both pass the check and one request will fail during insert on the unique key path instead of returning a clean conflict response.
Suggested fix
- const existingFollow = await this.prisma.followRelation.findUnique({ - where: { - followerId_targetUserId: { - followerId, - targetUserId, - }, - }, - select: { id: true }, - }); - - if (existingFollow) { - throw new ConflictException({ - message: "Already following this user", - code: "FOLLOW_ALREADY_EXISTS", - }); - } - - const [, follower] = await this.prisma.$transaction([ - this.prisma.followRelation.create({ - data: { - followerId, - targetUserId, - targetType: FollowTargetType.USER, - }, - select: { id: true }, - }), - this.prisma.user.findUnique({ - where: { id: followerId }, - select: { - id: true, - username: true, - displayName: true, - }, - }), - ]); + let follower: + | { id: string; username: string | null; displayName: string | null } + | null + | undefined; + try { + [, follower] = await this.prisma.$transaction([ + this.prisma.followRelation.create({ + data: { + followerId, + targetUserId, + targetType: FollowTargetType.USER, + }, + select: { id: true }, + }), + this.prisma.user.findUnique({ + where: { id: followerId }, + select: { + id: true, + username: true, + displayName: true, + }, + }), + ]); + } catch (error) { + throw new ConflictException({ + message: "Already following this user", + code: "FOLLOW_ALREADY_EXISTS", + }); + }🤖 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 `@apps/backend/src/domains/users/user/user.service.ts` around lines 255 - 280, The pre-check using this.prisma.followRelation.findUnique (existingFollow) creates a race; remove the separate existence check and make follow creation atomic by attempting the insert (this.prisma.followRelation.create / the current this.prisma.$transaction call) and catching unique constraint/database errors; on unique-constraint failure throw the same ConflictException with message "Already following this user" and code "FOLLOW_ALREADY_EXISTS" so concurrent requests return a clean conflict rather than relying on the separate findUnique check.
🧹 Nitpick comments (2)
apps/web/src/components/layout/LeftNav.tsx (1)
220-237: 💤 Low valueConsider adding ARIA roles for complete menu accessibility.
The trigger button has
aria-haspopup="menu"andaria-expanded, but the popup panel lacksrole="menu"and the logout button lacksrole="menuitem". Adding these would complete the ARIA menu pattern.Suggested enhancement
- <div className="absolute bottom-[calc(100%+0.5rem)] left-0 hidden min-w-[240px] rounded-2xl bg-[var(--color-espresso)] p-2 text-[var(--color-bianca)] shadow-lg lg:block"> + <div role="menu" className="absolute bottom-[calc(100%+0.5rem)] left-0 hidden min-w-[240px] rounded-2xl bg-[var(--color-espresso)] p-2 text-[var(--color-bianca)] shadow-lg lg:block"> <button type="button" + role="menuitem" onClick={handleLogout}Also applies to: 255-268
🤖 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 `@apps/web/src/components/layout/LeftNav.tsx` around lines 220 - 237, The popup panel rendered when accountMenuOpen is true should include ARIA menu roles—add role="menu" to the container div that currently renders the account menu and add role="menuitem" to the logout button (the button that calls handleLogout and uses isLoggingOut/visibleUsername). Ensure these attributes are added in the same JSX where accountMenuOpen is used so the trigger's aria-haspopup="menu" and aria-expanded remain consistent with the panel and menu item roles.apps/web/src/app/(auth)/verify-email/page.tsx (1)
58-63: ⚡ Quick winFrontend type doesn't match backend contract.
The backend's
verifyEmailmethod (auth.service.ts:473) returns{ verified: true; redirectTo: string }whereredirectTois required, but the frontend types it as optional (redirectTo?: string). While the fallback logic handles this safely, the type should reflect the actual API contract.📝 Suggested type correction
- const result = await api.post<{ verified: true; redirectTo?: string }>( + const result = await api.post<{ verified: true; redirectTo: string }>( "/auth/email/verify", { email, code }, ); setPageState("success"); - setTimeout(() => router.push(result.redirectTo || next), 1500); + setTimeout(() => router.push(result.redirectTo), 1500);🤖 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 `@apps/web/src/app/`(auth)/verify-email/page.tsx around lines 58 - 63, The frontend types for the email verification response are too permissive: update the API call in verify-email page (the api.post call that assigns to result in page.tsx) to match the backend auth.service.ts verifyEmail contract by changing the response type from { verified: true; redirectTo?: string } to { verified: true; redirectTo: string } so redirectTo is required; keep the existing fallback logic (result.redirectTo || next) if desired but ensure the generic type for api.post and any related usages reflect the required redirectTo.
🤖 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
`@apps/web/src/app/`(public)/u/[username]/_components/SocialGraphPageClient.tsx:
- Around line 51-84: Replace the manual useEffect/useState fetch with TanStack
Query: use useQuery(['socialGraph', username, listType], () =>
loadSocialGraph(username, listType)) to drive result/isLoading/error instead of
setResult/setIsLoading/setError, and for the follow/unfollow mutation code (the
handlers around lines 281-312) replace imperative API calls with useMutation
hooks that call your follow/unfollow API and onSuccess invalidate the
social-graph query (useQueryClient().invalidateQueries(['socialGraph',
username]) and/or invalidate by listType with ['socialGraph', username,
listType]) so post-mutation UI is consistent; ensure you import and use
useQuery, useMutation, and useQueryClient from `@tanstack/react-query` and remove
the active-flag cleanup logic from the old effect.
- Around line 106-111: The back-button uses a 40×40 touch area (className
contains h-10 w-10) which is under the 44×44 mobile minimum; update the button
in SocialGraphPageClient (the element with aria-label="Back to profile" and
onClick={() => router.push(profileHref)}) to provide at least 44×44px — e.g.,
replace h-10 w-10 with h-11 w-11 or add min-h-[44px] min-w-[44px] to the
className so the touch target meets the guideline.
In `@apps/web/src/app/`(public)/u/[username]/PublicUserProfileClient.tsx:
- Around line 212-214: In PublicUserProfileClient, update the heading elements
to use the heading font classes: replace "font-cabinet" with "font-syne
font-bold" on the <h1> that renders {displayName} and the other heading at the
block around lines 243-245; keep "font-cabinet" for body text elsewhere so only
the heading elements change.
- Around line 203-208: The back button in PublicUserProfileClient (the <button>
that calls handleBackNavigation) uses h-10 w-10 (40×40) which is below the 44×44
mobile touch target; update its className to provide at least 44×44 hit area
(for example replace h-10 w-10 with h-11 w-11 or otherwise ensure min 44px
width/height) so the button meets the mobile minimum touch target while
preserving existing styles like rounded-full and focus/hover classes.
In `@apps/web/src/app/`(shopper)/buyer/settings/SettingsClient.tsx:
- Around line 336-343: The back button in SettingsClient uses h-10 w-10
(40×40px) which is below the 44×44px touch-target requirement; update the button
element in SettingsClient.tsx (the <button> that wraps ArrowLeft and calls
router.push(getPublicUserHref(profile.username))) so its interactive size is at
least 44×44px—either change the classes to h-11 w-11 (or min-h-11 min-w-11) or
add equivalent padding (e.g., p-1) while keeping the icon ArrowLeft at h-5 w-5
to preserve visual sizing; ensure CSS focus/hover classes remain applied.
---
Outside diff comments:
In `@apps/backend/src/domains/users/user/user.service.ts`:
- Around line 255-280: The pre-check using this.prisma.followRelation.findUnique
(existingFollow) creates a race; remove the separate existence check and make
follow creation atomic by attempting the insert
(this.prisma.followRelation.create / the current this.prisma.$transaction call)
and catching unique constraint/database errors; on unique-constraint failure
throw the same ConflictException with message "Already following this user" and
code "FOLLOW_ALREADY_EXISTS" so concurrent requests return a clean conflict
rather than relying on the separate findUnique check.
---
Nitpick comments:
In `@apps/web/src/app/`(auth)/verify-email/page.tsx:
- Around line 58-63: The frontend types for the email verification response are
too permissive: update the API call in verify-email page (the api.post call that
assigns to result in page.tsx) to match the backend auth.service.ts verifyEmail
contract by changing the response type from { verified: true; redirectTo?:
string } to { verified: true; redirectTo: string } so redirectTo is required;
keep the existing fallback logic (result.redirectTo || next) if desired but
ensure the generic type for api.post and any related usages reflect the required
redirectTo.
In `@apps/web/src/components/layout/LeftNav.tsx`:
- Around line 220-237: The popup panel rendered when accountMenuOpen is true
should include ARIA menu roles—add role="menu" to the container div that
currently renders the account menu and add role="menuitem" to the logout button
(the button that calls handleLogout and uses isLoggingOut/visibleUsername).
Ensure these attributes are added in the same JSX where accountMenuOpen is used
so the trigger's aria-haspopup="menu" and aria-expanded remain consistent with
the panel and menu item roles.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5c1870b3-4f5d-4f3a-95ae-a8c49edd2c49
📒 Files selected for processing (20)
apps/backend/src/domains/social/notification/notification.service.tsapps/backend/src/domains/users/auth/auth.controller.tsapps/backend/src/domains/users/auth/auth.service.registration.spec.tsapps/backend/src/domains/users/auth/auth.service.tsapps/backend/src/domains/users/user/user.controller.tsapps/backend/src/domains/users/user/user.module.tsapps/backend/src/domains/users/user/user.service.tsapps/web/src/app/(auth)/layout.tsxapps/web/src/app/(auth)/login/page.tsxapps/web/src/app/(auth)/register/page.tsxapps/web/src/app/(auth)/verify-email/page.tsxapps/web/src/app/(public)/u/[username]/PublicUserProfileClient.tsxapps/web/src/app/(public)/u/[username]/_components/SocialGraphPageClient.tsxapps/web/src/app/(public)/u/[username]/followers/page.tsxapps/web/src/app/(public)/u/[username]/following/page.tsxapps/web/src/app/(public)/u/[username]/verified_followers/page.tsxapps/web/src/app/(shopper)/buyer/settings/SettingsClient.tsxapps/web/src/components/layout/LeftNav.tsxapps/web/src/lib/user.tsapps/web/src/middleware.ts
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 `@apps/web/src/components/layout/LeftNav.tsx`:
- Around line 225-229: The logout button currently calls handleLogout and
navigates away even when the /auth/logout request fails under JwtAuthGuard
(e.g., expired access token), which lets a stale refresh cookie keep the
session; update handleLogout to treat non-2xx responses as failures and, on a
401 (or any failure), call a server endpoint that explicitly clears HttpOnly
cookies without requiring auth (e.g., a new POST /auth/clear-cookies or
/auth/logout/public) so both cookies are removed server-side; only navigate away
after a successful clear and ensure isLoggingOut is set/unset correctly on both
success and failure paths and errors are surfaced instead of swallowed.
- Around line 221-227: The popup in LeftNav.tsx currently uses
role="menu"/role="menuitem" (the div with role="menu" and the button with
role="menuitem") but does not implement APG menu-button behavior; either
implement full WAI-ARIA menu semantics (on open move focus into first menu item,
implement ArrowUp/ArrowDown/Home/End keyboard handlers with roving tabindex or
aria-activedescendant, and restore focus to the trigger and update aria-expanded
on close) or remove the menu-specific ARIA roles/aria-haspopup/aria-expanded
from the trigger and menu items and rely on native button semantics; pick one
approach, update the code around the popup div and trigger button in LeftNav.tsx
accordingly, and ensure focus restoration if you implement the APG behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6c0fb0c-6120-4985-9d7f-3ab6b49afa35
📒 Files selected for processing (6)
apps/backend/src/domains/users/user/user.service.tsapps/web/src/app/(auth)/verify-email/page.tsxapps/web/src/app/(public)/u/[username]/PublicUserProfileClient.tsxapps/web/src/app/(public)/u/[username]/_components/SocialGraphPageClient.tsxapps/web/src/app/(shopper)/buyer/settings/SettingsClient.tsxapps/web/src/components/layout/LeftNav.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/backend/src/domains/users/user/user.service.ts
- apps/web/src/app/(public)/u/[username]/_components/SocialGraphPageClient.tsx
- apps/web/src/app/(auth)/verify-email/page.tsx
- apps/web/src/app/(public)/u/[username]/PublicUserProfileClient.tsx
- apps/web/src/app/(shopper)/buyer/settings/SettingsClient.tsx
| <div | ||
| role="menu" | ||
| className="absolute bottom-[calc(100%+0.5rem)] left-0 hidden min-w-[240px] rounded-2xl bg-[var(--color-espresso)] p-2 text-[var(--color-bianca)] shadow-lg lg:block" | ||
| > | ||
| <button | ||
| type="button" | ||
| role="menuitem" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For a menu button pattern using role="menu" and role="menuitem", what keyboard and focus-management behaviors are required by the WAI-ARIA Authoring Practices?
💡 Result:
According to the WAI-ARIA Authoring Practices (APG), a menu button is a button that opens a menu, typically styled with an indicator like a downward-pointing arrow [1]. The pattern requires the following keyboard and focus-management behaviors: Keyboard Behaviors for the Menu Button When focus is on the button: - Enter or Space: Opens the menu and places focus on the first menu item [1][2]. - Down Arrow (optional): Opens the menu and moves focus to the first menu item [1][2]. - Up Arrow (optional): Opens the menu and moves focus to the last menu item [1][2]. Keyboard Behaviors for the Menu Once the menu is open, the menu acts as a composite widget where Tab and Shift + Tab do not move focus among the menu items [3]. Instead, users navigate using the following keys: - Down Arrow: Moves focus to the next menu item; if focus is on the last item, it wraps to the first [4][5]. - Up Arrow: Moves focus to the previous menu item; if focus is on the first item, it wraps to the last [4][5]. - Home: Moves focus to the first menu item [4][6]. - End: Moves focus to the last menu item [4][6]. - Escape: Closes the menu and returns focus to the menu button [4][5][6]. - A-Z/a-z: Moves focus to the next menu item starting with the typed character [4][6]. - Enter/Space: Activates the menu item and closes the menu [4][5][6]. Focus Management The APG mandates that the menu button must manage focus such that it returns to the button when the menu is closed [4][6][1]. Within the menu, focus management is typically implemented using one of two patterns: - Roving Tabindex: Each menu item has tabindex="-1" (to be focusable via script), and JavaScript manages focus movement using element.focus [5][3]. - aria-activedescendant: The menu container has tabindex="0" (or -1) and uses the aria-activedescendant property to indicate the currently "focused" item while the DOM focus remains on the container [4][3]. Additionally, the menu button must use the following roles, states, and properties: - Role: button [1]. - aria-haspopup: Set to "menu" or "true" [1]. - aria-expanded: Set to "true" when the menu is open and "false" when it is hidden [1]. - aria-controls: Optionally used to refer to the element with role menu [1].
Citations:
- 1: https://www.w3.org/WAI/ARIA/apg/patterns/menu-button/
- 2: https://github.com/w3c/aria-practices/blob/main/content/patterns/menu-button/menu-button-pattern.html
- 3: https://www.w3.org/WAI/ARIA/apg/patterns/menubar/
- 4: https://www.w3.org/WAI/ARIA/apg/patterns/menu-button/examples/menu-button-actions-active-descendant/
- 5: https://www.w3.org/WAI/ARIA/apg/patterns/menu-button/examples/menu-button-actions/
- 6: https://www.w3.org/WAI/ARIA/apg/patterns/menu-button/examples/menu-button-links/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/web/src/components/layout/LeftNav.tsx"
# Ensure file exists
test -f "$FILE"
echo "== Lines around 221-240 =="
nl -ba "$FILE" | sed -n '200,260p'
echo
echo "== Search for menu-button/focus-management patterns =="
rg -n --hidden --no-mmap -S "role=\"menu\"|role=\"menuitem\"|aria-activedescendant|aria-haspopup|aria-expanded|aria-controls|Escape|onKeyDown|keyDown|ArrowDown|ArrowUp|Home|End|tabindex=\"-1\"|focus\(|restore focus|activeDescendant|roving" "$FILE"
echo
echo "== Search for refs/focusable menu items =="
rg -n --hidden --no-mmap -S "useRef|useEffect|querySelectorAll|menuItem|menuItemRefs|first.*item|last.*item|focusFirst|focusLast" "$FILE"Repository: coded-devs/twizrr
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/web/src/components/layout/LeftNav.tsx"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
echo "== Lines around 221-240 =="
cat -n "$FILE" | sed -n '200,260p'
echo
echo "== Search for menu-button/focus-management patterns =="
rg -n --hidden --no-mmap -S 'role="menu"|role="menuitem"|aria-activedescendant|aria-haspopup|aria-expanded|aria-controls|Escape|onKeyDown|keyDown|ArrowDown|ArrowUp|Home|End|tabindex="-1"|focus\(|restore focus|activeDescendant|roving|menu-button|menubutton' "$FILE"
echo
echo "== Search for refs/focusable menu items =="
rg -n --hidden --no-mmap -S 'useRef|useEffect|querySelectorAll|menuItem|menuItemRefs|first.*item|last.*item|focusFirst|focusLast|aria-activedescendant' "$FILE"Repository: coded-devs/twizrr
Length of output: 3598
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/web/src/components/layout/LeftNav.tsx"
test -f "$FILE"
echo "== Imports / state / effects (1-120) =="
cat -n "$FILE" | sed -n '1,140p'
echo
echo "== Escape handling block (50-110) =="
cat -n "$FILE" | sed -n '40,110p'
echo
echo "== Account menu toggle button (240-320) =="
cat -n "$FILE" | sed -n '240,330p'Repository: coded-devs/twizrr
Length of output: 9588
Fix ARIA menu semantics to match the WAI-ARIA APG focus/keyboard contract.
The popup uses role="menu"/role="menuitem" and the trigger has aria-haspopup="menu"/aria-expanded, but it only closes via outside-click and Escape—it does not (a) move focus to the first menu item when opened, (b) restore focus to the trigger when closed, or (c) implement the APG keyboard navigation for an open menu (ArrowUp/ArrowDown/Home/End with roving focus via tabindex and/or aria-activedescendant). Either implement the full APG menu-button behavior or remove role="menu"/role="menuitem" and rely on native button semantics.
🤖 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 `@apps/web/src/components/layout/LeftNav.tsx` around lines 221 - 227, The popup
in LeftNav.tsx currently uses role="menu"/role="menuitem" (the div with
role="menu" and the button with role="menuitem") but does not implement APG
menu-button behavior; either implement full WAI-ARIA menu semantics (on open
move focus into first menu item, implement ArrowUp/ArrowDown/Home/End keyboard
handlers with roving tabindex or aria-activedescendant, and restore focus to the
trigger and update aria-expanded on close) or remove the menu-specific ARIA
roles/aria-haspopup/aria-expanded from the trigger and menu items and rely on
native button semantics; pick one approach, update the code around the popup div
and trigger button in LeftNav.tsx accordingly, and ensure focus restoration if
you implement the APG behavior.
| <button | ||
| type="button" | ||
| role="menuitem" | ||
| onClick={handleLogout} | ||
| disabled={isLoggingOut} |
There was a problem hiding this comment.
Don't treat logout failures as success when the refresh cookie can still mark a session.
/auth/logout is guarded by JwtAuthGuard, so an expired access token will 401 before the backend clears the HttpOnly cookies. This click path still swallows that failure and navigates away; with the new middleware behavior that treats either the access or refresh cookie as a valid session marker, a stale refresh cookie can immediately keep the user signed in after “logout”. Make the logout flow clear both cookies even when only the refresh token remains, or route failures through a server-side clear-cookie path instead of ignoring them.
🤖 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 `@apps/web/src/components/layout/LeftNav.tsx` around lines 225 - 229, The
logout button currently calls handleLogout and navigates away even when the
/auth/logout request fails under JwtAuthGuard (e.g., expired access token),
which lets a stale refresh cookie keep the session; update handleLogout to treat
non-2xx responses as failures and, on a 401 (or any failure), call a server
endpoint that explicitly clears HttpOnly cookies without requiring auth (e.g., a
new POST /auth/clear-cookies or /auth/logout/public) so both cookies are removed
server-side; only navigate away after a successful clear and ensure isLoggingOut
is set/unset correctly on both success and failure paths and errors are surfaced
instead of swallowed.
1 participant
Summary
Details
/u/[username]/followers/u/[username]/following/u/[username]/verified_followersValidation
npx.cmd tsc --noEmitpnpm.cmd test -- auth.service.registration.spec.ts --runInBand(8/8)pnpm.cmd run buildpnpm.cmd run lintnpx.cmd tsc --noEmitpnpm.cmd run buildgit diff --checkpassedNotes
Summary by CodeRabbit
New Features
Improvements
Bug Fixes