Commit 3b78576e authored by Jan Provaznik's avatar Jan Provaznik

Fix N+1 issue in note discussions

When rendering discussions, we refer back to note.discussion, but
because this association is not set, we find discussion by id
again for each note. TO avoid this, note's discussion is preset
when discussion instance is created.
parent fe2022ba
...@@ -2,7 +2,13 @@ ...@@ -2,7 +2,13 @@
class DiscussionEntity < BaseDiscussionEntity class DiscussionEntity < BaseDiscussionEntity
expose :notes do |discussion, opts| expose :notes do |discussion, opts|
request.note_entity.represent(discussion.notes, opts.merge(with_base_discussion: false)) request.note_entity.represent(
discussion.notes,
opts.merge(
with_base_discussion: false,
discussion: discussion
)
)
end end
expose :positions, if: -> (d, _) { display_merge_ref_discussions?(d) } do |discussion| expose :positions, if: -> (d, _) { display_merge_ref_discussions?(d) } do |discussion|
......
...@@ -36,7 +36,8 @@ class NoteEntity < API::Entities::Note ...@@ -36,7 +36,8 @@ class NoteEntity < API::Entities::Note
end end
expose :can_resolve_discussion do |note| expose :can_resolve_discussion do |note|
note.discussion.resolvable? && note.discussion.can_resolve?(current_user) discussion = options.fetch(:discussion, nil) || note.discussion
discussion.resolvable? && discussion.can_resolve?(current_user)
end end
end end
......
---
title: Fix N+1 issue when loading merge request comments
merge_request: 57374
author:
type: performance
...@@ -26,6 +26,10 @@ RSpec.describe 'merge requests discussions' do ...@@ -26,6 +26,10 @@ RSpec.describe 'merge requests discussions' do
# https://docs.gitlab.com/ee/development/query_recorder.html#use-request-specs-instead-of-controller-specs # https://docs.gitlab.com/ee/development/query_recorder.html#use-request-specs-instead-of-controller-specs
it 'avoids N+1 DB queries', :request_store do it 'avoids N+1 DB queries', :request_store do
send_request # warm up
create(:diff_note_on_merge_request, noteable: merge_request,
project: merge_request.project)
control = ActiveRecord::QueryRecorder.new { send_request } control = ActiveRecord::QueryRecorder.new { send_request }
create(:diff_note_on_merge_request, noteable: merge_request, create(:diff_note_on_merge_request, noteable: merge_request,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment