Merge branch '214581-use-selected-dashboard-getter-2' into 'master'

(Resubmitted) Metrics: Use a single source for the selected dashboard (Vuex getter)

Closes #214581

See merge request gitlab-org/gitlab!32125

app/assets/javascripts/monitoring/components/dashboard.vue

@@ -228,13 +228,11 @@ export default {
       'promVariables',
       'isUpdatingStarredValue',
     ]),
-    ...mapGetters('monitoringDashboard', ['getMetricStates', 'filteredEnvironments']),
-    firstDashboard() {
-      return this.allDashboards.length > 0 ? this.allDashboards[0] : {};
-    },
-    selectedDashboard() {
-      return this.allDashboards.find(d => d.path === this.currentDashboard) || this.firstDashboard;
-    },
+    ...mapGetters('monitoringDashboard', [
+      'selectedDashboard',
+      'getMetricStates',
+      'filteredEnvironments',
+    ]),
     showRearrangePanelsBtn() {
       return !this.showEmptyState && this.rearrangePanelsAvailable;
     },
@@ -242,7 +240,10 @@ export default {
       return (
         this.customMetricsAvailable &&
         !this.showEmptyState &&
-        this.firstDashboard === this.selectedDashboard
+        // Custom metrics only avaialble on system dashboards because
+        // they are stored in the database. This can be improved. See:
+        // https://gitlab.com/gitlab-org/gitlab/-/issues/28241
+        this.selectedDashboard?.system_dashboard
       );
     },
     shouldShowEnvironmentsDropdownNoMatchedMsg() {
@@ -269,7 +270,7 @@ export default {
     },
     expandedPanel: {
       handler({ group, panel }) {
-        const dashboardPath = this.currentDashboard || this.firstDashboard.path;
+        const dashboardPath = this.currentDashboard || this.selectedDashboard?.path;
         updateHistory({
           url: panelToUrl(dashboardPath, group, panel),
           title: document.title,
@@ -341,7 +342,7 @@ export default {
       this.selectedTimeRange = defaultTimeRange;
     },
     generatePanelUrl(groupKey, panel) {
-      const dashboardPath = this.currentDashboard || this.firstDashboard.path;
+      const dashboardPath = this.currentDashboard || this.selectedDashboard?.path;
       return panelToUrl(dashboardPath, groupKey, panel);
     },
     hideAddMetricModal() {
@@ -597,7 +598,10 @@ export default {
           </gl-modal>
         </div>
 
-        <div v-if="selectedDashboard.can_edit" class="mb-2 mr-2 d-flex d-sm-block">
+        <div
+          v-if="selectedDashboard && selectedDashboard.can_edit"
+          class="mb-2 mr-2 d-flex d-sm-block"
+        >
           <gl-deprecated-button
             class="flex-grow-1 js-edit-link"
             :href="selectedDashboard.project_blob_path"

app/assets/javascripts/monitoring/stores/getters.js

@@ -14,7 +14,9 @@ const metricsIdsInPanel = panel =>
 export const selectedDashboard = state => {
   const { allDashboards } = state;
   return (
-    allDashboards.find(({ path }) => path === state.currentDashboard) || allDashboards[0] || null
+    allDashboards.find(d => d.path === state.currentDashboard) ||
+    allDashboards.find(d => d.default) ||
+    null
   );
 };
 

spec/frontend/monitoring/components/dashboard_spec.js

@@ -19,6 +19,7 @@ import DashboardPanel from '~/monitoring/components/dashboard_panel.vue';
 import { createStore } from '~/monitoring/stores';
 import * as types from '~/monitoring/stores/mutation_types';
 import {
+  setupAllDashboards,
   setupStoreWithDashboard,
   setMetricResult,
   setupStoreWithData,
@@ -279,7 +280,7 @@ describe('Dashboard', () => {
         expect(window.history.pushState).toHaveBeenCalledWith(
           expect.anything(), // state
           expect.any(String), // document title
-          expect.stringContaining(`?${expectedSearch}`),
+          expect.stringContaining(`${expectedSearch}`),
         );
       });
     });
@@ -302,7 +303,7 @@ describe('Dashboard', () => {
         expect(window.history.pushState).toHaveBeenCalledWith(
           expect.anything(), // state
           expect.any(String), // document title
-          expect.stringContaining(`?${expectedSearch}`),
+          expect.stringContaining(`${expectedSearch}`),
         );
       });
     });
@@ -317,7 +318,7 @@ describe('Dashboard', () => {
         expect(window.history.pushState).toHaveBeenCalledWith(
           expect.anything(), // state
           expect.any(String), // document title
-          expect.not.stringContaining('?'), // no params
+          expect.not.stringMatching(/group|title|y_label/), // no panel params
         );
       });
     });
@@ -359,6 +360,7 @@ describe('Dashboard', () => {
 
     beforeEach(() => {
       createShallowWrapper();
+      setupAllDashboards(store);
     });
 
     it('toggle star button is shown', () => {
@@ -380,10 +382,7 @@ describe('Dashboard', () => {
       const getToggleTooltip = () => findToggleStar().element.parentElement.getAttribute('title');
 
       beforeEach(() => {
-        wrapper.vm.$store.commit(
-          `monitoringDashboard/${types.SET_ALL_DASHBOARDS}`,
-          dashboardGitResponse,
-        );
+        setupAllDashboards(store);
         jest.spyOn(store, 'dispatch');
       });
 
@@ -400,7 +399,9 @@ describe('Dashboard', () => {
 
       describe('when dashboard is not starred', () => {
         beforeEach(() => {
-          wrapper.setProps({ currentDashboard: dashboardGitResponse[0].path });
+          store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
+            currentDashboard: dashboardGitResponse[0].path,
+          });
           return wrapper.vm.$nextTick();
         });
 
@@ -415,7 +416,9 @@ describe('Dashboard', () => {
 
       describe('when dashboard is starred', () => {
         beforeEach(() => {
-          wrapper.setProps({ currentDashboard: dashboardGitResponse[1].path });
+          store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
+            currentDashboard: dashboardGitResponse[1].path,
+          });
           return wrapper.vm.$nextTick();
         });
 
@@ -551,7 +554,7 @@ describe('Dashboard', () => {
 
       it('sets a link to the expanded panel', () => {
         const searchQuery =
-          '?group=System%20metrics%20(Kubernetes)&title=Memory%20Usage%20(Total)&y_label=Total%20Memory%20Used%20(GB)';
+          '?dashboard=config%2Fprometheus%2Fcommon_metrics.yml&group=System%20metrics%20(Kubernetes)&title=Memory%20Usage%20(Total)&y_label=Total%20Memory%20Used%20(GB)';
 
         expect(findExpandedPanel().attributes('clipboard-text')).toEqual(
           expect.stringContaining(searchQuery),
@@ -808,10 +811,7 @@ describe('Dashboard', () => {
     beforeEach(() => {
       createShallowWrapper({ hasMetrics: true });
 
-      wrapper.vm.$store.commit(
-        `monitoringDashboard/${types.SET_ALL_DASHBOARDS}`,
-        dashboardGitResponse,
-      );
+      setupAllDashboards(store);
       return wrapper.vm.$nextTick();
     });
 
@@ -820,10 +820,11 @@ describe('Dashboard', () => {
     });
 
     it('is present for a custom dashboard, and links to its edit_path', () => {
-      const dashboard = dashboardGitResponse[1]; // non-default dashboard
-      const currentDashboard = dashboard.path;
+      const dashboard = dashboardGitResponse[1];
+      store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
+        currentDashboard: dashboard.path,
+      });
 
-      wrapper.setProps({ currentDashboard });
       return wrapper.vm.$nextTick().then(() => {
         expect(findEditLink().exists()).toBe(true);
         expect(findEditLink().attributes('href')).toBe(dashboard.project_blob_path);
@@ -834,12 +835,7 @@ describe('Dashboard', () => {
   describe('Dashboard dropdown', () => {
     beforeEach(() => {
       createMountedWrapper({ hasMetrics: true });
-
-      wrapper.vm.$store.commit(
-        `monitoringDashboard/${types.SET_ALL_DASHBOARDS}`,
-        dashboardGitResponse,
-      );
-
+      setupAllDashboards(store);
       return wrapper.vm.$nextTick();
     });
 
@@ -872,7 +868,7 @@ describe('Dashboard', () => {
   });
 
   describe('Clipboard text in panels', () => {
-    const currentDashboard = 'TEST_DASHBOARD';
+    const currentDashboard = dashboardGitResponse[1].path;
     const panelIndex = 1; // skip expanded panel
 
     const getClipboardTextFirstPanel = () =>
@@ -882,37 +878,20 @@ describe('Dashboard', () => {
         .props('clipboardText');
 
     beforeEach(() => {
+      setupStoreWithData(store);
       createShallowWrapper({ hasMetrics: true, currentDashboard });
 
-      setupStoreWithData(wrapper.vm.$store);
-
       return wrapper.vm.$nextTick();
     });
 
     it('contains a link to the dashboard', () => {
-      expect(getClipboardTextFirstPanel()).toContain(`dashboard=${currentDashboard}`);
+      const dashboardParam = `dashboard=${encodeURIComponent(currentDashboard)}`;
+
+      expect(getClipboardTextFirstPanel()).toContain(dashboardParam);
       expect(getClipboardTextFirstPanel()).toContain(`group=`);
       expect(getClipboardTextFirstPanel()).toContain(`title=`);
       expect(getClipboardTextFirstPanel()).toContain(`y_label=`);
     });
-
-    it('strips the undefined parameter', () => {
-      wrapper.setProps({ currentDashboard: undefined });
-
-      return wrapper.vm.$nextTick(() => {
-        expect(getClipboardTextFirstPanel()).not.toContain(`dashboard=`);
-        expect(getClipboardTextFirstPanel()).toContain(`y_label=`);
-      });
-    });
-
-    it('null parameter is stripped', () => {
-      wrapper.setProps({ currentDashboard: null });
-
-      return wrapper.vm.$nextTick(() => {
-        expect(getClipboardTextFirstPanel()).not.toContain(`dashboard=`);
-        expect(getClipboardTextFirstPanel()).toContain(`y_label=`);
-      });
-    });
   });
 
   describe('add custom metrics', () => {

spec/frontend/monitoring/components/dashboard_template_spec.js

@@ -3,6 +3,7 @@ import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import Dashboard from '~/monitoring/components/dashboard.vue';
 import { createStore } from '~/monitoring/stores';
+import { setupAllDashboards } from '../store_utils';
 import { propsData } from '../mock_data';
 
 jest.mock('~/lib/utils/url_utility');
@@ -15,6 +16,8 @@ describe('Dashboard template', () => {
   beforeEach(() => {
     store = createStore();
     mock = new MockAdapter(axios);
+
+    setupAllDashboards(store);
   });
 
   afterEach(() => {

spec/frontend/monitoring/store_utils.js

 import * as types from '~/monitoring/stores/mutation_types';
-import { metricsResult, environmentData } from './mock_data';
+import { metricsResult, environmentData, dashboardGitResponse } from './mock_data';
 import { metricsDashboardPayload } from './fixture_data';
 
 export const setMetricResult = ({ $store, result, group = 0, panel = 0, metric = 0 }) => {
@@ -16,11 +16,19 @@ const setEnvironmentData = $store => {
   $store.commit(`monitoringDashboard/${types.RECEIVE_ENVIRONMENTS_DATA_SUCCESS}`, environmentData);
 };
 
+export const setupAllDashboards = $store => {
+  $store.commit(`monitoringDashboard/${types.SET_ALL_DASHBOARDS}`, dashboardGitResponse);
+};
+
 export const setupStoreWithDashboard = $store => {
   $store.commit(
     `monitoringDashboard/${types.RECEIVE_METRICS_DASHBOARD_SUCCESS}`,
     metricsDashboardPayload,
   );
+  $store.commit(
+    `monitoringDashboard/${types.RECEIVE_METRICS_DASHBOARD_SUCCESS}`,
+    metricsDashboardPayload,
+  );
 };
 
 export const setupStoreWithVariable = $store => {
@@ -30,6 +38,7 @@ export const setupStoreWithVariable = $store => {
 };
 
 export const setupStoreWithData = $store => {
+  setupAllDashboards($store);
   setupStoreWithDashboard($store);
 
   setMetricResult({ $store, result: [], panel: 0 });