[Security Solution] expanded flyout - right section skeleton by PhilippeOberti · Pull Request #152047 · elastic/kibana

PhilippeOberti · 2023-02-23T22:32:50Z

Summary

This PR leverages the expandable flyout work done in a previous PR and adds the alert details right section skeleton:

static header
3 tabs with placeholder components: Overview, Table and Json

How to test

add xpack.securitySolution.enableExperimental: ['securityFlyoutEnabled'] to the kibana.json file
go to the Alerts page, and click on the expand detail button on any row of the table

Notes:

the expandable flyout work needs to be merged first.
integration/e2e tests have been written but are now skipped because the feature is protected behind a feature flag, disabled by default. To check them, add 'securityFlyoutEnabled' here

https://github.com/elastic/security-team/issues/6065

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
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)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)

elasticmachine · 2023-02-23T22:33:51Z

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

michaelolo24 · 2023-02-28T16:09:12Z

x-pack/plugins/security_solution/cypress/tasks/alert_details_expandable_flyout.ts

+/**
+ * Open the Overview tab in the alert details expandable flyout right section
+ */
+export const openOverviewTab = () => cy.get(ALERT_DETAILS_FLYOUT_OVERVIEW_TAB).click();


should we add any should('be.visible') to account for potential delays in the flyout opening in the CI environment? May not be necessary, but a flakiness issue I've seen with some of the popovers and flyouts

michaelolo24 · 2023-02-28T16:10:01Z

x-pack/plugins/security_solution/public/common/components/control_columns/row_action/index.tsx

    if (isSecurityFlyoutEnabled) {
-      openFlyout({});
+      openFlyout({
+        right: {


Woohoo, a real panel! 😄

michaelolo24 · 2023-02-28T16:12:10Z

x-pack/plugins/security_solution/public/flyout/alerts/panels/right/components/header_title.tsx

+/**
+ * Alert details flyout right section header
+ */
+export const HeaderTitle: FC = memo(() => {


One thing to note is that this flyout should probably be thought of as configurable as possible for both alerts and just simple events. Whether that means having separate components based on one vs the other, or a generalized unified component (i.e. a unified header) that works for both.

you're totally correct, I actually have a separate ticket for the header itself, as I feel there be a need of some more discussion with Product and UI to handle different scenarios

michaelolo24 · 2023-02-28T16:13:15Z

x-pack/plugins/security_solution/public/flyout/alerts/panels/right/content.tsx

+  return (
+    <EuiFlyoutBody
+      css={css`
+        height: calc(100vh - 262px);


I think I did the same thing with the explicit 262, but I think we should see if we can get the topbar heights from some shared constant if possible in case that changes

you're right again, I stole this from you without really thinking about why we're using it. I feel like this shouldn't be part of the PR, and I'll add it back in a cleaner way once I implement the content of the tabs!

michaelolo24 · 2023-02-28T16:13:59Z

x-pack/plugins/security_solution/public/flyout/alerts/panels/right/context.tsx

+  const contextValue = useContext(RightPanelContext);
+
+  if (!contextValue) {
+    throw new Error('RightPanelContext can only be used within RightPanelContext provider');


👍🏾 for the developer experience 😄

michaelolo24 · 2023-02-28T16:16:42Z

x-pack/plugins/security_solution/public/flyout/alerts/panels/right/tabs/test_ids.ts

+ * 2.0.
+ */
+
+export const OVERVIEW_TAB_CONTENT_TEST_ID = 'securitySolutionAlertDetailsFlyoutOverviewTabContent';


I like this pattern and ultra separation of concerns. I know people tend to keep items like this in constants.ts files. I'm not sure which is better, but just wanted to point it out

yeah I've seen many translations.ts files but not as many test_ids.ts files. What I like about having them separated, is when we declare variable in Cypress (within the screens folder) we can import those from the test_ids files. It avoids making typo mistakes

- header with simple static title - 3 empty tabs (overview, table and json)

PhilippeOberti · 2023-02-28T22:02:42Z

@michaelolo24 I've decided to change slightly the folder structure under the flyout folder, and added a README to explain a bit the current structure and how we would do in the future if other panels were to be created. I've assumed that for now, we'd have a single expanded flyout within the Security Solution plugin, that would handle all types of documents (signals, events, indicators, assets and findings)
Let me know if you think that isn't appropriate and I can easily revert.

kibana-ci · 2023-03-01T01:16:17Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 0c6df06

Failed CI Steps

Security Solution Tests #2

Test Failures

[job] [logs] Security Solution Tests #2 / Detection rules, bulk edit of rule actions Restricted action privileges User with no privileges can't add rule actions

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3723	3737	+14

Async chunks

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

id	before	after	diff
`securitySolution`	15.7MB	15.7MB	+14.4KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	428	430	+2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	506	508	+2

History

💚 Build #110938 succeeded 5f0302a
💔 Build #110454 failed d8e8bad
💚 Build #110411 succeeded a8d11557ab77cd1c08ff994d78ead354ee945b48
💔 Build #110382 failed 6ba530ee5def80bef5d78520b35aca3996895371
💚 Build #110172 succeeded ae8f562
💔 Build #110169 failed 2b9f3c04bebc912bebe4eccd407f499c42f3c119

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

michaelolo24 · 2023-03-07T14:18:24Z

x-pack/plugins/security_solution/public/flyout/right/content.tsx

+ */
+
+import { EuiFlyoutBody } from '@elastic/eui';
+import type { VFC } from 'react';


We'll have to update this whenever we move to React 18 since it's deprecated in the most recent version

michaelolo24

Nice work! LGTM!

…#152047) - header with simple static title - 3 empty tabs (overview, table and json)

PhilippeOberti requested review from a team as code owners February 23, 2023 22:32

PhilippeOberti added Team:Threat Hunting Security Solution Threat Hunting Team release_note:feature Makes this part of the condensed release notes Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team labels Feb 23, 2023

PhilippeOberti added the v8.8.0 label Feb 23, 2023

PhilippeOberti force-pushed the expanded-flyout-6065 branch 5 times, most recently from a8d1155 to d8e8bad Compare February 24, 2023 21:28

PhilippeOberti mentioned this pull request Feb 24, 2023

[Security Solution] expanded flyout - left section skeleton #152150

Merged

4 tasks

PhilippeOberti force-pushed the expanded-flyout-6065 branch from d8e8bad to 5f0302a Compare February 28, 2023 03:38

michaelolo24 reviewed Feb 28, 2023

View reviewed changes

[Security Solution] expandable flyout right section skeleton

145f7ed

- header with simple static title - 3 empty tabs (overview, table and json)

PhilippeOberti force-pushed the expanded-flyout-6065 branch from 5f0302a to 2ce05a5 Compare February 28, 2023 22:00

[Security Solution] PR comments and update folder structure

0c6df06

PhilippeOberti force-pushed the expanded-flyout-6065 branch from 2ce05a5 to 0c6df06 Compare February 28, 2023 22:14

This was referenced Mar 2, 2023

[Security Solution] expanded flyout - right section - table tab implementation #152303

Merged

[Security Solution] expanded flyout - right section - overview tab - mitre attack #152767

Merged

michaelolo24 reviewed Mar 7, 2023

View reviewed changes

michaelolo24 approved these changes Mar 7, 2023

View reviewed changes

PhilippeOberti merged commit ffc8080 into main Mar 7, 2023

PhilippeOberti deleted the expanded-flyout-6065 branch March 7, 2023 14:45

kibanamachine added the backport:skip This PR does not require backporting label Mar 7, 2023

PhilippeOberti mentioned this pull request Mar 8, 2023

[Security Solution] expanded flyout - right section - json tab implementation #152935

Merged

3 tasks

PhilippeOberti mentioned this pull request Mar 10, 2023

[Security Solution] add highlighted fields to overview tab #153074

Merged

4 tasks

bmorelli25 pushed a commit to bmorelli25/kibana that referenced this pull request Mar 10, 2023

[Security Solution] expanded flyout - right section skeleton (elastic…

0f1c4e0

…#152047) - header with simple static title - 3 empty tabs (overview, table and json)

PhilippeOberti mentioned this pull request Mar 13, 2023

[Security Solution] add expandable flyout header components #153187

Merged

4 tasks

Conversation

PhilippeOberti commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test

Notes:

Checklist

Uh oh!

elasticmachine commented Feb 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilippeOberti commented Feb 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibana-ci commented Mar 1, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelolo24 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PhilippeOberti commented Feb 23, 2023 •

edited

Loading

PhilippeOberti commented Feb 28, 2023 •

edited

Loading