Merge branch '207050-move-url-parameter-updating-to-renderless-component' into 'master'

Move url parameter updating to mixin

Closes #207050

See merge request gitlab-org/gitlab!26733

ee/app/assets/javascripts/analytics/cycle_analytics/components/base.vue

@@ -13,6 +13,7 @@ import StageDropdownFilter from './stage_dropdown_filter.vue';
 import SummaryTable from './summary_table.vue';
 import StageTable from './stage_table.vue';
 import TasksByTypeChart from './tasks_by_type_chart.vue';
+import UrlSyncMixin from '../mixins/url_sync_mixin';
 
 export default {
   name: 'CycleAnalytics',
@@ -28,7 +29,7 @@ export default {
     Scatterplot,
     TasksByTypeChart,
   },
-  mixins: [glFeatureFlagsMixin()],
+  mixins: [glFeatureFlagsMixin(), UrlSyncMixin],
   props: {
     emptyStateSvgPath: {
       type: String,
@@ -200,7 +201,6 @@ export default {
   maxDateRange: DATE_RANGE_LIMIT,
 };
 </script>
-
 <template>
   <div class="js-cycle-analytics">
     <div class="page-title-holder d-flex align-items-center">

ee/app/assets/javascripts/analytics/cycle_analytics/mixins/url_sync_mixin.js

+import { mapState, mapGetters } from 'vuex';
+import { historyPushState } from '~/lib/utils/common_utils';
+import { setUrlParams } from '~/lib/utils/url_utility';
+import { toYmd } from '../../shared/utils';
+
+export default {
+  computed: {
+    ...mapGetters(['currentGroupPath', 'selectedProjectIds']),
+    ...mapState(['startDate', 'endDate']),
+    query() {
+      return {
+        group_id: this.currentGroupPath,
+        'project_ids[]': this.selectedProjectIds,
+        created_after: toYmd(this.startDate),
+        created_before: toYmd(this.endDate),
+      };
+    },
+  },
+  watch: {
+    query() {
+      historyPushState(setUrlParams(this.query, window.location.href, true));
+    },
+  },
+};

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

 import dateFormat from 'dateformat';
 import Api from 'ee/api';
 import { getDayDifference, getDateInPast } from '~/lib/utils/datetime_utility';
-import { historyPushState } from '~/lib/utils/common_utils';
-import { setUrlParams } from '~/lib/utils/url_utility';
 import createFlash from '~/flash';
 import { __, sprintf } from '~/locale';
 import httpStatus from '~/lib/utils/http_status';
 import * as types from './mutation_types';
 import { dateFormats } from '../../shared/constants';
-import { toYmd } from '../../shared/utils';
 import { removeFlash } from '../utils';
 
 const handleErrorOrRethrow = ({ action, error }) => {
@@ -26,50 +23,19 @@ const isStageNameExistsError = ({ status, errors }) => {
   return false;
 };
 
-const updateUrlParams = (
-  { getters: { currentGroupPath, selectedProjectIds } },
-  additionalParams = {},
-) => {
-  historyPushState(
-    setUrlParams(
-      {
-        group_id: currentGroupPath,
-        'project_ids[]': selectedProjectIds,
-        ...additionalParams,
-      },
-      window.location.href,
-      true,
-    ),
-  );
-};
-
 export const setFeatureFlags = ({ commit }, featureFlags) =>
   commit(types.SET_FEATURE_FLAGS, featureFlags);
 
-export const setSelectedGroup = ({ commit, getters }, group) => {
-  commit(types.SET_SELECTED_GROUP, group);
-  updateUrlParams({ getters });
-};
+export const setSelectedGroup = ({ commit }, group) => commit(types.SET_SELECTED_GROUP, group);
 
-export const setSelectedProjects = ({ commit, getters }, projects) => {
+export const setSelectedProjects = ({ commit }, projects) => {
   commit(types.SET_SELECTED_PROJECTS, projects);
-  updateUrlParams({ getters });
 };
 
 export const setSelectedStage = ({ commit }, stage) => commit(types.SET_SELECTED_STAGE, stage);
 
-export const setDateRange = (
-  { commit, dispatch, getters },
-  { skipFetch = false, startDate, endDate },
-) => {
+export const setDateRange = ({ commit, dispatch }, { skipFetch = false, startDate, endDate }) => {
   commit(types.SET_DATE_RANGE, { startDate, endDate });
-  updateUrlParams(
-    { getters },
-    {
-      created_after: toYmd(startDate),
-      created_before: toYmd(endDate),
-    },
-  );
 
   if (skipFetch) return false;
 

ee/spec/frontend/analytics/cycle_analytics/components/base_spec.js

@@ -17,12 +17,18 @@ import Daterange from 'ee/analytics/shared/components/daterange.vue';
 import TasksByTypeChart from 'ee/analytics/cycle_analytics/components/tasks_by_type_chart.vue';
 import waitForPromises from 'helpers/wait_for_promises';
 import httpStatusCodes from '~/lib/utils/http_status';
+import * as commonUtils from '~/lib/utils/common_utils';
+import * as urlUtils from '~/lib/utils/url_utility';
+import { toYmd } from 'ee/analytics/shared/utils';
 import * as mockData from '../mock_data';
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import UrlSyncMixin from 'ee/analytics/cycle_analytics/mixins/url_sync_mixin';
 
 const noDataSvgPath = 'path/to/no/data';
 const noAccessSvgPath = 'path/to/no/access';
 const emptyStateSvgPath = 'path/to/empty/state';
 const hideGroupDropDown = false;
+const selectedGroup = convertObjectPropsToCamelCase(mockData.group);
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -35,7 +41,9 @@ const defaultStubs = {
 };
 
 function createComponent({
-  opts = {},
+  opts = {
+    stubs: defaultStubs,
+  },
   shallow = true,
   withStageSelected = false,
   scatterplotEnabled = true,
@@ -47,6 +55,7 @@ function createComponent({
   const comp = func(Component, {
     localVue,
     store,
+    mixins: [UrlSyncMixin],
     propsData: {
       emptyStateSvgPath,
       noDataSvgPath,
@@ -66,14 +75,13 @@ function createComponent({
   });
 
   comp.vm.$store.dispatch('initializeCycleAnalytics', {
-    group: mockData.group,
     createdAfter: mockData.startDate,
     createdBefore: mockData.endDate,
   });
 
   if (withStageSelected) {
     comp.vm.$store.dispatch('setSelectedGroup', {
-      ...mockData.group,
+      ...selectedGroup,
     });
 
     comp.vm.$store.dispatch('receiveGroupStagesAndEventsSuccess', {
@@ -95,6 +103,13 @@ describe('Cycle Analytics component', () => {
       .findAll('.stage-nav-item')
       .at(index);
 
+  const shouldSetUrlParams = result => {
+    return wrapper.vm.$nextTick().then(() => {
+      expect(urlUtils.setUrlParams).toHaveBeenCalledWith(result, window.location.href, true);
+      expect(commonUtils.historyPushState).toHaveBeenCalled();
+    });
+  };
+
   const displaysProjectsDropdownFilter = flag => {
     expect(wrapper.find(ProjectsDropdownFilter).exists()).toBe(flag);
   };
@@ -132,6 +147,7 @@ describe('Cycle Analytics component', () => {
   afterEach(() => {
     wrapper.destroy();
     mock.restore();
+    wrapper = null;
   });
 
   describe('displays the components as required', () => {
@@ -348,9 +364,6 @@ describe('Cycle Analytics component', () => {
         beforeEach(() => {
           mock = new MockAdapter(axios);
           wrapper = createComponent({
-            opts: {
-              stubs: defaultStubs,
-            },
             shallow: false,
             withStageSelected: true,
             customizableCycleAnalyticsEnabled: true,
@@ -372,9 +385,6 @@ describe('Cycle Analytics component', () => {
         beforeEach(() => {
           mock = new MockAdapter(axios);
           wrapper = createComponent({
-            opts: {
-              stubs: defaultStubs,
-            },
             shallow: false,
             withStageSelected: true,
             customizableCycleAnalyticsEnabled: false,
@@ -396,9 +406,6 @@ describe('Cycle Analytics component', () => {
         beforeEach(() => {
           mock = new MockAdapter(axios);
           wrapper = createComponent({
-            opts: {
-              stubs: defaultStubs,
-            },
             shallow: false,
             withStageSelected: true,
             customizableCycleAnalyticsEnabled: false,
@@ -600,4 +607,81 @@ describe('Cycle Analytics component', () => {
       });
     });
   });
+
+  describe('Url Sync', () => {
+    beforeEach(() => {
+      commonUtils.historyPushState = jest.fn();
+      urlUtils.setUrlParams = jest.fn();
+
+      mock = new MockAdapter(axios);
+
+      wrapper = createComponent({
+        shallow: false,
+        scatterplotEnabled: false,
+        tasksByTypeChartEnabled: false,
+        stubs: {
+          ...defaultStubs,
+        },
+      });
+
+      return wrapper.vm.$nextTick();
+    });
+
+    it('sets the created_after and created_before url parameters', () => {
+      return shouldSetUrlParams({
+        created_after: toYmd(mockData.startDate),
+        created_before: toYmd(mockData.endDate),
+        group_id: null,
+        'project_ids[]': [],
+      });
+    });
+
+    describe('with a group selected', () => {
+      const fakeGroup = {
+        id: 2,
+        path: 'new-test',
+        fullPath: 'new-test-group',
+        name: 'New test group',
+      };
+
+      beforeEach(() => {
+        wrapper.vm.$store.dispatch('setSelectedGroup', {
+          ...fakeGroup,
+        });
+        return wrapper.vm.$nextTick();
+      });
+
+      it('sets the group_id url parameter', () => {
+        return shouldSetUrlParams({
+          created_after: toYmd(mockData.startDate),
+          created_before: toYmd(mockData.endDate),
+          group_id: fakeGroup.fullPath,
+          'project_ids[]': [],
+        });
+      });
+    });
+
+    describe('with a group and selectedProjectIds set', () => {
+      const selectedProjectIds = mockData.selectedProjects.map(({ id }) => id);
+
+      beforeEach(() => {
+        wrapper.vm.$store.dispatch('setSelectedGroup', {
+          ...selectedGroup,
+        });
+
+        wrapper.vm.$store.dispatch('setSelectedProjects', mockData.selectedProjects);
+
+        return wrapper.vm.$nextTick();
+      });
+
+      it('sets the project_ids url parameter', () => {
+        return shouldSetUrlParams({
+          created_after: toYmd(mockData.startDate),
+          created_before: toYmd(mockData.endDate),
+          group_id: selectedGroup.fullPath,
+          'project_ids[]': selectedProjectIds,
+        });
+      });
+    });
+  });
 });

ee/spec/frontend/analytics/cycle_analytics/mixins/url_sync_mixin_spec.js

+import Vue from 'vue';
+import { createLocalVue, shallowMount } from '@vue/test-utils';
+import Vuex from 'vuex';
+import store from 'ee/analytics/cycle_analytics/store';
+import UrlSyncMixin from 'ee/analytics/cycle_analytics/mixins/url_sync_mixin';
+import { toYmd } from 'ee/analytics/shared/utils';
+import { startDate, endDate } from '../mock_data';
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+const createComponent = () => {
+  const Component = Vue.extend({
+    localVue,
+    store,
+    mixins: [UrlSyncMixin],
+    render(h) {
+      return h('div');
+    },
+  });
+
+  return shallowMount(Component);
+};
+
+describe('UrlSyncMixin', () => {
+  let wrapper;
+  beforeEach(() => {
+    wrapper = createComponent();
+    wrapper.vm.$store.dispatch('initializeCycleAnalytics', {
+      createdAfter: startDate,
+      createdBefore: endDate,
+    });
+  });
+
+  afterEach(() => {
+    wrapper.vm.$destroy();
+  });
+
+  describe('watch', () => {
+    describe('query', () => {
+      const defaultState = {
+        group_id: null,
+        'project_ids[]': [],
+        created_after: toYmd(startDate),
+        created_before: toYmd(endDate),
+      };
+
+      it('sets the start and end date to the default state values', () => {
+        expect(wrapper.vm.query).toEqual(defaultState);
+      });
+
+      it.each`
+        param               | action                   | payload                                                  | updatedParams
+        ${'group_id'}       | ${'setSelectedGroup'}    | ${{ fullPath: 'test-group', name: 'test group' }}        | ${{ group_id: 'test-group' }}
+        ${'project_ids'}    | ${'setSelectedProjects'} | ${[{ id: 1 }, { id: 2 }]}                                | ${{ 'project_ids[]': [1, 2] }}
+        ${'created_after'}  | ${'setDateRange'}        | ${{ startDate: '2020-06-18', endDate, skipFetch: true }} | ${{ created_after: toYmd('2020-06-18') }}
+        ${'created_before'} | ${'setDateRange'}        | ${{ endDate: '2020-06-18', startDate, skipFetch: true }} | ${{ created_before: toYmd('2020-06-18') }}
+      `(
+        'sets the $param parameter when $action is dispatched',
+        ({ action, payload, updatedParams }) => {
+          wrapper.vm.$store.dispatch(action, payload);
+
+          expect(wrapper.vm.query).toEqual({
+            ...defaultState,
+            ...updatedParams,
+          });
+        },
+      );
+    });
+  });
+});

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

-import * as commonUtils from '~/lib/utils/common_utils';
-import * as urlUtils from '~/lib/utils/url_utility';
 import axios from 'axios';
 import MockAdapter from 'axios-mock-adapter';
 import testAction from 'helpers/vuex_action_helper';
@@ -12,7 +10,6 @@ import {
 } from 'ee/analytics/cycle_analytics/constants';
 import createFlash from '~/flash';
 import httpStatusCodes from '~/lib/utils/http_status';
-import { toYmd } from 'ee/analytics/shared/utils';
 import {
   group,
   summaryData,
@@ -42,24 +39,7 @@ describe('Cycle analytics actions', () => {
   let state;
   let mock;
 
-  function shouldSetUrlParams({ action, payload, result }) {
-    const store = {
-      state,
-      getters,
-      commit: jest.fn(),
-      dispatch: jest.fn(() => Promise.resolve()),
-    };
-
-    return actions[action](store, payload).then(() => {
-      expect(urlUtils.setUrlParams).toHaveBeenCalledWith(result, window.location.href, true);
-      expect(commonUtils.historyPushState).toHaveBeenCalled();
-    });
-  }
-
   beforeEach(() => {
-    commonUtils.historyPushState = jest.fn();
-    urlUtils.setUrlParams = jest.fn();
-
     state = {
       startDate,
       endDate,
@@ -99,61 +79,10 @@ describe('Cycle analytics actions', () => {
     );
   });
 
-  describe('setSelectedGroup', () => {
-    const payload = { full_path: 'someNewGroup' };
-    it('calls setUrlParams with the group params', () => {
-      actions.setSelectedGroup(
-        {
-          state,
-          getters: {
-            currentGroupPath: 'someNewGroup',
-            selectedProjectIds: [],
-          },
-          commit: jest.fn(),
-        },
-        payload,
-      );
-
-      expect(urlUtils.setUrlParams).toHaveBeenCalledWith(
-        {
-          group_id: 'someNewGroup',
-          'project_ids[]': [],
-        },
-        window.location.href,
-        true,
-      );
-      expect(commonUtils.historyPushState).toHaveBeenCalled();
-    });
-  });
-
-  describe('setSelectedProjects', () => {
-    const payload = [1, 2];
-    it('calls setUrlParams with the date params', () => {
-      actions.setSelectedProjects(
-        {
-          state,
-          getters: {
-            currentGroupPath: 'test-group',
-            selectedProjectIds: payload,
-          },
-          commit: jest.fn(),
-        },
-        payload,
-      );
-
-      expect(urlUtils.setUrlParams).toHaveBeenCalledWith(
-        { 'project_ids[]': payload, group_id: 'test-group' },
-        window.location.href,
-        true,
-      );
-      expect(commonUtils.historyPushState).toHaveBeenCalled();
-    });
-  });
-
   describe('setDateRange', () => {
     const payload = { startDate, endDate };
 
-    it('sets the dates as expected and dispatches fetchCycleAnalyticsData', done => {
+    it('dispatches the fetchCycleAnalyticsData action', done => {
       testAction(
         actions.setDateRange,
         payload,
@@ -163,19 +92,6 @@ describe('Cycle analytics actions', () => {
         done,
       );
     });
-
-    it('calls setUrlParams with the date params', () => {
-      shouldSetUrlParams({
-        action: 'setDateRange',
-        payload,
-        result: {
-          group_id: getters.currentGroupPath,
-          'project_ids[]': getters.selectedProjectIds,
-          created_after: toYmd(payload.startDate),
-          created_before: toYmd(payload.endDate),
-        },
-      });
-    });
   });
 
   describe('fetchStageData', () => {
@@ -1600,8 +1516,6 @@ describe('Cycle analytics actions', () => {
     };
 
     beforeEach(() => {
-      commonUtils.historyPushState = jest.fn();
-      urlUtils.setUrlParams = jest.fn();
       mockDispatch = jest.fn(() => Promise.resolve());
       mockCommit = jest.fn();
       store = {