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.

5 files changed, 128 insertions, 16 deletions

diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb
index fd991293ee9..443b596b3c6 100644
--- a/spec/features/merge_requests/widget_spec.rb
+++ b/spec/features/merge_requests/widget_spec.rb
@@ -142,6 +142,24 @@ describe 'Merge request', :js do
     end
   end
 
+  context 'view merge request where project has CI setup but no CI status' do
+    before do
+      pipeline = create(:ci_pipeline, project: project,
+                                      sha: merge_request.diff_head_sha,
+                                      ref: merge_request.source_branch)
+      create(:ci_build, pipeline: pipeline)
+
+      visit project_merge_request_path(project, merge_request)
+    end
+
+    it 'has pipeline error text' do
+      # Wait for the `ci_status` and `merge_check` requests
+      wait_for_requests
+
+      expect(page).to have_text('Could not connect to the CI server. Please check your settings and try again')
+    end
+  end
+
   context 'view merge request with MWPS enabled but automatically merge fails' do
     before do
       merge_request.update(
diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js
index c763487d12f..690665ae12c 100644
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js
@@ -37,6 +37,26 @@ describe('MRWidgetPipeline', () => {
       });
     });
 
+    describe('hasPipeline', () => {
+      it('should return true when there is a pipeline', () => {
+        expect(Object.keys(mockData.pipeline).length).toBeGreaterThan(0);
+
+        const vm = createComponent({
+          pipeline: mockData.pipeline,
+        });
+
+        expect(vm.hasPipeline).toBeTruthy();
+      });
+
+      it('should return false when there is no pipeline', () => {
+        const vm = createComponent({
+          pipeline: null,
+        });
+
+        expect(vm.hasPipeline).toBeFalsy();
+      });
+    });
+
     describe('hasCIError', () => {
       it('should return false when there is no CI error', () => {
         const vm = createComponent({
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
index c607c9746a4..8f69458ffb0 100644
--- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
@@ -11,6 +11,7 @@ const createComponent = (customConfig = {}) => {
     isPipelineActive: false,
     pipeline: null,
     isPipelineFailed: false,
+    isPipelinePassing: false,
     onlyAllowMergeIfPipelineSucceeds: false,
     hasCI: false,
     ciStatus: null,
@@ -68,6 +69,18 @@ describe('MRWidgetReadyToMerge', () => {
   });
 
   describe('computed', () => {
+    describe('shouldShowMergeWhenPipelineSucceedsText', () => {
+      it('should return true with active pipeline', () => {
+        vm.mr.isPipelineActive = true;
+        expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeTruthy();
+      });
+
+      it('should return false with inactive pipeline', () => {
+        vm.mr.isPipelineActive = false;
+        expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeFalsy();
+      });
+    });
+
     describe('commitMessageLinkTitle', () => {
       const withDesc = 'Include description in commit message';
       const withoutDesc = "Don't include description in commit message";
@@ -203,20 +216,55 @@ describe('MRWidgetReadyToMerge', () => {
 
   describe('methods', () => {
     describe('isMergeAllowed', () => {
-      it('should return false with initial data', () => {
+      it('should return true when no pipeline and not required to succeed', () => {
+        vm.mr.onlyAllowMergeIfPipelineSucceeds = false;
+        vm.mr.isPipelinePassing = false;
         expect(vm.isMergeAllowed()).toBeTruthy();
       });
 
-      it('should return false when MR is set only merge when pipeline succeeds', () => {
-        vm.mr.onlyAllowMergeIfPipelineSucceeds = true;
+      it('should return true when pipeline failed and not required to succeed', () => {
+        vm.mr.onlyAllowMergeIfPipelineSucceeds = false;
+        vm.mr.isPipelinePassing = false;
         expect(vm.isMergeAllowed()).toBeTruthy();
       });
 
-      it('should return true true', () => {
+      it('should return false when pipeline failed and required to succeed', () => {
         vm.mr.onlyAllowMergeIfPipelineSucceeds = true;
-        vm.mr.isPipelineFailed = true;
+        vm.mr.isPipelinePassing = false;
         expect(vm.isMergeAllowed()).toBeFalsy();
       });
+
+      it('should return true when pipeline succeeded and required to succeed', () => {
+        vm.mr.onlyAllowMergeIfPipelineSucceeds = true;
+        vm.mr.isPipelinePassing = true;
+        expect(vm.isMergeAllowed()).toBeTruthy();
+      });
+    });
+
+    describe('shouldShowMergeControls', () => {
+      it('should return false when an external pipeline is running and required to succeed', () => {
+        spyOn(vm, 'isMergeAllowed').and.returnValue(false);
+        vm.mr.isPipelineActive = false;
+        expect(vm.shouldShowMergeControls()).toBeFalsy();
+      });
+
+      it('should return true when the build succeeded or build not required to succeed', () => {
+        spyOn(vm, 'isMergeAllowed').and.returnValue(true);
+        vm.mr.isPipelineActive = false;
+        expect(vm.shouldShowMergeControls()).toBeTruthy();
+      });
+
+      it('should return true when showing the MWPS button and a pipeline is running that needs to be successful', () => {
+        spyOn(vm, 'isMergeAllowed').and.returnValue(false);
+        vm.mr.isPipelineActive = true;
+        expect(vm.shouldShowMergeControls()).toBeTruthy();
+      });
+
+      it('should return true when showing the MWPS button but not required for the pipeline to succeed', () => {
+        spyOn(vm, 'isMergeAllowed').and.returnValue(true);
+        vm.mr.isPipelineActive = true;
+        expect(vm.shouldShowMergeControls()).toBeTruthy();
+      });
     });
 
     describe('updateCommitMessage', () => {
diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
index 669ee248bf1..da66c7504cb 100644
--- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
+++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
@@ -59,23 +59,15 @@ describe('mrWidgetOptions', () => {
     });
 
     describe('shouldRenderPipelines', () => {
-      it('should return true for the initial data', () => {
-        expect(vm.shouldRenderPipelines).toBeTruthy();
-      });
+      it('should return true when hasCI is true', () => {
+        vm.mr.hasCI = true;
 
-      it('should return true when pipeline is empty but MR.hasCI is set to true', () => {
-        vm.mr.pipeline = {};
         expect(vm.shouldRenderPipelines).toBeTruthy();
       });
 
-      it('should return true when pipeline available', () => {
+      it('should return false when hasCI is false', () => {
         vm.mr.hasCI = false;
-        expect(vm.shouldRenderPipelines).toBeTruthy();
-      });
 
-      it('should return false when there is no pipeline', () => {
-        vm.mr.pipeline = {};
-        vm.mr.hasCI = false;
         expect(vm.shouldRenderPipelines).toBeFalsy();
       });
     });
diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
index 56dd0198ae2..8e5614b20f0 100644
--- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
+++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
@@ -18,5 +18,39 @@ describe('MergeRequestStore', () => {
       store.setData({ ...mockData, work_in_progress: !mockData.work_in_progress });
       expect(store.hasSHAChanged).toBe(false);
     });
+
+    describe('isPipelinePassing', () => {
+      it('is true when the CI status is `success`', () => {
+        store.setData({ ...mockData, ci_status: 'success' });
+        expect(store.isPipelinePassing).toBe(true);
+      });
+
+      it('is true when the CI status is `success_with_warnings`', () => {
+        store.setData({ ...mockData, ci_status: 'success_with_warnings' });
+        expect(store.isPipelinePassing).toBe(true);
+      });
+
+      it('is false when the CI status is `failed`', () => {
+        store.setData({ ...mockData, ci_status: 'failed' });
+        expect(store.isPipelinePassing).toBe(false);
+      });
+
+      it('is false when the CI status is anything except `success`', () => {
+        store.setData({ ...mockData, ci_status: 'foobarbaz' });
+        expect(store.isPipelinePassing).toBe(false);
+      });
+    });
+
+    describe('isPipelineSkipped', () => {
+      it('should set isPipelineSkipped=true when the CI status is `skipped`', () => {
+        store.setData({ ...mockData, ci_status: 'skipped' });
+        expect(store.isPipelineSkipped).toBe(true);
+      });
+
+      it('should set isPipelineSkipped=false when the CI status is anything except `skipped`', () => {
+        store.setData({ ...mockData, ci_status: 'foobarbaz' });
+        expect(store.isPipelineSkipped).toBe(false);
+      });
+    });
   });
 });