Only load issue notes once when getting related MRs

As we always call both methods from the controller - and elsewhere we call the
more general method - and one uses all notes and the other uses system notes,
then we should just load the notes and their authors once, and filter on the
Ruby side.
This commit is contained in:
Sean McGivern 2018-08-21 12:56:01 +01:00
parent 569069eed0
commit b26e5546c3
2 changed files with 17 additions and 4 deletions

View file

@ -14,7 +14,7 @@ module Issues
end end
def referenced_merge_requests(issue) def referenced_merge_requests(issue)
merge_requests = extract_merge_requests(issue, issue.notes) merge_requests = extract_merge_requests(issue)
cross_project_filter = -> (merge_requests) do cross_project_filter = -> (merge_requests) do
merge_requests.select { |mr| mr.target_project == project } merge_requests.select { |mr| mr.target_project == project }
@ -32,7 +32,7 @@ module Issues
def closed_by_merge_requests(issue) def closed_by_merge_requests(issue)
return [] unless issue.open? return [] unless issue.open?
merge_requests = extract_merge_requests(issue, issue.notes.system).select(&:open?) merge_requests = extract_merge_requests(issue, filter: :system).select(&:open?)
return [] if merge_requests.empty? return [] if merge_requests.empty?
@ -42,16 +42,23 @@ module Issues
private private
def extract_merge_requests(issue, notes) def extract_merge_requests(issue, filter: nil)
ext = issue.all_references(current_user) ext = issue.all_references(current_user)
notes = issue_notes(issue)
notes = notes.select(&filter) if filter
notes.includes(:author).each do |note| notes.each do |note|
note.all_references(current_user, extractor: ext) note.all_references(current_user, extractor: ext)
end end
ext.merge_requests ext.merge_requests
end end
def issue_notes(issue)
@issue_notes ||= {}
@issue_notes[issue] ||= issue.notes.includes(:author)
end
def sort_by_iid(merge_requests) def sort_by_iid(merge_requests)
Gitlab::IssuableSorter.sort(project, merge_requests) { |mr| mr.iid.to_s } Gitlab::IssuableSorter.sort(project, merge_requests) { |mr| mr.iid.to_s }
end end

View file

@ -65,6 +65,12 @@ describe Issues::ReferencedMergeRequestsService do
expect { service.execute(issue).each(&pipeline_routes) } expect { service.execute(issue).each(&pipeline_routes) }
.not_to exceed_query_limit(control_count) .not_to exceed_query_limit(control_count)
end end
it 'only loads issue notes once' do
expect(issue).to receive(:notes).once.and_call_original
service.execute(issue)
end
end end
end end