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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Mar 20, 2023
Merged

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

merged 1 commit into from
Mar 20, 2023

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Mar 8, 2023 •
edited
Loading

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:

  • json tab (reusing the JsonView component)
  • 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 Json 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.34.20.AM.mov

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

Checklist

Delete any items that are not applicable to this PR.

@PhilippeOberti PhilippeOberti force-pushed the expanded-flyout-6066 branch 3 times, most recently from 9ff22ff to 8d45003 Compare March 9, 2023 23:51
@PhilippeOberti PhilippeOberti marked this pull request as ready for review March 10, 2023 01:30
@PhilippeOberti PhilippeOberti requested review from a team as code owners March 10, 2023 01:30
@PhilippeOberti PhilippeOberti added Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team v8.8.0 labels Mar 10, 2023
@elasticmachine
Copy link
Contributor

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

@PhilippeOberti PhilippeOberti added the release_note:feature Makes this part of the condensed release notes label Mar 10, 2023
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #4 / Row renderers Selected renderer can be disabled and enabled
  • [job] [logs] FTR Configs #8 / security APIs - Session Idle Session Idle cleanup should properly clean up session expired because of idle timeout when providers override global session config

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3778 3779 +1

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.8MB +2.7KB
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

michaelolo24
michaelolo24 reviewed Mar 20, 2023
View reviewed changes
x-pack/plugins/security_solution/cypress/tasks/document_expandable_flyout.ts
* // TODO revisit this as it seems very fragile: the first element found is the timeline flyout, which isn't visible but still exist in the DOM
*/
export const scrollWithinDocumentDetailsExpandableFlyoutRightSection = (x: number, y: number) =>
cy.get(getClassSelector('euiFlyout')).last().scrollTo(x, y);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏾

michaelolo24
michaelolo24 reviewed Mar 20, 2023
View reviewed changes
x-pack/plugins/security_solution/public/flyout/right/tabs/json_tab.test.tsx
</RightPanelContext.Provider>
);

expect(getByTestId(JSON_TAB_CONTENT_TEST_ID)).toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth it to make sure some_value shows up in the component as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same reasoning I was having with the table, as I'm reusing the existing JsonView component which internally is a simple EuiCodeBlock and is also already tested, I was thinking it wouldn't be necessary...

michaelolo24
michaelolo24 approved these changes Mar 20, 2023
View reviewed changes
Copy link
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!,Thanks! LGTM :)

@PhilippeOberti PhilippeOberti merged commit b23e57b into main Mar 20, 2023
@PhilippeOberti PhilippeOberti deleted the expanded-flyout-6066 branch March 20, 2023 19:29
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 20, 2023
v1v added a commit to v1v/kibana that referenced this pull request Mar 20, 2023
@v1v
Merge remote-tracking branch 'upstream/main' into feature/support-dep…
ddddab0 
…loy-my-kibana-oblt

* upstream/main: (727 commits)
  Upgrade caniuse-lite db (elastic#153318)
  [Security Solution] expanded flyout - right section - json tab implementation (elastic#152935)
  chore(slo): Make APM indicator's index required (elastic#153311)
  skip failing test suite (elastic#136688)
  [Security Solution] Fix security-solution storybook package codeowners (elastic#153307)
  [EUI] Add `scrollLock` workaround CSS to Kibana's `body` (elastic#153227)
  [Cloud Security] Show coming soon deployments of vulnerability management (elastic#153249)
  [Cloud Security] fixed onboarding link directs to cspm integration (elastic#153268)
  [Response Ops][Alerting] Reusable functions for FAAD resource installation (elastic#152849)
  remove geohash_grid aggregation support (elastic#152952)
  [Tech Debt] Reorder Rules page (elastic#152897)
  [Saved Object Finder] Add help text & left button (elastic#152742)
  [Transform] Replace SavedObjectsFinder component (elastic#153128)
  Make pipeline creation endpoint accept a full pipeline definition (elastic#153133)
  [Fleet] Displaying policy changes in Agent activity (elastic#153237)
  skip flaky suite (elastic#152852)
  [Security Solution][Endpoint] Add tests to cover RBAC entries in the Role Kibana Privileges flyout (elastic#153068)
  [Security Solution][Endpoint] Additional tests for Response Console History Log page (covers TestRail manual tests) (elastic#153042)
  [Monitoring] Display node roles in Nodes table (elastic#152127)
  Rename getEditAlertFlyout to getEditRuleFlyout (elastic#153243)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelolo24 michaelolo24 michaelolo24 approved these changes

@christineweng christineweng Awaiting requested review from christineweng

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:feature Makes this part of the condensed release notes Team:Threat Hunting:Investigations Security Solution 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.
5 participants
@PhilippeOberti @elasticmachine @kibana-ci @michaelolo24 @kibanamachine