Merge branch...

Merge branch '225660-vsa-median-data-is-being-requested-unnecessarily-for-hidden-stages' into 'master'

Median data is being requested unnecessarily for hidden stages

See merge request gitlab-org/gitlab!37336

ee/app/assets/javascripts/analytics/cycle_analytics/store/actions.js

@@ -71,10 +71,9 @@ const fetchStageMedian = (currentGroupPath, stageId, params) =>
     ...data,
   }));
 
-export const fetchStageMedianValues = ({ state, dispatch, getters }) => {
-  const { currentGroupPath, cycleAnalyticsRequestParams } = getters;
-  const { stages } = state;
-  const stageIds = stages.map(s => s.slug);
+export const fetchStageMedianValues = ({ dispatch, getters }) => {
+  const { currentGroupPath, cycleAnalyticsRequestParams, activeStages } = getters;
+  const stageIds = activeStages.map(s => s.slug);
 
   dispatch('requestStageMedianValues');
   return Promise.all(

ee/app/assets/javascripts/analytics/cycle_analytics/store/modules/duration_chart/actions.js

@@ -20,27 +20,23 @@ export const receiveDurationDataError = ({ commit }) => {
   createFlash(__('There was an error while fetching value stream analytics duration data.'));
 };
 
-export const fetchDurationData = ({ dispatch, rootGetters, rootState }) => {
+export const fetchDurationData = ({ dispatch, rootGetters }) => {
   dispatch('requestDurationData');
 
-  const {
-    stages,
-    selectedGroup: { fullPath },
-  } = rootState;
-
-  const { cycleAnalyticsRequestParams } = rootGetters;
-
+  const { cycleAnalyticsRequestParams, activeStages, currentGroupPath } = rootGetters;
   return Promise.all(
-    stages.map(stage => {
+    activeStages.map(stage => {
       const { slug } = stage;
 
-      return Api.cycleAnalyticsDurationChart(fullPath, slug, cycleAnalyticsRequestParams).then(
-        ({ data }) => ({
-          slug,
-          selected: true,
-          data,
-        }),
-      );
+      return Api.cycleAnalyticsDurationChart(
+        currentGroupPath,
+        slug,
+        cycleAnalyticsRequestParams,
+      ).then(({ data }) => ({
+        slug,
+        selected: true,
+        data,
+      }));
     }),
   )
     .then(data => dispatch('receiveDurationDataSuccess', data))
@@ -56,23 +52,18 @@ export const receiveDurationMedianDataError = ({ commit }) => {
 };
 
 export const fetchDurationMedianData = ({ dispatch, rootState, rootGetters }) => {
-  const {
-    stages,
-    selectedGroup: { fullPath },
-    startDate,
-    endDate,
-  } = rootState;
-  const { cycleAnalyticsRequestParams } = rootGetters;
+  const { startDate, endDate } = rootState;
+  const { cycleAnalyticsRequestParams, activeStages, currentGroupPath } = rootGetters;
 
   const offsetValue = getDayDifference(new Date(startDate), new Date(endDate));
   const offsetCreatedAfter = getDateInPast(new Date(startDate), offsetValue);
   const offsetCreatedBefore = getDateInPast(new Date(endDate), offsetValue);
 
   return Promise.all(
-    stages.map(stage => {
+    activeStages.map(stage => {
       const { slug } = stage;
 
-      return Api.cycleAnalyticsDurationChart(fullPath, slug, {
+      return Api.cycleAnalyticsDurationChart(currentGroupPath, slug, {
         ...cycleAnalyticsRequestParams,
         created_after: dateFormat(offsetCreatedAfter, dateFormats.isoDate),
         created_before: dateFormat(offsetCreatedBefore, dateFormats.isoDate),

ee/changelogs/unreleased/225660-vsa-median-data-is-being-requested-unnecessarily-for-hidden-sta.yml

+---
+title: VSA fetch duration data for active stages only
+merge_request: 37336
+author:
+type: fixed

ee/spec/frontend/analytics/cycle_analytics/store/actions_spec.js

@@ -19,7 +19,12 @@ import {
 const stageData = { events: [] };
 const error = new Error(`Request failed with status code ${httpStatusCodes.NOT_FOUND}`);
 const flashErrorMessage = 'There was an error while fetching value stream analytics data.';
-const [selectedStage] = stages;
+
+stages[0].hidden = true;
+const activeStages = stages.filter(({ hidden }) => !hidden);
+const hiddenStage = stages[0];
+
+const [selectedStage] = activeStages;
 const selectedStageSlug = selectedStage.slug;
 const [selectedValueStream] = valueStreams;
 
@@ -46,6 +51,7 @@ describe('Cycle analytics actions', () => {
         hasDurationChart: true,
         hasDurationChartMedian: true,
       },
+      activeStages,
     };
     mock = new MockAdapter(axios);
   });
@@ -266,7 +272,10 @@ describe('Cycle analytics actions', () => {
             .mockImplementation(actions.receiveStageMedianValuesError({ commit: () => {} })),
           commit: () => {},
           state: { ...state },
-          getters,
+          getters: {
+            ...getters,
+            activeStages,
+          },
         }),
       });
 
@@ -378,9 +387,7 @@ describe('Cycle analytics actions', () => {
       return testAction(
         actions.setDefaultSelectedStage,
         null,
-        {
-          activeStages: stages,
-        },
+        state,
         [],
         [
           { type: 'setSelectedStage', payload: selectedStage },
@@ -399,13 +406,10 @@ describe('Cycle analytics actions', () => {
     });
 
     it('will select the first active stage', () => {
-      stages[0].hidden = true;
       return testAction(
         actions.setDefaultSelectedStage,
         null,
-        {
-          activeStages: getters.activeStages({ stages }),
-        },
+        state,
         [],
         [
           { type: 'setSelectedStage', payload: stages[1] },
@@ -648,26 +652,44 @@ describe('Cycle analytics actions', () => {
 
   describe('fetchStageMedianValues', () => {
     let mockDispatch = jest.fn();
+    const fetchMedianResponse = activeStages.map(({ slug: id }) => ({ events: [], id }));
+
     beforeEach(() => {
-      state = { ...state, stages: [{ slug: selectedStageSlug }], selectedGroup };
+      state = { ...state, stages, selectedGroup };
       mock = new MockAdapter(axios);
       mock.onGet(endpoints.stageMedian).reply(httpStatusCodes.OK, { events: [] });
       mockDispatch = jest.fn();
     });
 
     it('dispatches receiveStageMedianValuesSuccess with received data on success', () => {
+      return testAction(
+        actions.fetchStageMedianValues,
+        null,
+        state,
+        [],
+        [
+          { type: 'requestStageMedianValues' },
+          { type: 'receiveStageMedianValuesSuccess', payload: fetchMedianResponse },
+        ],
+      );
+    });
+
+    it('does not request hidden stages', () => {
       return actions
         .fetchStageMedianValues({
           state,
-          getters,
+          getters: {
+            ...getters,
+            activeStages,
+          },
           commit: () => {},
           dispatch: mockDispatch,
         })
         .then(() => {
-          expect(mockDispatch).toHaveBeenCalledWith('requestStageMedianValues');
-          expect(mockDispatch).toHaveBeenCalledWith('receiveStageMedianValuesSuccess', [
-            { events: [], id: selectedStageSlug },
-          ]);
+          expect(mockDispatch).not.toHaveBeenCalledWith('receiveStageMedianValuesSuccess', {
+            events: [],
+            id: hiddenStage.id,
+          });
         });
     });
 
@@ -677,17 +699,16 @@ describe('Cycle analytics actions', () => {
       });
 
       it('will dispatch receiveStageMedianValuesError', () => {
-        return actions
-          .fetchStageMedianValues({
-            state,
-            getters,
-            commit: () => {},
-            dispatch: mockDispatch,
-          })
-          .then(() => {
-            expect(mockDispatch).toHaveBeenCalledWith('requestStageMedianValues');
-            expect(mockDispatch).toHaveBeenCalledWith('receiveStageMedianValuesError', error);
-          });
+        return testAction(
+          actions.fetchStageMedianValues,
+          null,
+          state,
+          [],
+          [
+            { type: 'requestStageMedianValues' },
+            { type: 'receiveStageMedianValuesError', payload: error },
+          ],
+        );
       });
     });
   });

ee/spec/frontend/analytics/cycle_analytics/store/modules/duration_chart/actions_spec.js

@@ -20,16 +20,23 @@ import { shouldFlashAMessage } from '../../../helpers';
 
 const selectedGroup = { fullPath: group.path };
 const [stage1, stage2] = stages;
+const hiddenStage = { ...stage1, hidden: true, id: 3, slug: 3 };
+const activeStages = [stage1, stage2];
 
 const rootState = {
   startDate,
   endDate,
-  stages: [stage1, stage2],
+  stages: [...activeStages, hiddenStage],
   selectedGroup,
   featureFlags: {
     hasDurationChart: true,
     hasDurationChartMedian: true,
   },
+  getters,
+  rootGetters: {
+    ...rootGetters,
+    activeStages,
+  },
 };
 
 describe('DurationChart actions', () => {
@@ -49,59 +56,54 @@ describe('DurationChart actions', () => {
       mock.onGet(endpoints.durationData).reply(200, [...rawDurationData]);
     });
 
-    it("dispatches the 'receiveDurationDataSuccess' action on success", () => {
-      const dispatch = jest.fn();
-
-      return actions
-        .fetchDurationData({
-          dispatch,
-          rootState,
-          rootGetters,
-        })
-        .then(() => {
-          expect(dispatch).toHaveBeenCalledWith(
-            'receiveDurationDataSuccess',
-            transformedDurationData,
-          );
-        });
+    it("dispatches the 'requestDurationData' and 'receiveDurationDataSuccess' actions on success", () => {
+      return testAction(
+        actions.fetchDurationData,
+        null,
+        { activeStages },
+        [],
+        [
+          { type: 'requestDurationData' },
+          {
+            type: 'receiveDurationDataSuccess',
+            payload: transformedDurationData,
+          },
+        ],
+      );
     });
 
-    it("dispatches the 'requestDurationData' action", () => {
+    it('does not request hidden stages', () => {
       const dispatch = jest.fn();
-
       return actions
         .fetchDurationData({
           dispatch,
-          rootState,
-          rootGetters,
+          ...rootState,
         })
         .then(() => {
-          expect(dispatch).toHaveBeenNthCalledWith(1, 'requestDurationData');
+          const requestedUrls = mock.history.get.map(({ url }) => url);
+          expect(requestedUrls).not.toContain(
+            `/groups/foo/-/analytics/value_stream_analytics/stages/${hiddenStage.id}/duration_chart`,
+          );
         });
     });
 
-    it("dispatches the 'receiveDurationDataError' action when there is an error", () => {
-      const brokenRootState = {
-        ...rootState,
-        stages: [
-          {
-            id: 'oops',
-          },
-        ],
-      };
-
-      const dispatch = jest.fn();
+    describe('receiveDurationDataError', () => {
+      beforeEach(() => {
+        mock.onGet(endpoints.durationData).reply(404);
+      });
 
-      return actions
-        .fetchDurationData({
-          dispatch,
-          getters,
-          rootState: brokenRootState,
-          rootGetters,
-        })
-        .then(() => {
-          expect(dispatch).toHaveBeenCalledWith('receiveDurationDataError');
-        });
+      it("dispatches the 'receiveDurationDataError' action when there is an error", () => {
+        const dispatch = jest.fn();
+
+        return actions
+          .fetchDurationData({
+            dispatch,
+            ...rootState,
+          })
+          .then(() => {
+            expect(dispatch).toHaveBeenCalledWith('receiveDurationDataError');
+          });
+      });
     });
   });
 
@@ -292,42 +294,41 @@ describe('DurationChart actions', () => {
     });
 
     it('dispatches the receiveDurationMedianDataSuccess action on success', () => {
-      const dispatch = jest.fn();
-
-      return actions
-        .fetchDurationMedianData({
-          dispatch,
-          rootState,
-          rootGetters,
-        })
-        .then(() => {
-          expect(dispatch).toHaveBeenCalledWith(
-            'receiveDurationMedianDataSuccess',
-            transformedDurationMedianData,
-          );
-        });
-    });
-
-    it('dispatches the receiveDurationMedianDataError action when there is an error', () => {
-      const brokenRootState = {
-        ...rootState,
-        stages: [
+      return testAction(
+        actions.fetchDurationMedianData,
+        null,
+        { ...rootState, activeStages },
+        [],
+        [
           {
-            id: 'oops',
+            type: 'receiveDurationMedianDataSuccess',
+            payload: transformedDurationMedianData,
           },
         ],
-      };
-      const dispatch = jest.fn();
+      );
+    });
 
-      return actions
-        .fetchDurationMedianData({
-          dispatch,
-          rootState: brokenRootState,
-          rootGetters,
-        })
-        .then(() => {
-          expect(dispatch).toHaveBeenCalledWith('receiveDurationMedianDataError');
-        });
+    describe('receiveDurationMedianDataError', () => {
+      beforeEach(() => {
+        mock.onGet(endpoints.durationData).reply(404);
+      });
+
+      it('dispatches the receiveDurationMedianDataError action when there is an error', () => {
+        const dispatch = jest.fn();
+        return actions
+          .fetchDurationMedianData({
+            dispatch,
+            rootState,
+            rootGetters: { activeStages },
+          })
+          .then(() => {
+            const requestedUrls = mock.history.get.map(({ url }) => url);
+            expect(requestedUrls).not.toContain(
+              `/groups/foo/-/analytics/value_stream_analytics/stages/${hiddenStage.id}/duration_chart`,
+            );
+            expect(dispatch).toHaveBeenCalledWith('receiveDurationMedianDataError');
+          });
+      });
     });
   });