fix(profile): prevent avatar twice error message toast and avatar removal when invalid avatar URL is provided#38964
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughTwo avatar-related components were modified to add URL validation and conditional avatar updates. A new helper function validates HTTP/HTTPS URLs in the UserAvatarEditor, displaying error toasts for invalid inputs. The AccountProfileForm now only calls updateAvatar when avatar is truthy and not 'reset', with an added originalAvatarEtag constant for reference. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/views/account/profile/AccountProfileForm.tsx">
<violation number="1" location="apps/meteor/client/views/account/profile/AccountProfileForm.tsx:128">
P2: Avatar reset via the 'reset' sentinel is no longer processed because updateAvatar() is now skipped when avatar === 'reset', even though the hook handles reset by calling /v1/users.resetAvatar.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx (1)
72-74: Inconsistent indentation in the valid-URL branch.Lines 72–74 mix a tab with extra spaces, whereas the rest of the function body uses tabs exclusively.
🔧 Suggested fix
- setNewAvatarSource(avatarFromUrl); - setAvatarObj({ avatarUrl: avatarFromUrl }); - }; + setNewAvatarSource(avatarFromUrl); + setAvatarObj({ avatarUrl: avatarFromUrl }); +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx` around lines 72 - 74, The valid-URL branch in the function that calls setNewAvatarSource and setAvatarObj has mixed indentation (a tab plus spaces) causing inconsistent formatting; update the indentation in that branch to use tabs consistent with the rest of the function (the lines containing setNewAvatarSource(avatarFromUrl); and setAvatarObj({ avatarUrl: avatarFromUrl });) so the file uses tabs only, matching surrounding code style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx`:
- Around line 61-70: The error branch that runs when
isValidHttpUrl(avatarFromUrl) is false should only show the toast and clear the
URL input; remove the lines that call setAvatarObj('reset') and
setNewAvatarSource(undefined) so we do not mutate existing avatar selection, and
keep setAvatarFromUrl('') to clear the text field; also delete the inline
implementation comment near that block to follow the coding guideline.
In `@apps/meteor/client/views/account/profile/AccountProfileForm.tsx`:
- Line 70: Remove the dead constant originalAvatarEtag from
AccountProfileForm.tsx: locate the declaration "const originalAvatarEtag =
user?.avatarETag;" and delete it (or the entire unused assignment) since it's
never referenced anywhere in the component; if the value was intended for future
use, add a TODO comment instead of keeping an unused variable.
- Around line 128-130: The guard prevents legitimate avatar resets because
handleAddUrl's error path sets avatarObj('reset'), so change the error path in
UserAvatarEditor.tsx (the error branch in handleAddUrl) to not assign the
sentinel 'reset' — use null/undefined or a dedicated error flag instead; then
restore the check in AccountProfileForm.tsx around updateAvatar() (replace the
current avatar && avatar !== 'reset' check) to only skip on falsy values (e.g.,
if (avatar) await updateAvatar()), relying on useUpdateAvatar to handle the
'reset' value server-side.
---
Nitpick comments:
In `@apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx`:
- Around line 72-74: The valid-URL branch in the function that calls
setNewAvatarSource and setAvatarObj has mixed indentation (a tab plus spaces)
causing inconsistent formatting; update the indentation in that branch to use
tabs consistent with the rest of the function (the lines containing
setNewAvatarSource(avatarFromUrl); and setAvatarObj({ avatarUrl: avatarFromUrl
});) so the file uses tabs only, matching surrounding code style.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsxapps/meteor/client/views/account/profile/AccountProfileForm.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsxapps/meteor/client/views/account/profile/AccountProfileForm.tsx
🔇 Additional comments (1)
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx (1)
15-22:isValidHttpUrlhelper looks correct.Using the
URLconstructor's throw-on-invalid behavior with an explicit protocol check is the idiomatic and reliable pattern for this validation.
1 participant
Proposed changes (including videos or screenshots)
Problem
In My Account → Profile → Avatar (Use URL for avatar):
If a user enters an invalid URL (example:
hello) and clicks the link button:error-invalid-image-urlappears (correct)This results in inconsistent UI behavior:
Fix
Added URL validation before setting avatar state:
Save Changesfrom triggering avatar updateResult
Now behavior is correct:
Invalid URL:
Valid URL:
No backend or API changes were made. Only UI state handling was corrected.
Issue(s)
UI bug discovered during profile avatar testing.
Screen recording Before :
Screen.Recording.2026-02-24.133411.mp4
Screen recording After:
Screen.Recording.2026-02-24.133552.mp4
Steps to test or reproduce
helloBefore fix
After fix
Further comments
This fix improves UX consistency by aligning toast messages with the actual avatar update state.
Summary by CodeRabbit
Bug Fixes