Fix failures

app/models/ci/group.rb

@@ -24,13 +24,13 @@ module Ci
     def status
       strong_memoize(:status) do
         if Feature.enabled?(:ci_composite_status, default_enabled: false)
-          all_statuses = @jobs.pluck(:status, :allow_failure)
-
           Gitlab::Ci::Status::Composite
-            .new(all_statuses, status_key: 0, allow_failure_key: 1)
+            .new(@jobs)
             .status
         else
-          CommitStatus.where(id: @jobs).legacy_status
+          CommitStatus
+            .where(id: @jobs)
+            .legacy_status
         end
       end
     end

app/models/ci/legacy_stage.rb

@@ -14,7 +14,8 @@ module Ci
       @pipeline = pipeline
       @name = name
       @status = status
-      @warnings = warnings
+      # support ints and booleans
+      @has_warnings = ActiveRecord::Type::Boolean.new.cast(warnings)
     end
 
     def groups
@@ -52,14 +53,12 @@ module Ci
     end
 
     def has_warnings?
-      case @warnings
-      when Integer
-        @warnings > 0
-      when TrueClass, FalseClass
-        @warnings
-      else
-        statuses.latest.failed_but_allowed.any?
+      # lazilly calculate the warnings
+      if @has_warnings.nil?
+        @has_warnings = statuses.latest.failed_but_allowed.any?
       end
+
+      @has_warnings
     end
 
     def manual_playable?

app/models/ci/pipeline.rb

@@ -411,10 +411,8 @@ module Ci
         .group_by(&:stage)
 
       stages.map do |stage_name, jobs|
-        jobs_statuses = jobs.pluck(:status, :allow_failure)
-
         composite_status = Gitlab::Ci::Status::Composite
-          .new(jobs_statuses, status_key: 0, allow_failure_key: 1)
+          .new(jobs)
 
         Ci::LegacyStage.new(self,
           name: stage_name,

app/models/concerns/has_status.rb

@@ -65,13 +65,8 @@ module HasStatus
     # 2. Or executes expensive SQL query
     def slow_composite_status
       if Feature.enabled?(:ci_composite_status, default_enabled: false)
-        columns = [:status]
-        columns << :allow_failure if column_names.include?('allow_failure')
-
-        all_statuses = all.pluck(*columns)
-
         Gitlab::Ci::Status::Composite
-          .new(all_statuses, status_key: 0, allow_failure_key: 1)
+          .new(all, with_allow_failure: columns_hash.key?('allow_failure'))
           .status
       else
         legacy_status

lib/gitlab/ci/status/composite.rb

@@ -6,15 +6,15 @@ module Gitlab
       class Composite
         include Gitlab::Utils::StrongMemoize
 
-        # This class accepts an array of arrays
-        # The `status_key` and `allow_failure_key` define an index
-        # or key in each entry
-        def initialize(all_statuses, status_key:, allow_failure_key:)
-          raise ArgumentError, "all_statuses needs to be an Array" unless all_statuses.is_a?(Array)
+        # This class accepts an array of arrays/hashes/or objects
+        def initialize(all_statuses, with_allow_failure: true)
+          unless all_statuses.respond_to?(:pluck)
+            raise ArgumentError, "all_statuses needs to respond to `.pluck`"
+          end
 
           @status_set = Set.new
-          @status_key = status_key
-          @allow_failure_key = allow_failure_key
+          @status_key = 0
+          @allow_failure_key = 1 if with_allow_failure
 
           consume_all_statuses(all_statuses)
         end
@@ -78,7 +78,13 @@ module Gitlab
         end
 
         def consume_all_statuses(all_statuses)
-          all_statuses.each(&method(:consume_status))
+          columns = []
+          columns[@status_key] = :status
+          columns[@allow_failure_key] = :allow_failure if @allow_failure_key
+
+          all_statuses
+            .pluck(*columns)
+            .each(&method(:consume_status))
         end
 
         def consume_status(description)

spec/lib/gitlab/ci/status/composite_spec.rb

@@ -14,11 +14,12 @@ describe Gitlab::Ci::Status::Composite do
   end
 
   describe '#status' do
-    subject { composite_status.status }
-
     shared_examples 'compares composite with SQL status' do
       it 'returns exactly the same result' do
-        is_expected.to eq(Ci::Build.where(id: all_statuses).legacy_status)
+        builds = Ci::Build.where(id: all_statuses)
+
+        expect(composite_status.status).to eq(builds.legacy_status)
+        expect(composite_status.warnings?).to eq(builds.failed_but_allowed.any?)
       end
     end
 
@@ -31,11 +32,7 @@ describe Gitlab::Ci::Status::Composite do
             end
 
             let(:composite_status) do
-              described_class.new(
-                all_statuses.pluck(:status),
-                status_key: 0,
-                allow_failure_key: nil
-              )
+              described_class.new(all_statuses)
             end
           end
 
@@ -48,11 +45,7 @@ describe Gitlab::Ci::Status::Composite do
                 end
 
                 let(:composite_status) do
-                  described_class.new(
-                    all_statuses.pluck(:status, :allow_failure),
-                    status_key: 0,
-                    allow_failure_key: 1
-                  )
+                  described_class.new(all_statuses)
                 end
               end
             end

spec/models/ci/legacy_stage_spec.rb

@@ -232,15 +232,6 @@ describe Ci::LegacyStage do
             expect(stage).not_to have_warnings
           end
         end
-
-        context 'when number of warnings is not a valid value' do
-          let(:stage) { build(:ci_stage, warnings: 'aa') }
-
-          it 'calculates statuses using database queries' do
-            expect(stage).to receive(:statuses).and_call_original
-            expect(stage).not_to have_warnings
-          end
-        end
       end
 
       context 'when calculating warnings from statuses' do