Take SHAs from commit diffs in PipelinesForMergeRequestFinder

Only takes "latest version" of commit diffs when fetching MR's SHAs

app/finders/ci/pipelines_for_merge_request_finder.rb

@@ -5,8 +5,6 @@ module Ci
   class PipelinesForMergeRequestFinder
     include Gitlab::Utils::StrongMemoize
 
-    COMMITS_LIMIT = 100
-
     def initialize(merge_request, current_user)
       @merge_request = merge_request
       @current_user = current_user
@@ -14,7 +12,7 @@ module Ci
 
     attr_reader :merge_request, :current_user
 
-    delegate :commit_shas, :target_project, :source_project, :source_branch, to: :merge_request
+    delegate :recent_diff_head_shas, :commit_shas, :target_project, :source_project, :source_branch, to: :merge_request
 
     # Fetch all pipelines that the user can read.
     def execute
@@ -83,11 +81,8 @@ module Ci
 
     def all_pipelines_for_merge_request
       if Feature.enabled?(:decomposed_ci_query_in_pipelines_for_merge_request_finder, source_project, default_enabled: :yaml)
-        pipelines_using_cte
-      else
-        shas = all_commit_shas
         pipelines_for_merge_request = triggered_by_merge_request
-        pipelines_for_branch = triggered_for_branch.for_sha(shas)
+        pipelines_for_branch = triggered_for_branch.for_sha(recent_diff_head_shas)
 
         Ci::Pipeline.from_union([pipelines_for_merge_request, pipelines_for_branch])
       else
@@ -95,26 +90,6 @@ module Ci
       end
     end
 
-    def all_commit_shas
-      total_commits = merge_request.all_commits.count
-
-      if total_commits > COMMITS_LIMIT
-        warn_total_commits_count_exceeded(total_commits)
-      end
-
-      # We're limiting the number of commits' SHAs to 100 since they are used in a WHERE clause of a query
-      merge_request.all_commits.order(id: :desc).limit(COMMITS_LIMIT).pluck(:sha).uniq # rubocop: disable CodeReuse/ActiveRecord
-    end
-
-    def warn_total_commits_count_exceeded(total_commits)
-      Gitlab::AppLogger.warn(
-        message: "A merge request has more than #{COMMITS_LIMIT} commits",
-        project_id: merge_request.source_project.id,
-        merge_request_id: merge_request.id,
-        total_commits: total_commits
-      )
-    end
-
     # NOTE: this method returns only parent merge request pipelines.
     # Child merge request pipelines have a different source.
     def triggered_by_merge_request

app/models/merge_request.rb

@@ -1658,6 +1658,10 @@ class MergeRequest < ApplicationRecord
     service_class.new(project, current_user, id: id, report_type: report_type).execute(comparison_base_pipeline(identifier), actual_head_pipeline)
   end
 
+  def recent_diff_head_shas
+    merge_request_diffs.recent.pluck(:head_commit_sha)
+  end
+
   def all_commits
     MergeRequestDiffCommit
       .where(merge_request_diff: merge_request_diffs.recent)

spec/finders/ci/pipelines_for_merge_request_finder_spec.rb

@@ -97,7 +97,7 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
 
     shared_examples 'returning pipelines with proper ordering' do
       let!(:all_pipelines) do
-        merge_request.all_commit_shas.map do |sha|
+        merge_request.recent_diff_head_shas.map do |sha|
           create(:ci_empty_pipeline,
             project: project, sha: sha, ref: merge_request.source_branch)
         end
@@ -215,14 +215,6 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
             expect(merge_request.all_pipelines).to include(detached_merge_request_pipeline)
           end
         end
-
-        it 'does not write a log message' do
-          allow(Gitlab::AppLogger).to receive(:warn).and_call_original
-
-          subject.all
-
-          expect(Gitlab::AppLogger).not_to have_received(:warn)
-        end
       end
 
       let(:source_ref) { 'feature' }
@@ -230,7 +222,7 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
 
       let!(:branch_pipeline) do
         create(:ci_pipeline, source: :push, project: project,
-               ref: source_ref, sha: shas.second)
+               ref: source_ref, sha: merge_request.merge_request_diff.head_commit_sha)
       end
 
       let!(:tag_pipeline) do
@@ -250,37 +242,12 @@ RSpec.describe Ci::PipelinesForMergeRequestFinder do
       let(:project) { create(:project, :repository) }
       let(:shas) { project.repository.commits(source_ref, limit: 2).map(&:id) }
 
-      before do
-        create(:merge_request_diff_commit,
-               merge_request_diff: merge_request.merge_request_diff,
-               sha: shas.second, relative_order: 1)
-      end
-
       context 'when `decomposed_ci_query_in_pipelines_for_merge_request_finder` feature flag enabled' do
         before do
           stub_feature_flags(decomposed_ci_query_in_pipelines_for_merge_request_finder: merge_request.source_project)
         end
 
         it_behaves_like 'returns all pipelines for merge request'
-
-        context 'when total commits exceed the limit' do
-          before do
-            stub_const("#{described_class}::COMMITS_LIMIT", 1)
-          end
-
-          it 'writes a log message' do
-            allow(Gitlab::AppLogger).to receive(:warn).and_call_original
-
-            subject.all
-
-            expect(Gitlab::AppLogger).to have_received(:warn).with(
-              message: "A merge request has more than #{described_class::COMMITS_LIMIT} commits",
-              project_id: merge_request.source_project.id,
-              merge_request_id: merge_request.id,
-              total_commits: 2
-            )
-          end
-        end
       end
 
       context 'when `decomposed_ci_query_in_pipelines_for_merge_request_finder` feature flag disabled' do

spec/requests/api/graphql/project/merge_request/pipelines_spec.rb

@@ -39,7 +39,7 @@ RSpec.describe 'Query.project.mergeRequests.pipelines' do
 
     before do
       merge_requests.each do |mr|
-        shas = mr.all_commits.limit(2).pluck(:sha)
+        shas = mr.recent_diff_head_shas
 
         shas.each do |sha|
           create(:ci_pipeline, :success, project: project, ref: mr.source_branch, sha: sha)
@@ -52,7 +52,7 @@ RSpec.describe 'Query.project.mergeRequests.pipelines' do
 
       p_nodes = graphql_data_at(:project, :merge_requests, :nodes)
 
-      expect(p_nodes).to all(match('iid' => be_present, 'pipelines' => match('count' => 2)))
+      expect(p_nodes).to all(match('iid' => be_present, 'pipelines' => match('count' => 1)))
     end
 
     it 'is scalable', :request_store, :use_clean_rails_memory_store_caching do