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

[Security Solution] Fix rule filters on the Rule Details page #177081

Merged
merged 3 commits into from
Feb 19, 2024

Conversation

banderror
Copy link
Contributor

@banderror banderror commented Feb 15, 2024

Fixes: #141458
Fixes: #176866
Fixes: #175306

Summary

Fixes the bugs above by changing the Filters component:

  • from using lower-level components like FilterBadgeGroup and custom rendering
  • to using a higher-level FilterItems component that's used inside a larger component QueryBar (see the first screenshot below) on the Rule Creation / Editing pages

Note that for some reason the end result still does not fully equal to how filters look on the Rule Creation / Editing pages, where there are fewer warnings. It's hard to say which rendering is the right one. UPD: could be related to #178908.

Screenshots

How filters look on the Rule Creation / Editing pages:

Screenshot 2024-02-15 at 21 25 00

Rule Details page BEFORE the fix:

Screenshot 2024-02-15 at 21 23 46 Screenshot 2024-02-15 at 21 24 02 Screenshot 2024-02-15 at 21 24 18

Rule Details page AFTER the fix 1 (filters use non-existing fields and show warnings):

Screenshot 2024-02-15 at 21 28 46

Rule Details page AFTER the fix 2 (filters use existing fields and look normal):

Screenshot 2024-02-15 at 21 37 45

Checklist

For maintainers

@banderror banderror added bug Fixes for quality problems that affect the customer experience release_note:fix Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Feature:Rule Details Security Solution Detection Rule Details page 8.13 candidate v8.13.0 v8.12.2 labels Feb 15, 2024
@banderror banderror self-assigned this Feb 15, 2024
@banderror banderror added the impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. label Feb 15, 2024
@banderror
Copy link
Contributor Author

/ci

@banderror banderror force-pushed the fix-rule-filters-on-details-page branch from df0d9da to 00eacc0 Compare February 15, 2024 21:12
@banderror
Copy link
Contributor Author

/ci

@banderror banderror marked this pull request as ready for review February 16, 2024 09:01
@banderror banderror requested a review from a team as a code owner February 16, 2024 09:01
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@banderror banderror requested review from jpdjere and removed request for nikitaindik February 16, 2024 09:01
@banderror banderror force-pushed the fix-rule-filters-on-details-page branch 2 times, most recently from 9541ff3 to fb67b07 Compare February 19, 2024 10:07
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@banderror thanks for fixing the bug 👍

I left a couple of minor comments but there is nothing critical.

.euiBadge__text {
white-space: pre-wrap !important;
}
const FiltersWidthLimiter = styled.div`
Copy link
Contributor

Choose a reason for hiding this comment

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

While Styled Components are legitimate to be used Emotion is preferred (for example see this PR). EUI is migrating to Emotion and usually new code added to Security Solution uses Emotion as well. Usage is the following

import { css } from '@emotion/css';

...

const filtersWidthLimiter = css`
  max-width: 600px;
`;

...

<EuiFlexGroup className={filtersWidthLimiter} wrap responsive={false} gutterSize="xs" data-test-subj={dataTestSubj}>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maximpn I really liked the styling pattern used in #174867 👍
Thank you for sharing this. Addressed.

@banderror banderror force-pushed the fix-rule-filters-on-details-page branch from fb67b07 to 28e3349 Compare February 19, 2024 18:48
@banderror
Copy link
Contributor Author

banderror commented Feb 19, 2024

@maximpn I addressed your comments and enabled auto-merge. Feel free to check it one more time if you're interested. We can push any other improvements separately. Thank you!

@banderror banderror merged commit 2672662 into elastic:main Feb 19, 2024
36 checks passed
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5002 5003 +1

Async chunks

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

id before after diff
securitySolution 11.6MB 11.6MB -1.1KB

History

  • 💛 Build #194108 was flaky fb67b07cfc84b0413c756ff98750622382492aa3
  • 💔 Build #194009 failed 9541ff3210ed02da70772c383489def4ed0ca989
  • 💔 Build #193952 failed 00eacc08009c53c29d73485d02ba2e9099c69dc8
  • 💔 Build #193950 failed df0d9dab8a5f7c622e6548922f9d5c4daaa44c70

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

cc @banderror

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 19, 2024
…c#177081)

**Fixes: elastic#141458
**Fixes: elastic#176866

## Summary

Fixes the bugs above by changing the `Filters` component:

- from using lower-level components like `FilterBadgeGroup` and custom
rendering
- to using a higher-level `FilterItems` component that's used inside a
larger component `QueryBar` (see the first screenshot below) on the Rule
Creation / Editing pages

Note that for some reason the end result still does not fully equal to
how filters look on the Rule Creation / Editing pages, where there are
fewer warnings. It's hard to say which rendering is the right one.

## Screenshots

**How filters look on the Rule Creation / Editing pages:**

<img width="989" alt="Screenshot 2024-02-15 at 21 25 00"
src="https://github.com/elastic/kibana/assets/7359339/01ca468f-be99-469a-8d75-ee5aa1a31fb0">

**Rule Details page BEFORE the fix:**

<img width="1792" alt="Screenshot 2024-02-15 at 21 23 46"
src="https://github.com/elastic/kibana/assets/7359339/d0e2aa6e-3050-4327-8025-f37125498fd6">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 02"
src="https://github.com/elastic/kibana/assets/7359339/a89302b2-f991-4547-bdac-c0f5a594a881">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 18"
src="https://github.com/elastic/kibana/assets/7359339/49c16b02-8d82-4f93-932f-3846880a0457">

**Rule Details page AFTER the fix 1 (filters use non-existing fields and
show warnings):**

<img width="1790" alt="Screenshot 2024-02-15 at 21 28 46"
src="https://github.com/elastic/kibana/assets/7359339/e229b4ff-6ee7-4444-b5c1-deb00d2b9b39">

**Rule Details page AFTER the fix 2 (filters use existing fields and
look normal):**

<img width="1792" alt="Screenshot 2024-02-15 at 21 37 45"
src="https://github.com/elastic/kibana/assets/7359339/b10905e7-803d-4404-aa02-8692ff964891">

### Checklist

- [ ] [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
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] 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))
- [ ] 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))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 2672662)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
8.12 Backport failed because of merge conflicts
8.13

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 177081

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Feb 19, 2024
…177081) (#177242)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[Security Solution] Fix rule filters on the Rule Details page
(#177081)](#177081)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Georgii
Gorbachev","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-02-19T21:26:12Z","message":"[Security
Solution] Fix rule filters on the Rule Details page
(#177081)\n\n**Fixes:
https://github.com/elastic/kibana/issues/141458**\r\n**Fixes:
https://github.com/elastic/kibana/issues/176866**\r\n\r\n##
Summary\r\n\r\nFixes the bugs above by changing the `Filters`
component:\r\n\r\n- from using lower-level components like
`FilterBadgeGroup` and custom\r\nrendering\r\n- to using a higher-level
`FilterItems` component that's used inside a\r\nlarger component
`QueryBar` (see the first screenshot below) on the Rule\r\nCreation /
Editing pages\r\n\r\nNote that for some reason the end result still does
not fully equal to\r\nhow filters look on the Rule Creation / Editing
pages, where there are\r\nfewer warnings. It's hard to say which
rendering is the right one.\r\n\r\n## Screenshots\r\n\r\n**How filters
look on the Rule Creation / Editing pages:**\r\n\r\n<img width=\"989\"
alt=\"Screenshot 2024-02-15 at 21 25
00\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/01ca468f-be99-469a-8d75-ee5aa1a31fb0\">\r\n\r\n**Rule
Details page BEFORE the fix:**\r\n\r\n<img width=\"1792\"
alt=\"Screenshot 2024-02-15 at 21 23
46\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/d0e2aa6e-3050-4327-8025-f37125498fd6\">\r\n<img
width=\"1792\" alt=\"Screenshot 2024-02-15 at 21 24
02\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/a89302b2-f991-4547-bdac-c0f5a594a881\">\r\n<img
width=\"1792\" alt=\"Screenshot 2024-02-15 at 21 24
18\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/49c16b02-8d82-4f93-932f-3846880a0457\">\r\n\r\n**Rule
Details page AFTER the fix 1 (filters use non-existing fields
and\r\nshow warnings):**\r\n\r\n<img width=\"1790\" alt=\"Screenshot
2024-02-15 at 21 28
46\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/e229b4ff-6ee7-4444-b5c1-deb00d2b9b39\">\r\n\r\n**Rule
Details page AFTER the fix 2 (filters use existing fields and\r\nlook
normal):**\r\n\r\n<img width=\"1792\" alt=\"Screenshot 2024-02-15 at 21
37
45\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/b10905e7-803d-4404-aa02-8692ff964891\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[ ] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n\r\n###
For maintainers\r\n\r\n- [x] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"26726625394dfc4a5143dd1e1c895487c2f90380","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","impact:medium","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Rule Details","8.13
candidate","v8.13.0","v8.12.2","v8.14.0"],"title":"[Security Solution]
Fix rule filters on the Rule Details
page","number":177081,"url":"https://github.com/elastic/kibana/pull/177081","mergeCommit":{"message":"[Security
Solution] Fix rule filters on the Rule Details page
(#177081)\n\n**Fixes:
https://github.com/elastic/kibana/issues/141458**\r\n**Fixes:
https://github.com/elastic/kibana/issues/176866**\r\n\r\n##
Summary\r\n\r\nFixes the bugs above by changing the `Filters`
component:\r\n\r\n- from using lower-level components like
`FilterBadgeGroup` and custom\r\nrendering\r\n- to using a higher-level
`FilterItems` component that's used inside a\r\nlarger component
`QueryBar` (see the first screenshot below) on the Rule\r\nCreation /
Editing pages\r\n\r\nNote that for some reason the end result still does
not fully equal to\r\nhow filters look on the Rule Creation / Editing
pages, where there are\r\nfewer warnings. It's hard to say which
rendering is the right one.\r\n\r\n## Screenshots\r\n\r\n**How filters
look on the Rule Creation / Editing pages:**\r\n\r\n<img width=\"989\"
alt=\"Screenshot 2024-02-15 at 21 25
00\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/01ca468f-be99-469a-8d75-ee5aa1a31fb0\">\r\n\r\n**Rule
Details page BEFORE the fix:**\r\n\r\n<img width=\"1792\"
alt=\"Screenshot 2024-02-15 at 21 23
46\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/d0e2aa6e-3050-4327-8025-f37125498fd6\">\r\n<img
width=\"1792\" alt=\"Screenshot 2024-02-15 at 21 24
02\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/a89302b2-f991-4547-bdac-c0f5a594a881\">\r\n<img
width=\"1792\" alt=\"Screenshot 2024-02-15 at 21 24
18\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/49c16b02-8d82-4f93-932f-3846880a0457\">\r\n\r\n**Rule
Details page AFTER the fix 1 (filters use non-existing fields
and\r\nshow warnings):**\r\n\r\n<img width=\"1790\" alt=\"Screenshot
2024-02-15 at 21 28
46\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/e229b4ff-6ee7-4444-b5c1-deb00d2b9b39\">\r\n\r\n**Rule
Details page AFTER the fix 2 (filters use existing fields and\r\nlook
normal):**\r\n\r\n<img width=\"1792\" alt=\"Screenshot 2024-02-15 at 21
37
45\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/b10905e7-803d-4404-aa02-8692ff964891\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[ ] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n\r\n###
For maintainers\r\n\r\n- [x] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"26726625394dfc4a5143dd1e1c895487c2f90380"}},"sourceBranch":"main","suggestedTargetBranches":["8.13","8.12"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.12","label":"v8.12.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/177081","number":177081,"mergeCommit":{"message":"[Security
Solution] Fix rule filters on the Rule Details page
(#177081)\n\n**Fixes:
https://github.com/elastic/kibana/issues/141458**\r\n**Fixes:
https://github.com/elastic/kibana/issues/176866**\r\n\r\n##
Summary\r\n\r\nFixes the bugs above by changing the `Filters`
component:\r\n\r\n- from using lower-level components like
`FilterBadgeGroup` and custom\r\nrendering\r\n- to using a higher-level
`FilterItems` component that's used inside a\r\nlarger component
`QueryBar` (see the first screenshot below) on the Rule\r\nCreation /
Editing pages\r\n\r\nNote that for some reason the end result still does
not fully equal to\r\nhow filters look on the Rule Creation / Editing
pages, where there are\r\nfewer warnings. It's hard to say which
rendering is the right one.\r\n\r\n## Screenshots\r\n\r\n**How filters
look on the Rule Creation / Editing pages:**\r\n\r\n<img width=\"989\"
alt=\"Screenshot 2024-02-15 at 21 25
00\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/01ca468f-be99-469a-8d75-ee5aa1a31fb0\">\r\n\r\n**Rule
Details page BEFORE the fix:**\r\n\r\n<img width=\"1792\"
alt=\"Screenshot 2024-02-15 at 21 23
46\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/d0e2aa6e-3050-4327-8025-f37125498fd6\">\r\n<img
width=\"1792\" alt=\"Screenshot 2024-02-15 at 21 24
02\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/a89302b2-f991-4547-bdac-c0f5a594a881\">\r\n<img
width=\"1792\" alt=\"Screenshot 2024-02-15 at 21 24
18\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/49c16b02-8d82-4f93-932f-3846880a0457\">\r\n\r\n**Rule
Details page AFTER the fix 1 (filters use non-existing fields
and\r\nshow warnings):**\r\n\r\n<img width=\"1790\" alt=\"Screenshot
2024-02-15 at 21 28
46\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/e229b4ff-6ee7-4444-b5c1-deb00d2b9b39\">\r\n\r\n**Rule
Details page AFTER the fix 2 (filters use existing fields and\r\nlook
normal):**\r\n\r\n<img width=\"1792\" alt=\"Screenshot 2024-02-15 at 21
37
45\"\r\nsrc=\"https://github.com/elastic/kibana/assets/7359339/b10905e7-803d-4404-aa02-8692ff964891\">\r\n\r\n\r\n###
Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n-
[ ] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n\r\n###
For maintainers\r\n\r\n- [x] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"26726625394dfc4a5143dd1e1c895487c2f90380"}}]}]
BACKPORT-->

Co-authored-by: Georgii Gorbachev <[email protected]>
banderror added a commit to banderror/kibana that referenced this pull request Feb 20, 2024
…c#177081)

**Fixes: elastic#141458
**Fixes: elastic#176866

## Summary

Fixes the bugs above by changing the `Filters` component:

- from using lower-level components like `FilterBadgeGroup` and custom
rendering
- to using a higher-level `FilterItems` component that's used inside a
larger component `QueryBar` (see the first screenshot below) on the Rule
Creation / Editing pages

Note that for some reason the end result still does not fully equal to
how filters look on the Rule Creation / Editing pages, where there are
fewer warnings. It's hard to say which rendering is the right one.

## Screenshots

**How filters look on the Rule Creation / Editing pages:**

<img width="989" alt="Screenshot 2024-02-15 at 21 25 00"
src="https://github.com/elastic/kibana/assets/7359339/01ca468f-be99-469a-8d75-ee5aa1a31fb0">

**Rule Details page BEFORE the fix:**

<img width="1792" alt="Screenshot 2024-02-15 at 21 23 46"
src="https://github.com/elastic/kibana/assets/7359339/d0e2aa6e-3050-4327-8025-f37125498fd6">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 02"
src="https://github.com/elastic/kibana/assets/7359339/a89302b2-f991-4547-bdac-c0f5a594a881">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 18"
src="https://github.com/elastic/kibana/assets/7359339/49c16b02-8d82-4f93-932f-3846880a0457">

**Rule Details page AFTER the fix 1 (filters use non-existing fields and
show warnings):**

<img width="1790" alt="Screenshot 2024-02-15 at 21 28 46"
src="https://github.com/elastic/kibana/assets/7359339/e229b4ff-6ee7-4444-b5c1-deb00d2b9b39">

**Rule Details page AFTER the fix 2 (filters use existing fields and
look normal):**

<img width="1792" alt="Screenshot 2024-02-15 at 21 37 45"
src="https://github.com/elastic/kibana/assets/7359339/b10905e7-803d-4404-aa02-8692ff964891">

### Checklist

- [ ] [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
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] 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))
- [ ] 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))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 2672662)

# Conflicts:
#	x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_definition_section.tsx
@banderror
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.12

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

Questions ?

Please refer to the Backport tool documentation

@banderror banderror deleted the fix-rule-filters-on-details-page branch February 20, 2024 10:09
banderror added a commit that referenced this pull request Feb 20, 2024
…177081) (#177260)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Security Solution] Fix rule filters on the Rule Details page
(#177081)](#177081)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)
@mistic
Copy link
Member

mistic commented Feb 22, 2024

This PR didn't landed on time to be included on v8.12.2. Updating the labels.

@mistic mistic added v8.12.3 and removed v8.12.2 labels Feb 22, 2024
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…c#177081)

**Fixes: elastic#141458
**Fixes: elastic#176866

## Summary

Fixes the bugs above by changing the `Filters` component:

- from using lower-level components like `FilterBadgeGroup` and custom
rendering
- to using a higher-level `FilterItems` component that's used inside a
larger component `QueryBar` (see the first screenshot below) on the Rule
Creation / Editing pages

Note that for some reason the end result still does not fully equal to
how filters look on the Rule Creation / Editing pages, where there are
fewer warnings. It's hard to say which rendering is the right one.

## Screenshots

**How filters look on the Rule Creation / Editing pages:**

<img width="989" alt="Screenshot 2024-02-15 at 21 25 00"
src="https://github.com/elastic/kibana/assets/7359339/01ca468f-be99-469a-8d75-ee5aa1a31fb0">

**Rule Details page BEFORE the fix:**

<img width="1792" alt="Screenshot 2024-02-15 at 21 23 46"
src="https://github.com/elastic/kibana/assets/7359339/d0e2aa6e-3050-4327-8025-f37125498fd6">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 02"
src="https://github.com/elastic/kibana/assets/7359339/a89302b2-f991-4547-bdac-c0f5a594a881">
<img width="1792" alt="Screenshot 2024-02-15 at 21 24 18"
src="https://github.com/elastic/kibana/assets/7359339/49c16b02-8d82-4f93-932f-3846880a0457">

**Rule Details page AFTER the fix 1 (filters use non-existing fields and
show warnings):**

<img width="1790" alt="Screenshot 2024-02-15 at 21 28 46"
src="https://github.com/elastic/kibana/assets/7359339/e229b4ff-6ee7-4444-b5c1-deb00d2b9b39">

**Rule Details page AFTER the fix 2 (filters use existing fields and
look normal):**

<img width="1792" alt="Screenshot 2024-02-15 at 21 37 45"
src="https://github.com/elastic/kibana/assets/7359339/b10905e7-803d-4404-aa02-8692ff964891">


### Checklist

- [ ] [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
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] 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))
- [ ] 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))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)



### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@shayfeld
Copy link

Hi @banderror ,

In 8.11.1 there is bug with "NOT" filter, I am using "AND" inside the filter.
Can you check that it's fixed?

and filter

Thank you :)

@banderror
Copy link
Contributor Author

@shayfeld That one is tracked in #176866, it's been fixed by this PR and the fix will be released in 8.13.0

@shayfeld
Copy link

@banderror are you sure? it's regular filter not custom label.

@banderror
Copy link
Contributor Author

@shayfeld Yes, the negation ("NOT") now works correctly for both regular filters and filters with custom labels. Here's an example from the latest 8.13.0 prerelease version:

Rule Creation page:

Screenshot 2024-03-18 at 18 34 18

Rule Details page:

Screenshot 2024-03-18 at 18 35 41

There's an issue with that Warnings on the Details page, but that's a different one. I'll probably open a dedicated ticket for it.

@banderror
Copy link
Contributor Author

Follow-up issue: #178908

banderror added a commit that referenced this pull request Mar 27, 2024
**Related to:** #177081

## Summary

This PR builds upon some custom CSS introduced in
#177081 according to @maximpn's
[suggestion](#177081 (comment)).

It slightly adjusts the way the styles are defined based on our prior
discussion at Tech Time, proposing a canonical way to write custom CSS
within the @elastic/security-detection-rule-management team.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 27, 2024
…ic#179282)

**Related to:** elastic#177081

## Summary

This PR builds upon some custom CSS introduced in
elastic#177081 according to @maximpn's
[suggestion](elastic#177081 (comment)).

It slightly adjusts the way the styles are defined based on our prior
discussion at Tech Time, proposing a canonical way to write custom CSS
within the @elastic/security-detection-rule-management team.

(cherry picked from commit fb1eb4c)
kibanamachine added a commit that referenced this pull request Mar 27, 2024
…#179282) (#179531)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[Security Solution] Align on custom CSS within Rule Management
(#179282)](#179282)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Georgii
Gorbachev","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-03-27T12:04:29Z","message":"[Security
Solution] Align on custom CSS within Rule Management
(#179282)\n\n**Related to:**
https://github.com/elastic/kibana/pull/177081\r\n\r\n##
Summary\r\n\r\nThis PR builds upon some custom CSS introduced
in\r\nhttps://github.com//pull/177081 according to
@maximpn's\r\n[suggestion](https://github.com/elastic/kibana/pull/177081#discussion_r1494357297).\r\n\r\nIt
slightly adjusts the way the styles are defined based on our
prior\r\ndiscussion at Tech Time, proposing a canonical way to write
custom CSS\r\nwithin the @elastic/security-detection-rule-management
team.","sha":"fb1eb4ca191e8d199ffe5a06290cdfe05e3e5a79","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["refactoring","release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","v8.14.0","v8.13.1"],"title":"[Security Solution] Align on
custom CSS within Rule
Management","number":179282,"url":"https://github.com/elastic/kibana/pull/179282","mergeCommit":{"message":"[Security
Solution] Align on custom CSS within Rule Management
(#179282)\n\n**Related to:**
https://github.com/elastic/kibana/pull/177081\r\n\r\n##
Summary\r\n\r\nThis PR builds upon some custom CSS introduced
in\r\nhttps://github.com//pull/177081 according to
@maximpn's\r\n[suggestion](https://github.com/elastic/kibana/pull/177081#discussion_r1494357297).\r\n\r\nIt
slightly adjusts the way the styles are defined based on our
prior\r\ndiscussion at Tech Time, proposing a canonical way to write
custom CSS\r\nwithin the @elastic/security-detection-rule-management
team.","sha":"fb1eb4ca191e8d199ffe5a06290cdfe05e3e5a79"}},"sourceBranch":"main","suggestedTargetBranches":["8.13"],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/179282","number":179282,"mergeCommit":{"message":"[Security
Solution] Align on custom CSS within Rule Management
(#179282)\n\n**Related to:**
https://github.com/elastic/kibana/pull/177081\r\n\r\n##
Summary\r\n\r\nThis PR builds upon some custom CSS introduced
in\r\nhttps://github.com//pull/177081 according to
@maximpn's\r\n[suggestion](https://github.com/elastic/kibana/pull/177081#discussion_r1494357297).\r\n\r\nIt
slightly adjusts the way the styles are defined based on our
prior\r\ndiscussion at Tech Time, proposing a canonical way to write
custom CSS\r\nwithin the @elastic/security-detection-rule-management
team.","sha":"fb1eb4ca191e8d199ffe5a06290cdfe05e3e5a79"}},{"branch":"8.13","label":"v8.13.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Georgii Gorbachev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.13 candidate bug Fixes for quality problems that affect the customer experience Feature:Rule Details Security Solution Detection Rule Details page impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. release_note:fix Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.12.3 v8.13.0 v8.14.0
Projects
None yet
7 participants