feat: add profile settings page with display name and avatar preference - #196
Merged
Conversation
- Add RequireAuth guard component for protected routes - Add ProfileSettingsPage with display name editing and avatar preference toggle (google vs generated DiceBear) - Add updateProfile() to profileService with trim/validation - Add avatar_preference column to profiles table with RLS-granted client UPDATE - Expose refreshProfile() in AuthContext for profile reload after save - Update resolveUserAvatar to respect avatar preference and prioritize saved display name - Add /settings/profile route with RequireAuth in main.jsx - Add 'Account settings' menu item in UserMenu with navigation - Add i18n translations for profile namespace in en/fr/ar - Add profileService.test.js (6 tests) and ProfileSettingsPage.test.jsx (4 tests) - Update AGENTS.md and AGENTS_REFERENCE.md with profile settings contracts
- Widen page from max-w-xl to max-w-4xl for comfortable spacing - Replace 'Profile settings' title with 'Settings' heading - Add tab bar with Profile (active), Notifications, Appearance, Privacy, Connected Accounts (disabled placeholders) - Add settings_tabs i18n namespace in en/fr/ar - Add test for tab bar rendering and disabled state
…and improve loading state UI
✅ Deploy Preview for dev-bayanflow ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
documentation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Preview for Bayan Flow Staging ready!
Preview alias |
📝 WalkthroughWalkthroughAdds an authenticated ChangesProfile Settings Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProfileSettingsPage
participant profileService
participant Supabase
participant AuthProvider
User->>ProfileSettingsPage: opens /settings/profile
ProfileSettingsPage->>profileService: getProfile(userId)
profileService->>Supabase: select profile row
Supabase-->>profileService: profile row
profileService-->>ProfileSettingsPage: display_name, avatar_preference
User->>ProfileSettingsPage: edits name / toggles avatar
ProfileSettingsPage->>profileService: updateProfile(userId, patch)
profileService->>Supabase: update profiles (scoped columns)
Supabase-->>profileService: updated row
profileService-->>ProfileSettingsPage: success
ProfileSettingsPage->>AuthProvider: refreshProfile()
AuthProvider-->>ProfileSettingsPage: updated context
ProfileSettingsPage-->>User: success toast
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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: 4
🧹 Nitpick comments (4)
src/utils/resolveUserAvatar.js (1)
61-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate seed-derivation logic.
The
(email ?? 'bayan-flow').trim() || 'bayan-flow'expression is now duplicated between the newgenerated-preference branch (Lines 69-70) and the existing fallback branch (Line 84).♻️ Extract shared seed helper
+function resolveAvatarSeed(email) { + return (email ?? 'bayan-flow').trim() || 'bayan-flow'; +} + export function resolveUserAvatar({ metadataUrl, profileUrl, email, size = 64, avatarPreference = 'google', }) { if (avatarPreference === 'generated') { - const seed = (email ?? 'bayan-flow').trim() || 'bayan-flow'; + const seed = resolveAvatarSeed(email); return { src: generateAvatarDataUri(seed, size), source: 'generated', }; } if (isHttpUrl(metadataUrl)) { return { src: metadataUrl.trim(), source: 'google' }; } if (isHttpUrl(profileUrl)) { return { src: profileUrl.trim(), source: 'profile' }; } - const seed = (email ?? 'bayan-flow').trim() || 'bayan-flow'; + const seed = resolveAvatarSeed(email); return { src: generateAvatarDataUri(seed, size), source: 'generated', }; }🤖 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 `@src/utils/resolveUserAvatar.js` around lines 61 - 89, The seed derivation for generated avatars is duplicated in resolveUserAvatar, making the function harder to maintain. Extract the repeated `(email ?? 'bayan-flow').trim() || 'bayan-flow'` logic into a small shared helper or local variable inside resolveUserAvatar, and use it in both the avatarPreference === 'generated' branch and the fallback generated-avatar branch to keep behavior identical.src/contexts/AuthProvider.jsx (1)
142-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct test coverage for
refreshProfileForUser.Codecov flags Line 143 as uncovered. This is the entry point
ProfileSettingsPagewill call after a successful save to refresh the displayed avatar/name, so it's worth a direct assertion thatcontext.refreshProfile()re-fetches the profile for the current user.🤖 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 `@src/contexts/AuthProvider.jsx` around lines 142 - 145, Add direct test coverage for the AuthProvider refreshProfileForUser callback, which is currently uncovered. Update the AuthProvider tests to assert that calling context.refreshProfile() invokes refreshProfile with the current user from user, so the ProfileSettingsPage post-save refresh path is exercised explicitly. Use the refreshProfileForUser and refreshProfile symbols to locate the behavior and verify the current-user re-fetch is wired correctly.Source: Linters/SAST tools
src/components/RequireAuth.jsx (1)
1-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for
RequireAuth.Codecov flags the entire file as uncovered, and no
RequireAuth.test.jsxappears in this cohort. Given it gates a protected route, a couple of focused tests (redirect-when-unauthenticated, render-children-when-authenticated) would materially reduce regression risk.🤖 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 `@src/components/RequireAuth.jsx` around lines 1 - 43, Add focused test coverage for RequireAuth because the whole component is currently uncovered. Create tests for the RequireAuth component that mock useAuth and useNavigate to verify it redirects to “/” when isConfigured is true, isLoading is false, and isAuthenticated is false, and that it renders children when isAuthenticated is true. Also cover the loading/unconfigured state by asserting the placeholder is shown before auth is resolved.src/i18n/locales/en/translation.json (1)
33-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused "bio" translation keys — no corresponding field in profile schema/service.
profileService.js(getProfile/updateProfile) and theProfileSettingsPage.jsxsnippets only handledisplay_nameandavatar_preference; there's nodescription/bio field anywhere in this PR. These keys (also duplicated infrandar) appear to be dead strings added ahead of the feature. Consider removing them until the bio field ships, to avoid confusing translators and future readers.🤖 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 `@src/i18n/locales/en/translation.json` around lines 33 - 34, Remove the unused bio-related translation entries from the locale files, since `profileService.js` (`getProfile`/`updateProfile`) and `ProfileSettingsPage.jsx` only use `display_name` and `avatar_preference`. Delete the `description` and `descriptionPlaceholder` keys from the `translation.json` entries in `en` and the matching duplicated keys in `fr` and `ar`, keeping only strings that correspond to actual profile schema/service fields.
🤖 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 `@AGENTS.md`:
- Line 9: The route inventory update belongs in docs/AGENTS_REFERENCE.md, not
AGENTS.md. Remove the added /settings/profile entry from the Product line in
AGENTS.md, and apply the route-list change in docs/AGENTS_REFERENCE.md instead,
keeping AGENTS.md limited to non-negotiable rules/contracts.
In `@src/components/RequireAuth.jsx`:
- Around line 18-33: RequireAuth is treating the unconfigured auth state as
loading, which leaves the app stuck on the skeleton when auth is disabled.
Update the RequireAuth component’s useEffect and early return so the
!isConfigured path is handled explicitly in addition to !isAuthenticated, using
navigate and/or a clear fallback instead of returning the spinner. Keep the fix
localized to RequireAuth and its isConfigured/isLoading guard logic.
In `@src/pages/ProfileSettingsPage.jsx`:
- Around line 79-93: The optimistic avatar toggle in autoSaveAvatar does not
revert when updateProfile fails, so the UI can մն leave AvatarPreference out of
sync with persisted state. Update autoSaveAvatar in ProfileSettingsPage to
capture the previous avatar preference and roll back the local state when the
save throws, or move the revert logic to the caller around
setAvatarPreference(next) so the switch returns to the prior value on failure.
Make sure the existing showToast error path remains intact and avoid duplicating
the rollback in both places.
In `@src/pages/ProfileSettingsPage.test.jsx`:
- Around line 142-156: The ProfileSettingsPage test is asserting only the final
submitted payload even though the profile picture switch triggers an earlier
autosave via updateProfileMock. Update the test around the ProfileSettingsPage
flow to account for both actions by asserting the switch-driven avatarPreference
write separately and then verifying the Save changes submission, or by checking
the updateProfileMock call count/order instead of a single toHaveBeenCalledWith
on the combined state.
---
Nitpick comments:
In `@src/components/RequireAuth.jsx`:
- Around line 1-43: Add focused test coverage for RequireAuth because the whole
component is currently uncovered. Create tests for the RequireAuth component
that mock useAuth and useNavigate to verify it redirects to “/” when
isConfigured is true, isLoading is false, and isAuthenticated is false, and that
it renders children when isAuthenticated is true. Also cover the
loading/unconfigured state by asserting the placeholder is shown before auth is
resolved.
In `@src/contexts/AuthProvider.jsx`:
- Around line 142-145: Add direct test coverage for the AuthProvider
refreshProfileForUser callback, which is currently uncovered. Update the
AuthProvider tests to assert that calling context.refreshProfile() invokes
refreshProfile with the current user from user, so the ProfileSettingsPage
post-save refresh path is exercised explicitly. Use the refreshProfileForUser
and refreshProfile symbols to locate the behavior and verify the current-user
re-fetch is wired correctly.
In `@src/i18n/locales/en/translation.json`:
- Around line 33-34: Remove the unused bio-related translation entries from the
locale files, since `profileService.js` (`getProfile`/`updateProfile`) and
`ProfileSettingsPage.jsx` only use `display_name` and `avatar_preference`.
Delete the `description` and `descriptionPlaceholder` keys from the
`translation.json` entries in `en` and the matching duplicated keys in `fr` and
`ar`, keeping only strings that correspond to actual profile schema/service
fields.
In `@src/utils/resolveUserAvatar.js`:
- Around line 61-89: The seed derivation for generated avatars is duplicated in
resolveUserAvatar, making the function harder to maintain. Extract the repeated
`(email ?? 'bayan-flow').trim() || 'bayan-flow'` logic into a small shared
helper or local variable inside resolveUserAvatar, and use it in both the
avatarPreference === 'generated' branch and the fallback generated-avatar branch
to keep behavior identical.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e823629-5d95-4610-960c-eb3a2ce24937
📒 Files selected for processing (22)
AGENTS.mddocs/AGENTS_REFERENCE.mdsrc/components/DocumentTitle.jsxsrc/components/RequireAuth.jsxsrc/components/UserMenu.jsxsrc/components/UserMenu.test.jsxsrc/contexts/AuthContextDefinition.jssrc/contexts/AuthProvider.jsxsrc/contexts/AuthProvider.test.jsxsrc/i18n/locales/ar/translation.jsonsrc/i18n/locales/en/translation.jsonsrc/i18n/locales/fr/translation.jsonsrc/index.csssrc/main.jsxsrc/pages/ProfileSettingsPage.jsxsrc/pages/ProfileSettingsPage.test.jsxsrc/services/profileService.jssrc/services/profileService.test.jssrc/test/supabaseMock.jssrc/utils/resolveUserAvatar.jssrc/utils/resolveUserAvatar.test.jssupabase/migrations/20260630120000_profiles_profile_settings.sql
- Refactor resolveUserAvatar to extract shared resolveAvatarSeed helper - Fix RequireAuth to handle !isConfigured path without infinite spinner - Add RequireAuth.test.jsx with 4 focused tests - Fix ProfileSettingsPage optimistic avatar toggle to revert on failure - Update ProfileSettingsPage.test.jsx to verify both avatar autosave and form submit calls - Add AuthProvider.test.jsx coverage for refreshProfileForUser callback - Remove unused bio/description translation keys from en/fr/ar locales - Update AGENTS.md with comprehensive project analysis snapshot
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/ProfileSettingsPage.jsx (3)
65-77: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winToast timeout isn't cleared on unmount.
toastTimeoutRefis set viasetTimeoutinshowToastbut there's no cleanup effect to clear it if the component unmounts before the 3s delay elapses (e.g., user navigates away right after a save). This callssetToaston an unmounted component.🔧 Proposed fix
const showToast = useCallback((message, type = 'success') => { if (toastTimeoutRef.current) { clearTimeout(toastTimeoutRef.current); } setToastType(type); setToast(message); toastTimeoutRef.current = setTimeout(() => setToast(null), 3000); }, []); + + useEffect(() => { + return () => { + if (toastTimeoutRef.current) { + clearTimeout(toastTimeoutRef.current); + } + }; + }, []);🤖 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 `@src/pages/ProfileSettingsPage.jsx` around lines 65 - 77, The toast timer in showToast can still fire after ProfileSettingsPage unmounts, causing setToast on an unmounted component. Add a cleanup effect in ProfileSettingsPage that clears toastTimeoutRef.current on unmount, and keep the existing timeout reset logic inside showToast so only one timer remains active at a time.
165-185: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
displayNameisn't trimmed before saving.The loaded value is trimmed (
row?.display_name?.trim()at line 115), but the value submitted here is used as-is, so leading/trailing whitespace can be persisted, creating inconsistency with the load path.🔧 Proposed fix
await updateProfile(user.id, { - displayName, + displayName: displayName.trim(), avatarPreference, });🤖 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 `@src/pages/ProfileSettingsPage.jsx` around lines 165 - 185, The submitted displayName in handleSubmit is saved without trimming, which can persist leading/trailing whitespace even though the load path normalizes it. Update the handleSubmit flow in ProfileSettingsPage so the displayName passed to updateProfile is trimmed before saving, keeping it consistent with the value loaded from the profile data.
423-441: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a polite live region to the toast
The toast message needs
role="status"andaria-live="polite"so save success/error feedback is announced to screen readers.🤖 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 `@src/pages/ProfileSettingsPage.jsx` around lines 423 - 441, The toast in ProfileSettingsPage is missing an accessible live region, so screen readers may not announce save feedback. Update the toast container in the AnimatePresence/motion.div block to include role="status" and aria-live="polite", keeping the existing success/error styling and toastType logic unchanged.
🤖 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.
Outside diff comments:
In `@src/pages/ProfileSettingsPage.jsx`:
- Around line 65-77: The toast timer in showToast can still fire after
ProfileSettingsPage unmounts, causing setToast on an unmounted component. Add a
cleanup effect in ProfileSettingsPage that clears toastTimeoutRef.current on
unmount, and keep the existing timeout reset logic inside showToast so only one
timer remains active at a time.
- Around line 165-185: The submitted displayName in handleSubmit is saved
without trimming, which can persist leading/trailing whitespace even though the
load path normalizes it. Update the handleSubmit flow in ProfileSettingsPage so
the displayName passed to updateProfile is trimmed before saving, keeping it
consistent with the value loaded from the profile data.
- Around line 423-441: The toast in ProfileSettingsPage is missing an accessible
live region, so screen readers may not announce save feedback. Update the toast
container in the AnimatePresence/motion.div block to include role="status" and
aria-live="polite", keeping the existing success/error styling and toastType
logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6df292a9-588b-45d0-987a-8693fe5e9a47
📒 Files selected for processing (10)
AGENTS.mdsrc/components/RequireAuth.jsxsrc/components/RequireAuth.test.jsxsrc/contexts/AuthProvider.test.jsxsrc/i18n/locales/ar/translation.jsonsrc/i18n/locales/en/translation.jsonsrc/i18n/locales/fr/translation.jsonsrc/pages/ProfileSettingsPage.jsxsrc/pages/ProfileSettingsPage.test.jsxsrc/utils/resolveUserAvatar.js
🚧 Files skipped from review as they are similar to previous changes (6)
- src/i18n/locales/en/translation.json
- src/i18n/locales/ar/translation.json
- src/utils/resolveUserAvatar.js
- AGENTS.md
- src/i18n/locales/fr/translation.json
- src/pages/ProfileSettingsPage.test.jsx
33 tasks
35 tasks
34 tasks
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
1 participant
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.
Contribution workflow
develop: This PR targetsdevelop, notmain.Description
Implements the private
/settings/profileroute introduced in the auth contracts for v0.5.0. Users can now update their display name and choose between their Google avatar or a generated DiceBear avatar. The page is protected behindRequireAuthand all writes are scoped by Supabase RLS — onlydisplay_nameandavatar_preferenceare client-writable.Type of Change
Related Issues
Fixes #
Changes Made
RequireAuthguard component — redirects unauthenticated users to/ProfileSettingsPagewith display name editing (trim + validation) and avatar preference toggle (googlevsgeneratedDiceBear)Profileactive;Notifications,Connectionsas disabled placeholders) withmax-w-4xlpage widthupdateProfile()toprofileServiceand exposerefreshProfile()fromAuthContextso the UI can reload after saveresolveUserAvatarto respectavatar_preferenceand prefer saveddisplay_name/settings/profileroute withRequireAuthinmain.jsxUserMenuUPDATE (display_name, avatar_preference)to theauthenticatedroleprofileandsettings_tabsnamespaces inen,fr, andarAGENTS.mdanddocs/AGENTS_REFERENCE.mdwith profile settings contractsreact-hooks/exhaustive-depswarning inProfileSettingsPageAlgorithm Details (if applicable)
N/A
Testing
pnpm test:run)Test Results
New test coverage added:
profileService.test.js— 6 tests coveringgetProfile,updateProfile, trim/validation, and error pathsProfileSettingsPage.test.jsx— 5 tests covering render, prefill from profile row, prefill from OAuth metadata, save success, and save error toastAuthProvider.test.jsx— additional coverage forrefreshProfile()resolveUserAvatar.test.js— extended to coveravatar_preferencelogicUserMenu.test.jsx— extended to cover "Account settings" navigation itemScreenshots/GIFs
/settings/profilewith display name editor and avatar selectorCode Quality
pnpm lint)pnpm format)Performance Impact
Accessibility
Breaking Changes
Checklist
Additional Notes
The
VisualizerApp.jsxpre-existingreact-hooks/exhaustive-depswarning (openFeature) is out of scope for this PR and was present before this branch.Summary by CodeRabbit
/settings/profileroute with automatic page title translation.