Fix MR widget with external CI services/integrations33287-fix-mr-widget-errors-with-external-services

Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/33287

The MR widget was trying to render the pipelines section when
there are no GitLab CI pipelines which was throwing some NPE
errors.

4 files changed, 24 insertions, 9 deletions

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
index 6c2e9ba1d30..c79b5c720eb 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
@@ -12,6 +12,9 @@ export default {
     ciIcon,
   },
   computed: {
+    hasPipeline() {
+      return this.mr.pipeline && Object.keys(this.mr.pipeline).length > 0;
+    },
     hasCIError() {
       const { hasCI, ciStatus } = this.mr;
 
@@ -28,7 +31,9 @@ export default {
     },
   },
   template: `
-    <div class="mr-widget-heading">
+    <div
+      v-if="hasPipeline || hasCIError"
+      class="mr-widget-heading">
       <div class="ci-widget media">
         <template v-if="hasCIError">
           <div class="ci-status-icon ci-status-icon-failed ci-error js-ci-error append-right-10">
@@ -40,7 +45,7 @@ export default {
             Could not connect to the CI server. Please check your settings and try again
           </div>
         </template>
-        <template v-else>
+        <template v-else-if="hasPipeline">
           <div class="ci-status-icon append-right-10">
             <a
               class="icon-link"
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js
index 1fb7150d20e..557b89b2b84 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js
@@ -29,6 +29,9 @@ export default {
     statusIcon,
   },
   computed: {
+    shouldShowMergeWhenPipelineSucceedsText() {
+      return this.mr.isPipelineActive;
+    },
     commitMessageLinkTitle() {
       const withDesc = 'Include description in commit message';
       const withoutDesc = "Don't include description in commit message";
@@ -56,7 +59,7 @@ export default {
     mergeButtonText() {
       if (this.isMergingImmediately) {
         return 'Merge in progress';
-      } else if (this.mr.isPipelineActive) {
+      } else if (this.shouldShowMergeWhenPipelineSucceedsText) {
         return 'Merge when pipeline succeeds';
       }
 
@@ -68,7 +71,7 @@ export default {
     isMergeButtonDisabled() {
       const { commitMessage } = this;
       return Boolean(!commitMessage.length
-        || !this.isMergeAllowed()
+        || !this.shouldShowMergeControls()
         || this.isMakingRequest
         || this.mr.preventMerge);
     },
@@ -82,7 +85,12 @@ export default {
   },
   methods: {
     isMergeAllowed() {
-      return !(this.mr.onlyAllowMergeIfPipelineSucceeds && this.mr.isPipelineFailed);
+      return !this.mr.onlyAllowMergeIfPipelineSucceeds ||
+        this.mr.isPipelinePassing ||
+        this.mr.isPipelineSkipped;
+    },
+    shouldShowMergeControls() {
+      return this.isMergeAllowed() || this.shouldShowMergeWhenPipelineSucceedsText;
     },
     updateCommitMessage() {
       const cmwd = this.mr.commitMessageWithDescription;
@@ -261,7 +269,7 @@ export default {
             </ul>
           </span>
           <div class="media-body-wrap space-children">
-            <template v-if="isMergeAllowed()">
+            <template v-if="shouldShowMergeControls()">
               <label>
                 <input
                   id="remove-source-branch-input"
@@ -286,7 +294,7 @@ export default {
             </template>
             <template v-else>
               <span class="bold">
-                The pipeline for this merge request failed. Please retry the job or push a new commit to fix the failure
+                The pipeline for this merge request has not succeeded yet
               </span>
             </template>
           </div>
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
index 0042c48816f..2f237262028 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
@@ -57,7 +57,7 @@ export default {
       return stateMaps.statesToShowHelpWidget.indexOf(this.mr.state) > -1;
     },
     shouldRenderPipelines() {
-      return Object.keys(this.mr.pipeline).length || this.mr.hasCI;
+      return this.mr.hasCI;
     },
     shouldRenderRelatedLinks() {
       return this.mr.relatedLinks;
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index fbea764b739..29464662578 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -85,7 +85,9 @@ export default class MergeRequestStore {
     this.ciEnvironmentsStatusPath = data.ci_environments_status_path;
     this.hasCI = data.has_ci;
     this.ciStatus = data.ci_status;
-    this.isPipelineFailed = this.ciStatus ? (this.ciStatus === 'failed' || this.ciStatus === 'canceled') : false;
+    this.isPipelineFailed = this.ciStatus === 'failed' || this.ciStatus === 'canceled';
+    this.isPipelinePassing = this.ciStatus === 'success' || this.ciStatus === 'success_with_warnings';
+    this.isPipelineSkipped = this.ciStatus === 'skipped';
     this.pipelineDetailedStatus = pipelineStatus;
     this.isPipelineActive = data.pipeline ? data.pipeline.active : false;
     this.isPipelineBlocked = pipelineStatus ? pipelineStatus.group === 'manual' : false;