Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add toast notification to handle errors when updating a destination #232

Merged
merged 1 commit into from
Feb 5, 2021

Conversation

ftianli-amzn
Copy link

@ftianli-amzn ftianli-amzn commented Jan 12, 2021

Issue #, if available:

Description of changes:

Reason for the change:

When user have both permissions of cluster:admin/opendistro/alerting/destination/get and cluster:monitor/state, user can enter the Edit destination page.
There is no notifications to handle the backend errors, especially the permission error, when the user attempts to click the Update button in the Edit destination page.

image

Note:
When user only has cluster:admin/opendistro/alerting/destination/get permission, the Edit button is greyed out. It's not because the user don't have the permission to update destinations, but because the user don't have the permission to check the cluster setting, especially opendistro.alerting.destination.allow_list, thus no destinations is allowed for the user to edit.

Snipaste_2021-01-11_17-19-58 edit grey

After the change:
A toast notification shows up when there is an error from backend, especially the permission error, occurs when clicking the Update button.
image

Testing:
unit test result: (Actually there is no unit tests for the toast notifications😐)

Test Suites: 6 skipped, 83 passed, 83 of 89 total
Tests:       20 skipped, 385 passed, 405 total
Snapshots:   135 passed, 135 total
Time:        63.306 s
Ran all test suites.
✨  Done in 64.28s.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ftianli-amzn ftianli-amzn marked this pull request as ready for review January 12, 2021 01:42
@ftianli-amzn ftianli-amzn merged commit 9f318e9 into opendistro-for-elasticsearch:master Feb 5, 2021
@ftianli-amzn ftianli-amzn deleted the toast-update-dest branch February 5, 2021 18:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants