Merge branch 'dbodicherla-change-fetching-dashboard-logic-for-variables' into 'master'

Update data fetch strategy when variable updates

See merge request gitlab-org/gitlab!32720

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

@@ -13,20 +13,18 @@ export default {
     ...mapState('monitoringDashboard', ['variables']),
   },
   methods: {
-    ...mapActions('monitoringDashboard', ['fetchDashboardData', 'updateVariableValues']),
+    ...mapActions('monitoringDashboard', ['updateVariablesAndFetchData']),
     refreshDashboard(variable, value) {
       if (this.variables[variable].value !== value) {
         const changedVariable = { key: variable, value };
         // update the Vuex store
-        this.updateVariableValues(changedVariable);
+        this.updateVariablesAndFetchData(changedVariable);
         // the below calls can ideally be moved out of the
         // component and into the actions and let the
         // mutation respond directly.
         // This can be further investigate in
         // https://gitlab.com/gitlab-org/gitlab/-/issues/217713
         setCustomVariablesFromUrl(this.variables);
-        // fetch data
-        this.fetchDashboardData();
       }
     },
     variableComponent(type) {

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

@@ -415,8 +415,10 @@ export const duplicateSystemDashboard = ({ state }, payload) => {
 
 // Variables manipulation
 
-export const updateVariableValues = ({ commit }, updatedVariable) => {
-  commit(types.UPDATE_VARIABLE_VALUES, updatedVariable);
+export const updateVariablesAndFetchData = ({ commit, dispatch }, updatedVariable) => {
+  commit(types.UPDATE_VARIABLES, updatedVariable);
+
+  return dispatch('fetchDashboardData');
 };
 
 // prevent babel-plugin-rewire from generating an invalid default during karma tests

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

@@ -3,7 +3,7 @@ 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_VALUES = 'UPDATE_VARIABLE_VALUES';
+export const UPDATE_VARIABLES = 'UPDATE_VARIABLES';
 
 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

@@ -191,7 +191,7 @@ export default {
   [types.SET_VARIABLES](state, variables) {
     state.variables = variables;
   },
-  [types.UPDATE_VARIABLE_VALUES](state, updatedVariable) {
+  [types.UPDATE_VARIABLES](state, updatedVariable) {
     Object.assign(state.variables[updatedVariable.key], {
       ...state.variables[updatedVariable.key],
       value: updatedVariable.value,

spec/frontend/monitoring/components/variables_section_spec.js

@@ -57,8 +57,7 @@ describe('Metrics dashboard/variables section component', () => {
   });
 
   describe('when changing the variable inputs', () => {
-    const fetchDashboardData = jest.fn();
-    const updateVariableValues = jest.fn();
+    const updateVariablesAndFetchData = jest.fn();
 
     beforeEach(() => {
       store = new Vuex.Store({
@@ -70,8 +69,7 @@ describe('Metrics dashboard/variables section component', () => {
               variables: sampleVariables,
             },
             actions: {
-              fetchDashboardData,
-              updateVariableValues,
+              updateVariablesAndFetchData,
             },
           },
         },
@@ -86,13 +84,12 @@ describe('Metrics dashboard/variables section component', () => {
       firstInput.vm.$emit('onUpdate', 'label1', 'test');
 
       return wrapper.vm.$nextTick(() => {
-        expect(updateVariableValues).toHaveBeenCalled();
+        expect(updateVariablesAndFetchData).toHaveBeenCalled();
         expect(mergeUrlParams).toHaveBeenCalledWith(
           convertVariablesForURL(sampleVariables),
           window.location.href,
         );
         expect(updateHistory).toHaveBeenCalled();
-        expect(fetchDashboardData).toHaveBeenCalled();
       });
     });
 
@@ -102,13 +99,12 @@ describe('Metrics dashboard/variables section component', () => {
       firstInput.vm.$emit('onUpdate', 'label1', 'test');
 
       return wrapper.vm.$nextTick(() => {
-        expect(updateVariableValues).toHaveBeenCalled();
+        expect(updateVariablesAndFetchData).toHaveBeenCalled();
         expect(mergeUrlParams).toHaveBeenCalledWith(
           convertVariablesForURL(sampleVariables),
           window.location.href,
         );
         expect(updateHistory).toHaveBeenCalled();
-        expect(fetchDashboardData).toHaveBeenCalled();
       });
     });
 
@@ -117,10 +113,9 @@ describe('Metrics dashboard/variables section component', () => {
 
       firstInput.vm.$emit('onUpdate', 'label1', 'Simple text');
 
-      expect(updateVariableValues).not.toHaveBeenCalled();
+      expect(updateVariablesAndFetchData).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,
-  updateVariableValues,
+  updateVariablesAndFetchData,
 } from '~/monitoring/stores/actions';
 import {
   gqClient,
@@ -417,19 +417,23 @@ describe('Monitoring store actions', () => {
     });
   });
 
-  describe('updateVariableValues', () => {
-    it('should commit UPDATE_VARIABLE_VALUES mutation', done => {
+  describe('updateVariablesAndFetchData', () => {
+    it('should commit UPDATE_VARIABLES mutation and fetch data', done => {
       testAction(
-        updateVariableValues,
+        updateVariablesAndFetchData,
         { pod: 'POD' },
         state,
         [
           {
-            type: types.UPDATE_VARIABLE_VALUES,
+            type: types.UPDATE_VARIABLES,
             payload: { pod: 'POD' },
           },
         ],
-        [],
+        [
+          {
+            type: 'fetchDashboardData',
+          },
+        ],
         done,
       );
     });

spec/frontend/monitoring/store/mutations_spec.js

@@ -418,14 +418,14 @@ describe('Monitoring mutations', () => {
     });
   });
 
-  describe('UPDATE_VARIABLE_VALUES', () => {
+  describe('UPDATE_VARIABLES', () => {
     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_VALUES](stateCopy, { key: 'environment', value: 'new prod' });
+      mutations[types.UPDATE_VARIABLES](stateCopy, { key: 'environment', value: 'new prod' });
 
       expect(stateCopy.variables).toEqual({ environment: { value: 'new prod', type: 'text' } });
     });