From 377c8c7846e5cf43134a87fef72f6f23d7298401 Mon Sep 17 00:00:00 2001
From: Filipa Lacerda <filipa@gitlab.com>
Date: Fri, 6 Apr 2018 15:42:19 +0000
Subject: [PATCH] Stop redirecting the page in graph main actions

---
 .../components/graph/action_component.vue     |  98 +++++-----
 .../components/graph/graph_component.vue      |  94 ++++-----
 .../components/graph/job_component.vue        | 178 +++++++++---------
 .../graph/stage_column_component.vue          |  90 ++++-----
 .../pipelines/pipeline_details_bundle.js      |  23 +++
 .../pipelines/pipeline_details_mediator.js    |   7 +-
 app/assets/stylesheets/pages/pipelines.scss   |  14 +-
 .../unreleased/fl-fix-annoying-actions.yml    |   5 +
 .../pipelines/graph/action_component_spec.js  |  25 ++-
 9 files changed, 300 insertions(+), 234 deletions(-)
 create mode 100644 changelogs/unreleased/fl-fix-annoying-actions.yml

diff --git a/app/assets/javascripts/pipelines/components/graph/action_component.vue b/app/assets/javascripts/pipelines/components/graph/action_component.vue
index d7effb27bff..e99d949801f 100644
--- a/app/assets/javascripts/pipelines/components/graph/action_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/action_component.vue
@@ -1,60 +1,72 @@
 <script>
-  import tooltip from '../../../vue_shared/directives/tooltip';
-  import icon from '../../../vue_shared/components/icon.vue';
-  import { dasherize } from '../../../lib/utils/text_utility';
-  /**
-   * Renders either a cancel, retry or play icon pointing to the given path.
-   * TODO: Remove UJS from here and use an async request instead.
-   */
-  export default {
-    components: {
-      icon,
+import $ from 'jquery';
+import tooltip from '../../../vue_shared/directives/tooltip';
+import Icon from '../../../vue_shared/components/icon.vue';
+import { dasherize } from '../../../lib/utils/text_utility';
+import eventHub from '../../event_hub';
+/**
+ * Renders either a cancel, retry or play icon pointing to the given path.
+ */
+export default {
+  components: {
+    Icon,
+  },
+
+  directives: {
+    tooltip,
+  },
+
+  props: {
+    tooltipText: {
+      type: String,
+      required: true,
     },
 
-    directives: {
-      tooltip,
+    link: {
+      type: String,
+      required: true,
     },
 
-    props: {
-      tooltipText: {
-        type: String,
-        required: true,
-      },
-
-      link: {
-        type: String,
-        required: true,
-      },
-
-      actionMethod: {
-        type: String,
-        required: true,
-      },
-
-      actionIcon: {
-        type: String,
-        required: true,
-      },
+    actionIcon: {
+      type: String,
+      required: true,
     },
 
-    computed: {
-      cssClass() {
-        const actionIconDash = dasherize(this.actionIcon);
-        return `${actionIconDash} js-icon-${actionIconDash}`;
-      },
+    buttonDisabled: {
+      type: String,
+      required: false,
+      default: null,
     },
-  };
+  },
+  computed: {
+    cssClass() {
+      const actionIconDash = dasherize(this.actionIcon);
+      return `${actionIconDash} js-icon-${actionIconDash}`;
+    },
+    isDisabled() {
+      return this.buttonDisabled === this.link;
+    },
+  },
+
+  methods: {
+    onClickAction() {
+      $(this.$el).tooltip('hide');
+      eventHub.$emit('graphAction', this.link);
+    },
+  },
+};
 </script>
 <template>
-  <a
+  <button
+    type="button"
+    @click="onClickAction"
     v-tooltip
-    :data-method="actionMethod"
     :title="tooltipText"
-    :href="link"
-    class="ci-action-icon-container ci-action-icon-wrapper"
+    class="btn btn-blank btn-transparent ci-action-icon-container ci-action-icon-wrapper"
     :class="cssClass"
     data-container="body"
+    :disabled="isDisabled"
   >
     <icon :name="actionIcon" />
-  </a>
+  </button>
 </template>
diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component.vue b/app/assets/javascripts/pipelines/components/graph/graph_component.vue
index ab84711d4a2..ac9ce7e47d6 100644
--- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue
@@ -1,54 +1,59 @@
 <script>
-  import loadingIcon from '~/vue_shared/components/loading_icon.vue';
-  import stageColumnComponent from './stage_column_component.vue';
+import LoadingIcon from '~/vue_shared/components/loading_icon.vue';
+import StageColumnComponent from './stage_column_component.vue';
 
-  export default {
-    components: {
-      stageColumnComponent,
-      loadingIcon,
+export default {
+  components: {
+    StageColumnComponent,
+    LoadingIcon,
+  },
+
+  props: {
+    isLoading: {
+      type: Boolean,
+      required: true,
+    },
+    pipeline: {
+      type: Object,
+      required: true,
+    },
+    actionDisabled: {
+      type: String,
+      required: false,
+      default: null,
+    },
+  },
+
+  computed: {
+    graph() {
+      return this.pipeline.details && this.pipeline.details.stages;
+    },
+  },
+
+  methods: {
+    capitalizeStageName(name) {
+      return name.charAt(0).toUpperCase() + name.slice(1);
     },
 
-    props: {
-      isLoading: {
-        type: Boolean,
-        required: true,
-      },
-      pipeline: {
-        type: Object,
-        required: true,
-      },
+    isFirstColumn(index) {
+      return index === 0;
     },
 
-    computed: {
-      graph() {
-        return this.pipeline.details && this.pipeline.details.stages;
-      },
+    stageConnectorClass(index, stage) {
+      let className;
+
+      // If it's the first stage column and only has one job
+      if (index === 0 && stage.groups.length === 1) {
+        className = 'no-margin';
+      } else if (index > 0) {
+        // If it is not the first column
+        className = 'left-margin';
+      }
+
+      return className;
     },
-
-    methods: {
-      capitalizeStageName(name) {
-        return name.charAt(0).toUpperCase() + name.slice(1);
-      },
-
-      isFirstColumn(index) {
-        return index === 0;
-      },
-
-      stageConnectorClass(index, stage) {
-        let className;
-
-        // If it's the first stage column and only has one job
-        if (index === 0 && stage.groups.length === 1) {
-          className = 'no-margin';
-        } else if (index > 0) {
-          // If it is not the first column
-          className = 'left-margin';
-        }
-
-        return className;
-      },
-    },
-  };
+  },
+};
 </script>
 <template>
   <div class="build-content middle-block js-pipeline-graph">
@@ -70,6 +75,7 @@
           :key="stage.name"
           :stage-connector-class="stageConnectorClass(index, stage)"
           :is-first-column="isFirstColumn(index)"
+          :action-disabled="actionDisabled"
         />
       </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 d501c465a96..c6e5ae6df41 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -1,96 +1,102 @@
 <script>
-  import actionComponent from './action_component.vue';
-  import dropdownActionComponent from './dropdown_action_component.vue';
-  import jobNameComponent from './job_name_component.vue';
-  import tooltip from '../../../vue_shared/directives/tooltip';
+import ActionComponent from './action_component.vue';
+import DropdownActionComponent from './dropdown_action_component.vue';
+import JobNameComponent from './job_name_component.vue';
+import tooltip from '../../../vue_shared/directives/tooltip';
 
-  /**
-   * Renders the badge for the pipeline graph and the job's dropdown.
-   *
-   * The following object should be provided as `job`:
-   *
-   * {
-   *   "id": 4256,
-   *   "name": "test",
-   *   "status": {
-   *     "icon": "icon_status_success",
-   *     "text": "passed",
-   *     "label": "passed",
-   *     "group": "success",
-   *     "tooltip": "passed",
-   *     "details_path": "/root/ci-mock/builds/4256",
-   *     "action": {
-   *       "icon": "retry",
-   *       "title": "Retry",
-   *       "path": "/root/ci-mock/builds/4256/retry",
-   *       "method": "post"
-   *     }
-   *   }
-   * }
-   */
+/**
+ * Renders the badge for the pipeline graph and the job's dropdown.
+ *
+ * The following object should be provided as `job`:
+ *
+ * {
+ *   "id": 4256,
+ *   "name": "test",
+ *   "status": {
+ *     "icon": "icon_status_success",
+ *     "text": "passed",
+ *     "label": "passed",
+ *     "group": "success",
+ *     "tooltip": "passed",
+ *     "details_path": "/root/ci-mock/builds/4256",
+ *     "action": {
+ *       "icon": "retry",
+ *       "title": "Retry",
+ *       "path": "/root/ci-mock/builds/4256/retry",
+ *       "method": "post"
+ *     }
+ *   }
+ * }
+ */
 
-  export default {
-    components: {
-      actionComponent,
-      dropdownActionComponent,
-      jobNameComponent,
+export default {
+  components: {
+    ActionComponent,
+    DropdownActionComponent,
+    JobNameComponent,
+  },
+
+  directives: {
+    tooltip,
+  },
+  props: {
+    job: {
+      type: Object,
+      required: true,
     },
 
-    directives: {
-      tooltip,
-    },
-    props: {
-      job: {
-        type: Object,
-        required: true,
-      },
-
-      cssClassJobName: {
-        type: String,
-        required: false,
-        default: '',
-      },
-
-      isDropdown: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
+    cssClassJobName: {
+      type: String,
+      required: false,
+      default: '',
     },
 
-    computed: {
-      status() {
-        return this.job && this.job.status ? this.job.status : {};
-      },
-
-      tooltipText() {
-        const textBuilder = [];
-
-        if (this.job.name) {
-          textBuilder.push(this.job.name);
-        }
-
-        if (this.job.name && this.status.tooltip) {
-          textBuilder.push('-');
-        }
-
-        if (this.status.tooltip) {
-          textBuilder.push(`${this.job.status.tooltip}`);
-        }
-
-        return textBuilder.join(' ');
-      },
-
-      /**
-       * Verifies if the provided job has an action path
-       *
-       * @return {Boolean}
-       */
-      hasAction() {
-        return this.job.status && this.job.status.action && this.job.status.action.path;
-      },
+    isDropdown: {
+      type: Boolean,
+      required: false,
+      default: false,
     },
-  };
+
+    actionDisabled: {
+      type: String,
+      required: false,
+      default: null,
+    },
+  },
+
+  computed: {
+    status() {
+      return this.job && this.job.status ? this.job.status : {};
+    },
+
+    tooltipText() {
+      const textBuilder = [];
+
+      if (this.job.name) {
+        textBuilder.push(this.job.name);
+      }
+
+      if (this.job.name && this.status.tooltip) {
+        textBuilder.push('-');
+      }
+
+      if (this.status.tooltip) {
+        textBuilder.push(`${this.job.status.tooltip}`);
+      }
+
+      return textBuilder.join(' ');
+    },
+
+    /**
+     * Verifies if the provided job has an action path
+     *
+     * @return {Boolean}
+     */
+    hasAction() {
+      return this.job.status && this.job.status.action && this.job.status.action.path;
+    },
+  },
+};
 </script>
 <template>
   <div class="ci-job-component">
@@ -132,7 +138,7 @@
       :tooltip-text="status.action.title"
       :link="status.action.path"
       :action-icon="status.action.icon"
-      :action-method="status.action.method"
+      :button-disabled="actionDisabled"
     />
 
     <dropdown-action-component
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 7adcf4017b8..f6e6569e15b 100644
--- a/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue
@@ -1,50 +1,55 @@
 <script>
-  import jobComponent from './job_component.vue';
-  import dropdownJobComponent from './dropdown_job_component.vue';
+import JobComponent from './job_component.vue';
+import DropdownJobComponent from './dropdown_job_component.vue';
 
-  export default {
-    components: {
-      jobComponent,
-      dropdownJobComponent,
-    },
-    props: {
-      title: {
-        type: String,
-        required: true,
-      },
-
-      jobs: {
-        type: Array,
-        required: true,
-      },
-
-      isFirstColumn: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
-
-      stageConnectorClass: {
-        type: String,
-        required: false,
-        default: '',
-      },
+export default {
+  components: {
+    JobComponent,
+    DropdownJobComponent,
+  },
+  props: {
+    title: {
+      type: String,
+      required: true,
     },
 
-    methods: {
-      firstJob(list) {
-        return list[0];
-      },
-
-      jobId(job) {
-        return `ci-badge-${job.name}`;
-      },
-
-      buildConnnectorClass(index) {
-        return index === 0 && !this.isFirstColumn ? 'left-connector' : '';
-      },
+    jobs: {
+      type: Array,
+      required: true,
     },
-  };
+
+    isFirstColumn: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+
+    stageConnectorClass: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    actionDisabled: {
+      type: String,
+      required: false,
+      default: null,
+    },
+  },
+
+  methods: {
+    firstJob(list) {
+      return list[0];
+    },
+
+    jobId(job) {
+      return `ci-badge-${job.name}`;
+    },
+
+    buildConnnectorClass(index) {
+      return index === 0 && !this.isFirstColumn ? 'left-connector' : '';
+    },
+  },
+};
 </script>
 <template>
   <li
@@ -69,6 +74,7 @@
             v-if="job.size === 1"
             :job="job"
             css-class-job-name="build-content"
+            :action-disabled="actionDisabled"
           />
 
           <dropdown-job-component
diff --git a/app/assets/javascripts/pipelines/pipeline_details_bundle.js b/app/assets/javascripts/pipelines/pipeline_details_bundle.js
index 6b26708148c..900eb7855f4 100644
--- a/app/assets/javascripts/pipelines/pipeline_details_bundle.js
+++ b/app/assets/javascripts/pipelines/pipeline_details_bundle.js
@@ -25,13 +25,36 @@ export default () => {
     data() {
       return {
         mediator,
+        actionDisabled: null,
       };
     },
+    created() {
+      eventHub.$on('graphAction', this.postAction);
+    },
+    beforeDestroy() {
+      eventHub.$off('graphAction', this.postAction);
+    },
+    methods: {
+      postAction(action) {
+        this.actionDisabled = action;
+
+        this.mediator.service.postAction(action)
+          .then(() => {
+            this.mediator.refreshPipeline();
+            this.actionDisabled = null;
+          })
+          .catch(() => {
+            this.actionDisabled = null;
+            Flash(__('An error occurred while making the request.'));
+          });
+      },
+    },
     render(createElement) {
       return createElement('pipeline-graph', {
         props: {
           isLoading: this.mediator.state.isLoading,
           pipeline: this.mediator.store.state.pipeline,
+          actionDisabled: this.actionDisabled,
         },
       });
     },
diff --git a/app/assets/javascripts/pipelines/pipeline_details_mediator.js b/app/assets/javascripts/pipelines/pipeline_details_mediator.js
index 10f238fe73b..621969cd622 100644
--- a/app/assets/javascripts/pipelines/pipeline_details_mediator.js
+++ b/app/assets/javascripts/pipelines/pipeline_details_mediator.js
@@ -52,8 +52,11 @@ export default class pipelinesMediator {
   }
 
   refreshPipeline() {
-    this.service.getPipeline()
+    this.poll.stop();
+
+    return this.service.getPipeline()
       .then(response => this.successCallback(response))
-      .catch(() => this.errorCallback());
+      .catch(() => this.errorCallback())
+      .finally(() => this.poll.restart());
   }
 }
diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss
index ce2f1482456..8d5eb2e8c5a 100644
--- a/app/assets/stylesheets/pages/pipelines.scss
+++ b/app/assets/stylesheets/pages/pipelines.scss
@@ -495,17 +495,17 @@
         svg {
           fill: $gl-text-color-secondary;
           position: relative;
-          left: 5px;
-          top: 2px;
-          width: 18px;
-          height: 18px;
+          left: 1px;
+          top: -1px;
+          width: 16px;
+          height: 16px;
         }
 
         &.play {
           svg {
-            width: #{$ci-action-icon-size - 8};
-            height: #{$ci-action-icon-size - 8};
-            left: 8px;
+            width: 16px;
+            height: 16px;
+            left: 3px;
           }
         }
       }
diff --git a/changelogs/unreleased/fl-fix-annoying-actions.yml b/changelogs/unreleased/fl-fix-annoying-actions.yml
new file mode 100644
index 00000000000..fe17f9a8978
--- /dev/null
+++ b/changelogs/unreleased/fl-fix-annoying-actions.yml
@@ -0,0 +1,5 @@
+---
+title: Stop redirecting the page in pipeline main actions
+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 e8fcd4b1a36..581209f215d 100644
--- a/spec/javascripts/pipelines/graph/action_component_spec.js
+++ b/spec/javascripts/pipelines/graph/action_component_spec.js
@@ -1,25 +1,30 @@
 import Vue from 'vue';
 import actionComponent from '~/pipelines/components/graph/action_component.vue';
+import eventHub from '~/pipelines/event_hub';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('pipeline graph action component', () => {
   let component;
 
   beforeEach((done) => {
     const ActionComponent = Vue.extend(actionComponent);
-    component = new ActionComponent({
-      propsData: {
-        tooltipText: 'bar',
-        link: 'foo',
-        actionMethod: 'post',
-        actionIcon: 'cancel',
-      },
-    }).$mount();
+    component = mountComponent(ActionComponent, {
+      tooltipText: 'bar',
+      link: 'foo',
+      actionIcon: 'cancel',
+    });
 
     Vue.nextTick(done);
   });
 
-  it('should render a link', () => {
-    expect(component.$el.getAttribute('href')).toEqual('foo');
+  afterEach(() => {
+    component.$destroy();
+  });
+
+  it('should emit an event with the provided link', () => {
+    eventHub.$on('graphAction', (link) => {
+      expect(link).toEqual('foo');
+    });
   });
 
   it('should render the provided title as a bootstrap tooltip', () => {