Remove Ci::PipelineDelegator in favor of Ci:Processable

- Moved all delegations into Ci::Processable
- Added specs for delegated method into Processable spec
- Created EE Processable spec for merge train related methods

app/models/ci/bridge.rb

@@ -3,7 +3,6 @@
 module Ci
   class Bridge < Ci::Processable
     include Ci::Contextable
-    include Ci::PipelineDelegator
     include Ci::Metadatable
     include Importable
     include AfterCommitQueue

app/models/ci/build.rb

@@ -4,7 +4,6 @@ module Ci
   class Build < Ci::Processable
     include Ci::Metadatable
     include Ci::Contextable
-    include Ci::PipelineDelegator
     include TokenAuthenticatable
     include AfterCommitQueue
     include ObjectStorage::BackgroundMove

app/models/ci/processable.rb

@@ -51,6 +51,12 @@ module Ci
     validates :type, presence: true
     validates :scheduling_type, presence: true, on: :create, if: :validate_scheduling_type?
 
+    delegate :merge_request?,
+      :merge_request_ref?,
+      :legacy_detached_merge_request_pipeline?,
+      :merge_train_pipeline?,
+      to: :pipeline
+
     def aggregated_needs_names
       read_attribute(:aggregated_needs_names)
     end

app/models/concerns/ci/has_ref.rb

@@ -2,7 +2,7 @@
 
 ##
 # We will disable `ref` and `sha` attributes in `Ci::Build` in the future
-# and remove this module in favor of Ci::PipelineDelegator.
+# and remove this module in favor of Ci::Processable.
 module Ci
   module HasRef
     extend ActiveSupport::Concern

app/models/concerns/ci/pipeline_delegator.rb

-# frozen_string_literal: true
-
-##
-# This module is mainly used by child associations of `Ci::Pipeline` that needs to look up
-# single source of truth. For example, `Ci::Build` has `git_ref` method, which behaves
-# slightly different from `Ci::Pipeline`'s `git_ref`. This is very confusing as
-# the system could behave differently time to time.
-# We should have a single interface in `Ci::Pipeline` and access the method always.
-module Ci
-  module PipelineDelegator
-    extend ActiveSupport::Concern
-
-    included do
-      delegate :merge_request?,
-               :merge_request_ref?,
-               :legacy_detached_merge_request_pipeline?,
-               :merge_train_pipeline?, to: :pipeline
-    end
-  end
-end

ee/spec/models/ci/processable_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::Processable do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
+
+  let_it_be(:detached_merge_request_pipeline) do
+    create(:ci_pipeline, :detached_merge_request_pipeline, :with_job, project: project)
+  end
+
+  let_it_be(:legacy_detached_merge_request_pipeline) do
+    create(:ci_pipeline, :legacy_detached_merge_request_pipeline, :with_job, project: project)
+  end
+
+  let_it_be(:merged_result_pipeline) do
+    create(:ci_pipeline, :merged_result_pipeline, :with_job, project: project)
+  end
+
+  describe '#merge_train_pipeline?' do
+    subject { pipeline.processables.first.merge_train_pipeline? }
+
+    context 'in a detached merge request pipeline' do
+      let(:pipeline) { detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_train_pipeline?) }
+    end
+
+    context 'in a legacy detached merge_request_pipeline' do
+      let(:pipeline) { legacy_detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_train_pipeline?) }
+    end
+
+    context 'in a pipeline for merged results' do
+      let(:pipeline) { merged_result_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_train_pipeline?) }
+    end
+  end
+end

spec/models/ci/bridge_spec.rb

@@ -17,8 +17,6 @@ describe Ci::Bridge do
     { trigger: { project: 'my/project', branch: 'master' } }
   end
 
-  it { is_expected.to include_module(Ci::PipelineDelegator) }
-
   it 'has many sourced pipelines' do
     expect(bridge).to have_many(:sourced_pipelines)
   end

spec/models/ci/build_spec.rb

@@ -37,8 +37,6 @@ describe Ci::Build do
   it { is_expected.to delegate_method(:merge_request_ref?).to(:pipeline) }
   it { is_expected.to delegate_method(:legacy_detached_merge_request_pipeline?).to(:pipeline) }
 
-  it { is_expected.to include_module(Ci::PipelineDelegator) }
-
   describe 'associations' do
     it 'has a bidirectional relationship with projects' do
       expect(described_class.reflect_on_association(:project).has_inverse?).to eq(:builds)

spec/models/ci/processable_spec.rb

@@ -6,6 +6,18 @@ describe Ci::Processable do
   let_it_be(:project) { create(:project) }
   let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
 
+  let_it_be(:detached_merge_request_pipeline) do
+    create(:ci_pipeline, :detached_merge_request_pipeline, :with_job, project: project)
+  end
+
+  let_it_be(:legacy_detached_merge_request_pipeline) do
+    create(:ci_pipeline, :legacy_detached_merge_request_pipeline, :with_job, project: project)
+  end
+
+  let_it_be(:merged_result_pipeline) do
+    create(:ci_pipeline, :merged_result_pipeline, :with_job, project: project)
+  end
+
   describe '#aggregated_needs_names' do
     let(:with_aggregated_needs) { pipeline.processables.select_with_aggregated_needs(project) }
 
@@ -155,4 +167,70 @@ describe Ci::Processable do
       end
     end
   end
+
+  describe '#merge_request?' do
+    subject { pipeline.processables.first.merge_request? }
+
+    context 'in a detached merge request pipeline' do
+      let(:pipeline) { detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_request?) }
+    end
+
+    context 'in a legacy detached merge_request_pipeline' do
+      let(:pipeline) { legacy_detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_request?) }
+    end
+
+    context 'in a pipeline for merged results' do
+      let(:pipeline) { merged_result_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_request?) }
+    end
+  end
+
+  describe '#merge_request_ref?' do
+    subject { pipeline.processables.first.merge_request_ref? }
+
+    context 'in a detached merge request pipeline' do
+      let(:pipeline) { detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_request_ref?) }
+    end
+
+    context 'in a legacy detached merge_request_pipeline' do
+      let(:pipeline) { legacy_detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_request_ref?) }
+    end
+
+    context 'in a pipeline for merged results' do
+      let(:pipeline) { merged_result_pipeline }
+
+      it { is_expected.to eq(pipeline.merge_request_ref?) }
+    end
+  end
+
+  describe '#legacy_detached_merge_request_pipeline?' do
+    subject { pipeline.processables.first.legacy_detached_merge_request_pipeline? }
+
+    context 'in a detached merge request pipeline' do
+      let(:pipeline) { detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.legacy_detached_merge_request_pipeline?) }
+    end
+
+    context 'in a legacy detached merge_request_pipeline' do
+      let(:pipeline) { legacy_detached_merge_request_pipeline }
+
+      it { is_expected.to eq(pipeline.legacy_detached_merge_request_pipeline?) }
+    end
+
+    context 'in a pipeline for merged results' do
+      let(:pipeline) { merged_result_pipeline }
+
+      it { is_expected.to eq(pipeline.legacy_detached_merge_request_pipeline?) }
+    end
+  end
 end