Merge branch...

Merge branch '444592-investigate-and-reduce-n-1-sql-queries-when-publishing-multiple-draft-notes-2' into 'master' 

Reduce N+1 queries when publishing draft notes

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/152323



Merged-by: Thong Kuah <tkuah@gitlab.com>
Approved-by: Heinrich Lee Yu <heinrich@gitlab.com>
Approved-by: Thong Kuah <tkuah@gitlab.com>
Reviewed-by: Sincheol (David) Kim <dkim@gitlab.com>
Co-authored-by: David Kim <dkim@gitlab.com>

app/models/draft_note.rb

@@ -5,7 +5,7 @@ class DraftNote < ApplicationRecord
   include Sortable
   include ShaAttribute
 
-  PUBLISH_ATTRS = %i[noteable_id noteable_type type note internal].freeze
+  PUBLISH_ATTRS = %i[noteable type note internal].freeze
   DIFF_ATTRS = %i[position original_position change_position commit_id].freeze
 
   sha_attribute :commit_id

app/services/notes/build_service.rb

@@ -20,7 +20,11 @@ def execute
 
         discussion = discussion.convert_to_discussion! if discussion.can_convert_to_discussion?
 
-        params.merge!(discussion.reply_attributes)
+        reply_attributes = discussion.reply_attributes
+        # NOTE: Avoid overriding noteable if it already exists so that we don't have to reload noteable.
+        reply_attributes = reply_attributes.except(:noteable_id, :noteable_type) if params[:noteable]
+
+        params.merge!(reply_attributes)
       end
 
       # The `confidential` param for notes is deprecated with 15.3

spec/services/draft_notes/publish_service_spec.rb

@@ -224,19 +224,17 @@ def publish(draft: nil)
     end
   end
 
-  context 'with many draft notes', :use_sql_query_cache do
+  context 'with many draft notes', :use_sql_query_cache, :request_store do
     let(:merge_request) { create(:merge_request) }
 
     it 'reduce N+1 queries' do
-      create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 1')
-      create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 2')
-      create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 3')
-      create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 4')
-      create(:draft_note_on_text_diff, merge_request: merge_request, author: user, note: 'note 5')
+      5.times do
+        create(:draft_note_on_discussion, merge_request: merge_request, author: user, note: 'some note')
+      end
 
       recorder = ActiveRecord::QueryRecorder.new(skip_cached: false) { publish }
 
-      expect(recorder.count).not_to be > 186
+      expect(recorder.count).not_to be > 105
     end
   end
 
@@ -303,6 +301,7 @@ def update_file(file_path, new_content)
         refresh = MergeRequests::RefreshService.new(project: project, current_user: user)
         refresh.execute(oldrev, newrev, merge_request.source_branch_ref)
 
+        merge_request.reload
         expect { publish(draft: draft) }.to change { Suggestion.count }.by(1)
           .and change { DiffNote.count }.from(0).to(1)