[Security Solution][Endpoint] Delete Event Filter from the List #99686

paul-tavares · 2021-05-10T20:26:30Z

Summary

Enables the Delete button on the Event Filter cards, which will show the user an confirmation modal before proceeding to delete the event filter entry

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…t-filter-delete # Conflicts: # x-pack/plugins/security_solution/public/management/pages/event_filters/state/index.ts # x-pack/plugins/security_solution/public/management/pages/event_filters/store/builders.ts # x-pack/plugins/security_solution/public/management/pages/event_filters/store/middleware.ts

elasticmachine · 2021-05-11T18:22:47Z

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

pzl · 2021-05-11T18:41:16Z

x-pack/plugins/security_solution/public/management/pages/event_filters/store/middleware.ts

+
+  dispatch({
+    type: 'eventFilterDeleteStatusChanged',
+    // Ignore will be fixed with when AsyncResourceState is refactored (#830)


…t-filter-delete

dasansol92

Just a few comments (mostly questions) but it looks great!!! 🔥 🔥
Could you add some tests for the middleware and the new reducers?

dasansol92 · 2021-05-12T09:27:28Z

...solution/public/management/pages/event_filters/view/components/event_filter_delete_modal.tsx

+  // Show toast for success
+  useEffect(() => {
+    if (wasDeleted) {
+      toasts.addSuccess(


I think you can manage this with the useEventFiltersNotification hook adding the new conditions for delete action.

I feel that having the notifications closer to the code that generates them makes the component (in this case the delete modal) clearer to understand. Someone looking at this component would not be able to understand how errors/success are presented back to the UI - they would have to go look elsewhere. I think it also overloads that hook in that it starts to have too broad of concerns. I remember having that same feedback when that Trusted Apps approach was done - several new approaches were trialed during that implementation, this being one of them.

Give this background, let me know if you still feel this should be moved over to the hook - I can do that if feels like the better approach.

Oh I see. Yeah it make sense.
Idea - Maybe we could move this logic to the middleware (not now) just to keep the component itself as clean as we can. What I'm thinking is have an entry on our redux store for notifications and have a component that listens it to display the toast or not.
Doing this we could move all of this logic (this and the ones in the hook) to the middleware and let components more clean.

@dasansol92 yeah, that could work. Maybe we add this as a topic of discussion for our weekly eng. office hours.

dasansol92 · 2021-05-12T09:27:50Z

...solution/public/management/pages/event_filters/view/components/event_filter_delete_modal.tsx

+  useEffect(() => {
+    if (deleteError) {
+      toasts.addDanger(
+        i18n.translate('xpack.securitySolution.eventFilters.deletionDialog.deleteFailure', {


Same comment as above

x-pack/plugins/security_solution/public/management/pages/event_filters/store/action.ts

...ins/security_solution/public/management/pages/event_filters/view/event_filters_list_page.tsx

…t-filter-delete # Conflicts: # x-pack/plugins/security_solution/public/management/pages/event_filters/view/event_filters_list_page.tsx

…ItemSchema`

kibanamachine · 2021-05-12T17:12:11Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	6.7MB	6.7MB	+8.5KB

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`canvas`	29	25	-4
`crossClusterReplication`	8	6	-2
`fleet`	4	2	-2
`globalSearch`	4	2	-2
`indexManagement`	12	7	-5
`infra`	5	3	-2
`licensing`	18	15	-3
`monitoring`	109	56	-53
total			-73

History

💚 Build #125408 succeeded 1de90c7
💛 Build #125382 was flaky 144e570
💔 Build #125175 failed 15ae3d8

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

cc @paul-tavares

dasansol92

🔥 🔥 🚢

…tic#99686) * modal for delete of event filter * Add total count of items to the List view

kibanamachine · 2021-05-13T12:06:59Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…) (#100024) * modal for delete of event filter * Add total count of items to the List view Co-authored-by: Paul Tavares <[email protected]>

paul-tavares added 3 commits May 10, 2021 13:46

Initial structure for modal delete dialog

02ef5d3

show and cancel delete modal

ba5beef

Delete event filters using API, show toast and refresh list

15ae3d8

paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels May 10, 2021

paul-tavares self-assigned this May 10, 2021

paul-tavares added 5 commits May 10, 2021 17:50

Fix type failure

a28189d

Fix types after merge

b52ba9a

test for list page to ensure delete modal is shown

a9e70cb

Tests for the Delete modal

144e570

paul-tavares marked this pull request as ready for review May 11, 2021 18:22

paul-tavares requested a review from a team as a code owner May 11, 2021 18:22

pzl approved these changes May 11, 2021

View reviewed changes

paul-tavares added 2 commits May 11, 2021 16:16

Add total count of items to the List view

af8dacc

Merge remote-tracking branch 'upstream/master' into task/olm-956-even…

1de90c7

…t-filter-delete

dasansol92 reviewed May 12, 2021

View reviewed changes

paul-tavares requested a review from dasansol92 May 12, 2021 12:59

paul-tavares added 2 commits May 12, 2021 09:59

Merge remote-tracking branch 'upstream/master' into task/olm-956-even…

1c9014d

…t-filter-delete # Conflicts: # x-pack/plugins/security_solution/public/management/pages/event_filters/view/event_filters_list_page.tsx

Change EventFilterForDeletion action to take in full `ExceptionList…

bc9ebb1

…ItemSchema`

dasansol92 approved these changes May 13, 2021

View reviewed changes

paul-tavares merged commit 6319544 into elastic:master May 13, 2021

paul-tavares deleted the task/olm-956-event-filter-delete branch May 13, 2021 12:03

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 13, 2021

[Security Solution][Endpoint] Delete Event Filter from the List (elas…

882a5e8

…tic#99686) * modal for delete of event filter * Add total count of items to the List view

kibanamachine mentioned this pull request May 13, 2021

[7.x] [Security Solution][Endpoint] Delete Event Filter from the List (#99686) #100024

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Endpoint] Delete Event Filter from the List #99686

[Security Solution][Endpoint] Delete Event Filter from the List #99686

paul-tavares commented May 10, 2021 •

edited

Loading

elasticmachine commented May 11, 2021

pzl May 11, 2021

dasansol92 left a comment

dasansol92 May 12, 2021

paul-tavares May 12, 2021

dasansol92 May 12, 2021

paul-tavares May 12, 2021

dasansol92 May 12, 2021

kibanamachine commented May 12, 2021

References to deprecated APIs

dasansol92 left a comment

kibanamachine commented May 13, 2021

[Security Solution][Endpoint] Delete Event Filter from the List #99686

[Security Solution][Endpoint] Delete Event Filter from the List #99686

Conversation

paul-tavares commented May 10, 2021 • edited Loading

Summary

Checklist

elasticmachine commented May 11, 2021

pzl May 11, 2021

Choose a reason for hiding this comment

dasansol92 left a comment

Choose a reason for hiding this comment

dasansol92 May 12, 2021

Choose a reason for hiding this comment

paul-tavares May 12, 2021

Choose a reason for hiding this comment

dasansol92 May 12, 2021

Choose a reason for hiding this comment

paul-tavares May 12, 2021

Choose a reason for hiding this comment

dasansol92 May 12, 2021

Choose a reason for hiding this comment

kibanamachine commented May 12, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

References to deprecated APIs

History

dasansol92 left a comment

Choose a reason for hiding this comment

kibanamachine commented May 13, 2021

💚 Backport successful

paul-tavares commented May 10, 2021 •

edited

Loading