Feedback from stanhu

3 files changed, 26 insertions, 14 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 1abda3530c4..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 || @opts.ci_status is ''
-        if @firstCICheck
-          @firstCICheck = false
-        @opts.ci_status = data.status
-        @showCIStatus data.status
-        if data.coverage
-          @showCICoverage data.coverage
+      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
index e1fb610654e..c0bd8a29e43 100644
--- a/spec/javascripts/merge_request_widget_spec.js.coffee
+++ b/spec/javascripts/merge_request_widget_spec.js.coffee
@@ -4,7 +4,21 @@ describe 'MergeRequestWidget', ->
 
   beforeEach ->
     window.notifyPermissions = () ->
-    @opts = {ci_status_url:"http://sampledomain.local/ci/getstatus",ci_status:""}
+    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}
 
@@ -25,11 +39,11 @@ describe 'MergeRequestWidget', ->
 
     it 'should call showCICoverage when the coverage rate is set', ->
       spy = spyOn(@class, 'showCICoverage').and.stub()
-      @class.getCIStatus(true)
+      @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(true)
+      @class.getCIStatus(false)
       expect(spy).not.toHaveBeenCalled()