Merge branch '327110-update-cached-count-for-done-todos-when-deleting-todo-targets' into 'master'

Update cached count for "done" todos when deleting todo targets

See merge request gitlab-org/gitlab!58773

app/finders/users_with_pending_todos_finder.rb

-# frozen_string_literal: true
-
-# Finder that given a target (e.g. an issue) finds all the users that have
-# pending todos for said target.
-class UsersWithPendingTodosFinder
-  attr_reader :target
-
-  # target - The target, such as an Issue or MergeRequest.
-  def initialize(target)
-    @target = target
-  end
-
-  def execute
-    User.for_todos(target.todos.pending)
-  end
-end

app/models/user.rb

@@ -376,7 +376,7 @@ class User < ApplicationRecord
   scope :by_name, -> (names) { iwhere(name: Array(names)) }
   scope :by_user_email, -> (emails) { iwhere(email: Array(emails)) }
   scope :by_emails, -> (emails) { joins(:emails).where(emails: { email: Array(emails).map(&:downcase) }) }
-  scope :for_todos, -> (todos) { where(id: todos.select(:user_id)) }
+  scope :for_todos, -> (todos) { where(id: todos.select(:user_id).distinct) }
   scope :with_emails, -> { preload(:emails) }
   scope :with_dashboard, -> (dashboard) { where(dashboard: dashboard) }
   scope :with_public_profile, -> { where(private_profile: false) }

app/services/todo_service.rb

@@ -43,7 +43,7 @@ class TodoService
   # updates the todo counts for those users.
   #
   def destroy_target(target)
-    todo_users = UsersWithPendingTodosFinder.new(target).execute.to_a
+    todo_users = User.for_todos(target.todos).to_a
 
     yield target
 

changelogs/unreleased/327110-update-cached-count-for-done-todos-when-deleting-todo-targets.yml

+---
+title: Update cached count for "done" todos when deleting todo targets
+merge_request: 58773
+author:
+type: fixed

spec/finders/users_with_pending_todos_finder_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe UsersWithPendingTodosFinder do
-  describe '#execute' do
-    it 'returns the users for all pending todos of a target' do
-      issue = create(:issue)
-      note = create(:note)
-      todo = create(:todo, :pending, target: issue)
-
-      create(:todo, :pending, target: note)
-
-      users = described_class.new(issue).execute
-
-      expect(users).to eq([todo.user])
-    end
-  end
-end

spec/models/user_spec.rb

@@ -1056,6 +1056,21 @@ RSpec.describe User do
           .to contain_exactly(user)
       end
     end
+
+    describe '.for_todos' do
+      let_it_be(:user1) { create(:user) }
+      let_it_be(:user2) { create(:user) }
+      let_it_be(:issue) { create(:issue) }
+
+      let_it_be(:todo1) { create(:todo, target: issue, author: user1, user: user1) }
+      let_it_be(:todo2) { create(:todo, target: issue, author: user1, user: user1) }
+      let_it_be(:todo3) { create(:todo, target: issue, author: user2, user: user2) }
+
+      it 'returns users for the given todos' do
+        expect(described_class.for_todos(issue.todos))
+          .to contain_exactly(user1, user2)
+      end
+    end
   end
 
   describe "Respond to" do

spec/services/todo_service_spec.rb

@@ -345,17 +345,10 @@ RSpec.describe TodoService do
 
     describe '#destroy_target' do
       it 'refreshes the todos count cache for users with todos on the target' do
-        create(:todo, state: :pending, target: issue, user: john_doe, author: john_doe, project: issue.project)
+        create(:todo, state: :pending, target: issue, user: author, author: author, project: issue.project)
+        create(:todo, state: :done, target: issue, user: assignee, author: assignee, project: issue.project)
 
-        expect_next(Users::UpdateTodoCountCacheService, [john_doe]).to receive(:execute)
-
-        service.destroy_target(issue) { issue.destroy! }
-      end
-
-      it 'does not refresh the todos count cache for users with only done todos on the target' do
-        create(:todo, :done, target: issue, user: john_doe, author: john_doe, project: issue.project)
-
-        expect(Users::UpdateTodoCountCacheService).not_to receive(:new)
+        expect_next(Users::UpdateTodoCountCacheService, [author, assignee]).to receive(:execute)
 
         service.destroy_target(issue) { issue.destroy! }
       end