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

[SecuritySolution] Add "exist filter" when value count is filtered in Lens cell action #181151

Merged
merged 11 commits into from
Apr 24, 2024

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Apr 18, 2024

Summary

Original issue and steps to reproduce: #181120

Before:

Screen.Recording.2024-04-17.at.2.13.45.PM.mov

After:

Screen.Recording.2024-04-18.at.14.15.34.mov

Checklist

Delete any items that are not applicable to this PR.

@angorayc angorayc added bug Fixes for quality problems that affect the customer experience v8.14.0 labels Apr 18, 2024
@angorayc angorayc marked this pull request as ready for review April 18, 2024 17:41
@angorayc angorayc requested review from a team as code owners April 18, 2024 17:41
Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

@angorayc thanks for fixing this and adding all the unit tests!

I was more expecting the hover actions to just not be displayed at all. Approving because this approach works as well!

@angorayc
Copy link
Contributor Author

angorayc commented Apr 22, 2024

@angorayc thanks for fixing this and adding all the unit tests!

I was more expecting the hover actions to just not be displayed at all. Approving because this approach works as well!

@PhilippeOberti You are right, I checked the existing behaviour when filter on a count value in Lens, turned out it shouldn't have cell actions. Let me rework on this.

Screen.Recording.2024-04-22.at.10.11.40.mov

@angorayc angorayc closed this Apr 22, 2024
@angorayc
Copy link
Contributor Author

angorayc commented Apr 23, 2024

When I was checking what's the correct behaviour, I found two different behaviours in Lens while filtering on a count value: Sometimes it has cell actions, sometime it doesn't.

Screen.Recording.2024-04-22.at.10.54.41.mov
Screen.Recording.2024-04-22.at.10.55.44.mov

After checking with the Kibana-visualization team, they confirmed that it's a bug. The count value should still have cell actions, so I think we are fine to add the exists filter when filtering on count value as the implementation in this PR.

However, it does not show cell actions when empty value in the Lens table, which is different from our existing logic at the moment, I think this is a behaviour we could consider aligning. I'm going to create another issue for this.

Security Solution still have cell actions available when value is empty:

Screen.Recording.2024-04-23.at.10.37.53.mov

Lens table does not have cell actions available when value is empty:

#181344

@angorayc angorayc reopened this Apr 23, 2024
@angorayc angorayc added the release_note:skip Skip the PR/issue when compiling release notes label Apr 23, 2024
@angorayc angorayc changed the title [SecuritySolution] Add exist filter when value count is filtered [SecuritySolution] Add "exist filter" when value count is filtered in Lens cell action Apr 23, 2024
Comment on lines 11 to 12
import { isEmptyFilterValue } from './create_filter';
import { addFilter } from './add_filter';
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: It probably makes more sense now to move the isEmptyFilterValue function to the new add_filter.ts module as well. So actions will only have to import from add_filter module, and the create_filter module will become an internal module to help create the filters of add_filter module.

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

Tested locally, everything is working as expected. Thanks @angorayc
LGTM!

@angorayc angorayc enabled auto-merge (squash) April 24, 2024 08:12
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #37 / discover/ccs_compatible discover search CCS timeout bfetch enabled timeout on single shard shows warning and results with bfetch enabled

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cloudSecurityPosture 433 434 +1
discover 790 791 +1
lens 1403 1404 +1
logsExplorer 728 729 +1
securitySolution 5452 5453 +1
slo 724 725 +1
total +6

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/cell-actions 44 46 +2

Async chunks

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

id before after diff
cloudSecurityPosture 451.2KB 451.3KB +94.0B
discover 638.2KB 638.3KB +92.0B
securitySolution 14.6MB 14.7MB +1.1KB
slo 722.8KB 722.9KB +94.0B
total +1.4KB
Unknown metric groups

API count

id before after diff
@kbn/cell-actions 62 64 +2

History

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

cc @angorayc

@angorayc angorayc merged commit 42e6f0c into elastic:main Apr 24, 2024
35 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 24, 2024
… Lens cell action (elastic#181151)

## Summary

Original issue and steps to reproduce:
elastic#181120

Before:

https://github.com/elastic/kibana/assets/6295984/10c7a2ce-d814-4750-8481-8f05b55384f8

After:

https://github.com/elastic/kibana/assets/6295984/00dfbcb6-244b-4f2b-8dd4-a1f7435385cf

### 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

---------

Co-authored-by: Sergi Massaneda <[email protected]>
(cherry picked from commit 42e6f0c)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.14

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 24, 2024
…nt is filtered in Lens cell action (#181151) (#181534)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[SecuritySolution] Add &quot;exist filter&quot; when value count is
filtered in Lens cell action
(#181151)](#181151)

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

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

<!--BACKPORT [{"author":{"name":"Angela
Chuang","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-04-24T09:41:21Z","message":"[SecuritySolution]
Add \"exist filter\" when value count is filtered in Lens cell action
(#181151)\n\n## Summary\r\n\r\nOriginal issue and steps to
reproduce:\r\nhttps://github.com//issues/181120\r\n\r\n\r\nBefore:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/10c7a2ce-d814-4750-8481-8f05b55384f8\r\n\r\n\r\n\r\n\r\nAfter:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/00dfbcb6-244b-4f2b-8dd4-a1f7435385cf\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n\r\n- [x] [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\r\n---------\r\n\r\nCo-authored-by: Sergi Massaneda
<[email protected]>","sha":"42e6f0cd73f9d80944136925f844757a0d775dc2","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v8.14.0","v8.15.0"],"title":"[SecuritySolution]
Add \"exist filter\" when value count is filtered in Lens cell
action","number":181151,"url":"https://github.com/elastic/kibana/pull/181151","mergeCommit":{"message":"[SecuritySolution]
Add \"exist filter\" when value count is filtered in Lens cell action
(#181151)\n\n## Summary\r\n\r\nOriginal issue and steps to
reproduce:\r\nhttps://github.com//issues/181120\r\n\r\n\r\nBefore:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/10c7a2ce-d814-4750-8481-8f05b55384f8\r\n\r\n\r\n\r\n\r\nAfter:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/00dfbcb6-244b-4f2b-8dd4-a1f7435385cf\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n\r\n- [x] [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\r\n---------\r\n\r\nCo-authored-by: Sergi Massaneda
<[email protected]>","sha":"42e6f0cd73f9d80944136925f844757a0d775dc2"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181151","number":181151,"mergeCommit":{"message":"[SecuritySolution]
Add \"exist filter\" when value count is filtered in Lens cell action
(#181151)\n\n## Summary\r\n\r\nOriginal issue and steps to
reproduce:\r\nhttps://github.com//issues/181120\r\n\r\n\r\nBefore:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/10c7a2ce-d814-4750-8481-8f05b55384f8\r\n\r\n\r\n\r\n\r\nAfter:\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/6295984/00dfbcb6-244b-4f2b-8dd4-a1f7435385cf\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n\r\n- [x] [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\r\n---------\r\n\r\nCo-authored-by: Sergi Massaneda
<[email protected]>","sha":"42e6f0cd73f9d80944136925f844757a0d775dc2"}}]}]
BACKPORT-->

Co-authored-by: Angela Chuang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants