From a527c9b915ac7c690c03f8b2cd16edf1777f1f32 Mon Sep 17 00:00:00 2001
From: Filipa Lacerda <filipa@gitlab.com>
Date: Fri, 20 Apr 2018 15:43:58 +0100
Subject: [PATCH] Fix disabled state while making a request

Provide props down instead of an event
---
 .../components/graph/action_component.vue     |  25 +++-
 .../graph/dropdown_job_component.vue          | 135 +++++++++---------
 .../components/graph/graph_component.vue      |   7 +-
 .../components/graph/job_component.vue        |  10 +-
 .../graph/stage_column_component.vue          |   7 +-
 .../pipelines/pipeline_details_bundle.js      |  17 ++-
 .../33697-remove-ujs-action-big-graph.yml     |   5 +
 .../pipelines/graph/action_component_spec.js  |  47 +++++-
 8 files changed, 159 insertions(+), 94 deletions(-)
 create mode 100644 changelogs/unreleased/33697-remove-ujs-action-big-graph.yml

diff --git a/app/assets/javascripts/pipelines/components/graph/action_component.vue b/app/assets/javascripts/pipelines/components/graph/action_component.vue
index 558bf812fef..29ee73a2a6f 100644
--- a/app/assets/javascripts/pipelines/components/graph/action_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/action_component.vue
@@ -32,26 +32,38 @@ export default {
       required: true,
     },
 
-    buttonDisabled: {
+    requestFinishedFor: {
       type: String,
       required: false,
-      default: null,
+      default: '',
     },
   },
+  data() {
+    return {
+      isDisabled: false,
+      linkRequested: '',
+    };
+  },
+
   computed: {
     cssClass() {
       const actionIconDash = dasherize(this.actionIcon);
       return `${actionIconDash} js-icon-${actionIconDash}`;
     },
-    isDisabled() {
-      return this.buttonDisabled === this.link;
+  },
+  watch: {
+    requestFinishedFor() {
+      if (this.requestFinishedFor === this.linkRequested) {
+        this.isDisabled = false;
+      }
     },
   },
-
   methods: {
     onClickAction() {
       $(this.$el).tooltip('hide');
       eventHub.$emit('graphAction', this.link);
+      this.linkRequested = this.link;
+      this.isDisabled = true;
     },
   },
 };
@@ -62,7 +74,8 @@ export default {
     @click="onClickAction"
     v-tooltip
     :title="tooltipText"
-    class="js-ci-action btn btn-blank btn-transparent ci-action-icon-container ci-action-icon-wrapper"
+    class="js-ci-action btn btn-blank
+btn-transparent ci-action-icon-container ci-action-icon-wrapper"
     :class="cssClass"
     data-container="body"
     :disabled="isDisabled"
diff --git a/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue b/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue
index 30e40259146..9adc8881831 100644
--- a/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue
@@ -1,77 +1,83 @@
 <script>
-  import $ from 'jquery';
-  import JobNameComponent from './job_name_component.vue';
-  import JobComponent from './job_component.vue';
-  import tooltip from '../../../vue_shared/directives/tooltip';
+import $ from 'jquery';
+import JobNameComponent from './job_name_component.vue';
+import JobComponent from './job_component.vue';
+import tooltip from '../../../vue_shared/directives/tooltip';
 
-  /**
-   * Renders the dropdown for the pipeline graph.
-   *
-   * The following object should be provided as `job`:
-   *
-   * {
-   *   "id": 4256,
-   *   "name": "test",
-   *   "status": {
-   *     "icon": "icon_status_success",
-   *     "text": "passed",
-   *     "label": "passed",
-   *     "group": "success",
-   *     "details_path": "/root/ci-mock/builds/4256",
-   *     "action": {
-   *       "icon": "retry",
-   *       "title": "Retry",
-   *       "path": "/root/ci-mock/builds/4256/retry",
-   *       "method": "post"
-   *     }
-   *   }
-   * }
-   */
-  export default {
-    directives: {
-      tooltip,
-    },
+/**
+ * Renders the dropdown for the pipeline graph.
+ *
+ * The following object should be provided as `job`:
+ *
+ * {
+ *   "id": 4256,
+ *   "name": "test",
+ *   "status": {
+ *     "icon": "icon_status_success",
+ *     "text": "passed",
+ *     "label": "passed",
+ *     "group": "success",
+ *     "details_path": "/root/ci-mock/builds/4256",
+ *     "action": {
+ *       "icon": "retry",
+ *       "title": "Retry",
+ *       "path": "/root/ci-mock/builds/4256/retry",
+ *       "method": "post"
+ *     }
+ *   }
+ * }
+ */
+export default {
+  directives: {
+    tooltip,
+  },
 
-    components: {
-      JobComponent,
-      JobNameComponent,
-    },
+  components: {
+    JobComponent,
+    JobNameComponent,
+  },
 
-    props: {
-      job: {
-        type: Object,
-        required: true,
-      },
+  props: {
+    job: {
+      type: Object,
+      required: true,
     },
+    requestFinishedFor: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
 
-    computed: {
-      tooltipText() {
-        return `${this.job.name} - ${this.job.status.label}`;
-      },
+  computed: {
+    tooltipText() {
+      return `${this.job.name} - ${this.job.status.label}`;
     },
+  },
 
-    mounted() {
-      this.stopDropdownClickPropagation();
-    },
+  mounted() {
+    this.stopDropdownClickPropagation();
+  },
 
-    methods: {
-      /**
-       * When the user right clicks or cmd/ctrl + click in the job name or the action icon
-       * the dropdown should not be closed so we stop propagation of the click event inside the dropdown.
-       *
-       * Since this component is rendered multiple times per page we need to guarantee we only
-       * target the click event of this component.
-       */
-      stopDropdownClickPropagation() {
-        $(
-          '.js-grouped-pipeline-dropdown button, .js-grouped-pipeline-dropdown a.mini-pipeline-graph-dropdown-item',
-          this.$el,
-        ).on('click', (e) => {
-            e.stopPropagation();
-          });
-      },
+  methods: {
+    /**
+     * When the user right clicks or cmd/ctrl + click in the job name or the action icon
+     * the dropdown should not be closed so we stop propagation
+     * of the click event inside the dropdown.
+     *
+     * Since this component is rendered multiple times per page we need to guarantee we only
+     * target the click event of this component.
+     */
+    stopDropdownClickPropagation() {
+      $(
+        '.js-grouped-pipeline-dropdown button, .js-grouped-pipeline-dropdown a.mini-pipeline-graph-dropdown-item',
+        this.$el
+      ).on('click', e => {
+        e.stopPropagation();
+      });
     },
-  };
+  },
+};
 </script>
 <template>
   <div class="ci-job-dropdown-container">
@@ -102,6 +108,7 @@
             <job-component
               :job="item"
               css-class-job-name="mini-pipeline-graph-dropdown-item"
+              :request-finished-for="requestFinishedFor"
             />
           </li>
         </ul>
diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component.vue b/app/assets/javascripts/pipelines/components/graph/graph_component.vue
index ac9ce7e47d6..7b8a5edcbff 100644
--- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue
@@ -7,7 +7,6 @@ export default {
     StageColumnComponent,
     LoadingIcon,
   },
-
   props: {
     isLoading: {
       type: Boolean,
@@ -17,10 +16,10 @@ export default {
       type: Object,
       required: true,
     },
-    actionDisabled: {
+    requestFinishedFor: {
       type: String,
       required: false,
-      default: null,
+      default: '',
     },
   },
 
@@ -75,7 +74,7 @@ export default {
           :key="stage.name"
           :stage-connector-class="stageConnectorClass(index, stage)"
           :is-first-column="isFirstColumn(index)"
-          :action-disabled="actionDisabled"
+          :request-finished-for="requestFinishedFor"
         />
       </ul>
     </div>
diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue
index 29680235a9e..4fcd4b79f4a 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -33,7 +33,6 @@ export default {
     ActionComponent,
     JobNameComponent,
   },
-
   directives: {
     tooltip,
   },
@@ -42,20 +41,17 @@ export default {
       type: Object,
       required: true,
     },
-
     cssClassJobName: {
       type: String,
       required: false,
       default: '',
     },
-
-    actionDisabled: {
+    requestFinishedFor: {
       type: String,
       required: false,
-      default: null,
+      default: '',
     },
   },
-
   computed: {
     status() {
       return this.job && this.job.status ? this.job.status : {};
@@ -130,7 +126,7 @@ export default {
       :tooltip-text="status.action.title"
       :link="status.action.path"
       :action-icon="status.action.icon"
-      :button-disabled="actionDisabled"
+      :request-finished-for="requestFinishedFor"
     />
   </div>
 </template>
diff --git a/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue b/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue
index f6e6569e15b..5461fdbbadd 100644
--- a/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue
@@ -29,10 +29,11 @@ export default {
       required: false,
       default: '',
     },
-    actionDisabled: {
+
+    requestFinishedFor: {
       type: String,
       required: false,
-      default: null,
+      default: '',
     },
   },
 
@@ -74,12 +75,12 @@ export default {
             v-if="job.size === 1"
             :job="job"
             css-class-job-name="build-content"
-            :action-disabled="actionDisabled"
           />
 
           <dropdown-job-component
             v-if="job.size > 1"
             :job="job"
+            :request-finished-for="requestFinishedFor"
           />
 
         </li>
diff --git a/app/assets/javascripts/pipelines/pipeline_details_bundle.js b/app/assets/javascripts/pipelines/pipeline_details_bundle.js
index 900eb7855f4..6584f96130b 100644
--- a/app/assets/javascripts/pipelines/pipeline_details_bundle.js
+++ b/app/assets/javascripts/pipelines/pipeline_details_bundle.js
@@ -25,7 +25,7 @@ export default () => {
     data() {
       return {
         mediator,
-        actionDisabled: null,
+        requestFinishedFor: null,
       };
     },
     created() {
@@ -36,15 +36,17 @@ export default () => {
     },
     methods: {
       postAction(action) {
-        this.actionDisabled = action;
+        // Click was made, reset this variable
+        this.requestFinishedFor = null;
 
-        this.mediator.service.postAction(action)
+        this.mediator.service
+          .postAction(action)
           .then(() => {
             this.mediator.refreshPipeline();
-            this.actionDisabled = null;
+            this.requestFinishedFor = action;
           })
           .catch(() => {
-            this.actionDisabled = null;
+            this.requestFinishedFor = action;
             Flash(__('An error occurred while making the request.'));
           });
       },
@@ -54,7 +56,7 @@ export default () => {
         props: {
           isLoading: this.mediator.state.isLoading,
           pipeline: this.mediator.store.state.pipeline,
-          actionDisabled: this.actionDisabled,
+          requestFinishedFor: this.requestFinishedFor,
         },
       });
     },
@@ -79,7 +81,8 @@ export default () => {
     },
     methods: {
       postAction(action) {
-        this.mediator.service.postAction(action.path)
+        this.mediator.service
+          .postAction(action.path)
           .then(() => this.mediator.refreshPipeline())
           .catch(() => Flash(__('An error occurred while making the request.')));
       },
diff --git a/changelogs/unreleased/33697-remove-ujs-action-big-graph.yml b/changelogs/unreleased/33697-remove-ujs-action-big-graph.yml
new file mode 100644
index 00000000000..43ce52c1209
--- /dev/null
+++ b/changelogs/unreleased/33697-remove-ujs-action-big-graph.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent pipeline actions in dropdown to redirct to a new page
+merge_request:
+author:
+type: fixed
diff --git a/spec/javascripts/pipelines/graph/action_component_spec.js b/spec/javascripts/pipelines/graph/action_component_spec.js
index 581209f215d..3de10392472 100644
--- a/spec/javascripts/pipelines/graph/action_component_spec.js
+++ b/spec/javascripts/pipelines/graph/action_component_spec.js
@@ -6,7 +6,7 @@ import mountComponent from '../../helpers/vue_mount_component_helper';
 describe('pipeline graph action component', () => {
   let component;
 
-  beforeEach((done) => {
+  beforeEach(done => {
     const ActionComponent = Vue.extend(actionComponent);
     component = mountComponent(ActionComponent, {
       tooltipText: 'bar',
@@ -22,7 +22,7 @@ describe('pipeline graph action component', () => {
   });
 
   it('should emit an event with the provided link', () => {
-    eventHub.$on('graphAction', (link) => {
+    eventHub.$on('graphAction', link => {
       expect(link).toEqual('foo');
     });
   });
@@ -31,7 +31,7 @@ describe('pipeline graph action component', () => {
     expect(component.$el.getAttribute('data-original-title')).toEqual('bar');
   });
 
-  it('should update bootstrap tooltip when title changes', (done) => {
+  it('should update bootstrap tooltip when title changes', done => {
     component.tooltipText = 'changed';
 
     setTimeout(() => {
@@ -44,4 +44,45 @@ describe('pipeline graph action component', () => {
     expect(component.$el.querySelector('.ci-action-icon-wrapper')).toBeDefined();
     expect(component.$el.querySelector('svg')).toBeDefined();
   });
+
+  it('disables the button when clicked', done => {
+    component.$el.click();
+
+    component.$nextTick(() => {
+      expect(component.$el.getAttribute('disabled')).toEqual('disabled');
+      done();
+    });
+  });
+
+  it('re-enabled the button when `requestFinishedFor` matches `linkRequested`', done => {
+    component.$el.click();
+
+    component
+      .$nextTick()
+      .then(() => {
+        expect(component.$el.getAttribute('disabled')).toEqual('disabled');
+        component.requestFinishedFor = 'foo';
+      })
+      .then(() => {
+        expect(component.$el.getAttribute('disabled')).toBeNull();
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('does not re-enable the button when `requestFinishedFor` does not matches `linkRequested`', done => {
+    component.$el.click();
+
+    component
+      .$nextTick()
+      .then(() => {
+        expect(component.$el.getAttribute('disabled')).toEqual('disabled');
+        component.requestFinishedFor = 'bar';
+      })
+      .then(() => {
+        expect(component.$el.getAttribute('disabled')).toEqual('disabled');
+      })
+      .then(done)
+      .catch(done.fail);
+  });
 });