Commit 54bdd83e authored by Phil Hughes's avatar Phil Hughes Committed by Igor Drozdov

Converts widget conflicts state data to GraphQL

Moves the state data used in the conflicts widget state
over to GraphQL.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/235712
parent dbb363fd
<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