diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index d0e84b1aa38..f2c246cd969 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -390,7 +390,11 @@ class ProjectPolicy < BasePolicy greedy_load_subject ||= !@user.persisted? if greedy_load_subject - project.team.members.include?(user) + # We want to load all the members with one query. Calling #include? on + # project.team.members will perform a separate query for each user, unless + # project.team.members was loaded before somewhere else. Calling #to_a + # ensures it's always loaded before checking for membership. + project.team.members.to_a.include?(user) else # otherwise we just make a specific query for # this particular user. diff --git a/spec/services/notification_recipient_service_spec.rb b/spec/services/notification_recipient_service_spec.rb index 9a24821860e..cea5ea125b9 100644 --- a/spec/services/notification_recipient_service_spec.rb +++ b/spec/services/notification_recipient_service_spec.rb @@ -10,8 +10,24 @@ describe NotificationRecipientService do let(:issue) { create(:issue, project: project, assignees: [assignee]) } let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id) } + shared_examples 'no N+1 queries' do + it 'avoids N+1 queries', :request_store do + create_user + + service.build_new_note_recipients(note) + + control_count = ActiveRecord::QueryRecorder.new do + service.build_new_note_recipients(note) + end + + create_user + + expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count) + end + end + context 'when there are multiple watchers' do - def create_watcher + def create_user watcher = create(:user) create(:notification_setting, source: project, user: watcher, level: :watch) @@ -20,39 +36,23 @@ describe NotificationRecipientService do end end - it 'avoids N+1 queries', :request_store do - create_watcher - - service.build_new_note_recipients(note) - - control_count = ActiveRecord::QueryRecorder.new do - service.build_new_note_recipients(note) - end - - create_watcher - - expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count) - end + include_examples 'no N+1 queries' end context 'when there are multiple subscribers' do - def create_subscriber + def create_user subscriber = create(:user) issue.subscriptions.create(user: subscriber, project: project, subscribed: true) end - it 'avoids N+1 queries' do - create_subscriber + include_examples 'no N+1 queries' - service.build_new_note_recipients(note) - - control_count = ActiveRecord::QueryRecorder.new do - service.build_new_note_recipients(note) + context 'when the project is private' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) end - create_subscriber - - expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count) + include_examples 'no N+1 queries' end end end