Simplify variable manipulation by using arrays

Variables are represented by users in the backend by using hashes
of objects to represent unique fields in their dashboards.

However, this can lead to difficulties when creating mock data and
constructing flexible data structures.

This change addresses this issue by using a simpler data structure for
variables based on arrays.

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: '',
+        });
+      },
+    );
+  });
 });