[Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal #76145

yctercero · 2020-08-27T19:47:07Z

Summary

Fixes bug found in 7.9.

Current behavior:

Go to alerts page
Select action to add exception item
Select checkbox to close all alerts matching exception
Click Add exception
Note that the alerts table does not update until after you refresh or toggle between Closed and Open

New behavior:

Go to alerts page
Select action to add exception item
Select checkbox to close all alerts matching exception
Click Add exception
Note that the alerts table updates

Fix

Note the updated alerts total at the end.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

elasticmachine · 2020-08-27T20:19:37Z

Pinging @elastic/siem (Team:SIEM)

yctercero · 2020-08-27T20:21:48Z

x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.tsx

@@ -259,6 +261,7 @@ const EventsViewerComponent: React.FC<Props> = ({
                      </HeaderFilterGroupWrapper>
                    )}
                  </HeaderSection>
+                  {exceptionsModal && exceptionsModal(refetch)}


This is the callback that the modal needed access to. I followed the pattern used for the utility bar here.

yctercero · 2020-08-27T20:22:53Z

x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx

@@ -220,6 +223,7 @@ type PropsFromRedux = ConnectedProps<typeof connector>;
 export const StatefulEventsViewer = connector(
  React.memo(
    StatefulEventsViewerComponent,
+    // eslint-disable-next-line complexity


The complexity warning is a good warning that we can use to track tech debt. Looking below, this indeed looks close to needing a refactor from someone soon-ish.

dhurley14

LGTM! I think a future update could include a count of success / failures for when all the alerts are closed. But this looks great!

dplumlee

Left a thought about tests we might wanna consider later on but other than that looks good

dplumlee · 2020-08-27T22:03:15Z

x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.test.tsx

+      });
+    });
+
+    test('it does not render exception modal if "exceptionModal" callback does not exist', async () => {


i think for the future we might wanna take a look at the structure of these files so we can lessen the number of mounts we're using, especially in cases where this test block is identical to others in this file save for the expect function. Could be a way to speed up the tests

That's a great point - @patrykkopycinski is working on refactoring this code, may be interesting to look at patterns there #73228

yctercero · 2020-08-27T22:10:33Z

@elasticmachine merge upstream

peluja1012 · 2020-08-27T22:12:21Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/index.tsx

+          />
+        );
+      } else {
+        return <></>;


Small nit. We could return null here instead. Otherwise I think we'll end up with an unnecessary empty div.

👍 that's a good point. Since this file is being largely refactored for 7.10, I'm leaving here for now if that's ok. Happy to revisit too.

For clarity a react fragment <></> won't insert an empty div:
https://reactjs.org/docs/fragments.html

However, returning null should be faster than a react fragment and when possible to return null I think we should.

Thanks for the info there @FrankHassanabad ! The code was reworked here so I think it's been updated - https://github.com/elastic/kibana/pull/73228/files#diff-2d4b35336b307ef04e3129bd4aecf10aL470

peluja1012

Pulled down the branch and tested functionality in both the Detection Alerts page and the Rule Details page with both the "close" and "bulk close" options. It's working as expected now. Thanks for the fix!

kibanamachine · 2020-08-27T23:58:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 77b8b33

Build metrics

async chunks size

id	value	diff	baseline
securitySolution	9.5MB	+875.0B	9.5MB

History

💚 Build #71040 succeeded 15b89cf
💛 Build #71019 was flaky 7cf701a

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

…r closure from exceptions modal (elastic#76145) ## Summary Fixes bug found in 7.9. **Current behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table does not update until after you refresh or toggle between `Closed` and `Open` **New behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table updates

…r closure from exceptions modal (#76145) (#76217) ## Summary Fixes bug found in 7.9. **Current behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table does not update until after you refresh or toggle between `Closed` and `Open` **New behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table updates

…r closure from exceptions modal (#76145) (#76216) ## Summary Fixes bug found in 7.9. **Current behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table does not update until after you refresh or toggle between `Closed` and `Open` **New behavior:** - Go to alerts page - Select action to add exception item - Select checkbox to close all alerts matching exception - Click `Add exception` - Note that the alerts table updates Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

…s-for-710 * 'master' of github.com:elastic/kibana: (43 commits) [APM] Chart units don't update when toggling the chart legends (elastic#74931) [ILM] Add support for frozen phase in UI (elastic#75968) Hides advanced json for count metric (elastic#74636) add client-side feature usage API (elastic#75486) [Maps] add drilldown support map embeddable (elastic#75598) [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106) [Resolver] model `location.search` in redux (elastic#76140) [APM] Prevent imports of public in server code (elastic#75979) fix eslint issue skip flaky suite (elastic#76223) [APM] Transaction duration anomaly alerting integration (elastic#75719) [Transforms] Avoid using "Are you sure" (elastic#75932) [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145) [plugin-helpers] improve 3rd party KP plugin support (elastic#75019) [docs/getting-started] link to yarn v1 specifically (elastic#76169) [Security_Solution][Resolver] Resolver loading and error state (elastic#75600) Fixes App Search documentation links (elastic#76133) Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079) [Resolver] Fix useSelector usage (elastic#76129) [Enterprise Search] Migrate util and components from ent-search (elastic#76051) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts # x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts

FrankHassanabad · 2020-08-31T14:57:32Z

x-pack/plugins/security_solution/public/detections/components/alerts_table/index.tsx

@@ -228,7 +228,7 @@ export const AlertsTableComponent: React.FC<AlertsTableComponentProps> = ({
      exceptionListType,
      alertData,
    }: AddExceptionModalBaseProps) => {
-      if (alertData !== null && alertData !== undefined) {
+      if (alertData != null) {


Nice idiomatic JS fix using the != here 👍

elasticmachine · 2021-09-23T14:26:23Z

Pinging @elastic/security-solution (Team: SecuritySolution)

fix bug of alerts not updating after closure from exceptions modal

7cf701a

yctercero self-assigned this Aug 27, 2020

yctercero added release_note:fix v7.10.0 v7.9.1 v8.0.0 Feature:Detection Rules Anything related to Security Solution's Detection Rules Team:SIEM labels Aug 27, 2020

yctercero marked this pull request as ready for review August 27, 2020 20:19

yctercero requested review from a team as code owners August 27, 2020 20:19

yctercero commented Aug 27, 2020

View reviewed changes

no functional changes, just cleanup

15b89cf

dhurley14 approved these changes Aug 27, 2020

View reviewed changes

dplumlee approved these changes Aug 27, 2020

View reviewed changes

Merge branch 'master' into update_alerts

77b8b33

peluja1012 reviewed Aug 27, 2020

View reviewed changes

peluja1012 approved these changes Aug 27, 2020

View reviewed changes

yctercero merged commit 69599e0 into elastic:master Aug 28, 2020

yctercero mentioned this pull request Aug 28, 2020

[7.x] [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (#76145) #76216

Merged

yctercero mentioned this pull request Aug 28, 2020

[7.9] [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (#76145) #76217

Merged

FrankHassanabad reviewed Aug 31, 2020

View reviewed changes

yctercero deleted the update_alerts branch September 3, 2020 14:23

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal #76145

[Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal #76145

yctercero commented Aug 27, 2020 •

edited

Loading

elasticmachine commented Aug 27, 2020

yctercero Aug 27, 2020

yctercero Aug 27, 2020

FrankHassanabad Aug 31, 2020

dhurley14 left a comment •

edited

Loading

dplumlee left a comment

dplumlee Aug 27, 2020

yctercero Aug 27, 2020

yctercero commented Aug 27, 2020

peluja1012 Aug 27, 2020

yctercero Aug 27, 2020

FrankHassanabad Aug 31, 2020

yctercero Sep 2, 2020

peluja1012 left a comment

kibanamachine commented Aug 27, 2020

FrankHassanabad Aug 31, 2020

elasticmachine commented Sep 23, 2021

[Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal #76145

[Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal #76145

Conversation

yctercero commented Aug 27, 2020 • edited Loading

Summary

Fix

Checklist

elasticmachine commented Aug 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhurley14 left a comment • edited Loading

Choose a reason for hiding this comment

dplumlee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yctercero commented Aug 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peluja1012 left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 27, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

Choose a reason for hiding this comment

elasticmachine commented Sep 23, 2021

yctercero commented Aug 27, 2020 •

edited

Loading

dhurley14 left a comment •

edited

Loading