summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhil Hughes <me@iamphill.com>2018-03-08 13:33:59 +0000
committerPhil Hughes <me@iamphill.com>2018-03-08 13:33:59 +0000
commit28155c9580b72b991fc1b2165b589c882d1fdb11 (patch)
treec871d2342e5faf7d973f57dcbaa73841fb614868
parent7734e85bc6592c5ad3330c611c5f83a051b680b0 (diff)
downloadgitlab-ce-28155c9580b72b991fc1b2165b589c882d1fdb11.tar.gz
Dont show remove source branch checkbox when user cannot remove the branch
Closes #33264
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/components/source_branch_removal_status.vue34
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js4
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/dependencies.js4
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js8
-rw-r--r--changelogs/unreleased/merge-request-widget-source-branch-improvements.yml6
-rw-r--r--spec/features/merge_request/user_sees_merge_widget_spec.rb2
-rw-r--r--spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js12
-rw-r--r--spec/javascripts/vue_mr_widget/mr_widget_options_spec.js41
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();
+ });
+ });
+ });
});