[Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. #155861

logeekal · 2023-04-26T12:02:48Z

Summary

This PR handles : [Security Solution] New alert filters are not taking into consideration alert table filters #155173 and #156252

Currently, Alert Page Controls do not take Alert Table Checkboxes ( Building block + Threat indicator alerts only ) into account. This PR enables the effect of Alert Table Checkboxes on Alert Page controls

Before	After
Screen.Recording.2023-04-26.at.14.00.50.mov	Screen.Recording.2023-04-26.at.13.33.20.mov

Checklist

Delete any items that are not applicable to this PR.

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

michaelolo24 · 2023-04-27T15:29:59Z

Do you plan on making the change for the enabling building_block_alerts by default on this branch or a separate branch?

michaelolo24 · 2023-04-27T15:36:13Z

x-pack/plugins/security_solution/public/detections/pages/detection_engine/detection_engine.tsx

-    },
-    [clearEventsLoading, clearEventsDeleted, setStatusFilter]
-  );
+  const onFilterGroupChangedCallback = useCallback((newFilterGroup: Status) => {}, []);


This isn't doing anything anymore? Do we not need to clear the loading/deleted states or update the status filter onChange anymore?

+1, wondering this as well.

@janmonschke and @michaelolo24

Sorry, this change should have been in another branch. I have reverted this. In a separate branch, I was trying to remove old Alert State toggle from the alert page so that feature flag of Alert Page Controls will no longer be needed.

But now I am not sure, because we need to keep it in the minor releases of 8.7 so for now I will keep it and we can remove it later. Hence, reverted that change to same as main

janmonschke · 2023-04-28T09:19:25Z

...plugins/security_solution/public/detections/pages/detection_engine/detection_engine.test.tsx

+  const store = createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);
+  return store;


Suggested change

const store = createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);

return store;

return createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);

💅

One less const store in this file ;)

janmonschke · 2023-04-28T09:24:55Z

...plugins/security_solution/public/detections/pages/detection_engine/detection_engine.test.tsx

+      );
+    });
+
+    expect(MockedFilterGroup.mock.calls[0][0].filters[0]).toMatchObject(


I wonder if it's possible to use toHaveBeenCalledWith here to avoid reaching into the specifics of the mock implementation.

E.g.

expect(MockedFilterGroup.toMatchObject(expect.objectContaining({ filters: expect.objectContaining({ ...

janmonschke · 2023-04-28T09:25:13Z

...plugins/security_solution/public/detections/pages/detection_engine/detection_engine.test.tsx

    });
+
+    expect(MockedFilterGroup.mock.calls[0][0].filters[0]).toMatchObject(


And then here as well

janmonschke · 2023-04-28T09:27:33Z

x-pack/plugins/security_solution/public/detections/pages/detection_engine/detection_engine.tsx

-    },
-    [clearEventsLoading, clearEventsDeleted, setStatusFilter]
-  );
+  const onFilterGroupChangedCallback = useCallback((newFilterGroup: Status) => {}, []);


+1, wondering this as well.

logeekal · 2023-05-02T11:29:47Z

Do you plan on making the change for the enabling building_block_alerts by default on this branch or a separate branch?

I can do that separately as I need some discussion on the same. Thanks.

michaelolo24

Thanks for making the changes! Tested locally and the filters respond as expected to changes in the table additional filters 👍🏾

kibana-ci · 2023-05-02T13:47:13Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 0fc3af5

Failed CI Steps

Test Failures

[job] [logs] Security Solution Tests #3 / timeline flyout button the (+) button popover menu owns focus

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`	9.1MB	9.1MB	+33.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	399	402	+3
total			+5

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	479	482	+3
total			+5

History

💚 Build #124701 succeeded 2b5f74c
💔 Build #124283 failed 00d53f5
💛 Build #123437 was flaky 20b2af9

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

… "Additional Filters" into account. (elastic#155861) ## Summary This PR handles : [Security Solution] New alert filters are not taking into consideration alert table filters elastic#155173 and elastic#156252 Currently, Alert Page Controls do not take Alert Table Checkboxes ( Building block + Threat indicator alerts only ) into account. This PR enables the effect of Alert Table Checkboxes on Alert Page controls | Before | After | |--|--| | <video src="https://user-images.githubusercontent.com/7485038/234568673-b05ffc32-09bc-4378-aecb-bb64dbc5cbb6.mov" /> | <video src="https://user-images.githubusercontent.com/7485038/234562759-6309d6a0-f7db-48fd-bd9e-62788a1c89fe.mov"/> | ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios. (cherry picked from commit aeded80)

kibanamachine · 2023-05-02T13:54:51Z

💚 All backports created successfully

Status	Branch	Result
✅	8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

… Table "Additional Filters" into account. (#155861) (#156388) # Backport This will backport the following commits from `main` to `8.8`: - [[Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. (#155861)](#155861)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jatin Kathuria <jatin.kathuria@elastic.co>

fix: include alert table Filters in Page Controls

20b2af9

logeekal added the Team:Threat Hunting:Investigations Security Solution Investigations Team label Apr 26, 2023

logeekal requested a review from a team as a code owner April 26, 2023 12:02

logeekal changed the title ~~fix: include alert table Filters in Page Controls~~ [Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. Apr 26, 2023

logeekal added v8.8.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Apr 26, 2023

michaelolo24 reviewed Apr 27, 2023

View reviewed changes

janmonschke reviewed Apr 28, 2023

View reviewed changes

logeekal added 2 commits April 28, 2023 16:21

tests: better

00d53f5

Merge branch 'main' into fix/filter_group_controls_query_#155173

2b5f74c

michaelolo24 approved these changes May 2, 2023

View reviewed changes

Merge branch 'main' into fix/filter_group_controls_query_#155173

0fc3af5

logeekal enabled auto-merge (squash) May 2, 2023 12:14

logeekal merged commit aeded80 into elastic:main May 2, 2023

kibanamachine mentioned this pull request May 2, 2023

[8.8] [Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. (#155861) #156388

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. #155861

[Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. #155861

logeekal commented Apr 26, 2023 •

edited by kibanamachine

Loading

michaelolo24 commented Apr 27, 2023

michaelolo24 Apr 27, 2023

janmonschke Apr 28, 2023

logeekal Apr 28, 2023 •

edited

Loading

janmonschke Apr 28, 2023

logeekal Apr 28, 2023

janmonschke Apr 28, 2023

logeekal Apr 28, 2023

janmonschke Apr 28, 2023

logeekal Apr 28, 2023

janmonschke Apr 28, 2023

logeekal commented May 2, 2023

michaelolo24 left a comment

kibana-ci commented May 2, 2023

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented May 2, 2023

		const store = createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);
		return store;

	const store = createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);
	return store;
	return createStore(newState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);

		});

		expect(MockedFilterGroup.mock.calls[0][0].filters[0]).toMatchObject(

[Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. #155861

[Security Solution] [Fix] Alert Page Controls do not take Alert Table "Additional Filters" into account. #155861

Conversation

logeekal commented Apr 26, 2023 • edited by kibanamachine Loading

Summary

Checklist

michaelolo24 commented Apr 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

logeekal Apr 28, 2023 • edited Loading

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

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

logeekal commented May 2, 2023

michaelolo24 left a comment

Choose a reason for hiding this comment

kibana-ci commented May 2, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented May 2, 2023

💚 All backports created successfully

Questions ?

logeekal commented Apr 26, 2023 •

edited by kibanamachine

Loading

logeekal Apr 28, 2023 •

edited

Loading