Feature/oauth profile completion #248
Conversation
- Improve first name and last name extraction from OAuth providers (Google, GitHub, etc.) - Support multiple OAuth provider data formats: - first_name/given_name for first name - last_name/family_name for last name - name field parsing for full name splitting - Prioritize OAuth provider data over existing profile data - Add visual indicators showing when fields are pre-filled from OAuth - Enhance profile service to better handle OAuth metadata during profile creation - Add debugging logs to track OAuth provider data extraction
- Fix useCallback dependency array by removing unnecessary firstName/lastName dependencies - Wrap useSearchParams in Suspense boundary to fix Next.js build error - Refactor CompleteProfile component into CompleteProfileContent with Suspense wrapper - Add proper loading fallback for Suspense boundary - Resolve all build and linting issues for production deployment
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces Suspense to the complete-profile page, refactors component structure, and updates OAuth-based name prefill/display logic. Enhances profile creation service to robustly derive first/last names from various OAuth metadata fields. Adds debugging logs and adjusts effect dependencies related to returnUrl. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NextPage as CompleteProfile (Next.js Page)
participant Suspense as React Suspense
participant Auth as Auth/User Session
participant Prefill as OAuth Name Parser
participant UI as Form UI
User->>NextPage: Navigate to /complete-profile?returnUrl=...
NextPage->>Suspense: Render with fallback (loading)
Note over Suspense: New Suspense boundary
Suspense->>Auth: Fetch session/user (async)
Auth-->>Suspense: user + user_metadata
Suspense->>Prefill: Derive first/last name from metadata
Note right of Prefill: Uses first_name/given_name/name<br/>last_name/family_name/name remainder
Prefill-->>UI: Set state: firstName/lastName + oauthProvider
UI-->>User: Render form (labels show "(pre-filled from {provider})" when applicable)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/complete-profile/page.tsx (1)
25-33: Sanitize returnUrl to prevent open redirects.As written, a crafted returnUrl like https://evil.com would be accepted and pushed. Restrict to same‑origin paths.
Apply this minimal fix:
- const returnUrl = searchParams.get('returnUrl') || '/protected/dashboard'; + const rawReturnUrl = searchParams.get('returnUrl'); + const returnUrl = + rawReturnUrl && rawReturnUrl.startsWith('/') && !rawReturnUrl.startsWith('//') + ? rawReturnUrl + : '/protected/dashboard';
🧹 Nitpick comments (5)
lib/services/profile.ts (1)
44-58: Normalize OAuth name parsing and de-duplicate logic with the page code.Trim and collapse whitespace before splitting, and centralize this logic to avoid divergent behavior between service and UI. Also handles single‑token names more safely.
[secretly suggests changes; propose diff below]
- // Extract name data from OAuth provider metadata - const metadata = user.user?.user_metadata || {}; - - // Extract first name from various OAuth provider formats - const firstName = metadata.first_name || - metadata.given_name || - metadata.name?.split(' ')[0] || - ''; - - // Extract last name from various OAuth provider formats - const lastName = metadata.last_name || - metadata.family_name || - metadata.name?.split(' ').slice(1).join(' ') || - ''; + // Extract name data from OAuth provider metadata + const metadata = user.user?.user_metadata || {}; + const fullName = typeof metadata.name === 'string' + ? metadata.name.trim().replace(/\s+/g, ' ') + : ''; + const [firstToken, ...restTokens] = fullName ? fullName.split(' ') : []; + // Extract first/last name from various OAuth provider formats (trimmed) + const firstName = (metadata.first_name || metadata.given_name || firstToken || '').trim(); + const lastName = (metadata.last_name || metadata.family_name || restTokens.join(' ') || '').trim();Optional follow-up: extract a shared helper (e.g., lib/utils/name.ts) and reuse here and in app/complete-profile/page.tsx to keep behavior identical.
app/complete-profile/page.tsx (4)
86-120: Get provider from app_metadata (not user_metadata) and align name parsing with service.Supabase sets provider under user.app_metadata.provider (or identities[].provider), not user_metadata. Also, mirror the normalized name parsing used in the service to avoid drift.
- // Pre-fill from OAuth provider data if available (prioritize OAuth data over existing profile data) + // Pre-fill from OAuth provider data if available (prioritize OAuth data over existing profile data) if (user.user_metadata) { const metadata = user.user_metadata; - - // Extract first name from various OAuth provider formats - const oauthFirstName = metadata.first_name || - metadata.given_name || - metadata.name?.split(' ')[0] || - ''; - - // Extract last name from various OAuth provider formats - const oauthLastName = metadata.last_name || - metadata.family_name || - metadata.name?.split(' ').slice(1).join(' ') || - ''; + // Normalize and extract names + const fullName = typeof metadata.name === 'string' + ? metadata.name.trim().replace(/\s+/g, ' ') + : ''; + const [firstToken, ...restTokens] = fullName ? fullName.split(' ') : []; + const oauthFirstName = (metadata.first_name || metadata.given_name || firstToken || '').trim(); + const oauthLastName = (metadata.last_name || metadata.family_name || restTokens.join(' ') || '').trim(); // Use OAuth data if available, otherwise keep existing profile data if (oauthFirstName) { setFirstName(oauthFirstName); } if (oauthLastName) { setLastName(oauthLastName); } - - // Set OAuth provider for UI display - setOauthProvider(metadata.provider || 'unknown'); + // Set OAuth provider for UI display (prefer app_metadata; fallback to first identity) + const provider = + (user as any).app_metadata?.provider || + (Array.isArray((user as any).identities) && (user as any).identities[0]?.provider) || + ''; + setOauthProvider(provider); - console.log('OAuth provider data:', { - provider: metadata.provider || 'unknown', - firstName: oauthFirstName, - lastName: oauthLastName, - fullName: metadata.name, - metadata: metadata - }); + if (process.env.NODE_ENV !== 'production') { + console.debug('OAuth provider:', provider, { + firstName: oauthFirstName, + lastName: oauthLastName, + fullName: metadata.name, + }); + } }Additionally, extend the local User interface to include app_metadata/identities if you want to keep type safety.
113-119: Remove/guard PII‑bearing debug logs.Avoid logging full metadata in production; it may contain PII. The diff above swaps console.log for a production‑guarded console.debug without dumping the entire metadata object.
316-320: Hide or prettify provider label when unknown; improve UX.Currently this can show “pre‑filled from unknown”. Either don’t render when provider is falsy/unknown or map ids to brand names.
Option A (minimal; if you keep setting 'unknown'):
- First Name * {oauthProvider && firstName && ( + First Name * {oauthProvider && oauthProvider !== 'unknown' && firstName && ( ... - Last Name * {oauthProvider && lastName && ( + Last Name * {oauthProvider && oauthProvider !== 'unknown' && lastName && (Option B: after the provider fix above, map ids to labels:
const PROVIDER_LABELS: Record<string,string> = { google: 'Google', github: 'GitHub', apple: 'Apple' }; const providerLabel = PROVIDER_LABELS[oauthProvider] || '';Then render only when providerLabel is set.
Also applies to: 335-339
494-508: Suspense wrapper likely redundant here.This page is a client component and doesn’t render any child that actually suspends. You already have an explicit “isValidating” skeleton, so this double‑spinner can be removed.
-export default function CompleteProfile() { - return ( - <Suspense fallback={ - <div className="min-h-screen flex items-center justify-center bg-gradient-to-br from-background via-muted/30 to-muted/50"> - <div className="relative"> - <div className="animate-spin rounded-full h-12 w-12 border-4 border-primary border-t-transparent"></div> - <div className="absolute inset-0 rounded-full border-4 border-primary/20 animate-ping"></div> - </div> - <span className="ml-4 text-lg text-muted-foreground">Loading...</span> - </div> - }> - <CompleteProfileContent /> - </Suspense> - ); -} +export default function CompleteProfile() { + return <CompleteProfileContent />; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/complete-profile/page.tsx(8 hunks)lib/services/profile.ts(1 hunks)
⏰ 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). (1)
- GitHub Check: Test Suite
🔇 Additional comments (2)
lib/services/profile.ts (1)
61-62: LGTM: use derived first/last name variables.Consistently sources names from normalized values.
app/complete-profile/page.tsx (1)
128-129: LGTM: stabilized effect deps.Switching to [router, returnUrl] avoids unnecessary re-runs when names change.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores