Commit 453a1d76 authored by Phil Hughes's avatar Phil Hughes

Fixes bugs in the widgets GraphQL queries

This fixes some bugs that are blocking the rollout of
enabling GraphQL in the merge request widget.
parent c8141f4c
...@@ -159,6 +159,7 @@ export default { ...@@ -159,6 +159,7 @@ export default {
.then((data) => { .then((data) => {
this.mr.setApprovals(data); this.mr.setApprovals(data);
eventHub.$emit('MRWidgetUpdateRequested'); eventHub.$emit('MRWidgetUpdateRequested');
eventHub.$emit('ApprovalUpdated');
this.$emit('updated'); this.$emit('updated');
}) })
.catch(errFn) .catch(errFn)
......
...@@ -4,6 +4,7 @@ import autoMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/auto_merg ...@@ -4,6 +4,7 @@ import autoMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/auto_merg
import autoMergeEnabledQuery from 'ee_else_ce/vue_merge_request_widget/queries/states/auto_merge_enabled.query.graphql'; import autoMergeEnabledQuery from 'ee_else_ce/vue_merge_request_widget/queries/states/auto_merge_enabled.query.graphql';
import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { __ } from '~/locale'; import { __ } from '~/locale';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import { deprecatedCreateFlash as Flash } from '../../../flash'; import { deprecatedCreateFlash as Flash } from '../../../flash';
import statusIcon from '../mr_widget_status_icon.vue'; import statusIcon from '../mr_widget_status_icon.vue';
import MrWidgetAuthor from '../mr_widget_author.vue'; import MrWidgetAuthor from '../mr_widget_author.vue';
...@@ -53,7 +54,11 @@ export default { ...@@ -53,7 +54,11 @@ export default {
}, },
computed: { computed: {
loading() { loading() {
return this.glFeatures.mergeRequestWidgetGraphql && this.$apollo.queries.state.loading; return (
this.glFeatures.mergeRequestWidgetGraphql &&
this.$apollo.queries.state.loading &&
Object.keys(this.state).length === 0
);
}, },
mergeUser() { mergeUser() {
if (this.glFeatures.mergeRequestWidgetGraphql) { if (this.glFeatures.mergeRequestWidgetGraphql) {
...@@ -78,7 +83,7 @@ export default { ...@@ -78,7 +83,7 @@ export default {
canRemoveSourceBranch() { canRemoveSourceBranch() {
const { currentUserId } = this.mr; const { currentUserId } = this.mr;
const mergeUserId = this.glFeatures.mergeRequestWidgetGraphql const mergeUserId = this.glFeatures.mergeRequestWidgetGraphql
? this.state.mergeUser?.id ? getIdFromGraphQLId(this.state.mergeUser?.id)
: this.mr.mergeUserId; : this.mr.mergeUserId;
const canRemoveSourceBranch = this.glFeatures.mergeRequestWidgetGraphql const canRemoveSourceBranch = this.glFeatures.mergeRequestWidgetGraphql
? this.state.userPermissions.removeSourceBranch ? this.state.userPermissions.removeSourceBranch
...@@ -96,7 +101,11 @@ export default { ...@@ -96,7 +101,11 @@ export default {
.cancelAutomaticMerge() .cancelAutomaticMerge()
.then((res) => res.data) .then((res) => res.data)
.then((data) => { .then((data) => {
eventHub.$emit('UpdateWidgetData', data); if (this.glFeatures.mergeRequestWidgetGraphql) {
eventHub.$emit('MRWidgetUpdateRequested');
} else {
eventHub.$emit('UpdateWidgetData', data);
}
}) })
.catch(() => { .catch(() => {
this.isCancellingAutoMerge = false; this.isCancellingAutoMerge = false;
...@@ -119,6 +128,11 @@ export default { ...@@ -119,6 +128,11 @@ export default {
eventHub.$emit('MRWidgetUpdateRequested'); eventHub.$emit('MRWidgetUpdateRequested');
} }
}) })
.then(() => {
if (this.glFeatures.mergeRequestWidgetGraphql) {
this.$apollo.queries.state.refetch();
}
})
.catch(() => { .catch(() => {
this.isRemovingSourceBranch = false; this.isRemovingSourceBranch = false;
Flash(__('Something went wrong. Please try again.')); Flash(__('Something went wrong. Please try again.'));
......
...@@ -7,7 +7,7 @@ import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; ...@@ -7,7 +7,7 @@ import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import simplePoll from '../../../lib/utils/simple_poll'; import simplePoll from '../../../lib/utils/simple_poll';
import eventHub from '../../event_hub'; import eventHub from '../../event_hub';
import statusIcon from '../mr_widget_status_icon.vue'; import statusIcon from '../mr_widget_status_icon.vue';
import rebaseQuery from '../../queries/states/ready_to_merge.query.graphql'; import rebaseQuery from '../../queries/states/rebase.query.graphql';
import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables'; import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables';
import { deprecatedCreateFlash as Flash } from '../../../flash'; import { deprecatedCreateFlash as Flash } from '../../../flash';
......
...@@ -53,8 +53,8 @@ export default { ...@@ -53,8 +53,8 @@ export default {
result({ data }) { result({ data }) {
this.state = { this.state = {
...data.project.mergeRequest, ...data.project.mergeRequest,
mergeRequestsFfOnlyEnabled: data.mergeRequestsFfOnlyEnabled, mergeRequestsFfOnlyEnabled: data.project.mergeRequestsFfOnlyEnabled,
onlyAllowMergeIfPipelineSucceeds: data.onlyAllowMergeIfPipelineSucceeds, onlyAllowMergeIfPipelineSucceeds: data.project.onlyAllowMergeIfPipelineSucceeds,
}; };
this.removeSourceBranch = data.project.mergeRequest.shouldRemoveSourceBranch; this.removeSourceBranch = data.project.mergeRequest.shouldRemoveSourceBranch;
this.commitMessage = data.project.mergeRequest.defaultMergeCommitMessage; this.commitMessage = data.project.mergeRequest.defaultMergeCommitMessage;
...@@ -277,7 +277,20 @@ export default { ...@@ -277,7 +277,20 @@ export default {
return this.mr.mergeRequestDiffsPath; return this.mr.mergeRequestDiffsPath;
}, },
}, },
mounted() {
if (this.glFeatures.mergeRequestWidgetGraphql) {
eventHub.$on('ApprovalUpdated', this.updateGraphqlState);
}
},
beforeDestroy() {
if (this.glFeatures.mergeRequestWidgetGraphql) {
eventHub.$off('ApprovalUpdated', this.updateGraphqlState);
}
},
methods: { methods: {
updateGraphqlState() {
return this.$apollo.queries.state.refetch();
},
updateMergeCommitMessage(includeDescription) { updateMergeCommitMessage(includeDescription) {
const commitMessage = this.glFeatures.mergeRequestWidgetGraphql const commitMessage = this.glFeatures.mergeRequestWidgetGraphql
? this.state.defaultMergeCommitMessage ? this.state.defaultMergeCommitMessage
...@@ -326,6 +339,10 @@ export default { ...@@ -326,6 +339,10 @@ export default {
} else if (hasError) { } else if (hasError) {
eventHub.$emit('FailedToMerge', data.merge_error); eventHub.$emit('FailedToMerge', data.merge_error);
} }
if (this.glFeatures.mergeRequestWidgetGraphql) {
this.updateGraphqlState();
}
}) })
.catch(() => { .catch(() => {
this.isMakingRequest = false; this.isMakingRequest = false;
...@@ -532,7 +549,7 @@ export default { ...@@ -532,7 +549,7 @@ export default {
</div> </div>
<merge-train-helper-text <merge-train-helper-text
v-if="shouldRenderMergeTrainHelperText" v-if="shouldRenderMergeTrainHelperText"
:pipeline-id="pipeline.id" :pipeline-id="pipelineId"
:pipeline-link="pipeline.path" :pipeline-link="pipeline.path"
:merge-train-length="stateData.mergeTrainsCount" :merge-train-length="stateData.mergeTrainsCount"
:merge-train-when-pipeline-succeeds-docs-path="mr.mergeTrainWhenPipelineSucceedsDocsPath" :merge-train-when-pipeline-succeeds-docs-path="mr.mergeTrainWhenPipelineSucceedsDocsPath"
......
...@@ -35,5 +35,8 @@ export default { ...@@ -35,5 +35,8 @@ export default {
shouldRenderMergeTrainHelperText() { shouldRenderMergeTrainHelperText() {
return false; return false;
}, },
pipelineId() {
return this.pipeline.id;
},
}, },
}; };
...@@ -94,7 +94,6 @@ export default { ...@@ -94,7 +94,6 @@ export default {
state: { state: {
query: getStateQuery, query: getStateQuery,
manual: true, manual: true,
pollInterval: 10 * 1000,
skip() { skip() {
return !this.mr || !window.gon?.features?.mergeRequestWidgetGraphql; return !this.mr || !window.gon?.features?.mergeRequestWidgetGraphql;
}, },
...@@ -286,6 +285,10 @@ export default { ...@@ -286,6 +285,10 @@ export default {
return new MRWidgetService(this.getServiceEndpoints(store)); return new MRWidgetService(this.getServiceEndpoints(store));
}, },
checkStatus(cb, isRebased) { checkStatus(cb, isRebased) {
if (window.gon?.features?.mergeRequestWidgetGraphql) {
this.$apollo.queries.state.refetch();
}
return this.service return this.service
.checkStatus() .checkStatus()
.then(({ data }) => { .then(({ data }) => {
......
...@@ -18,6 +18,7 @@ query getState($projectPath: ID!, $iid: String!) { ...@@ -18,6 +18,7 @@ query getState($projectPath: ID!, $iid: String!) {
} }
shouldBeRebased shouldBeRebased
sourceBranchExists sourceBranchExists
state
targetBranchExists targetBranchExists
userPermissions { userPermissions {
canMerge canMerge
......
fragment autoMergeEnabled on MergeRequest { fragment autoMergeEnabled on MergeRequest {
autoMergeStrategy autoMergeStrategy
mergeUser { mergeUser {
id
name name
username username
webUrl webUrl
......
...@@ -4,7 +4,6 @@ query autoMergeEnabledQuery($projectPath: ID!, $iid: String!) { ...@@ -4,7 +4,6 @@ query autoMergeEnabledQuery($projectPath: ID!, $iid: String!) {
project(fullPath: $projectPath) { project(fullPath: $projectPath) {
mergeRequest(iid: $iid) { mergeRequest(iid: $iid) {
...autoMergeEnabled ...autoMergeEnabled
mergeTrainsCount
} }
} }
} }
...@@ -156,9 +156,9 @@ export default class MergeRequestStore { ...@@ -156,9 +156,9 @@ export default class MergeRequestStore {
this.setState(); this.setState();
mrEventHub.$emit('mr.state.updated', { if (!window.gon?.features?.mergeRequestWidgetGraphql) {
state: this.mergeRequestState, this.emitUpdatedState();
}); }
} }
setGraphqlData(project) { setGraphqlData(project) {
...@@ -182,7 +182,9 @@ export default class MergeRequestStore { ...@@ -182,7 +182,9 @@ export default class MergeRequestStore {
this.isSHAMismatch = this.sha !== mergeRequest.diffHeadSha; this.isSHAMismatch = this.sha !== mergeRequest.diffHeadSha;
this.shouldBeRebased = mergeRequest.shouldBeRebased; this.shouldBeRebased = mergeRequest.shouldBeRebased;
this.workInProgress = mergeRequest.workInProgress; this.workInProgress = mergeRequest.workInProgress;
this.mergeRequestState = mergeRequest.state;
this.emitUpdatedState();
this.setState(); this.setState();
} }
...@@ -208,6 +210,12 @@ export default class MergeRequestStore { ...@@ -208,6 +210,12 @@ export default class MergeRequestStore {
} }
} }
emitUpdatedState() {
mrEventHub.$emit('mr.state.updated', {
state: this.mergeRequestState,
});
}
setPaths(data) { setPaths(data) {
// Paths are set on the first load of the page and not auto-refreshed // Paths are set on the first load of the page and not auto-refreshed
this.squashBeforeMergeHelpPath = data.squash_before_merge_help_path; this.squashBeforeMergeHelpPath = data.squash_before_merge_help_path;
......
...@@ -2,6 +2,7 @@ import { isNumber, isString } from 'lodash'; ...@@ -2,6 +2,7 @@ import { isNumber, isString } from 'lodash';
import { MTWPS_MERGE_STRATEGY, MT_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants'; import { MTWPS_MERGE_STRATEGY, MT_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants';
import { __ } from '~/locale'; import { __ } from '~/locale';
import base from '~/vue_merge_request_widget/mixins/ready_to_merge'; import base from '~/vue_merge_request_widget/mixins/ready_to_merge';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
export const MERGE_DISABLED_TEXT_UNAPPROVED = __( export const MERGE_DISABLED_TEXT_UNAPPROVED = __(
'You can only merge once this merge request is approved.', 'You can only merge once this merge request is approved.',
...@@ -49,10 +50,17 @@ export default { ...@@ -49,10 +50,17 @@ export default {
} }
return __('Merge when pipeline succeeds'); return __('Merge when pipeline succeeds');
}, },
pipelineId() {
if (this.glFeatures.mergeRequestWidgetGraphql) {
return getIdFromGraphQLId(this.pipeline.id);
}
return this.pipeline.id;
},
shouldRenderMergeTrainHelperText() { shouldRenderMergeTrainHelperText() {
return ( return (
this.pipeline && this.pipeline &&
isNumber(this.pipeline.id) && isNumber(this.pipelineId) &&
isString(this.pipeline.path) && isString(this.pipeline.path) &&
this.preferredAutoMergeStrategy === MTWPS_MERGE_STRATEGY && this.preferredAutoMergeStrategy === MTWPS_MERGE_STRATEGY &&
!this.stateData.autoMergeEnabled !this.stateData.autoMergeEnabled
......
...@@ -4,6 +4,7 @@ query autoMergeEnabledQuery($projectPath: ID!, $iid: String!) { ...@@ -4,6 +4,7 @@ query autoMergeEnabledQuery($projectPath: ID!, $iid: String!) {
project(fullPath: $projectPath) { project(fullPath: $projectPath) {
mergeRequest(iid: $iid) { mergeRequest(iid: $iid) {
...autoMergeEnabled ...autoMergeEnabled
mergeTrainsCount
} }
} }
} }
...@@ -4,7 +4,7 @@ require 'spec_helper' ...@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe 'User approves a merge request', :js do RSpec.describe 'User approves a merge request', :js do
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
let(:project) { create(:project, :repository, approvals_before_merge: 1) } let(:project) { create(:project, :repository, :public, approvals_before_merge: 1) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:user2) { create(:user) } let(:user2) { create(:user) }
let(:user3) { create(:user) } let(:user3) { create(:user) }
...@@ -77,7 +77,8 @@ RSpec.describe 'User approves a merge request', :js do ...@@ -77,7 +77,8 @@ RSpec.describe 'User approves a merge request', :js do
context 'when user cannot approve' do context 'when user cannot approve' do
before do before do
merge_request.approvers.create!(user_id: user2.id) sign_out(user)
sign_in(create(:user))
visit(merge_request_path(merge_request)) visit(merge_request_path(merge_request))
end end
......
...@@ -68,7 +68,7 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js do ...@@ -68,7 +68,7 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js do
wait_for_requests wait_for_requests
expect(page).to have_content 'Merge when pipeline succeeds', wait: 0 expect(page).to have_content 'Merge when pipeline succeeds'
end end
it_behaves_like 'Merge when pipeline succeeds activator' it_behaves_like 'Merge when pipeline succeeds activator'
......
...@@ -202,7 +202,11 @@ describe('MRWidgetAutoMergeEnabled', () => { ...@@ -202,7 +202,11 @@ describe('MRWidgetAutoMergeEnabled', () => {
wrapper.vm.cancelAutomaticMerge(); wrapper.vm.cancelAutomaticMerge();
setImmediate(() => { setImmediate(() => {
expect(wrapper.vm.isCancellingAutoMerge).toBeTruthy(); expect(wrapper.vm.isCancellingAutoMerge).toBeTruthy();
expect(eventHub.$emit).toHaveBeenCalledWith('UpdateWidgetData', mrObj); if (mergeRequestWidgetGraphql) {
expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested');
} else {
expect(eventHub.$emit).toHaveBeenCalledWith('UpdateWidgetData', mrObj);
}
done(); done();
}); });
}); });
......
...@@ -220,7 +220,7 @@ RSpec.configure do |config| ...@@ -220,7 +220,7 @@ RSpec.configure do |config|
# Merge request widget GraphQL requests are disabled in the tests # Merge request widget GraphQL requests are disabled in the tests
# for now whilst we migrate as much as we can over the GraphQL # for now whilst we migrate as much as we can over the GraphQL
stub_feature_flags(merge_request_widget_graphql: false) # stub_feature_flags(merge_request_widget_graphql: false)
# Using FortiAuthenticator as OTP provider is disabled by default in # Using FortiAuthenticator as OTP provider is disabled by default in
# tests, until we introduce it in user settings # tests, until we introduce it in user settings
......
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