Merge branch...

Merge branch '32888-productivity-analytics-days_to_merge-filtering-option-doesn-t-reset' into 'master'

Resolve "Productivity Analytics "days_to_merge" filtering option doesn't reset"

Closes #32888

See merge request gitlab-org/gitlab!17810

ee/app/assets/javascripts/analytics/productivity_analytics/components/app.vue

@@ -102,13 +102,13 @@ export default {
     ...mapActions('charts', [
       'fetchChartData',
       'setMetricType',
-      'chartItemClicked',
+      'updateSelectedItems',
       'setChartEnabled',
     ]),
     ...mapActions('table', ['setSortField', 'setPage', 'toggleSortOrder', 'setColumnMetric']),
     onMainChartItemClicked({ params }) {
       const itemValue = params.data.value[0];
-      this.chartItemClicked({ chartKey: this.chartKeys.main, item: itemValue });
+      this.updateSelectedItems({ chartKey: this.chartKeys.main, item: itemValue });
     },
     getColumnChartOption(chartKey) {
       return {

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/actions.js

@@ -50,14 +50,19 @@ export const setMetricType = ({ commit, dispatch }, { chartKey, metricType }) =>
   dispatch('fetchChartData', chartKey);
 };
 
-export const chartItemClicked = ({ commit, dispatch }, { chartKey, item }) => {
+export const updateSelectedItems = (
+  { commit, dispatch },
+  { chartKey, item, skipReload = false },
+) => {
   commit(types.UPDATE_SELECTED_CHART_ITEMS, { chartKey, item });
 
-  // update secondary charts
-  dispatch('fetchSecondaryChartData');
+  if (!skipReload) {
+    // update secondary charts
+    dispatch('fetchSecondaryChartData');
 
-  // let's reset the page on the MR table and fetch data
-  dispatch('table/setPage', 0, { root: true });
+    // let's reset the page on the MR table and fetch data
+    dispatch('table/setPage', 0, { root: true });
+  }
 };
 
 export const setChartEnabled = ({ commit }, { chartKey, isEnabled }) =>

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutations.js

@@ -22,11 +22,15 @@ export default {
     state.charts[chartKey].params.metricType = metricType;
   },
   [types.UPDATE_SELECTED_CHART_ITEMS](state, { chartKey, item }) {
-    const idx = state.charts[chartKey].selected.indexOf(item);
-    if (idx === -1) {
-      state.charts[chartKey].selected.push(item);
+    if (!item) {
+      state.charts[chartKey].selected = [];
     } else {
-      state.charts[chartKey].selected.splice(idx, 1);
+      const idx = state.charts[chartKey].selected.indexOf(item);
+      if (idx === -1) {
+        state.charts[chartKey].selected.push(item);
+      } else {
+        state.charts[chartKey].selected.splice(idx, 1);
+      }
     }
   },
   [types.SET_CHART_ENABLED](state, { chartKey, isEnabled }) {

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/filters/actions.js

@@ -4,6 +4,14 @@ import { chartKeys } from '../../../constants';
 export const setGroupNamespace = ({ commit, dispatch }, groupNamespace) => {
   commit(types.SET_GROUP_NAMESPACE, groupNamespace);
 
+  // let's reset the current selection first
+  // with skipReload=true we avoid data from being fetched here
+  dispatch(
+    'charts/updateSelectedItems',
+    { chartKey: chartKeys.main, item: null, skipReload: true },
+    { root: true },
+  );
+
   // let's fetch the main chart data first to see if the user has access to the selected group
   // if there's no 403, then we fetch all remaining chart data and table data
   return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
@@ -16,6 +24,12 @@ export const setGroupNamespace = ({ commit, dispatch }, groupNamespace) => {
 export const setProjectPath = ({ commit, dispatch }, projectPath) => {
   commit(types.SET_PROJECT_PATH, projectPath);
 
+  dispatch(
+    'charts/updateSelectedItems',
+    { chartKey: chartKeys.main, item: null, skipReload: true },
+    { root: true },
+  );
+
   return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
     dispatch('charts/fetchSecondaryChartData', null, { root: true });
     // let's reset the page on the MR table and fetch data
@@ -26,6 +40,12 @@ export const setProjectPath = ({ commit, dispatch }, projectPath) => {
 export const setPath = ({ commit, dispatch }, path) => {
   commit(types.SET_PATH, path);
 
+  dispatch(
+    'charts/updateSelectedItems',
+    { chartKey: chartKeys.main, item: null, skipReload: true },
+    { root: true },
+  );
+
   return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
     dispatch('charts/fetchSecondaryChartData', null, { root: true });
     // let's reset the page on the MR table and fetch data
@@ -36,6 +56,12 @@ export const setPath = ({ commit, dispatch }, path) => {
 export const setDaysInPast = ({ commit, dispatch }, days) => {
   commit(types.SET_DAYS_IN_PAST, days);
 
+  dispatch(
+    'charts/updateSelectedItems',
+    { chartKey: chartKeys.main, item: null, skipReload: true },
+    { root: true },
+  );
+
   return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
     dispatch('charts/fetchSecondaryChartData', null, { root: true });
     // let's reset the page on the MR table and fetch data

ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js

@@ -25,7 +25,7 @@ describe('ProductivityApp component', () => {
   };
 
   const actionSpies = {
-    chartItemClicked: jest.fn(),
+    updateSelectedItems: jest.fn(),
     setSortField: jest.fn(),
     setPage: jest.fn(),
     toggleSortOrder: jest.fn(),
@@ -170,8 +170,8 @@ describe('ProductivityApp component', () => {
                   .vm.$emit('chartItemClicked', data);
               });
 
-              it('dispatches chartItemClicked action', () => {
-                expect(actionSpies.chartItemClicked).toHaveBeenCalledWith({
+              it('dispatches updateSelectedItems action', () => {
+                expect(actionSpies.updateSelectedItems).toHaveBeenCalledWith({
                   chartKey: chartKeys.main,
                   item: 0,
                 });

ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/actions_spec.js

@@ -13,7 +13,7 @@ describe('Productivity analytics chart actions', () => {
   let mockedState;
   let mock;
 
-  const chartKey = 'main';
+  const chartKey = chartKeys.main;
   const globalParams = {
     group_id: 'gitlab-org',
     project_id: 'gitlab-org/gitlab-test',
@@ -212,17 +212,37 @@ describe('Productivity analytics chart actions', () => {
     });
   });
 
-  describe('chartItemClicked', () => {
-    const item = 5;
-    it('should commit selected chart item', done => {
-      testAction(
-        actions.chartItemClicked,
-        { chartKey, item },
-        mockedContext.state,
-        [{ type: types.UPDATE_SELECTED_CHART_ITEMS, payload: { chartKey, item } }],
-        [{ type: 'fetchSecondaryChartData' }, { type: 'table/setPage', payload: 0 }],
-        done,
-      );
+  describe('updateSelectedItems', () => {
+    describe('when skipReload is false (by default)', () => {
+      const item = 5;
+      it('should commit selected chart item and dispatch fetchSecondaryChartData and setPage', done => {
+        testAction(
+          actions.updateSelectedItems,
+          { chartKey, item },
+          mockedContext.state,
+          [{ type: types.UPDATE_SELECTED_CHART_ITEMS, payload: { chartKey, item } }],
+          [{ type: 'fetchSecondaryChartData' }, { type: 'table/setPage', payload: 0 }],
+          done,
+        );
+      });
+    });
+
+    describe('when skipReload is true', () => {
+      it('should commit selected chart and it should not dispatch any further actions', done => {
+        testAction(
+          actions.updateSelectedItems,
+          { chartKey, item: null, skipReload: true },
+          mockedContext.state,
+          [
+            {
+              type: types.UPDATE_SELECTED_CHART_ITEMS,
+              payload: { chartKey: chartKeys.main, item: null },
+            },
+          ],
+          [],
+          done,
+        );
+      });
     });
   });
 

ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/mutations_spec.js

@@ -71,6 +71,12 @@ describe('Productivity analytics chart mutations', () => {
     chartKey = chartKeys.timeBasedHistogram;
     const item = 5;
 
+    it('sets the list of selected items to [] when the item is null', () => {
+      mutations[types.UPDATE_SELECTED_CHART_ITEMS](state, { chartKey, item: null });
+
+      expect(state.charts[chartKey].selected).toEqual([]);
+    });
+
     it('adds the item to the list of selected items when not included', () => {
       mutations[types.UPDATE_SELECTED_CHART_ITEMS](state, { chartKey, item });
 

ee/spec/frontend/analytics/productivity_analytics/store/modules/filters/actions_spec.js

@@ -21,18 +21,24 @@ describe('Productivity analytics filter actions', () => {
           expect(store.commit).toHaveBeenCalledWith(types.SET_GROUP_NAMESPACE, groupNamespace);
 
           expect(store.dispatch.mock.calls[0]).toEqual([
+            'charts/updateSelectedItems',
+            { chartKey: chartKeys.main, item: null, skipReload: true },
+            { root: true },
+          ]);
+
+          expect(store.dispatch.mock.calls[1]).toEqual([
             'charts/fetchChartData',
             chartKeys.main,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[1]).toEqual([
+          expect(store.dispatch.mock.calls[2]).toEqual([
             'charts/fetchSecondaryChartData',
             null,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[2]).toEqual(['table/setPage', 0, { root: true }]);
+          expect(store.dispatch.mock.calls[3]).toEqual(['table/setPage', 0, { root: true }]);
         })
         .then(done)
         .catch(done.fail);
@@ -52,18 +58,24 @@ describe('Productivity analytics filter actions', () => {
           expect(store.commit).toHaveBeenCalledWith(types.SET_PROJECT_PATH, projectPath);
 
           expect(store.dispatch.mock.calls[0]).toEqual([
+            'charts/updateSelectedItems',
+            { chartKey: chartKeys.main, item: null, skipReload: true },
+            { root: true },
+          ]);
+
+          expect(store.dispatch.mock.calls[1]).toEqual([
             'charts/fetchChartData',
             chartKeys.main,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[1]).toEqual([
+          expect(store.dispatch.mock.calls[2]).toEqual([
             'charts/fetchSecondaryChartData',
             null,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[2]).toEqual(['table/setPage', 0, { root: true }]);
+          expect(store.dispatch.mock.calls[3]).toEqual(['table/setPage', 0, { root: true }]);
         })
         .then(done)
         .catch(done.fail);
@@ -83,18 +95,24 @@ describe('Productivity analytics filter actions', () => {
           expect(store.commit).toHaveBeenCalledWith(types.SET_PATH, path);
 
           expect(store.dispatch.mock.calls[0]).toEqual([
+            'charts/updateSelectedItems',
+            { chartKey: chartKeys.main, item: null, skipReload: true },
+            { root: true },
+          ]);
+
+          expect(store.dispatch.mock.calls[1]).toEqual([
             'charts/fetchChartData',
             chartKeys.main,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[1]).toEqual([
+          expect(store.dispatch.mock.calls[2]).toEqual([
             'charts/fetchSecondaryChartData',
             null,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[2]).toEqual(['table/setPage', 0, { root: true }]);
+          expect(store.dispatch.mock.calls[3]).toEqual(['table/setPage', 0, { root: true }]);
         })
         .then(done)
         .catch(done.fail);
@@ -114,18 +132,24 @@ describe('Productivity analytics filter actions', () => {
           expect(store.commit).toHaveBeenCalledWith(types.SET_DAYS_IN_PAST, daysInPast);
 
           expect(store.dispatch.mock.calls[0]).toEqual([
+            'charts/updateSelectedItems',
+            { chartKey: chartKeys.main, item: null, skipReload: true },
+            { root: true },
+          ]);
+
+          expect(store.dispatch.mock.calls[1]).toEqual([
             'charts/fetchChartData',
             chartKeys.main,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[1]).toEqual([
+          expect(store.dispatch.mock.calls[2]).toEqual([
             'charts/fetchSecondaryChartData',
             null,
             { root: true },
           ]);
 
-          expect(store.dispatch.mock.calls[2]).toEqual(['table/setPage', 0, { root: true }]);
+          expect(store.dispatch.mock.calls[3]).toEqual(['table/setPage', 0, { root: true }]);
         })
         .then(done)
         .catch(done.fail);