Update the create value stream actions

Submits the stages configuration along
with the value stream name

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

@@ -125,6 +125,7 @@ export const DEFAULT_STAGE_CONFIG = BASE_DEFAULT_STAGE_CONFIG.map(({ id, ...rest
 }));
 
 export const PRESET_OPTIONS_DEFAULT = 'default';
+export const PRESET_OPTIONS_BLANK = 'blank';
 export const PRESET_OPTIONS = [
   {
     text: I18N.TEMPLATE_DEFAULT,
@@ -132,6 +133,6 @@ export const PRESET_OPTIONS = [
   },
   {
     text: I18N.TEMPLATE_BLANK,
-    value: 'blank',
+    value: PRESET_OPTIONS_BLANK,
   },
 ];

ee/app/assets/javascripts/analytics/cycle_analytics/components/create_value_stream_form/custom_stage_fields.vue

@@ -32,7 +32,7 @@ export default {
     errors: {
       type: Object,
       required: false,
-      default: () => {},
+      default: () => ({}),
     },
     stageEvents: {
       type: Array,

ee/app/assets/javascripts/analytics/cycle_analytics/components/create_value_stream_form/default_stage_fields.vue

@@ -40,7 +40,7 @@ export default {
     errors: {
       type: Object,
       required: false,
-      default: () => {},
+      default: () => ({}),
     },
     stageEvents: {
       type: Array,
@@ -49,10 +49,10 @@ export default {
   },
   methods: {
     isValid(field) {
-      return !this.errors[field]?.length;
+      return !this.errors[field] || !this.errors[field]?.length;
     },
     renderError(field) {
-      return this.errors[field]?.join('\n');
+      return this.errors[field] ? this.errors[field]?.join('\n') : null;
     },
     eventName(eventIds = []) {
       return eventIdsToName(this.stageEvents, eventIds);
@@ -68,6 +68,7 @@ export default {
         class="gl-flex-grow-1 gl-mb-0"
         :state="isValid('name')"
         :invalid-feedback="renderError('name')"
+        :data-testid="`default-stage-name-${index}`"
       >
         <!-- eslint-disable vue/no-mutating-props -->
         <gl-form-input

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

@@ -22,6 +22,47 @@ const findStageIndexByName = (stages, target = '') =>
 const initializeStageErrors = (selectedPreset = PRESET_OPTIONS_DEFAULT) =>
   selectedPreset === PRESET_OPTIONS_DEFAULT ? DEFAULT_STAGE_CONFIG.map(() => ({})) : [{}];
 
+// Not great, we're mixing types
+// better to make everything arrays 🤔
+const maybeFirstElem = (arr = null) => {
+  if (Array.isArray(arr)) {
+    return arr.length ? arr[0] : null;
+  }
+  return arr || null;
+};
+
+// TODO: move to utils
+const formatStageData = (stages) => {
+  return stages
+    .filter(({ hidden = false }) => !hidden)
+    .map(
+      ({
+        startEventIdentifier,
+        endEventIdentifier,
+        startEventLabelId,
+        endEventLabelId,
+        custom = false,
+        name,
+        ...rest
+      }) => {
+        const additionalProps = custom
+          ? {
+              start_event_identifier: maybeFirstElem(startEventIdentifier),
+              end_event_identifier: maybeFirstElem(endEventIdentifier),
+              start_event_label_id: maybeFirstElem(startEventLabelId),
+              end_event_label_id: maybeFirstElem(endEventLabelId),
+            }
+          : {};
+        return {
+          ...rest,
+          ...additionalProps,
+          custom,
+          name,
+        };
+      },
+    );
+};
+
 export default {
   name: 'ValueStreamForm',
   components: {
@@ -40,6 +81,16 @@ export default {
       required: false,
       default: () => ({}),
     },
+    initialPreset: {
+      type: String,
+      required: false,
+      default: PRESET_OPTIONS_DEFAULT,
+    },
+    initialFormErrors: {
+      type: Object,
+      required: false,
+      default: () => ({}),
+    },
     hasExtendedFormFields: {
       type: Boolean,
       required: false,
@@ -47,41 +98,43 @@ export default {
     },
   },
   data() {
-    const { hasExtendedFormFields, initialData } = this;
+    const { hasExtendedFormFields, initialData, initialFormErrors, initialPreset } = this;
+    const { name: nameError = [], stages: stageErrors = [{}] } = initialFormErrors;
     const additionalFields = hasExtendedFormFields
       ? {
           stages: DEFAULT_STAGE_CONFIG,
-          stageErrors: initializeStageErrors(PRESET_OPTIONS_DEFAULT),
+          stageErrors: stageErrors || initializeStageErrors(initialPreset),
           ...initialData,
         }
-      : { stages: [] };
+      : { stages: [], nameError }; // TODO: not sure if i should pass empty stages here
     return {
-      selectedPreset: PRESET_OPTIONS[0].value,
+      selectedPreset: initialPreset,
       presetOptions: PRESET_OPTIONS,
       name: '',
-      nameError: { name: [] },
-      stageErrors: [{}],
+      nameError,
+      stageErrors,
       ...additionalFields,
     };
   },
   computed: {
     ...mapState({
-      initialFormErrors: 'createValueStreamErrors',
       isCreating: 'isCreatingValueStream',
     }),
     ...mapState('customStages', ['formEvents']),
     isValueStreamNameValid() {
-      return !this.nameError.name?.length;
+      return !this.nameError?.length;
     },
     invalidFeedback() {
-      return this.nameError.name?.join('\n');
+      return this.nameError?.length ? this.nameError.join('\n\n') : null;
     },
     hasInitialFormErrors() {
+      // TODO: do we need this + should we check the contained arrays instead
       const { initialFormErrors } = this;
       return Boolean(Object.keys(initialFormErrors).length);
     },
-    isValid() {
-      return this.isValueStreamNameValid && !this.hasInitialFormErrors;
+    isSuccessfullyCreated() {
+      // TODO: get this from state somehow
+      return false;
     },
     isLoading() {
       return this.isCreating;
@@ -91,7 +144,7 @@ export default {
         text: this.$options.I18N.FORM_TITLE,
         attributes: [
           { variant: 'success' },
-          { disabled: !this.isValid },
+          { disabled: this.isSuccessfullyCreated },
           { loading: this.isLoading },
         ],
       };
@@ -114,27 +167,19 @@ export default {
     },
   },
   watch: {
-    initialFormErrors(newErrors = {}) {
-      this.stageErrors = newErrors;
+    initialFormErrors({ name: nameError, stages: stageErrors }) {
+      Vue.set(this, 'nameError', nameError);
+      Vue.set(this, 'stageErrors', stageErrors);
     },
   },
-  mounted() {
-    const { initialFormErrors } = this;
-    if (this.hasInitialFormErrors) {
-      this.stageErrors = initialFormErrors;
-    }
-  },
   methods: {
     ...mapActions(['createValueStream']),
     onSubmit() {
       const { name, stages } = this;
+      // TODO: validate before submission
       return this.createValueStream({
         name,
-        stages: stages.map(({ name: stageName, ...rest }) => ({
-          name: stageName,
-          ...rest,
-          title: stageName,
-        })),
+        stages: formatStageData(stages),
       }).then(() => {
         if (!this.hasInitialFormErrors) {
           this.$toast.show(sprintf(this.$options.I18N.FORM_CREATED, { name }), {
@@ -178,7 +223,7 @@ export default {
       Vue.set(this.stageErrors, index, validateStage(this.activeStages[index]));
     },
     fieldErrors(index) {
-      return this.stageErrors[index];
+      return this.stageErrors && this.stageErrors[index] ? this.stageErrors[index] : {};
     },
     onHide(index) {
       const stage = this.stages[index];
@@ -242,6 +287,7 @@ export default {
   >
     <gl-form>
       <gl-form-group
+        data-testid="create-value-stream-name"
         label-for="create-value-stream-name"
         :label="$options.I18N.FORM_FIELD_NAME_LABEL"
         :invalid-feedback="invalidFeedback"

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

@@ -47,6 +47,7 @@ export default {
       deleteValueStreamError: 'deleteValueStreamError',
       data: 'valueStreams',
       selectedValueStream: 'selectedValueStream',
+      initialFormErrors: 'createValueStreamErrors',
     }),
     hasValueStreams() {
       return Boolean(this.data.length);
@@ -123,7 +124,10 @@ export default {
     <gl-button v-else v-gl-modal-directive="'value-stream-form-modal'">{{
       $options.I18N.CREATE_VALUE_STREAM
     }}</gl-button>
-    <value-stream-form :has-extended-form-fields="hasExtendedFormFields" />
+    <value-stream-form
+      :initial-form-errors="initialFormErrors"
+      :has-extended-form-fields="hasExtendedFormFields"
+    />
     <gl-modal
       data-testid="delete-value-stream-modal"
       modal-id="delete-value-stream-modal"

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

@@ -41,9 +41,8 @@ export const setDateRange = ({ commit, dispatch }, { skipFetch = false, startDat
 };
 
 export const requestStageData = ({ commit }) => commit(types.REQUEST_STAGE_DATA);
-export const receiveStageDataSuccess = ({ commit }, data) => {
+export const receiveStageDataSuccess = ({ commit }, data) =>
   commit(types.RECEIVE_STAGE_DATA_SUCCESS, data);
-};
 
 export const receiveStageDataError = ({ commit }, error) => {
   const { message = '' } = error;
@@ -351,7 +350,7 @@ export const createValueStream = ({ commit, dispatch, getters }, data) => {
     .then(({ data: newValueStream }) => dispatch('receiveCreateValueStreamSuccess', newValueStream))
     .catch(({ response } = {}) => {
       const { data: { message, payload: { errors } } = null } = response;
-      commit(types.RECEIVE_CREATE_VALUE_STREAM_ERROR, { message, errors });
+      commit(types.RECEIVE_CREATE_VALUE_STREAM_ERROR, { message, errors, data });
     });
 };
 

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

@@ -66,6 +66,9 @@ export default {
   },
   [types.RECEIVE_CREATE_STAGE_ERROR](state) {
     state.isSavingCustomStage = false;
+    // ??
+    state.isCreatingCustomStage = false;
+    state.isEditingCustomStage = false;
   },
   [types.RECEIVE_CREATE_STAGE_SUCCESS](state) {
     state.formErrors = null;

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

+import { toArray } from 'lodash';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { transformRawStages } from '../utils';
 import * as types from './mutation_types';
@@ -67,7 +68,8 @@ export default {
     state.stages = [];
   },
   [types.RECEIVE_GROUP_STAGES_SUCCESS](state, stages) {
-    state.stages = transformRawStages(stages);
+    const transformedStages = transformRawStages(stages);
+    state.stages = transformedStages.sort((a, b) => a?.id > b?.id);
   },
   [types.REQUEST_UPDATE_STAGE](state) {
     state.isLoading = true;
@@ -120,9 +122,21 @@ export default {
     state.isCreatingValueStream = true;
     state.createValueStreamErrors = {};
   },
-  [types.RECEIVE_CREATE_VALUE_STREAM_ERROR](state, { errors } = {}) {
+  [types.RECEIVE_CREATE_VALUE_STREAM_ERROR](state, { data: { stages }, errors = {} }) {
     state.isCreatingValueStream = false;
-    state.createValueStreamErrors = errors;
+
+    // TODO: move to utils + add additional specs
+    // TODO: should test that we end up with the same amount of errors as stages
+    // This is because the JSON response only includes failed stages with an index of the stage
+    const { stages: stageErrors = {}, ...rest } = errors;
+    const fullStageErrors = Object.keys(stageErrors).length
+      ? stages.map((_, index) => {
+          return convertObjectPropsToCamelCase(stageErrors[index]) || {};
+        })
+      : {};
+
+    // NOTE: BE currently returns the equivalent of a JS hash for the stages errors, an array simplifies things
+    state.createValueStreamErrors = { ...rest, stages: fullStageErrors };
   },
   [types.RECEIVE_CREATE_VALUE_STREAM_SUCCESS](state, valueStream) {
     state.isCreatingValueStream = false;

ee/spec/features/groups/analytics/cycle_analytics/multiple_value_streams_spec.rb

@@ -12,6 +12,8 @@ RSpec.describe 'Multiple value streams', :js do
   end
 
   value_stream_selector = '[data-testid="dropdown-value-streams"]'
+  extended_form_fields_selector = '[data-testid="extended-form-fields"]'
+  custom_value_stream_name = "New created value stream"
 
   let(:value_stream_dropdown) { page.find(value_stream_selector) }
   let!(:default_value_stream) { create(:cycle_analytics_group_value_stream, group: group, name: 'default') }
@@ -23,7 +25,24 @@ RSpec.describe 'Multiple value streams', :js do
   def select_group(target_group = group)
     visit group_analytics_cycle_analytics_path(target_group)
 
-    expect(page).to have_selector '.js-stage-table' # wait_for_stages_to_load
+    expect(page).to have_selector '.js-stage-table'
+    wait_for_requests
+  end
+
+  # TODO: these methods are borrowed from ee/spec/features/groups/analytics/cycle_analytics/customizable_cycle_analytics_spec.rb
+  def toggle_dropdown(field)
+    page.within("[data-testid='#{field}']") do
+      find('.dropdown-toggle').click
+
+      wait_for_requests
+
+      expect(find('.dropdown-menu')).to have_selector('.dropdown-item')
+    end
+  end
+
+  def select_dropdown_option_by_value(name, value, elem = '.dropdown-item')
+    toggle_dropdown name
+    page.find("[data-testid='#{name}'] .dropdown-menu").find("#{elem}[value='#{value}']").click
   end
 
   before do
@@ -47,16 +66,58 @@ RSpec.describe 'Multiple value streams', :js do
     before do
       select_group
 
-      wait_for_requests
+      toggle_value_stream_dropdown
+      page.find_button(_('Create new Value Stream')).click
+    end
+
+    it 'includes additional form fields' do
+      expect(page).to have_selector(extended_form_fields_selector)
     end
 
     it 'can create a value stream' do
-      custom_value_stream_name = "New created value stream"
+      fill_in 'create-value-stream-name', with: custom_value_stream_name
+
+      page.find_button(_('Create Value Stream')).click
+      wait_for_requests
+
+      expect(page).to have_text(_("'%{name}' Value Stream created") % { name: custom_value_stream_name })
+    end
+
+    it 'can create a value stream with a custom stage' do
+      fill_in 'create-value-stream-name', with: custom_value_stream_name
+      page.find_button(_('Add another stage')).click
+
+      fill_in "custom-stage-name-6", with: "Cool custom stage - name"
+      select_dropdown_option_by_value "custom-stage-start-event-6", :merge_request_created
+      select_dropdown_option_by_value "custom-stage-end-event-6", :merge_request_merged
+
+      page.find_button(_('Create Value Stream')).click
+      wait_for_requests
+
+      expect(page).to have_text(_("'%{name}' Value Stream created") % { name: custom_value_stream_name })
+    end
+  end
+
+  describe 'with the `value_stream_analytics_extended_form` feature flag disabled' do
+    before do
+      stub_licensed_features(cycle_analytics_for_groups: true, type_of_work_analytics: true)
+      stub_feature_flags(value_stream_analytics_extended_form: false)
+
+      sign_in(user)
+
+      select_group
       toggle_value_stream_dropdown
 
       page.find_button(_('Create new Value Stream')).click
+    end
 
+    it 'does not include additional form fields' do
+      expect(page).not_to have_selector(extended_form_fields_selector)
+    end
+
+    it 'can create a value stream' do
       fill_in 'create-value-stream-name', with: custom_value_stream_name
+
       page.find_button(_('Create Value Stream')).click
       wait_for_requests
 
@@ -72,8 +133,6 @@ RSpec.describe 'Multiple value streams', :js do
       create(:cycle_analytics_group_stage, value_stream: value_stream)
 
       select_group
-
-      wait_for_requests
     end
 
     it 'can delete a value stream' do

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

@@ -2,6 +2,9 @@ import { GlModal } from '@gitlab/ui';
 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 import ValueStreamForm from 'ee/analytics/cycle_analytics/components/value_stream_form.vue';
+import DefaultStageFields from 'ee/analytics/cycle_analytics/components/create_value_stream_form/default_stage_fields.vue';
+import CustomStageFields from 'ee/analytics/cycle_analytics/components/create_value_stream_form/custom_stage_fields.vue';
+import { PRESET_OPTIONS_BLANK } from 'ee/analytics/cycle_analytics/components/create_value_stream_form/constants';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import { customStageEvents as formEvents } from '../mock_data';
 
@@ -15,13 +18,20 @@ describe('ValueStreamForm', () => {
   const mockEvent = { preventDefault: jest.fn() };
   const mockToastShow = jest.fn();
   const streamName = 'Cool stream';
-  const createValueStreamErrors = { name: ['Name field required'] };
+  const initialFormErrors = { name: ['Name field required'] };
+  const initialFormStageErrors = {
+    stages: [
+      {
+        name: ['Name field is required'],
+        endEventIdentifier: ['Please select a start event first'],
+      },
+    ],
+  };
 
   const fakeStore = ({ initialState = {} }) =>
     new Vuex.Store({
       state: {
         isCreatingValueStream: false,
-        createValueStreamErrors: {},
         ...initialState,
       },
       actions: {
@@ -37,7 +47,7 @@ describe('ValueStreamForm', () => {
       },
     });
 
-  const createComponent = ({ props = {}, data = {}, initialState = {} } = {}) =>
+  const createComponent = ({ props = {}, data = {}, initialState = {}, stubs = {} } = {}) =>
     extendedWrapper(
       shallowMount(ValueStreamForm, {
         localVue,
@@ -55,6 +65,9 @@ describe('ValueStreamForm', () => {
             show: mockToastShow,
           },
         },
+        stubs: {
+          ...stubs,
+        },
       }),
     );
 
@@ -66,6 +79,8 @@ describe('ValueStreamForm', () => {
   const findBtn = (btn) => findModal().props(btn);
   const findSubmitDisabledAttribute = (attribute) =>
     findBtn('actionPrimary').attributes[1][attribute];
+  const expectFieldError = (testId, error = '') =>
+    expect(wrapper.findByTestId(testId).attributes('invalid-feedback')).toBe(error);
 
   afterEach(() => {
     wrapper.destroy();
@@ -124,20 +139,59 @@ describe('ValueStreamForm', () => {
         expect(wrapper.vm.stages.length).toBe(7);
       });
     });
+
+    describe.only('form errors', () => {
+      const commonExtendedData = {
+        props: {
+          hasExtendedFormFields: true,
+          initialFormErrors: initialFormStageErrors,
+        },
+      };
+
+      it('renders errors for a default stage name', () => {
+        wrapper = createComponent({
+          ...commonExtendedData,
+          stubs: {
+            DefaultStageFields,
+          },
+        });
+
+        expectFieldError('default-stage-name-0', initialFormStageErrors.stages[0].name[0]);
+      });
+
+      it('renders errors for a custom stage field', async () => {
+        wrapper = createComponent({
+          props: {
+            ...commonExtendedData.props,
+            initialPreset: PRESET_OPTIONS_BLANK,
+          },
+          stubs: {
+            CustomStageFields,
+          },
+        });
+
+        console.log('wrapper', wrapper.html());
+        expectFieldError('custom-stage-name-0', initialFormStageErrors.stages[0].name[0]);
+        expectFieldError(
+          'custom-stage-name-0',
+          initialFormStageErrors.stages[0].endEventIdentifier[0],
+        );
+      });
+    });
   });
 
   describe('form errors', () => {
     beforeEach(() => {
       wrapper = createComponent({
-        data: { name: streamName },
-        initialState: {
-          createValueStreamErrors,
+        data: { name: '' },
+        props: {
+          initialFormErrors,
         },
       });
     });
 
-    it('submit button is disabled', () => {
-      expect(findSubmitDisabledAttribute('disabled')).toBe(true);
+    it('renders errors for the name field', () => {
+      expectFieldError('create-value-stream-name', initialFormErrors.name[0]);
     });
   });
 
@@ -146,10 +200,6 @@ describe('ValueStreamForm', () => {
       wrapper = createComponent({ data: { name: streamName } });
     });
 
-    it('submit button is enabled', () => {
-      expect(findSubmitDisabledAttribute('disabled')).toBe(false);
-    });
-
     describe('form submitted successfully', () => {
       beforeEach(() => {
         clickSubmit();
@@ -177,8 +227,8 @@ describe('ValueStreamForm', () => {
       beforeEach(() => {
         wrapper = createComponent({
           data: { name: streamName },
-          initialState: {
-            createValueStreamErrors,
+          props: {
+            initialFormErrors,
           },
         });