Merge branch 'mo-fix-codequality-merge-request-pipeline' into 'master'

Fix codequality for merge request pipeline

See merge request gitlab-org/gitlab!54620

app/models/ci/pipeline.rb

@@ -1165,6 +1165,10 @@ module Ci
       end
     end
 
+    def for_merged_result?
+      merge_request_event_type == :merged_result
+    end
+
     def persistent_ref
       @persistent_ref ||= PersistentRef.new(pipeline: self)
     end

app/models/merge_request.rb

@@ -36,6 +36,10 @@ class MergeRequest < ApplicationRecord
 
   SORTING_PREFERENCE_FIELD = :merge_requests_sort
 
+  ALLOWED_TO_USE_MERGE_BASE_PIPELINE_FOR_COMPARISON = {
+    'Ci::CompareCodequalityReportsService' => ->(project) { ::Gitlab::Ci::Features.display_codequality_backend_comparison?(project) }
+  }.freeze
+
   belongs_to :target_project, class_name: "Project"
   belongs_to :source_project, class_name: "Project"
   belongs_to :merge_user, class_name: "User"
@@ -1564,7 +1568,7 @@ class MergeRequest < ApplicationRecord
   def compare_reports(service_class, current_user = nil, report_type = nil )
     with_reactive_cache(service_class.name, current_user&.id, report_type) do |data|
       unless service_class.new(project, current_user, id: id, report_type: report_type)
-        .latest?(base_pipeline, actual_head_pipeline, data)
+        .latest?(comparison_base_pipeline(service_class.name), actual_head_pipeline, data)
         raise InvalidateReactiveCache
       end
 
@@ -1600,7 +1604,7 @@ class MergeRequest < ApplicationRecord
     raise NameError, service_class unless service_class < Ci::CompareReportsBaseService
 
     current_user = User.find_by(id: current_user_id)
-    service_class.new(project, current_user, id: id, report_type: report_type).execute(base_pipeline, actual_head_pipeline)
+    service_class.new(project, current_user, id: id, report_type: report_type).execute(comparison_base_pipeline(identifier), actual_head_pipeline)
   end
 
   def all_commits
@@ -1724,6 +1728,14 @@ class MergeRequest < ApplicationRecord
     end
   end
 
+  def use_merge_base_pipeline_for_comparison?(service_class)
+    ALLOWED_TO_USE_MERGE_BASE_PIPELINE_FOR_COMPARISON[service_class]&.call(project)
+  end
+
+  def comparison_base_pipeline(service_class)
+    (use_merge_base_pipeline_for_comparison?(service_class) && merge_base_pipeline) || base_pipeline
+  end
+
   def base_pipeline
     @base_pipeline ||= project.ci_pipelines
       .order(id: :desc)

app/serializers/merge_request_widget_entity.rb

@@ -157,7 +157,7 @@ class MergeRequestWidgetEntity < Grape::Entity
   end
 
   def use_merge_base_with_merged_results?
-    object.actual_head_pipeline&.merge_request_event_type == :merged_result
+    object.actual_head_pipeline&.for_merged_result?
   end
 
   def head_pipeline_downloadable_path_for_report_type(file_type)

spec/models/ci/pipeline_spec.rb

@@ -412,6 +412,24 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
     end
   end
 
+  describe '#for_merged_result?' do
+    subject { pipeline.for_merged_result? }
+
+    let(:pipeline) { merge_request.all_pipelines.last }
+
+    context 'when pipeline is merge request pipeline' do
+      let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when pipeline is detached merge request pipeline' do
+      let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#legacy_detached_merge_request_pipeline?' do
     subject { pipeline.legacy_detached_merge_request_pipeline? }
 

spec/models/merge_request_spec.rb

@@ -3839,6 +3839,87 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
+  describe '#use_merge_base_pipeline_for_comparison?' do
+    let(:project) { create(:project, :public, :repository) }
+    let(:merge_request) { create(:merge_request, :with_codequality_reports, source_project: project) }
+
+    subject { merge_request.use_merge_base_pipeline_for_comparison?(service_class) }
+
+    context 'when service class is Ci::CompareCodequalityReportsService' do
+      let(:service_class) { 'Ci::CompareCodequalityReportsService' }
+
+      context 'when feature flag is enabled' do
+        it { is_expected.to be_truthy }
+      end
+
+      context 'when feature flag is disabled' do
+        before do
+          stub_feature_flags(codequality_backend_comparison: false)
+        end
+
+        it { is_expected.to be_falsey }
+      end
+    end
+
+    context 'when service class is different' do
+      let(:service_class) { 'Ci::GenerateCoverageReportsService' }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
+  describe '#comparison_base_pipeline' do
+    subject(:pipeline) { merge_request.comparison_base_pipeline(service_class) }
+
+    let(:project) { create(:project, :public, :repository) }
+    let(:merge_request) { create(:merge_request, :with_codequality_reports, source_project: project) }
+    let!(:base_pipeline) do
+      create(:ci_pipeline,
+        :with_test_reports,
+        project: project,
+        ref: merge_request.target_branch,
+        sha: merge_request.diff_base_sha
+      )
+    end
+
+    context 'when service class is Ci::CompareCodequalityReportsService' do
+      let(:service_class) { 'Ci::CompareCodequalityReportsService' }
+
+      context 'when merge request has a merge request pipeline' do
+        let(:merge_request) do
+          create(:merge_request, :with_merge_request_pipeline)
+        end
+
+        let(:merge_base_pipeline) do
+          create(:ci_pipeline, ref: merge_request.target_branch, sha: merge_request.target_branch_sha)
+        end
+
+        before do
+          merge_base_pipeline
+          merge_request.update_head_pipeline
+        end
+
+        it 'returns the merge_base_pipeline' do
+          expect(pipeline).to eq(merge_base_pipeline)
+        end
+      end
+
+      context 'when merge does not have a merge request pipeline' do
+        it 'returns the base_pipeline' do
+          expect(pipeline).to eq(base_pipeline)
+        end
+      end
+    end
+
+    context 'when service_class is different' do
+      let(:service_class) { 'Ci::GenerateCoverageReportsService' }
+
+      it 'returns the base_pipeline' do
+        expect(pipeline).to eq(base_pipeline)
+      end
+    end
+  end
+
   describe '#base_pipeline' do
     let(:pipeline_arguments) do
       {