Merge branch '66701-done-quick-action-is-available-even-no-To-Do-exists' into 'master'

Refactored todo model to check only for pending todos

See merge request gitlab-org/gitlab!16837

app/finders/todos_finder.rb

@@ -47,11 +47,12 @@ class TodosFinder
     sort(items)
   end
 
-  # Returns `true` if the current user has any todos for the given target.
+  # Returns `true` if the current user has any todos for the given target with the optional given state.
   #
   # target - The value of the `target_type` column, such as `Issue`.
-  def any_for_target?(target)
-    current_user.todos.any_for_target?(target)
+  # state - The value of the `state` column, such as `pending` or `done`.
+  def any_for_target?(target, state = nil)
+    current_user.todos.any_for_target?(target, state)
   end
 
   private

app/models/todo.rb

@@ -89,11 +89,12 @@ class Todo < ApplicationRecord
       ])
     end
 
-    # Returns `true` if the current user has any todos for the given target.
+    # Returns `true` if the current user has any todos for the given target with the optional given state.
     #
     # target - The value of the `target_type` column, such as `Issue`.
-    def any_for_target?(target)
-      exists?(target: target)
+    # state - The value of the `state` column, such as `pending` or `done`.
+    def any_for_target?(target, state = nil)
+      state.nil? ? exists?(target: target) : exists?(target: target, state: state)
     end
 
     # Updates the state of a relation of todos to the new state.

app/services/todo_service.rb

@@ -191,7 +191,7 @@ class TodoService
   end
 
   def todo_exist?(issuable, current_user)
-    TodosFinder.new(current_user).any_for_target?(issuable)
+    TodosFinder.new(current_user).any_for_target?(issuable, :pending)
   end
 
   private

changelogs/unreleased/66701-done-quick-action-is-available-even-no-To-Do-exists.yml

+---
+title: Changed todo/done quick actions to work not only for first usage
+merge_request: 16837
+author: Marc Schwede
+type: fixed

spec/models/todo_spec.rb

@@ -273,6 +273,22 @@ describe Todo do
       expect(described_class.any_for_target?(todo.target)).to eq(true)
     end
 
+    it 'returns true if there is at least one todo for a given target with state pending' do
+      issue = create(:issue)
+      create(:todo, state: :done, target: issue)
+      create(:todo, state: :pending, target: issue)
+
+      expect(described_class.any_for_target?(issue)).to eq(true)
+    end
+
+    it 'returns false if there are only todos for a given target with state done while searching for pending' do
+      issue = create(:issue)
+      create(:todo, state: :done, target: issue)
+      create(:todo, state: :done, target: issue)
+
+      expect(described_class.any_for_target?(issue, :pending)).to eq(false)
+    end
+
     it 'returns false if there are no todos for a given target' do
       issue = create(:issue)