Stop serialising project when removing todos

`Todos::Destroy::EntityLeaveService#project_ids` was returning
ActiveRecord objects with IDs, not simply IDs. That means we were
serialising more than we needed to in Sidekiq.

We can simply rename this method to `#projects` as this class doesn't
use any of the superclass methods that would use `#project_ids`.
This commit is contained in:
Sean McGivern 2019-04-29 14:42:16 +01:00
parent d232137aa7
commit 9bc3dfea4f
2 changed files with 20 additions and 7 deletions

View File

@ -37,8 +37,8 @@ module Todos
private private
def enqueue_private_features_worker def enqueue_private_features_worker
project_ids.each do |project_id| projects.each do |project|
TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user.id) TodosDestroyer::PrivateFeaturesWorker.perform_async(project.id, user.id)
end end
end end
@ -62,9 +62,8 @@ module Todos
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
override :project_ids
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def project_ids def projects
condition = case entity condition = case entity
when Project when Project
{ id: entity.id } { id: entity.id }
@ -72,13 +71,13 @@ module Todos
{ namespace_id: non_member_groups } { namespace_id: non_member_groups }
end end
Project.where(condition).select(:id) Project.where(condition)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def non_authorized_projects def non_authorized_projects
project_ids.where('id NOT IN (?)', user.authorized_projects.select(:id)) projects.where('id NOT IN (?)', user.authorized_projects.select(:id))
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
@ -110,7 +109,7 @@ module Todos
authorized_reporter_projects = user authorized_reporter_projects = user
.authorized_projects(Gitlab::Access::REPORTER).select(:id) .authorized_projects(Gitlab::Access::REPORTER).select(:id)
Issue.where(project_id: project_ids, confidential: true) Issue.where(project_id: projects, confidential: true)
.where('project_id NOT IN(?)', authorized_reporter_projects) .where('project_id NOT IN(?)', authorized_reporter_projects)
.where('author_id != ?', user.id) .where('author_id != ?', user.id)
.where('id NOT IN (?)', assigned_ids) .where('id NOT IN (?)', assigned_ids)

View File

@ -75,6 +75,13 @@ describe Todos::Destroy::EntityLeaveService do
project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
end end
it 'enqueues the PrivateFeaturesWorker' do
expect(TodosDestroyer::PrivateFeaturesWorker)
.to receive(:perform_async).with(project.id, user.id)
subject
end
context 'confidential issues' do context 'confidential issues' do
context 'when a user is not an author of confidential issue' do context 'when a user is not an author of confidential issue' do
it 'removes only confidential issues todos' do it 'removes only confidential issues todos' do
@ -246,6 +253,13 @@ describe Todos::Destroy::EntityLeaveService do
project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
end end
it 'enqueues the PrivateFeaturesWorker' do
expect(TodosDestroyer::PrivateFeaturesWorker)
.to receive(:perform_async).with(project.id, user.id)
subject
end
context 'when user is not member' do context 'when user is not member' do
it 'removes only confidential issues todos' do it 'removes only confidential issues todos' do
expect { subject }.to change { Todo.count }.from(5).to(4) expect { subject }.to change { Todo.count }.from(5).to(4)