Skip to content

[Security Solution] add expandable flyout header components#153187

Merged
PhilippeOberti merged 2 commits intomainfrom
expanded-flyout-6117
Mar 27, 2023
Merged

[Security Solution] add expandable flyout header components#153187
PhilippeOberti merged 2 commits intomainfrom
expanded-flyout-6117

Conversation

@PhilippeOberti
Copy link
Copy Markdown
Contributor

@PhilippeOberti PhilippeOberti commented Mar 13, 2023 •
edited
Loading

Summary

This PR leverages the work done in a previous PR and add the content of the header component of expandable flyout right section:

  • display a title (for now if the document is of type alert, display the name of the rule, otherwise the default Document details
  • below the title, display the timestamp found on the document from the @timestamp property
  • display the risk score by leveraging the existin useGetFieldsData hook
  • display the severity by leveraging the existin useGetFieldsData hook

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

Notes:

  • the expandable flyout right section skeleton 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

Screenshot 2023-03-20 at 4 51 48 PM

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

Checklist

Delete any items that are not applicable to this PR.

@PhilippeOberti PhilippeOberti force-pushed the expanded-flyout-6117 branch 4 times, most recently from 638d0c9 to cbf56c2 Compare March 20, 2023 19:49
@PhilippeOberti PhilippeOberti marked this pull request as ready for review March 20, 2023 20:52
@PhilippeOberti PhilippeOberti requested review from a team as code owners March 20, 2023 20:52
@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 20, 2023
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 20, 2023

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

lgestc
lgestc reviewed Mar 21, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@lgestc lgestc left a comment

Choose a reason for hiding this comment

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

casting is okay to me when it comes to storybook, testing etc, but I would go with something better / stricter when it comes to an actual program. I hope you are not offended sir 🗡️

x-pack/plugins/security_solution/public/flyout/right/components/header_title.tsx Outdated
);

if (!dataFormattedForFieldBrowser) {
return <></>;
Copy link
Copy Markdown
Contributor

@lgestc lgestc Mar 21, 2023

Choose a reason for hiding this comment

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

null would work just as well :)

Copy link
Copy Markdown
Contributor Author

@PhilippeOberti PhilippeOberti Mar 21, 2023

Choose a reason for hiding this comment

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

looking at some docs, it seems that null is preferred over React.Fragment, thanks for pointing this out! 👍

x-pack/plugins/security_solution/public/flyout/right/components/risk_score.tsx Outdated
*/
export const RiskScore: FC = memo(() => {
const { getFieldsData } = useRightPanelContext();
const alertRiskScore = getFieldsData(ALERT_RISK_SCORE) as string;
Copy link
Copy Markdown
Contributor

@lgestc lgestc Mar 21, 2023

Choose a reason for hiding this comment

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

I would write some kind of assertion here, I can see that this can return unknown | unknown[] so its not really safe to cast it to string.

Copy link
Copy Markdown
Contributor Author

@PhilippeOberti PhilippeOberti Mar 21, 2023

Choose a reason for hiding this comment

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

yes absolutely, thanks for pushing for this, fixed!

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Mar 22, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3790 3792 +2

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 +10.6KB
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

christineweng
christineweng approved these changes Mar 27, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Desk tested on both alert page and events, LGTM!

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

Reviewers

@lgestc lgestc lgestc left review comments

@christineweng christineweng christineweng approved these changes

@michaelolo24 michaelolo24 Awaiting requested review from michaelolo24

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@PhilippeOberti @elasticmachine @kibana-ci @lgestc @christineweng @kibanamachine