diff options
author | Phil Hughes <me@iamphill.com> | 2018-03-08 13:33:59 +0000 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-03-08 13:33:59 +0000 |
commit | 28155c9580b72b991fc1b2165b589c882d1fdb11 (patch) | |
tree | c871d2342e5faf7d973f57dcbaa73841fb614868 | |
parent | 7734e85bc6592c5ad3330c611c5f83a051b680b0 (diff) | |
download | gitlab-ce-28155c9580b72b991fc1b2165b589c882d1fdb11.tar.gz |
Dont show remove source branch checkbox when user cannot remove the branch
Closes #33264
8 files changed, 99 insertions, 12 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/source_branch_removal_status.vue b/app/assets/javascripts/vue_merge_request_widget/components/source_branch_removal_status.vue new file mode 100644 index 00000000000..460437ceeff --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/source_branch_removal_status.vue @@ -0,0 +1,34 @@ +<script> + import tooltip from '../../vue_shared/directives/tooltip'; + import { __ } from '../../locale'; + + export default { + directives: { + tooltip, + }, + created() { + this.removesBranchText = __('<strong>Removes</strong> source branch'); + this.tooltipTitle = __('A user with write access to the source branch selected this option'); + }, + }; +</script> + +<template> + <p + v-once + class="mr-info-list mr-links source-branch-removal-status append-bottom-0" + > + <span + class="status-text" + v-html="removesBranchText" + > + </span> + <i + v-tooltip + class="fa fa-question-circle" + :title="tooltipTitle" + :aria-label="tooltipTitle" + > + </i> + </p> +</template> 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 162f048aac7..3c781ccddc8 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 @@ -93,7 +93,7 @@ export default { || this.mr.preventMerge); }, isRemoveSourceBranchButtonDisabled() { - return this.isMergeButtonDisabled || !this.mr.canRemoveSourceBranch; + return this.isMergeButtonDisabled; }, shouldShowSquashBeforeMerge() { const { commitsCount, enableSquashBeforeMerge } = this.mr; @@ -282,7 +282,7 @@ export default { </span> <div class="media-body-wrap space-children"> <template v-if="shouldShowMergeControls()"> - <label> + <label v-if="mr.canRemoveSourceBranch"> <input id="remove-source-branch-input" v-model="removeSourceBranch" diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js index a1bc28873df..b867dd90a41 100644 --- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js +++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js @@ -40,7 +40,9 @@ export { default as MRWidgetStore } from './stores/mr_widget_store'; export { default as MRWidgetService } from './services/mr_widget_service'; export { default as eventHub } from './event_hub'; export { default as getStateKey } from './stores/get_state_key'; -export { default as mrWidgetOptions } from './mr_widget_options'; export { default as stateMaps } from './stores/state_maps'; export { default as SquashBeforeMerge } from './components/states/mr_widget_squash_before_merge'; export { default as notify } from '../lib/utils/notify'; +export { default as SourceBranchRemovalStatus } from './components/source_branch_removal_status.vue'; + +export { default as mrWidgetOptions } from './mr_widget_options'; diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js index df3eb86f35c..01365b70897 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js @@ -33,6 +33,7 @@ import { stateMaps, SquashBeforeMerge, notify, + SourceBranchRemovalStatus, } from './dependencies'; import { setFavicon } from '../lib/utils/common_utils'; @@ -69,6 +70,9 @@ export default { shouldRenderDeployments() { return this.mr.deployments.length; }, + shouldRenderSourceBranchRemovalStatus() { + return !this.mr.canRemoveSourceBranch && this.mr.shouldRemoveSourceBranch; + }, }, methods: { createService(store) { @@ -234,6 +238,7 @@ export default { 'mr-widget-merge-when-pipeline-succeeds': MergeWhenPipelineSucceedsState, 'mr-widget-auto-merge-failed': AutoMergeFailed, 'mr-widget-rebase': RebaseState, + SourceBranchRemovalStatus, }, template: ` <div class="mr-state-widget prepend-top-default"> @@ -259,6 +264,9 @@ export default { v-if="shouldRenderRelatedLinks" :state="mr.state" :related-links="mr.relatedLinks" /> + <source-branch-removal-status + v-if="shouldRenderSourceBranchRemovalStatus" + /> </div> <div class="mr-widget-footer" diff --git a/changelogs/unreleased/merge-request-widget-source-branch-improvements.yml b/changelogs/unreleased/merge-request-widget-source-branch-improvements.yml new file mode 100644 index 00000000000..942eb6062fd --- /dev/null +++ b/changelogs/unreleased/merge-request-widget-source-branch-improvements.yml @@ -0,0 +1,6 @@ +--- +title: Fixes remove source branch checkbox being visible when user cannot remove the + branch +merge_request: +author: +type: changed diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index 56224e505d9..ffc16b42690 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -285,7 +285,7 @@ describe 'Merge request > User sees merge widget', :js do end it 'user cannot remove source branch' do - expect(page).to have_field('remove-source-branch-input', disabled: true) + expect(page).not_to have_field('remove-source-branch-input') end end 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 073f26cc78f..58f683fb3e6 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 @@ -517,13 +517,9 @@ describe('MRWidgetReadyToMerge', () => { describe('Remove source branch checkbox', () => { describe('when user can merge but cannot delete branch', () => { - it('isRemoveSourceBranchButtonDisabled should be true', () => { - expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true); - }); - it('should be disabled in the rendered output', () => { const checkboxElement = vm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBe('disabled'); + expect(checkboxElement).toBeNull(); }); }); @@ -540,7 +536,7 @@ describe('MRWidgetReadyToMerge', () => { it('should be enabled in rendered output', () => { const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBeNull(); + expect(checkboxElement).not.toBeNull(); }); }); }); @@ -549,12 +545,12 @@ describe('MRWidgetReadyToMerge', () => { describe('when allowed to merge', () => { beforeEach(() => { vm = createComponent({ - mr: { isMergeAllowed: true }, + mr: { isMergeAllowed: true, canRemoveSourceBranch: true }, }); }); it('shows remove source branch checkbox', () => { - expect(vm.$el.querySelector('.js-remove-source-branch-checkbox')).toBeDefined(); + expect(vm.$el.querySelector('.js-remove-source-branch-checkbox')).not.toBeNull(); }); it('shows modify commit message button', () => { diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index ebe151ac3b1..13c8a0ac19f 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -81,6 +81,29 @@ describe('mrWidgetOptions', () => { }); }); + describe('shouldRenderSourceBranchRemovalStatus', () => { + it('should return true when cannot remove source branch and branch will be removed', () => { + vm.mr.canRemoveSourceBranch = false; + vm.mr.shouldRemoveSourceBranch = true; + + expect(vm.shouldRenderSourceBranchRemovalStatus).toBeTruthy(); + }); + + it('should return false when can remove source branch and branch will be removed', () => { + vm.mr.canRemoveSourceBranch = true; + vm.mr.shouldRemoveSourceBranch = true; + + expect(vm.shouldRenderSourceBranchRemovalStatus).toBeFalsy(); + }); + + it('should return false when cannot remove source branch and branch will not be removed', () => { + vm.mr.canRemoveSourceBranch = false; + vm.mr.shouldRemoveSourceBranch = false; + + expect(vm.shouldRenderSourceBranchRemovalStatus).toBeFalsy(); + }); + }); + describe('shouldRenderDeployments', () => { it('should return false for the initial data', () => { expect(vm.shouldRenderDeployments).toBeFalsy(); @@ -379,4 +402,22 @@ describe('mrWidgetOptions', () => { }); }); }); + + describe('rendering source branch removal status', () => { + it('renders when user cannot remove branch and branch should be removed', (done) => { + vm.mr.canRemoveSourceBranch = false; + vm.mr.shouldRemoveSourceBranch = true; + + vm.$nextTick(() => { + const tooltip = vm.$el.querySelector('.fa-question-circle'); + + expect(vm.$el.textContent).toContain('Removes source branch'); + expect(tooltip.getAttribute('data-original-title')).toBe( + 'A user with write access to the source branch selected this option', + ); + + done(); + }); + }); + }); }); |