Merge branch 'merge-request-widget-source-branch-improvements' into 'master'
Dont show remove source branch checkbox when user cannot remove the branch Closes #33264 See merge request gitlab-org/gitlab-ce!17642
This commit is contained in:
commit
475dd106a8
|
@ -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>
|
|
@ -93,7 +93,7 @@ export default {
|
||||||
|| this.mr.preventMerge);
|
|| this.mr.preventMerge);
|
||||||
},
|
},
|
||||||
isRemoveSourceBranchButtonDisabled() {
|
isRemoveSourceBranchButtonDisabled() {
|
||||||
return this.isMergeButtonDisabled || !this.mr.canRemoveSourceBranch;
|
return this.isMergeButtonDisabled;
|
||||||
},
|
},
|
||||||
shouldShowSquashBeforeMerge() {
|
shouldShowSquashBeforeMerge() {
|
||||||
const { commitsCount, enableSquashBeforeMerge } = this.mr;
|
const { commitsCount, enableSquashBeforeMerge } = this.mr;
|
||||||
|
@ -282,7 +282,7 @@ export default {
|
||||||
</span>
|
</span>
|
||||||
<div class="media-body-wrap space-children">
|
<div class="media-body-wrap space-children">
|
||||||
<template v-if="shouldShowMergeControls()">
|
<template v-if="shouldShowMergeControls()">
|
||||||
<label>
|
<label v-if="mr.canRemoveSourceBranch">
|
||||||
<input
|
<input
|
||||||
id="remove-source-branch-input"
|
id="remove-source-branch-input"
|
||||||
v-model="removeSourceBranch"
|
v-model="removeSourceBranch"
|
||||||
|
|
|
@ -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 MRWidgetService } from './services/mr_widget_service';
|
||||||
export { default as eventHub } from './event_hub';
|
export { default as eventHub } from './event_hub';
|
||||||
export { default as getStateKey } from './stores/get_state_key';
|
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 stateMaps } from './stores/state_maps';
|
||||||
export { default as SquashBeforeMerge } from './components/states/mr_widget_squash_before_merge';
|
export { default as SquashBeforeMerge } from './components/states/mr_widget_squash_before_merge';
|
||||||
export { default as notify } from '../lib/utils/notify';
|
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';
|
||||||
|
|
|
@ -33,6 +33,7 @@ import {
|
||||||
stateMaps,
|
stateMaps,
|
||||||
SquashBeforeMerge,
|
SquashBeforeMerge,
|
||||||
notify,
|
notify,
|
||||||
|
SourceBranchRemovalStatus,
|
||||||
} from './dependencies';
|
} from './dependencies';
|
||||||
import { setFavicon } from '../lib/utils/common_utils';
|
import { setFavicon } from '../lib/utils/common_utils';
|
||||||
|
|
||||||
|
@ -69,6 +70,9 @@ export default {
|
||||||
shouldRenderDeployments() {
|
shouldRenderDeployments() {
|
||||||
return this.mr.deployments.length;
|
return this.mr.deployments.length;
|
||||||
},
|
},
|
||||||
|
shouldRenderSourceBranchRemovalStatus() {
|
||||||
|
return !this.mr.canRemoveSourceBranch && this.mr.shouldRemoveSourceBranch;
|
||||||
|
},
|
||||||
},
|
},
|
||||||
methods: {
|
methods: {
|
||||||
createService(store) {
|
createService(store) {
|
||||||
|
@ -234,6 +238,7 @@ export default {
|
||||||
'mr-widget-merge-when-pipeline-succeeds': MergeWhenPipelineSucceedsState,
|
'mr-widget-merge-when-pipeline-succeeds': MergeWhenPipelineSucceedsState,
|
||||||
'mr-widget-auto-merge-failed': AutoMergeFailed,
|
'mr-widget-auto-merge-failed': AutoMergeFailed,
|
||||||
'mr-widget-rebase': RebaseState,
|
'mr-widget-rebase': RebaseState,
|
||||||
|
SourceBranchRemovalStatus,
|
||||||
},
|
},
|
||||||
template: `
|
template: `
|
||||||
<div class="mr-state-widget prepend-top-default">
|
<div class="mr-state-widget prepend-top-default">
|
||||||
|
@ -259,6 +264,9 @@ export default {
|
||||||
v-if="shouldRenderRelatedLinks"
|
v-if="shouldRenderRelatedLinks"
|
||||||
:state="mr.state"
|
:state="mr.state"
|
||||||
:related-links="mr.relatedLinks" />
|
:related-links="mr.relatedLinks" />
|
||||||
|
<source-branch-removal-status
|
||||||
|
v-if="shouldRenderSourceBranchRemovalStatus"
|
||||||
|
/>
|
||||||
</div>
|
</div>
|
||||||
<div
|
<div
|
||||||
class="mr-widget-footer"
|
class="mr-widget-footer"
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
title: Fixes remove source branch checkbox being visible when user cannot remove the
|
||||||
|
branch
|
||||||
|
merge_request:
|
||||||
|
author:
|
||||||
|
type: changed
|
|
@ -1,6 +1,8 @@
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
|
|
||||||
describe 'Merge request > User sees merge widget', :js do
|
describe 'Merge request > User sees merge widget', :js do
|
||||||
|
include ProjectForksHelper
|
||||||
|
|
||||||
let(:project) { create(:project, :repository) }
|
let(:project) { create(:project, :repository) }
|
||||||
let(:project_only_mwps) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) }
|
let(:project_only_mwps) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) }
|
||||||
let(:user) { project.creator }
|
let(:user) { project.creator }
|
||||||
|
@ -285,7 +287,29 @@ describe 'Merge request > User sees merge widget', :js do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'user cannot remove source branch' do
|
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
|
||||||
|
|
||||||
|
context 'user cannot merge project and cannot push to fork', :js do
|
||||||
|
let(:forked_project) { fork_project(project, nil, repository: true) }
|
||||||
|
let(:user2) { create(:user) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
project.add_developer(user2)
|
||||||
|
sign_out(:user)
|
||||||
|
sign_in(user2)
|
||||||
|
merge_request.update(
|
||||||
|
source_project: forked_project,
|
||||||
|
target_project: project,
|
||||||
|
merge_params: { 'force_remove_source_branch' => '1' }
|
||||||
|
)
|
||||||
|
visit project_merge_request_path(project, merge_request)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'user cannot remove source branch' do
|
||||||
|
expect(page).not_to have_field('remove-source-branch-input')
|
||||||
|
expect(page).to have_content('Removes source branch')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -517,13 +517,9 @@ describe('MRWidgetReadyToMerge', () => {
|
||||||
|
|
||||||
describe('Remove source branch checkbox', () => {
|
describe('Remove source branch checkbox', () => {
|
||||||
describe('when user can merge but cannot delete branch', () => {
|
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', () => {
|
it('should be disabled in the rendered output', () => {
|
||||||
const checkboxElement = vm.$el.querySelector('#remove-source-branch-input');
|
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', () => {
|
it('should be enabled in rendered output', () => {
|
||||||
const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input');
|
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', () => {
|
describe('when allowed to merge', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vm = createComponent({
|
vm = createComponent({
|
||||||
mr: { isMergeAllowed: true },
|
mr: { isMergeAllowed: true, canRemoveSourceBranch: true },
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('shows remove source branch checkbox', () => {
|
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', () => {
|
it('shows modify commit message button', () => {
|
||||||
|
|
|
@ -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).toEqual(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
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).toEqual(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
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).toEqual(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('shouldRenderDeployments', () => {
|
describe('shouldRenderDeployments', () => {
|
||||||
it('should return false for the initial data', () => {
|
it('should return false for the initial data', () => {
|
||||||
expect(vm.shouldRenderDeployments).toBeFalsy();
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
Loading…
Reference in New Issue