[Cloud Security] add compliance dashboard ui tab persistence #173853

Omolola-Akinleye · 2023-12-21T16:09:03Z

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Add UI tab persistence to the Compliance Dashboard. The selected posture type tab is persisted in local storage. Here are some of the test cases for Compliance Dashboard Tab Persistence.

Test case: Changing url in the same tab will persist tab
Test case: Logging in and out of Kibana and going a back to Kibana will persist tab
Test Case: Navigating to Cloud Security Dashboard within Kibana will persist tab
Test Case: Dashboard recovery mode witching from the onboarding state to generating a Dashboard with findings still works and tab will persist.
Test Case: Refreshing url tab continues to persist.
Test Case: If no tab is selected, then CSPM dashboard will show.
Test Case: Redirect subscribes to the persisted selected tab in the url.

Screen.Recording.2023-12-21.at.11.06.10.AM.mov

elasticmachine · 2023-12-21T16:47:52Z

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

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

opauloh · 2023-12-22T00:44:13Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+        <Routes>
+          <Route
+            exact
+            path={cloudPosturePages.dashboard.path}
+            render={() => <DashboardTabRedirecter lastTabSelected={selectedPostureTypeTab} />}
+          />
+          <Route path={cloudPosturePages.cspm_dashboard.path}>
+            <IntegrationPostureDashboard
+              dashboardType={policyTemplate}
+              complianceData={getDashboardData.data}
+              notInstalledConfig={getNotInstalledConfig(policyTemplate, integrationLink)}
+              isIntegrationInstalled={setupStatus !== 'not-installed'}
+            />
+          </Route>
+
+          <Route path={cloudPosturePages.kspm_dashboard.path}>
+            <IntegrationPostureDashboard
+              dashboardType={policyTemplate}
+              complianceData={getDashboardData.data}
+              notInstalledConfig={getNotInstalledConfig(policyTemplate, integrationLink)}
+              isIntegrationInstalled={setupStatus !== 'not-installed'}
+            />
+          </Route>
+          {/* Redirect to default cspm dashboard if no match */}
+          <Route path="*" render={() => <Redirect to={cloudPosturePages.cspm_dashboard.path} />} />
+        </Routes>


Following my previous comment, I think any routing logic should be handled by the parent

@opauloh The only challenge here is the effort and potential side effects of moving routing logic from the TabContent component to ComplianceDashboard. Compliance Dashboard Component has useEffect concerned with getDefaultTab, and list a number of dependencies also feature in TabContent Component.

useEffect(() => { if (selectedTabLocalStorage) { return; } const preferredDashboard = getDefaultTab( getSetupStatus, getCspmDashboardData.data, getKspmDashboardData.data ); setSelectedTabLocalStorage(preferredDashboard); }, [ getCspmDashboardData.data, selectedTabLocalStorage, setSelectedTabLocalStorage, getCspmDashboardData.data?.stats.totalFindings, getKspmDashboardData.data, getKspmDashboardData.data?.stats.totalFindings, getSetupStatus, getSetupStatus?.cspm?.status, getSetupStatus?.kspm?.status, ]);

The TabContent component's routing logic currently has a re-fetching dependency listening to prop and then proceeds to fetch status and dashboard data until there are findings.

…talled.

...ugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.test.tsx

…condition

…ye/kibana into dashboard-tab-persistence

x-pack/performance/journeys/cloud_security_dashboard.ts

dmlemeshko · 2024-02-09T08:49:01Z

I removed appex-qa review since journey changes were reverted.

Omolola-Akinleye · 2024-02-09T16:03:33Z

@dmlemeshko Thanks!

LGTM, waits for Paulo approval

opauloh

Tested locally, the URL params changes seem ok, but I noticed that when navigating back to the Dashboard from the application itself it will always open the Dashboard in the Cloud tab instead of the previous Tab selected by the user.

Screen.Recording.2024-02-12.at.12.14.19.PM.mov

This is by design, or it's missing the local storage persistence? I ask because the original ticket #148435 asks for persisting in the local storage.

opauloh · 2024-02-12T19:54:35Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+      ? [
+          {
+            label: i18n.translate('xpack.csp.dashboardTabs.cloudTab.tabTitle', {
+              defaultMessage: 'Cloud',


Shouldn't this point to CSPM_DASHBOARD_TAB_NAME?

I believe since we are using translation, we just preserve the default text message. Cloud and Kubernetes. I don't know if it's necessary to add a parameter since we are only using one word.

opauloh · 2024-02-12T19:55:03Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+          },
+          {
+            label: i18n.translate('xpack.csp.dashboardTabs.kubernetesTab.tabTitle', {
+              defaultMessage: 'Kubernetes',


Shouldn't this point to KSPM_DASHBOARD_TAB_NAME

opauloh · 2024-02-12T20:10:11Z

x-pack/plugins/cloud_security_posture/public/common/constants.ts

@@ -48,6 +48,8 @@ export const LOCAL_STORAGE_DASHBOARD_CLUSTER_SORT_KEY =
 export const LOCAL_STORAGE_DASHBOARD_BENCHMARK_SORT_KEY =
  'cloudPosture:complianceDashboard:benchmarkSort';
 export const LOCAL_STORAGE_FINDINGS_LAST_SELECTED_TAB_KEY = 'cloudPosture:findings:lastSelectedTab';
+export const LOCAL_STORAGE_COMPLIANCE_DASHBOARD_SELECTED_TAB_KEY =


It seems this constant is declared but is not being reused elsewhere

Thanks! I'll remove that!

Omolola-Akinleye · 2024-02-12T20:36:09Z

Tested locally, the URL params changes seem ok, but I noticed that when navigating back to the Dashboard from the application itself it will always open the Dashboard in the Cloud tab instead of the previous Tab selected by the user.

Screen.Recording.2024-02-12.at.12.14.19.PM.mov
This is by design, or it's missing the local storage persistence? I ask because the original ticket #148435 asks for persisting in the local storage.

Yes the original ticket asked for local storage, but talked it over with @kfirpeled, and local storage was redundant and not utilized as common practice for routing with tabs in Kibana.

opauloh

LGTM, just some minor comments

JordanSh · 2024-02-13T15:41:31Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+          {/* Redirect to default cspm dashboard if no match */}
+          {/* <Route path="*" render={() => <Redirect to={cloudPosturePages.cspm_dashboard.path} />} />*/}


looks to be missing from code

Thanks! I will address the following code in follow-up PR. I want to push PR before FF

JordanSh · 2024-02-13T15:44:09Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+  if (!cspmStats || !pluginStatus) {
+    return;
+  }


shouldn't kspmStats be checked as well?
what happens of the one of those calls failed? do we get an empty screen or the error log?

When first routing to Dashboard page, CSPM stats and KSPM stats are undefined. There is no need to check if kspmStats are defined. This check helps resolves race condition when we are waiting for cspm stats and kspm stats to be defined then show the correct tabs based on plugin status, cspm and kspm status.

JordanSh · 2024-02-13T15:46:55Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+    if (location.pathname === cloudPosturePages.kspm_dashboard.path) {
+      tab = POSTURE_TYPE_KSPM;


you can also add the postureType as an optional parameter to the default url, and then check the value with useParams of react-router

example

JordanSh · 2024-02-13T15:50:08Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+        search: encodeQuery({
+          // Set query language from user's preference
+          query: services.data.query.queryString.getDefaultQuery(),
+          filters: services.data.query.filterManager.getFilters(),
+        }),


do we need those filters/queries on the dashboard?

This preserves filters/queries on the dashboard when clicking on each Tab. We are preserving the filters and queries when clicking tabs and routing between tabs. We are keeping the url consistent with filters/queries similar practice across Kibana.

JordanSh · 2024-02-13T15:55:31Z

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx

+  );
+
+  const tabs = useMemo(() => {
+    const navigateToPostureTypeDashboardTab = (pathname: string) => {


this should probably be a hook in a separate, file next to use_navigate_findings
but also navigateToPostureTypeDashboardTab is just a nav to what ever path is being passed to it. which makes it a bit random. i would recommand to change the implementation for something like:

const navigateToPostureTypeDashboardTab = (postureType: 'kspm' | 'cspm') => { history.push( generatePath(benchmarksNavigation.rules.path, { postureType // this is assuming you are adding the postureType as optional param to the path, but if not you can also just do an "if" and navigate to the selected tab based on the value of postureType }) ); }

I tried use_navigate_findings pattern hooks. TabContent onClick Handler behaves differently than React.jsx element click handler. onclick does support hook being called

dmlemeshko · 2024-02-13T16:03:44Z

x-pack/performance/journeys/cloud_security_dashboard.ts

+  const shouldSkip = true;
+
+  if (!shouldSkip) {
+    await page.goto(kbnUrl.get(`/app/security/cloud_security_posture/dashboard`));
+    await page.waitForSelector(subj('csp:dashboard-sections-table-header-score'));
+  }


Could you explain this change? If the step is redundant, I suggest to remove it.
But if it is temporary fix, adding a comment would be a good thing.

I was attempting to skip the test. I will just comment out the code and add a comment! Thank you.

dmlemeshko

journey update LGTM

kibana-ci · 2024-02-13T20:54:03Z

💚 Build Succeeded

Buildkite Build
Commit: ec44e93

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`	412.2KB	412.7KB	+485.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cloudSecurityPosture`	15.8KB	16.0KB	+209.0B

History

💔 Build #193381 failed ca6be22
💔 Build #193312 failed 897f75c
💚 Build #193163 succeeded b5d51b4
💚 Build #193113 succeeded 664ebc5
💔 Build #193095 failed dae4dff

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

cc @Omolola-Akinleye

…#173853) ## Summary Summarize your PR. If it involves visual changes include a screenshot or gif. Add UI tab persistence to the Compliance Dashboard. The selected posture type tab is persisted in local storage. Here are some of the test cases for Compliance Dashboard Tab Persistence. - Test case: Changing url in the same tab will persist tab - Test case: Logging in and out of Kibana and going a back to Kibana will persist tab - Test Case: Navigating to Cloud Security Dashboard within Kibana will persist tab - Test Case: Dashboard recovery mode witching from the onboarding state to generating a Dashboard with findings still works and tab will persist. - Test Case: Refreshing url tab continues to persist. - Test Case: If no tab is selected, then CSPM dashboard will show. - Test Case: Redirect subscribes to the persisted selected tab in the url. https://github.com/elastic/kibana/assets/17135495/1ce156f6-6d0c-4436-be78-d76aba19e8f5

add dashboard ui tab persistence

5d8d425

Omolola-Akinleye added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v8.13.0 labels Dec 21, 2023

Omolola-Akinleye self-assigned this Dec 21, 2023

Omolola-Akinleye linked an issue Dec 21, 2023 that may be closed by this pull request

[Cloud Security] Misconfiguration - Multiple integration support - selected tab to be URL based #148435

Closed

Omolola-Akinleye marked this pull request as ready for review December 21, 2023 16:47

Omolola-Akinleye requested a review from a team as a code owner December 21, 2023 16:47

opauloh reviewed Dec 22, 2023

View reviewed changes

...ck/plugins/cloud_security_posture/public/pages/compliance_dashboard/compliance_dashboard.tsx Show resolved Hide resolved

opauloh reviewed Dec 22, 2023

View reviewed changes

merge main into branch and resolve conflict

af3831e

Omolola-Akinleye force-pushed the dashboard-tab-persistence branch from c165413 to af3831e Compare January 2, 2024 19:19

Omolola-Akinleye added 4 commits January 2, 2024 14:22

fix prop name casing

42b0cb1

change dashboard re-direct logic

fb2007a

fix unit tests for compliance dashboard

64bf84d

Fix re-direct on default tab security posture package hasn't been ins…

8c57d53

…talled.

Omolola-Akinleye mentioned this pull request Jan 10, 2024

[Fix] Journey Performance FTR for Compliance Dashboard Page #174646

Open

kfirpeled previously requested changes Jan 25, 2024

View reviewed changes

fix performance test and revert dashboard redirect code causing race …

d634053

…condition

Omolola-Akinleye requested a review from a team as a code owner January 29, 2024 17:21

Omolola-Akinleye added 8 commits January 29, 2024 12:32

resolve merge conflicts

f8237d4

Merge branch 'main' into dashboard-tab-persistence

f2827f6

use url state

d3b675d

Merge branch 'dashboard-tab-persistence' of github.com:Omolola-Akinle…

cf390b3

…ye/kibana into dashboard-tab-persistence

use usememo for url state changes

a9358ba

rework dashboard url routing

c40b629

fix getDefaultTab tests

4deafe1

fix getDefaultTab tests

861d374

kfirpeled reviewed Feb 4, 2024

View reviewed changes

x-pack/performance/journeys/cloud_security_dashboard.ts Outdated Show resolved Hide resolved

Omolola-Akinleye requested a review from kfirpeled February 8, 2024 22:40

dmlemeshko removed the request for review from a team February 9, 2024 08:47

Merge branch 'main' into dashboard-tab-persistence

5410128

Omolola-Akinleye added 3 commits February 12, 2024 09:51

Merge branch 'main' into dashboard-tab-persistence

d57a8bc

skip e2e journey test

dae4dff

Merge branch 'main' into dashboard-tab-persistence

664ebc5

opauloh requested changes Feb 12, 2024

View reviewed changes

opauloh approved these changes Feb 12, 2024

View reviewed changes

Omolola-Akinleye enabled auto-merge (squash) February 12, 2024 22:46

Omolola-Akinleye added 2 commits February 12, 2024 17:46

Merge branch 'main' into dashboard-tab-persistence

b5d51b4

Merge branch 'main' into dashboard-tab-persistence

897f75c

Omolola-Akinleye requested a review from a team February 13, 2024 15:51

JordanSh reviewed Feb 13, 2024

View reviewed changes

dmlemeshko reviewed Feb 13, 2024

View reviewed changes

Omolola-Akinleye added 2 commits February 13, 2024 11:36

Merge branch 'main' into dashboard-tab-persistence

0075932

add comment and remove dead code

ca6be22

dmlemeshko approved these changes Feb 13, 2024

View reviewed changes

Omolola-Akinleye added backport and removed backport labels Feb 13, 2024

Merge branch 'main' into dashboard-tab-persistence

ec44e93

Omolola-Akinleye merged commit 06d4b3e into elastic:main Feb 13, 2024
16 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Feb 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cloud Security] add compliance dashboard ui tab persistence #173853

[Cloud Security] add compliance dashboard ui tab persistence #173853

Omolola-Akinleye commented Dec 21, 2023 •

edited

Loading

elasticmachine commented Dec 21, 2023

opauloh Dec 22, 2023

Omolola-Akinleye Jan 2, 2024 •

edited

Loading

dmlemeshko commented Feb 9, 2024

Omolola-Akinleye commented Feb 9, 2024

opauloh left a comment

opauloh Feb 12, 2024

Omolola-Akinleye Feb 12, 2024

opauloh Feb 12, 2024

opauloh Feb 12, 2024

Omolola-Akinleye Feb 12, 2024 •

edited

Loading

Omolola-Akinleye commented Feb 12, 2024

opauloh left a comment

JordanSh Feb 13, 2024

Omolola-Akinleye Feb 13, 2024 •

edited

Loading

JordanSh Feb 13, 2024

Omolola-Akinleye Feb 13, 2024

JordanSh Feb 13, 2024

JordanSh Feb 13, 2024

Omolola-Akinleye Feb 13, 2024 •

edited

Loading

JordanSh Feb 13, 2024

Omolola-Akinleye Feb 13, 2024

dmlemeshko Feb 13, 2024

Omolola-Akinleye Feb 13, 2024

dmlemeshko left a comment

kibana-ci commented Feb 13, 2024

		{/* Redirect to default cspm dashboard if no match */}
		{/* <Route path="" render={() => <Redirect to={cloudPosturePages.cspm_dashboard.path} />} />/}

		if (location.pathname === cloudPosturePages.kspm_dashboard.path) {
		tab = POSTURE_TYPE_KSPM;

[Cloud Security] add compliance dashboard ui tab persistence #173853

[Cloud Security] add compliance dashboard ui tab persistence #173853

Conversation

Omolola-Akinleye commented Dec 21, 2023 • edited Loading

Summary

elasticmachine commented Dec 21, 2023

Choose a reason for hiding this comment

Omolola-Akinleye Jan 2, 2024 • edited Loading

Choose a reason for hiding this comment

dmlemeshko commented Feb 9, 2024

Omolola-Akinleye commented Feb 9, 2024

opauloh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Omolola-Akinleye Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

Omolola-Akinleye commented Feb 12, 2024

opauloh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Omolola-Akinleye Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Omolola-Akinleye Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmlemeshko left a comment

Choose a reason for hiding this comment

kibana-ci commented Feb 13, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Omolola-Akinleye commented Dec 21, 2023 •

edited

Loading

Omolola-Akinleye Jan 2, 2024 •

edited

Loading

Omolola-Akinleye Feb 12, 2024 •

edited

Loading

Omolola-Akinleye Feb 13, 2024 •

edited

Loading

Omolola-Akinleye Feb 13, 2024 •

edited

Loading