From 9bc3dfea4f365d5dc0b93a5fef0418dfad971361 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Mon, 29 Apr 2019 14:42:16 +0100
Subject: [PATCH] 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`.
---
 app/services/todos/destroy/entity_leave_service.rb | 13 ++++++-------
 .../todos/destroy/entity_leave_service_spec.rb     | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb
index ebfb20132d0..4743e9b02ce 100644
--- a/app/services/todos/destroy/entity_leave_service.rb
+++ b/app/services/todos/destroy/entity_leave_service.rb
@@ -37,8 +37,8 @@ module Todos
       private
 
       def enqueue_private_features_worker
-        project_ids.each do |project_id|
-          TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user.id)
+        projects.each do |project|
+          TodosDestroyer::PrivateFeaturesWorker.perform_async(project.id, user.id)
         end
       end
 
@@ -62,9 +62,8 @@ module Todos
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      override :project_ids
       # rubocop: disable CodeReuse/ActiveRecord
-      def project_ids
+      def projects
         condition = case entity
                     when Project
                       { id: entity.id }
@@ -72,13 +71,13 @@ module Todos
                       { namespace_id: non_member_groups }
                     end
 
-        Project.where(condition).select(:id)
+        Project.where(condition)
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
       # rubocop: disable CodeReuse/ActiveRecord
       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
       # rubocop: enable CodeReuse/ActiveRecord
 
@@ -110,7 +109,7 @@ module Todos
         authorized_reporter_projects = user
           .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('author_id != ?', user.id)
           .where('id NOT IN (?)', assigned_ids)
diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb
index 1447b9d4126..2a553e18807 100644
--- a/spec/services/todos/destroy/entity_leave_service_spec.rb
+++ b/spec/services/todos/destroy/entity_leave_service_spec.rb
@@ -75,6 +75,13 @@ describe Todos::Destroy::EntityLeaveService do
           project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
         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 'when a user is not an author of confidential issue' do
             it 'removes only confidential issues todos' do
@@ -246,6 +253,13 @@ describe Todos::Destroy::EntityLeaveService do
           project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
         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
           it 'removes only confidential issues todos' do
             expect { subject }.to change { Todo.count }.from(5).to(4)