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

[SIEM] Use bulk actions API when updating or deleting rules #54521

Merged
merged 4 commits into from
Jan 13, 2020

Conversation

spong
Copy link
Member

@spong spong commented Jan 11, 2020

Summary

This PR updates the All Rules Table actions to use the new bulk API introduced in #53543. More robust error reporting has also been added to let the user know exactly which operation has failed. Note that individual update/delete requests now also go through the bulk API as this simplifies the implementation and error handling.

Additional features:

Activate/Deactivate Before:

bulk_activate_before
(Ignore failed requests from test env -- request count is important here)

Activate/Deactivate After:

bulk_activate_after

Delete Before:

bulk_delete_before
(Ignore failed requests from test env -- request count is important here)

Delete After:

bulk_delete_after

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@spong spong added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Jan 11, 2020
@spong spong self-assigned this Jan 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

},
tags: [],
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

});
});
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the tests! 👍

export const DUPLICATE_RULE_ERROR = i18n.translate(
'xpack.siem.detectionEngine.rules.allRules.actions.duplicateRuleErrorDescription',
{
defaultMessage: 'Error duplicating rule…',
Copy link
Contributor

Choose a reason for hiding this comment

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

You want to use ellipses or three dots?

If so just to let you know we have these other two that aren't?

Screen Shot 2020-01-13 at 1 52 03 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

Design has been having them as ellipses iirc, so I've been following that convention. @MichaelMarcialis, can you confirm which we should be conforming too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Great question. I'd prefer our usage be of a horizontal ellipsis ( or …) going forward.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect -- thanks @MichaelMarcialis! I've created #54677 for tracking so it doesn't get lost in the shuffle! 🙂

Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

Checked it out and tested it and everything works as expected.

Really fast changes for this appreciate it!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@spong spong merged commit 6f54c06 into elastic:master Jan 13, 2020
@spong spong deleted the rules-batch-actions branch January 13, 2020 22:25
spong added a commit to spong/kibana that referenced this pull request Jan 13, 2020
…54521)

## Summary

This PR updates the `All Rules Table` actions to use the new bulk API introduced in elastic#53543. More robust error reporting has also been added to let the user know exactly which operation has failed. Note that individual `update`/`delete` requests now also go through the bulk API as this simplifies the implementation and error handling.

Additional features:
* Adds toast error when failing to activate, deactivate or delete a rule (related elastic#54515)
* Extracted commonly used toast utility for better re-use
* Removes ability to delete `immutable` rules


##### Activate/Deactivate Before:
![bulk_activate_before](https://user-images.githubusercontent.com/2946766/72196245-0ea50300-33d4-11ea-8d49-5ebdb63db1a1.gif)
(Ignore failed requests from test env -- request count is important here)


##### Activate/Deactivate After:
![bulk_activate_after](https://user-images.githubusercontent.com/2946766/72196361-c0443400-33d4-11ea-9a42-11f66c64e925.gif)



##### Delete Before:
![bulk_delete_before](https://user-images.githubusercontent.com/2946766/72196249-149ae400-33d4-11ea-80fc-b2f7fb83245f.gif)
(Ignore failed requests from test env -- request count is important here)

##### Delete After:
![bulk_delete_after](https://user-images.githubusercontent.com/2946766/72196366-c803d880-33d4-11ea-90d8-f1917b18035f.gif)

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

- [x] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)
- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] ~[Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] ~This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~

### For maintainers

- [ ] ~This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
- [ ] ~This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
spong added a commit that referenced this pull request Jan 14, 2020
…54663)

## Summary

This PR updates the `All Rules Table` actions to use the new bulk API introduced in #53543. More robust error reporting has also been added to let the user know exactly which operation has failed. Note that individual `update`/`delete` requests now also go through the bulk API as this simplifies the implementation and error handling.

Additional features:
* Adds toast error when failing to activate, deactivate or delete a rule (related #54515)
* Extracted commonly used toast utility for better re-use
* Removes ability to delete `immutable` rules


##### Activate/Deactivate Before:
![bulk_activate_before](https://user-images.githubusercontent.com/2946766/72196245-0ea50300-33d4-11ea-8d49-5ebdb63db1a1.gif)
(Ignore failed requests from test env -- request count is important here)


##### Activate/Deactivate After:
![bulk_activate_after](https://user-images.githubusercontent.com/2946766/72196361-c0443400-33d4-11ea-9a42-11f66c64e925.gif)



##### Delete Before:
![bulk_delete_before](https://user-images.githubusercontent.com/2946766/72196249-149ae400-33d4-11ea-80fc-b2f7fb83245f.gif)
(Ignore failed requests from test env -- request count is important here)

##### Delete After:
![bulk_delete_after](https://user-images.githubusercontent.com/2946766/72196366-c803d880-33d4-11ea-90d8-f1917b18035f.gif)

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

- [x] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)
- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] ~[Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] ~This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~

### For maintainers

- [ ] ~This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
- [ ] ~This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 14, 2020
* upstream/master: (26 commits)
  Take page offset into account too (elastic#54567)
  [APM] Support error.{log,exception}.stacktrace.classname (elastic#54577)
  Np migration tsvb route validation (elastic#51850)
  [ML] MML calculator enhancements for multi-metric job wizard  (elastic#54573)
  [SIEM] Fix Inspect query 'request timestamp' value changes when curso… (elastic#54223)
  Fix chromeless NP apps not using full page width (elastic#54550)
  Remove extraneous public import to prevent failing Kibana startup (elastic#54676)
  [Uptime] Temporarily skip flakey tests (elastic#54675)
  Skip failing uptime tests
  Create UI for alerting and actions plugin (elastic#48959)
  [dev/build/sass] build stylesheets for disabled plugins too (elastic#54654)
  [SIEM] Use bulk actions API when updating or deleting rules (elastic#54521)
  Support "Deprecated" label in advanced settings (elastic#54539)
  [Maps] add text halo color and width style properties (elastic#53827)
  Service Map Data API at Runtime (elastic#54027)
  [SIEM] Detection Engine Create Rule Design Review #1 (elastic#54442)
  Skip flaky test
  [Canvas] Enable Embeddable maps (elastic#53971)
  [SIEM][Detection Engine] Increases the number or rules you can view on a single page (elastic#54628)
  uiSettings - use validation field for image field maxSize (elastic#54522)
  ...
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 17, 2020
…54521)

## Summary

This PR updates the `All Rules Table` actions to use the new bulk API introduced in elastic#53543. More robust error reporting has also been added to let the user know exactly which operation has failed. Note that individual `update`/`delete` requests now also go through the bulk API as this simplifies the implementation and error handling.

Additional features:
* Adds toast error when failing to activate, deactivate or delete a rule (related elastic#54515)
* Extracted commonly used toast utility for better re-use
* Removes ability to delete `immutable` rules


##### Activate/Deactivate Before:
![bulk_activate_before](https://user-images.githubusercontent.com/2946766/72196245-0ea50300-33d4-11ea-8d49-5ebdb63db1a1.gif)
(Ignore failed requests from test env -- request count is important here)


##### Activate/Deactivate After:
![bulk_activate_after](https://user-images.githubusercontent.com/2946766/72196361-c0443400-33d4-11ea-9a42-11f66c64e925.gif)



##### Delete Before:
![bulk_delete_before](https://user-images.githubusercontent.com/2946766/72196249-149ae400-33d4-11ea-80fc-b2f7fb83245f.gif)
(Ignore failed requests from test env -- request count is important here)

##### Delete After:
![bulk_delete_after](https://user-images.githubusercontent.com/2946766/72196366-c803d880-33d4-11ea-90d8-f1917b18035f.gif)

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

- [x] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)
- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] ~[Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] ~This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~

### For maintainers

- [ ] ~This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
- [ ] ~This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants