[Security Solution] Add an enable or disabled state rules filter #150153

maximpn · 2023-02-02T11:26:25Z

Addresses: #132497

Summary

This PR adds a rules table filter for the enabled and disabled state.

Filter in action:

Screen.Recording.2023-02-02.at.12.45.17.mov

Since the added filter occupies some space the search field may shrink too much. To address this issue wrapping was enabled for the filter row.

Layout responsiveness:

Screen.Recording.2023-02-02.at.17.16.44.mov

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
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 · 2023-02-02T12:04:33Z

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

kibana-ci · 2023-02-02T17:48:38Z

💚 Build Succeeded

Buildkite Build
Commit: d61662f

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`	12.9MB	12.9MB	+2.4KB

History

💚 Build #105064 succeeded a036b39

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

cc @maximpn

spong · 2023-02-02T22:15:15Z

...engine/rule_management_ui/components/rules_table/rules_table_filters/rules_table_filters.tsx

+        <EuiFilterGroup>
+          <EuiFilterButton
+            hasActiveFilters={enabled === true}
+            onClick={handleShowEnabledRulesClick}
+            data-test-subj="showEnabledRulesFilterButton"
+            withNext
+          >
+            {i18n.ENABLED_RULES}
+          </EuiFilterButton>
+          <EuiFilterButton
+            hasActiveFilters={enabled === false}
+            onClick={handleShowDisabledRulesClick}
+            data-test-subj="showDisabledRulesFilterButton"
+          >
+            {i18n.DISABLED_RULES}
+          </EuiFilterButton>


Since space is of concern with regards to shrinking the search bar, we could always use a drop down for Rule state like they're doing on the Stack Mgmt Rules table (though w/o the extra width 😅):

We will need something like that for 8.8 since we plan to add even more filters. For now I'd like to keep it simple.

spong · 2023-02-02T22:24:44Z

...ne/rule_management_ui/components/rules_table/rules_table/use_sync_rules_table_saved_state.ts

+    if (state.filterOptions.enabled !== undefined) {
+      urlStateToSave.enabled = state.filterOptions.enabled;
+      storageStateToSave.enabled = state.filterOptions.enabled;
+    }
+


Commenting here since this is where sessionStorage.set() is called below to set the local storage values.

In testing, I had cleared my localstorage via devtools, and now I'm not seeing it be set again? I did a clean deployment and am still seeing this behavior. I'm seeing the urlParams get set, but not localstorage... 🤔

When debugging, for some reason RULES_TABLE_STATE_STORAGE_KEY is showing as undefined (even though it is definitely in the constants file).

Also seeing this Typescript error, but when debugging it seems to go into that function and work fine:

@spong thank you for the review!

Could you describe a reproduction scenario when you don't see the state restored in the session storage?

Basically something should happen to update the saved state in the session storage, for example a page reload or state update. I've tested in Chrome and FF and it works well though I have issues with displaying all the session and local storage values in newest Chrome. It worked well after reinstalling Chrome but I have the same problem again. Maybe you experience the same.

Regarding unavailability of RULES_TABLE_STATE_STORAGE_KEY value for debugging. I experience the same but it looks related to the dev tool or it's transpiled the way when constants are inlined.

Regarding TS errors it seems like a local issue. I don't see anything like that. If you just switched a branch and IDE hasn't updated its cache or installed another TS version locally it can be an issue.

I'll double check with the PR implementing the persistence rules table state test plan.

Could you describe a reproduction scenario when you don't see the state restored in the session storage?

Basically something should happen to update the saved state in the session storage, for example a page reload or state update. I've tested in Chrome and FF and it works well though I have issues with displaying all the session and local storage values in newest Chrome. It worked well after reinstalling Chrome but I have the same problem again. Maybe you experience the same.

Interesting... 🤔 As for repro steps, the only thing I did here was manually delete the securitySolution.rulesTable key under Application -> Local Storage in Dev Tools (at least I recall it being Local Storage, not Session). After poking around a bit more though, it seems it's being added back to the Session Storage section in dev tools instead of Local Storage where the other securitySolution keys are:

Local Storage

Sessions Storage

I guess this explains why the app is working as expected, as I didn't notice that it was storing to session vs local storage. I was pretty sure I first saw the key in Local Storage, but I could be mistaken here. Either way, sounds like everything is 👍 here and working as expected.

Regarding unavailability of RULES_TABLE_STATE_STORAGE_KEY value for debugging. I experience the same but it looks related to the dev tool or it's transpiled the way when constants are inlined.

Regarding TS errors it seems like a local issue. I don't see anything like that. If you just switched a branch and IDE hasn't updated its cache or installed another TS version locally it can be an issue.

Yeah that was my thought as well (as I often see type issues the end up getting resolved when clicking through to the source). Though with this now merged on main and after a fresh bootstrap, I'm still seeing the same type error. Modifying line 34 to be const urlStateToSave: RisonValue = {}; instead of RulesTableUrlSavedState resolves the error, or of course as casting urlStateToSave as RisonValue inline on line 93 as well. So seems RulesTableUrlSavedState should extend from RisonValue or we should update our types in useReplaceUrlParams() then?

spong

Checked out, tested locally, and code reviewed -- LGTM!

I had one question about localstorage as once I cleared my localstorage via devtools I'm no longer seeing it being set, but if there's no issue there then this is good to merge. 👍

elasticmachine · 2023-02-03T21:43:32Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

…stic#150153) **Addresses:** elastic#132497 ## Summary This PR adds a rules table filter for the enabled and disabled state. *Filter in action*: https://user-images.githubusercontent.com/3775283/216316405-27ad8b21-6392-4705-8af8-53c746a00acf.mov Since the added filter occupies some space the search field may shrink too much. To address this issue wrapping was enabled for the filter row. *Layout responsiveness:* https://user-images.githubusercontent.com/3775283/216380837-3bda2072-4f5b-4754-8e57-8978c5e2b0d5.mov ### Checklist - [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/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

add an ability to filter rules by enabled field

1aa010f

maximpn self-assigned this Feb 2, 2023

persists rules enabled state filter

a036b39

maximpn marked this pull request as ready for review February 2, 2023 12:04

maximpn requested a review from a team as a code owner February 2, 2023 12:04

maximpn requested a review from spong February 2, 2023 12:04

banderror added release_note:enhancement and removed release_note:feature Makes this part of the condensed release notes labels Feb 2, 2023

avoid too much shrinking the rule search field

d61662f

spong reviewed Feb 2, 2023

View reviewed changes

spong approved these changes Feb 2, 2023

View reviewed changes

banderror linked an issue Feb 3, 2023 that may be closed by this pull request

[Security Solution] Add a way to filter rules on enabled or disabled state #132497

Closed

banderror added Team:Detections and Resp Security Detection Response Team and removed Team:Detection Alerts Security Detection Alerts Area Team labels Feb 3, 2023

maximpn merged commit 949cb58 into elastic:main Feb 4, 2023

maximpn deleted the filter-rules-by-enabled-field branch February 7, 2023 12:59

joepeeples mentioned this pull request Feb 14, 2023

Rules table UX enhancements for 8.7 elastic/security-docs#2979

Closed

banderror mentioned this pull request Feb 21, 2023

[DOCS] Update screenshots of the Rules table elastic/security-docs#3005

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Add an enable or disabled state rules filter #150153

[Security Solution] Add an enable or disabled state rules filter #150153

maximpn commented Feb 2, 2023 •

edited

elasticmachine commented Feb 2, 2023

kibana-ci commented Feb 2, 2023

spong Feb 2, 2023

maximpn Feb 4, 2023

spong Feb 2, 2023

maximpn Feb 3, 2023 •

edited

spong Feb 6, 2023 •

edited

spong left a comment

elasticmachine commented Feb 3, 2023

[Security Solution] Add an enable or disabled state rules filter #150153

[Security Solution] Add an enable or disabled state rules filter #150153

Conversation

maximpn commented Feb 2, 2023 • edited

Summary

Checklist

elasticmachine commented Feb 2, 2023

kibana-ci commented Feb 2, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

History

spong Feb 2, 2023

Choose a reason for hiding this comment

maximpn Feb 4, 2023

Choose a reason for hiding this comment

spong Feb 2, 2023

Choose a reason for hiding this comment

maximpn Feb 3, 2023 • edited

Choose a reason for hiding this comment

spong Feb 6, 2023 • edited

Choose a reason for hiding this comment

Local Storage

Sessions Storage

spong left a comment

Choose a reason for hiding this comment

elasticmachine commented Feb 3, 2023

maximpn commented Feb 2, 2023 •

edited

maximpn Feb 3, 2023 •

edited

spong Feb 6, 2023 •

edited