Merge branch 'id-n-1-for-commits-notes' into 'master'

Resolve N + 1 for commits notes API

See merge request gitlab-org/gitlab!57641

app/models/note.rb

@@ -109,6 +109,7 @@ class Note < ApplicationRecord
   scope :with_updated_at, ->(time) { where(updated_at: time) }
   scope :inc_author_project, -> { includes(:project, :author) }
   scope :inc_author, -> { includes(:author) }
+  scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) }
   scope :inc_relations_for_view, -> do
     includes(:project, { author: :status }, :updated_by, :resolved_by, :award_emoji,
              { system_note_metadata: :description_version }, :note_diff_file, :diff_note_positions, :suggestions)

changelogs/unreleased/id-n-1-for-commits-notes.yml

+---
+title: Resolve N + 1 for commits notes API
+merge_request: 57641
+author:
+type: performance

lib/api/commits.rb

@@ -186,16 +186,14 @@ module API
         use :pagination
         requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag'
       end
-      # rubocop: disable CodeReuse/ActiveRecord
       get ':id/repository/commits/:sha/comments', requirements: API::COMMIT_ENDPOINT_REQUIREMENTS do
         commit = user_project.commit(params[:sha])
 
         not_found! 'Commit' unless commit
-        notes = commit.notes.order(:created_at)
+        notes = commit.notes.with_api_entity_associations.fresh
 
         present paginate(notes), with: Entities::CommitNote
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       desc 'Cherry pick commit into a branch' do
         detail 'This feature was introduced in GitLab 8.15'

spec/requests/api/commits_spec.rb

@@ -1439,6 +1439,22 @@ RSpec.describe API::Commits do
           it_behaves_like 'ref comments'
         end
       end
+
+      context 'multiple notes' do
+        let!(:note) { create(:diff_note_on_commit, project: project) }
+        let(:commit) { note.commit }
+        let(:commit_id) { note.commit_id }
+
+        it 'are returned without N + 1' do
+          get api(route, current_user) # warm up the cache
+
+          control_count = ActiveRecord::QueryRecorder.new { get api(route, current_user) }.count
+
+          create(:diff_note_on_commit, project: project, author: create(:user))
+
+          expect { get api(route, current_user) }.not_to exceed_query_limit(control_count)
+        end
+      end
     end
 
     context 'when the commit is present on two projects' do