Merge branch 'fix-inline-error-display-when-creating-custom-stage' into 'master'

Fix inline error display when creating a custom stage

See merge request gitlab-org/gitlab!25509

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

@@ -13,7 +13,7 @@ import {
 import { s__ } from '~/locale';
 import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
 import LabelsSelector from './labels_selector.vue';
-import { STAGE_ACTIONS } from '../constants';
+import { STAGE_ACTIONS, DEFAULT_STAGE_NAMES } from '../constants';
 import {
   isStartEvent,
   isLabelEvent,
@@ -32,6 +32,28 @@ const defaultFields = {
   endEventLabelId: null,
 };
 
+export const initializeFormData = ({ emptyFieldState, initialFields, errors }) => {
+  const defaultErrors = initialFields?.endEventIdentifier
+    ? { ...emptyFieldState, endEventIdentifier: null }
+    : {
+        ...emptyFieldState,
+        endEventIdentifier:
+          initialFields && !initialFields.startEventIdentifier
+            ? [s__('CustomCycleAnalytics|Please select a start event first')]
+            : null,
+      };
+  return {
+    fields: {
+      ...emptyFieldState,
+      ...initialFields,
+    },
+    fieldErrors: {
+      ...defaultErrors,
+      ...errors,
+    },
+  };
+};
+
 export default {
   components: {
     GlFormGroup,
@@ -70,24 +92,20 @@ export default {
     errors: {
       type: Object,
       required: false,
-      default: () => ({}),
+      default: null,
     },
   },
   data() {
-    const defaultErrors = this?.initialFields?.endEventIdentifier
-      ? {}
-      : { endEventIdentifier: [s__('CustomCycleAnalytics|Please select a start event first')] };
+    const { initialFields = {}, errors = null } = this;
+    const { fields, fieldErrors } = initializeFormData({
+      emptyFieldState: defaultFields,
+      initialFields,
+      errors,
+    });
     return {
       labelEvents: getLabelEventsIdentifiers(this.events),
-      fields: {
-        ...defaultFields,
-        ...this.initialFields,
-      },
-      fieldErrors: {
-        ...defaultFields,
-        ...this.errors,
-        ...defaultErrors,
-      },
+      fields,
+      fieldErrors,
     };
   },
   computed: {
@@ -114,8 +132,14 @@ export default {
     endEventRequiresLabel() {
       return isLabelEvent(this.labelEvents, this.fields.endEventIdentifier);
     },
+    hasErrors() {
+      return (
+        this.eventMismatchError ||
+        Object.values(this.fieldErrors).some(errArray => errArray?.length)
+      );
+    },
     isComplete() {
-      if (this.eventMismatchError) {
+      if (this.hasErrors) {
         return false;
       }
       const {
@@ -171,18 +195,24 @@ export default {
         ...defaultFields,
         ...newFields,
       };
+    },
+    errors(newErrors) {
       this.fieldErrors = {
         ...defaultFields,
-        ...this.errors,
+        ...newErrors,
       };
     },
   },
   methods: {
     handleCancel() {
-      this.fields = {
-        ...defaultFields,
-        ...this.initialFields,
-      };
+      const { initialFields = {}, errors = null } = this;
+      const formData = initializeFormData({
+        emptyFieldState: defaultFields,
+        initialFields,
+        errors,
+      });
+      this.$set(this, 'fields', formData.fields);
+      this.$set(this, 'fieldErrors', formData.fieldErrors);
       this.$emit('cancel');
     },
     handleSave() {
@@ -206,6 +236,15 @@ export default {
     fieldErrorMessage(key) {
       return this.fieldErrors[key]?.join('\n');
     },
+    onUpdateNameField() {
+      if (DEFAULT_STAGE_NAMES.includes(this.fields.name.toLowerCase())) {
+        this.$set(this.fieldErrors, 'name', [
+          s__('CustomCycleAnalytics|Stage name already exists'),
+        ]);
+      } else {
+        this.$set(this.fieldErrors, 'name', []);
+      }
+    },
     onUpdateStartEventField() {
       const initVal = this.initialFields?.endEventIdentifier
         ? this.initialFields.endEventIdentifier
@@ -255,6 +294,7 @@ export default {
         name="custom-stage-name"
         :placeholder="s__('CustomCycleAnalytics|Enter a name for the stage')"
         required
+        @change.native="onUpdateNameField"
       />
     </gl-form-group>
     <div class="d-flex" :class="{ 'justify-content-between': startEventRequiresLabel }">
@@ -298,7 +338,6 @@ export default {
           :label="s__('CustomCycleAnalytics|Stop event')"
           :state="!hasFieldErrors('endEventIdentifier')"
           :invalid-feedback="fieldErrorMessage('endEventIdentifier')"
-          @change.native="onUpdateEndEventField"
         >
           <gl-form-select
             v-model="fields.endEventIdentifier"
@@ -306,6 +345,7 @@ export default {
             :options="endEventOptions"
             :required="true"
             :disabled="!hasStartEvent"
+            @change.native="onUpdateEndEventField"
           />
         </gl-form-group>
       </div>

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

@@ -30,6 +30,8 @@ export const EMPTY_STAGE_TEXT = {
   ),
 };
 
+export const DEFAULT_STAGE_NAMES = [...Object.keys(EMPTY_STAGE_TEXT), 'total'];
+
 export const TASKS_BY_TYPE_SUBJECT_ISSUE = 'Issue';
 export const TASKS_BY_TYPE_SUBJECT_MERGE_REQUEST = 'MergeRequest';
 

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

@@ -488,11 +488,10 @@ describe 'Group Value Stream Analytics', :js do
           end
 
           it 'with a default name' do
-            name = 'issue'
-            fill_in 'custom-stage-name', with: name
+            fill_in 'custom-stage-name', with: 'issue'
             click_button 'Add stage'
 
-            expect(page.find('.flash-alert')).to have_text("'#{name}' stage already exists")
+            expect(page).to have_button('Add stage', disabled: true)
           end
         end
 
@@ -616,11 +615,10 @@ describe 'Group Value Stream Analytics', :js do
           end
 
           it 'with a default name' do
-            name = 'issue'
-            fill_in name_field, with: name
+            fill_in name_field, with: 'issue'
             page.find(stage_save_button).click
 
-            expect(page.find('.flash-alert')).to have_text("'#{name}' stage already exists")
+            expect(page.find(stage_form_class)).to have_text("Stage name already exists")
           end
         end
       end

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

@@ -7,7 +7,7 @@ exports[`CustomStageForm Editing a custom stage isSavingCustomStage=true display
 `;
 
 exports[`CustomStageForm Start event with events does not select events with canBeStartEvent=false for the start events dropdown 1`] = `
-"<select name=\\"custom-stage-start-event\\" required=\\"required\\" aria-required=\\"true\\" class=\\"gl-form-select custom-select\\" id=\\"__BVID__177\\">
+"<select name=\\"custom-stage-start-event\\" required=\\"required\\" aria-required=\\"true\\" class=\\"gl-form-select custom-select\\" id=\\"__BVID__257\\">
   <option value=\\"\\">Select start event</option>
   <option value=\\"issue_created\\">Issue created</option>
   <option value=\\"issue_first_mentioned_in_commit\\">Issue first mentioned in a commit</option>
@@ -30,7 +30,7 @@ exports[`CustomStageForm Start event with events does not select events with can
 `;
 
 exports[`CustomStageForm Start event with events selects events with canBeStartEvent=true for the start events dropdown 1`] = `
-"<select name=\\"custom-stage-start-event\\" required=\\"required\\" aria-required=\\"true\\" class=\\"gl-form-select custom-select\\" id=\\"__BVID__137\\">
+"<select name=\\"custom-stage-start-event\\" required=\\"required\\" aria-required=\\"true\\" class=\\"gl-form-select custom-select\\" id=\\"__BVID__217\\">
   <option value=\\"\\">Select start event</option>
   <option value=\\"issue_created\\">Issue created</option>
   <option value=\\"issue_first_mentioned_in_commit\\">Issue first mentioned in a commit</option>

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

@@ -2,7 +2,9 @@ import Vue from 'vue';
 import Vuex from 'vuex';
 import createStore from 'ee/analytics/cycle_analytics/store';
 import { createLocalVue, mount } from '@vue/test-utils';
-import CustomStageForm from 'ee/analytics/cycle_analytics/components/custom_stage_form.vue';
+import CustomStageForm, {
+  initializeFormData,
+} from 'ee/analytics/cycle_analytics/components/custom_stage_form.vue';
 import { STAGE_ACTIONS } from 'ee/analytics/cycle_analytics/constants';
 import {
   groupLabels,
@@ -77,6 +79,15 @@ describe('CustomStageForm', () => {
     });
   }
 
+  const findNameField = _wrapper => _wrapper.find({ ref: 'name' });
+  const findNameFieldInput = _wrapper => _wrapper.find(sel.name);
+
+  function setNameField(_wrapper, value = '') {
+    findNameFieldInput(_wrapper).setValue(value);
+    findNameFieldInput(_wrapper).trigger('change');
+    return _wrapper.vm.$nextTick();
+  }
+
   beforeEach(() => {
     wrapper = createComponent({});
   });
@@ -104,6 +115,25 @@ describe('CustomStageForm', () => {
     });
   });
 
+  describe('Name', () => {
+    describe('with a reserved name', () => {
+      beforeEach(() => {
+        wrapper = createComponent({});
+        return setNameField(wrapper, 'issue');
+      });
+
+      it('displays an error', () => {
+        expect(findNameField(wrapper).text()).toContain('Stage name already exists');
+      });
+
+      it('clears the error when the field changes', () => {
+        return setNameField(wrapper, 'not an issue').then(() => {
+          expect(findNameField(wrapper).text()).not.toContain('Stage name already exists');
+        });
+      });
+    });
+  });
+
   describe('Start event', () => {
     describe('with events', () => {
       beforeEach(() => {
@@ -163,7 +193,6 @@ describe('CustomStageForm', () => {
         expect(wrapper.vm.fields.startEventLabelId).toEqual(null);
 
         wrapper.find(sel.startEvent).setValue(labelStartEvent.identifier);
-        // TODO: make func for setting single field
         return Vue.nextTick()
           .then(() => {
             wrapper
@@ -749,4 +778,118 @@ describe('CustomStageForm', () => {
       });
     });
   });
+
+  describe('initializeFormData', () => {
+    describe('without a startEventIdentifier', () => {
+      it('with no errors', () => {
+        const res = initializeFormData({
+          initialFields: {},
+        });
+        expect(res.fields).toEqual({});
+        expect(res.fieldErrors).toEqual({
+          endEventIdentifier: ['Please select a start event first'],
+        });
+      });
+
+      it('with field errors', () => {
+        const res = initializeFormData({
+          initialFields: {},
+          errors: {
+            name: ['is reserved'],
+          },
+        });
+        expect(res.fields).toEqual({});
+        expect(res.fieldErrors).toEqual({
+          endEventIdentifier: ['Please select a start event first'],
+          name: ['is reserved'],
+        });
+      });
+    });
+
+    describe('with a startEventIdentifier', () => {
+      it('with no errors', () => {
+        const res = initializeFormData({
+          initialFields: {
+            startEventIdentifier: 'start-event',
+          },
+          errors: {},
+        });
+        expect(res.fields).toEqual({ startEventIdentifier: 'start-event' });
+        expect(res.fieldErrors).toEqual({
+          endEventIdentifier: null,
+        });
+      });
+
+      it('with field errors', () => {
+        const res = initializeFormData({
+          initialFields: {
+            startEventIdentifier: 'start-event',
+          },
+          errors: {
+            name: ['is reserved'],
+          },
+        });
+        expect(res.fields).toEqual({ startEventIdentifier: 'start-event' });
+        expect(res.fieldErrors).toEqual({
+          endEventIdentifier: null,
+          name: ['is reserved'],
+        });
+      });
+    });
+
+    describe('with all fields set', () => {
+      it('with no errors', () => {
+        const res = initializeFormData({
+          initialFields: {
+            id: 1,
+            name: 'cool-stage',
+            startEventIdentifier: 'start-event',
+            endEventIdentifier: 'end-event',
+            startEventLabelId: 10,
+            endEventLabelId: 20,
+          },
+          errors: {},
+        });
+        expect(res.fields).toEqual({
+          id: 1,
+          name: 'cool-stage',
+          startEventIdentifier: 'start-event',
+          endEventIdentifier: 'end-event',
+          startEventLabelId: 10,
+          endEventLabelId: 20,
+        });
+        expect(res.fieldErrors).toEqual({
+          endEventIdentifier: null,
+        });
+      });
+
+      it('with field errors', () => {
+        const res = initializeFormData({
+          initialFields: {
+            id: 1,
+            name: 'cool-stage',
+            startEventIdentifier: 'start-event',
+            endEventIdentifier: 'end-event',
+            startEventLabelId: 10,
+            endEventLabelId: 20,
+          },
+          errors: {
+            name: ['is reserved'],
+          },
+        });
+        expect(res.fields).toEqual({
+          id: 1,
+          name: 'cool-stage',
+          startEventIdentifier: 'start-event',
+          endEventIdentifier: 'end-event',
+          startEventLabelId: 10,
+          endEventLabelId: 20,
+        });
+        expect(res.fieldErrors).toEqual({
+          endEventIdentifier: null,
+          name: ['is reserved'],
+        });
+      });
+    });
+  });
 });

locale/gitlab.pot

@@ -5848,6 +5848,9 @@ msgstr ""
 msgid "CustomCycleAnalytics|Select stop event"
 msgstr ""
 
+msgid "CustomCycleAnalytics|Stage name already exists"
+msgstr ""
+
 msgid "CustomCycleAnalytics|Start event"
 msgstr ""