Merge branch 'dbodicherla-update-urls-params-only-update-existing-variables' into 'master'

URL params in monitoring sets existing variables

See merge request gitlab-org/gitlab!31662

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

@@ -12,14 +12,14 @@ export default {
     ...mapState('monitoringDashboard', ['promVariables']),
   },
   methods: {
-    ...mapActions('monitoringDashboard', ['fetchDashboardData', 'setVariableData']),
+    ...mapActions('monitoringDashboard', ['fetchDashboardData', 'setVariableValues']),
     refreshDashboard(event) {
       const { name, value } = event.target;
 
       if (this.promVariables[name] !== value) {
         const changedVariable = { [name]: value };
 
-        this.setVariableData(changedVariable);
+        this.setVariableValues(changedVariable);
 
         updateHistory({
           url: mergeUrlParams(this.promVariables, window.location.href),

app/assets/javascripts/monitoring/monitoring_bundle.js

@@ -14,7 +14,7 @@ export default (props = {}) => {
   if (el && el.dataset) {
     const [currentDashboard] = getParameterValues('dashboard');
 
-    store.dispatch('monitoringDashboard/setVariables', promCustomVariablesFromUrl());
+    store.dispatch('monitoringDashboard/setVariableValues', promCustomVariablesFromUrl());
 
     // eslint-disable-next-line no-new
     new Vue({

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

@@ -77,7 +77,7 @@ export const setTimeRange = ({ commit }, timeRange) => {
 };
 
 export const setVariables = ({ commit }, variables) => {
-  commit(types.SET_PROM_QUERY_VARIABLES, variables);
+  commit(types.SET_VARIABLES, variables);
 };
 
 export const filterEnvironments = ({ commit, dispatch }, searchTerm) => {
@@ -413,8 +413,8 @@ export const duplicateSystemDashboard = ({ state }, payload) => {
 
 // Variables manipulation
 
-export const setVariableData = ({ commit }, updatedVariable) => {
-  commit(types.UPDATE_VARIABLE_DATA, updatedVariable);
+export const setVariableValues = ({ commit }, updatedVariable) => {
+  commit(types.UPDATE_VARIABLE_VALUES, updatedVariable);
 };
 
 // prevent babel-plugin-rewire from generating an invalid default during karma tests

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

@@ -2,8 +2,8 @@
 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_PROM_QUERY_VARIABLES = 'SET_PROM_QUERY_VARIABLES';
-export const UPDATE_VARIABLE_DATA = 'UPDATE_VARIABLE_DATA';
+export const SET_VARIABLES = 'SET_VARIABLES';
+export const UPDATE_VARIABLE_VALUES = 'UPDATE_VARIABLE_VALUES';
 
 export const REQUEST_DASHBOARD_STARRING = 'REQUEST_DASHBOARD_STARRING';
 export const RECEIVE_DASHBOARD_STARRING_SUCCESS = 'RECEIVE_DASHBOARD_STARRING_SUCCESS';

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

@@ -188,13 +188,14 @@ export default {
     state.expandedPanel.group = group;
     state.expandedPanel.panel = panel;
   },
-  [types.SET_PROM_QUERY_VARIABLES](state, variables) {
+  [types.SET_VARIABLES](state, variables) {
     state.promVariables = variables;
   },
-  [types.UPDATE_VARIABLE_DATA](state, newVariable) {
-    Object.assign(state.promVariables, {
-      ...state.promVariables,
-      ...newVariable,
+  [types.UPDATE_VARIABLE_VALUES](state, newVariable) {
+    Object.keys(newVariable).forEach(key => {
+      if (Object.prototype.hasOwnProperty.call(state.promVariables, key)) {
+        state.promVariables[key] = newVariable[key];
+      }
     });
   },
 };

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

@@ -244,7 +244,10 @@ export const addPrefixToLabels = label => `${VARIABLE_PREFIX}${label}`;
  * Before the templating variables are passed to the backend the
  * prefix needs to be removed.
  *
+ * This method removes the prefix at the beginning of the string.
+ *
  * @param {String} label label to remove prefix from
  * @returns {String}
  */
-export const removePrefixFromLabels = label => label.replace(VARIABLE_PREFIX, '');
+export const removePrefixFromLabels = label =>
+  (label || '').replace(new RegExp(`^${VARIABLE_PREFIX}`), '');

changelogs/unreleased/dbodicherla-update-urls-params-only-update-existing-variables.yml

+---
+title: URL params in the monitoring dashboard update variable values defined in yml
+  file
+merge_request: 31662
+author:
+type: changed

spec/frontend/monitoring/components/variables_section_spec.js

@@ -43,10 +43,7 @@ describe('Metrics dashboard/variables section component', () => {
 
   it('shows the variables section', () => {
     createShallowWrapper();
-    wrapper.vm.$store.commit(
-      `monitoringDashboard/${types.SET_PROM_QUERY_VARIABLES}`,
-      sampleVariables,
-    );
+    wrapper.vm.$store.commit(`monitoringDashboard/${types.SET_VARIABLES}`, sampleVariables);
 
     return wrapper.vm.$nextTick(() => {
       const allInputs = findAllFormInputs();
@@ -57,7 +54,7 @@ describe('Metrics dashboard/variables section component', () => {
 
   describe('when changing the variable inputs', () => {
     const fetchDashboardData = jest.fn();
-    const setVariableData = jest.fn();
+    const setVariableValues = jest.fn();
 
     beforeEach(() => {
       store = new Vuex.Store({
@@ -70,7 +67,7 @@ describe('Metrics dashboard/variables section component', () => {
             },
             actions: {
               fetchDashboardData,
-              setVariableData,
+              setVariableValues,
             },
           },
         },
@@ -86,7 +83,7 @@ describe('Metrics dashboard/variables section component', () => {
       firstInput.vm.$emit('input');
       firstInput.trigger('blur');
 
-      expect(setVariableData).toHaveBeenCalled();
+      expect(setVariableValues).toHaveBeenCalled();
       expect(mergeUrlParams).toHaveBeenCalledWith(sampleVariables, window.location.href);
       expect(updateHistory).toHaveBeenCalled();
       expect(fetchDashboardData).toHaveBeenCalled();
@@ -99,7 +96,7 @@ describe('Metrics dashboard/variables section component', () => {
       firstInput.vm.$emit('input');
       firstInput.trigger('keyup.enter');
 
-      expect(setVariableData).toHaveBeenCalled();
+      expect(setVariableValues).toHaveBeenCalled();
       expect(mergeUrlParams).toHaveBeenCalledWith(sampleVariables, window.location.href);
       expect(updateHistory).toHaveBeenCalled();
       expect(fetchDashboardData).toHaveBeenCalled();
@@ -111,7 +108,7 @@ describe('Metrics dashboard/variables section component', () => {
       firstInput.vm.$emit('input');
       firstInput.trigger('keyup.enter');
 
-      expect(setVariableData).not.toHaveBeenCalled();
+      expect(setVariableValues).not.toHaveBeenCalled();
       expect(mergeUrlParams).not.toHaveBeenCalled();
       expect(updateHistory).not.toHaveBeenCalled();
       expect(fetchDashboardData).not.toHaveBeenCalled();

spec/frontend/monitoring/store/actions_spec.js

@@ -26,7 +26,7 @@ import {
   clearExpandedPanel,
   setGettingStartedEmptyState,
   duplicateSystemDashboard,
-  setVariables,
+  setVariableValues,
 } from '~/monitoring/stores/actions';
 import {
   gqClient,
@@ -442,19 +442,19 @@ describe('Monitoring store actions', () => {
     });
   });
 
-  describe('setVariables', () => {
+  describe('setVariableValues', () => {
     let mockedState;
     beforeEach(() => {
       mockedState = storeState();
     });
-    it('should commit SET_PROM_QUERY_VARIABLES mutation', done => {
+    it('should commit UPDATE_VARIABLE_VALUES mutation', done => {
       testAction(
-        setVariables,
+        setVariableValues,
         { pod: 'POD' },
         mockedState,
         [
           {
-            type: types.SET_PROM_QUERY_VARIABLES,
+            type: types.UPDATE_VARIABLE_VALUES,
             payload: { pod: 'POD' },
           },
         ],

spec/frontend/monitoring/store/getters_spec.js

@@ -338,14 +338,14 @@ describe('Monitoring store Getters', () => {
     });
 
     it('transforms the promVariables object to an array in the [variable, variable_value] format', () => {
-      mutations[types.SET_PROM_QUERY_VARIABLES](state, sampleVariables);
+      mutations[types.SET_VARIABLES](state, sampleVariables);
       const variablesArray = getters.getCustomVariablesArray(state);
 
       expect(variablesArray).toEqual(['label1', 'pod', 'label2', 'env']);
     });
 
     it('transforms the promVariables object to an empty array when no keys are present', () => {
-      mutations[types.SET_PROM_QUERY_VARIABLES](state, {});
+      mutations[types.SET_VARIABLES](state, {});
       const variablesArray = getters.getCustomVariablesArray(state);
 
       expect(variablesArray).toEqual([]);

spec/frontend/monitoring/store/mutations_spec.js

@@ -408,27 +408,35 @@ describe('Monitoring mutations', () => {
     });
   });
 
-  describe('SET_PROM_QUERY_VARIABLES', () => {
+  describe('SET_VARIABLES', () => {
     it('stores an empty variables array when no custom variables are given', () => {
-      mutations[types.SET_PROM_QUERY_VARIABLES](stateCopy, {});
+      mutations[types.SET_VARIABLES](stateCopy, {});
 
       expect(stateCopy.promVariables).toEqual({});
     });
 
     it('stores variables in the key key_value format in the array', () => {
-      mutations[types.SET_PROM_QUERY_VARIABLES](stateCopy, { pod: 'POD', stage: 'main ops' });
+      mutations[types.SET_VARIABLES](stateCopy, { pod: 'POD', stage: 'main ops' });
 
       expect(stateCopy.promVariables).toEqual({ pod: 'POD', stage: 'main ops' });
     });
   });
 
-  describe('UPDATE_VARIABLE_DATA', () => {
-    beforeEach(() => {
-      mutations[types.SET_PROM_QUERY_VARIABLES](stateCopy, { pod: 'POD' });
+  describe('UPDATE_VARIABLE_VALUES', () => {
+    afterEach(() => {
+      mutations[types.SET_VARIABLES](stateCopy, {});
+    });
+
+    it('ignores updates that are not already in promVariables', () => {
+      mutations[types.SET_VARIABLES](stateCopy, { environment: 'prod' });
+      mutations[types.UPDATE_VARIABLE_VALUES](stateCopy, { pod: 'new pod' });
+
+      expect(stateCopy.promVariables).toEqual({ environment: 'prod' });
     });
 
-    it('sets a new value for an existing key', () => {
-      mutations[types.UPDATE_VARIABLE_DATA](stateCopy, { pod: 'new pod' });
+    it('only updates existing variables', () => {
+      mutations[types.SET_VARIABLES](stateCopy, { pod: 'POD' });
+      mutations[types.UPDATE_VARIABLE_VALUES](stateCopy, { pod: 'new pod' });
 
       expect(stateCopy.promVariables).toEqual({ pod: 'new pod' });
     });

spec/frontend/monitoring/store/utils_spec.js

@@ -5,6 +5,7 @@ import {
   parseAnnotationsResponse,
   removeLeadingSlash,
   mapToDashboardViewModel,
+  removePrefixFromLabels,
 } from '~/monitoring/stores/utils';
 import { annotationsData } from '../mock_data';
 import { NOT_IN_DB_PREFIX } from '~/monitoring/constants';
@@ -419,3 +420,24 @@ describe('removeLeadingSlash', () => {
     });
   });
 });
+
+describe('removePrefixFromLabels', () => {
+  it.each`
+    input               | expected
+    ${undefined}        | ${''}
+    ${null}             | ${''}
+    ${''}               | ${''}
+    ${'    '}           | ${'    '}
+    ${'pod-1'}          | ${'pod-1'}
+    ${'pod-var-1'}      | ${'pod-var-1'}
+    ${'pod-1-var'}      | ${'pod-1-var'}
+    ${'podvar--1'}      | ${'podvar--1'}
+    ${'povar-d-1'}      | ${'povar-d-1'}
+    ${'var-pod-1'}      | ${'pod-1'}
+    ${'var-var-pod-1'}  | ${'var-pod-1'}
+    ${'varvar-pod-1'}   | ${'varvar-pod-1'}
+    ${'var-pod-1-var-'} | ${'pod-1-var-'}
+  `('removePrefixFromLabels returns $expected with input $input', ({ input, expected }) => {
+    expect(removePrefixFromLabels(input)).toEqual(expected);
+  });
+});

spec/frontend/monitoring/store_utils.js

@@ -24,7 +24,7 @@ export const setupStoreWithDashboard = $store => {
 };
 
 export const setupStoreWithVariable = $store => {
-  $store.commit(`monitoringDashboard/${types.SET_PROM_QUERY_VARIABLES}`, {
+  $store.commit(`monitoringDashboard/${types.SET_VARIABLES}`, {
     label1: 'pod',
   });
 };