Merge branch '340073-fix-editing-value-stream' into 'master'

Ensure restored stages have a transition key

See merge request gitlab-org/gitlab!69580

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

@@ -43,6 +43,12 @@ const initializeStages = (defaultStageConfig, selectedPreset = PRESET_OPTIONS_DE
   return stages.map((stage) => ({ ...stage, transitionKey: uniqueId('stage-') }));
 };
 
+const initializeEditingStages = (stages = []) =>
+  filterStagesByHiddenStatus(cloneDeep(stages), false).map((stage) => ({
+    ...stage,
+    transitionKey: uniqueId(`stage-${stage.name}-`),
+  }));
+
 export default {
   name: 'ValueStreamForm',
   components: {
@@ -93,7 +99,7 @@ export default {
     const { name: nameError = [], stages: stageErrors = [{}] } = initialFormErrors;
     const additionalFields = {
       stages: this.isEditing
-        ? filterStagesByHiddenStatus(cloneDeep(initialStages), false)
+        ? initializeEditingStages(initialStages)
         : initializeStages(defaultStageConfig, initialPreset),
       stageErrors:
         cloneDeep(stageErrors) || initializeStageErrors(defaultStageConfig, initialPreset),
@@ -228,8 +234,8 @@ export default {
     handleMove({ index, direction }) {
       const newStages = this.moveItem(this.stages, index, direction);
       const newErrors = this.moveItem(this.stageErrors, index, direction);
-      Vue.set(this, 'stageErrors', cloneDeep(newErrors));
       Vue.set(this, 'stages', cloneDeep(newStages));
+      Vue.set(this, 'stageErrors', cloneDeep(newErrors));
     },
     validateStageFields(index) {
       Vue.set(this.stageErrors, index, validateStage(this.stages[index]));
@@ -253,7 +259,10 @@ export default {
       Vue.set(this, 'hiddenStages', [
         ...this.hiddenStages.filter((_, i) => i !== hiddenStageIndex),
       ]);
-      Vue.set(this, 'stages', [...this.stages, target]);
+      Vue.set(this, 'stages', [
+        ...this.stages,
+        { ...target, transitionKey: uniqueId(`stage-${target.name}-`) },
+      ]);
     },
     lastStage() {
       const stages = this.$refs.formStages;
@@ -282,6 +291,15 @@ export default {
       const updatedStage = { ...this.stages[activeStageIndex], [field]: value };
       Vue.set(this.stages, activeStageIndex, updatedStage);
     },
+    resetAllFieldsToDefault() {
+      this.name = '';
+      Vue.set(this, 'stages', initializeStages(this.defaultStageConfig, this.selectedPreset));
+      Vue.set(
+        this,
+        'stageErrors',
+        initializeStageErrors(this.defaultStageConfig, this.selectedPreset),
+      );
+    },
     handleResetDefaults() {
       if (this.isEditing) {
         const {
@@ -289,30 +307,18 @@ export default {
         } = this;
         Vue.set(this, 'name', initialName);
         Vue.set(this, 'nameError', []);
-        Vue.set(this, 'stages', cloneDeep(initialStages));
+        Vue.set(this, 'stages', initializeStages(initialStages));
         Vue.set(this, 'stageErrors', [{}]);
       } else {
-        this.name = '';
-        this.defaultStageConfig.forEach((stage, index) => {
-          Vue.set(this.stages, index, { ...stage, hidden: false });
-        });
+        this.resetAllFieldsToDefault();
       }
     },
-    handleResetBlank() {
-      this.name = '';
-      Vue.set(this, 'stages', initializeStages(this.defaultStageConfig, this.selectedPreset));
-    },
     onSelectPreset() {
       if (this.selectedPreset === PRESET_OPTIONS_DEFAULT) {
         this.handleResetDefaults();
       } else {
-        this.handleResetBlank();
+        this.resetAllFieldsToDefault();
       }
-      Vue.set(
-        this,
-        'stageErrors',
-        initializeStageErrors(this.defaultStageConfig, this.selectedPreset),
-      );
     },
     restoreActionTestId(index) {
       return `stage-action-restore-${index}`;

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

@@ -7,6 +7,8 @@ RSpec.describe 'Multiple value streams', :js do
   let_it_be(:group) { create(:group, name: 'CA-test-group') }
   let_it_be(:project) { create(:project, :repository, namespace: group, group: group, name: 'Cool fun project') }
   let_it_be(:sub_group) { create(:group, name: 'CA-sub-group', parent: group) }
+  let_it_be(:group_label1) { create(:group_label, group: group) }
+  let_it_be(:group_label2) { create(:group_label, group: group) }
   let_it_be(:user) do
     create(:user).tap do |u|
       group.add_owner(u)
@@ -70,6 +72,7 @@ RSpec.describe 'Multiple value streams', :js do
 
     it 'can create a value stream with a custom stage and hidden defaults' do
       add_custom_stage_to_form
+      add_custom_label_stage_to_form
 
       # Hide some default stages
       click_action_button('hide', 5)
@@ -126,6 +129,7 @@ RSpec.describe 'Multiple value streams', :js do
 
       it 'can add and remove custom stages' do
         add_custom_stage_to_form
+        add_custom_label_stage_to_form
 
         page.find_button(_('Save Value Stream')).click
         wait_for_requests
@@ -138,6 +142,10 @@ RSpec.describe 'Multiple value streams', :js do
         click_action_button('remove', 7)
         click_action_button('remove', 6)
 
+        # re-order some stages
+        page.all("[data-testid*='stage-action-move-down-']").first.click
+        page.all("[data-testid*='stage-action-move-up-']").last.click
+
         page.find_button(_('Save Value Stream')).click
         wait_for_requests
 

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

@@ -103,6 +103,8 @@ describe('ValueStreamForm', () => {
   const clickRestoreStageAtIndex = (index) => findRestoreStageButton(index).vm.$emit('click');
   const expectFieldError = (testId, error = '') =>
     expect(wrapper.findByTestId(testId).attributes('invalid-feedback')).toBe(error);
+  const expectStageTransitionKeys = (stages) =>
+    stages.forEach((stage) => expect(stage.transitionKey).toContain('stage-'));
 
   afterEach(() => {
     wrapper.destroy();
@@ -140,6 +142,16 @@ describe('ValueStreamForm', () => {
       });
     });
 
+    it('each stage has a transition key when toggling', () => {
+      findPresetSelector().vm.$emit('input', PRESET_OPTIONS_BLANK);
+
+      expectStageTransitionKeys(wrapper.vm.stages);
+
+      findPresetSelector().vm.$emit('input', PRESET_OPTIONS_DEFAULT);
+
+      expectStageTransitionKeys(wrapper.vm.stages);
+    });
+
     it('does not display any hidden stages', () => {
       expect(findHiddenStages().length).toBe(0);
     });
@@ -176,6 +188,10 @@ describe('ValueStreamForm', () => {
 
         expect(wrapper.vm.nameError).toEqual(['Name is required']);
       });
+
+      it('each stage has a transition key', () => {
+        expectStageTransitionKeys(wrapper.vm.stages);
+      });
     });
 
     describe('form errors', () => {
@@ -239,6 +255,10 @@ describe('ValueStreamForm', () => {
         expect(findHiddenStages().length).toBe(0);
       });
 
+      it('each stage has a transition key', () => {
+        expectStageTransitionKeys(wrapper.vm.stages);
+      });
+
       describe('restore defaults button', () => {
         it('will clear the form fields', async () => {
           expect(wrapper.vm.stages).toHaveLength(stageCount);
@@ -280,6 +300,14 @@ describe('ValueStreamForm', () => {
           expect(findHiddenStages().length).toBe(hiddenStages.length - 1);
           expect(wrapper.vm.stages.length).toBe(stageCount + 1);
         });
+
+        it('when a stage is restored it has a transition key', async () => {
+          await clickRestoreStageAtIndex(1);
+
+          expect(wrapper.vm.stages[stageCount].transitionKey).toContain(
+            `stage-${hiddenStages[1].name}-`,
+          );
+        });
       });
 
       describe('Add stage button', () => {

spec/support/helpers/cycle_analytics_helpers.rb

@@ -23,12 +23,39 @@ module CycleAnalyticsHelpers
     end
   end
 
+  def select_event_label(sel)
+    page.within(sel) do
+      find('.dropdown-toggle').click
+      page.find(".dropdown-menu").all(".dropdown-item")[1].click
+    end
+  end
+
+  def fill_in_custom_label_stage_fields
+    index = page.all('[data-testid="value-stream-stage-fields"]').length
+    last_stage = page.all('[data-testid="value-stream-stage-fields"]').last
+
+    within last_stage do
+      find('[name*="custom-stage-name-"]').fill_in with: "Cool custom label stage - name #{index}"
+      select_dropdown_option_by_value "custom-stage-start-event-", :issue_label_added
+      select_dropdown_option_by_value "custom-stage-end-event-", :issue_label_removed
+
+      select_event_label("[data-testid*='custom-stage-start-event-label-']")
+      select_event_label("[data-testid*='custom-stage-end-event-label-']")
+    end
+  end
+
   def add_custom_stage_to_form
     page.find_button(s_('CreateValueStreamForm|Add another stage')).click
 
     fill_in_custom_stage_fields
   end
 
+  def add_custom_label_stage_to_form
+    page.find_button(s_('CreateValueStreamForm|Add another stage')).click
+
+    fill_in_custom_label_stage_fields
+  end
+
   def save_value_stream(custom_value_stream_name)
     fill_in 'create-value-stream-name', with: custom_value_stream_name