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

Feature: AD: Add delete batch action #204

Merged
merged 5 commits into from
Jun 9, 2020
Merged

Feature: AD: Add delete batch action #204

merged 5 commits into from
Jun 9, 2020

Conversation

ohltyler
Copy link
Contributor

@ohltyler ohltyler commented Jun 8, 2020 •
edited
Loading

Issue #, if available:

Description of changes: #194

This PR adds a 'delete' batch action for the detector list page.

Specific changes:

  • Adds a 'Delete' action in the actions dropdown
  • Adds a delete confirmation modal (will show an additional callout if any of the selected detectors are currently running)
  • Improves loading state by showing a loading state of the confirmation modal until the action has completed. Once finished, the modal closes and an updated list of detectors is shown.
  • Refactors ConfirmActionModals to their own files
  • Refactors List to a containers directory
  • Renames containers and components folders in DetectorsList to be lowercase

Notes:

  • All UT pass

Screenshot (no detectors running):
Screen Shot 2020-06-09 at 12 56 18 PM

Screenshot (some detectors running):
Screen Shot 2020-06-09 at 12 55 49 PM

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ohltyler ohltyler linked an issue Jun 8, 2020 that may be closed by this pull request
Add delete batch action on detector list page #194
Closed
@sean-zheng-amazon sean-zheng-amazon changed the title Add delete batch action Feature: AD: Add delete batch action Jun 8, 2020
yizheliu-amazon
yizheliu-amazon approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@yizheliu-amazon yizheliu-amazon left a comment

Choose a reason for hiding this comment

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

Most of them look good to me

ylwu-amzn
ylwu-amzn approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the change!

@ohltyler
Copy link
Contributor Author

ohltyler commented Jun 9, 2020

Thanks for the reviews. Will hold off on merging until I get UX confirmation on wording/layout.

ylwu-amzn
ylwu-amzn approved these changes Jun 9, 2020
View reviewed changes
Copy link
Contributor

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the change.

@ohltyler ohltyler merged commit 827b0fb into opendistro-for-elasticsearch:master Jun 9, 2020
@ohltyler ohltyler deleted the addDeleteAction branch June 9, 2020 22:06
@ohltyler ohltyler mentioned this pull request Jun 9, 2020
Merged
@ohltyler ohltyler added the feature new feature label Jun 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@yizheliu-amazon yizheliu-amazon yizheliu-amazon approved these changes

@ylwu-amzn ylwu-amzn ylwu-amzn approved these changes

@kaituo kaituo Awaiting requested review from kaituo

Assignees
No one assigned
Labels
feature new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add delete batch action on detector list page
3 participants
@ohltyler @ylwu-amzn @yizheliu-amazon