-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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.
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
[Cloud Security] [Vulnerabilities] Add grouping capabilities #174413
[Cloud Security] [Vulnerabilities] Add grouping capabilities #174413
Conversation
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
started with the review, found couple of skipped tests and commented out code. Will continue tomorrow
@@ -93,15 +93,15 @@ describe('<VulnerabilityFindingFlyout/>', () => { | |||
}); | |||
}); | |||
|
|||
it('should allow pagination with next', async () => { | |||
it.skip('should allow pagination with next', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the reason for skipping? do you plan to unskip in the context of this PR or do you see it as a tech debt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, I skipped those tests because there's a plan to re-enable them later once #168619 is resolved. I added a TODO comment now on the skipped tests so we can know when to re-enable them.
…/github.com/opauloh/kibana into findings-enhancements/vulnerabilities-table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm overall, there are a couple of things I want to clarify before approving
x-pack/plugins/cloud_security_posture/public/common/api/use_latest_findings_data_view.ts
Show resolved
Hide resolved
x-pack/plugins/cloud_security_posture/public/common/api/use_latest_findings_data_view.ts
Show resolved
Hide resolved
x-pack/plugins/cloud_security_posture/public/pages/vulnerabilities/constants.ts
Outdated
Show resolved
Hide resolved
...ins/cloud_security_posture/public/pages/vulnerabilities/hooks/use_latest_vulnerabilities.tsx
Outdated
Show resolved
Hide resolved
/** | ||
* Return first non-null value. If the field contains an array, this will return the first value that isn't null. If the field isn't an array it'll be returned unless it's null. | ||
*/ | ||
export function firstNonNullValue<T>(valueOrCollection: ECSField<T>): T | undefined { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this exists for findings as well. Worth sharing the code between the two?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and maybe it's true for some of the components in this file as well, the ones which are not specific for vulnerabilties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was a great idea, I ended up sharing the common components and utils in the cloud_security_grouping folder
Approving, but there is one comment unanswered about the code duplication in the |
…/github.com/opauloh/kibana into findings-enhancements/vulnerabilities-table
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
…#174413) ## Summary It closes elastic#165788 and elastic#169050 and elastic#174119 This PR adds the following changes to the Findings -> Vulnerabilities page: - Replaced the DataGrid component with the new CloudSecurityDataTable component - Added the new Grouping component with custom default renderers It also introduced the following changes: - Added context for DataView for easier reuse of the `dataView` methods. - Refactored the Misconfigurations component to consume the `dataView` context - added FTR tests for the Vulnerabilities page - dropped Flyout navigation support (will be readded in the future) ## Screenshots ![image](https://github.com/elastic/kibana/assets/19270322/c6cb7871-ce99-4c33-a615-142aaba63ec9) ![image](https://github.com/elastic/kibana/assets/19270322/ead3358f-7300-489b-b26d-5a37cc99e01d) ## Recording https://github.com/elastic/kibana/assets/19270322/a47be0cf-2849-424d-b19a-d65cb6f502a8 https://github.com/elastic/kibana/assets/19270322/fa395488-7a22-476e-a50e-d0f0e28429e1 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Summary
It closes #165788 and #169050 and #174119
This PR adds the following changes to the Findings -> Vulnerabilities page:
It also introduced the following changes:
dataView
methods.dataView
contextScreenshots
Recording
Screen.Recording.2024-01-09.at.12.09.57.PM.mov
Screen.Recording.2024-01-09.at.12.09.04.PM.mov