fix: profile details not updating after changing workspace #6764
Conversation
WalkthroughProfileView's form reset behavior was changed: the component now calls Changes
Sequence Diagram(s)sequenceDiagram
participant View as ProfileView
participant Store as Redux selector
participant Form as react-hook-form
rect rgb(248,249,255)
View->>Store: select `user` (and related settings)
end
Note over View,Form: On mount and when `user` or `reset` changes
rect rgb(235,250,235)
View->>Form: useEffect -> reset({ name, username, email, currentPassword, bio, nickname, saving })
Form-->>View: form state rehydrated with current user values
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
Gemfile.lockis excluded by!**/*.lockios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
app/views/ProfileView/index.tsx(3 hunks)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/views/ProfileView/index.tsx (1)
283-291: Incomplete form reset when workspace changes.The change from
useFocusEffecttouseEffect([user, reset])correctly fixes the workspace-change bug by only resetting when the user object changes, not on every screen focus. However, the reset omitscurrentPasswordandsavingfields that are defined indefaultValues(lines 100, 103) and included in the post-submit reset (lines 219, 222).Impact:
currentPasswordcould retain a value from the previous workspace, creating a privacy risk or causing failed submissions with wrong credentials.savingcould show a stale loading state after workspace switch.Apply this diff to include all form fields in the reset:
useEffect(() => { reset({ name: user?.name || '', username: user?.username || '', email: user?.emails?.[0]?.address || '', bio: user?.bio || '', - nickname: user?.nickname || '' + nickname: user?.nickname || '', + currentPassword: null, + saving: false }); }, [user, reset]);
🧹 Nitpick comments (1)
app/views/ProfileView/index.tsx (1)
96-104: Align defaultValues with reset patterns for consistency.The
defaultValuesuse mixed patterns (as string, ternary withnull, optional chaining allowingundefined), while reset calls consistently use|| ''to default to empty strings. This inconsistency could cause subtle form behavior differences between initial load and post-reset states.Apply this diff to align defaultValues with the reset pattern:
defaultValues: { - name: user?.name as string, + name: user?.name || '', username: user?.username, - email: user?.emails ? user?.emails[0].address : null, + email: user?.emails?.[0]?.address || '', currentPassword: null, - bio: user?.bio, - nickname: user?.nickname, + bio: user?.bio || '', + nickname: user?.nickname || '', saving: false },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
Gemfile.lockis excluded by!**/*.lockios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
app/views/ProfileView/index.tsx(3 hunks)
🔇 Additional comments (2)
app/views/ProfileView/index.tsx (2)
3-3: LGTM: Import changes align with the fix.The removal of
useFocusEffectanduseCallbackimports is correct, as the fix properly replaces focus-based reset with effect-based reset.
60-82: LGTM: Well-structured selector consolidation.The consolidated
useAppSelectorproperly extracts all necessary state and settings in a single call with appropriate type assertions.
2 participants
Proposed changes
reset all fields of Form after changing workspace
Issue(s)
closes #6762
How to test or reproduce
Screenshots
Workspaces Comparison
Video:
Before / After
Types of changes
ly)
Checklist
Further comments
Summary by CodeRabbit