Merge branch 'fix-team-build-state-in-mr-widget' into 'master'

Merge request widget displays TeamCity build state and code coverage correctly again

## What does this MR do?
This MR contains a fix for a regression introduced in `8.7`. In former version, the TeamCity build status was always displayed correctly. In `8.7` the build state is still checked, but the UI is not updated correctly any longer.

## Are there points in the code the reviewer needs to double check?
The changes are quite simple, so please simply double check them.

## Why was this MR needed?
This MR is needed to make the TeamCity build status working again.

## What are the relevant issue numbers?
#17080 

See merge request !3998

3 files changed, 57 insertions, 10 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 0e6e2505ec6..d2ff4c32f2d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -22,6 +22,7 @@ v 8.8.0 (unreleased)
   - API support for the 'since' and 'until' operators on commit requests (Paco Guzman)
   - Fix Gravatar hint in user profile when Gravatar is disabled. !3988 (Artem Sidorenko)
   - Expire repository exists? and has_visible_content? caches after a push if necessary
+  - Merge request widget displays TeamCity build state and code coverage correctly again.
 
 v 8.7.3
   - Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented
diff --git a/app/assets/javascripts/merge_request_widget.js.coffee b/app/assets/javascripts/merge_request_widget.js.coffee
index 065626beeb8..17a5a057a94 100644
--- a/app/assets/javascripts/merge_request_widget.js.coffee
+++ b/app/assets/javascripts/merge_request_widget.js.coffee
@@ -68,20 +68,18 @@ class @MergeRequestWidget
     $.getJSON @opts.ci_status_url, (data) =>
       @readyForCICheck = true
 
-      if @firstCICheck
-        @firstCICheck = false
-        @opts.ci_status = data.status
-
-      if @opts.ci_status is ''
-        @opts.ci_status = data.status
+      if data.status is ''
         return
 
-      if data.status isnt @opts.ci_status and data.status?
+      if @firstCiCheck || data.status isnt @opts.ci_status and data.status?
+        @opts.ci_status = data.status
         @showCIStatus data.status
         if data.coverage
           @showCICoverage data.coverage
 
-        if showNotification
+        # The first check should only update the UI, a notification
+        # should only be displayed on status changes
+        if showNotification and not @firstCiCheck
           status = @ciLabelForStatus(data.status)
 
           if status is "preparing"
@@ -104,8 +102,7 @@ class @MergeRequestWidget
               @close()
               Turbolinks.visit _this.opts.builds_path
           )
-
-        @opts.ci_status = data.status
+        @firstCiCheck = false
 
   showCIStatus: (state) ->
     $('.ci_widget').hide()
diff --git a/spec/javascripts/merge_request_widget_spec.js.coffee b/spec/javascripts/merge_request_widget_spec.js.coffee
new file mode 100644
index 00000000000..c0bd8a29e43
--- /dev/null
+++ b/spec/javascripts/merge_request_widget_spec.js.coffee
@@ -0,0 +1,49 @@
+#= require merge_request_widget
+
+describe 'MergeRequestWidget', ->
+
+  beforeEach ->
+    window.notifyPermissions = () ->
+    window.notify = () ->
+    @opts = {
+      ci_status_url:"http://sampledomain.local/ci/getstatus",
+      ci_status:"",
+      ci_message: {
+        normal: "Build {{status}} for \"{{title}}\"",
+        preparing: "{{status}} build for \"{{title}}\""
+      },
+      ci_title: {
+        preparing: "{{status}} build",
+        normal: "Build {{status}}"
+      },
+      gitlab_icon:"gitlab_logo.png",
+      builds_path:"http://sampledomain.local/sampleBuildsPath"
+    }
+    @class = new MergeRequestWidget(@opts)
+    @ciStatusData = {"title":"Sample MR title","sha":"12a34bc5","status":"success","coverage":98}
+
+  describe 'getCIStatus', ->
+    beforeEach ->
+      spyOn(jQuery, 'getJSON').and.callFake (req, cb) =>
+        cb(@ciStatusData)
+
+    it 'should call showCIStatus even if a notification should not be displayed', ->
+      spy = spyOn(@class, 'showCIStatus').and.stub()
+      @class.getCIStatus(false)
+      expect(spy).toHaveBeenCalledWith(@ciStatusData.status)
+
+    it 'should call showCIStatus when a notification should be displayed', ->
+      spy = spyOn(@class, 'showCIStatus').and.stub()
+      @class.getCIStatus(true)
+      expect(spy).toHaveBeenCalledWith(@ciStatusData.status)
+
+    it 'should call showCICoverage when the coverage rate is set', ->
+      spy = spyOn(@class, 'showCICoverage').and.stub()
+      @class.getCIStatus(false)
+      expect(spy).toHaveBeenCalledWith(@ciStatusData.coverage)
+
+    it 'should not call showCICoverage when the coverage rate is not set', ->
+      @ciStatusData.coverage = null
+      spy = spyOn(@class, 'showCICoverage').and.stub()
+      @class.getCIStatus(false)
+      expect(spy).not.toHaveBeenCalled()