Skip to content

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

Merged
PhilippeOberti merged 2 commits intomainfrom
expanded-flyout-6067
Mar 20, 2023
Merged

[Security Solution] expanded flyout - right section - table tab implementation#152303
PhilippeOberti merged 2 commits intomainfrom
expanded-flyout-6067

Conversation

@PhilippeOberti
Copy link
Copy Markdown
Contributor

@PhilippeOberti PhilippeOberti commented Feb 27, 2023

Summary

This PR leverages the work done in a previous PR and adds the table and json tabs to the Security Solution expandable flyout right section panel:

  • table tab (reusing the EventFieldsBrowsercomponent) with filtering and cell actions (filter in, filter out, add to timeline and copy to clipboard)
  • update the right panel context component to share data

How to test

  • add xpack.securitySolution.enableExperimental: ['securityFlyoutEnabled'] to the kibana.json file
  • run yarn es snapshot --license trial, yarn test:generate and yarn start --no-base-path
  • go to the Alerts page, and click on the expand detail button on any row of the table
  • navigate to the Table tab

Run tests and storybook

  • node scripts/storybook security_solution to run Storybook
  • npm run test:jest --config ./x-pack/plugins/security_solution/public/flyout to run the unit tests
  • yarn cypress:open-as-ci but note that the 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
Screen.Recording.2023-03-08.at.10.24.35.AM.mov

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

Checklist

Delete any items that are not applicable to this PR.

@PhilippeOberti PhilippeOberti changed the title [Security Solution] expanded flyout - right section - table tab implementation [Security Solution] expanded flyout - right section - table and json tab implementation Mar 2, 2023
@PhilippeOberti PhilippeOberti changed the title [Security Solution] expanded flyout - right section - table and json tab implementation [Security Solution] expanded flyout - right section - table and json tabs implementation Mar 2, 2023
@PhilippeOberti PhilippeOberti marked this pull request as ready for review March 6, 2023 20:06
@PhilippeOberti PhilippeOberti requested review from a team as code owners March 6, 2023 20:06
@PhilippeOberti PhilippeOberti marked this pull request as draft March 7, 2023 20:58
@PhilippeOberti PhilippeOberti changed the title [Security Solution] expanded flyout - right section - table and json tabs implementation [Security Solution] expanded flyout - right section - table tab implementation Mar 8, 2023
@PhilippeOberti PhilippeOberti force-pushed the expanded-flyout-6067 branch 3 times, most recently from 25355bc to 685eecd Compare March 10, 2023 00:15
@PhilippeOberti PhilippeOberti marked this pull request as ready for review March 10, 2023 01:58
@PhilippeOberti PhilippeOberti requested review from a team as code owners March 10, 2023 01:58
@PhilippeOberti 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 v8.8.0 labels Mar 10, 2023
@elasticmachine
Copy link
Copy Markdown
Contributor

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

});

describe('<TableTab />', () => {
it('should render table component', () => {
Copy link
Copy Markdown
Contributor

@michaelolo24 michaelolo24 Mar 20, 2023

Choose a reason for hiding this comment

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

Is it worth testing that an actual field shows up in the table as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I felt that it wasn't needed here as I'm using the existing EventFieldsBrowser component which already has some unit tests.

browserFields={browserFields}
data={dataFormattedForFieldBrowser}
eventId={eventId}
isDraggable={false}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to determine what we do with this, as we'll have to pass some value for this for the flyout in the timeline context, though we'll probably want to take some time and look at the new timeline designs to figure out how we may want to treat this as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah that is something I need to be more aware of, at the moment I'm focusing only to the flyout from the alerts page, as I'm trying to get the complexity as low as possible...


/**
* Table view displayed in the document details expandable flyout right section
* Table view displayed in the alert details expandable flyout right section
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you were fine with document here as it would show for a regular event as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i didn't want to make this change, thanks for catching this!

@michaelolo24
Copy link
Copy Markdown
Contributor

Works well! I think it's worth discussing with design about using hover actions instead of the actions row in the table. Horizontal space is going to be really important for us here.

Copy link
Copy Markdown
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Looks good, just some nits, and some conversation about switching to hover actions

@PhilippeOberti
Copy link
Copy Markdown
Contributor Author

PhilippeOberti commented Mar 20, 2023

Works well! I think it's worth discussing with design about using hover actions instead of the actions row in the table. Horizontal space is going to be really important for us here.

that is a really good point. I honestly didn't even think about it as I was reusing the existing EventFieldsBrowser component but I'll check that with @ferenrigue tommorrow!

@PhilippeOberti PhilippeOberti force-pushed the expanded-flyout-6067 branch 2 times, most recently from 2a8add4 to d682cf2 Compare March 20, 2023 18:28
@kibana-ci
Copy link
Copy Markdown

💚 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 15.8MB 15.8MB +2.2KB
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 513 516 +3

History

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

@PhilippeOberti PhilippeOberti merged commit 8b9fb2b into main Mar 20, 2023
@PhilippeOberti PhilippeOberti deleted the expanded-flyout-6067 branch March 20, 2023 21:36
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:feature Makes this part of the condensed release notes Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.8.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants