Set stage id correctly

Checks if the stage is persisted, if it is
it will use the stage id, otherwise the stage
title should be used as the id for stage data
requests.

Minor cleanup and linting

Remove unneeded getter

Minor clean up

Added tests for convertObjectKeysToSnakeCase

Address minor review comments

Cleanup event handlers

Moved convertObjectKeysToSnakeCase function to
common_utils, this was previously located in
text_utility.

Filter for stage stats by stage name

app/assets/javascripts/lib/utils/common_utils.js

@@ -5,7 +5,7 @@
 import $ from 'jquery';
 import axios from './axios_utils';
 import { getLocationHash } from './url_utility';
-import { convertToCamelCase } from './text_utility';
+import { convertToCamelCase, convertToSnakeCase } from './text_utility';
 import { isObject } from './type_utility';
 import breakpointInstance from '../../breakpoints';
 
@@ -697,6 +697,20 @@ export const convertObjectPropsToCamelCase = (obj = {}, options = {}) => {
   }, initial);
 };
 
+/**
+ * Converts all the object keys to snake case
+ *
+ * @param {Object} obj    Object to transform
+ * @returns {Object}
+ */
+export const convertObjectKeysToSnakeCase = (obj = {}) =>
+  obj
+    ? Object.entries(obj).reduce(
+        (acc, [key, value]) => ({ ...acc, [convertToSnakeCase(key)]: value }),
+        {},
+      )
+    : {};
+
 export const imagePath = imgUrl =>
   `${gon.asset_host || ''}${gon.relative_url_root || ''}/assets/${imgUrl}`;
 

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

@@ -70,13 +70,9 @@ export default {
       'endDate',
       'tasksByType',
     ]),
-    ...mapGetters([
-      'defaultStage',
-      'hasNoAccessError',
-      'durationChartPlottableData',
-    ]),
+    ...mapGetters(['hasNoAccessError', 'currentGroupPath', 'durationChartPlottableData']),
     shouldRenderEmptyState() {
-      return !this.selectedGroup;
+      return !this.selectedGroup;g
     },
     hasCustomizableCycleAnalytics() {
       return Boolean(this.glFeatures.customizableCycleAnalytics);
@@ -137,7 +133,7 @@ export default {
     onStageSelect(stage) {
       this.hideCustomStageForm();
       this.setSelectedStage(stage);
-      this.fetchStageData(this.selectedStage);
+      this.fetchStageData(this.selectedStage.slug);
     },
     onShowAddStageForm() {
       this.showCustomStageForm();

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

@@ -2,7 +2,7 @@
 import { isEqual } from 'underscore';
 import { GlButton, GlFormGroup, GlFormInput, GlFormSelect, GlLoadingIcon } from '@gitlab/ui';
 import { s__ } from '~/locale';
-import { convertToSnakeCase } from '~/lib/utils/text_utility';
+import { convertObjectKeysToSnakeCase } from '~/lib/utils/common_utils';
 import LabelsSelector from './labels_selector.vue';
 import { STAGE_ACTIONS } from '../constants';
 import {
@@ -23,13 +23,6 @@ const initFields = {
   endEventLabelId: null,
 };
 
-// TODO: should be a util / use a util if exists...
-const snakeFields = (fields = {}) =>
-  Object.entries(fields).reduce((acc, curr) => {
-    const [key, value] = curr;
-    return { ...acc, [convertToSnakeCase(key)]: value };
-  }, {});
-
 export default {
   components: {
     GlButton,
@@ -97,17 +90,25 @@ export default {
       return isLabelEvent(this.labelEvents, this.fields.endEventIdentifier);
     },
     isComplete() {
-      if (!this.hasValidStartAndEndEventPair) return false;
-      const requiredFields = [
-        this.fields.startEventIdentifier,
-        this.fields.endEventIdentifier,
-        this.fields.name,
-      ];
+      if (!this.hasValidStartAndEndEventPair) {
+        return false;
+      }
+      const {
+        fields: {
+          name,
+          startEventIdentifier,
+          startEventLabelId,
+          endEventIdentifier,
+          endEventLabelId,
+        },
+      } = this;
+
+      const requiredFields = [startEventIdentifier, endEventIdentifier, name];
       if (this.startEventRequiresLabel) {
-        requiredFields.push(this.fields.startEventLabelId);
+        requiredFields.push(startEventLabelId);
       }
       if (this.endEventRequiresLabel) {
-        requiredFields.push(this.fields.endEventLabelId);
+        requiredFields.push(endEventLabelId);
       }
       return requiredFields.every(
         fieldValue => fieldValue && (fieldValue.length > 0 || fieldValue > 0),
@@ -151,7 +152,7 @@ export default {
       this.$emit('cancel');
     },
     handleSave() {
-      const data = snakeFields(this.fields);
+      const data = convertObjectKeysToSnakeCase(this.fields);
       if (this.isEditingCustomStage) {
         const { id } = this.initialFields;
         this.$emit(STAGE_ACTIONS.UPDATE, { ...data, id });
@@ -159,7 +160,7 @@ export default {
         this.$emit(STAGE_ACTIONS.CREATE, data);
       }
     },
-    handleSelectLabel(key, labelId = null) {
+    handleSelectLabel(key, labelId) {
       this.fields[key] = labelId;
     },
     handleClearLabel(key) {
@@ -200,7 +201,7 @@ export default {
             :labels="labels"
             :selected-label-id="fields.startEventLabelId"
             name="custom-stage-start-event-label"
-            @selectLabel="labelId => handleSelectLabel('startEventLabelId', labelId)"
+            @selectLabel="handleSelectLabel('startEventLabelId', $event)"
             @clearLabel="handleClearLabel('startEventLabelId')"
           />
         </gl-form-group>
@@ -231,7 +232,7 @@ export default {
             :labels="labels"
             :selected-label-id="fields.endEventLabelId"
             name="custom-stage-stop-event-label"
-            @selectLabel="labelId => handleSelectLabel('endEventLabelId', labelId)"
+            @selectLabel="handleSelectLabel('endEventLabelId', $event)"
             @clearLabel="handleClearLabel('endEventLabelId')"
           />
         </gl-form-group>

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

@@ -122,7 +122,12 @@ export default {
     },
   },
   methods: {
-    // TODO: DRY These up
+    selectStage(stage) {
+      this.$emit(STAGE_ACTIONS.SELECT, stage);
+    },
+    editStage(stage) {
+      this.$emit(STAGE_ACTIONS.EDIT, stage);
+    },
     hideStage(stageId) {
       this.$emit(STAGE_ACTIONS.HIDE, { id: stageId, hidden: true });
     },
@@ -163,8 +168,8 @@ export default {
               :is-default-stage="!stage.custom"
               @remove="removeStage(stage.id)"
               @hide="hideStage(stage.id)"
-              @select="$emit($options.STAGE_ACTIONS.SELECT, stage)"
-              @edit="$emit($options.STAGE_ACTIONS.EDIT, stage)"
+              @select="selectStage(stage)"
+              @edit="editStage(stage)"
             />
             <add-stage-button
               v-if="canEditStages"

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

@@ -37,9 +37,8 @@ export const receiveStageDataError = ({ commit }) => {
   createFlash(__('There was an error fetching data for the selected stage'));
 };
 
-export const fetchStageData = ({ state, dispatch, getters }, stage) => {
+export const fetchStageData = ({ state, dispatch, getters }, slug) => {
   const { cycleAnalyticsRequestParams = {} } = getters;
-  const { id, slug } = stage;
   const {
     selectedGroup: { fullPath },
   } = state;
@@ -48,7 +47,7 @@ export const fetchStageData = ({ state, dispatch, getters }, stage) => {
 
   return Api.cycleAnalyticsStageEvents(
     fullPath,
-    stage.custom ? id : slug,
+    slug,
     nestQueryStringKeys(cycleAnalyticsRequestParams, 'cycle_analytics'),
   )
     .then(({ data }) => dispatch('receiveStageDataSuccess', data))
@@ -89,12 +88,8 @@ export const hideCustomStageForm = ({ commit }) => commit(types.HIDE_CUSTOM_STAG
 export const showCustomStageForm = ({ commit }) => commit(types.SHOW_CUSTOM_STAGE_FORM);
 
 export const editCustomStage = ({ commit, dispatch }, selectedStage = {}) => {
-  // TODO: should this be in the if
   commit(types.EDIT_CUSTOM_STAGE);
-
-  if (selectedStage.id) {
-    dispatch('setSelectedStage', selectedStage);
-  }
+  dispatch('setSelectedStage', selectedStage);
 };
 
 export const requestSummaryData = ({ commit }) => commit(types.REQUEST_SUMMARY_DATA);
@@ -147,8 +142,8 @@ export const fetchGroupLabels = ({ dispatch, state }) => {
     .catch(error => dispatch('receiveGroupLabelsError', error));
 };
 
-export const receiveGroupStagesAndEventsError = ({ commit }) => {
-  commit(types.RECEIVE_GROUP_STAGES_AND_EVENTS_ERROR);
+export const receiveGroupStagesAndEventsError = ({ commit }, error) => {
+  commit(types.RECEIVE_GROUP_STAGES_AND_EVENTS_ERROR, error);
   createFlash(__('There was an error fetching cycle analytics stages.'));
 };
 
@@ -156,8 +151,9 @@ export const receiveGroupStagesAndEventsSuccess = ({ state, commit, dispatch },
   commit(types.RECEIVE_GROUP_STAGES_AND_EVENTS_SUCCESS, data);
   const { stages = [] } = state;
   if (stages && stages.length) {
-    dispatch('setSelectedStage', stages[0]);
-    dispatch('fetchStageData', stages[0]);
+    const [firstStage] = stages;
+    dispatch('setSelectedStage', firstStage);
+    dispatch('fetchStageData', firstStage.slug);
   } else {
     createFlash(__('There was an error while fetching cycle analytics data.'));
   }

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

@@ -3,8 +3,6 @@ import httpStatus from '~/lib/utils/http_status';
 import { dateFormats } from '../../shared/constants';
 import { getDurationChartData } from '../utils';
 
-export const defaultStage = ({ stages = [] }) => (stages.length ? stages[0] : null);
-
 export const hasNoAccessError = state => state.errorCode === httpStatus.FORBIDDEN;
 
 export const currentGroupPath = ({ selectedGroup }) =>

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

@@ -32,13 +32,27 @@ export const isLabelEvent = (labelEvents = [], ev = null) =>
 export const getLabelEventsIdentifiers = (events = []) =>
   events.filter(ev => ev.type && ev.type === EVENT_TYPE_LABEL).map(i => i.identifier);
 
+/**
+ * Checks if the specified stage is in memory or persisted to storage based on the id
+ *
+ * Default cycle analytics stages are initially stored in memory, when they are first
+ * created the id for the stage is the name of the stage in lowercase. This string id
+ * is used to fetch stage data (events, median calculation)
+ *
+ * When either a custom stage is created or an edit is made to a default stage then the
+ * default stages get persisted to storage and will have a numeric id. The new numeric
+ * id should then be used to access stage data
+ *
+ * This will be fixed in https://gitlab.com/gitlab-org/gitlab/merge_requests/19278
+ */
+
 export const transformRawStages = (stages = []) =>
   stages
-    .map(({ title, name = null, ...rest }) => ({
+    .map(({ id, title, custom = false, ...rest }) => ({
       ...convertObjectPropsToCamelCase(rest, { deep: true }),
-      slug: convertToSnakeCase(title),
+      id,
       title,
-      name: name || title,
+      slug: custom ? id : convertToSnakeCase(title),
     }))
     .sort((a, b) => a.id > b.id);
 

ee/spec/features/analytics/cycle_analytics/cycle_analytics_spec.rb

@@ -377,7 +377,7 @@ describe 'Group Cycle Analytics', :js do
         context 'with changes' do
           before do
             select_edit_stage
-          end 
+          end
 
           it 'enables the submit button' do
             fill_in name_field, with: updated_custom_stage_name

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

@@ -16,7 +16,6 @@ import Scatterplot from 'ee/analytics/shared/components/scatterplot.vue';
 import waitForPromises from 'helpers/wait_for_promises';
 import httpStatusCodes from '~/lib/utils/http_status';
 import * as mockData from '../mock_data';
-import Api from 'ee/api';
 
 const noDataSvgPath = 'path/to/no/data';
 const noAccessSvgPath = 'path/to/no/access';

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

@@ -23,10 +23,11 @@ const error = new Error('Request failed with status code 404');
 const flashErrorMessage = 'There was an error while fetching cycle analytics data.';
 const selectedGroup = { fullPath: group.path };
 const [selectedStage] = stages;
+const selectedStageSlug = selectedStage.slug;
 const endpoints = {
   groupLabels: `/groups/${group.path}/-/labels`,
   cycleAnalyticsData: `/groups/${group.path}/-/cycle_analytics`,
-  stageData: `/groups/${group.path}/-/cycle_analytics/events/${selectedStage.slug}.json`,
+  stageData: `/groups/${group.path}/-/cycle_analytics/events/${selectedStageSlug}.json`,
   baseStagesEndpoint: '/-/analytics/cycle_analytics/stages',
 };
 
@@ -100,7 +101,7 @@ describe('Cycle analytics actions', () => {
     it('dispatches receiveStageDataSuccess with received data on success', done => {
       testAction(
         actions.fetchStageData,
-        selectedStage,
+        selectedStageSlug,
         state,
         [],
         [
@@ -397,7 +398,7 @@ describe('Cycle analytics actions', () => {
     });
 
     it('will flash an error when there are no stages', () => {
-      [([], null)].forEach(emptyStages => {
+      [[], null].forEach(emptyStages => {
         actions.receiveGroupStagesAndEventsSuccess(
           {
             commit: () => {},
@@ -503,7 +504,7 @@ describe('Cycle analytics actions', () => {
         ],
         [
           { type: 'setSelectedStage', payload: selectedStage },
-          { type: 'fetchStageData', payload: selectedStage },
+          { type: 'fetchStageData', payload: selectedStageSlug },
         ],
         done,
       );

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

 import * as getters from 'ee/analytics/cycle_analytics/store/getters';
 import {
-  allowedStages as stages,
   startDate,
   endDate,
   transformedDurationData,
@@ -11,44 +10,6 @@ let state = null;
 const selectedProjectIds = [5, 8, 11];
 
 describe('Cycle analytics getters', () => {
-  describe('with default state', () => {
-    beforeEach(() => {
-      state = {
-        stages: [],
-        selectedStage: null,
-      };
-    });
-
-    afterEach(() => {
-      state = null;
-    });
-
-    describe('defaultStage', () => {
-      it('will return null', () => {
-        expect(getters.defaultStage(state)).toEqual(null);
-      });
-    });
-  });
-
-  describe('with a set of stages', () => {
-    beforeEach(() => {
-      state = {
-        stages,
-        selectedStage: null,
-      };
-    });
-
-    afterEach(() => {
-      state = null;
-    });
-
-    describe('defaultStage', () => {
-      it('will return the first stage', () => {
-        expect(getters.defaultStage(state)).toEqual(stages[0]);
-      });
-    });
-  });
-
   describe('hasNoAccessError', () => {
     beforeEach(() => {
       state = {

locale/gitlab.pot

@@ -5196,9 +5196,6 @@ msgstr ""
 msgid "CustomCycleAnalytics|Add stage"
 msgstr ""
 
-msgid "CustomCycleAnalytics|Edit stage"
-msgstr ""
-
 msgid "CustomCycleAnalytics|Editing stage"
 msgstr ""
 
@@ -5235,6 +5232,9 @@ msgstr ""
 msgid "CustomCycleAnalytics|Stop event label"
 msgstr ""
 
+msgid "CustomCycleAnalytics|Update stage"
+msgstr ""
+
 msgid "Customize colors"
 msgstr ""
 

spec/javascripts/lib/utils/common_utils_spec.js

@@ -721,6 +721,28 @@ describe('common_utils', () => {
       });
     });
 
+    describe('convertObjectKeysToSnakeCase', () => {
+      it('converts each object key to snake case', () => {
+        const obj = {
+          some: 'some',
+          'cool object': 'cool object',
+          likeThisLongOne: 'likeThisLongOne',
+        };
+
+        expect(commonUtils.convertObjectKeysToSnakeCase(obj)).toEqual({
+          some: 'some',
+          cool_object: 'cool object',
+          like_this_long_one: 'likeThisLongOne',
+        });
+      });
+
+      it('returns an empty object if there are no keys', () => {
+        ['', {}, [], null].forEach(badObj => {
+          expect(commonUtils.convertObjectKeysToSnakeCase(badObj)).toEqual({});
+        });
+      });
+    });
+
     describe('with options', () => {
       const objWithoutChildren = {
         project_name: 'GitLab CE',