Commit 73803b08 authored by Igor Drozdov's avatar Igor Drozdov

Merge branch 'ph/235712/widgetConflictsToGraphql' into 'master'

Converts widget conflicts state data to GraphQL

See merge request gitlab-org/gitlab!48125
parents e8fb6df8 54bdd83e
<script> <script>
import $ from 'jquery'; import $ from 'jquery';
import { escape } from 'lodash'; import { escape } from 'lodash';
import { GlButton, GlModalDirective } from '@gitlab/ui'; import { GlButton, GlModalDirective, GlSkeletonLoader } from '@gitlab/ui';
import { s__, sprintf } from '~/locale'; import { s__, sprintf } from '~/locale';
import { mouseenter, debouncedMouseleave, togglePopover } from '~/shared/popover'; import { mouseenter, debouncedMouseleave, togglePopover } from '~/shared/popover';
import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables';
import StatusIcon from '../mr_widget_status_icon.vue'; import StatusIcon from '../mr_widget_status_icon.vue';
import userPermissionsQuery from '../../queries/permissions.query.graphql';
import conflictsStateQuery from '../../queries/states/conflicts.query.graphql';
export default { export default {
name: 'MRWidgetConflicts', name: 'MRWidgetConflicts',
components: { components: {
GlSkeletonLoader,
StatusIcon, StatusIcon,
GlButton, GlButton,
}, },
directives: { directives: {
GlModalDirective, GlModalDirective,
}, },
mixins: [glFeatureFlagMixin(), mergeRequestQueryVariablesMixin],
apollo: {
userPermissions: {
query: userPermissionsQuery,
skip() {
return !this.glFeatures.mergeRequestWidgetGraphql;
},
variables() {
return this.mergeRequestQueryVariables;
},
update: data => data.project.mergeRequest.userPermissions,
},
stateData: {
query: conflictsStateQuery,
skip() {
return !this.glFeatures.mergeRequestWidgetGraphql;
},
variables() {
return this.mergeRequestQueryVariables;
},
update: data => data.project.mergeRequest,
},
},
props: { props: {
/* TODO: This is providing all store and service down when it /* TODO: This is providing all store and service down when it
only needs a few props */ only needs a few props */
...@@ -24,21 +52,72 @@ export default { ...@@ -24,21 +52,72 @@ export default {
default: () => ({}), default: () => ({}),
}, },
}, },
data() {
return {
userPermissions: {},
stateData: {},
};
},
computed: { computed: {
isLoading() {
return (
this.glFeatures.mergeRequestWidgetGraphql &&
this.$apollo.queries.userPermissions.loading &&
this.$apollo.queries.stateData.loading
);
},
canPushToSourceBranch() {
if (this.glFeatures.mergeRequestWidgetGraphql) {
return this.userPermissions.pushToSourceBranch;
}
return this.mr.canPushToSourceBranch;
},
canMerge() {
if (this.glFeatures.mergeRequestWidgetGraphql) {
return this.userPermissions.canMerge;
}
return this.mr.canMerge;
},
shouldBeRebased() {
if (this.glFeatures.mergeRequestWidgetGraphql) {
return this.stateData.shouldBeRebased;
}
return this.mr.shouldBeRebased;
},
sourceBranchProtected() {
if (this.glFeatures.mergeRequestWidgetGraphql) {
return this.stateData.sourceBranchProtected;
}
return this.mr.sourceBranchProtected;
},
popoverTitle() { popoverTitle() {
return s__( return s__(
'mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected.', 'mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected.',
); );
}, },
showResolveButton() { showResolveButton() {
return this.mr.conflictResolutionPath && this.mr.canPushToSourceBranch; return this.mr.conflictResolutionPath && this.canPushToSourceBranch;
}, },
showPopover() { showPopover() {
return this.showResolveButton && this.mr.sourceBranchProtected; return this.showResolveButton && this.sourceBranchProtected;
}, },
}, },
mounted() { watch: {
if (this.showPopover) { showPopover: {
handler(newVal) {
if (newVal) {
this.$nextTick(this.initPopover);
}
},
immediate: true,
},
},
methods: {
initPopover() {
const $el = $(this.$refs.popover); const $el = $(this.$refs.popover);
$el $el
...@@ -68,7 +147,7 @@ export default { ...@@ -68,7 +147,7 @@ export default {
.on('show.bs.popover', () => { .on('show.bs.popover', () => {
window.addEventListener('scroll', togglePopover.bind($el, false), { once: true }); window.addEventListener('scroll', togglePopover.bind($el, false), { once: true });
}); });
} },
}, },
}; };
</script> </script>
...@@ -76,34 +155,41 @@ export default { ...@@ -76,34 +155,41 @@ export default {
<div class="mr-widget-body media"> <div class="mr-widget-body media">
<status-icon :show-disabled-button="true" status="warning" /> <status-icon :show-disabled-button="true" status="warning" />
<div class="media-body space-children"> <div v-if="isLoading" class="gl-ml-4 gl-w-full mr-conflict-loader">
<span v-if="mr.shouldBeRebased" class="bold"> <gl-skeleton-loader :width="334" :height="30">
<rect x="0" y="7" width="150" height="16" rx="4" />
<rect x="158" y="7" width="84" height="16" rx="4" />
<rect x="250" y="7" width="84" height="16" rx="4" />
</gl-skeleton-loader>
</div>
<div v-else class="media-body space-children">
<span v-if="shouldBeRebased" class="bold">
{{ {{
s__(`mrWidget|Fast-forward merge is not possible. s__(`mrWidget|Fast-forward merge is not possible.
To merge this request, first rebase locally.`) To merge this request, first rebase locally.`)
}} }}
</span> </span>
<template v-else> <template v-else>
<span class="bold"> <span class="bold">
{{ s__('mrWidget|There are merge conflicts') }}<span v-if="!mr.canMerge">.</span> {{ s__('mrWidget|There are merge conflicts') }}<span v-if="!canMerge">.</span>
<span v-if="!mr.canMerge"> <span v-if="!canMerge">
{{ {{
s__(`mrWidget|Resolve these conflicts or ask someone s__(`mrWidget|Resolve these conflicts or ask someone
with write access to this repository to merge it locally`) with write access to this repository to merge it locally`)
}} }}
</span> </span>
</span> </span>
<span v-if="showResolveButton" ref="popover"> <span v-if="showResolveButton" ref="popover">
<gl-button <gl-button
:href="mr.conflictResolutionPath" :href="!sourceBranchProtected && mr.conflictResolutionPath"
:disabled="mr.sourceBranchProtected" :disabled="sourceBranchProtected"
class="js-resolve-conflicts-button" class="js-resolve-conflicts-button"
> >
{{ s__('mrWidget|Resolve conflicts') }} {{ s__('mrWidget|Resolve conflicts') }}
</gl-button> </gl-button>
</span> </span>
<gl-button <gl-button
v-if="mr.canMerge" v-if="canMerge"
v-gl-modal-directive="'modal-merge-info'" v-gl-modal-directive="'modal-merge-info'"
class="js-merge-locally-button" class="js-merge-locally-button"
> >
......
query userPermissionsQuery($projectPath: ID!, $iid: String!) {
project(fullPath: $projectPath) {
mergeRequest(iid: $iid) {
userPermissions {
canMerge
pushToSourceBranch
}
}
}
}
query workInProgressQuery($projectPath: ID!, $iid: String!) {
project(fullPath: $projectPath) {
mergeRequest(iid: $iid) {
shouldBeRebased
sourceBranchProtected
}
}
}
...@@ -1039,3 +1039,11 @@ $mr-widget-min-height: 69px; ...@@ -1039,3 +1039,11 @@ $mr-widget-min-height: 69px;
.diff-file-row.is-active { .diff-file-row.is-active {
background-color: $gray-50; background-color: $gray-50;
} }
.mr-conflict-loader {
max-width: 334px;
> svg {
vertical-align: middle;
}
}
...@@ -49,6 +49,8 @@ module Types ...@@ -49,6 +49,8 @@ module Types
description: 'ID of the merge request target project' description: 'ID of the merge request target project'
field :source_branch, GraphQL::STRING_TYPE, null: false, field :source_branch, GraphQL::STRING_TYPE, null: false,
description: 'Source branch of the merge request' description: 'Source branch of the merge request'
field :source_branch_protected, GraphQL::BOOLEAN_TYPE, null: false, calls_gitaly: true,
description: 'Indicates if the source branch is protected'
field :target_branch, GraphQL::STRING_TYPE, null: false, field :target_branch, GraphQL::STRING_TYPE, null: false,
description: 'Target branch of the merge request' description: 'Target branch of the merge request'
field :work_in_progress, GraphQL::BOOLEAN_TYPE, method: :work_in_progress?, null: false, field :work_in_progress, GraphQL::BOOLEAN_TYPE, method: :work_in_progress?, null: false,
...@@ -194,6 +196,10 @@ module Types ...@@ -194,6 +196,10 @@ module Types
def commit_count def commit_count
object&.metrics&.commits_count object&.metrics&.commits_count
end end
def source_branch_protected
object.source_project.present? && ProtectedBranch.protected?(object.source_project, object.source_branch)
end
end end
end end
Types::MergeRequestType.prepend_if_ee('::EE::Types::MergeRequestType') Types::MergeRequestType.prepend_if_ee('::EE::Types::MergeRequestType')
...@@ -12780,6 +12780,11 @@ type MergeRequest implements CurrentUserTodos & Noteable { ...@@ -12780,6 +12780,11 @@ type MergeRequest implements CurrentUserTodos & Noteable {
""" """
sourceBranchExists: Boolean! sourceBranchExists: Boolean!
"""
Indicates if the source branch is protected
"""
sourceBranchProtected: Boolean!
""" """
Source project of the merge request Source project of the merge request
""" """
......
...@@ -35042,6 +35042,24 @@ ...@@ -35042,6 +35042,24 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
}, },
{
"name": "sourceBranchProtected",
"description": "Indicates if the source branch is protected",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "Boolean",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
},
{ {
"name": "sourceProject", "name": "sourceProject",
"description": "Source project of the merge request", "description": "Source project of the merge request",
...@@ -1950,6 +1950,7 @@ Autogenerated return type of MarkAsSpamSnippet. ...@@ -1950,6 +1950,7 @@ Autogenerated return type of MarkAsSpamSnippet.
| `shouldRemoveSourceBranch` | Boolean | Indicates if the source branch of the merge request will be deleted after merge | | `shouldRemoveSourceBranch` | Boolean | Indicates if the source branch of the merge request will be deleted after merge |
| `sourceBranch` | String! | Source branch of the merge request | | `sourceBranch` | String! | Source branch of the merge request |
| `sourceBranchExists` | Boolean! | Indicates if the source branch of the merge request exists | | `sourceBranchExists` | Boolean! | Indicates if the source branch of the merge request exists |
| `sourceBranchProtected` | Boolean! | Indicates if the source branch is protected |
| `sourceProject` | Project | Source project of the merge request | | `sourceProject` | Project | Source project of the merge request |
| `sourceProjectId` | Int | ID of the merge request source project | | `sourceProjectId` | Int | ID of the merge request source project |
| `state` | MergeRequestState! | State of the merge request | | `state` | MergeRequestState! | State of the merge request |
......
...@@ -6,6 +6,7 @@ import ConflictsComponent from '~/vue_merge_request_widget/components/states/mr_ ...@@ -6,6 +6,7 @@ import ConflictsComponent from '~/vue_merge_request_widget/components/states/mr_
describe('MRWidgetConflicts', () => { describe('MRWidgetConflicts', () => {
let vm; let vm;
let mergeRequestWidgetGraphql = null;
const path = '/conflicts'; const path = '/conflicts';
function createComponent(propsData = {}) { function createComponent(propsData = {}) {
...@@ -13,7 +14,35 @@ describe('MRWidgetConflicts', () => { ...@@ -13,7 +14,35 @@ describe('MRWidgetConflicts', () => {
vm = shallowMount(localVue.extend(ConflictsComponent), { vm = shallowMount(localVue.extend(ConflictsComponent), {
propsData, propsData,
provide: {
glFeatures: {
mergeRequestWidgetGraphql,
},
},
mocks: {
$apollo: {
queries: {
userPermissions: { loading: false },
stateData: { loading: false },
},
},
},
}); });
if (mergeRequestWidgetGraphql) {
vm.setData({
userPermissions: {
canMerge: propsData.mr.canMerge,
pushToSourceBranch: propsData.mr.canPushToSourceBranch,
},
stateData: {
shouldBeRebased: propsData.mr.shouldBeRebased,
sourceBranchProtected: propsData.mr.sourceBranchProtected,
},
});
}
return vm.vm.$nextTick();
} }
beforeEach(() => { beforeEach(() => {
...@@ -21,206 +50,215 @@ describe('MRWidgetConflicts', () => { ...@@ -21,206 +50,215 @@ describe('MRWidgetConflicts', () => {
}); });
afterEach(() => { afterEach(() => {
mergeRequestWidgetGraphql = null;
vm.destroy(); vm.destroy();
}); });
// There are two permissions we need to consider: [false, true].forEach(featureEnabled => {
// describe(`with GraphQL feature flag ${featureEnabled ? 'enabled' : 'disabled'}`, () => {
// 1. Is the user allowed to merge to the target branch? beforeEach(() => {
// 2. Is the user allowed to push to the source branch? mergeRequestWidgetGraphql = featureEnabled;
//
// 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(() => {
createComponent({
mr: {
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', () => { // There are two permissions we need to consider:
beforeEach(() => { //
createComponent({ // 1. Is the user allowed to merge to the target branch?
mr: { // 2. Is the user allowed to push to the source branch?
canMerge: false, //
canPushToSourceBranch: true, // This yields 4 possible permutations that we need to test, and
conflictResolutionPath: path, // we test them below. A user who can push to the source
conflictsDocsPath: '', // 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(async () => {
await createComponent({
it('should tell you about conflicts', () => { mr: {
expect(vm.text()).toContain('There are merge conflicts'); canMerge: true,
expect(vm.text()).toContain('ask someone with write access'); canPushToSourceBranch: false,
}); conflictResolutionPath: path,
conflictsDocsPath: '',
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 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 have merge buttons', () => { });
expect(vm.text()).not.toContain('Merge locally');
}); it('should not allow you to resolve the conflicts', () => {
}); expect(vm.text()).not.toContain('Resolve conflicts');
});
describe('when allowed to merge and push to source branch', () => {
beforeEach(() => { it('should have merge buttons', () => {
createComponent({ const mergeLocallyButton = vm.find('.js-merge-locally-button');
mr: {
canMerge: true, expect(mergeLocallyButton.text()).toContain('Merge locally');
canPushToSourceBranch: true, });
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 allow you to resolve the conflicts', () => { describe('when not allowed to merge but allowed to push to source branch', () => {
const resolveButton = vm.find('.js-resolve-conflicts-button'); beforeEach(async () => {
await createComponent({
expect(resolveButton.text()).toContain('Resolve conflicts'); mr: {
expect(resolveButton.attributes('href')).toEqual(path); canMerge: false,
}); canPushToSourceBranch: true,
conflictResolutionPath: path,
it('should have merge buttons', () => { conflictsDocsPath: '',
const mergeLocallyButton = vm.find('.js-merge-locally-button'); },
});
expect(mergeLocallyButton.text()).toContain('Merge locally'); });
});
}); it('should tell you about conflicts', () => {
expect(vm.text()).toContain('There are merge conflicts');
describe('when user does not have permission to push to source branch', () => { expect(vm.text()).toContain('ask someone with write access');
it('should show proper message', () => { });
createComponent({
mr: { it('should allow you to resolve the conflicts', () => {
canMerge: false, const resolveButton = vm.find('.js-resolve-conflicts-button');
canPushToSourceBranch: false,
conflictsDocsPath: '', 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');
});
}); });
expect( describe('when allowed to merge and push to source branch', () => {
vm beforeEach(async () => {
.text() await createComponent({
.trim() mr: {
.replace(/\s\s+/g, ' '), canMerge: true,
).toContain('ask someone with write access'); canPushToSourceBranch: true,
}); conflictResolutionPath: path,
conflictsDocsPath: '',
it('should not have action buttons', () => { },
createComponent({ });
mr: { });
canMerge: false,
canPushToSourceBranch: false, it('should tell you about conflicts without bothering other people', () => {
conflictsDocsPath: '', expect(vm.text()).toContain('There are merge conflicts');
}, expect(vm.text()).not.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 have merge buttons', () => {
const mergeLocallyButton = vm.find('.js-merge-locally-button');
expect(mergeLocallyButton.text()).toContain('Merge locally');
});
}); });
expect(vm.find('.js-resolve-conflicts-button').exists()).toBe(false); describe('when user does not have permission to push to source branch', () => {
expect(vm.find('.js-merge-locally-button').exists()).toBe(false); it('should show proper message', async () => {
}); await createComponent({
mr: {
it('should not have resolve button when no conflict resolution path', () => { canMerge: false,
createComponent({ canPushToSourceBranch: false,
mr: { conflictsDocsPath: '',
canMerge: true, },
conflictResolutionPath: null, });
conflictsDocsPath: '',
}, expect(
vm
.text()
.trim()
.replace(/\s\s+/g, ' '),
).toContain('ask someone with write access');
});
it('should not have action buttons', async () => {
await createComponent({
mr: {
canMerge: false,
canPushToSourceBranch: false,
conflictsDocsPath: '',
},
});
expect(vm.find('.js-resolve-conflicts-button').exists()).toBe(false);
expect(vm.find('.js-merge-locally-button').exists()).toBe(false);
});
it('should not have resolve button when no conflict resolution path', async () => {
await createComponent({
mr: {
canMerge: true,
conflictResolutionPath: null,
conflictsDocsPath: '',
},
});
expect(vm.find('.js-resolve-conflicts-button').exists()).toBe(false);
});
}); });
expect(vm.find('.js-resolve-conflicts-button').exists()).toBe(false); describe('when fast-forward or semi-linear merge enabled', () => {
}); it('should tell you to rebase locally', async () => {
}); await createComponent({
mr: {
describe('when fast-forward or semi-linear merge enabled', () => { shouldBeRebased: true,
it('should tell you to rebase locally', () => { conflictsDocsPath: '',
createComponent({ },
mr: { });
shouldBeRebased: true,
conflictsDocsPath: '', expect(removeBreakLine(vm.text()).trim()).toContain(
}, 'Fast-forward merge is not possible. To merge this request, first rebase locally.',
);
});
}); });
expect(removeBreakLine(vm.text()).trim()).toContain( describe('when source branch protected', () => {
'Fast-forward merge is not possible. To merge this request, first rebase locally.', beforeEach(async () => {
); await createComponent({
}); mr: {
}); canMerge: true,
canPushToSourceBranch: true,
describe('when source branch protected', () => { conflictResolutionPath: TEST_HOST,
beforeEach(() => { sourceBranchProtected: true,
createComponent({ conflictsDocsPath: '',
mr: { },
canMerge: true, });
canPushToSourceBranch: true, });
conflictResolutionPath: TEST_HOST,
sourceBranchProtected: true, it('sets resolve button as disabled', () => {
conflictsDocsPath: '', expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe('true');
}, });
it('renders popover', () => {
expect($.fn.popover).toHaveBeenCalled();
});
}); });
});
it('sets resolve button as disabled', () => {
expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe('true');
});
it('renders popover', () => { describe('when source branch not protected', () => {
expect($.fn.popover).toHaveBeenCalled(); beforeEach(async () => {
}); await createComponent({
}); mr: {
canMerge: true,
describe('when source branch not protected', () => { canPushToSourceBranch: true,
beforeEach(() => { conflictResolutionPath: TEST_HOST,
createComponent({ sourceBranchProtected: false,
mr: { conflictsDocsPath: '',
canMerge: true, },
canPushToSourceBranch: true, });
conflictResolutionPath: TEST_HOST, });
sourceBranchProtected: false,
conflictsDocsPath: '', it('sets resolve button as disabled', () => {
}, expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe(undefined);
});
it('renders popover', () => {
expect($.fn.popover).not.toHaveBeenCalled();
});
}); });
}); });
it('sets resolve button as disabled', () => {
expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe(undefined);
});
it('renders popover', () => {
expect($.fn.popover).not.toHaveBeenCalled();
});
}); });
}); });
...@@ -27,7 +27,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do ...@@ -27,7 +27,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do
upvotes downvotes head_pipeline pipelines task_completion_status upvotes downvotes head_pipeline pipelines task_completion_status
milestone assignees participants subscribed labels discussion_locked time_estimate milestone assignees participants subscribed labels discussion_locked time_estimate
total_time_spent reference author merged_at commit_count current_user_todos total_time_spent reference author merged_at commit_count current_user_todos
conflicts auto_merge_enabled approved_by conflicts auto_merge_enabled approved_by source_branch_protected
] ]
if Gitlab.ee? if Gitlab.ee?
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment