Commit 1d96b8fa authored by Frederic Caplette's avatar Frederic Caplette

Handle conflict no CI and pipelines must succeed

When there are no CI services found and
the pipeline must succeed option is turned
on, this is considered an invalid usecase
since the pipeline will never be successful.
Because of this, we add a meaningful message
to the UI that explain this situation and link
to our documentation while in the Merge request.
parent 4ee562ce
...@@ -42,6 +42,10 @@ export default { ...@@ -42,6 +42,10 @@ export default {
type: String, type: String,
required: false, required: false,
}, },
pipelineMustSucceed: {
type: Boolean,
required: false,
},
sourceBranchLink: { sourceBranchLink: {
type: String, type: String,
required: false, required: false,
...@@ -60,7 +64,10 @@ export default { ...@@ -60,7 +64,10 @@ export default {
return this.pipeline && Object.keys(this.pipeline).length > 0; return this.pipeline && Object.keys(this.pipeline).length > 0;
}, },
hasCIError() { hasCIError() {
return this.hasCi && !this.ciStatus; return (this.hasCi && !this.ciStatus) || this.hasPipelineMustSucceedConflict;
},
hasPipelineMustSucceedConflict() {
return !this.hasCi && Boolean(this.pipelineMustSucceed);
}, },
status() { status() {
return this.pipeline.details && this.pipeline.details.status return this.pipeline.details && this.pipeline.details.status
...@@ -76,9 +83,13 @@ export default { ...@@ -76,9 +83,13 @@ export default {
return this.pipeline.commit && Object.keys(this.pipeline.commit).length > 0; return this.pipeline.commit && Object.keys(this.pipeline.commit).length > 0;
}, },
errorText() { errorText() {
if (this.hasPipelineMustSucceedConflict) {
return s__('Pipeline|No pipeline has been run for this commit.');
}
return sprintf( return sprintf(
s__( s__(
'Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}', 'Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation%{linkEnd}.',
), ),
{ {
linkStart: `<a href="${this.troubleshootingDocsPath}">`, linkStart: `<a href="${this.troubleshootingDocsPath}">`,
......
...@@ -79,6 +79,7 @@ export default { ...@@ -79,6 +79,7 @@ export default {
:pipeline-coverage-delta="mr.pipelineCoverageDelta" :pipeline-coverage-delta="mr.pipelineCoverageDelta"
:ci-status="mr.ciStatus" :ci-status="mr.ciStatus"
:has-ci="mr.hasCI" :has-ci="mr.hasCI"
:pipeline-must-succeed="mr.onlyAllowMergeIfPipelineSucceeds"
:source-branch="branch" :source-branch="branch"
:source-branch-link="branchLink" :source-branch-link="branchLink"
:troubleshooting-docs-path="mr.troubleshootingDocsPath" :troubleshooting-docs-path="mr.troubleshootingDocsPath"
......
<script> <script>
import { isEmpty } from 'lodash'; import { isEmpty } from 'lodash';
import { GlIcon, GlDeprecatedButton } from '@gitlab/ui'; import { GlIcon, GlDeprecatedButton, GlSprintf, GlLink } from '@gitlab/ui';
import successSvg from 'icons/_icon_status_success.svg'; import successSvg from 'icons/_icon_status_success.svg';
import warningSvg from 'icons/_icon_status_warning.svg'; import warningSvg from 'icons/_icon_status_warning.svg';
import readyToMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/ready_to_merge'; import readyToMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/ready_to_merge';
...@@ -26,6 +26,8 @@ export default { ...@@ -26,6 +26,8 @@ export default {
CommitEdit, CommitEdit,
CommitMessageDropdown, CommitMessageDropdown,
GlIcon, GlIcon,
GlSprintf,
GlLink,
GlDeprecatedButton, GlDeprecatedButton,
MergeImmediatelyConfirmationDialog: () => MergeImmediatelyConfirmationDialog: () =>
import( import(
...@@ -56,7 +58,7 @@ export default { ...@@ -56,7 +58,7 @@ export default {
status() { status() {
const { pipeline, isPipelineFailed, hasCI, ciStatus } = this.mr; const { pipeline, isPipelineFailed, hasCI, ciStatus } = this.mr;
if (hasCI && !ciStatus) { if ((hasCI && !ciStatus) || this.hasPipelineMustSucceedConflict) {
return 'failed'; return 'failed';
} else if (this.isAutoMergeAvailable) { } else if (this.isAutoMergeAvailable) {
return 'pending'; return 'pending';
...@@ -97,6 +99,9 @@ export default { ...@@ -97,6 +99,9 @@ export default {
return __('Merge'); return __('Merge');
}, },
hasPipelineMustSucceedConflict() {
return !this.mr.hasCI && this.mr.onlyAllowMergeIfPipelineSucceeds;
},
isRemoveSourceBranchButtonDisabled() { isRemoveSourceBranchButtonDisabled() {
return this.isMergeButtonDisabled; return this.isMergeButtonDisabled;
}, },
...@@ -321,7 +326,10 @@ export default { ...@@ -321,7 +326,10 @@ export default {
</li> </li>
</ul> </ul>
</span> </span>
<div class="media-body-wrap space-children"> <div
class="media-body-wrap"
:class="{ 'space-children': !hasPipelineMustSucceedConflict }"
>
<template v-if="shouldShowMergeControls"> <template v-if="shouldShowMergeControls">
<label v-if="mr.canRemoveSourceBranch"> <label v-if="mr.canRemoveSourceBranch">
<input <input
...@@ -343,9 +351,19 @@ export default { ...@@ -343,9 +351,19 @@ export default {
/> />
</template> </template>
<template v-else> <template v-else>
<span class="bold js-resolve-mr-widget-items-message"> <div class="bold js-resolve-mr-widget-items-message">
{{ mergeDisabledText }} <gl-sprintf
</span> v-if="hasPipelineMustSucceedConflict"
:message="pipelineMustSucceedConflictText"
>
<template #link="{ content }">
<gl-link :href="mr.pipelineMustSucceedDocsPath" target="_blank">
{{ content }}
</gl-link>
</template>
</gl-sprintf>
<gl-sprintf v-else :message="mergeDisabledText" />
</div>
</template> </template>
</div> </div>
</div> </div>
......
import { __ } from '~/locale'; import { __ } from '~/locale';
export const MERGE_DISABLED_TEXT = __('You can only merge once the items above are resolved.'); export const MERGE_DISABLED_TEXT = __('You can only merge once the items above are resolved.');
export const PIPELINE_MUST_SUCCEED_CONFLICT_TEXT = __(
'Pipelines must succeed for merge requests to be eligible to merge. Please enable pipelines for this project to continue. For more information, see the %{linkStart}documentation.%{linkEnd}',
);
export default { export default {
computed: { computed: {
...@@ -16,6 +19,9 @@ export default { ...@@ -16,6 +19,9 @@ export default {
mergeDisabledText() { mergeDisabledText() {
return MERGE_DISABLED_TEXT; return MERGE_DISABLED_TEXT;
}, },
pipelineMustSucceedConflictText() {
return PIPELINE_MUST_SUCCEED_CONFLICT_TEXT;
},
autoMergeText() { autoMergeText() {
// MWPS is currently the only auto merge strategy available in CE // MWPS is currently the only auto merge strategy available in CE
return __('Merge when pipeline succeeds'); return __('Merge when pipeline succeeds');
......
...@@ -104,8 +104,11 @@ export default { ...@@ -104,8 +104,11 @@ export default {
shouldRenderMergeHelp() { shouldRenderMergeHelp() {
return stateMaps.statesToShowHelpWidget.indexOf(this.mr.state) > -1; return stateMaps.statesToShowHelpWidget.indexOf(this.mr.state) > -1;
}, },
hasPipelineMustSucceedConflict() {
return !this.mr.hasCI && this.mr.onlyAllowMergeIfPipelineSucceeds;
},
shouldRenderPipelines() { shouldRenderPipelines() {
return this.mr.hasCI; return this.mr.hasCI || this.hasPipelineMustSucceedConflict;
}, },
shouldSuggestPipelines() { shouldSuggestPipelines() {
return gon.features?.suggestPipeline && !this.mr.hasCI && this.mr.mergeRequestAddCiConfigPath; return gon.features?.suggestPipeline && !this.mr.hasCI && this.mr.mergeRequestAddCiConfigPath;
...@@ -432,7 +435,9 @@ export default { ...@@ -432,7 +435,9 @@ export default {
<source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" /> <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
</div> </div>
</div> </div>
<div v-if="shouldRenderMergeHelp" class="mr-widget-footer"><mr-widget-merge-help /></div> <div v-if="shouldRenderMergeHelp" class="mr-widget-footer">
<mr-widget-merge-help />
</div>
</div> </div>
<mr-widget-pipeline-container <mr-widget-pipeline-container
v-if="shouldRenderMergedPipeline" v-if="shouldRenderMergedPipeline"
......
...@@ -161,6 +161,7 @@ export default class MergeRequestStore { ...@@ -161,6 +161,7 @@ export default class MergeRequestStore {
// 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;
this.troubleshootingDocsPath = data.troubleshooting_docs_path; this.troubleshootingDocsPath = data.troubleshooting_docs_path;
this.pipelineMustSucceedDocsPath = data.pipeline_must_succeed_docs_path;
this.mergeRequestBasicPath = data.merge_request_basic_path; this.mergeRequestBasicPath = data.merge_request_basic_path;
this.mergeRequestWidgetPath = data.merge_request_widget_path; this.mergeRequestWidgetPath = data.merge_request_widget_path;
this.mergeRequestCachedWidgetPath = data.merge_request_cached_widget_path; this.mergeRequestCachedWidgetPath = data.merge_request_cached_widget_path;
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}'; window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: 'troubleshooting')}'; window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: 'troubleshooting')}';
window.gl.mrWidgetData.pipeline_must_succeed_docs_path = '#{help_page_path('user/project/merge_requests/merge_when_pipeline_succeeds.md', anchor: 'only-allow-merge-requests-to-be-merged-if-the-pipeline-succeeds')}';
window.gl.mrWidgetData.security_approvals_help_page_path = '#{help_page_path('user/application_security/index.html', anchor: 'security-approvals-in-merge-requests-ultimate')}'; window.gl.mrWidgetData.security_approvals_help_page_path = '#{help_page_path('user/application_security/index.html', anchor: 'security-approvals-in-merge-requests-ultimate')}';
window.gl.mrWidgetData.eligible_approvers_docs_path = '#{help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers')}'; window.gl.mrWidgetData.eligible_approvers_docs_path = '#{help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers')}';
window.gl.mrWidgetData.pipelines_empty_svg_path = '#{image_path('illustrations/pipelines_empty.svg')}'; window.gl.mrWidgetData.pipelines_empty_svg_path = '#{image_path('illustrations/pipelines_empty.svg')}';
......
...@@ -5,6 +5,9 @@ import base from '~/vue_merge_request_widget/mixins/ready_to_merge'; ...@@ -5,6 +5,9 @@ import base from '~/vue_merge_request_widget/mixins/ready_to_merge';
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.',
); );
export const PIPELINE_MUST_SUCCEED_CONFLICT_TEXT = __(
'Pipelines must succeed for merge requests to be eligible to merge. Please enable pipelines for this project to continue. For more information, see the %{linkStart}documentation.%{linkEnd}',
);
export default { export default {
computed: { computed: {
...@@ -28,6 +31,9 @@ export default { ...@@ -28,6 +31,9 @@ export default {
return base.computed.mergeDisabledText.call(this); return base.computed.mergeDisabledText.call(this);
}, },
pipelineMustSucceedConflictText() {
return PIPELINE_MUST_SUCCEED_CONFLICT_TEXT;
},
autoMergeText() { autoMergeText() {
if (this.mr.preferredAutoMergeStrategy === MTWPS_MERGE_STRATEGY) { if (this.mr.preferredAutoMergeStrategy === MTWPS_MERGE_STRATEGY) {
if (this.mr.mergeTrainsCount === 0) { if (this.mr.mergeTrainsCount === 0) {
......
...@@ -7,7 +7,10 @@ import { ...@@ -7,7 +7,10 @@ import {
MT_MERGE_STRATEGY, MT_MERGE_STRATEGY,
MTWPS_MERGE_STRATEGY, MTWPS_MERGE_STRATEGY,
} from '~/vue_merge_request_widget/constants'; } from '~/vue_merge_request_widget/constants';
import { MERGE_DISABLED_TEXT } from '~/vue_merge_request_widget/mixins/ready_to_merge'; import {
MERGE_DISABLED_TEXT,
PIPELINE_MUST_SUCCEED_CONFLICT_TEXT,
} from '~/vue_merge_request_widget/mixins/ready_to_merge';
describe('ReadyToMerge', () => { describe('ReadyToMerge', () => {
let wrapper; let wrapper;
...@@ -247,7 +250,10 @@ describe('ReadyToMerge', () => { ...@@ -247,7 +250,10 @@ describe('ReadyToMerge', () => {
}); });
it('should not ask for confirmation in non-merge train scenarios', () => { it('should not ask for confirmation in non-merge train scenarios', () => {
factory({ isPipelineActive: true, onlyAllowMergeIfPipelineSucceeds: false }); factory({
isPipelineActive: true,
onlyAllowMergeIfPipelineSucceeds: false,
});
return clickMergeImmediately().then(() => { return clickMergeImmediately().then(() => {
expect(dialog.vm.show).not.toHaveBeenCalled(); expect(dialog.vm.show).not.toHaveBeenCalled();
expect(vm.handleMergeButtonClick).toHaveBeenCalled(); expect(vm.handleMergeButtonClick).toHaveBeenCalled();
...@@ -258,11 +264,14 @@ describe('ReadyToMerge', () => { ...@@ -258,11 +264,14 @@ describe('ReadyToMerge', () => {
describe('cannot merge', () => { describe('cannot merge', () => {
describe('when isMergeAllowed=false', () => { describe('when isMergeAllowed=false', () => {
beforeEach(() => { beforeEach(() => {
factory({ isMergeAllowed: false, availableAutoMergeStrategies: [] }); factory({
isMergeAllowed: false,
availableAutoMergeStrategies: [],
});
}); });
it('should show cannot merge text', () => { it('should show cannot merge text', () => {
expect(findResolveItemsMessage().text()).toEqual(MERGE_DISABLED_TEXT); expect(findResolveItemsMessage().html()).toContain(MERGE_DISABLED_TEXT);
}); });
it('should show disabled merge button', () => { it('should show disabled merge button', () => {
...@@ -285,7 +294,22 @@ describe('ReadyToMerge', () => { ...@@ -285,7 +294,22 @@ describe('ReadyToMerge', () => {
}); });
it('should show approvals needed text', () => { it('should show approvals needed text', () => {
expect(findResolveItemsMessage().text()).toEqual(MERGE_DISABLED_TEXT_UNAPPROVED); expect(findResolveItemsMessage().html()).toContain(MERGE_DISABLED_TEXT_UNAPPROVED);
});
});
describe('when no CI service are found and enforce `Pipeline must succeed`', () => {
beforeEach(() => {
factory({
isMergeAllowed: false,
availableAutoMergeStrategies: [],
hasCI: false,
onlyAllowMergeIfPipelineSucceeds: true,
});
});
it('should show a custom message that explains the conflict', () => {
expect(findResolveItemsMessage().html()).toContain(PIPELINE_MUST_SUCCEED_CONFLICT_TEXT);
}); });
}); });
}); });
...@@ -15009,6 +15009,9 @@ msgstr "" ...@@ -15009,6 +15009,9 @@ msgstr ""
msgid "Pipelines for merge requests are configured. A detached pipeline runs in the context of the merge request, and not against the merged result. Learn more in the documentation for Pipelines for Merged Results." msgid "Pipelines for merge requests are configured. A detached pipeline runs in the context of the merge request, and not against the merged result. Learn more in the documentation for Pipelines for Merged Results."
msgstr "" msgstr ""
msgid "Pipelines must succeed for merge requests to be eligible to merge. Please enable pipelines for this project to continue. For more information, see the %{linkStart}documentation.%{linkEnd}"
msgstr ""
msgid "Pipelines settings for '%{project_name}' were successfully updated." msgid "Pipelines settings for '%{project_name}' were successfully updated."
msgstr "" msgstr ""
...@@ -15072,7 +15075,7 @@ msgstr "" ...@@ -15072,7 +15075,7 @@ msgstr ""
msgid "Pipeline|Commit" msgid "Pipeline|Commit"
msgstr "" msgstr ""
msgid "Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation.%{linkEnd}" msgid "Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation%{linkEnd}."
msgstr "" msgstr ""
msgid "Pipeline|Coverage" msgid "Pipeline|Coverage"
...@@ -15099,6 +15102,9 @@ msgstr "" ...@@ -15099,6 +15102,9 @@ msgstr ""
msgid "Pipeline|Merged result pipeline" msgid "Pipeline|Merged result pipeline"
msgstr "" msgstr ""
msgid "Pipeline|No pipeline has been run for this commit."
msgstr ""
msgid "Pipeline|Pipeline" msgid "Pipeline|Pipeline"
msgstr "" msgstr ""
......
...@@ -122,6 +122,19 @@ describe('MRWidgetPipeline', () => { ...@@ -122,6 +122,19 @@ describe('MRWidgetPipeline', () => {
); );
}); });
it('should render CI error when no CI is provided and pipeline must succeed is turned on', () => {
vm = mountComponent(Component, {
pipeline: {},
hasCi: false,
pipelineMustSucceed: true,
troubleshootingDocsPath: 'help',
});
expect(vm.$el.querySelector('.media-body').textContent.trim()).toContain(
'No pipeline has been run for this commit.',
);
});
describe('with a pipeline', () => { describe('with a pipeline', () => {
beforeEach(() => { beforeEach(() => {
vm = mountComponent(Component, { vm = mountComponent(Component, {
......
...@@ -18,6 +18,7 @@ const createTestMr = customConfig => { ...@@ -18,6 +18,7 @@ const createTestMr = customConfig => {
isPipelineFailed: false, isPipelineFailed: false,
isPipelinePassing: false, isPipelinePassing: false,
isMergeAllowed: true, isMergeAllowed: true,
isApproved: true,
onlyAllowMergeIfPipelineSucceeds: false, onlyAllowMergeIfPipelineSucceeds: false,
ffOnlyEnabled: false, ffOnlyEnabled: false,
hasCI: false, hasCI: false,
......
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