Fix "Resolve conflicts" button not appearing for some users
Previously the frontend assumed that the user had to be able to merge to that project in order to resolve conflicts. However, this is overly restrictive, as the user only has to be able to push to the source branch. In fact, appending the text /conflicts to the merge request would bring up the conflict resolution page. This confusion happens when a project contains a protected branch that only allows maintainers to push. Users with Developer access no longer have permission to merge, but they still can create branches in that project. To fix this issue, we now loosen the permission check for the "Resolve conflicts" button and only check for access to push to the source branch. This is consistent with what the backend does in MergeRequests::Conflicts::ListService#can_be_resolved_by?. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/47954
This commit is contained in:
parent
4373d0ddad
commit
1405b9cd50
3 changed files with 79 additions and 3 deletions
|
@ -26,7 +26,7 @@ export default {
|
||||||
);
|
);
|
||||||
},
|
},
|
||||||
showResolveButton() {
|
showResolveButton() {
|
||||||
return this.mr.conflictResolutionPath && this.mr.canMerge;
|
return this.mr.conflictResolutionPath && this.mr.canPushToSourceBranch;
|
||||||
},
|
},
|
||||||
showPopover() {
|
showPopover() {
|
||||||
return this.showResolveButton && this.mr.sourceBranchProtected;
|
return this.showResolveButton && this.mr.sourceBranchProtected;
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Fix "Resolve conflicts" button not appearing for some users
|
||||||
|
merge_request: 29535
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -23,11 +23,78 @@ describe('MRWidgetConflicts', () => {
|
||||||
vm.destroy();
|
vm.destroy();
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('when allowed to merge', () => {
|
// There are two permissions we need to consider:
|
||||||
|
//
|
||||||
|
// 1. Is the user allowed to merge to the target branch?
|
||||||
|
// 2. Is the user allowed to push to the source branch?
|
||||||
|
//
|
||||||
|
// This yields 4 possible permutations that we need to test, and
|
||||||
|
// we test them below. A user who can push to the source
|
||||||
|
// branch should be allowed to resolve conflicts. This is
|
||||||
|
// consistent with what the backend does.
|
||||||
|
describe('when allowed to merge but not allowed to push to source branch', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
createComponent({
|
createComponent({
|
||||||
mr: {
|
mr: {
|
||||||
canMerge: true,
|
canMerge: true,
|
||||||
|
canPushToSourceBranch: false,
|
||||||
|
conflictResolutionPath: path,
|
||||||
|
conflictsDocsPath: '',
|
||||||
|
},
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should tell you about conflicts without bothering other people', () => {
|
||||||
|
expect(vm.text()).toContain('There are merge conflicts');
|
||||||
|
expect(vm.text()).not.toContain('ask someone with write access');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not allow you to resolve the conflicts', () => {
|
||||||
|
expect(vm.text()).not.toContain('Resolve conflicts');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should have merge buttons', () => {
|
||||||
|
const mergeLocallyButton = vm.find('.js-merge-locally-button');
|
||||||
|
|
||||||
|
expect(mergeLocallyButton.text()).toContain('Merge locally');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('when not allowed to merge but allowed to push to source branch', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
createComponent({
|
||||||
|
mr: {
|
||||||
|
canMerge: false,
|
||||||
|
canPushToSourceBranch: true,
|
||||||
|
conflictResolutionPath: path,
|
||||||
|
conflictsDocsPath: '',
|
||||||
|
},
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should tell you about conflicts', () => {
|
||||||
|
expect(vm.text()).toContain('There are merge conflicts');
|
||||||
|
expect(vm.text()).toContain('ask someone with write access');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should allow you to resolve the conflicts', () => {
|
||||||
|
const resolveButton = vm.find('.js-resolve-conflicts-button');
|
||||||
|
|
||||||
|
expect(resolveButton.text()).toContain('Resolve conflicts');
|
||||||
|
expect(resolveButton.attributes('href')).toEqual(path);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not have merge buttons', () => {
|
||||||
|
expect(vm.text()).not.toContain('Merge locally');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('when allowed to merge and push to source branch', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
createComponent({
|
||||||
|
mr: {
|
||||||
|
canMerge: true,
|
||||||
|
canPushToSourceBranch: true,
|
||||||
conflictResolutionPath: path,
|
conflictResolutionPath: path,
|
||||||
conflictsDocsPath: '',
|
conflictsDocsPath: '',
|
||||||
},
|
},
|
||||||
|
@ -53,11 +120,12 @@ describe('MRWidgetConflicts', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('when user does not have permission to merge', () => {
|
describe('when user does not have permission to push to source branch', () => {
|
||||||
it('should show proper message', () => {
|
it('should show proper message', () => {
|
||||||
createComponent({
|
createComponent({
|
||||||
mr: {
|
mr: {
|
||||||
canMerge: false,
|
canMerge: false,
|
||||||
|
canPushToSourceBranch: false,
|
||||||
conflictsDocsPath: '',
|
conflictsDocsPath: '',
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
@ -74,6 +142,7 @@ describe('MRWidgetConflicts', () => {
|
||||||
createComponent({
|
createComponent({
|
||||||
mr: {
|
mr: {
|
||||||
canMerge: false,
|
canMerge: false,
|
||||||
|
canPushToSourceBranch: false,
|
||||||
conflictsDocsPath: '',
|
conflictsDocsPath: '',
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
@ -115,6 +184,7 @@ describe('MRWidgetConflicts', () => {
|
||||||
createComponent({
|
createComponent({
|
||||||
mr: {
|
mr: {
|
||||||
canMerge: true,
|
canMerge: true,
|
||||||
|
canPushToSourceBranch: true,
|
||||||
conflictResolutionPath: gl.TEST_HOST,
|
conflictResolutionPath: gl.TEST_HOST,
|
||||||
sourceBranchProtected: true,
|
sourceBranchProtected: true,
|
||||||
conflictsDocsPath: '',
|
conflictsDocsPath: '',
|
||||||
|
@ -136,6 +206,7 @@ describe('MRWidgetConflicts', () => {
|
||||||
createComponent({
|
createComponent({
|
||||||
mr: {
|
mr: {
|
||||||
canMerge: true,
|
canMerge: true,
|
||||||
|
canPushToSourceBranch: true,
|
||||||
conflictResolutionPath: gl.TEST_HOST,
|
conflictResolutionPath: gl.TEST_HOST,
|
||||||
sourceBranchProtected: false,
|
sourceBranchProtected: false,
|
||||||
conflictsDocsPath: '',
|
conflictsDocsPath: '',
|
||||||
|
|
Loading…
Reference in a new issue