From 655b2640ae9cbe2737369401969e99caeddf192d Mon Sep 17 00:00:00 2001 From: Benedikt Huss Date: Wed, 4 May 2016 09:34:25 +0200 Subject: Merge request widget displays TeamCity build state and code coverage correctly again --- .../javascripts/merge_request_widget.js.coffee | 12 ++++---- .../merge_request_widget_spec.js.coffee | 35 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 spec/javascripts/merge_request_widget_spec.js.coffee diff --git a/app/assets/javascripts/merge_request_widget.js.coffee b/app/assets/javascripts/merge_request_widget.js.coffee index 065626beeb8..1abda3530c4 100644 --- a/app/assets/javascripts/merge_request_widget.js.coffee +++ b/app/assets/javascripts/merge_request_widget.js.coffee @@ -68,13 +68,13 @@ class @MergeRequestWidget $.getJSON @opts.ci_status_url, (data) => @readyForCICheck = true - if @firstCICheck - @firstCICheck = false + if @firstCICheck || @opts.ci_status is '' + if @firstCICheck + @firstCICheck = false @opts.ci_status = data.status - - if @opts.ci_status is '' - @opts.ci_status = data.status - return + @showCIStatus data.status + if data.coverage + @showCICoverage data.coverage if data.status isnt @opts.ci_status and data.status? @showCIStatus data.status 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..e1fb610654e --- /dev/null +++ b/spec/javascripts/merge_request_widget_spec.js.coffee @@ -0,0 +1,35 @@ +#= require merge_request_widget + +describe 'MergeRequestWidget', -> + + beforeEach -> + window.notifyPermissions = () -> + @opts = {ci_status_url:"http://sampledomain.local/ci/getstatus",ci_status:""} + @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(true) + 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) + expect(spy).not.toHaveBeenCalled() -- cgit v1.2.1 From 0e9c2e721b08e244e63f1d26ac3771a8d858cd76 Mon Sep 17 00:00:00 2001 From: Benedikt Huss Date: Wed, 4 May 2016 20:45:12 +0200 Subject: Feedback from stanhu --- CHANGELOG | 1 + .../javascripts/merge_request_widget.js.coffee | 19 ++++++++----------- spec/javascripts/merge_request_widget_spec.js.coffee | 20 +++++++++++++++++--- 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() -- cgit v1.2.1