Commit 9b137533 authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'sh-optimize-discussion-json' into 'master'

Eliminate N+1 queries in loading discussions.json endpoint

Closes #37955

See merge request gitlab-org/gitlab-ce!14327
parents cffb325e 8690ca5c
...@@ -87,9 +87,9 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -87,9 +87,9 @@ class Projects::IssuesController < Projects::ApplicationController
.inc_relations_for_view .inc_relations_for_view
.includes(:noteable) .includes(:noteable)
.fresh .fresh
.reject { |n| n.cross_reference_not_visible_for?(current_user) }
prepare_notes_for_rendering(notes) notes = prepare_notes_for_rendering(notes)
notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
discussions = Discussion.build_collection(notes, @issue) discussions = Discussion.build_collection(notes, @issue)
......
...@@ -146,7 +146,7 @@ class ProjectTeam ...@@ -146,7 +146,7 @@ class ProjectTeam
def member?(user, min_access_level = Gitlab::Access::GUEST) def member?(user, min_access_level = Gitlab::Access::GUEST)
return false unless user return false unless user
user.authorized_project?(project, min_access_level) max_member_access(user.id) >= min_access_level
end end
def human_max_access(user_id) def human_max_access(user_id)
......
---
title: Eliminate N+1 queries in loading discussions.json endpoint
merge_request:
author:
type: fixed
...@@ -900,5 +900,37 @@ describe Projects::IssuesController do ...@@ -900,5 +900,37 @@ describe Projects::IssuesController do
expect(JSON.parse(response.body).first.keys).to match_array(%w[id reply_id expanded notes individual_note]) expect(JSON.parse(response.body).first.keys).to match_array(%w[id reply_id expanded notes individual_note])
end end
context 'with cross-reference system note', :request_store do
let(:new_issue) { create(:issue) }
let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" }
before do
create(:discussion_note_on_issue, :system, noteable: issue, project: issue.project, note: cross_reference)
end
it 'filters notes that the user should not see' do
get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid
expect(JSON.parse(response.body).count).to eq(1)
end
it 'does not result in N+1 queries' do
# Instantiate the controller variables to ensure QueryRecorder has an accurate base count
get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid
RequestStore.clear!
control_count = ActiveRecord::QueryRecorder.new do
get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid
end.count
RequestStore.clear!
create_list(:discussion_note_on_issue, 2, :system, noteable: issue, project: issue.project, note: cross_reference)
expect { get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid }.not_to exceed_query_limit(control_count)
end
end
end end
end end
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