From 3593b83a0279bab40f8ba97dc339b32c56f6e0df Mon Sep 17 00:00:00 2001
From: Filipa Lacerda <filipa@gitlab.com>
Date: Tue, 15 May 2018 16:55:07 +0100
Subject: [PATCH] Handles action icons requests in a contained way and shows a
 loading icon to the user

---
 .../components/graph/action_component.vue     | 67 +++++++++------
 .../graph/dropdown_job_component.vue          | 13 ++-
 .../components/graph/graph_component.vue      | 11 ++-
 .../components/graph/job_component.vue        | 12 +--
 .../graph/stage_column_component.vue          | 13 ++-
 .../components/pipelines_table_row.vue        |  1 +
 .../pipelines/components/stage.vue            | 17 ++++
 .../pipelines/pipeline_details_bundle.js      | 30 ++-----
 .../pipelines/graph/action_component_spec.js  | 83 ++++++++-----------
 spec/javascripts/pipelines/stage_spec.js      | 49 +++++++++++
 10 files changed, 177 insertions(+), 119 deletions(-)

diff --git a/app/assets/javascripts/pipelines/components/graph/action_component.vue b/app/assets/javascripts/pipelines/components/graph/action_component.vue
index fd3491c7fe0..11a8bcb0772 100644
--- a/app/assets/javascripts/pipelines/components/graph/action_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/action_component.vue
@@ -1,15 +1,27 @@
 <script>
 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';
+import axios from '~/lib/utils/axios_utils';
+import { dasherize } from '~/lib/utils/text_utility';
+import { __ } from '~/locale';
+import createFlash from '~/flash';
+import tooltip from '~/vue_shared/directives/tooltip';
+import LoadingIcon from '~/vue_shared/components/loading_icon.vue';
+import Icon from '~/vue_shared/components/icon.vue';
+
 /**
- * Renders either a cancel, retry or play icon pointing to the given path.
+ * Renders either a cancel, retry or play icon button and handles the post request
+ *
+ * Used in:
+ * - mr widget mini pipeline graph: `mr_widget_pipeline.vue`
+ * - pipelines table
+ * - pipelines table in merge request page
+ * - pipelines table in commit page
+ * - pipelines detail page in big graph
  */
 export default {
   components: {
     Icon,
+    LoadingIcon,
   },
 
   directives: {
@@ -32,16 +44,10 @@ export default {
       required: true,
     },
 
-    requestFinishedFor: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
   data() {
     return {
-      isDisabled: false,
-      linkRequested: '',
+      isLoading: false,
     };
   },
 
@@ -51,19 +57,28 @@ export default {
       return `${actionIconDash} js-icon-${actionIconDash}`;
     },
   },
-  watch: {
-    requestFinishedFor() {
-      if (this.requestFinishedFor === this.linkRequested) {
-        this.isDisabled = false;
-      }
-    },
-  },
   methods: {
+    /**
+     * The request should not be handled here.
+     * However due to this component being used in several
+     * different apps it avoids repetition & complexity.
+     *
+     */
     onClickAction() {
       $(this.$el).tooltip('hide');
-      eventHub.$emit('postAction', this.link);
-      this.linkRequested = this.link;
-      this.isDisabled = true;
+
+      this.isLoading = true;
+
+      axios.post(`${this.link}.json`)
+        .then(() => {
+          this.isLoading = false;
+          this.$emit('pipelineActionRequestComplete');
+        })
+        .catch(() => {
+          this.isLoading = false;
+
+          createFlash(__('An error occurred while making the request.'));
+        });
     },
   },
 };
@@ -78,8 +93,12 @@ export default {
 btn-transparent ci-action-icon-container ci-action-icon-wrapper"
     :class="cssClass"
     data-container="body"
-    :disabled="isDisabled"
+    :disabled="isLoading"
   >
-    <icon :name="actionIcon" />
+    <icon
+      v-if="!isLoading"
+      :name="actionIcon"
+    />
+    <loading-icon v-else />
   </button>
 </template>
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 4027d26098f..7bfe11ab8cd 100644
--- a/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue
@@ -42,11 +42,6 @@ export default {
       type: Object,
       required: true,
     },
-    requestFinishedFor: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
 
   computed: {
@@ -76,11 +71,15 @@ export default {
         e.stopPropagation();
       });
     },
+
+    pipelineActionRequestComplete() {
+      this.$emit('pipelineActionRequestComplete');
+    },
   },
 };
 </script>
 <template>
-  <div class="ci-job-dropdown-container">
+  <div class="ci-job-dropdown-container dropdown">
     <button
       v-tooltip
       type="button"
@@ -110,7 +109,7 @@ export default {
             <job-component
               :job="item"
               css-class-job-name="mini-pipeline-graph-dropdown-item"
-              :request-finished-for="requestFinishedFor"
+              @pipelineActionRequestComplete="pipelineActionRequestComplete"
             />
           </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 7b8a5edcbff..4ec67f6c01b 100644
--- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue
@@ -16,11 +16,6 @@ export default {
       type: Object,
       required: true,
     },
-    requestFinishedFor: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
 
   computed: {
@@ -51,6 +46,10 @@ export default {
 
       return className;
     },
+
+    refreshPipelineGraph() {
+      this.$emit('refreshPipelineGraph');
+    },
   },
 };
 </script>
@@ -74,7 +73,7 @@ export default {
           :key="stage.name"
           :stage-connector-class="stageConnectorClass(index, stage)"
           :is-first-column="isFirstColumn(index)"
-          :request-finished-for="requestFinishedFor"
+          @refreshPipelineGraph="refreshPipelineGraph"
         />
       </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 c1f0f051b63..27b938c4985 100644
--- a/app/assets/javascripts/pipelines/components/graph/job_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue
@@ -46,11 +46,6 @@ export default {
       required: false,
       default: '',
     },
-    requestFinishedFor: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
   computed: {
     status() {
@@ -84,6 +79,11 @@ export default {
       return this.job.status && this.job.status.action && this.job.status.action.path;
     },
   },
+  methods: {
+    pipelineActionRequestComplete() {
+      this.$emit('pipelineActionRequestComplete');
+    },
+  },
 };
 </script>
 <template>
@@ -126,7 +126,7 @@ export default {
       :tooltip-text="status.action.title"
       :link="status.action.path"
       :action-icon="status.action.icon"
-      :request-finished-for="requestFinishedFor"
+      @pipelineActionRequestComplete="pipelineActionRequestComplete"
     />
   </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 5461fdbbadd..f32368947e8 100644
--- a/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/stage_column_component.vue
@@ -29,12 +29,6 @@ export default {
       required: false,
       default: '',
     },
-
-    requestFinishedFor: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
 
   methods: {
@@ -49,6 +43,10 @@ export default {
     buildConnnectorClass(index) {
       return index === 0 && !this.isFirstColumn ? 'left-connector' : '';
     },
+
+    pipelineActionRequestComplete() {
+      this.$emit('refreshPipelineGraph');
+    },
   },
 };
 </script>
@@ -75,12 +73,13 @@ export default {
             v-if="job.size === 1"
             :job="job"
             css-class-job-name="build-content"
+            @pipelineActionRequestComplete="pipelineActionRequestComplete"
           />
 
           <dropdown-job-component
             v-if="job.size > 1"
             :job="job"
-            :request-finished-for="requestFinishedFor"
+            @pipelineActionRequestComplete="pipelineActionRequestComplete"
           />
 
         </li>
diff --git a/app/assets/javascripts/pipelines/components/pipelines_table_row.vue b/app/assets/javascripts/pipelines/components/pipelines_table_row.vue
index 498a97851fa..41b5e418dd1 100644
--- a/app/assets/javascripts/pipelines/components/pipelines_table_row.vue
+++ b/app/assets/javascripts/pipelines/components/pipelines_table_row.vue
@@ -297,6 +297,7 @@
             v-for="(stage, index) in pipeline.details.stages"
             :key="index">
             <pipeline-stage
+              type="PIPELINES_TABLE"
               :stage="stage"
               :update-dropdown="updateGraphDropdown"
             />
diff --git a/app/assets/javascripts/pipelines/components/stage.vue b/app/assets/javascripts/pipelines/components/stage.vue
index a65485c05eb..8e7430bf69e 100644
--- a/app/assets/javascripts/pipelines/components/stage.vue
+++ b/app/assets/javascripts/pipelines/components/stage.vue
@@ -44,6 +44,12 @@ export default {
       required: false,
       default: false,
     },
+
+    type: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
 
   data() {
@@ -133,6 +139,16 @@ export default {
     isDropdownOpen() {
       return this.$el.classList.contains('open');
     },
+
+    pipelineActionRequestComplete() {
+      if (this.type === 'PIPELINES_TABLE') {
+        // warn the table to update
+        eventHub.$emit('clickedDropdown');
+      } else {
+        // refresh the content
+        this.fetchJobs();
+      }
+    },
   },
 };
 </script>
@@ -188,6 +204,7 @@ export default {
             <job-component
               :job="job"
               css-class-job-name="mini-pipeline-graph-dropdown-item"
+              @pipelineActionRequestComplete="pipelineActionRequestComplete"
             />
           </li>
         </ul>
diff --git a/app/assets/javascripts/pipelines/pipeline_details_bundle.js b/app/assets/javascripts/pipelines/pipeline_details_bundle.js
index 04fe7958fe6..b49a16a87e6 100644
--- a/app/assets/javascripts/pipelines/pipeline_details_bundle.js
+++ b/app/assets/javascripts/pipelines/pipeline_details_bundle.js
@@ -25,30 +25,14 @@ export default () => {
     data() {
       return {
         mediator,
-        requestFinishedFor: null,
       };
     },
-    created() {
-      eventHub.$on('postAction', this.postAction);
-    },
-    beforeDestroy() {
-      eventHub.$off('postAction', this.postAction);
-    },
     methods: {
-      postAction(action) {
-        // Click was made, reset this variable
-        this.requestFinishedFor = null;
-
-        this.mediator.service
-          .postAction(action)
-          .then(() => {
-            this.mediator.refreshPipeline();
-            this.requestFinishedFor = action;
-          })
-          .catch(() => {
-            this.requestFinishedFor = action;
-            Flash(__('An error occurred while making the request.'));
-          });
+      requestRefreshPipelineGraph() {
+        // When an action is clicked
+        // (wether in the dropdown or in the main nodes, we refresh the big graph)
+        this.mediator.refreshPipeline()
+          .catch(() => Flash(__('An error occurred while making the request.')));
       },
     },
     render(createElement) {
@@ -56,7 +40,9 @@ export default () => {
         props: {
           isLoading: this.mediator.state.isLoading,
           pipeline: this.mediator.store.state.pipeline,
-          requestFinishedFor: this.requestFinishedFor,
+        },
+        on: {
+          refreshPipelineGraph: this.requestRefreshPipelineGraph,
         },
       });
     },
diff --git a/spec/javascripts/pipelines/graph/action_component_spec.js b/spec/javascripts/pipelines/graph/action_component_spec.js
index d646bef96f5..97f8ae3cd89 100644
--- a/spec/javascripts/pipelines/graph/action_component_spec.js
+++ b/spec/javascripts/pipelines/graph/action_component_spec.js
@@ -1,13 +1,19 @@
 import Vue from 'vue';
+import MockAdapter from 'axios-mock-adapter';
+import axios from '~/lib/utils/axios_utils';
 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;
+  let mock;
 
   beforeEach(done => {
     const ActionComponent = Vue.extend(actionComponent);
+    mock = new MockAdapter(axios);
+
+    mock.onPost('foo.json').reply(200);
+
     component = mountComponent(ActionComponent, {
       tooltipText: 'bar',
       link: 'foo',
@@ -18,15 +24,10 @@ describe('pipeline graph action component', () => {
   });
 
   afterEach(() => {
+    mock.restore();
     component.$destroy();
   });
 
-  it('should emit an event with the provided link', () => {
-    eventHub.$on('postAction', link => {
-      expect(link).toEqual('foo');
-    });
-  });
-
   it('should render the provided title as a bootstrap tooltip', () => {
     expect(component.$el.getAttribute('data-original-title')).toEqual('bar');
   });
@@ -34,10 +35,12 @@ describe('pipeline graph action component', () => {
   it('should update bootstrap tooltip when title changes', done => {
     component.tooltipText = 'changed';
 
-    setTimeout(() => {
+    component.$nextTick()
+    .then(() => {
       expect(component.$el.getAttribute('data-original-title')).toBe('changed');
-      done();
-    });
+    })
+    .then(done)
+    .catch(done.fail);
   });
 
   it('should render an svg', () => {
@@ -45,44 +48,30 @@ describe('pipeline graph action component', () => {
     expect(component.$el.querySelector('svg')).toBeDefined();
   });
 
-  it('disables the button when clicked', done => {
-    component.$el.click();
+  it('renders a loading icon while component is loading', done => {
+    component.isLoading = true;
 
-    component.$nextTick(() => {
-      expect(component.$el.getAttribute('disabled')).toEqual('disabled');
-      done();
+    component.$nextTick()
+      .then(() => {
+        expect(component.$el.querySelector('.fa-spin')).not.toBeNull();
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  describe('on click', () => {
+    it('emits `pipelineActionRequestComplete` after a successfull request', done => {
+      spyOn(component, '$emit');
+
+      component.$el.click();
+      expect(component.isLoading).toEqual(true);
+
+      component.$nextTick()
+        .then(() => {
+          expect(component.$emit).toHaveBeenCalledWith('pipelineActionRequestComplete');
+        })
+        .then(done)
+        .catch(done.fail);
     });
   });
-
-  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);
-  });
 });
diff --git a/spec/javascripts/pipelines/stage_spec.js b/spec/javascripts/pipelines/stage_spec.js
index 75156e7bdfd..2ba5ecf92e7 100644
--- a/spec/javascripts/pipelines/stage_spec.js
+++ b/spec/javascripts/pipelines/stage_spec.js
@@ -102,4 +102,53 @@ describe('Pipelines stage component', () => {
       });
     });
   });
+
+  describe('pipelineActionRequestComplete', () => {
+    beforeEach(() => {
+      mock.onGet('path.json').reply(200, stageReply);
+
+      mock.onPost(`${stageReply.latest_statuses[0].status.action.path}.json`).reply(200);
+    });
+
+    describe('within pipeline table', () => {
+      it('emits `clickedDropdown` event when `pipelineActionRequestComplete` is triggered', done => {
+        spyOn(eventHub, '$emit');
+
+        component.type = 'PIPELINES_TABLE';
+        component.$el.querySelector('button').click();
+
+        setTimeout(() => {
+          component.$el.querySelector('.js-ci-action').click();
+          component.$nextTick()
+          .then(() => {
+            expect(eventHub.$emit).toHaveBeenCalledWith('clickedDropdown');
+
+            expect(eventHub.$emit).toHaveBeenCalledTimes(2);
+          })
+          .then(done)
+          .catch(done.fail);
+        }, 0);
+      });
+    });
+
+    describe('without a type', () => {
+      it('fetches dropdown content again', done => {
+        spyOn(component, 'fetchJobs').and.callThrough();
+
+        component.$el.querySelector('button').click();
+
+        expect(component.fetchJobs).toHaveBeenCalledTimes(1);
+
+        setTimeout(() => {
+          component.$el.querySelector('.js-ci-action').click();
+          component.$nextTick()
+            .then(() => {
+              expect(component.fetchJobs).toHaveBeenCalledTimes(2);
+            })
+            .then(done)
+            .catch(done.fail);
+        }, 0);
+      });
+    });
+  });
 });