Fix Bugs in User and Facility Avatar Upload and Deletion Workflow #10547
Conversation
WalkthroughThis pull request updates the avatar upload and deletion processes across three components: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
rajku-dev
changed the title
Uploading.. and Preview image after User Avatar Delete
![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
🧹 Nitpick comments (2)
src/components/Users/UserAvatar.tsx (1)
89-94: Consider enhancing error handling.The catch block could be more informative by including the error details in the callback.
Consider this improvement:
try { await mutateAvatarDelete(); onSuccess(); - } catch { - onError(); + } catch (error) { + onError(error); }src/components/Common/AvatarEditModal.tsx (1)
57-64: Consider using useReducer for complex state management.The component manages multiple related state variables (
isProcessing,selectedFile,preview,isCameraOpen,previewImage,isCaptureImgBeingUploaded). Consider usinguseReducerto centralize state management and make state transitions more predictable.Here's a suggested implementation:
type State = { isProcessing: boolean; selectedFile?: File; preview?: string; isCameraOpen: boolean; previewImage: string | null; isCaptureImgBeingUploaded: boolean; }; type Action = | { type: 'START_PROCESSING' } | { type: 'STOP_PROCESSING' } | { type: 'SET_FILE'; file?: File } | { type: 'SET_PREVIEW'; url?: string } | { type: 'TOGGLE_CAMERA'; isOpen: boolean } | { type: 'SET_PREVIEW_IMAGE'; image: string | null } | { type: 'START_UPLOAD' } | { type: 'STOP_UPLOAD' } | { type: 'RESET' }; const initialState: State = { isProcessing: false, selectedFile: undefined, preview: undefined, isCameraOpen: false, previewImage: null, isCaptureImgBeingUploaded: false, }; function reducer(state: State, action: Action): State { switch (action.type) { case 'START_PROCESSING': return { ...state, isProcessing: true }; case 'STOP_PROCESSING': return { ...state, isProcessing: false }; case 'SET_FILE': return { ...state, selectedFile: action.file }; case 'SET_PREVIEW': return { ...state, preview: action.url }; case 'TOGGLE_CAMERA': return { ...state, isCameraOpen: action.isOpen }; case 'SET_PREVIEW_IMAGE': return { ...state, previewImage: action.image }; case 'START_UPLOAD': return { ...state, isCaptureImgBeingUploaded: true }; case 'STOP_UPLOAD': return { ...state, isCaptureImgBeingUploaded: false }; case 'RESET': return initialState; default: return state; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Common/AvatarEditModal.tsx(2 hunks)src/components/Users/UserAvatar.tsx(2 hunks)
🔇 Additional comments (4)
src/components/Users/UserAvatar.tsx (2)
30-30: LGTM! Good switch tomutateAsync.The change from
mutatetomutateAsyncenables proper handling of the mutation lifecycle, which is essential for managing UI feedback states.
85-92: LGTM! Improved async flow control.The updated signature with
onSuccesscallback and proper async/await usage enables better UI state management after avatar deletion.src/components/Common/AvatarEditModal.tsx (2)
25-25: LGTM! Good improvement to the handleDelete signature.The addition of the
onSuccesscallback enables proper cleanup of UI state after successful avatar deletion, which helps fix the "Uploading..." message issue.
140-150: LGTM! Proper state cleanup after avatar deletion.The implementation correctly handles both success and error cases, with appropriate state cleanup in the success callback. This fixes the issue with preview images persisting after avatar deletion.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Common/AvatarEditModal.tsx (1)
140-150: Consider enhancing error feedback.While the implementation correctly handles the avatar deletion flow, consider providing user feedback in the error case.
const deleteAvatar = async () => { setIsProcessing(true); await handleDelete( () => { setIsProcessing(false); setPreview(undefined); setPreviewImage(null); }, - () => setIsProcessing(false), + () => { + setIsProcessing(false); + toast.error(t("failed_to_delete_avatar")); + }, ); };src/components/Users/UserAvatar.tsx (2)
85-95: Well-structured error handling with proper async/await!The updated implementation properly handles both success and error cases, ensuring UI state is updated only after the operation completes.
Consider adding TypeScript type annotations for the callback parameters:
const handleAvatarDelete = async ( - onSuccess: () => void, - onError: () => void, + onSuccess: VoidFunction, + onError: VoidFunction, ) => {
85-95: Consider adding loading state during avatar deletion.While the async handling is improved, the UI doesn't indicate when deletion is in progress. Consider adding a loading state to prevent multiple deletion attempts and provide better user feedback.
Here's a suggested implementation:
+ const [isDeleting, setIsDeleting] = useState(false); const handleAvatarDelete = async ( onSuccess: () => void, onError: () => void, ) => { try { + setIsDeleting(true); await mutateAvatarDelete(); onSuccess(); } catch { onError(); + } finally { + setIsDeleting(false); } }; return ( <> <AvatarEditModal title={t("edit_avatar")} open={editAvatar} imageUrl={userData?.profile_picture_url} handleUpload={handleAvatarUpload} handleDelete={handleAvatarDelete} + isDeleting={isDeleting} onClose={() => setEditAvatar(false)} />Also applies to: 99-106
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Common/AvatarEditModal.tsx(2 hunks)src/components/Users/UserAvatar.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/components/Common/AvatarEditModal.tsx (2)
25-25: LGTM! Good improvement to the Props interface.The addition of the
onSuccesscallback parameter tohandleDeleteenables better control over state management after successful avatar deletion.
57-64: LGTM! Well-structured state management.The component maintains clear separation of concerns with distinct states for processing, previews, and file handling. The state updates during avatar deletion effectively address the "Uploading..." message issue mentioned in the PR objectives.
src/components/Users/UserAvatar.tsx (1)
30-30: Great improvement usingmutateAsync!The switch from
mutatetomutateAsyncensures proper async handling of the avatar deletion operation, allowing the component to wait for the operation to complete before updating the UI state.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
yep! forgot to check that, also fixed related issues
|
rajku-dev
changed the title
Uploading.. and Preview image after User Avatar DeleteUploading.. | Preview image | Save button active on User/Facility Cover Photo Delete/Update
rajku-dev
changed the title
Uploading.. | Preview image | Save button active on User/Facility Cover Photo Delete/UpdateUploading.. | Preview image | Save button active on User/Facility Cover Photo Delete and Update
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Common/AvatarEditModal.tsx(3 hunks)src/components/Facility/FacilityHome.tsx(2 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
src/components/Common/AvatarEditModal.tsx
[warning] 202-202: DOM text reinterpreted as HTML
DOM text is reinterpreted as HTML without escaping meta-characters.
🔇 Additional comments (5)
src/components/Common/AvatarEditModal.tsx (3)
25-25: LGTM! Props interface update enhances control flow.The addition of the
onSuccesscallback parameter tohandleDeleteimproves the control flow by allowing proper state cleanup after successful deletion.
137-137: LGTM! Proper cleanup in finally block.Good practice to clear the
selectedFilestate in the finally block, ensuring it's reset regardless of the upload outcome.
141-151: LGTM! Improved error handling and state cleanup.The
deleteAvatarfunction now properly handles both success and error cases, with appropriate state cleanup on success.src/components/Facility/FacilityHome.tsx (2)
139-139: LGTM! Proper async handling.Good addition of
awaitto ensure the upload completes before proceeding with subsequent operations.
162-172: LGTM! Enhanced error handling with success callback.The updated
handleCoverImageDeletefunction properly handles both success and error cases, with appropriate callback invocations.
|
When changing an existing image, the preview doesn't reflect new image - go ahead and address this as well since this is a small PR and it's directly related. |
Jacobjeevan
added
work-in-progress
and removed
question
…om/rajku-dev/care_fe into issue/10525/user-avatat-delete-bug
rajku-dev
changed the title
Uploading.. | Preview image | Save button active on User/Facility Cover Photo Delete and Update
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/Common/AvatarEditModal.tsx (1)
75-81: 🛠️ Refactor suggestionFix the missing dependency in useCallback hook.
The
useCallbackhook is missing theconstraint.facingModedependency.Apply this diff to fix the dependency array:
const handleSwitchCamera = useCallback(() => { setConstraint( constraint.facingMode === "user" ? VideoConstraints.environment : VideoConstraints.user, ); - }, []); + }, [constraint.facingMode]);🧰 Tools
🪛 GitHub Actions: Cypress Tests
[warning] 81-6: React Hook useCallback has a missing dependency: 'constraint.facingMode'. Either include it or remove the dependency array. You can also replace multiple useState variables with useReducer if 'setConstraint' needs the current value of 'constraint.facingMode'
[error] 134-11: Expected an assignment or function call and instead saw an expression
🧹 Nitpick comments (2)
src/components/Common/AvatarEditModal.tsx (2)
196-196: Remove debug console.log statement.Debug console.log statement should be removed before merging.
Apply this diff to remove the debug statement:
- console.log(preview, imageUrl);
65-65: Fix theanytype on webRef.The
anytype should be replaced with a more specific type.Consider using the proper type from the
react-webcamlibrary:- const webRef = useRef<any>(null); + const webRef = useRef<Webcam>(null);Don't forget to import the
Webcamtype:import Webcam, { type WebcamProps } from 'react-webcam';🧰 Tools
🪛 GitHub Actions: Cypress Tests
[warning] 65-25: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
[warning] 81-6: React Hook useCallback has a missing dependency: 'constraint.facingMode'. Either include it or remove the dependency array. You can also replace multiple useState variables with useReducer if 'setConstraint' needs the current value of 'constraint.facingMode'
[error] 134-11: Expected an assignment or function call and instead saw an expression
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Common/AvatarEditModal.tsx(3 hunks)src/components/Facility/FacilityHome.tsx(2 hunks)src/components/Users/UserAvatar.tsx(4 hunks)
🧰 Additional context used
🪛 ESLint
src/components/Common/AvatarEditModal.tsx
[error] 134-134: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
🪛 GitHub Check: cypress-run (1)
src/components/Common/AvatarEditModal.tsx
[failure] 134-134:
Expected an assignment or function call and instead saw an expression
🪛 GitHub Check: lint
src/components/Common/AvatarEditModal.tsx
[failure] 134-134:
Expected an assignment or function call and instead saw an expression
🪛 GitHub Actions: Lint Code Base
src/components/Common/AvatarEditModal.tsx
[error] 134-134: Expected an assignment or function call and instead saw an expression @typescript-eslint/no-unused-expressions
🪛 GitHub Actions: Cypress Tests
src/components/Common/AvatarEditModal.tsx
[warning] 65-25: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
[warning] 81-6: React Hook useCallback has a missing dependency: 'constraint.facingMode'. Either include it or remove the dependency array. You can also replace multiple useState variables with useReducer if 'setConstraint' needs the current value of 'constraint.facingMode'
[error] 134-11: Expected an assignment or function call and instead saw an expression
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
🔇 Additional comments (8)
src/components/Users/UserAvatar.tsx (3)
30-30: LGTM! Improved async handling.Changed to use
mutateAsyncfor better control over the asynchronous operation.
55-59: LGTM! Enhanced callback handling.Updated function signatures to include success callbacks, improving control over the upload and deletion processes.
Also applies to: 89-92
94-95: LGTM! Proper async/await usage.Added await for the async operation and properly invoked the success callback.
src/components/Common/AvatarEditModal.tsx (2)
24-29: LGTM! Enhanced Props interface.Updated Props interface to include success callbacks for better control over upload and deletion processes.
🧰 Tools
🪛 GitHub Actions: Cypress Tests
[warning] 65-25: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
[warning] 81-6: React Hook useCallback has a missing dependency: 'constraint.facingMode'. Either include it or remove the dependency array. You can also replace multiple useState variables with useReducer if 'setConstraint' needs the current value of 'constraint.facingMode'
[error] 134-11: Expected an assignment or function call and instead saw an expression
153-160: LGTM! Proper callback handling.Updated deleteAvatar implementation to properly handle success and error callbacks.
src/components/Facility/FacilityHome.tsx (3)
134-138: LGTM! Enhanced callback handling.Updated function signatures to include success callbacks, improving control over the upload and deletion processes.
Also applies to: 167-170
143-143: LGTM! Proper async/await and callback handling.Updated implementation to properly handle async operations and success callbacks.
Also applies to: 150-151, 156-156
172-173: LGTM! Proper callback invocation.Updated implementation to properly invoke the success callback after deletion.
I have fixed this issue in recent changes. Please test it and let me know if any further adjustments are needed. |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
Proposed Changes
Uploadingafter deleting the avatarUploadingnot showing while updating the Facility cover photoSavebutton still active after successful User/Facility Avatar Update@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor