Merge branch 'refactor-variables-for-array' into 'master'

Simplify dashboard variable manipulation by using arrays

See merge request gitlab-org/gitlab!34821

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

@@ -164,7 +164,7 @@ export default {
     ]),
     ...mapGetters('monitoringDashboard', ['selectedDashboard', 'getMetricStates']),
     shouldShowVariablesSection() {
-      return Object.keys(this.variables).length > 0;
+      return Boolean(this.variables.length);
     },
     shouldShowLinksSection() {
       return Object.keys(this.links).length > 0;

app/assets/javascripts/monitoring/components/variables/dropdown_field.vue

@@ -34,7 +34,7 @@ export default {
   },
   methods: {
     onUpdate(value) {
-      this.$emit('onUpdate', this.name, value);
+      this.$emit('input', value);
     },
   },
 };

app/assets/javascripts/monitoring/components/variables/text_field.vue

@@ -22,7 +22,7 @@ export default {
   },
   methods: {
     onUpdate(event) {
-      this.$emit('onUpdate', this.name, event.target.value);
+      this.$emit('input', event.target.value);
     },
   },
 };

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

@@ -16,10 +16,9 @@ export default {
   methods: {
     ...mapActions('monitoringDashboard', ['updateVariablesAndFetchData']),
     refreshDashboard(variable, value) {
-      if (this.variables[variable].value !== value) {
-        const changedVariable = { key: variable, value };
+      if (variable.value !== value) {
+        this.updateVariablesAndFetchData({ name: variable.name, value });
         // update the Vuex store
-        this.updateVariablesAndFetchData(changedVariable);
         // the below calls can ideally be moved out of the
         // component and into the actions and let the
         // mutation respond directly.
@@ -39,15 +38,15 @@ export default {
 </script>
 <template>
   <div ref="variablesSection" class="d-sm-flex flex-sm-wrap pt-2 pr-1 pb-0 pl-2 variables-section">
-    <div v-for="(variable, key) in variables" :key="key" class="mb-1 pr-2 d-flex d-sm-block">
+    <div v-for="variable in variables" :key="variable.name" class="mb-1 pr-2 d-flex d-sm-block">
       <component
         :is="variableField(variable.type)"
         class="mb-0 flex-grow-1"
         :label="variable.label"
         :value="variable.value"
-        :name="key"
+        :name="variable.name"
         :options="variable.options"
-        @onUpdate="refreshDashboard"
+        @input="refreshDashboard(variable, $event)"
       />
     </div>
   </div>

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

@@ -77,10 +77,6 @@ export const setTimeRange = ({ commit }, timeRange) => {
   commit(types.SET_TIME_RANGE, timeRange);
 };
 
-export const setVariables = ({ commit }, variables) => {
-  commit(types.SET_VARIABLES, variables);
-};
-
 export const filterEnvironments = ({ commit, dispatch }, searchTerm) => {
   commit(types.SET_ENVIRONMENTS_FILTER, searchTerm);
   dispatch('fetchEnvironmentsData');
@@ -235,7 +231,7 @@ export const fetchPrometheusMetric = (
     queryParams.step = metric.step;
   }
 
-  if (Object.keys(state.variables).length > 0) {
+  if (state.variables.length > 0) {
     queryParams = {
       ...queryParams,
       ...getters.getCustomVariablesParams,
@@ -480,7 +476,7 @@ export const fetchVariableMetricLabelValues = ({ state, commit }, { defaultQuery
   const { start_time, end_time } = defaultQueryParams;
   const optionsRequests = [];
 
-  Object.entries(state.variables).forEach(([key, variable]) => {
+  state.variables.forEach(variable => {
     if (variable.type === VARIABLE_TYPES.metric_label_values) {
       const { prometheusEndpointPath, label } = variable.options;
 
@@ -496,7 +492,7 @@ export const fetchVariableMetricLabelValues = ({ state, commit }, { defaultQuery
         .catch(() => {
           createFlash(
             sprintf(s__('Metrics|There was an error getting options for variable "%{name}".'), {
-              name: key,
+              name: variable.name,
             }),
           );
         });

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

@@ -133,8 +133,8 @@ export const linksWithMetadata = state => {
 };
 
 /**
- * Maps an variables object to an array along with stripping
- * the variable prefix.
+ * Maps a variables array to an object for replacement in
+ * prometheus queries.
  *
  * This method outputs an object in the below format
  *
@@ -147,14 +147,17 @@ export const linksWithMetadata = state => {
  * user-defined variables coming through the URL and differentiate
  * from other variables used for Prometheus API endpoint.
  *
- * @param {Object} variables - Custom variables provided by the user
- * @returns {Array} The custom variables array to be send to the API
+ * @param {Object} state - State containing variables provided by the user
+ * @returns {Array} The custom variables object to be send to the API
  * in the format of {variables[key1]=value1, variables[key2]=value2}
  */
 
 export const getCustomVariablesParams = state =>
-  Object.keys(state.variables).reduce((acc, variable) => {
-    acc[addPrefixToCustomVariableParams(variable)] = state.variables[variable]?.value;
+  state.variables.reduce((acc, variable) => {
+    const { name, value } = variable;
+    if (value !== null) {
+      acc[addPrefixToCustomVariableParams(name)] = value;
+    }
     return acc;
   }, {});
 

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

@@ -2,7 +2,6 @@
 export const REQUEST_METRICS_DASHBOARD = 'REQUEST_METRICS_DASHBOARD';
 export const RECEIVE_METRICS_DASHBOARD_SUCCESS = 'RECEIVE_METRICS_DASHBOARD_SUCCESS';
 export const RECEIVE_METRICS_DASHBOARD_FAILURE = 'RECEIVE_METRICS_DASHBOARD_FAILURE';
-export const SET_VARIABLES = 'SET_VARIABLES';
 export const UPDATE_VARIABLE_VALUE = 'UPDATE_VARIABLE_VALUE';
 export const UPDATE_VARIABLE_METRIC_LABEL_VALUES = 'UPDATE_VARIABLE_METRIC_LABEL_VALUES';
 

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

@@ -203,14 +203,13 @@ export default {
     state.expandedPanel.group = group;
     state.expandedPanel.panel = panel;
   },
-  [types.SET_VARIABLES](state, variables) {
-    state.variables = variables;
-  },
-  [types.UPDATE_VARIABLE_VALUE](state, { key, value }) {
-    Object.assign(state.variables[key], {
-      ...state.variables[key],
-      value,
-    });
+  [types.UPDATE_VARIABLE_VALUE](state, { name, value }) {
+    const variable = state.variables.find(v => v.name === name);
+    if (variable) {
+      Object.assign(variable, {
+        value,
+      });
+    }
   },
   [types.UPDATE_VARIABLE_METRIC_LABEL_VALUES](state, { variable, label, data = [] }) {
     const values = optionsFromSeriesData({ label, data });

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

@@ -47,7 +47,7 @@ export default () => ({
    * User-defined custom variables are passed
    * via the dashboard yml file.
    */
-  variables: {},
+  variables: [],
   /**
    * User-defined custom links are passed
    * via the dashboard yml file.

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

@@ -289,7 +289,7 @@ export const mapToDashboardViewModel = ({
 }) => {
   return {
     dashboard,
-    variables: mergeURLVariables(parseTemplatingVariables(templating)),
+    variables: mergeURLVariables(parseTemplatingVariables(templating.variables)),
     links: links.map(mapLinksToViewModel),
     panelGroups: panel_groups.map(mapToPanelGroupViewModel),
   };
@@ -453,10 +453,10 @@ export const normalizeQueryResponseData = data => {
  *
  * This is currently only used by getters/getCustomVariablesParams
  *
- * @param {String} key Variable key that needs to be prefixed
+ * @param {String} name Variable key that needs to be prefixed
  * @returns {String}
  */
-export const addPrefixToCustomVariableParams = key => `variables[${key}]`;
+export const addPrefixToCustomVariableParams = name => `variables[${name}]`;
 
 /**
  * Normalize custom dashboard paths. This method helps support

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

@@ -46,7 +46,7 @@ const textAdvancedVariableParser = advTextVar => ({
  * @param {Object} custom variable option
  * @returns {Object} normalized custom variable options
  */
-const normalizeVariableValues = ({ default: defaultOpt = false, text, value }) => ({
+const normalizeVariableValues = ({ default: defaultOpt = false, text, value = null }) => ({
   default: defaultOpt,
   text: text || value,
   value,
@@ -68,10 +68,10 @@ const customAdvancedVariableParser = advVariable => {
   return {
     type: VARIABLE_TYPES.custom,
     label: advVariable.label,
-    value: defaultValue?.value,
     options: {
       values,
     },
+    value: defaultValue?.value || null,
   };
 };
 
@@ -100,27 +100,24 @@ const customSimpleVariableParser = simpleVar => {
   const values = (simpleVar || []).map(parseSimpleCustomValues);
   return {
     type: VARIABLE_TYPES.custom,
-    value: values[0].value,
     label: null,
+    value: values[0].value || null,
     options: {
       values: values.map(normalizeVariableValues),
     },
   };
 };
 
-const metricLabelValuesVariableParser = variable => {
-  const { label, options = {} } = variable;
-  return {
-    type: VARIABLE_TYPES.metric_label_values,
-    value: null,
-    label,
-    options: {
-      prometheusEndpointPath: options.prometheus_endpoint_path || '',
-      label: options.label || null,
-      values: [], // values are initially empty
-    },
-  };
-};
+const metricLabelValuesVariableParser = ({ label, options = {} }) => ({
+  type: VARIABLE_TYPES.metric_label_values,
+  label,
+  value: null,
+  options: {
+    prometheusEndpointPath: options.prometheus_endpoint_path || '',
+    label: options.label || null,
+    values: [], // values are initially empty
+  },
+});
 
 /**
  * Utility method to determine if a custom variable is
@@ -161,29 +158,26 @@ const getVariableParser = variable => {
  * for the user to edit. The values from input elements are relayed to
  * backend and eventually Prometheus API.
  *
- * This method currently is not used anywhere. Once the issue
- * https://gitlab.com/gitlab-org/gitlab/-/issues/214536 is completed,
- * this method will have been used by the monitoring dashboard.
- *
- * @param {Object} templating templating variables from the dashboard yml file
- * @returns {Object} a map of processed templating variables
+ * @param {Object} templating variables from the dashboard yml file
+ * @returns {array} An array of variables to display as inputs
  */
-export const parseTemplatingVariables = ({ variables = {} } = {}) =>
-  Object.entries(variables).reduce((acc, [key, variable]) => {
+export const parseTemplatingVariables = (ymlVariables = {}) =>
+  Object.entries(ymlVariables).reduce((acc, [name, ymlVariable]) => {
     // get the parser
-    const parser = getVariableParser(variable);
+    const parser = getVariableParser(ymlVariable);
     // parse the variable
-    const parsedVar = parser(variable);
+    const variable = parser(ymlVariable);
     // for simple custom variable label is null and it should be
     // replace with key instead
-    if (parsedVar) {
-      acc[key] = {
-        ...parsedVar,
-        label: parsedVar.label || key,
-      };
+    if (variable) {
+      acc.push({
+        ...variable,
+        name,
+        label: variable.label || name,
+      });
     }
     return acc;
-  }, {});
+  }, []);
 
 /**
  * Custom variables are defined in the dashboard yml file
@@ -201,23 +195,18 @@ export const parseTemplatingVariables = ({ variables = {} } = {}) =>
  * This method can be improved further. See the below issue
  * https://gitlab.com/gitlab-org/gitlab/-/issues/217713
  *
- * @param {Object} varsFromYML template variables from yml file
+ * @param {array} parsedYmlVariables - template variables from yml file
  * @returns {Object}
  */
-export const mergeURLVariables = (varsFromYML = {}) => {
+export const mergeURLVariables = (parsedYmlVariables = []) => {
   const varsFromURL = templatingVariablesFromUrl();
-  const variables = {};
-  Object.keys(varsFromYML).forEach(key => {
-    if (Object.prototype.hasOwnProperty.call(varsFromURL, key)) {
-      variables[key] = {
-        ...varsFromYML[key],
-        value: varsFromURL[key],
-      };
-    } else {
-      variables[key] = varsFromYML[key];
+  parsedYmlVariables.forEach(variable => {
+    const { name } = variable;
+    if (Object.prototype.hasOwnProperty.call(varsFromURL, name)) {
+      Object.assign(variable, { value: varsFromURL[name] });
     }
   });
-  return variables;
+  return parsedYmlVariables;
 };
 
 /**

app/assets/javascripts/monitoring/utils.js

@@ -201,8 +201,10 @@ export const removePrefixFromLabel = label =>
  * @returns {Object}
  */
 export const convertVariablesForURL = variables =>
-  Object.keys(variables || {}).reduce((acc, key) => {
-    acc[addPrefixToLabel(key)] = variables[key]?.value;
+  variables.reduce((acc, { name, value }) => {
+    if (value !== null) {
+      acc[addPrefixToLabel(name)] = value;
+    }
     return acc;
   }, {});
 

spec/frontend/monitoring/components/dashboard_spec.js

@@ -26,10 +26,9 @@ import {
   setMetricResult,
   setupStoreWithData,
   setupStoreWithDataForPanelCount,
-  setupStoreWithVariable,
   setupStoreWithLinks,
 } from '../store_utils';
-import { environmentData, dashboardGitResponse } from '../mock_data';
+import { environmentData, dashboardGitResponse, storeVariables } from '../mock_data';
 import {
   metricsDashboardViewModel,
   metricsDashboardPanelCount,
@@ -604,8 +603,7 @@ describe('Dashboard', () => {
     beforeEach(() => {
       createShallowWrapper({ hasMetrics: true });
       setupStoreWithData(store);
-      setupStoreWithVariable(store);
-
+      store.state.monitoringDashboard.variables = storeVariables;
       return wrapper.vm.$nextTick();
     });
 

spec/frontend/monitoring/components/variables/dropdown_field_spec.js

@@ -59,7 +59,7 @@ describe('Custom variable component', () => {
       .vm.$emit('click');
 
     return wrapper.vm.$nextTick(() => {
-      expect(wrapper.vm.$emit).toHaveBeenCalledWith('onUpdate', 'env', 'canary');
+      expect(wrapper.vm.$emit).toHaveBeenCalledWith('input', 'canary');
     });
   });
 });

spec/frontend/monitoring/components/variables/text_field_spec.js

@@ -40,7 +40,7 @@ describe('Text variable component', () => {
     findInput().trigger('keyup.enter');
 
     return wrapper.vm.$nextTick(() => {
-      expect(wrapper.vm.$emit).toHaveBeenCalledWith('onUpdate', 'pod', 'prod-pod');
+      expect(wrapper.vm.$emit).toHaveBeenCalledWith('input', 'prod-pod');
     });
   });
 
@@ -53,7 +53,7 @@ describe('Text variable component', () => {
     findInput().trigger('blur');
 
     return wrapper.vm.$nextTick(() => {
-      expect(wrapper.vm.$emit).toHaveBeenCalledWith('onUpdate', 'pod', 'canary-pod');
+      expect(wrapper.vm.$emit).toHaveBeenCalledWith('input', 'canary-pod');
     });
   });
 });

spec/frontend/monitoring/components/variables_section_spec.js

@@ -6,8 +6,7 @@ import TextField from '~/monitoring/components/variables/text_field.vue';
 import { updateHistory, mergeUrlParams } from '~/lib/utils/url_utility';
 import { createStore } from '~/monitoring/stores';
 import { convertVariablesForURL } from '~/monitoring/utils';
-import * as types from '~/monitoring/stores/mutation_types';
-import { mockTemplatingDataResponses } from '../mock_data';
+import { storeVariables } from '../mock_data';
 
 jest.mock('~/lib/utils/url_utility', () => ({
   updateHistory: jest.fn(),
@@ -17,12 +16,6 @@ jest.mock('~/lib/utils/url_utility', () => ({
 describe('Metrics dashboard/variables section component', () => {
   let store;
   let wrapper;
-  const sampleVariables = {
-    label1: mockTemplatingDataResponses.simpleText.simpleText,
-    label2: mockTemplatingDataResponses.advText.advText,
-    label3: mockTemplatingDataResponses.simpleCustom.simpleCustom,
-    label4: mockTemplatingDataResponses.metricLabelValues.simple,
-  };
 
   const createShallowWrapper = () => {
     wrapper = shallowMount(VariablesSection, {
@@ -48,22 +41,23 @@ describe('Metrics dashboard/variables section component', () => {
 
   describe('when variables are set', () => {
     beforeEach(() => {
+      store.state.monitoringDashboard.variables = storeVariables;
       createShallowWrapper();
-      store.commit(`monitoringDashboard/${types.SET_VARIABLES}`, sampleVariables);
+
       return wrapper.vm.$nextTick;
     });
 
     it('shows the variables section', () => {
       const allInputs = findTextInputs().length + findCustomInputs().length;
 
-      expect(allInputs).toBe(Object.keys(sampleVariables).length);
+      expect(allInputs).toBe(storeVariables.length);
     });
 
     it('shows the right custom variable inputs', () => {
       const customInputs = findCustomInputs();
 
-      expect(customInputs.at(0).props('name')).toBe('label3');
-      expect(customInputs.at(1).props('name')).toBe('label4');
+      expect(customInputs.at(0).props('name')).toBe('customSimple');
+      expect(customInputs.at(1).props('name')).toBe('customAdvanced');
     });
   });
 
@@ -77,7 +71,7 @@ describe('Metrics dashboard/variables section component', () => {
             namespaced: true,
             state: {
               showEmptyState: false,
-              variables: sampleVariables,
+              variables: storeVariables,
             },
             actions: {
               updateVariablesAndFetchData,
@@ -92,12 +86,12 @@ describe('Metrics dashboard/variables section component', () => {
     it('merges the url params and refreshes the dashboard when a text-based variables inputs are updated', () => {
       const firstInput = findTextInputs().at(0);
 
-      firstInput.vm.$emit('onUpdate', 'label1', 'test');
+      firstInput.vm.$emit('input', 'test');
 
       return wrapper.vm.$nextTick(() => {
         expect(updateVariablesAndFetchData).toHaveBeenCalled();
         expect(mergeUrlParams).toHaveBeenCalledWith(
-          convertVariablesForURL(sampleVariables),
+          convertVariablesForURL(storeVariables),
           window.location.href,
         );
         expect(updateHistory).toHaveBeenCalled();
@@ -107,12 +101,12 @@ describe('Metrics dashboard/variables section component', () => {
     it('merges the url params and refreshes the dashboard when a custom-based variables inputs are updated', () => {
       const firstInput = findCustomInputs().at(0);
 
-      firstInput.vm.$emit('onUpdate', 'label1', 'test');
+      firstInput.vm.$emit('input', 'test');
 
       return wrapper.vm.$nextTick(() => {
         expect(updateVariablesAndFetchData).toHaveBeenCalled();
         expect(mergeUrlParams).toHaveBeenCalledWith(
-          convertVariablesForURL(sampleVariables),
+          convertVariablesForURL(storeVariables),
           window.location.href,
         );
         expect(updateHistory).toHaveBeenCalled();
@@ -122,7 +116,7 @@ describe('Metrics dashboard/variables section component', () => {
     it('does not merge the url params and refreshes the dashboard if the value entered is not different that is what currently stored', () => {
       const firstInput = findTextInputs().at(0);
 
-      firstInput.vm.$emit('onUpdate', 'label1', 'Simple text');
+      firstInput.vm.$emit('input', 'My default value');
 
       expect(updateVariablesAndFetchData).not.toHaveBeenCalled();
       expect(mergeUrlParams).not.toHaveBeenCalled();

spec/frontend/monitoring/store/actions_spec.js

@@ -44,7 +44,6 @@ import {
   deploymentData,
   environmentData,
   annotationsData,
-  mockTemplatingData,
   dashboardGitResponse,
   mockDashboardsErrorResponse,
 } from '../mock_data';
@@ -305,32 +304,6 @@ describe('Monitoring store actions', () => {
       expect(dispatch).toHaveBeenCalledWith('fetchDashboardData');
     });
 
-    it('stores templating variables', () => {
-      const response = {
-        ...metricsDashboardResponse.dashboard,
-        ...mockTemplatingData.allVariableTypes.dashboard,
-      };
-
-      receiveMetricsDashboardSuccess(
-        { state, commit, dispatch },
-        {
-          response: {
-            ...metricsDashboardResponse,
-            dashboard: {
-              ...metricsDashboardResponse.dashboard,
-              ...mockTemplatingData.allVariableTypes.dashboard,
-            },
-          },
-        },
-      );
-
-      expect(commit).toHaveBeenCalledWith(
-        types.RECEIVE_METRICS_DASHBOARD_SUCCESS,
-
-        response,
-      );
-    });
-
     it('sets the dashboards loaded from the repository', () => {
       const params = {};
       const response = metricsDashboardResponse;
@@ -1144,11 +1117,13 @@ describe('Monitoring store actions', () => {
   describe('fetchVariableMetricLabelValues', () => {
     const variable = {
       type: 'metric_label_values',
+      name: 'label1',
       options: {
-        prometheusEndpointPath: '/series',
+        prometheusEndpointPath: '/series?match[]=metric_name',
         label: 'job',
       },
     };
+
     const defaultQueryParams = {
       start_time: '2019-08-06T12:40:02.184Z',
       end_time: '2019-08-06T20:40:02.184Z',
@@ -1158,9 +1133,7 @@ describe('Monitoring store actions', () => {
       state = {
         ...state,
         timeRange: defaultTimeRange,
-        variables: {
-          label1: variable,
-        },
+        variables: [variable],
       };
     });
 
@@ -1176,7 +1149,7 @@ describe('Monitoring store actions', () => {
         },
       ];
 
-      mock.onGet('/series').reply(200, {
+      mock.onGet('/series?match[]=metric_name').reply(200, {
         status: 'success',
         data,
       });
@@ -1196,7 +1169,7 @@ describe('Monitoring store actions', () => {
     });
 
     it('should notify the user that dynamic options were not loaded', () => {
-      mock.onGet('/series').reply(500);
+      mock.onGet('/series?match[]=metric_name').reply(500);
 
       return testAction(fetchVariableMetricLabelValues, { defaultQueryParams }, state, [], []).then(
         () => {

spec/frontend/monitoring/store/getters_spec.js

@@ -8,7 +8,7 @@ import {
   environmentData,
   metricsResult,
   dashboardGitResponse,
-  mockTemplatingDataResponses,
+  storeVariables,
   mockLinks,
 } from '../mock_data';
 import {
@@ -344,19 +344,21 @@ describe('Monitoring store Getters', () => {
     });
 
     it('transforms the variables object to an array in the [variable, variable_value] format for all variable types', () => {
-      mutations[types.SET_VARIABLES](state, mockTemplatingDataResponses.allVariableTypes);
+      state.variables = storeVariables;
       const variablesArray = getters.getCustomVariablesParams(state);
 
       expect(variablesArray).toEqual({
-        'variables[advCustomNormal]': 'value2',
-        'variables[advText]': 'default',
-        'variables[simpleCustom]': 'value1',
-        'variables[simpleText]': 'Simple text',
+        'variables[textSimple]': 'My default value',
+        'variables[textAdvanced]': 'A default value',
+        'variables[customSimple]': 'value1',
+        'variables[customAdvanced]': 'value2',
+        'variables[customAdvancedWithoutLabel]': 'value2',
+        'variables[customAdvancedWithoutOptText]': 'value2',
       });
     });
 
     it('transforms the variables object to an empty array when no keys are present', () => {
-      mutations[types.SET_VARIABLES](state, {});
+      state.variables = [];
       const variablesArray = getters.getCustomVariablesParams(state);
 
       expect(variablesArray).toEqual({});

spec/frontend/monitoring/store/mutations_spec.js

@@ -5,7 +5,7 @@ import * as types from '~/monitoring/stores/mutation_types';
 import state from '~/monitoring/stores/state';
 import { metricStates } from '~/monitoring/constants';
 
-import { deploymentData, dashboardGitResponse } from '../mock_data';
+import { deploymentData, dashboardGitResponse, storeTextVariables } from '../mock_data';
 import { metricsDashboardPayload } from '../fixture_data';
 
 describe('Monitoring mutations', () => {
@@ -427,30 +427,12 @@ describe('Monitoring mutations', () => {
     });
   });
 
-  describe('SET_VARIABLES', () => {
-    it('stores an empty variables array when no custom variables are given', () => {
-      mutations[types.SET_VARIABLES](stateCopy, {});
-
-      expect(stateCopy.variables).toEqual({});
-    });
-
-    it('stores variables in the key key_value format in the array', () => {
-      mutations[types.SET_VARIABLES](stateCopy, { pod: 'POD', stage: 'main ops' });
-
-      expect(stateCopy.variables).toEqual({ pod: 'POD', stage: 'main ops' });
-    });
-  });
-
   describe('UPDATE_VARIABLE_VALUE', () => {
-    afterEach(() => {
-      mutations[types.SET_VARIABLES](stateCopy, {});
-    });
-
     it('updates only the value of the variable in variables', () => {
-      mutations[types.SET_VARIABLES](stateCopy, { environment: { value: 'prod', type: 'text' } });
-      mutations[types.UPDATE_VARIABLE_VALUE](stateCopy, { key: 'environment', value: 'new prod' });
+      stateCopy.variables = storeTextVariables;
+      mutations[types.UPDATE_VARIABLE_VALUE](stateCopy, { name: 'textSimple', value: 'New Value' });
 
-      expect(stateCopy.variables).toEqual({ environment: { value: 'new prod', type: 'text' } });
+      expect(stateCopy.variables[0].value).toEqual('New Value');
     });
   });
 

spec/frontend/monitoring/store/utils_spec.js

@@ -22,7 +22,7 @@ describe('mapToDashboardViewModel', () => {
       dashboard: '',
       panelGroups: [],
       links: [],
-      variables: {},
+      variables: [],
     });
   });
 
@@ -52,7 +52,7 @@ describe('mapToDashboardViewModel', () => {
     expect(mapToDashboardViewModel(response)).toEqual({
       dashboard: 'Dashboard Name',
       links: [],
-      variables: {},
+      variables: [],
       panelGroups: [
         {
           group: 'Group 1',
@@ -424,22 +424,20 @@ describe('mapToDashboardViewModel', () => {
 
       urlUtils.queryToObject.mockReturnValueOnce();
 
-      expect(mapToDashboardViewModel(response)).toMatchObject({
-        dashboard: 'Dashboard Name',
-        links: [],
-        variables: {
-          pod: {
-            label: 'pod',
-            type: 'text',
-            value: 'kubernetes',
-          },
-          pod_2: {
-            label: 'pod_2',
-            type: 'text',
-            value: 'kubernetes-2',
-          },
+      expect(mapToDashboardViewModel(response).variables).toEqual([
+        {
+          name: 'pod',
+          label: 'pod',
+          type: 'text',
+          value: 'kubernetes',
         },
-      });
+        {
+          name: 'pod_2',
+          label: 'pod_2',
+          type: 'text',
+          value: 'kubernetes-2',
+        },
+      ]);
     });
 
     it('sets variables as-is from yml file if URL has no matching variables', () => {
@@ -458,22 +456,20 @@ describe('mapToDashboardViewModel', () => {
         'var-environment': 'POD',
       });
 
-      expect(mapToDashboardViewModel(response)).toMatchObject({
-        dashboard: 'Dashboard Name',
-        links: [],
-        variables: {
-          pod: {
-            label: 'pod',
-            type: 'text',
-            value: 'kubernetes',
-          },
-          pod_2: {
-            label: 'pod_2',
-            type: 'text',
-            value: 'kubernetes-2',
-          },
+      expect(mapToDashboardViewModel(response).variables).toEqual([
+        {
+          label: 'pod',
+          name: 'pod',
+          type: 'text',
+          value: 'kubernetes',
         },
-      });
+        {
+          label: 'pod_2',
+          name: 'pod_2',
+          type: 'text',
+          value: 'kubernetes-2',
+        },
+      ]);
     });
 
     it('merges variables from URL with the ones from yml file', () => {
@@ -494,22 +490,20 @@ describe('mapToDashboardViewModel', () => {
         'var-pod_2': 'POD2',
       });
 
-      expect(mapToDashboardViewModel(response)).toMatchObject({
-        dashboard: 'Dashboard Name',
-        links: [],
-        variables: {
-          pod: {
-            label: 'pod',
-            type: 'text',
-            value: 'POD1',
-          },
-          pod_2: {
-            label: 'pod_2',
-            type: 'text',
-            value: 'POD2',
-          },
+      expect(mapToDashboardViewModel(response).variables).toEqual([
+        {
+          label: 'pod',
+          name: 'pod',
+          type: 'text',
+          value: 'POD1',
         },
-      });
+        {
+          label: 'pod_2',
+          name: 'pod_2',
+          type: 'text',
+          value: 'POD2',
+        },
+      ]);
     });
   });
 });

spec/frontend/monitoring/store/variable_mapping_spec.js

@@ -3,29 +3,31 @@ import {
   mergeURLVariables,
   optionsFromSeriesData,
 } from '~/monitoring/stores/variable_mapping';
+import {
+  templatingVariablesExamples,
+  storeTextVariables,
+  storeCustomVariables,
+  storeMetricLabelValuesVariables,
+} from '../mock_data';
 import * as urlUtils from '~/lib/utils/url_utility';
-import { mockTemplatingData, mockTemplatingDataResponses } from '../mock_data';
 
 describe('Monitoring variable mapping', () => {
   describe('parseTemplatingVariables', () => {
     it.each`
-      case                                                                            | input                                         | expected
-      ${'Returns empty object for no dashboard input'}                                | ${{}}                                         | ${{}}
-      ${'Returns empty object for empty dashboard input'}                             | ${{ dashboard: {} }}                          | ${{}}
-      ${'Returns empty object for empty templating prop'}                             | ${mockTemplatingData.emptyTemplatingProp}     | ${{}}
-      ${'Returns empty object for empty variables prop'}                              | ${mockTemplatingData.emptyVariablesProp}      | ${{}}
-      ${'Returns parsed object for simple text variable'}                             | ${mockTemplatingData.simpleText}              | ${mockTemplatingDataResponses.simpleText}
-      ${'Returns parsed object for advanced text variable'}                           | ${mockTemplatingData.advText}                 | ${mockTemplatingDataResponses.advText}
-      ${'Returns parsed object for simple custom variable'}                           | ${mockTemplatingData.simpleCustom}            | ${mockTemplatingDataResponses.simpleCustom}
-      ${'Returns parsed object for advanced custom variable without options'}         | ${mockTemplatingData.advCustomWithoutOpts}    | ${mockTemplatingDataResponses.advCustomWithoutOpts}
-      ${'Returns parsed object for advanced custom variable for option without text'} | ${mockTemplatingData.advCustomWithoutOptText} | ${mockTemplatingDataResponses.advCustomWithoutOptText}
-      ${'Returns parsed object for advanced custom variable without type'}            | ${mockTemplatingData.advCustomWithoutType}    | ${{}}
-      ${'Returns parsed object for advanced custom variable without label'}           | ${mockTemplatingData.advCustomWithoutLabel}   | ${mockTemplatingDataResponses.advCustomWithoutLabel}
-      ${'Returns parsed object for simple and advanced custom variables'}             | ${mockTemplatingData.simpleAndAdv}            | ${mockTemplatingDataResponses.simpleAndAdv}
-      ${'Returns parsed object for metricLabelValues'}                                | ${mockTemplatingData.metricLabelValues}       | ${mockTemplatingDataResponses.metricLabelValues}
-      ${'Returns parsed object for all variable types'}                               | ${mockTemplatingData.allVariableTypes}        | ${mockTemplatingDataResponses.allVariableTypes}
-    `('$case', ({ input, expected }) => {
-      expect(parseTemplatingVariables(input?.dashboard?.templating)).toEqual(expected);
+      case                                 | input
+      ${'For undefined templating object'} | ${undefined}
+      ${'For empty templating object'}     | ${{}}
+    `('$case, returns an empty array', ({ input }) => {
+      expect(parseTemplatingVariables(input)).toEqual([]);
+    });
+
+    it.each`
+      case                                                        | input                                            | output
+      ${'Returns parsed object for text variables'}               | ${templatingVariablesExamples.text}              | ${storeTextVariables}
+      ${'Returns parsed object for custom variables'}             | ${templatingVariablesExamples.custom}            | ${storeCustomVariables}
+      ${'Returns parsed object for metric label value variables'} | ${templatingVariablesExamples.metricLabelValues} | ${storeMetricLabelValuesVariables}
+    `('$case, returns an empty array', ({ input, output }) => {
+      expect(parseTemplatingVariables(input)).toEqual(output);
     });
   });
 
@@ -41,7 +43,7 @@ describe('Monitoring variable mapping', () => {
     it('returns empty object if variables are not defined in yml or URL', () => {
       urlUtils.queryToObject.mockReturnValueOnce({});
 
-      expect(mergeURLVariables({})).toEqual({});
+      expect(mergeURLVariables([])).toEqual([]);
     });
 
     it('returns empty object if variables are defined in URL but not in yml', () => {
@@ -50,18 +52,24 @@ describe('Monitoring variable mapping', () => {
         'var-instance': 'localhost',
       });
 
-      expect(mergeURLVariables({})).toEqual({});
+      expect(mergeURLVariables([])).toEqual([]);
     });
 
     it('returns yml variables if variables defined in yml but not in the URL', () => {
       urlUtils.queryToObject.mockReturnValueOnce({});
 
-      const params = {
-        env: 'one',
-        instance: 'localhost',
-      };
+      const variables = [
+        {
+          name: 'env',
+          value: 'one',
+        },
+        {
+          name: 'instance',
+          value: 'localhost',
+        },
+      ];
 
-      expect(mergeURLVariables(params)).toEqual(params);
+      expect(mergeURLVariables(variables)).toEqual(variables);
     });
 
     it('returns yml variables if variables defined in URL do not match with yml variables', () => {
@@ -69,13 +77,19 @@ describe('Monitoring variable mapping', () => {
         'var-env': 'one',
         'var-instance': 'localhost',
       };
-      const ymlParams = {
-        pod: { value: 'one' },
-        service: { value: 'database' },
-      };
+      const variables = [
+        {
+          name: 'env',
+          value: 'one',
+        },
+        {
+          name: 'service',
+          value: 'database',
+        },
+      ];
       urlUtils.queryToObject.mockReturnValueOnce(urlParams);
 
-      expect(mergeURLVariables(ymlParams)).toEqual(ymlParams);
+      expect(mergeURLVariables(variables)).toEqual(variables);
     });
 
     it('returns merged yml and URL variables if there is some match', () => {
@@ -83,19 +97,29 @@ describe('Monitoring variable mapping', () => {
         'var-env': 'one',
         'var-instance': 'localhost:8080',
       };
-      const ymlParams = {
-        instance: { value: 'localhost' },
-        service: { value: 'database' },
-      };
-
-      const merged = {
-        instance: { value: 'localhost:8080' },
-        service: { value: 'database' },
-      };
+      const variables = [
+        {
+          name: 'instance',
+          value: 'localhost',
+        },
+        {
+          name: 'service',
+          value: 'database',
+        },
+      ];
 
       urlUtils.queryToObject.mockReturnValueOnce(urlParams);
 
-      expect(mergeURLVariables(ymlParams)).toEqual(merged);
+      expect(mergeURLVariables(variables)).toEqual([
+        {
+          name: 'instance',
+          value: 'localhost:8080',
+        },
+        {
+          name: 'service',
+          value: 'database',
+        },
+      ]);
     });
   });
 

spec/frontend/monitoring/store_utils.js

@@ -35,12 +35,6 @@ export const setupStoreWithDashboard = store => {
   );
 };
 
-export const setupStoreWithVariable = store => {
-  store.commit(`monitoringDashboard/${types.SET_VARIABLES}`, {
-    label1: 'pod',
-  });
-};
-
 export const setupStoreWithLinks = store => {
   store.commit(`monitoringDashboard/${types.RECEIVE_METRICS_DASHBOARD_SUCCESS}`, {
     ...metricsDashboardPayload,

spec/frontend/monitoring/utils_spec.js

@@ -429,14 +429,41 @@ describe('monitoring/utils', () => {
 
   describe('convertVariablesForURL', () => {
     it.each`
-      input                               | expected
-      ${undefined}                        | ${{}}
-      ${null}                             | ${{}}
-      ${{}}                               | ${{}}
-      ${{ env: { value: 'prod' } }}       | ${{ 'var-env': 'prod' }}
-      ${{ 'var-env': { value: 'prod' } }} | ${{ 'var-var-env': 'prod' }}
+      input                                                               | expected
+      ${[]}                                                               | ${{}}
+      ${[{ name: 'env', value: 'prod' }]}                                 | ${{ 'var-env': 'prod' }}
+      ${[{ name: 'env1', value: 'prod' }, { name: 'env2', value: null }]} | ${{ 'var-env1': 'prod' }}
+      ${[{ name: 'var-env', value: 'prod' }]}                             | ${{ 'var-var-env': 'prod' }}
     `('convertVariablesForURL returns $expected with input $input', ({ input, expected }) => {
       expect(monitoringUtils.convertVariablesForURL(input)).toEqual(expected);
     });
   });
+
+  describe('setCustomVariablesFromUrl', () => {
+    beforeEach(() => {
+      jest.spyOn(urlUtils, 'updateHistory');
+    });
+
+    afterEach(() => {
+      urlUtils.updateHistory.mockRestore();
+    });
+
+    it.each`
+      input                                                               | urlParams
+      ${[]}                                                               | ${''}
+      ${[{ name: 'env', value: 'prod' }]}                                 | ${'?var-env=prod'}
+      ${[{ name: 'env1', value: 'prod' }, { name: 'env2', value: null }]} | ${'?var-env=prod&var-env1=prod'}
+    `(
+      'setCustomVariablesFromUrl updates history with query "$urlParams" with input $input',
+      ({ input, urlParams }) => {
+        monitoringUtils.setCustomVariablesFromUrl(input);
+
+        expect(urlUtils.updateHistory).toHaveBeenCalledTimes(1);
+        expect(urlUtils.updateHistory).toHaveBeenCalledWith({
+          url: `http://localhost/${urlParams}`,
+          title: '',
+        });
+      },
+    );
+  });
 });