Pass memoizable warnings attribute to stage object

4 files changed, 30 insertions, 13 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 2a97e8bae4a..fab8497ec7d 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -128,16 +128,21 @@ module Ci
     end
 
     def stages
+      # TODO, this needs refactoring, see gitlab-ce#26481.
+
+      stages_query = statuses
+        .group('stage').select(:stage).order('max(stage_idx)')
+
       status_sql = statuses.latest.where('stage=sg.stage').status_sql
 
-      stages_query = statuses.group('stage').select(:stage)
-                       .order('max(stage_idx)')
+      warnings_sql = statuses.latest.select('COUNT(*) > 0')
+        .where('stage=sg.stage').failed_but_allowed.to_sql
 
-      stages_with_statuses = CommitStatus.from(stages_query, :sg).
-        pluck('sg.stage', status_sql)
+      stages_with_statuses = CommitStatus.from(stages_query, :sg)
+        .pluck('sg.stage', status_sql, "(#{warnings_sql})")
 
       stages_with_statuses.map do |stage|
-        Ci::Stage.new(self, name: stage.first, status: stage.last)
+        Ci::Stage.new(self, Hash[%i[name status warnings].zip(stage)])
       end
     end
 
diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb
index d4b6ff910aa..ca76d82f358 100644
--- a/app/models/ci/stage.rb
+++ b/app/models/ci/stage.rb
@@ -8,10 +8,11 @@ module Ci
 
     delegate :project, to: :pipeline
 
-    def initialize(pipeline, name:, status: nil)
+    def initialize(pipeline, name:, status: nil, warnings: nil)
       @pipeline = pipeline
       @name = name
       @status = status
+      @warnings = warnings
     end
 
     def to_param
@@ -45,7 +46,7 @@ module Ci
     end
 
     def has_warnings?
-      statuses.latest.failed_but_allowed.any?
+      @warnings ||= statuses.latest.failed_but_allowed.any?
     end
   end
 end
diff --git a/spec/factories/ci/stages.rb b/spec/factories/ci/stages.rb
index ee3b17b8bf1..7f557b25ccb 100644
--- a/spec/factories/ci/stages.rb
+++ b/spec/factories/ci/stages.rb
@@ -3,11 +3,12 @@ FactoryGirl.define do
     transient do
       name 'test'
       status nil
+      warnings nil
       pipeline factory: :ci_empty_pipeline
     end
 
     initialize_with do
-      Ci::Stage.new(pipeline, name: name, status: status)
+      Ci::Stage.new(pipeline, name: name, status: status, warnings: warnings)
     end
   end
 end
diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb
index 3d387d52c8e..cca0cb1e3b0 100644
--- a/spec/models/ci/stage_spec.rb
+++ b/spec/models/ci/stage_spec.rb
@@ -168,13 +168,23 @@ describe Ci::Stage, models: true do
 
   describe '#has_warnings?' do
     context 'when stage has warnings' do
-      before do
-        create(:ci_build, :failed, :allowed_to_fail,
-               stage: stage_name, pipeline: pipeline)
+      context 'when using memoized warnings flag' do
+        let(:stage) { build(:ci_stage, warnings: true) }
+
+        it 'has warnings' do
+          expect(stage).to have_warnings
+        end
       end
 
-      it 'has warnings' do
-        expect(stage).to have_warnings
+      context 'when calculating warnings from statuses' do
+        before do
+          create(:ci_build, :failed, :allowed_to_fail,
+                 stage: stage_name, pipeline: pipeline)
+        end
+
+        it 'has warnings' do
+          expect(stage).to have_warnings
+        end
       end
     end