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][Notes] - update cases alerts table action column width to show analyzer and session view icons #195693

Merged

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Oct 10, 2024

Summary

This PR makes a small change to the width of the actions column for the alerts table displayed in the Alerts tab of Cases. An issue was introduced when this EUI PR was merged a few weeks back. It removed some css that was hiding some icons with a overflow: hidden

Before EUI merge After EUI merge
Screenshot 2024-10-10 at 11 44 31 AM Screenshot 2024-10-10 at 11 48 08 AM

One fix could have to just hide the session view and analyzer graph icons on the Cases alerts table, but with this upcoming change (PR) we can now display those icons on the cases alerts table. We were previously hiding the icons because we did not have the ability to visualize session view and analyzer graph within the table, but we now that we have the ability to render these two in the flyout we can show the icons and have a consistent UI between all alerts tables.
Note that the visualization in flyout is controlled via an advanced settings (securitySolution:enableVisualizationsInFlyout).

Here are the scenario we want to support and that should be tested when reviewing this PR:

  • alert page
    • session view and analyzer graph icon should always be visible (granted the data for the alerts allows it) wether the aforementioned advanced settings in enabled or disabled
  • cases page
    • hide the analyzer graph and session view icons if the aforementioned advanced settings is disabled
    • show the analyzer graph and session view icons if the aforementioned advanced settings is enabled

Also take into account when testing that the securitySolutionNotesEnabled feature flag can be toggled on or off, which has an impact on how many icons we show in the actions columns.

Current behavior

Screenshot 2024-10-09 at 6 34 37 PM

New behavior when visualization in flyout advanced settings are enabled

Screenshot 2024-10-09 at 7 01 19 PM

New behavior when visualization in flyout advanced settings are disabled
Screenshot 2024-10-09 at 7 12 57 PM

@PhilippeOberti PhilippeOberti added backport release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 labels Oct 10, 2024
@PhilippeOberti PhilippeOberti force-pushed the cases-alerts-table-action-column branch from 0607465 to a35e527 Compare October 10, 2024 17:43
@PhilippeOberti PhilippeOberti marked this pull request as ready for review October 10, 2024 17:51
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner October 10, 2024 17:51
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

);
if (!visualizationInFlyoutEnabled && tableId === TableId.alertsOnCasePage) {
ACTION_BUTTON_COUNT -= 2;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@PhilippeOberti When license in not enterprise and advanced setting is off, the 3 dots still overlaps with the next column. Other scenarios LGTM!
image

Copy link
Contributor

Choose a reason for hiding this comment

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

i see now, the session view count was done on line 33, so if it is not enterprise, line 50 should be -=1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haaaaa good point. Let me fix that! Too many scenarios, I'm glad you're testing it in depth like that 😆

Copy link
Contributor Author

@PhilippeOberti PhilippeOberti Oct 11, 2024

Choose a reason for hiding this comment

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

I reworked a bit that code and added comments to make it more clear cb46c19

@PhilippeOberti PhilippeOberti force-pushed the cases-alerts-table-action-column branch from b4f1d3d to cb46c19 Compare October 11, 2024 16:24
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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 20.7MB 20.7MB +1.0KB

History

Copy link
Contributor

@christineweng christineweng left a comment

Choose a reason for hiding this comment

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

Thanks for adding comments to explain the logic. LGTM 🚀🚀

@PhilippeOberti PhilippeOberti merged commit ab248df into elastic:main Oct 11, 2024
42 checks passed
@PhilippeOberti PhilippeOberti deleted the cases-alerts-table-action-column branch October 11, 2024 19:05
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11297932434

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

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 Oct 11, 2024
…olumn width to show analyzer and session view icons (#195693) (#195981)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Notes] - update cases alerts table action column
width to show analyzer and session view icons
(#195693)](#195693)

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

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

<!--BACKPORT [{"author":{"name":"Philippe
Oberti","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-11T19:05:43Z","message":"[Security
Solution][Notes] - update cases alerts table action column width to show
analyzer and session view icons
(#195693)","sha":"ab248df4f8752c066f99acd2150578eab370211d","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","release_note:skip","v9.0.0","Team:Threat
Hunting:Investigations","v8.16.0"],"title":"[Security Solution][Notes] -
update cases alerts table action column width to show analyzer and
session view
icons","number":195693,"url":"https://github.com/elastic/kibana/pull/195693","mergeCommit":{"message":"[Security
Solution][Notes] - update cases alerts table action column width to show
analyzer and session view icons
(#195693)","sha":"ab248df4f8752c066f99acd2150578eab370211d"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195693","number":195693,"mergeCommit":{"message":"[Security
Solution][Notes] - update cases alerts table action column width to show
analyzer and session view icons
(#195693)","sha":"ab248df4f8752c066f99acd2150578eab370211d"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Philippe Oberti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants