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.

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 Posture] [Quick wins] Enable edit DataView on the Misconfigurations Findings Table #173870

Merged

Conversation

opauloh
Copy link
Contributor

@opauloh opauloh commented Dec 21, 2023

Summary

This PR is part of the Quick Wins improvements for 8.13.0. It enables the Edit DataView option on the Misconfigurations Findings Page.

It also adds the following:

  • Unit tests to test the basic functionality of the CloudSecurityDataTable component.
  • Updated unit tests on the configurations page to reflect the current hook
  • Added deprecation notice on the useCloudSecurityTable hook.
  • FTR test to check if the option to edit DataView is enabled.
  • A Check on the useLatestFindingsDataView hook to only update when needed (This update logic is planned to be replaced by a server-side callback when installing the Cloud Security integration. Still, for now, it is an improvement over the current logic).

Screenshots

image

image

image

@opauloh opauloh added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related csp: quick win cloud-security label: tagging issues which are relatively small in effort and lowered in priority v8.13.0 labels Dec 21, 2023
@opauloh opauloh requested a review from a team as a code owner December 21, 2023 19:45
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@maxcold maxcold left a comment

Choose a reason for hiding this comment

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

tested, works great!
couple of general questions, not directly related to the PR

@@ -62,15 +62,19 @@ export const useLatestFindingsDataView = (dataView: string) => {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

btw what use perfected kibana data views means (in the TODO for this hook)? not directly related to this PR, but I'd add more context to this todo, as I for instance wouldn't know what to do with it

@@ -50,7 +50,11 @@ export const Configurations = () => {
path={findingsNavigation.findings_default.path}
render={() => (
<TrackApplicationView viewId={findingsNavigation.findings_default.id}>
<LatestFindingsContainer dataView={dataViewQuery.data!} />
<LatestFindingsContainer
dataView={dataViewQuery.data!}
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple of general questions:

  • any downside in passing dataViewQuery to avoid having these three props being passed together all the time?
  • we don't use context that much in our code, wdyt about the context for the dataView to avoid this multi layer prop drill?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any downside in passing dataViewQuery to avoid having these three props being passed together all the time?

IMO the downside is that we add a deep coupling with an object from the react-query third-party library which is an implementation detail, then we need to mock the library to be able to unit test it, and these tests won't be so reliable in case something break in future releases of the library.

we don't use context that much in our code, wdyt about the context for the dataView to avoid this multi layer prop drill?

that's a good suggestion, better than passing down a react-query object, I created a technical debt ticket to address that in a follow up PR

@opauloh opauloh enabled auto-merge (squash) January 2, 2024 19:14
@kibana-ci
Copy link
Collaborator

💚 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
cloudSecurityPosture 454.9KB 455.2KB +377.0B

History

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

@opauloh opauloh merged commit e62b7ac into elastic:main Jan 2, 2024
19 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 2, 2024
jloleysens added a commit that referenced this pull request Jan 4, 2024
* main: (4129 commits)
  [Logs Explorer] Change the default link for "Discover" in the serverless nav (#173420)
  [Fleet] fix unhandled error in agent details when components are missing (#174152)
  [Obs UX] Unskip transaction duration alerts test (#174069)
  [Fleet] Fix keep policies up to date after package install (#174093)
  [Profiling] Stack traces embeddable (#173905)
  [main] Sync bundled packages with Package Storage (#174115)
  [SLO Form] Refactor to use kibana data view component (#173513)
  [Obs UX] Unskip APM Service Inventory Journey (#173510)
  [Obs UX] Unskip preview_chart_error_count test (#173624)
  [api-docs] 2024-01-03 Daily api_docs build (#174142)
  Update babel runtime helpers (#171745)
  Handle content stream errors in report pre-deletion (#173792)
  [Cloud Posture] [Quick wins] Enable edit DataView on the Misconfigurations Findings Table (#173870)
  [ftr] abort retry on invalid webdriver session (#174092)
  Upgrade openai to 4.24.1 (#173934)
  chore(NA): bump node into v20 (#173461)
  [Security Solution][Endpoint] Fix index name pattern in SentinelOne dev. script (#174105)
  fix versions.json
  [Obs AI Assistant] Add guardrails (#174060)
  [ML] Transforms: Refactor validators and add unit tests. (#173736)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting csp: quick win cloud-security label: tagging issues which are relatively small in effort and lowered in priority release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants