Revert back to GlDropdown

Minor cleanup tests including removing
unused refs, returning promises instead
of using a done callback etc

Move toggle label ids function to utils

Address minor review comments

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

@@ -2,8 +2,8 @@
 import {
   GlDropdownDivider,
   GlSegmentedControl,
-  GlNewDropdown,
-  GlNewDropdownItem,
+  GlDropdown,
+  GlDropdownItem,
   GlSearchBoxByType,
   GlIcon,
 } from '@gitlab/ui';
@@ -23,8 +23,8 @@ export default {
     GlSegmentedControl,
     GlDropdownDivider,
     GlIcon,
-    GlNewDropdown,
-    GlNewDropdownItem,
+    GlDropdown,
+    GlDropdownItem,
     GlSearchBoxByType,
   },
   props: {
@@ -132,13 +132,16 @@ export default {
       <p>{{ selectedFiltersText }}</p>
     </div>
     <div class="flex-column">
-      <gl-new-dropdown
-        icon="settings"
+      <gl-dropdown
         aria-expanded="false"
         :aria-label="__('CycleAnalytics|Display chart filters')"
         right
       >
-        <div ref="subjectFilter" class="js-tasks-by-type-chart-filters-subject mb-3 px-3">
+        <template #button-content>
+          <gl-icon class="vertical-align-top" name="settings" />
+          <gl-icon name="chevron-down" />
+        </template>
+        <div class="mb-3 px-3">
           <p class="font-weight-bold text-left mb-2">{{ s__('CycleAnalytics|Show') }}</p>
           <gl-segmented-control
             v-model="selectedSubjectFilter"
@@ -150,16 +153,13 @@ export default {
           />
         </div>
         <gl-dropdown-divider />
-        <div ref="labelsFilter" class="js-tasks-by-type-chart-filters-labels mb-3 px-3">
+        <div class="mb-3 px-3">
           <p class="font-weight-bold text-left my-2">
             {{ s__('CycleAnalytics|Select labels') }}
+            <br /><small>{{ selectedLabelLimitText }}</small>
           </p>
-          <small>{{ selectedLabelLimitText }}</small>
-          <gl-search-box-by-type
-            v-model.trim="labelsSearchTerm"
-            class="js-tasks-by-type-chart-filters-subject mb-2"
-          />
-          <gl-new-dropdown-item
+          <gl-search-box-by-type v-model.trim="labelsSearchTerm" class="mb-2" />
+          <gl-dropdown-item
             v-for="label in availableLabels"
             :key="label.id"
             :disabled="isLabelDisabled(label.id)"
@@ -179,12 +179,12 @@ export default {
               class="d-inline-block dropdown-label-box"
             ></span>
             {{ label.name }}
-          </gl-new-dropdown-item>
+          </gl-dropdown-item>
           <div v-show="!hasMatchingLabels" class="text-secondary">
             {{ __('No matching labels') }}
           </div>
         </div>
-      </gl-new-dropdown>
+      </gl-dropdown>
     </div>
   </div>
 </template>

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

@@ -279,9 +279,7 @@ export const fetchTopRankedGroupLabels = ({
   },
 }) => {
   dispatch('requestTopRankedGroupLabels');
-  const {
-    tasksByType: { subject },
-  } = state;
+  const { subject } = state.tasksByType;
 
   return Api.cycleAnalyticsTopLabels({
     subject,
@@ -403,22 +401,21 @@ export const fetchTasksByTypeData = ({ dispatch, state, getters }) => {
   const {
     currentGroupPath,
     cycleAnalyticsRequestParams: { created_after, created_before, project_ids },
-    topRankedLabelIds,
   } = getters;
 
   const {
-    tasksByType: { subject },
+    tasksByType: { subject, selectedLabelIds },
   } = state;
 
   // dont request if we have no labels selected...for now
-  if (topRankedLabelIds.length) {
+  if (selectedLabelIds.length) {
     const params = {
       group_id: currentGroupPath,
       created_after,
       created_before,
       project_ids,
       subject,
-      label_ids: topRankedLabelIds,
+      label_ids: selectedLabelIds,
     };
 
     dispatch('requestTasksByTypeData');

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

@@ -11,9 +11,6 @@ export const currentGroupPath = ({ selectedGroup }) =>
 export const selectedProjectIds = ({ selectedProjects }) =>
   selectedProjects.length ? selectedProjects.map(({ id }) => id) : [];
 
-export const topRankedLabelIds = ({ topRankedLabels }) =>
-  topRankedLabels.length ? topRankedLabels.map(({ id }) => id) : [];
-
 export const cycleAnalyticsRequestParams = ({ startDate = null, endDate = null }, getters) => ({
   project_ids: getters.selectedProjectIds,
   created_after: startDate ? dateFormat(startDate, dateFormats.isoDate) : null,

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

 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import * as types from './mutation_types';
-import { transformRawStages, transformRawTasksByTypeData } from '../utils';
+import { transformRawStages, transformRawTasksByTypeData, toggleSelectedLabel } from '../utils';
 import { TASKS_BY_TYPE_FILTERS } from '../constants';
 
 export default {
@@ -74,10 +74,10 @@ export default {
   },
   [types.RECEIVE_TOP_RANKED_GROUP_LABELS_SUCCESS](state, data = []) {
     const { tasksByType } = state;
-    state.topRankedLabels = data.length ? data.map(convertObjectPropsToCamelCase) : [];
+    state.topRankedLabels = data.map(convertObjectPropsToCamelCase);
     state.tasksByType = {
       ...tasksByType,
-      selectedLabelIds: data.length ? data.map(({ id }) => id) : [],
+      selectedLabelIds: data.map(({ id }) => id),
     };
   },
   [types.RECEIVE_TOP_RANKED_GROUP_LABELS_ERROR](state) {
@@ -232,9 +232,7 @@ export default {
     switch (filter) {
       case TASKS_BY_TYPE_FILTERS.LABEL:
         updatedFilter = {
-          selectedLabelIds: selectedLabelIds.includes(value)
-            ? selectedLabelIds.filter(v => v !== value)
-            : [...selectedLabelIds, value],
+          selectedLabelIds: toggleSelectedLabel({ selectedLabelIds, value }),
         };
         break;
       case TASKS_BY_TYPE_FILTERS.SUBJECT:

ee/app/assets/javascripts/analytics/cycle_analytics/utils.js

@@ -25,6 +25,13 @@ export const removeFlash = (type = 'alert') => {
   }
 };
 
+export const toggleSelectedLabel = ({ selectedLabelIds = [], value = null }) => {
+  if (!value) return selectedLabelIds;
+  return selectedLabelIds.includes(value)
+    ? selectedLabelIds.filter(v => v !== value)
+    : [...selectedLabelIds, value];
+};
+
 export const isStartEvent = ev => Boolean(ev) && Boolean(ev.canBeStartEvent) && ev.canBeStartEvent;
 
 export const eventToOption = (obj = null) => {

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

@@ -419,14 +419,13 @@ describe('Cycle Analytics component', () => {
   });
 
   describe('with failed requests while loading', () => {
-    function mockRequestCycleAnalyticsData({
+    const mockRequestCycleAnalyticsData = ({
       overrides = {},
       mockFetchStageData = true,
       mockFetchStageMedian = true,
       mockFetchDurationData = true,
       mockFetchTasksByTypeData = true,
-      mockFetchTasksByTypeTopLabelsData = true,
-    }) {
+    }) => {
       const defaultStatus = 200;
       const defaultRequests = {
         fetchSummaryData: {
@@ -447,18 +446,16 @@ describe('Cycle Analytics component', () => {
         ...overrides,
       };
 
+      mock
+        .onGet(mockData.endpoints.tasksByTypeTopLabelsData)
+        .reply(defaultStatus, mockData.groupLabels);
+
       if (mockFetchTasksByTypeData) {
         mock
           .onGet(mockData.endpoints.tasksByTypeData)
           .reply(defaultStatus, { ...mockData.tasksByTypeData });
       }
 
-      if (mockFetchTasksByTypeTopLabelsData) {
-        mock
-          .onGet(mockData.endpoints.tasksByTypeTopLabelsData)
-          .reply(defaultStatus, mockData.groupLabels);
-      }
-
       if (mockFetchDurationData) {
         mock
           .onGet(mockData.endpoints.durationData)
@@ -476,7 +473,7 @@ describe('Cycle Analytics component', () => {
       Object.values(defaultRequests).forEach(({ endpoint, status, response }) => {
         mock.onGet(endpoint).replyOnce(status, response);
       });
-    }
+    };
 
     beforeEach(() => {
       setFixtures('<div class="flash-container"></div>');

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

 import { shallowMount, mount } from '@vue/test-utils';
-import { GlNewDropdownItem, GlSegmentedControl } from '@gitlab/ui';
+import { GlDropdownItem, GlSegmentedControl } from '@gitlab/ui';
 import TasksByTypeFilters from 'ee/analytics/cycle_analytics/components/tasks_by_type_filters.vue';
 import {
   TASKS_BY_TYPE_SUBJECT_ISSUE,
   TASKS_BY_TYPE_SUBJECT_MERGE_REQUEST,
   TASKS_BY_TYPE_FILTERS,
 } from 'ee/analytics/cycle_analytics/constants';
+import { shouldFlashAMessage } from '../helpers';
 import { groupLabels } from '../mock_data';
 
 const selectedLabelIds = [groupLabels[0].id];
 
-const findSubjectFilters = ctx =>
-  ctx.find('.js-tasks-by-type-chart-filters-subject').find(GlSegmentedControl);
+const findSubjectFilters = ctx => ctx.find(GlSegmentedControl);
 const findSelectedSubjectFilters = ctx => findSubjectFilters(ctx).attributes('checked');
-const findDropdownLabels = ctx =>
-  ctx.find('.js-tasks-by-type-chart-filters-labels').findAll(GlNewDropdownItem);
-
-const shouldFlashAMessage = (msg = '') =>
-  expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(msg);
+const findDropdownLabels = ctx => ctx.findAll(GlDropdownItem);
 
 const selectLabelAtIndex = (ctx, index) => {
   findDropdownLabels(ctx)
@@ -26,9 +22,8 @@ const selectLabelAtIndex = (ctx, index) => {
   return ctx.vm.$nextTick();
 };
 
-function createComponent({ props = {}, shallow = true }) {
-  const fn = shallow ? shallowMount : mount;
-  return fn(TasksByTypeFilters, {
+function createComponent({ props = {}, mountFn = shallowMount }) {
+  return mountFn(TasksByTypeFilters, {
     propsData: {
       selectedLabelIds,
       labels: groupLabels,
@@ -37,7 +32,7 @@ function createComponent({ props = {}, shallow = true }) {
     },
     stubs: {
       GlNewDropdown: true,
-      GlNewDropdownItem: true,
+      GlDropdownItem: true,
     },
   });
 }
@@ -59,11 +54,11 @@ describe('TasksByTypeFilters', () => {
     });
 
     it('emits the `updateFilter` event when a subject label is clicked', () => {
-      expect(wrapper.emitted().updateFilter).toBeUndefined();
+      expect(wrapper.emitted('updateFilter')).toBeUndefined();
       return selectLabelAtIndex(wrapper, 0).then(() => {
-        expect(wrapper.emitted().updateFilter).toBeDefined();
+        expect(wrapper.emitted('updateFilter')).toBeDefined();
 
-        expect(wrapper.emitted().updateFilter[0]).toEqual([
+        expect(wrapper.emitted('updateFilter')[0]).toEqual([
           { filter: TASKS_BY_TYPE_FILTERS.LABEL, value: groupLabels[0].id },
         ]);
       });
@@ -107,7 +102,7 @@ describe('TasksByTypeFilters', () => {
       });
 
       it('should not allow selecting another label', () => {
-        expect(wrapper.emitted().updateFilter).toBeUndefined();
+        expect(wrapper.emitted('updateFilter')).toBeUndefined();
       });
 
       it('should display a message', () => {
@@ -122,8 +117,8 @@ describe('TasksByTypeFilters', () => {
     });
 
     it('emits the `updateFilter` event when a subject filter is clicked', () => {
-      wrapper = createComponent({ shallow: false });
-      expect(wrapper.emitted().updateFilter).toBeUndefined();
+      wrapper = createComponent({ mountFn: mount });
+      expect(wrapper.emitted('updateFilter')).toBeUndefined();
 
       findSubjectFilters(wrapper)
         .findAll('label:not(.active)')
@@ -131,8 +126,8 @@ describe('TasksByTypeFilters', () => {
         .trigger('click');
 
       return wrapper.vm.$nextTick(() => {
-        expect(wrapper.emitted().updateFilter).toBeDefined();
-        expect(wrapper.emitted().updateFilter[0]).toEqual([
+        expect(wrapper.emitted('updateFilter')).toBeDefined();
+        expect(wrapper.emitted('updateFilter')[0]).toEqual([
           {
             filter: TASKS_BY_TYPE_FILTERS.SUBJECT,
             value: TASKS_BY_TYPE_SUBJECT_MERGE_REQUEST,

ee/spec/frontend/analytics/cycle_analytics/helpers.js

@@ -14,6 +14,10 @@ export function renderTotalTime(selector, element, totalTime = {}) {
   }
 }
 
+export const shouldFlashAMessage = (msg = '') =>
+  expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(msg);
+
 export default {
   renderTotalTime,
+  shouldFlashAMessage,
 };

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

@@ -27,6 +27,7 @@ import {
   transformedDurationMedianData,
   endpoints,
 } from '../mock_data';
+import { shouldFlashAMessage } from '../helpers';
 
 const stageData = { events: [] };
 const error = new Error(`Request failed with status code ${httpStatusCodes.NOT_FOUND}`);
@@ -41,10 +42,6 @@ describe('Cycle analytics actions', () => {
   let state;
   let mock;
 
-  function shouldFlashAMessage(msg = flashErrorMessage) {
-    expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(msg);
-  }
-
   function shouldSetUrlParams({ action, payload, result }) {
     const store = {
       state,
@@ -338,7 +335,7 @@ describe('Cycle analytics actions', () => {
   describe('fetchTopRankedGroupLabels', () => {
     beforeEach(() => {
       gon.api_version = 'v4';
-      state = { selectedGroup, tasksByType: { subject: TASKS_BY_TYPE_SUBJECT_ISSUE } };
+      state = { selectedGroup, tasksByType: { subject: TASKS_BY_TYPE_SUBJECT_ISSUE }, ...getters };
     });
 
     describe('succeeds', () => {
@@ -347,23 +344,18 @@ describe('Cycle analytics actions', () => {
       });
 
       it('dispatches receiveTopRankedGroupLabelsSuccess if the request succeeds', () => {
-        const dispatch = jest.fn();
-
-        return actions
-          .fetchTopRankedGroupLabels({
-            dispatch,
+        return testAction(
+          actions.fetchTopRankedGroupLabels,
+          null,
           state,
-            getters,
-          })
-          .then(() => {
-            expect(dispatch).toHaveBeenCalledWith('requestTopRankedGroupLabels');
-            expect(dispatch).toHaveBeenCalledWith(
-              'receiveTopRankedGroupLabelsSuccess',
-              groupLabels,
+          [],
+          [
+            { type: 'requestTopRankedGroupLabels' },
+            { type: 'receiveTopRankedGroupLabelsSuccess', payload: groupLabels },
+          ],
         );
       });
     });
-    });
 
     describe('with an error', () => {
       beforeEach(() => {
@@ -371,18 +363,16 @@ describe('Cycle analytics actions', () => {
       });
 
       it('dispatches receiveTopRankedGroupLabelsError if the request fails', () => {
-        const dispatch = jest.fn();
-
-        return actions
-          .fetchTopRankedGroupLabels({
-            dispatch,
+        return testAction(
+          actions.fetchTopRankedGroupLabels,
+          null,
           state,
-            getters,
-          })
-          .then(() => {
-            expect(dispatch).toHaveBeenCalledWith('requestTopRankedGroupLabels');
-            expect(dispatch).toHaveBeenCalledWith('receiveTopRankedGroupLabelsError', error);
-          });
+          [],
+          [
+            { type: 'requestTopRankedGroupLabels' },
+            { type: 'receiveTopRankedGroupLabelsError', payload: error },
+          ],
+        );
       });
     });
 
@@ -628,7 +618,7 @@ describe('Cycle analytics actions', () => {
           {},
         );
 
-        shouldFlashAMessage();
+        shouldFlashAMessage(flashErrorMessage);
       });
     });
   });
@@ -681,7 +671,7 @@ describe('Cycle analytics actions', () => {
         { response },
       );
 
-      shouldFlashAMessage();
+      shouldFlashAMessage(flashErrorMessage);
     });
   });
 
@@ -741,7 +731,7 @@ describe('Cycle analytics actions', () => {
         );
       });
 
-      shouldFlashAMessage();
+      shouldFlashAMessage(flashErrorMessage);
     });
   });
 

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

@@ -8,7 +8,6 @@ import {
   durationChartPlottableMedianData,
   allowedStages,
   selectedProjects,
-  groupLabels,
 } from '../mock_data';
 
 let state = null;
@@ -38,7 +37,7 @@ describe('Cycle analytics getters', () => {
           selectedProjects,
         };
 
-        expect(getters.selectedProjectIds(state)).toEqual(selectedProjects.map(({ id }) => id));
+        expect(getters.selectedProjectIds(state)).toEqual([1, 2]);
       });
     });
 
@@ -50,25 +49,6 @@ describe('Cycle analytics getters', () => {
     });
   });
 
-  describe('topRankedLabelIds', () => {
-    describe('with topRankedLabels set', () => {
-      it('returns the `fullPath` value of the group', () => {
-        state = {
-          topRankedLabels: groupLabels,
-        };
-
-        expect(getters.topRankedLabelIds(state)).toEqual(groupLabels.map(({ id }) => id));
-      });
-    });
-
-    describe('without topRankedLabels set', () => {
-      it('will return an empty array', () => {
-        state = { topRankedLabels: [] };
-        expect(getters.topRankedLabelIds(state)).toEqual([]);
-      });
-    });
-  });
-
   describe('currentGroupPath', () => {
     describe('with selectedGroup set', () => {
       it('returns the `fullPath` value of the group', () => {

ee/spec/frontend/analytics/cycle_analytics/utils_spec.js

@@ -15,6 +15,7 @@ import {
   getTasksByTypeData,
   flattenTaskByTypeSeries,
   orderByDate,
+  toggleSelectedLabel,
 } from 'ee/analytics/cycle_analytics/utils';
 import { toYmd } from 'ee/analytics/shared/utils';
 import {
@@ -302,4 +303,19 @@ describe('Cycle analytics utils', () => {
       });
     });
   });
+
+  describe('toggleSelectedLabel', () => {
+    const selectedLabelIds = [1, 2, 3];
+
+    it('will return the array if theres no value given', () => {
+      expect(toggleSelectedLabel({ selectedLabelIds })).toEqual([1, 2, 3]);
+    });
+
+    it('will remove an id that exists', () => {
+      expect(toggleSelectedLabel({ selectedLabelIds, value: 2 })).toEqual([1, 3]);
+    });
+    it('will add an id that does not exist', () => {
+      expect(toggleSelectedLabel({ selectedLabelIds, value: 4 })).toEqual([1, 2, 3, 4]);
+    });
+  });
 });