Merge branch '36946-todo-mark-done-mutation-refactoring-pd' into 'master'

Use direct model update instead of using TodoService for GraphQL Todo API

See merge request gitlab-org/gitlab!20551

app/graphql/mutations/todos/mark_done.rb

@@ -16,22 +16,21 @@ module Mutations
             null: false,
             description: 'The requested todo'
 
-      # rubocop: disable CodeReuse/ActiveRecord
       def resolve(id:)
         todo = authorized_find!(id: id)
-        mark_done(Todo.where(id: todo.id)) unless todo.done?
+
+        mark_done(todo)
 
         {
           todo: todo.reset,
           errors: errors_on_object(todo)
         }
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       private
 
       def mark_done(todo)
-        TodoService.new.mark_todos_as_done(todo, current_user)
+        TodoService.new.mark_todo_as_done(todo, current_user)
       end
     end
   end

app/services/todo_service.rb

@@ -179,6 +179,14 @@ class TodoService
     mark_todos_as_done(todos, current_user)
   end
 
+  def mark_todo_as_done(todo, current_user)
+    return if todo.done?
+
+    todo.update(state: :done)
+
+    current_user.update_todos_count_cache
+  end
+
   # When user marks some todos as pending
   def mark_todos_as_pending(todos, current_user)
     update_todos_state(todos, current_user, :pending)

spec/services/todo_service_spec.rb

@@ -1015,6 +1015,30 @@ describe TodoService do
     end
   end
 
+  describe '#mark_todo_as_done' do
+    it 'marks a todo done' do
+      todo1 = create(:todo, :pending, user: john_doe)
+
+      described_class.new.mark_todo_as_done(todo1, john_doe)
+
+      expect(todo1.reload.state).to eq('done')
+    end
+
+    context 'when todo is already in state done' do
+      let(:todo1) { create(:todo, :done, user: john_doe) }
+
+      it 'does not update the todo' do
+        expect { described_class.new.mark_todo_as_done(todo1, john_doe) }.not_to change(todo1.reload, :state)
+      end
+
+      it 'does not update cache count' do
+        expect(john_doe).not_to receive(:update_todos_count_cache)
+
+        described_class.new.mark_todo_as_done(todo1, john_doe)
+      end
+    end
+  end
+
   describe '#mark_all_todos_as_done_by_user' do
     it 'marks all todos done' do
       todo1 = create(:todo, user: john_doe, state: :pending)