Address minor review comments

Cleans up some test expectations and unused
functions that are no longer needed.

Moves `filterStagesByHiddenStatus` from getters to utils

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

@@ -122,7 +122,7 @@ export default {
       :stages="pathNavigationData"
       :selected-stage="selectedStage"
       :with-stage-counts="false"
-      @selected="(ev) => onSelectStage(ev)"
+      @selected="onSelectStage"
     />
     <gl-loading-icon v-if="isLoading" size="lg" />
     <div v-else class="wrapper">
@@ -197,7 +197,7 @@ export default {
             </nav>
           </div>
           <div class="stage-panel-body">
-            <section class="stage-events overflow-auto gl-w-full">
+            <section class="stage-events gl-overflow-auto gl-w-full">
               <gl-loading-icon v-show="isLoadingStage" size="lg" />
               <template v-if="displayNoAccess">
                 <gl-empty-state

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

-import { transformStagesForPathNavigation } from '../utils';
-
-export const filterStagesByHiddenStatus = (stages = [], isHidden = true) =>
-  stages.filter(({ hidden = false }) => hidden === isHidden);
+import { transformStagesForPathNavigation, filterStagesByHiddenStatus } from '../utils';
 
 export const pathNavigationData = ({ stages, medians, stageCounts, selectedStage }) => {
   return transformStagesForPathNavigation({

app/assets/javascripts/cycle_analytics/utils.js

@@ -94,7 +94,7 @@ export const transformStagesForPathNavigation = ({
   const formattedStages = stages.map((stage) => {
     return {
       metric: medians[stage?.id],
-      selected: stage.id === selectedStage.id,
+      selected: stage?.id === selectedStage?.id, // Also could null === null cause an issue here?
       stageCount: stageCounts && stageCounts[stage?.id],
       icon: null,
       ...stage,
@@ -155,3 +155,6 @@ export const formatMedianValues = (medians = []) =>
       [id]: value ? medianTimeToParsedSeconds(value) : '-',
     };
   }, {});
+
+export const filterStagesByHiddenStatus = (stages = [], isHidden = true) =>
+  stages.filter(({ hidden = false }) => hidden === isHidden);

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

 import dateFormat from 'dateformat';
 import { isNumber } from 'lodash';
 import { OVERVIEW_STAGE_ID } from '~/cycle_analytics/constants';
-import {
-  filterStagesByHiddenStatus,
-  pathNavigationData as basePathNavigationData,
-} from '~/cycle_analytics/store/getters';
+import { pathNavigationData as basePathNavigationData } from '~/cycle_analytics/store/getters';
+import { filterStagesByHiddenStatus } from '~/cycle_analytics/utils';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import httpStatus from '~/lib/utils/http_status';
 import { filterToQueryObject } from '~/vue_shared/components/filtered_search_bar/filtered_search_utils';

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

@@ -24,7 +24,8 @@ describe('Group PathNavigation', () => {
     return wrapper.findByTestId('gl-path-nav').findAll('li');
   };
 
-  const pathItemContent = () => pathNavigationItems().wrappers;
+  const pathItemContent = () => pathNavigationItems().wrappers.map(extendedWrapper);
+  const firstPopover = () => wrapper.findAllByTestId('stage-item-popover').at(0);
 
   const stagesWithCounts = transformedStagePathData.filter(
     (stage) => stage.id !== OVERVIEW_STAGE_ID,
@@ -48,35 +49,32 @@ describe('Group PathNavigation', () => {
       const [overviewStage, ...popoverStages] = pathItemContent();
 
       expect(overviewStage.text()).toContain('Overview');
-      expect(overviewStage.find('[data-testid="stage-item-popover"]').exists()).toBe(false);
+      expect(overviewStage.findByTestId('stage-item-popover').exists()).toBe(false);
 
       popoverStages.forEach((stage) => {
-        expect(stage.find('[data-testid="stage-item-popover"]').exists()).toBe(true);
+        expect(stage.findByTestId('stage-item-popover').exists()).toBe(true);
       });
     });
 
     it('shows the sanitized start event description for the first stage item', () => {
-      const firstPopover = wrapper.findAll('[data-testid="stage-item-popover"]').at(0);
       const expectedStartEventDescription = 'Issue created';
-      expect(firstPopover.text()).toContain(expectedStartEventDescription);
+      expect(firstPopover().text()).toContain(expectedStartEventDescription);
     });
 
     it('shows the sanitized end event description for the first stage item', () => {
-      const firstPopover = wrapper.findAll('[data-testid="stage-item-popover"]').at(0);
       const expectedStartEventDescription =
         'Issue first associated with a milestone or issue first added to a board';
-      expect(firstPopover.text()).toContain(expectedStartEventDescription);
+      expect(firstPopover().text()).toContain(expectedStartEventDescription);
     });
 
     it('shows the median stage time for the first stage item', () => {
-      const firstPopover = wrapper.findAll('[data-testid="stage-item-popover"]').at(0);
-      expect(firstPopover.text()).toContain('Stage time (median)');
+      expect(firstPopover().text()).toContain('Stage time (median)');
     });
 
     it('renders each stage with its stage count', () => {
       const popoverStages = pathItemContent().slice(1); // skip the first stage, the overview does not have a popover
       popoverStages.forEach((stage, index) => {
-        const content = stage.find('[data-testid="stage-item-popover"]').html();
+        const content = stage.findByTestId('stage-item-popover').html();
         expect(content).toContain('Items in stage');
         expect(content).toContain(`${stagesWithCounts[index].stageCount} items`);
       });

spec/frontend/cycle_analytics/base_spec.js

-import { shallowMount, createLocalVue } from '@vue/test-utils';
+import { shallowMount } from '@vue/test-utils';
+import Vue from 'vue';
 import Vuex from 'vuex';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import Component from '~/cycle_analytics/components/base.vue';
@@ -8,8 +9,7 @@ import createStore from '~/cycle_analytics/store';
 const noDataSvgPath = 'path/to/no/data';
 const noAccessSvgPath = 'path/to/no/access';
 
-const localVue = createLocalVue();
-localVue.use(Vuex);
+Vue.use(Vuex);
 
 let wrapper;
 
@@ -17,7 +17,6 @@ function createComponent() {
   const store = createStore();
   return extendedWrapper(
     shallowMount(Component, {
-      localVue,
       store,
       propsData: {
         noDataSvgPath,

spec/frontend/cycle_analytics/path_navigation_spec.js

@@ -38,6 +38,9 @@ describe('Project PathNavigation', () => {
     findPathNavigationItems().at(index).find('button').trigger('click');
   };
 
+  const pathItemContent = () => findPathNavigationItems().wrappers.map(extendedWrapper);
+  const firstPopover = () => wrapper.findAllByTestId('stage-item-popover').at(0);
+
   beforeEach(() => {
     wrapper = createComponent();
   });
@@ -53,10 +56,9 @@ describe('Project PathNavigation', () => {
     });
 
     it('contains all the expected stages', () => {
-      const html = wrapper.find(GlPath).html();
-
-      transformedProjectStagePathData.forEach((stage) => {
-        expect(html).toContain(stage.title);
+      const stageContent = findPathNavigationTitles();
+      transformedProjectStagePathData.forEach((stage, index) => {
+        expect(stageContent[index]).toContain(stage.title);
       });
     });
 
@@ -89,16 +91,13 @@ describe('Project PathNavigation', () => {
           });
 
           it('renders popovers for all stages', () => {
-            const pathItemContent = findPathNavigationItems().wrappers;
-
-            pathItemContent.forEach((stage) => {
-              expect(stage.find('[data-testid="stage-item-popover"]').exists()).toBe(true);
+            pathItemContent().forEach((stage) => {
+              expect(stage.findByTestId('stage-item-popover').exists()).toBe(true);
             });
           });
 
           it('shows the median stage time for the first stage item', () => {
-            const firstPopover = wrapper.findAll('[data-testid="stage-item-popover"]').at(0);
-            expect(firstPopover.text()).toContain('Stage time (median)');
+            expect(firstPopover().text()).toContain('Stage time (median)');
           });
         });
       });

spec/frontend/cycle_analytics/store/getters_spec.js

@@ -13,22 +13,4 @@ describe('Value stream analytics getters', () => {
       expect(getters.pathNavigationData(state)).toEqual(transformedProjectStagePathData);
     });
   });
-
-  describe('filterStagesByHiddenStatus', () => {
-    const hiddenStages = [{ title: 'three', hidden: true }];
-    const visibleStages = [
-      { title: 'one', hidden: false },
-      { title: 'two', hidden: false },
-    ];
-    const mockStages = [...visibleStages, ...hiddenStages];
-
-    it.each`
-      isHidden     | result
-      ${false}     | ${visibleStages}
-      ${undefined} | ${hiddenStages}
-      ${true}      | ${hiddenStages}
-    `('with isHidden=$isHidden returns matching stages', ({ isHidden, result }) => {
-      expect(getters.filterStagesByHiddenStatus(mockStages, isHidden)).toEqual(result);
-    });
-  });
 });

spec/frontend/cycle_analytics/utils_spec.js

@@ -5,6 +5,7 @@ import {
   timeSummaryForPathNavigation,
   medianTimeToParsedSeconds,
   formatMedianValues,
+  filterStagesByHiddenStatus,
 } from '~/cycle_analytics/utils';
 import {
   selectedStage,
@@ -102,19 +103,19 @@ describe('Value stream analytics utils', () => {
     describe('transforms the data as expected', () => {
       it('returns an array of stages', () => {
         expect(Array.isArray(response)).toBe(true);
-        expect(response.length).toEqual(stages.length);
+        expect(response.length).toBe(stages.length);
       });
 
       it('selects the correct stage', () => {
         const selected = response.filter((stage) => stage.selected === true)[0];
 
-        expect(selected.title).toEqual(selectedStage.title);
+        expect(selected.title).toBe(selectedStage.title);
       });
 
       it('includes the correct metric for the associated stage', () => {
         const issue = response.filter((stage) => stage.name === 'issue')[0];
 
-        expect(issue.metric).toEqual(pathNavIssueMetric);
+        expect(issue.metric).toBe(pathNavIssueMetric);
       });
     });
   });
@@ -130,7 +131,7 @@ describe('Value stream analytics utils', () => {
       ${'seconds'} | ${10}   | ${'<1m'}
       ${'seconds'} | ${0}    | ${'-'}
     `('will format $value $unit to $result', ({ unit, value, result }) => {
-      expect(timeSummaryForPathNavigation({ [unit]: value })).toEqual(result);
+      expect(timeSummaryForPathNavigation({ [unit]: value })).toBe(result);
     });
   });
 
@@ -146,7 +147,7 @@ describe('Value stream analytics utils', () => {
       ${59}      | ${'<1m'}
       ${0}       | ${'-'}
     `('will correctly parse $value seconds into $result', ({ value, result }) => {
-      expect(medianTimeToParsedSeconds(value)).toEqual(result);
+      expect(medianTimeToParsedSeconds(value)).toBe(result);
     });
   });
 
@@ -159,4 +160,22 @@ describe('Value stream analytics utils', () => {
       });
     });
   });
+
+  describe('filterStagesByHiddenStatus', () => {
+    const hiddenStages = [{ title: 'three', hidden: true }];
+    const visibleStages = [
+      { title: 'one', hidden: false },
+      { title: 'two', hidden: false },
+    ];
+    const mockStages = [...visibleStages, ...hiddenStages];
+
+    it.each`
+      isHidden     | result
+      ${false}     | ${visibleStages}
+      ${undefined} | ${hiddenStages}
+      ${true}      | ${hiddenStages}
+    `('with isHidden=$isHidden returns matching stages', ({ isHidden, result }) => {
+      expect(filterStagesByHiddenStatus(mockStages, isHidden)).toEqual(result);
+    });
+  });
 });