diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2018-05-15 16:55:07 +0100 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2018-05-15 16:55:07 +0100 |
commit | 3593b83a0279bab40f8ba97dc339b32c56f6e0df (patch) | |
tree | a7fd75dd081e5b30795892582a3065ce3af0aac5 | |
parent | bf806712835869046c819f84ee515caa8829c10d (diff) | |
download | gitlab-ce-3593b83a0279bab40f8ba97dc339b32c56f6e0df.tar.gz |
Handles action icons requests in a contained way and shows a loading icon to the user
10 files changed, 171 insertions, 113 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(); - - component.$nextTick(() => { - expect(component.$el.getAttribute('disabled')).toEqual('disabled'); - done(); - }); - }); - - it('re-enabled the button when `requestFinishedFor` matches `linkRequested`', done => { - component.$el.click(); + it('renders a loading icon while component is loading', done => { + component.isLoading = true; - component - .$nextTick() + component.$nextTick() .then(() => { - expect(component.$el.getAttribute('disabled')).toEqual('disabled'); - component.requestFinishedFor = 'foo'; - }) - .then(() => { - expect(component.$el.getAttribute('disabled')).toBeNull(); + expect(component.$el.querySelector('.fa-spin')).not.toBeNull(); }) .then(done) .catch(done.fail); }); - it('does not re-enable the button when `requestFinishedFor` does not matches `linkRequested`', done => { - component.$el.click(); + describe('on click', () => { + it('emits `pipelineActionRequestComplete` after a successfull request', done => { + spyOn(component, '$emit'); - 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); + component.$el.click(); + expect(component.isLoading).toEqual(true); + + component.$nextTick() + .then(() => { + expect(component.$emit).toHaveBeenCalledWith('pipelineActionRequestComplete'); + }) + .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); + }); + }); + }); }); |