diff options
| -rw-r--r-- | app/controllers/projects/issues_controller.rb | 4 | ||||
| -rw-r--r-- | app/models/project_team.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-optimize-discussion-json.yml | 5 | ||||
| -rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 32 |
4 files changed, 40 insertions, 3 deletions
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 8990c919ca0..42bfa4b9d4f 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -87,9 +87,9 @@ class Projects::IssuesController < Projects::ApplicationController .inc_relations_for_view .includes(:noteable) .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) diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 09049824ff7..1d35426050e 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -146,7 +146,7 @@ class ProjectTeam def member?(user, min_access_level = Gitlab::Access::GUEST) return false unless user - user.authorized_project?(project, min_access_level) + max_member_access(user.id) >= min_access_level end def human_max_access(user_id) diff --git a/changelogs/unreleased/sh-optimize-discussion-json.yml b/changelogs/unreleased/sh-optimize-discussion-json.yml new file mode 100644 index 00000000000..4be1bc89a91 --- /dev/null +++ b/changelogs/unreleased/sh-optimize-discussion-json.yml @@ -0,0 +1,5 @@ +--- +title: Eliminate N+1 queries in loading discussions.json endpoint +merge_request: +author: +type: fixed diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 5d9403c23ac..b4a22a46b51 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -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]) 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 |