Merge branch 'remove-init-state' into 'master'

Refactor state initialization for monitoring dashboard

See merge request gitlab-org/gitlab!32529

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

@@ -111,28 +111,10 @@ export default {
       type: String,
       required: true,
     },
-    projectPath: {
-      type: String,
-      required: true,
-    },
-    logsPath: {
-      type: String,
-      required: false,
-      default: invalidUrl,
-    },
     defaultBranch: {
       type: String,
       required: true,
     },
-    metricsEndpoint: {
-      type: String,
-      required: true,
-    },
-    deploymentsEndpoint: {
-      type: String,
-      required: false,
-      default: null,
-    },
     emptyGettingStartedSvgPath: {
       type: String,
       required: true,
@@ -153,10 +135,6 @@ export default {
       type: String,
       required: true,
     },
-    currentEnvironmentName: {
-      type: String,
-      required: true,
-    },
     customMetricsAvailable: {
       type: Boolean,
       required: false,
@@ -172,21 +150,6 @@ export default {
       required: false,
       default: invalidUrl,
     },
-    dashboardEndpoint: {
-      type: String,
-      required: false,
-      default: invalidUrl,
-    },
-    dashboardsEndpoint: {
-      type: String,
-      required: false,
-      default: invalidUrl,
-    },
-    currentDashboard: {
-      type: String,
-      required: false,
-      default: '',
-    },
     smallEmptyState: {
       type: Boolean,
       required: false,
@@ -228,6 +191,8 @@ export default {
       'expandedPanel',
       'variables',
       'isUpdatingStarredValue',
+      'currentDashboard',
+      'currentEnvironmentName',
     ]),
     ...mapGetters('monitoringDashboard', [
       'selectedDashboard',
@@ -281,16 +246,6 @@ export default {
     },
   },
   created() {
-    this.setInitialState({
-      metricsEndpoint: this.metricsEndpoint,
-      deploymentsEndpoint: this.deploymentsEndpoint,
-      dashboardEndpoint: this.dashboardEndpoint,
-      dashboardsEndpoint: this.dashboardsEndpoint,
-      currentDashboard: this.currentDashboard,
-      projectPath: this.projectPath,
-      logsPath: this.logsPath,
-      currentEnvironmentName: this.currentEnvironmentName,
-    });
     window.addEventListener('keyup', this.onKeyup);
   },
   destroyed() {
@@ -310,7 +265,6 @@ export default {
       'fetchData',
       'fetchDashboardData',
       'setGettingStartedEmptyState',
-      'setInitialState',
       'setPanelGroupMetrics',
       'filterEnvironments',
       'setExpandedPanel',

app/assets/javascripts/monitoring/constants.js

@@ -140,7 +140,6 @@ export const dateFormats = {
  * Currently used in `receiveMetricsDashboardSuccess` action.
  */
 export const endpointKeys = [
-  'metricsEndpoint',
   'deploymentsEndpoint',
   'dashboardEndpoint',
   'dashboardsEndpoint',

app/assets/javascripts/monitoring/monitoring_bundle.js

@@ -3,7 +3,7 @@ import { GlToast } from '@gitlab/ui';
 import Dashboard from '~/monitoring/components/dashboard.vue';
 import { parseBoolean } from '~/lib/utils/common_utils';
 import { getParameterValues } from '~/lib/utils/url_utility';
-import store from './stores';
+import { createStore } from './stores';
 
 Vue.use(GlToast);
 
@@ -13,6 +13,31 @@ export default (props = {}) => {
   if (el && el.dataset) {
     const [currentDashboard] = getParameterValues('dashboard');
 
+    const {
+      deploymentsEndpoint,
+      dashboardEndpoint,
+      dashboardsEndpoint,
+      projectPath,
+      logsPath,
+      currentEnvironmentName,
+      ...dataProps
+    } = el.dataset;
+
+    const store = createStore({
+      currentDashboard,
+      deploymentsEndpoint,
+      dashboardEndpoint,
+      dashboardsEndpoint,
+      projectPath,
+      logsPath,
+      currentEnvironmentName,
+    });
+
+    // HTML attributes are always strings, parse other types.
+    dataProps.hasMetrics = parseBoolean(dataProps.hasMetrics);
+    dataProps.customMetricsAvailable = parseBoolean(dataProps.customMetricsAvailable);
+    dataProps.prometheusAlertsAvailable = parseBoolean(dataProps.prometheusAlertsAvailable);
+
     // eslint-disable-next-line no-new
     new Vue({
       el,
@@ -20,11 +45,7 @@ export default (props = {}) => {
       render(createElement) {
         return createElement(Dashboard, {
           props: {
-            ...el.dataset,
-            currentDashboard,
-            customMetricsAvailable: parseBoolean(el.dataset.customMetricsAvailable),
-            prometheusAlertsAvailable: parseBoolean(el.dataset.prometheusAlertsAvailable),
-            hasMetrics: parseBoolean(el.dataset.hasMetrics),
+            ...dataProps,
             ...props,
           },
         });

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

@@ -314,8 +314,7 @@ export const receiveEnvironmentsDataFailure = ({ commit }) => {
 
 export const fetchAnnotations = ({ state, dispatch }) => {
   const { start } = convertToFixedRange(state.timeRange);
-  const dashboardPath =
-    state.currentDashboard === '' ? DEFAULT_DASHBOARD_PATH : state.currentDashboard;
+  const dashboardPath = state.currentDashboard || DEFAULT_DASHBOARD_PATH;
   return gqClient
     .mutate({
       mutation: getAnnotations,

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

@@ -15,11 +15,15 @@ export const monitoringDashboard = {
   state,
 };
 
-export const createStore = () =>
+export const createStore = (initState = {}) =>
   new Vuex.Store({
     modules: {
-      monitoringDashboard,
+      monitoringDashboard: {
+        ...monitoringDashboard,
+        state: {
+          ...state(),
+          ...initState,
+        },
+      },
     },
   });
-
-export default createStore();

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

@@ -2,9 +2,9 @@ import invalidUrl from '~/lib/utils/invalid_url';
 
 export default () => ({
   // API endpoints
-  metricsEndpoint: null,
   deploymentsEndpoint: null,
   dashboardEndpoint: invalidUrl,
+  dashboardsEndpoint: invalidUrl,
 
   // Dashboard request parameters
   timeRange: null,
@@ -46,6 +46,7 @@ export default () => ({
   environments: [],
   environmentsSearchTerm: '',
   environmentsLoading: false,
+  currentEnvironmentName: null,
 
   // GitLab paths to other pages
   projectPath: null,

spec/frontend/monitoring/components/dashboard_spec.js

@@ -6,7 +6,6 @@ import { objectToQuery } from '~/lib/utils/url_utility';
 import VueDraggable from 'vuedraggable';
 import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
-import statusCodes from '~/lib/utils/http_status';
 import { metricStates } from '~/monitoring/constants';
 import Dashboard from '~/monitoring/components/dashboard.vue';
 
@@ -80,19 +79,6 @@ describe('Dashboard', () => {
     it('shows the environment selector', () => {
       expect(findEnvironmentsDropdown().exists()).toBe(true);
     });
-
-    it('sets initial state', () => {
-      expect(store.dispatch).toHaveBeenCalledWith('monitoringDashboard/setInitialState', {
-        currentDashboard: '',
-        currentEnvironmentName: 'production',
-        dashboardEndpoint: 'https://invalid',
-        dashboardsEndpoint: 'https://invalid',
-        deploymentsEndpoint: null,
-        logsPath: '/path/to/logs',
-        metricsEndpoint: 'http://test.host/monitoring/mock',
-        projectPath: '/path/to/project',
-      });
-    });
   });
 
   describe('no data found', () => {
@@ -288,7 +274,10 @@ describe('Dashboard', () => {
     it('URL is updated with panel parameters and custom dashboard', () => {
       const dashboard = 'dashboard.yml';
 
-      createMountedWrapper({ hasMetrics: true, currentDashboard: dashboard });
+      store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
+        currentDashboard: dashboard,
+      });
+      createMountedWrapper({ hasMetrics: true });
       expandPanel(group, panel);
 
       const expectedSearch = objectToQuery({
@@ -326,8 +315,10 @@ describe('Dashboard', () => {
 
   describe('when all requests have been commited by the store', () => {
     beforeEach(() => {
+      store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
+        currentEnvironmentName: 'production',
+      });
       createMountedWrapper({ hasMetrics: true });
-
       setupStoreWithData(store);
 
       return wrapper.vm.$nextTick();
@@ -785,7 +776,6 @@ describe('Dashboard', () => {
 
   describe('cluster health', () => {
     beforeEach(() => {
-      mock.onGet(propsData.metricsEndpoint).reply(statusCodes.OK, JSON.stringify({}));
       createShallowWrapper({ hasMetrics: true, showHeader: false });
 
       // all_dashboards is not defined in health dashboards
@@ -877,7 +867,10 @@ describe('Dashboard', () => {
 
     beforeEach(() => {
       setupStoreWithData(store);
-      createShallowWrapper({ hasMetrics: true, currentDashboard });
+      store.commit(`monitoringDashboard/${types.SET_INITIAL_STATE}`, {
+        currentDashboard,
+      });
+      createShallowWrapper({ hasMetrics: true });
 
       return wrapper.vm.$nextTick();
     });

spec/frontend/monitoring/components/dashboard_template_spec.js

@@ -14,7 +14,9 @@ describe('Dashboard template', () => {
   let mock;
 
   beforeEach(() => {
-    store = createStore();
+    store = createStore({
+      currentEnvironmentName: 'production',
+    });
     mock = new MockAdapter(axios);
 
     setupAllDashboards(store);

spec/frontend/monitoring/mock_data.js

@@ -11,17 +11,12 @@ export const propsData = {
   settingsPath: '/path/to/settings',
   clustersPath: '/path/to/clusters',
   tagsPath: '/path/to/tags',
-  projectPath: '/path/to/project',
-  logsPath: '/path/to/logs',
   defaultBranch: 'master',
-  metricsEndpoint: mockApiEndpoint,
-  deploymentsEndpoint: null,
   emptyGettingStartedSvgPath: '/path/to/getting-started.svg',
   emptyLoadingSvgPath: '/path/to/loading.svg',
   emptyNoDataSvgPath: '/path/to/no-data.svg',
   emptyNoDataSmallSvgPath: '/path/to/no-data-small.svg',
   emptyUnableToConnectSvgPath: '/path/to/unable-to-connect.svg',
-  currentEnvironmentName: 'production',
   customMetricsAvailable: false,
   customMetricsPath: '',
   validateQueryPath: '',

spec/frontend/monitoring/store/actions_spec.js

@@ -8,7 +8,7 @@ import createFlash from '~/flash';
 import { defaultTimeRange } from '~/vue_shared/constants';
 import { ENVIRONMENT_AVAILABLE_STATE } from '~/monitoring/constants';
 
-import store from '~/monitoring/stores';
+import { createStore } from '~/monitoring/stores';
 import * as types from '~/monitoring/stores/mutation_types';
 import {
   fetchData,
@@ -52,20 +52,16 @@ import {
 
 jest.mock('~/flash');
 
-const resetStore = str => {
-  str.replaceState({
-    showEmptyState: true,
-    emptyState: 'loading',
-    groups: [],
-  });
-};
-
 describe('Monitoring store actions', () => {
   const { convertObjectPropsToCamelCase } = commonUtils;
 
   let mock;
+  let store;
+  let state;
 
   beforeEach(() => {
+    store = createStore();
+    state = store.state.monitoringDashboard;
     mock = new MockAdapter(axios);
 
     jest.spyOn(commonUtils, 'backOff').mockImplementation(callback => {
@@ -83,7 +79,6 @@ describe('Monitoring store actions', () => {
     });
   });
   afterEach(() => {
-    resetStore(store);
     mock.reset();
 
     commonUtils.backOff.mockReset();
@@ -92,8 +87,6 @@ describe('Monitoring store actions', () => {
 
   describe('fetchData', () => {
     it('dispatches fetchEnvironmentsData and fetchEnvironmentsData', () => {
-      const { state } = store;
-
       return testAction(
         fetchData,
         null,
@@ -111,8 +104,6 @@ describe('Monitoring store actions', () => {
       const origGon = window.gon;
       window.gon = { features: { metricsDashboardAnnotations: true } };
 
-      const { state } = store;
-
       return testAction(
         fetchData,
         null,
@@ -131,7 +122,6 @@ describe('Monitoring store actions', () => {
 
   describe('fetchDeploymentsData', () => {
     it('dispatches receiveDeploymentsDataSuccess on success', () => {
-      const { state } = store;
       state.deploymentsEndpoint = '/success';
       mock.onGet(state.deploymentsEndpoint).reply(200, {
         deployments: deploymentData,
@@ -146,7 +136,6 @@ describe('Monitoring store actions', () => {
       );
     });
     it('dispatches receiveDeploymentsDataFailure on error', () => {
-      const { state } = store;
       state.deploymentsEndpoint = '/error';
       mock.onGet(state.deploymentsEndpoint).reply(500);
 
@@ -164,11 +153,8 @@ describe('Monitoring store actions', () => {
   });
 
   describe('fetchEnvironmentsData', () => {
-    const { state } = store;
-    state.projectPath = 'gitlab-org/gitlab-test';
-
-    afterEach(() => {
-      resetStore(store);
+    beforeEach(() => {
+      state.projectPath = 'gitlab-org/gitlab-test';
     });
 
     it('setting SET_ENVIRONMENTS_FILTER should dispatch fetchEnvironmentsData', () => {
@@ -269,17 +255,14 @@ describe('Monitoring store actions', () => {
   });
 
   describe('fetchAnnotations', () => {
-    const { state } = store;
-    state.timeRange = {
-      start: '2020-04-15T12:54:32.137Z',
-      end: '2020-08-15T12:54:32.137Z',
-    };
-    state.projectPath = 'gitlab-org/gitlab-test';
-    state.currentEnvironmentName = 'production';
-    state.currentDashboard = '.gitlab/dashboards/custom_dashboard.yml';
-
-    afterEach(() => {
-      resetStore(store);
+    beforeEach(() => {
+      state.timeRange = {
+        start: '2020-04-15T12:54:32.137Z',
+        end: '2020-08-15T12:54:32.137Z',
+      };
+      state.projectPath = 'gitlab-org/gitlab-test';
+      state.currentEnvironmentName = 'production';
+      state.currentDashboard = '.gitlab/dashboards/custom_dashboard.yml';
     });
 
     it('fetches annotations data and dispatches receiveAnnotationsSuccess', () => {
@@ -353,7 +336,6 @@ describe('Monitoring store actions', () => {
   });
 
   describe('Toggles starred value of current dashboard', () => {
-    const { state } = store;
     let unstarredDashboard;
     let starredDashboard;
 
@@ -396,23 +378,19 @@ describe('Monitoring store actions', () => {
   });
 
   describe('Set initial state', () => {
-    let mockedState;
-    beforeEach(() => {
-      mockedState = storeState();
-    });
     it('should commit SET_INITIAL_STATE mutation', done => {
       testAction(
         setInitialState,
         {
-          metricsEndpoint: 'additional_metrics.json',
+          currentDashboard: '.gitlab/dashboards/dashboard.yml',
           deploymentsEndpoint: 'deployments.json',
         },
-        mockedState,
+        state,
         [
           {
             type: types.SET_INITIAL_STATE,
             payload: {
-              metricsEndpoint: 'additional_metrics.json',
+              currentDashboard: '.gitlab/dashboards/dashboard.yml',
               deploymentsEndpoint: 'deployments.json',
             },
           },
@@ -423,15 +401,11 @@ describe('Monitoring store actions', () => {
     });
   });
   describe('Set empty states', () => {
-    let mockedState;
-    beforeEach(() => {
-      mockedState = storeState();
-    });
     it('should commit SET_METRICS_ENDPOINT mutation', done => {
       testAction(
         setGettingStartedEmptyState,
         null,
-        mockedState,
+        state,
         [
           {
             type: types.SET_GETTING_STARTED_EMPTY_STATE,
@@ -444,15 +418,11 @@ describe('Monitoring store actions', () => {
   });
 
   describe('updateVariableValues', () => {
-    let mockedState;
-    beforeEach(() => {
-      mockedState = storeState();
-    });
     it('should commit UPDATE_VARIABLE_VALUES mutation', done => {
       testAction(
         updateVariableValues,
         { pod: 'POD' },
-        mockedState,
+        state,
         [
           {
             type: types.UPDATE_VARIABLE_VALUES,
@@ -467,13 +437,11 @@ describe('Monitoring store actions', () => {
 
   describe('fetchDashboard', () => {
     let dispatch;
-    let state;
     let commit;
     const response = metricsDashboardResponse;
     beforeEach(() => {
       dispatch = jest.fn();
       commit = jest.fn();
-      state = storeState();
       state.dashboardEndpoint = '/dashboard';
     });
 
@@ -557,12 +525,10 @@ describe('Monitoring store actions', () => {
   describe('receiveMetricsDashboardSuccess', () => {
     let commit;
     let dispatch;
-    let state;
 
     beforeEach(() => {
       commit = jest.fn();
       dispatch = jest.fn();
-      state = storeState();
     });
 
     it('stores groups', () => {
@@ -623,13 +589,11 @@ describe('Monitoring store actions', () => {
   describe('fetchDashboardData', () => {
     let commit;
     let dispatch;
-    let state;
 
     beforeEach(() => {
       jest.spyOn(Tracking, 'event');
       commit = jest.fn();
       dispatch = jest.fn();
-      state = storeState();
 
       state.timeRange = defaultTimeRange;
     });
@@ -731,7 +695,6 @@ describe('Monitoring store actions', () => {
       step: 60,
     };
     let metric;
-    let state;
     let data;
     let prometheusEndpointPath;
 
@@ -929,10 +892,7 @@ describe('Monitoring store actions', () => {
   });
 
   describe('duplicateSystemDashboard', () => {
-    let state;
-
     beforeEach(() => {
-      state = storeState();
       state.dashboardsEndpoint = '/dashboards.json';
     });
 
@@ -1010,12 +970,6 @@ describe('Monitoring store actions', () => {
   });
 
   describe('setExpandedPanel', () => {
-    let state;
-
-    beforeEach(() => {
-      state = storeState();
-    });
-
     it('Sets a panel as expanded', () => {
       const group = 'group_1';
       const panel = { title: 'A Panel' };
@@ -1031,12 +985,6 @@ describe('Monitoring store actions', () => {
   });
 
   describe('clearExpandedPanel', () => {
-    let state;
-
-    beforeEach(() => {
-      state = storeState();
-    });
-
     it('Clears a panel as expanded', () => {
       return testAction(
         clearExpandedPanel,

spec/frontend/monitoring/store/index_spec.js

+import { createStore } from '~/monitoring/stores';
+
+describe('Monitoring Store Index', () => {
+  it('creates store with a `monitoringDashboard` namespace', () => {
+    expect(createStore().state).toEqual({
+      monitoringDashboard: expect.any(Object),
+    });
+  });
+
+  it('creates store with initial values', () => {
+    const defaults = {
+      deploymentsEndpoint: '/mock/deployments',
+      dashboardEndpoint: '/mock/dashboard',
+      dashboardsEndpoint: '/mock/dashboards',
+    };
+
+    const { state } = createStore(defaults);
+
+    expect(state).toEqual({
+      monitoringDashboard: expect.objectContaining(defaults),
+    });
+  });
+});

spec/frontend/monitoring/store/mutations_spec.js

@@ -128,13 +128,11 @@ describe('Monitoring mutations', () => {
   describe('SET_INITIAL_STATE', () => {
     it('should set all the endpoints', () => {
       mutations[types.SET_INITIAL_STATE](stateCopy, {
-        metricsEndpoint: 'additional_metrics.json',
         deploymentsEndpoint: 'deployments.json',
         dashboardEndpoint: 'dashboard.json',
         projectPath: '/gitlab-org/gitlab-foss',
         currentEnvironmentName: 'production',
       });
-      expect(stateCopy.metricsEndpoint).toEqual('additional_metrics.json');
       expect(stateCopy.deploymentsEndpoint).toEqual('deployments.json');
       expect(stateCopy.dashboardEndpoint).toEqual('dashboard.json');
       expect(stateCopy.projectPath).toEqual('/gitlab-org/gitlab-foss');
@@ -179,12 +177,10 @@ describe('Monitoring mutations', () => {
   describe('SET_ENDPOINTS', () => {
     it('should set all the endpoints', () => {
       mutations[types.SET_ENDPOINTS](stateCopy, {
-        metricsEndpoint: 'additional_metrics.json',
         deploymentsEndpoint: 'deployments.json',
         dashboardEndpoint: 'dashboard.json',
         projectPath: '/gitlab-org/gitlab-foss',
       });
-      expect(stateCopy.metricsEndpoint).toEqual('additional_metrics.json');
       expect(stateCopy.deploymentsEndpoint).toEqual('deployments.json');
       expect(stateCopy.dashboardEndpoint).toEqual('dashboard.json');
       expect(stateCopy.projectPath).toEqual('/gitlab-org/gitlab-foss');