Refactor bridge status update logic

Refactors the bridge status update logic to be centralized in one single
place.

app/models/ci/pipeline.rb

@@ -329,7 +329,7 @@ module Ci
     end
 
     def self.bridgeable_statuses
-      ::Ci::Pipeline.all_statuses - [:created, :preparing, :pending]
+      ::Ci::Pipeline::AVAILABLE_STATUSES - %w[created preparing pending]
     end
 
     def stages_count

app/models/concerns/has_status.rb

@@ -72,10 +72,6 @@ module HasStatus
     def completed_statuses
       COMPLETED_STATUSES.map(&:to_sym)
     end
-
-    def all_statuses
-      AVAILABLE_STATUSES.map(&:to_sym)
-    end
   end
 
   included do

ee/app/models/ee/ci/pipeline.rb

@@ -97,7 +97,7 @@ module EE
             end
           end
 
-          after_transition any => ::Ci::Pipeline.bridgeable_statuses do |pipeline|
+          after_transition any => ::Ci::Pipeline.bridgeable_statuses.map(&:to_sym) do |pipeline|
             next unless pipeline.downstream_bridges.any?
 
             pipeline.run_after_commit do

ee/app/services/ci/pipeline_bridge_status_service.rb

@@ -6,26 +6,13 @@ module Ci
 
     def execute(pipeline)
       pipeline.downstream_bridges.each do |bridge|
-        process_bridge(pipeline.status, bridge)
+        bridge.save! if self.class.process_bridge(pipeline.status, bridge)
       end
     end
 
-    def process_bridge(status, bridge)
-      case status
-      when 'success'
-        bridge.success!
-      when 'failed'
-        bridge.drop!
-      when 'canceled'
-        bridge.cancel!
-      when 'skipped'
-        bridge.skip!
-      when 'running'
-        bridge.run!
-      when 'manual'
-        bridge.update(status: 'manual')
-      when 'scheduled'
-        bridge.update(status: 'scheduled')
+    def self.process_bridge(status, bridge)
+      if ::Ci::Pipeline.bridgeable_statuses.include?(status)
+        bridge.status = status
       else
         raise InvalidUpstreamStatusError
       end

ee/app/services/ci/subscribe_bridge_service.rb

@@ -19,13 +19,13 @@ module Ci
 
       return unless upstream_pipeline
 
-      bridge_updates = { upstream_pipeline: upstream_pipeline }
+      bridge.upstream_pipeline = upstream_pipeline
 
-      if ::Ci::Pipeline.bridgeable_statuses.include?(upstream_pipeline.status.to_sym)
-        bridge_updates[:status] = upstream_pipeline.status
+      if ::Ci::Pipeline.bridgeable_statuses.include?(upstream_pipeline.status)
+        ::Ci::PipelineBridgeStatusService.process_bridge(upstream_pipeline.status, bridge)
       end
 
-      bridge.update!(bridge_updates)
+      bridge.save!
     end
 
     private

ee/spec/services/ci/pipeline_bridge_status_service_spec.rb

@@ -5,40 +5,68 @@ require 'spec_helper'
 describe Ci::PipelineBridgeStatusService do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
-  let(:pipeline) { create(:ci_pipeline, status, project: project) }
+  let(:pipeline) { create(:ci_pipeline, status: status, project: project) }
 
   describe '#execute' do
     subject { described_class.new(project, user).execute(pipeline) }
 
     context 'when pipeline has bridged jobs' do
-      let(:bridge) { create(:ci_bridge, status: :pending) }
+      let(:bridge) { create(:ci_bridge, status: 'pending') }
 
       before do
         pipeline.downstream_bridges << bridge
       end
 
+      context 'when pipeline has the same status as the bridge' do
+        let(:status) { 'running' }
+
+        before do
+          bridge.status = 'running'
+        end
+
+        it 'does not update the bridge status' do
+          expect { subject }.not_to change { bridge.status }
+        end
+
+        it 'does not save the bridge' do
+          expect(bridge).not_to receive(:save!)
+        end
+      end
+
       context 'when pipeline starts running' do
-        let(:status) { :running }
+        let(:status) { 'running' }
 
         it 'updates the bridge status with the pipeline status' do
           expect { subject }.to change { bridge.status }.from('pending').to('running')
         end
+
+        it 'persists the status change' do
+          expect(bridge).to be_persisted
+        end
       end
 
       context 'when pipeline succeeds' do
-        let(:status) { :success }
+        let(:status) { 'success' }
 
         it 'updates the bridge status with the pipeline status' do
           expect { subject }.to change { bridge.status }.from('pending').to('success')
         end
+
+        it 'persists the status change' do
+          expect(bridge).to be_persisted
+        end
       end
 
       context 'when pipeline gets blocked' do
-        let(:status) { :blocked }
+        let(:status) { 'manual' }
 
         it 'updates the bridge status with the pipeline status' do
           expect { subject }.to change { bridge.status }.from('pending').to('manual')
         end
+
+        it 'persists the status change' do
+          expect(bridge).to be_persisted
+        end
       end
     end
   end

spec/models/ci/pipeline_spec.rb

@@ -1929,6 +1929,13 @@ describe Ci::Pipeline, :mailer do
     it { is_expected.to be_an(Array) }
   end
 
+  describe '.bridgeable_statuses' do
+    subject { described_class.bridgeable_statuses }
+
+    it { is_expected.to be_an(Array) }
+    it { is_expected.not_to include('created', 'preparing', 'pending') }
+  end
+
   describe '#status' do
     let(:build) do
       create(:ci_build, :created, pipeline: pipeline, name: 'test')