Merge branch 'fix-cycle-analytics-group-labels-endpoint' into 'master'

Replace cycle analytics group labels endpoint

See merge request gitlab-org/gitlab!25505

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

@@ -26,6 +26,11 @@ export default {
     },
   },
   methods: {
+    labelTitle(label) {
+      // there are 2 possible endpoints for group labels
+      // one returns label.name the other label.title
+      return label?.name || label.title;
+    },
     isSelectedLabel(id) {
       return this.selectedLabelId && id === this.selectedLabelId;
     },
@@ -41,7 +46,7 @@ export default {
           class="d-inline-block dropdown-label-box"
         >
         </span>
-        {{ selectedLabel.title }}
+        {{ labelTitle(selectedLabel) }}
       </span>
       <span v-else>{{ __('Select a label') }}</span>
     </template>
@@ -56,7 +61,7 @@ export default {
     >
       <span :style="{ backgroundColor: label.color }" class="d-inline-block dropdown-label-box">
       </span>
-      {{ label.title }}
+      {{ labelTitle(label) }}
     </gl-dropdown-item>
   </gl-dropdown>
 </template>

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

@@ -257,8 +257,8 @@ export const fetchGroupLabels = ({ dispatch, state }) => {
     selectedGroup: { fullPath },
   } = state;
 
-  return Api.groupLabels(fullPath)
-    .then(data => dispatch('receiveGroupLabelsSuccess', data))
+  return Api.cycleAnalyticsGroupLabels(fullPath)
+    .then(({ data }) => dispatch('receiveGroupLabelsSuccess', data))
     .catch(error =>
       handleErrorOrRethrow({ error, action: () => dispatch('receiveGroupLabelsError', error) }),
     );

ee/app/assets/javascripts/api.js

@@ -22,6 +22,7 @@ export default {
   cycleAnalyticsStagePath: '/-/analytics/value_stream_analytics/stages/:stage_id',
   cycleAnalyticsDurationChartPath:
     '/-/analytics/value_stream_analytics/stages/:stage_id/duration_chart',
+  cycleAnalyticsGroupLabelsPath: '/api/:version/groups/:namespace_path/labels',
   codeReviewAnalyticsPath: '/api/:version/analytics/code_review',
   countriesPath: '/-/countries',
   countryStatesPath: '/-/country_states',
@@ -214,6 +215,19 @@ export default {
     });
   },
 
+  cycleAnalyticsGroupLabels(groupId, params = {}) {
+    // TODO: This can be removed when we resolve the labels endpoint
+    // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25746
+    const url = Api.buildUrl(this.cycleAnalyticsGroupLabelsPath).replace(
+      ':namespace_path',
+      groupId,
+    );
+
+    return axios.get(url, {
+      params,
+    });
+  },
+
   codeReviewAnalytics(params = {}) {
     const url = Api.buildUrl(this.codeReviewAnalyticsPath);
     return axios.get(url, { params });

ee/spec/frontend/analytics/cycle_analytics/components/__snapshots__/labels_selector_spec.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Value Stream Analytics LabelsSelector with no item selected will render the label selector 1`] = `
+"<gl-dropdown-stub text=\\"\\" toggle-class=\\"overflow-hidden\\" class=\\"w-100\\"><template></template>
+  <gl-dropdown-item-stub active=\\"true\\">Select a label
+  </gl-dropdown-item-stub>
+  <gl-dropdown-item-stub><span class=\\"d-inline-block dropdown-label-box\\" style=\\"background-color: rgb(255, 0, 0);\\"></span>
+    roses
+  </gl-dropdown-item-stub>
+  <gl-dropdown-item-stub><span class=\\"d-inline-block dropdown-label-box\\" style=\\"background-color: rgb(255, 255, 255);\\"></span>
+    some space
+  </gl-dropdown-item-stub>
+  <gl-dropdown-item-stub><span class=\\"d-inline-block dropdown-label-box\\" style=\\"background-color: rgb(0, 0, 255);\\"></span>
+    violets
+  </gl-dropdown-item-stub>
+</gl-dropdown-stub>"
+`;
+
+exports[`Value Stream Analytics LabelsSelector with selectedLabelId set will render the label selector 1`] = `
+"<gl-dropdown-stub text=\\"\\" toggle-class=\\"overflow-hidden\\" class=\\"w-100\\"><template></template>
+  <gl-dropdown-item-stub>Select a label
+  </gl-dropdown-item-stub>
+  <gl-dropdown-item-stub><span class=\\"d-inline-block dropdown-label-box\\" style=\\"background-color: rgb(255, 0, 0);\\"></span>
+    roses
+  </gl-dropdown-item-stub>
+  <gl-dropdown-item-stub><span class=\\"d-inline-block dropdown-label-box\\" style=\\"background-color: rgb(255, 255, 255);\\"></span>
+    some space
+  </gl-dropdown-item-stub>
+  <gl-dropdown-item-stub active=\\"true\\"><span class=\\"d-inline-block dropdown-label-box\\" style=\\"background-color: rgb(0, 0, 255);\\"></span>
+    violets
+  </gl-dropdown-item-stub>
+</gl-dropdown-stub>"
+`;

ee/spec/frontend/analytics/cycle_analytics/components/__snapshots__/tasks_by_type_chart_spec.js.snap

@@ -28,17 +28,25 @@ exports[`TasksByTypeChart with data available filters labels with label dropdown
   <ul>
     <li>
       <a href=\\"#\\" class=\\"dropdown-menu-link is-active\\">
-        <span style=\\"background-color: #BADA55;\\" class=\\"d-inline-block dropdown-label-box\\">
+        <span style=\\"background-color: #FF0000;\\" class=\\"d-inline-block dropdown-label-box\\">
               </span>
-        Foo Label
+
       </a>
     </li>
 
     <li>
       <a href=\\"#\\" class=\\"dropdown-menu-link is-active\\">
-        <span style=\\"background-color: #0033CC;\\" class=\\"d-inline-block dropdown-label-box\\">
+        <span style=\\"background-color: #FFFFFF;\\" class=\\"d-inline-block dropdown-label-box\\">
               </span>
-        Foo::Bar
+
+      </a>
+    </li>
+
+    <li>
+      <a href=\\"#\\" class=\\"dropdown-menu-link is-active\\">
+        <span style=\\"background-color: #0000FF;\\" class=\\"d-inline-block dropdown-label-box\\">
+              </span>
+
       </a>
     </li>
   </ul>
@@ -58,7 +66,7 @@ exports[`TasksByTypeChart with data available should render the loading chart 1`
     <h3>Type of work</h3>
     <div>
       <p>Showing data for group 'Gitlab Org' from Dec 11, 2019 to Jan 10, 2020</p>
-      <tasks-by-type-filters-stub labels=\\"[object Object],[object Object]\\" selectedlabelids=\\"1,2,3\\" subjectfilter=\\"Issue\\"></tasks-by-type-filters-stub>
+      <tasks-by-type-filters-stub labels=\\"[object Object],[object Object],[object Object]\\" selectedlabelids=\\"1,2,3\\" subjectfilter=\\"Issue\\"></tasks-by-type-filters-stub>
       <gl-stacked-column-chart-stub data=\\"0,1,2,5,2,3,2,4,1\\" option=\\"[object Object]\\" presentation=\\"stacked\\" groupby=\\"Group 1,Group 2,Group 3\\" xaxistype=\\"category\\" xaxistitle=\\"Date\\" yaxistitle=\\"Number of tasks\\" seriesnames=\\"Cool label,Normal label\\" legendaveragetext=\\"Avg\\" legendmaxtext=\\"Max\\" y-axis-type=\\"value\\"></gl-stacked-column-chart-stub>
     </div>
   </div>

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

@@ -22,7 +22,6 @@ import * as mockData from '../mock_data';
 const noDataSvgPath = 'path/to/no/data';
 const noAccessSvgPath = 'path/to/no/access';
 const emptyStateSvgPath = 'path/to/empty/state';
-const baseStagesEndpoint = '/-/analytics/cycle_analytics/stages';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -50,7 +49,7 @@ function createComponent({
       emptyStateSvgPath,
       noDataSvgPath,
       noAccessSvgPath,
-      baseStagesEndpoint,
+      baseStagesEndpoint: mockData.endpoints.baseStagesEndpoint,
     },
     provide: {
       glFeatures: {
@@ -401,8 +400,6 @@ describe('Cycle Analytics component', () => {
   });
 
   describe('with failed requests while loading', () => {
-    const { full_path: groupId } = mockData.group;
-
     function mockRequestCycleAnalyticsData({
       overrides = {},
       mockFetchStageData = true,
@@ -414,17 +411,17 @@ describe('Cycle Analytics component', () => {
       const defaultRequests = {
         fetchSummaryData: {
           status: defaultStatus,
-          endpoint: `/-/analytics/value_stream_analytics/summary`,
+          endpoint: mockData.endpoints.summaryData,
           response: [...mockData.summaryData],
         },
         fetchGroupStagesAndEvents: {
           status: defaultStatus,
-          endpoint: `/-/analytics/value_stream_analytics/stages`,
+          endpoint: mockData.endpoints.baseStagesEndpoint,
           response: { ...mockData.customizableStagesAndEvents },
         },
         fetchGroupLabels: {
           status: defaultStatus,
-          endpoint: `/groups/${groupId}/-/labels`,
+          endpoint: mockData.endpoints.groupLabels,
           response: [...mockData.groupLabels],
         },
         ...overrides,
@@ -432,26 +429,22 @@ describe('Cycle Analytics component', () => {
 
       if (mockFetchTasksByTypeData) {
         mock
-          .onGet(/analytics\/type_of_work\/tasks_by_type/)
+          .onGet(mockData.endpoints.tasksByTypeData)
           .reply(defaultStatus, { ...mockData.tasksByTypeData });
       }
 
       if (mockFetchDurationData) {
         mock
-          .onGet(/analytics\/value_stream_analytics\/stages\/\d+\/duration_chart/)
+          .onGet(mockData.endpoints.durationData)
           .reply(defaultStatus, [...mockData.rawDurationData]);
       }
 
       if (mockFetchStageMedian) {
-        mock
-          .onGet(/analytics\/value_stream_analytics\/stages\/\d+\/median/)
-          .reply(defaultStatus, { value: null });
+        mock.onGet(mockData.endpoints.stageMedian).reply(defaultStatus, { value: null });
       }
 
       if (mockFetchStageData) {
-        mock
-          .onGet(/analytics\/value_stream_analytics\/stages\/\d+\/records/)
-          .reply(defaultStatus, mockData.issueEvents);
+        mock.onGet(mockData.endpoints.stageData).reply(defaultStatus, mockData.issueEvents);
       }
 
       Object.values(defaultRequests).forEach(({ endpoint, status, response }) => {
@@ -487,7 +480,7 @@ describe('Cycle Analytics component', () => {
         overrides: {
           fetchSummaryData: {
             status: httpStatusCodes.NOT_FOUND,
-            endpoint: '/-/analytics/value_stream_analytics/summary',
+            endpoint: mockData.endpoints.summaryData,
             response: { response: { status: httpStatusCodes.NOT_FOUND } },
           },
         },
@@ -504,6 +497,7 @@ describe('Cycle Analytics component', () => {
       mockRequestCycleAnalyticsData({
         overrides: {
           fetchGroupLabels: {
+            endpoint: mockData.endpoints.groupLabels,
             status: httpStatusCodes.NOT_FOUND,
             response: { response: { status: httpStatusCodes.NOT_FOUND } },
           },
@@ -521,7 +515,7 @@ describe('Cycle Analytics component', () => {
       mockRequestCycleAnalyticsData({
         overrides: {
           fetchGroupStagesAndEvents: {
-            endPoint: '/-/analytics/value_stream_analytics/stages',
+            endPoint: mockData.endpoints.baseStagesEndpoint,
             status: httpStatusCodes.NOT_FOUND,
             response: { response: { status: httpStatusCodes.NOT_FOUND } },
           },

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

@@ -4,6 +4,12 @@ import { groupLabels } from '../mock_data';
 
 const selectedLabel = groupLabels[groupLabels.length - 1];
 
+const findActiveItem = wrapper =>
+  wrapper
+    .findAll('gl-dropdown-item-stub')
+    .filter(d => d.attributes('active'))
+    .at(0);
+
 describe('Value Stream Analytics LabelsSelector', () => {
   function createComponent({ props = {}, shallow = true } = {}) {
     const func = shallow ? shallowMount : mount;
@@ -16,7 +22,7 @@ describe('Value Stream Analytics LabelsSelector', () => {
   }
 
   let wrapper = null;
-  const labelNames = groupLabels.map(({ title }) => title);
+  const labelNames = groupLabels.map(({ name }) => name);
 
   describe('with no item selected', () => {
     beforeEach(() => {
@@ -25,14 +31,19 @@ describe('Value Stream Analytics LabelsSelector', () => {
 
     afterEach(() => {
       wrapper.destroy();
+      wrapper = null;
+    });
+
+    it('will render the label selector', () => {
+      expect(wrapper.html()).toMatchSnapshot();
     });
 
-    it.each(labelNames)('generate a label item for the label %s', title => {
-      expect(wrapper.text()).toContain(title);
+    it.each(labelNames)('generate a label item for the label %s', name => {
+      expect(wrapper.text()).toContain(name);
     });
 
     it('will render with the default option selected', () => {
-      const activeItem = wrapper.find('[active="true"]');
+      const activeItem = findActiveItem(wrapper);
 
       expect(activeItem.exists()).toBe(true);
       expect(activeItem.text()).toEqual('Select a label');
@@ -77,11 +88,15 @@ describe('Value Stream Analytics LabelsSelector', () => {
       wrapper.destroy();
     });
 
-    it('will set the active class', () => {
-      const activeItem = wrapper.find('[active="true"]');
+    it('will render the label selector', () => {
+      expect(wrapper.html()).toMatchSnapshot();
+    });
+
+    it('will set the active label', () => {
+      const activeItem = findActiveItem(wrapper);
 
       expect(activeItem.exists()).toBe(true);
-      expect(activeItem.text()).toEqual(selectedLabel.title);
+      expect(activeItem.text()).toEqual(selectedLabel.name);
     });
   });
 });

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

@@ -6,18 +6,30 @@ import * as types from 'ee/analytics/cycle_analytics/store/mutation_types';
 import { DEFAULT_DAYS_IN_PAST } from 'ee/analytics/cycle_analytics/constants';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { getDateInPast, getDatesInRange } from '~/lib/utils/datetime_utility';
-import { mockLabels } from '../../../../../spec/javascripts/vue_shared/components/sidebar/labels_select/mock_data';
 import { toYmd } from 'ee/analytics/shared/utils';
 import { transformRawTasksByTypeData } from 'ee/analytics/cycle_analytics/utils';
 
-const endpoints = {
+const fixtureEndpoints = {
   customizableCycleAnalyticsStagesAndEvents: 'analytics/value_stream_analytics/stages.json', // customizable stages and events endpoint
   stageEvents: stage => `analytics/value_stream_analytics/stages/${stage}/records.json`,
   stageMedian: stage => `analytics/value_stream_analytics/stages/${stage}/median.json`,
   summaryData: 'analytics/value_stream_analytics/summary.json',
+  groupLabels: 'api/group_labels.json',
 };
 
-export const groupLabels = mockLabels;
+export const endpoints = {
+  groupLabels: /groups\/[A-Z|a-z|\d|\-|_]+\/labels/,
+  summaryData: /analytics\/value_stream_analytics\/summary/,
+  durationData: /analytics\/value_stream_analytics\/stages\/\d+\/duration_chart/,
+  stageData: /analytics\/value_stream_analytics\/stages\/\d+\/records/,
+  stageMedian: /analytics\/value_stream_analytics\/stages\/\d+\/median/,
+  baseStagesEndpoint: /analytics\/value_stream_analytics\/stages$/,
+  tasksByTypeData: /analytics\/type_of_work\/tasks_by_type/,
+};
+
+export const groupLabels = getJSONFixture(fixtureEndpoints.groupLabels).map(
+  convertObjectPropsToCamelCase,
+);
 
 export const group = {
   id: 1,
@@ -30,10 +42,10 @@ export const group = {
 const getStageByTitle = (stages, title) =>
   stages.find(stage => stage.title && stage.title.toLowerCase().trim() === title) || {};
 
-export const summaryData = getJSONFixture(endpoints.summaryData);
+export const summaryData = getJSONFixture(fixtureEndpoints.summaryData);
 
 export const customizableStagesAndEvents = getJSONFixture(
-  endpoints.customizableCycleAnalyticsStagesAndEvents,
+  fixtureEndpoints.customizableCycleAnalyticsStagesAndEvents,
 );
 
 const dummyState = {};
@@ -56,7 +68,7 @@ const deepCamelCase = obj => convertObjectPropsToCamelCase(obj, { deep: true });
 export const defaultStages = ['issue', 'plan', 'review', 'code', 'test', 'staging', 'production'];
 
 const stageFixtures = defaultStages.reduce((acc, stage) => {
-  const events = getJSONFixture(endpoints.stageEvents(stage));
+  const events = getJSONFixture(fixtureEndpoints.stageEvents(stage));
   return {
     ...acc,
     [stage]: events,
@@ -64,7 +76,7 @@ const stageFixtures = defaultStages.reduce((acc, stage) => {
 }, {});
 
 export const stageMedians = defaultStages.reduce((acc, stage) => {
-  const { value } = getJSONFixture(endpoints.stageMedian(stage));
+  const { value } = getJSONFixture(fixtureEndpoints.stageMedian(stage));
   return {
     ...acc,
     [stage]: value,

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

@@ -22,6 +22,7 @@ import {
   rawDurationMedianData,
   transformedDurationData,
   transformedDurationMedianData,
+  endpoints,
 } from '../mock_data';
 
 const stageData = { events: [] };
@@ -30,14 +31,6 @@ const flashErrorMessage = 'There was an error while fetching value stream analyt
 const selectedGroup = { fullPath: group.path };
 const [selectedStage] = stages;
 const selectedStageSlug = selectedStage.slug;
-const endpoints = {
-  groupLabels: `/groups/${group.path}/-/labels`,
-  summaryData: '/analytics/value_stream_analytics/summary',
-  durationData: /analytics\/value_stream_analytics\/stages\/\d+\/duration_chart/,
-  stageData: /analytics\/value_stream_analytics\/stages\/\d+\/records/,
-  stageMedian: /analytics\/value_stream_analytics\/stages\/\d+\/median/,
-  baseStagesEndpoint: '/analytics/value_stream_analytics/stages',
-};
 
 const stageEndpoint = ({ stageId }) => `/-/analytics/value_stream_analytics/stages/${stageId}`;
 
@@ -253,8 +246,8 @@ describe('Cycle analytics actions', () => {
     beforeEach(() => {
       setFixtures('<div class="flash-container"></div>');
     });
-    it(`commits the ${types.RECEIVE_STAGE_DATA_ERROR} mutation`, done => {
-      testAction(
+    it(`commits the ${types.RECEIVE_STAGE_DATA_ERROR} mutation`, () => {
+      return testAction(
         actions.receiveStageDataError,
         null,
         state,
@@ -264,7 +257,6 @@ describe('Cycle analytics actions', () => {
           },
         ],
         [],
-        done,
       );
     });
 
@@ -278,13 +270,15 @@ describe('Cycle analytics actions', () => {
   });
 
   describe('fetchGroupLabels', () => {
+    describe('succeeds', () => {
       beforeEach(() => {
-      state = { ...state, selectedGroup };
+        gon.api_version = 'v4';
+        state = { selectedGroup };
         mock.onGet(endpoints.groupLabels).replyOnce(200, groupLabels);
       });
 
-    it('dispatches receiveGroupLabels if the request succeeds', done => {
-      testAction(
+      it('dispatches receiveGroupLabels if the request succeeds', () => {
+        return testAction(
           actions.fetchGroupLabels,
           null,
           state,
@@ -296,15 +290,21 @@ describe('Cycle analytics actions', () => {
               payload: groupLabels,
             },
           ],
-        done,
         );
       });
+    });
 
-    it('dispatches receiveGroupLabelsError if the request fails', done => {
-      testAction(
+    describe('with an error', () => {
+      beforeEach(() => {
+        state = { selectedGroup };
+        mock.onGet(endpoints.groupLabels).replyOnce(404);
+      });
+
+      it('dispatches receiveGroupLabelsError if the request fails', () => {
+        return testAction(
           actions.fetchGroupLabels,
           null,
-        { ...state, selectedGroup: { fullPath: null } },
+          state,
           [],
           [
             { type: 'requestGroupLabels' },
@@ -313,14 +313,15 @@ describe('Cycle analytics actions', () => {
               payload: error,
             },
           ],
-        done,
         );
       });
+    });
 
     describe('receiveGroupLabelsError', () => {
       beforeEach(() => {
         setFixtures('<div class="flash-container"></div>');
       });
+
       it('flashes an error message if the request fails', () => {
         actions.receiveGroupLabelsError({
           commit: () => {},

ee/spec/frontend/api_spec.js

@@ -543,6 +543,22 @@ describe('Api', () => {
           .catch(done.fail);
       });
     });
+
+    describe('cycleAnalyticsGroupLabels', () => {
+      it('fetches group level labels', done => {
+        const response = [];
+        const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${groupId}/labels`;
+        mock.onGet(expectedUrl).reply(200, response);
+
+        Api.cycleAnalyticsGroupLabels(groupId)
+          .then(({ data, config: { url } }) => {
+            expect(data).toEqual(response);
+            expect(url).toEqual(expectedUrl);
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
   });
 
   describe('GeoDesigns', () => {

spec/frontend/fixtures/labels.rb

@@ -41,6 +41,23 @@ describe 'Labels (JavaScript fixtures)' do
     end
   end
 
+  describe API::Helpers::LabelHelpers, type: :request do
+    include JavaScriptFixturesHelpers
+    include ApiHelpers
+
+    let(:user) { create(:user) }
+
+    before do
+      group.add_owner(user)
+    end
+
+    it 'api/group_labels.json' do
+      get api("/groups/#{group.id}/labels", user)
+
+      expect(response).to be_successful
+    end
+  end
+
   describe Projects::LabelsController, '(JavaScript fixtures)', type: :controller do
     render_views