Include MRs for merge commits for changelogs

When generating a changelog, we now include links to merge requests for
any merge commits to use as input. Prior to this commit we'd only
consider merge requests that contained a commit.

The process of fetching merge requests by their merge SHA is a bit
different (and easier) compared to fetching them by their diffs. In this
commit we do so using an extra database query, but this query is simple
and should not pose any problems.

This fixes https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/1579

app/finders/merge_requests/oldest_per_commit_finder.rb

@@ -15,19 +15,45 @@ module MergeRequests
     # Returns a Hash that maps a commit ID to the oldest merge request that
     # introduced that commit.
     def execute(commits)
+      mapping = {}
+      shas = commits.map(&:id)
+
+      # To include merge requests by the commit SHA, we don't need to go through
+      # any diff rows.
+      #
+      # We can't squeeze all this into a single query, as the diff based data
+      # relies on a GROUP BY. On the other hand, retrieving MRs by their merge
+      # SHAs separately is much easier, and plenty fast.
+      @project
+        .merge_requests
+        .preload_target_project
+        .by_merge_commit_sha(shas)
+        .each do |mr|
+          # Merge SHAs can't be in the merge request itself. It _is_ possible a
+          # newer merge request includes the merge commit, but in that case we
+          # still want the oldest merge request.
+          mapping[mr.merge_commit_sha] = mr
+        end
+
+      remaining = shas - mapping.keys
+
+      return mapping if remaining.empty?
+
       id_rows = MergeRequestDiffCommit
-        .oldest_merge_request_id_per_commit(@project.id, commits.map(&:id))
+        .oldest_merge_request_id_per_commit(@project.id, remaining)
 
       mrs = MergeRequest
         .preload_target_project
         .id_in(id_rows.map { |r| r[:merge_request_id] })
         .index_by(&:id)
 
-      id_rows.each_with_object({}) do |row, hash|
+      id_rows.each do |row|
         if (mr = mrs[row[:merge_request_id]])
-          hash[row[:sha]] = mr
+          mapping[row[:sha]] = mr
         end
       end
+
+      mapping
     end
   end
 end

changelogs/unreleased/changelog-include-merge-commits.yml

+---
+title: Include MRs for merge commits for changelogs
+merge_request: 55371
+author:
+type: fixed

spec/finders/merge_requests/oldest_per_commit_finder_spec.rb

@@ -6,12 +6,20 @@ RSpec.describe MergeRequests::OldestPerCommitFinder do
   describe '#execute' do
     it 'returns a Hash mapping commit SHAs to their oldest merge requests' do
       project = create(:project)
+      sha1 = Digest::SHA1.hexdigest('foo')
+      sha2 = Digest::SHA1.hexdigest('bar')
+      sha3 = Digest::SHA1.hexdigest('baz')
       mr1 = create(:merge_request, :merged, target_project: project)
       mr2 = create(:merge_request, :merged, target_project: project)
+      mr3 = create(
+        :merge_request,
+        :merged,
+        target_project: project,
+        merge_commit_sha: sha3
+      )
+
       mr1_diff = create(:merge_request_diff, merge_request: mr1)
       mr2_diff = create(:merge_request_diff, merge_request: mr2)
-      sha1 = Digest::SHA1.hexdigest('foo')
-      sha2 = Digest::SHA1.hexdigest('bar')
 
       create(:merge_request_diff_commit, merge_request_diff: mr1_diff, sha: sha1)
       create(:merge_request_diff_commit, merge_request_diff: mr2_diff, sha: sha1)
@@ -22,11 +30,16 @@ RSpec.describe MergeRequests::OldestPerCommitFinder do
         relative_order: 1
       )
 
-      commits = [double(:commit, id: sha1), double(:commit, id: sha2)]
+      commits = [
+        double(:commit, id: sha1),
+        double(:commit, id: sha2),
+        double(:commit, id: sha3)
+      ]
 
       expect(described_class.new(project).execute(commits)).to eq(
         sha1 => mr1,
-        sha2 => mr2
+        sha2 => mr2,
+        sha3 => mr3
       )
     end
 
@@ -42,5 +55,45 @@ RSpec.describe MergeRequests::OldestPerCommitFinder do
       expect(described_class.new(mr.target_project).execute(commits))
         .to be_empty
     end
+
+    it 'includes the merge request for a merge commit' do
+      project = create(:project)
+      sha = Digest::SHA1.hexdigest('foo')
+      mr = create(
+        :merge_request,
+        :merged,
+        target_project: project,
+        merge_commit_sha: sha
+      )
+
+      commits = [double(:commit, id: sha)]
+
+      # This expectation is set so we're certain that the merge commit SHAs (if
+      # a matching merge request is found) aren't also used for finding MRs
+      # according to diffs.
+      expect(MergeRequestDiffCommit)
+        .not_to receive(:oldest_merge_request_id_per_commit)
+
+      expect(described_class.new(project).execute(commits)).to eq(sha => mr)
+    end
+
+    it 'includes the oldest merge request when a merge commit is present in a newer merge request' do
+      project = create(:project)
+      sha = Digest::SHA1.hexdigest('foo')
+      mr1 = create(
+        :merge_request,
+        :merged,
+        target_project: project, merge_commit_sha: sha
+      )
+
+      mr2 = create(:merge_request, :merged, target_project: project)
+      mr_diff = create(:merge_request_diff, merge_request: mr2)
+
+      create(:merge_request_diff_commit, merge_request_diff: mr_diff, sha: sha)
+
+      commits = [double(:commit, id: sha)]
+
+      expect(described_class.new(project).execute(commits)).to eq(sha => mr1)
+    end
   end
 end

spec/services/repositories/changelog_service_spec.rb

@@ -76,7 +76,7 @@ RSpec.describe Repositories::ChangelogService do
       recorder = ActiveRecord::QueryRecorder.new { service.execute }
       changelog = project.repository.blob_at('master', 'CHANGELOG.md')&.data
 
-      expect(recorder.count).to eq(10)
+      expect(recorder.count).to eq(11)
       expect(changelog).to include('Title 1', 'Title 2')
     end