From 06b31461f34bac86d31d898e4f0e5b573d6b0345 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Mon, 11 Sep 2017 21:47:33 +1000 Subject: [PATCH 1/2] improve merge request widget status icon UX x to indicate failure or cannot merge --- .../states/mr_widget_ready_to_merge.js | 32 ++++++-- .../states/mr_widget_ready_to_merge_spec.js | 79 +++++++++++++++---- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js index ad709da51ee..a0aba7718d4 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js @@ -38,24 +38,40 @@ export default { return this.useCommitMessageWithDescription ? withoutDesc : withDesc; }, + status() { + const { pipeline, isPipelineActive, isPipelineFailed, hasCI, ciStatus } = this.mr; + + if (hasCI && !ciStatus) { + return 'failed'; + } else if (!pipeline) { + return 'success'; + } else if (isPipelineActive) { + return 'pending'; + } else if (isPipelineFailed) { + return 'failed'; + } + + return 'success'; + }, mergeButtonClass() { const defaultClass = 'btn btn-sm btn-success accept-merge-request'; const failedClass = `${defaultClass} btn-danger`; const inActionClass = `${defaultClass} btn-info`; - const { pipeline, isPipelineActive, isPipelineFailed, hasCI, ciStatus } = this.mr; - if (hasCI && !ciStatus) { + if (this.status === 'failed') { return failedClass; - } else if (!pipeline) { - return defaultClass; - } else if (isPipelineActive) { + } else if (this.status === 'pending') { return inActionClass; - } else if (isPipelineFailed) { - return failedClass; } return defaultClass; }, + iconClass() { + if (this.status === 'failed' || !this.commitMessage.length || !this.isMergeAllowed() || this.mr.preventMerge) { + return 'failed'; + } + return 'success'; + }, mergeButtonText() { if (this.isMergingImmediately) { return 'Merge in progress'; @@ -208,7 +224,7 @@ export default { }, template: `
- +
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index 03a52f1f91c..b0511c25929 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -95,38 +95,87 @@ describe('MRWidgetReadyToMerge', () => { }); }); + describe('status', () => { + it('defaults to success', () => { + vm.mr.pipeline = true; + expect(vm.status).toEqual('success'); + }); + + it('returns failed when MR has CI but also has an unknown status', () => { + vm.mr.hasCI = true; + expect(vm.status).toEqual('failed'); + }); + + it('returns default when MR has no pipeline', () => { + expect(vm.status).toEqual('success'); + }); + + it('returns pending when pipeline is active', () => { + vm.mr.pipeline = {}; + vm.mr.isPipelineActive = true; + expect(vm.status).toEqual('pending'); + }); + + it('returns failed when pipeline is failed', () => { + vm.mr.pipeline = {}; + vm.mr.isPipelineFailed = true; + expect(vm.status).toEqual('failed'); + }); + }); + describe('mergeButtonClass', () => { const defaultClass = 'btn btn-sm btn-success accept-merge-request'; const failedClass = `${defaultClass} btn-danger`; const inActionClass = `${defaultClass} btn-info`; - it('should return default class', () => { + it('defaults to success class', () => { + expect(vm.mergeButtonClass).toEqual(defaultClass); + }); + + it('returns success class for success status', () => { vm.mr.pipeline = true; expect(vm.mergeButtonClass).toEqual(defaultClass); }); - it('should return failed class when MR has CI but also has an unknown status', () => { - vm.mr.hasCI = true; - expect(vm.mergeButtonClass).toEqual(failedClass); - }); - - it('should return default class when MR has no pipeline', () => { - expect(vm.mergeButtonClass).toEqual(defaultClass); - }); - - it('should return in action class when pipeline is active', () => { + it('returns info class for pending status', () => { vm.mr.pipeline = {}; vm.mr.isPipelineActive = true; expect(vm.mergeButtonClass).toEqual(inActionClass); }); - it('should return failed class when pipeline is failed', () => { - vm.mr.pipeline = {}; - vm.mr.isPipelineFailed = true; + it('returns failed class for failed status', () => { + vm.mr.hasCI = true; expect(vm.mergeButtonClass).toEqual(failedClass); }); }); + describe('status icon', () => { + it('defaults to tick icon', () => { + expect(vm.iconClass).toEqual('success'); + }); + + it('shows tick for success status', () => { + vm.mr.pipeline = true; + expect(vm.iconClass).toEqual('success'); + }); + + it('shows tick for pending status', () => { + vm.mr.pipeline = {}; + vm.mr.isPipelineActive = true; + expect(vm.iconClass).toEqual('success'); + }); + + it('shows x for failed status', () => { + vm.mr.hasCI = true; + expect(vm.iconClass).toEqual('failed'); + }); + + it('shows x for merge not allowed', () => { + vm.mr.hasCI = true; + expect(vm.iconClass).toEqual('failed'); + }); + }); + describe('mergeButtonText', () => { it('should return Merge', () => { expect(vm.mergeButtonText).toEqual('Merge'); @@ -177,7 +226,7 @@ describe('MRWidgetReadyToMerge', () => { expect(vm.isMergeButtonDisabled).toBeTruthy(); }); - it('should return true when there vm instance is making request', () => { + it('should return true when the vm instance is making request', () => { vm.isMakingRequest = true; expect(vm.isMergeButtonDisabled).toBeTruthy(); }); From ff043cbaeee159b4d3b97d131e4ddef373908855 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Tue, 3 Oct 2017 08:43:26 +1100 Subject: [PATCH 2/2] add changelog --- changelogs/unreleased/37229-mr-widget-status-icon.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/37229-mr-widget-status-icon.yml diff --git a/changelogs/unreleased/37229-mr-widget-status-icon.yml b/changelogs/unreleased/37229-mr-widget-status-icon.yml new file mode 100644 index 00000000000..6d84d1964ca --- /dev/null +++ b/changelogs/unreleased/37229-mr-widget-status-icon.yml @@ -0,0 +1,5 @@ +--- +title: fix merge request widget status icon for failed CI +merge_request: +author: +type: fixed