[Cloud Security] Refactor Contextual Flyout by animehart · Pull Request #200291 · elastic/kibana

animehart · 2024-11-15T02:14:19Z

Summary

This PR is for reducing code duplication by Encapsulating Hooks, Functions, constants that are used multiple times in a same manner accross multiple files

animehart · 2024-11-15T06:38:01Z

/ci

animehart · 2024-11-15T07:05:13Z

/ci

…ana into refactor-contextual-flyout

animehart · 2024-11-17T18:51:58Z

/ci

animehart · 2024-11-17T22:53:49Z

/ci

elasticmachine · 2024-11-18T02:26:38Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

tiansivive

LGTM from Entity Analytics

albertoblaz

Very nice work encapsulating a lot of repititions into these hooks. I'm still not familiar with the code and how these hooks work, but I added some comments. Feel free to disregard if they don't make sense

albertoblaz · 2024-11-18T12:55:42Z

x-pack/plugins/security_solution/public/flyout/entity_details/host_right/index.tsx

-    signalIndexName,
-    queryId: `${DETECTION_RESPONSE_ALERTS_BY_STATUS_ID}HOST_NAME_RIGHT`,
+  const { hasNonClosedAlerts } = useNonClosedAlerts({
+    fieldName: 'host.name',


I'd put host.name in a constant because this literal is passed down to all these hooks

albertoblaz · 2024-11-18T12:57:02Z

x-pack/plugins/security_solution/public/flyout/entity_details/host_right/index.tsx

+                (isRiskScoreExist ||
+                  hasMisconfigurationFindings ||
+                  hasVulnerabilitiesFindings ||
+                  hasNonClosedAlerts)


Why are you adding hasNonClosedAlerts here? Is it fixing a bug? We should try not to mix changes of functionality in refactor PRs

Hi !, yes this is to fix a small bug that I found when refactoring the code

I have created a separate PR for this small fix as this PR is too big and risky to be included before FF, but this fix should be small enough and safe

Awesome! I just approved that PR. I'd ship it separately and remove hasNonClosedAlerts from here. That way, we'll have a clean refactor-only change-set on this PR

albertoblaz · 2024-11-18T12:57:58Z

x-pack/plugins/security_solution/public/cloud_security_posture/components/entity_insight.tsx

-  const { data } = useVulnerabilitiesPreview({
-    query: buildEntityFlyoutPreviewQuery(fieldName, name),
-    sort: [],
-    enabled: true,
-    pageSize: 1,
-  });


it seems we didn't use ignore_unavailable here, but we'll do so now with the hook. How will that affect the behavior?

useVulnerabilitiesPreview doesnt use the ignore_unavailable field so it should have no effect

Sorry, wrong line. I meant the other useMisconfigurationsPreview.

I saw you removed ignore_unavailable from the hook, so 👍

albertoblaz · 2024-11-18T12:58:18Z

...olution/public/cloud_security_posture/components/vulnerabilities/vulnerabilities_preview.tsx

-  const { data: dataMisconfiguration } = useMisconfigurationPreview({
-    query: buildEntityFlyoutPreviewQuery('host.name', name),
-    sort: [],
-    enabled: true,
-    pageSize: 1,
-  });


it seems we didn't use ignore_unavailable here, but we'll do so now with the hook. How will that affect the behavior?

good catch, I left this one when debugging an issue, will remove it

albertoblaz · 2024-11-18T13:00:41Z

You might want to add a backport:versions tag if you're specifically targeting v8.17 and v9. Or isn't it necessary?

oas_docs/output/kibana.serverless.yaml

x-pack/packages/kbn-cloud-security-posture/public/src/hooks/use_has_misconfigurations.ts

x-pack/plugins/security_solution/public/cloud_security_posture/hooks/use_entity_insight.ts

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

x-pack/plugins/security_solution/public/cloud_security_posture/hooks/use_entity_insight.ts

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

x-pack/plugins/security_solution/public/cloud_security_posture/components/entity_insight.tsx

...olution/public/cloud_security_posture/components/vulnerabilities/vulnerabilities_preview.tsx

…ana into refactor-contextual-flyout

maxcold · 2024-11-19T10:59:31Z

@albertoblaz @animehart I suggest we wait till 8.17 is branched out to have this refactoring in 8.18 to be on the safe side. Not sure if merging it in 8.17 on the day of the FF is a good idea

maxcold

blocking until the 8.17 is branched out, just to be in the safe side

maxcold · 2024-11-21T09:14:48Z

Unblocking as 8.17 has been branched out and 8.x now pointing to 8.18

elasticmachine · 2024-11-21T11:16:22Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 7109558

Failed CI Steps

Jest Tests #3

Test Failures

[job] [logs] Jest Tests #3 / PackagePolicyActionsMenu Should disable upgrade button if package does not have upgrade

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	6218	6223	+5

Async chunks

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

id	before	after	diff
`securitySolution`	13.4MB	13.4MB	-1.5KB

History

💚 Build #252212 succeeded b37f6cd
💚 Build #252056 succeeded 588e010
💔 Build #251743 failed 4cdc3e1
💔 Build #251657 failed 9f5c3aa
💔 Build #251655 failed 6477b5a

kibanamachine · 2024-11-21T20:33:50Z

Starting backport for target branches: 8.x

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

## Summary This PR is for reducing code duplication by Encapsulating Hooks, Functions, constants that are used multiple times in a same manner accross multiple files --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Maxim Kholod <maxim.kholod@elastic.co> (cherry picked from commit c842db5)

kibanamachine · 2024-11-21T20:38:50Z

💚 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

# Backport This will backport the following commits from `main` to `8.x`: - [[Cloud Security] Refactor Contextual Flyout (#200291)](#200291)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Rickyanto Ang <rickyangwyn@gmail.com>

## Summary This PR is for reducing code duplication by Encapsulating Hooks, Functions, constants that are used multiple times in a same manner accross multiple files --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Maxim Kholod <maxim.kholod@elastic.co>

refactor part 1

6477b5a

[CI] Auto-commit changed files from 'make api-docs'

9f5c3aa

animehart added 3 commits November 17, 2024 10:14

refactor part 2, encapsulated risk hook and open panel hook

4393437

Merge branch 'refactor-contextual-flyout' of github.com:animehart/kib…

fc6dac8

…ana into refactor-contextual-flyout

fixed some constant name

4cdc3e1

animehart marked this pull request as ready for review November 18, 2024 02:26

animehart requested review from a team as code owners November 18, 2024 02:26

animehart requested a review from tiansivive November 18, 2024 02:26

animehart added the Team:Cloud Security Cloud Security team related label Nov 18, 2024

animehart added release_note:skip Skip the PR/issue when compiling release notes v8.17.0 v9.0.0 backport:prev-minor labels Nov 18, 2024

animehart mentioned this pull request Nov 18, 2024

[Cloud Security] Alerts Datagrids for Contextual Flyout #199573

Merged

animehart requested a review from maxcold November 18, 2024 02:28

tiansivive approved these changes Nov 18, 2024

View reviewed changes

albertoblaz reviewed Nov 18, 2024

View reviewed changes

maxcold reviewed Nov 18, 2024

View reviewed changes

animehart added 5 commits November 18, 2024 08:20

Merge branch 'main' into refactor-contextual-flyout

588e010

pr comments pt1

bac64c7

pr comments pt2

3016775

Merge branch 'refactor-contextual-flyout' of github.com:animehart/kib…

bc57213

…ana into refactor-contextual-flyout

pr comments pt 3

b6148a9

pr comments pt4, change fieldName to field, name to value

b37f6cd

albertoblaz added the backport:version Backport to applied version labels label Nov 19, 2024

maxcold removed backport:version Backport to applied version labels v8.17.0 labels Nov 19, 2024

maxcold self-requested a review November 19, 2024 10:59

maxcold requested changes Nov 19, 2024

View reviewed changes

maxcold approved these changes Nov 21, 2024

View reviewed changes

Merge branch 'main' into refactor-contextual-flyout

7109558

animehart added the v8.18.0 label Nov 21, 2024

animehart merged commit c842db5 into elastic:main Nov 21, 2024

kibanamachine mentioned this pull request Nov 21, 2024

[8.x] [Cloud Security] Refactor Contextual Flyout (#200291) #201287

Merged

Conversation

animehart commented Nov 15, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

animehart commented Nov 15, 2024

Uh oh!

animehart commented Nov 15, 2024

Uh oh!

animehart commented Nov 17, 2024

Uh oh!

animehart commented Nov 17, 2024

Uh oh!

elasticmachine commented Nov 18, 2024

Uh oh!

tiansivive left a comment

Choose a reason for hiding this comment

Uh oh!

albertoblaz left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albertoblaz commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxcold commented Nov 19, 2024

Uh oh!

maxcold left a comment

Choose a reason for hiding this comment

Uh oh!

maxcold commented Nov 21, 2024

Uh oh!

elasticmachine commented Nov 21, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

kibanamachine commented Nov 21, 2024

Uh oh!

kibanamachine commented Nov 21, 2024

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

animehart commented Nov 15, 2024 •

edited by kibanamachine

Loading