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

Add notification toasts and alerts for error/success messages in the management section #5726

Merged
merged 18 commits into from
May 22, 2023

Conversation

HelNershingThapa
Copy link
Contributor

@HelNershingThapa HelNershingThapa commented Apr 24, 2023

This pull request partially addresses issue #3424 of adding notification toasts to inform users of error/success messages in the management section of the app when interacting with APIs. The react-hot-toast dependency has been added to display toasts. The toasts are displayed at the bottom left of the app for success messages in the following sub-sections: Projects, Organizations, Teams, Campaigns, Categories, and Licenses. Toasts are shown when entities are created, updated, or deleted successfully. On failure, the callout alert component will be displayed. The toast component is centralized for consistency across the app.

Changes Made:

  • Added react-hot-toast as a dependency to display toasts.
  • Added notification toasts to the bottom left of the app for error/success messages in the management section of the app when interacting with APIs for the following sub-sections: Projects, Organizations, Teams, Campaigns, Categories, and Licenses. Toasts are displayed when entities are created, updated, or deleted.

    Some example toasts:
    Screenshot from 2023-04-24 16-23-46Screenshot from 2023-04-24 16-23-59
    Screenshot from 2023-04-24 16-24-08
    Screenshot from 2023-04-24 16-24-26
  • Updated tests to accommodate the changes wherever feasible.
  • Adds EntityError component that displays error messages in alerts for entity creation and updation failure.

In addition to the above changes, there were some SonarCloud duplication issues. To fix those, the following changes were made:

  • Adds a new hook, useModifyMembers, to handle the logic for updating members and managers, reducing duplicate code.
  • Refactors code to use a common function for handling network error responses.
  • Refactors update functions for managers and members, using a common updateAffiliation() function and updating function calls and toast messages to be dynamic.
  • Refactors entity updation functions for organizations, teams, campaigns, categories, and licenses to eliminate code duplication, using a reusable function called updateEntity().
  • Fixes broken link when navigating to the categories list page after a category deletion.

- Added toast messages to provide user feedback on success and failure actions while creating and updating information about a license.
- Intercepted network requests to mock error responses.
- Mock subset implementations of the toast library and updated test cases to include assertions to calls for toast messages.
- Added toast messages to provide user feedback on success and failure actions while creating and updating information about a category.
- Intercepted network requests to mock error responses (only for creation).
- Mock subset implementations of the toast library and updated test cases to include assertions to calls for toast messages(only for creation).
- Render test component with createComponentWithMemoryRouter to pass parameters during tests run time.
- Added toast messages to provide user feedback on success and failure actions while creating and updating information about a campaign.
- Intercepted network requests to mock error responses.
- Mock subset implementations of the toast library and updated test cases to include assertions to calls for toast messages.
- Added toast messages to provide user feedback on success and failure actions while creating and updating information about a team.
- Intercepted network requests to mock error responses.
- Mock subset implementations of the toast library and updated test cases to include assertions to calls for toast messages.
- Added toast messages to provide user feedback on success and failure actions while creating, deleting and updating information about a campaign.
- Intercepted a network request to mock error responses for organization creation failure.
- Mock subset implementations of the toast library and updated test cases to include assertions to calls for toast messages.
@github-actions github-actions bot added scope: frontend dependencies Pull requests that update a dependency file javascript labels Apr 24, 2023
…esponses

Address SonarCloud duplication issue by extracting common error handling logic to generate a network failure error in MSW while intercepting network requests for faulty handlers
- Refactored the updateManagers() and updateMembers() functions to use a common updateAffiliation() function.
- Updated function calls to getMembersDiff() to pass the correct parameters based on affiliationType.
- Replaced hardcoded affiliationType values with dynamic affiliationType parameter in success and failure toast messages.
- Refactored the updation function for organizations, teams, campaigns, categories and licenses to eliminate code duplication flagged by SonarCloud.
- Extracted common functionality into a reusable function called updateEntity(). Replaced the duplicated code with a single function call to updateEntity(), passing the required parameters.
- Updated success and failure toast messages to use a dynamic entity parameter instead of hardcoding entity type.
Copy link
Contributor

@Aadesh-Baral Aadesh-Baral left a comment

Choose a reason for hiding this comment

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

  • Out of scope for this PR but lets also add a toast message while updating a user role and level in user management section, updating user info on user settings page and notification page.

  • While there is an error during creating organisation another alert message is also shown alongside toast message. I feel showing 2 error messages are redundant here as both are displaying same information. Is this expected behavior?
    image

  • Same case if error occurs while creating campaign.
    image

  • Getting this error instead of toast with error message if project creation fails.
    image

  • Add toast message while sending messages to all contributors of a project instead of alert as toast message is shown while sending messages to all members of a team.
    image

- Adds toast messages for user role and mapper level update. Adds test cases for the same.
- Intercepted request handlers to mock error responses for these updates.
- Button role has been added to the attribute container to improve screen reading.
@HelNershingThapa
Copy link
Contributor Author

HelNershingThapa commented May 1, 2023

Out of scope for this PR but lets also add a toast message while updating a user role and level in user management section, updating user info on user settings page and notification page.

Added the toast messages for the user role and mapper level update.

While there is an error during creating organisation another alert message is also shown alongside toast message. I feel showing 2 error messages are redundant here as both are displaying same information. Is this expected behavior?

Yes, it is the expected behavior. While implementing the toast messages, I hadn't given much thought to it. I went with the idea that the toast message is a generic message that provides feedback to the user about the success or failure of their action. In contrast, the alert message provides specific details about the error. However, I understand the concern, and we can review and decide on this implementation to determine if it can be improved.

Getting this error instead of toast with error message if project creation fails.

haha nice catch. I discovered that the error is not being handled gracefully, resulting in the crash. While handling such errors is important, it's not within the scope of this pull request, which focuses on implementing toasts for CRUD operations for the entities in the management section. Nonetheless, I agree that we should handle these errors gracefully wherever possible, and I have created a separate issue #5743 to address this problem.

Add toast message while sending messages to all contributors of a project instead of alert as toast message is shown while sending messages to all members of a team.

Yes, we could add toast messages when messaging all contributors of the project. But, like you said, wouldn't that be redundant haha :P ? Up for discussion. cc @ramyaragupathy

Copy link
Contributor

@d-rita d-rita left a comment

Choose a reason for hiding this comment

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

Tested the creating, updating, and deleting of the different resources under the manage tab, i.e. projects, licences, categories, campaigns, teams, organisations. The toasts appear for the different actions undertaken which works as expected.
However, as noted by previous reviewers, there is some redundancy with having a toast appear as well as an alert while showing the same thing, for example via project creation.

I also noticed that after deleting a category, the user is redirected to a non-existent page instead of the categories list, i.e. is redirected to /manage/interests instead of /manage/categories/

@ramyaragupathy
Copy link
Member

Per discussion today w/ @Aadesh-Baral @HelNershingThapa :

  • we will retain alert messages for team messaging
  • use alert messages for failure scenarios under manage sections
  • use toast messages for success scenarios under manager sections

- Adds a new EntityError component to display it when creation and updation of entities fail. The same component is display when an entity updation or deletion fails
- Removed toast success and error messages for deletion actions
@HelNershingThapa HelNershingThapa changed the title Add notification toast for error/success messages in management section Add notification toasts and alerts for error/success messages in the management section May 9, 2023
@sonarcloud
Copy link

sonarcloud bot commented May 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@Aadesh-Baral Aadesh-Baral left a comment

Choose a reason for hiding this comment

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

LGTM

@HelNershingThapa HelNershingThapa merged commit 45d5ced into develop May 22, 2023
5 checks passed
@HelNershingThapa HelNershingThapa deleted the feature/3424-toast-management-section branch May 22, 2023 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file scope: frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants