Merge branch...

Merge branch '216031-show-accurate-error-message-when-pipelines-disabled-mr-must-succeed-option' into 'master'

Show error message when pipelines disabled and Pipeline must succeed

See merge request gitlab-org/gitlab!31112

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue

@@ -42,6 +42,10 @@ export default {
       type: String,
       required: false,
     },
+    pipelineMustSucceed: {
+      type: Boolean,
+      required: false,
+    },
     sourceBranchLink: {
       type: String,
       required: false,
@@ -60,7 +64,10 @@ export default {
       return this.pipeline && Object.keys(this.pipeline).length > 0;
     },
     hasCIError() {
-      return this.hasCi && !this.ciStatus;
+      return (this.hasCi && !this.ciStatus) || this.hasPipelineMustSucceedConflict;
+    },
+    hasPipelineMustSucceedConflict() {
+      return !this.hasCi && this.pipelineMustSucceed;
     },
     status() {
       return this.pipeline.details && this.pipeline.details.status
@@ -76,9 +83,13 @@ export default {
       return this.pipeline.commit && Object.keys(this.pipeline.commit).length > 0;
     },
     errorText() {
+      if (this.hasPipelineMustSucceedConflict) {
+        return s__('Pipeline|No pipeline has been run for this commit.');
+      }
+
       return sprintf(
         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}">`,

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline_container.vue

@@ -79,6 +79,7 @@ export default {
       :pipeline-coverage-delta="mr.pipelineCoverageDelta"
       :ci-status="mr.ciStatus"
       :has-ci="mr.hasCI"
+      :pipeline-must-succeed="mr.onlyAllowMergeIfPipelineSucceeds"
       :source-branch="branch"
       :source-branch-link="branchLink"
       :troubleshooting-docs-path="mr.troubleshootingDocsPath"

app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue

 <script>
 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 warningSvg from 'icons/_icon_status_warning.svg';
 import readyToMergeMixin from 'ee_else_ce/vue_merge_request_widget/mixins/ready_to_merge';
@@ -26,6 +26,8 @@ export default {
     CommitEdit,
     CommitMessageDropdown,
     GlIcon,
+    GlSprintf,
+    GlLink,
     GlDeprecatedButton,
     MergeImmediatelyConfirmationDialog: () =>
       import(
@@ -56,7 +58,7 @@ export default {
     status() {
       const { pipeline, isPipelineFailed, hasCI, ciStatus } = this.mr;
 
-      if (hasCI && !ciStatus) {
+      if ((hasCI && !ciStatus) || this.hasPipelineMustSucceedConflict) {
         return 'failed';
       } else if (this.isAutoMergeAvailable) {
         return 'pending';
@@ -97,6 +99,9 @@ export default {
 
       return __('Merge');
     },
+    hasPipelineMustSucceedConflict() {
+      return !this.mr.hasCI && this.mr.onlyAllowMergeIfPipelineSucceeds;
+    },
     isRemoveSourceBranchButtonDisabled() {
       return this.isMergeButtonDisabled;
     },
@@ -343,9 +348,19 @@ export default {
               />
             </template>
             <template v-else>
-              <span class="bold js-resolve-mr-widget-items-message">
-                {{ mergeDisabledText }}
-              </span>
+              <div class="bold js-resolve-mr-widget-items-message">
+                <gl-sprintf
+                  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>
           </div>
         </div>

app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js

 import { __ } from '~/locale';
 
 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 {
   computed: {
@@ -16,6 +19,9 @@ export default {
     mergeDisabledText() {
       return MERGE_DISABLED_TEXT;
     },
+    pipelineMustSucceedConflictText() {
+      return PIPELINE_MUST_SUCCEED_CONFLICT_TEXT;
+    },
     autoMergeText() {
       // MWPS is currently the only auto merge strategy available in CE
       return __('Merge when pipeline succeeds');

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -104,8 +104,11 @@ export default {
     shouldRenderMergeHelp() {
       return stateMaps.statesToShowHelpWidget.indexOf(this.mr.state) > -1;
     },
+    hasPipelineMustSucceedConflict() {
+      return !this.mr.hasCI && this.mr.onlyAllowMergeIfPipelineSucceeds;
+    },
     shouldRenderPipelines() {
-      return this.mr.hasCI;
+      return this.mr.hasCI || this.hasPipelineMustSucceedConflict;
     },
     shouldSuggestPipelines() {
       return gon.features?.suggestPipeline && !this.mr.hasCI && this.mr.mergeRequestAddCiConfigPath;
@@ -432,7 +435,9 @@ export default {
           <source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
         </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>
     <mr-widget-pipeline-container
       v-if="shouldRenderMergedPipeline"

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -161,6 +161,7 @@ export default class MergeRequestStore {
     // Paths are set on the first load of the page and not auto-refreshed
     this.squashBeforeMergeHelpPath = data.squash_before_merge_help_path;
     this.troubleshootingDocsPath = data.troubleshooting_docs_path;
+    this.pipelineMustSucceedDocsPath = data.pipeline_must_succeed_docs_path;
     this.mergeRequestBasicPath = data.merge_request_basic_path;
     this.mergeRequestWidgetPath = data.merge_request_widget_path;
     this.mergeRequestCachedWidgetPath = data.merge_request_cached_widget_path;

app/views/projects/merge_requests/_widget.html.haml

@@ -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.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.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')}';

changelogs/unreleased/216031-show-accurate-error-message-when-pipelines-disabled-mr-must-succee.yml

+---
+title:
+  Add clear explanation to the MR widget when no CI is available and Pipeline
+  must succeed option is activated
+merge_request: 31112
+author:
+type: changed

ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js

@@ -5,6 +5,9 @@ import base from '~/vue_merge_request_widget/mixins/ready_to_merge';
 export const MERGE_DISABLED_TEXT_UNAPPROVED = __(
   '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 {
   computed: {
@@ -28,6 +31,9 @@ export default {
 
       return base.computed.mergeDisabledText.call(this);
     },
+    pipelineMustSucceedConflictText() {
+      return PIPELINE_MUST_SUCCEED_CONFLICT_TEXT;
+    },
     autoMergeText() {
       if (this.mr.preferredAutoMergeStrategy === MTWPS_MERGE_STRATEGY) {
         if (this.mr.mergeTrainsCount === 0) {

ee/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

@@ -7,7 +7,11 @@ import {
   MT_MERGE_STRATEGY,
   MTWPS_MERGE_STRATEGY,
 } 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';
+import { GlSprintf } from '@gitlab/ui';
 
 describe('ReadyToMerge', () => {
   let wrapper;
@@ -55,7 +59,7 @@ describe('ReadyToMerge', () => {
     ({ vm } = wrapper);
   };
 
-  const findResolveItemsMessage = () => wrapper.find('.js-resolve-mr-widget-items-message');
+  const findResolveItemsMessage = () => wrapper.find(GlSprintf);
   const findMergeButton = () => wrapper.find('.qa-merge-button');
   const findMergeButtonDropdown = () => wrapper.find('.js-merge-moment');
   const findMergeImmediatelyButton = () => wrapper.find('.js-merge-immediately-button');
@@ -247,7 +251,10 @@ describe('ReadyToMerge', () => {
     });
 
     it('should not ask for confirmation in non-merge train scenarios', () => {
-      factory({ isPipelineActive: true, onlyAllowMergeIfPipelineSucceeds: false });
+      factory({
+        isPipelineActive: true,
+        onlyAllowMergeIfPipelineSucceeds: false,
+      });
       return clickMergeImmediately().then(() => {
         expect(dialog.vm.show).not.toHaveBeenCalled();
         expect(vm.handleMergeButtonClick).toHaveBeenCalled();
@@ -258,11 +265,14 @@ describe('ReadyToMerge', () => {
   describe('cannot merge', () => {
     describe('when isMergeAllowed=false', () => {
       beforeEach(() => {
-        factory({ isMergeAllowed: false, availableAutoMergeStrategies: [] });
+        factory({
+          isMergeAllowed: false,
+          availableAutoMergeStrategies: [],
+        });
       });
 
       it('should show cannot merge text', () => {
-        expect(findResolveItemsMessage().text()).toEqual(MERGE_DISABLED_TEXT);
+        expect(findResolveItemsMessage().attributes('message')).toBe(MERGE_DISABLED_TEXT);
       });
 
       it('should show disabled merge button', () => {
@@ -285,7 +295,24 @@ describe('ReadyToMerge', () => {
     });
 
     it('should show approvals needed text', () => {
-      expect(findResolveItemsMessage().text()).toEqual(MERGE_DISABLED_TEXT_UNAPPROVED);
+      expect(findResolveItemsMessage().attributes('message')).toBe(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().attributes('message')).toBe(
+        PIPELINE_MUST_SUCCEED_CONFLICT_TEXT,
+      );
     });
   });
 });

locale/gitlab.pot

@@ -15111,6 +15111,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."
 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."
 msgstr ""
 
@@ -15174,7 +15177,7 @@ msgstr ""
 msgid "Pipeline|Commit"
 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 ""
 
 msgid "Pipeline|Coverage"
@@ -15201,6 +15204,9 @@ msgstr ""
 msgid "Pipeline|Merged result pipeline"
 msgstr ""
 
+msgid "Pipeline|No pipeline has been run for this commit."
+msgstr ""
+
 msgid "Pipeline|Pipeline"
 msgstr ""
 

spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js

@@ -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', () => {
       beforeEach(() => {
         vm = mountComponent(Component, {

spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

@@ -18,6 +18,7 @@ const createTestMr = customConfig => {
     isPipelineFailed: false,
     isPipelinePassing: false,
     isMergeAllowed: true,
+    isApproved: true,
     onlyAllowMergeIfPipelineSucceeds: false,
     ffOnlyEnabled: false,
     hasCI: false,