[Monitoring] Fix cluster listing page in how it handles global state (#…

…78979) (#81049) * Properly unset global state on the listing page * Fix how we handle global state in getSafeForExternalLink * Fix breadcrumbs for clusters link * Fix tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Oct 19, 2020 · a0d84e6 · a0d84e6
1 parent ec56dbc
commit a0d84e6
Show file tree

Hide file tree

Showing 10 changed files with 163 additions and 43 deletions.
diff --git a/x-pack/plugins/monitoring/public/components/cluster/listing/listing.js b/x-pack/plugins/monitoring/public/components/cluster/listing/listing.js
@@ -79,7 +79,7 @@ const getColumns = (
         if (cluster.isSupported) {
           return (
             <EuiLink
-              href={getSafeForExternalLink(`#/overview?_g=(cluster_uuid:${cluster.cluster_uuid})`)}
+              href={getSafeForExternalLink(`#/overview`, { cluster_uuid: cluster.cluster_uuid })}
               data-test-subj="clusterLink"
             >
               {value}

diff --git a/x-pack/plugins/monitoring/public/legacy_shims.ts b/x-pack/plugins/monitoring/public/legacy_shims.ts
@@ -20,6 +20,7 @@ interface BreadcrumbItem {
   ['data-test-subj']?: string;
   href?: string;
   text: string;
+  ignoreGlobalState?: boolean;
 }
 
 export interface KFetchQuery {
@@ -96,9 +97,10 @@ export class Legacy {
           }
           breadcrumbs.forEach((breadcrumb: BreadcrumbItem) => {
             const breadcrumbHref = breadcrumb.href?.split('?')[0];
-            if (breadcrumbHref) {
+            if (breadcrumbHref && !breadcrumb.ignoreGlobalState) {
               breadcrumb.href = `${breadcrumbHref}?${globalStateStr}`;
             }
+            delete breadcrumb.ignoreGlobalState;
           });
           core.chrome.setBreadcrumbs(breadcrumbs.slice(0));
         },

diff --git a/x-pack/plugins/monitoring/public/lib/get_cluster_from_clusters.js b/x-pack/plugins/monitoring/public/lib/get_cluster_from_clusters.js
@@ -6,7 +6,7 @@
 
 import _ from 'lodash';
 
-export function getClusterFromClusters(clusters, globalState) {
+export function getClusterFromClusters(clusters, globalState, unsetGlobalState = false) {
   const cluster = (() => {
     const existingCurrent = _.find(clusters, { cluster_uuid: globalState.cluster_uuid });
     if (existingCurrent) {
@@ -22,8 +22,8 @@ export function getClusterFromClusters(clusters, globalState) {
   })();
 
   if (cluster && cluster.license) {
-    globalState.cluster_uuid = cluster.cluster_uuid;
-    globalState.ccs = cluster.ccs;
+    globalState.cluster_uuid = unsetGlobalState ? undefined : cluster.cluster_uuid;
+    globalState.ccs = unsetGlobalState ? undefined : cluster.ccs;
     globalState.save();
     return cluster;
   }

diff --git a/x-pack/plugins/monitoring/public/lib/get_safe_for_external_link.test.ts b/x-pack/plugins/monitoring/public/lib/get_safe_for_external_link.test.ts
@@ -0,0 +1,74 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { getSafeForExternalLink } from './get_safe_for_external_link';
+
+describe('getSafeForExternalLink', () => {
+  it('should work without a query string', async () => {
+    const location = {
+      hash: '',
+    };
+    expect(getSafeForExternalLink('#/overview', {}, location)).toBe('#/overview');
+  });
+
+  it('should work with a query string', async () => {
+    const location = {
+      hash: '',
+    };
+    expect(getSafeForExternalLink('#/overview?foo=bar', {}, location)).toBe('#/overview?foo=bar');
+  });
+
+  it('should work without a hash', async () => {
+    const location = {
+      hash: undefined,
+    };
+    expect(getSafeForExternalLink('#/overview?foo=bar', {}, location)).toBe('#/overview?foo=bar');
+  });
+
+  it('should work with existing query string', async () => {
+    const location = {
+      hash:
+        '#/overview?_g=(cluster_uuid:ae2womLaSMmZBioEQ9wFjw,refreshInterval:(pause:!t,value:10000),time:(from:now-1h,to:now))',
+    };
+    expect(getSafeForExternalLink('#/overview', {}, location)).toBe(
+      '#/overview?_g=(cluster_uuid:ae2womLaSMmZBioEQ9wFjw,refreshInterval:(pause:!t,value:10000),time:(from:now-1h,to:now))'
+    );
+  });
+
+  it('should override global state, for at least cluster_uuid', async () => {
+    const location = {
+      hash: `#/home?_g=(cluster_uuid:1,filters:!(),refreshInterval:(pause:!t,value:10000),time:(from:'2017-09-07T20:12:04.011Z',to:'2017-09-07T20:18:55.733Z'))`,
+    };
+    expect(
+      getSafeForExternalLink(
+        '#/overview',
+        {
+          cluster_uuid: 'NDKg6VXAT6-TaGzEK2Zy7g',
+        },
+        location
+      )
+    ).toBe(
+      `#/overview?_g=(cluster_uuid:NDKg6VXAT6-TaGzEK2Zy7g,filters:!(),refreshInterval:(pause:!t,value:10000),time:(from:'2017-09-07T20:12:04.011Z',to:'2017-09-07T20:18:55.733Z'))`
+    );
+  });
+
+  it('should add global state if it does not exist, for at least cluster_uuid', async () => {
+    const location = {
+      hash: `#/home?_g=(filters:!(),refreshInterval:(pause:!t,value:10000),time:(from:'2017-09-07T20:12:04.011Z',to:'2017-09-07T20:18:55.733Z'))`,
+    };
+    expect(
+      getSafeForExternalLink(
+        '#/overview',
+        {
+          cluster_uuid: 'NDKg6VXAT6-TaGzEK2Zy7g',
+        },
+        location
+      )
+    ).toBe(
+      `#/overview?_g=(filters:!(),refreshInterval:(pause:!t,value:10000),time:(from:'2017-09-07T20:12:04.011Z',to:'2017-09-07T20:18:55.733Z'),cluster_uuid:NDKg6VXAT6-TaGzEK2Zy7g)`
+    );
+  });
+});
diff --git a/x-pack/plugins/monitoring/public/lib/get_safe_for_external_link.ts b/x-pack/plugins/monitoring/public/lib/get_safe_for_external_link.ts
@@ -4,6 +4,38 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export function getSafeForExternalLink(url: string) {
-  return `${url.split('?')[0]}?${location.hash.split('?')[1]}`;
+interface LocationHash {
+  hash?: string;
+}
+
+export function getSafeForExternalLink(
+  url: string,
+  globalState: Record<string, any> = {},
+  location: LocationHash = window.location
+) {
+  let hash = location.hash ? location.hash.split('?')[1] : '';
+  const globalStateExecResult = /_g=\((.+)\)$/.exec(hash);
+  if (!globalStateExecResult || !globalStateExecResult.length) {
+    if (!hash) {
+      return url;
+    }
+    return `${url.split('?')[0]}?${hash}`;
+  }
+
+  let newGlobalState = globalStateExecResult[1];
+  Object.keys(globalState).forEach((globalStateKey) => {
+    const keyRegExp = new RegExp(`${globalStateKey}:([^,]+)`);
+    const execResult = keyRegExp.exec(newGlobalState);
+    if (execResult && execResult.length) {
+      newGlobalState = newGlobalState.replace(
+        execResult[0],
+        `${globalStateKey}:${globalState[globalStateKey]}`
+      );
+    } else {
+      newGlobalState += `,${globalStateKey}:${globalState[globalStateKey]}`;
+    }
+  });
+
+  hash = hash.replace(globalStateExecResult[0], `_g=(${newGlobalState})`);
+  return `${url.split('?')[0]}?${hash}`;
 }
diff --git a/x-pack/plugins/monitoring/public/lib/route_init.js b/x-pack/plugins/monitoring/public/lib/route_init.js
@@ -22,14 +22,14 @@ export function routeInitProvider(Private, monitoringClusters, globalState, lice
    * the data just has a single cluster or
    * all the clusters are basic and this is the primary cluster
    */
-  return function routeInit({ codePaths, fetchAllClusters }) {
+  return function routeInit({ codePaths, fetchAllClusters, unsetGlobalState = false }) {
     const clusterUuid = fetchAllClusters ? null : globalState.cluster_uuid;
     return (
       monitoringClusters(clusterUuid, undefined, codePaths)
         // Set the clusters collection and current cluster in globalState
         .then((clusters) => {
           const inSetupMode = isInSetupMode();
-          const cluster = getClusterFromClusters(clusters, globalState);
+          const cluster = getClusterFromClusters(clusters, globalState, unsetGlobalState);
           if (!cluster && !inSetupMode) {
             window.history.replaceState(null, null, '#/no-data');
             return Promise.resolve();

diff --git a/x-pack/plugins/monitoring/public/services/breadcrumbs.js b/x-pack/plugins/monitoring/public/services/breadcrumbs.js
@@ -8,8 +8,8 @@ import { Legacy } from '../legacy_shims';
 import { i18n } from '@kbn/i18n';
 
 // Helper for making objects to use in a link element
-const createCrumb = (url, label, testSubj) => {
-  const crumb = { url, label };
+const createCrumb = (url, label, testSubj, ignoreGlobalState = false) => {
+  const crumb = { url, label, ignoreGlobalState };
   if (testSubj) {
     crumb.testSubj = testSubj;
   }
@@ -177,7 +177,7 @@ export function breadcrumbsProvider() {
       defaultMessage: 'Clusters',
     });
 
-    let breadcrumbs = [createCrumb('#/home', homeCrumb, 'breadcrumbClusters')];
+    let breadcrumbs = [createCrumb('#/home', homeCrumb, 'breadcrumbClusters', true)];
 
     if (!mainInstance.inOverview && clusterName) {
       breadcrumbs.push(createCrumb('#/overview', clusterName));
@@ -204,6 +204,7 @@ export function breadcrumbsProvider() {
         text: b.label,
         href: b.url,
         'data-test-subj': b.testSubj,
+        ignoreGlobalState: b.ignoreGlobalState,
       }))
     );
 

diff --git a/x-pack/plugins/monitoring/public/services/breadcrumbs.test.js b/x-pack/plugins/monitoring/public/services/breadcrumbs.test.js
@@ -50,8 +50,8 @@ describe('Monitoring Breadcrumbs Service', () => {
       },
     });
     expect(controller.breadcrumbs).to.eql([
-      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters' },
-      { url: '#/overview', label: 'test-cluster-foo' },
+      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters', ignoreGlobalState: true },
+      { url: '#/overview', label: 'test-cluster-foo', ignoreGlobalState: false },
     ]);
   });
 
@@ -66,7 +66,7 @@ describe('Monitoring Breadcrumbs Service', () => {
       },
     });
     expect(controller.breadcrumbs).to.eql([
-      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters' },
+      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters', ignoreGlobalState: true },
     ]);
   });
 
@@ -87,11 +87,16 @@ describe('Monitoring Breadcrumbs Service', () => {
       },
     });
     expect(controller.breadcrumbs).to.eql([
-      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters' },
-      { url: '#/overview', label: 'test-cluster-foo' },
-      { url: '#/elasticsearch', label: 'Elasticsearch' },
-      { url: '#/elasticsearch/nodes', label: 'Nodes', testSubj: 'breadcrumbEsNodes' },
-      { url: null, label: 'es-node-name-01' },
+      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters', ignoreGlobalState: true },
+      { url: '#/overview', label: 'test-cluster-foo', ignoreGlobalState: false },
+      { url: '#/elasticsearch', label: 'Elasticsearch', ignoreGlobalState: false },
+      {
+        url: '#/elasticsearch/nodes',
+        label: 'Nodes',
+        testSubj: 'breadcrumbEsNodes',
+        ignoreGlobalState: false,
+      },
+      { url: null, label: 'es-node-name-01', ignoreGlobalState: false },
     ]);
   });
 
@@ -107,9 +112,9 @@ describe('Monitoring Breadcrumbs Service', () => {
       },
     });
     expect(controller.breadcrumbs).to.eql([
-      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters' },
-      { url: '#/overview', label: 'test-cluster-foo' },
-      { url: null, label: 'Kibana' },
+      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters', ignoreGlobalState: true },
+      { url: '#/overview', label: 'test-cluster-foo', ignoreGlobalState: false },
+      { url: null, label: 'Kibana', ignoreGlobalState: false },
     ]);
   });
 
@@ -128,9 +133,9 @@ describe('Monitoring Breadcrumbs Service', () => {
       },
     });
     expect(controller.breadcrumbs).to.eql([
-      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters' },
-      { url: '#/overview', label: 'test-cluster-foo' },
-      { url: null, label: 'Logstash' },
+      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters', ignoreGlobalState: true },
+      { url: '#/overview', label: 'test-cluster-foo', ignoreGlobalState: false },
+      { url: null, label: 'Logstash', ignoreGlobalState: false },
     ]);
   });
 
@@ -151,10 +156,10 @@ describe('Monitoring Breadcrumbs Service', () => {
       },
     });
     expect(controller.breadcrumbs).to.eql([
-      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters' },
-      { url: '#/overview', label: 'test-cluster-foo' },
-      { url: '#/logstash', label: 'Logstash' },
-      { url: '#/logstash/pipelines', label: 'Pipelines' },
+      { url: '#/home', label: 'Clusters', testSubj: 'breadcrumbClusters', ignoreGlobalState: true },
+      { url: '#/overview', label: 'test-cluster-foo', ignoreGlobalState: false },
+      { url: '#/logstash', label: 'Logstash', ignoreGlobalState: false },
+      { url: '#/logstash/pipelines', label: 'Pipelines', ignoreGlobalState: false },
     ]);
   });
 });
diff --git a/x-pack/plugins/monitoring/public/views/cluster/listing/index.js b/x-pack/plugins/monitoring/public/views/cluster/listing/index.js
@@ -26,7 +26,11 @@ uiRoutes
     resolve: {
       clusters: (Private) => {
         const routeInit = Private(routeInitProvider);
-        return routeInit({ codePaths: CODE_PATHS, fetchAllClusters: true }).then((clusters) => {
+        return routeInit({
+          codePaths: CODE_PATHS,
+          fetchAllClusters: true,
+          unsetGlobalState: true,
+        }).then((clusters) => {
           if (!clusters || !clusters.length) {
             window.location.hash = '#/no-data';
             return Promise.reject();

diff --git a/x-pack/plugins/monitoring/public/views/loading/index.js b/x-pack/plugins/monitoring/public/views/loading/index.js
@@ -49,19 +49,21 @@ uiRoutes.when('/loading', {
       };
 
       const routeInit = Private(routeInitProvider);
-      routeInit({ codePaths: CODE_PATHS, fetchAllClusters: true }).then((clusters) => {
-        if (!clusters || !clusters.length) {
-          window.location.hash = '#/no-data';
-          return;
-        }
-        if (clusters.length === 1) {
-          // Bypass the cluster listing if there is just 1 cluster
-          window.history.replaceState(null, null, '#/overview');
-          return;
-        }
+      routeInit({ codePaths: CODE_PATHS, fetchAllClusters: true, unsetGlobalState: true }).then(
+        (clusters) => {
+          if (!clusters || !clusters.length) {
+            window.location.hash = '#/no-data';
+            return;
+          }
+          if (clusters.length === 1) {
+            // Bypass the cluster listing if there is just 1 cluster
+            window.history.replaceState(null, null, '#/overview');
+            return;
+          }
 
-        window.history.replaceState(null, null, '#/home');
-      });
+          window.history.replaceState(null, null, '#/home');
+        }
+      );
     }
   },
 });