Remove the Commit#merge_requests method

This is used in just one place, a metrics helper, but that is reaching
into both Note and Commit to build the list anyway.

By moving the logic into Note, we can clean up Commit significantly and
remove the transitive dependency on `repo_type`.

app/models/commit.rb

@@ -291,14 +291,6 @@ class Commit
     notes.includes(:author, :award_emoji)
   end
 
-  def merge_requests
-    strong_memoize(:merge_requests) do
-      next MergeRequest.none unless repository.repo_type.project? && project
-
-      project.merge_requests.by_commit_sha(sha)
-    end
-  end
-
   def method_missing(method, *args, &block)
     @raw.__send__(method, *args, &block) # rubocop:disable GitlabSecurity/PublicSend
   end

app/models/merge_request.rb

@@ -257,6 +257,10 @@ class MergeRequest < ApplicationRecord
     with_state(:opened).where(auto_merge_enabled: true)
   end
 
+  scope :including_metrics, -> do
+    includes(:metrics)
+  end
+
   ignore_column :state, remove_with: '12.7', remove_after: '2019-12-22'
 
   after_save :keep_around_commit, unless: :importing?

app/models/note.rb

@@ -290,6 +290,19 @@ class Note < ApplicationRecord
     @commit ||= project.commit(commit_id) if commit_id.present?
   end
 
+  # Notes on merge requests and commits can be traced back to one or several
+  # MRs. This method returns a relation if the note is for one of these types,
+  # or nil if it is a note on some other object.
+  def merge_requests
+    if for_commit?
+      project.merge_requests.by_commit_sha(commit_id)
+    elsif for_merge_request?
+      MergeRequest.id_in(noteable_id)
+    else
+      nil
+    end
+  end
+
   # override to return commits, which are not active record
   def noteable
     return commit if for_commit?

ee/lib/analytics/refresh_comments_data.rb

@@ -4,19 +4,13 @@ module Analytics
   class RefreshCommentsData
     include MergeRequestMetricsRefresh
 
-    # rubocop: disable CodeReuse/ActiveRecord
-    def self.for_note(note)
-      if note.for_commit?
-        merge_requests = note.noteable.merge_requests.includes(:metrics)
-      elsif note.for_merge_request?
-        merge_requests = [note.noteable]
-      else
-        return
-      end
+    class << self
+      def for_note(note)
+        merge_requests = note.merge_requests&.including_metrics
 
-      new(*merge_requests)
+        new(*merge_requests) unless merge_requests.nil?
+      end
     end
-    # rubocop: enable CodeReuse/ActiveRecord
 
     private
 

spec/models/commit_spec.rb

@@ -720,32 +720,6 @@ eos
     end
   end
 
-  describe '#merge_requests' do
-    let!(:project) { create(:project, :repository) }
-    let!(:merge_request1) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') }
-    let!(:merge_request2) { create(:merge_request, source_project: project, source_branch: 'merged-target', target_branch: 'feature') }
-    let(:commit1) { merge_request1.merge_request_diff.commits.last }
-    let(:commit2) { merge_request1.merge_request_diff.commits.first }
-
-    it 'returns merge_requests that introduced that commit' do
-      expect(commit1.merge_requests).to contain_exactly(merge_request1, merge_request2)
-      expect(commit2.merge_requests).to contain_exactly(merge_request1)
-    end
-
-    context 'with personal snippet' do
-      it 'returns empty relation' do
-        expect(personal_snippet.repository.commit.merge_requests).to eq MergeRequest.none
-      end
-    end
-
-    context 'with project snippet' do
-      it 'returns empty relation' do
-        expect(project_snippet.project).not_to receive(:merge_requests)
-        expect(project_snippet.repository.commit.merge_requests).to eq MergeRequest.none
-      end
-    end
-  end
-
   describe 'signed commits' do
     let(:gpg_signed_commit) { project.commit_by(oid: '0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
     let(:x509_signed_commit) { project.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') }