Merge branch '296856-prevent-creating-duplicate-pipelines-manually' into 'master'

Prevent creating duplicate pipelines manually

See merge request gitlab-org/gitlab!51076

app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue

@@ -116,6 +116,7 @@ export default {
       totalWarnings: 0,
       isWarningDismissed: false,
       isLoading: false,
+      submitted: false,
     };
   },
   computed: {
@@ -294,6 +295,7 @@ export default {
         });
     },
     createPipeline() {
+      this.submitted = true;
       const filteredVariables = this.variables
         .filter(({ key, value }) => key !== '' && value !== '')
         .map(({ variable_type, key, value }) => ({
@@ -313,8 +315,16 @@ export default {
           redirectTo(`${this.pipelinesPath}/${data.id}`);
         })
         .catch((err) => {
-          const { errors, warnings, total_warnings: totalWarnings } = err.response.data;
+          // always re-enable submit button
+          this.submitted = false;
+
+          const {
+            errors = [],
+            warnings = [],
+            total_warnings: totalWarnings = 0,
+          } = err?.response?.data;
           const [error] = errors;
+
           this.error = error;
           this.warnings = warnings;
           this.totalWarnings = totalWarnings;
@@ -464,6 +474,8 @@ export default {
         variant="success"
         class="js-no-auto-disable"
         data-qa-selector="run_pipeline_button"
+        data-testid="run_pipeline_button"
+        :disabled="submitted"
         >{{ s__('Pipeline|Run Pipeline') }}</gl-button
       >
       <gl-button :href="pipelinesPath">{{ __('Cancel') }}</gl-button>

changelogs/unreleased/296856-prevent-creating-duplicate-pipelines-manually.yml

+---
+title: Prevent creating duplicate pipelines manually
+merge_request: 51076
+author: Kev @KevSlashNull
+type: changed

spec/frontend/pipeline_new/components/pipeline_new_form_spec.js

@@ -34,6 +34,7 @@ describe('Pipeline New Form', () => {
   const findForm = () => wrapper.find(GlForm);
   const findDropdown = () => wrapper.find(GlDropdown);
   const findDropdownItems = () => wrapper.findAll(GlDropdownItem);
+  const findSubmitButton = () => wrapper.find('[data-testid="run_pipeline_button"]');
   const findVariableRows = () => wrapper.findAll('[data-testid="ci-variable-row"]');
   const findRemoveIcons = () => wrapper.findAll('[data-testid="remove-ci-variable-row"]');
   const findKeyInputs = () => wrapper.findAll('[data-testid="pipeline-form-ci-variable-key"]');
@@ -155,6 +156,18 @@ describe('Pipeline New Form', () => {
 
       await waitForPromises();
     });
+
+    it('disables the submit button immediately after submitting', async () => {
+      createComponent();
+
+      expect(findSubmitButton().props('disabled')).toBe(false);
+
+      findForm().vm.$emit('submit', dummySubmitEvent);
+      await waitForPromises();
+
+      expect(findSubmitButton().props('disabled')).toBe(true);
+    });
+
     it('creates pipeline with full ref and variables', async () => {
       createComponent();
 
@@ -167,6 +180,7 @@ describe('Pipeline New Form', () => {
       expect(getExpectedPostParams().ref).toEqual(wrapper.vm.$data.refValue.fullName);
       expect(redirectTo).toHaveBeenCalledWith(`${pipelinesPath}/${postResponse.id}`);
     });
+
     it('creates a pipeline with short ref and variables', async () => {
       // query params are used
       createComponent('', mockParams);
@@ -312,31 +326,55 @@ describe('Pipeline New Form', () => {
   describe('Form errors and warnings', () => {
     beforeEach(() => {
       createComponent();
+    });
 
-      mock.onPost(pipelinesPath).reply(httpStatusCodes.BAD_REQUEST, mockError);
+    describe('when the error response can be handled', () => {
+      beforeEach(async () => {
+        mock.onPost(pipelinesPath).reply(httpStatusCodes.BAD_REQUEST, mockError);
 
-      findForm().vm.$emit('submit', dummySubmitEvent);
+        findForm().vm.$emit('submit', dummySubmitEvent);
 
-      return waitForPromises();
-    });
+        await waitForPromises();
+      });
 
-    it('shows both error and warning', () => {
-      expect(findErrorAlert().exists()).toBe(true);
-      expect(findWarningAlert().exists()).toBe(true);
-    });
+      it('shows both error and warning', () => {
+        expect(findErrorAlert().exists()).toBe(true);
+        expect(findWarningAlert().exists()).toBe(true);
+      });
 
-    it('shows the correct error', () => {
-      expect(findErrorAlert().text()).toBe(mockError.errors[0]);
-    });
+      it('shows the correct error', () => {
+        expect(findErrorAlert().text()).toBe(mockError.errors[0]);
+      });
 
-    it('shows the correct warning title', () => {
-      const { length } = mockError.warnings;
+      it('shows the correct warning title', () => {
+        const { length } = mockError.warnings;
+
+        expect(findWarningAlertSummary().attributes('message')).toBe(`${length} warnings found:`);
+      });
+
+      it('shows the correct amount of warnings', () => {
+        expect(findWarnings()).toHaveLength(mockError.warnings.length);
+      });
 
-      expect(findWarningAlertSummary().attributes('message')).toBe(`${length} warnings found:`);
+      it('re-enables the submit button', () => {
+        expect(findSubmitButton().props('disabled')).toBe(false);
+      });
     });
 
-    it('shows the correct amount of warnings', () => {
-      expect(findWarnings()).toHaveLength(mockError.warnings.length);
+    describe('when the error response cannot be handled', () => {
+      beforeEach(async () => {
+        mock
+          .onPost(pipelinesPath)
+          .reply(httpStatusCodes.INTERNAL_SERVER_ERROR, 'something went wrong');
+
+        findForm().vm.$emit('submit', dummySubmitEvent);
+
+        await waitForPromises();
+      });
+
+      it('re-enables the submit button', () => {
+        expect(findSubmitButton().props('disabled')).toBe(false);
+      });
     });
   });
 });