Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Bugs in User and Facility Avatar Upload and Deletion Workflow #10547

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

rajku-dev
Copy link
Contributor

@rajku-dev rajku-dev commented Feb 11, 2025

Proposed Changes

  • Fixes User Avatar Delete UI Bug #10525
  • Fixes - button text stuck at Uploading after deleting the avatar
  • Fixes - Uploading not showing while updating the Facility cover photo
  • Fixes - Preview image not showing while changing a User/Facility Avatar
  • Fixes - Showing deleted image preview after deleting the avatar
  • Fixes - Save button still active after successful User/Facility Avatar Update

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Enhanced avatar upload and deletion processes with improved success and error handling through callback mechanisms.
    • Updated cover image management functions to include success notifications upon upload and deletion.
  • Bug Fixes

    • Improved the avatar removal process so that successful deletions now promptly update the interface by clearing previews and resetting states, while errors are handled more reliably.
  • Refactor

    • Streamlined the deletion operation using asynchronous handling to ensure consistent and dependable execution during avatar removal.
    • Enhanced the cover image management functions to ensure that uploads and deletions complete before proceeding with subsequent actions.

@rajku-dev rajku-dev requested a review from a team as a code owner February 11, 2025 00:01
Copy link
Contributor

coderabbitai bot commented Feb 11, 2025

Caution

Review failed

The head commit changed during the review from c04c9f8 to 2d89b35.

Walkthrough

This pull request updates the avatar upload and deletion processes across three components: AvatarEditModal, UserAvatar, and FacilityHome. The functions now accept separate success and error callbacks, enhancing control over these operations. The UserAvatar component has transitioned to using mutateAsync for asynchronous mutation handling, ensuring operations are awaited. Additionally, the upload functions in FacilityHome have been modified to await their respective operations, improving the overall control flow.

Changes

File(s) Change Summary
src/components/Common/AvatarEditModal.tsx Updated handleUpload and handleDelete signatures to include separate onSuccess and onError callbacks. Adjusted uploadAvatar and deleteAvatar implementations to utilize these callbacks effectively.
src/components/Users/UserAvatar.tsx Modified handleAvatarUpload and handleAvatarDelete signatures to accept onSuccess callbacks. Changed mutation handling from mutate to mutateAsync for proper asynchronous operations.
src/components/Facility/FacilityHome.tsx Updated handleCoverImageUpload and handleCoverImageDelete signatures to include onSuccess callbacks, ensuring success handling is explicitly managed.

Possibly related PRs

Suggested labels

needs review, tested

Suggested reviewers

  • Jacobjeevan
  • rithviknishad

Poem

Hi, I'm a rabbit on a coding spree,
Hopping through changes so light and free.
With callbacks split for success and fail,
Our avatar deletion now sets its sail.
Bugs hop away as our code sings with glee!
🐰💻
Happy trails and smooth commits to thee!


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Feb 11, 2025

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 2d89b35
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/67ade6e4e9c60000087f3d29
😎 Deploy Preview https://deploy-preview-10547.preview.ohc.network
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rajku-dev rajku-dev changed the title Fix User Avatar Delete Workflow Fix Displaying Uploading.. and Preview image after User Avatar Delete Feb 11, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 using useReducer to 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

📥 Commits

Reviewing files that changed from the base of the PR and between cbcb64f and 47fc60d.

📒 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 to mutateAsync.

The change from mutate to mutateAsync enables 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 onSuccess callback 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 onSuccess callback 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between cbcb64f and 4f01089.

📒 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 onSuccess callback parameter to handleDelete enables 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 using mutateAsync!

The switch from mutate to mutateAsync ensures proper async handling of the avatar deletion operation, allowing the component to wait for the operation to complete before updating the UI state.

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FacilityHome also uses the AvatarEditModal; the handleDelete on facility home would now be broken since it wasn't adjusted right?

image

@rajku-dev
Copy link
Contributor Author

rajku-dev commented Feb 11, 2025

FacilityHome also uses the AvatarEditModal; the handleDelete on facility home would now be broken since it wasn't adjusted right?

yep! forgot to check that,

also fixed related issues

  • Save button still active after successful facility/user cover photo update
  • Uploading... not showing when updating the facility cover photo

@github-actions github-actions bot added needs-triage question Further information is requested labels Feb 11, 2025
@rajku-dev rajku-dev changed the title Fix Displaying Uploading.. and Preview image after User Avatar Delete Fix Displaying Uploading.. | Preview image | Save button active on User/Facility Cover Photo Delete/Update Feb 11, 2025
@rajku-dev rajku-dev changed the title Fix Displaying Uploading.. | Preview image | Save button active on User/Facility Cover Photo Delete/Update Fix Displaying Uploading.. | Preview image | Save button active on User/Facility Cover Photo Delete and Update Feb 11, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f01089 and d94134d.

📒 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 onSuccess callback parameter to handleDelete improves the control flow by allowing proper state cleanup after successful deletion.


137-137: LGTM! Proper cleanup in finally block.

Good practice to clear the selectedFile state in the finally block, ensuring it's reset regardless of the upload outcome.


141-151: LGTM! Improved error handling and state cleanup.

The deleteAvatar function 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 await to ensure the upload completes before proceeding with subsequent operations.


162-172: LGTM! Enhanced error handling with success callback.

The updated handleCoverImageDelete function properly handles both success and error cases, with appropriate callback invocations.

src/components/Common/AvatarEditModal.tsx Outdated Show resolved Hide resolved
@Jacobjeevan
Copy link
Contributor

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 Jacobjeevan added work-in-progress and removed question Further information is requested changes required needs-triage labels Feb 13, 2025
@Jacobjeevan Jacobjeevan removed the request for review from rithviknishad February 13, 2025 06:29
@rajku-dev rajku-dev changed the title Fix Displaying Uploading.. | Preview image | Save button active on User/Facility Cover Photo Delete and Update Fix Bugs in User and Facility Avatar Upload and Deletion Workflow Feb 13, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (1)
src/components/Common/AvatarEditModal.tsx (1)

75-81: 🛠️ Refactor suggestion

Fix the missing dependency in useCallback hook.

The useCallback hook is missing the constraint.facingMode dependency.

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 the any type on webRef.

The any type should be replaced with a more specific type.

Consider using the proper type from the react-webcam library:

-  const webRef = useRef<any>(null);
+  const webRef = useRef<Webcam>(null);

Don't forget to import the Webcam type:

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

📥 Commits

Reviewing files that changed from the base of the PR and between d94134d and c0ecfc5.

📒 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 mutateAsync for 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.

src/components/Common/AvatarEditModal.tsx Outdated Show resolved Hide resolved
@rajku-dev
Copy link
Contributor Author

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.

I have fixed this issue in recent changes. Please test it and let me know if any further adjustments are needed.

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Feb 13, 2025
Copy link

Conflicts have been detected against the base branch. Please merge the base branch into your branch.
cc: @rajku-dev

See: https://docs.ohc.network/docs/contributing#how-to-resolve-merge-conflicts

@github-actions github-actions bot removed the merge conflict pull requests with merge conflict label Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User Avatar Delete UI Bug
3 participants