Cache todo counters (pending/done)18034-cache-todo-counter

- As todos are created/updated inside the TodoService
we repopulate the cache just there for both pending/done todos
- Todos as mark as done from the TodosController we update cache
there too
- All the added methods are kept in the User class for cohesion

7 files changed, 91 insertions, 9 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 620a7eea378..5657de69c3a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -123,6 +123,7 @@ v 8.9.0 (unreleased)
   - Update tanuki logo highlight/loading colors
   - Use Git cached counters for branches and tags on project page
   - Filter parameters for request_uri value on instrumented transactions.
+  - Cache user todo counts from TodoService
 
 v 8.8.5
   - Import GitHub repositories respecting the API rate limit !4166
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index f9a1929c117..7842fb9ce63 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -6,7 +6,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
   end
 
   def destroy
-    todo.done
+    TodoService.new.mark_todos_as_done([todo], current_user)
 
     todo_notice = 'Todo was successfully marked as done.'
 
@@ -14,20 +14,20 @@ class Dashboard::TodosController < Dashboard::ApplicationController
       format.html { redirect_to dashboard_todos_path, notice: todo_notice }
       format.js { head :ok }
       format.json do
-        render json: { count: @todos.size, done_count: current_user.todos.done.count }
+        render json: { count: @todos.size, done_count: current_user.todos_done_count }
       end
     end
   end
 
   def destroy_all
-    @todos.each(&:done)
+    TodoService.new.mark_todos_as_done(@todos, current_user)
 
     respond_to do |format|
       format.html { redirect_to dashboard_todos_path, notice: 'All todos were marked as done.' }
       format.js { head :ok }
       format.json do
         find_todos
-        render json: { count: @todos.size, done_count: current_user.todos.done.count }
+        render json: { count: @todos.size, done_count: current_user.todos_done_count }
       end
     end
   end
diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb
index a51bd5e2b49..648d42c56c5 100644
--- a/app/controllers/projects/todos_controller.rb
+++ b/app/controllers/projects/todos_controller.rb
@@ -4,7 +4,7 @@ class Projects::TodosController < Projects::ApplicationController
 
     render json: {
       todo: todos,
-      count: current_user.todos.pending.count,
+      count: current_user.todos_pending_count,
     }
   end
 
@@ -12,7 +12,7 @@ class Projects::TodosController < Projects::ApplicationController
     current_user.todos.find_by_id(params[:id]).update(state: :done)
 
     render json: {
-      count: current_user.todos.pending.count,
+      count: current_user.todos_pending_count,
     }
   end
 
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index 9adf5ef29f7..c7aeed4b9fc 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -1,10 +1,10 @@
 module TodosHelper
   def todos_pending_count
-    current_user.todos.pending.count
+    current_user.todos_pending_count
   end
 
   def todos_done_count
-    current_user.todos.done.count
+    current_user.todos_done_count
   end
 
   def todo_action_name(todo)
diff --git a/app/models/user.rb b/app/models/user.rb
index 051745fe252..2e458329cb9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -827,6 +827,23 @@ class User < ActiveRecord::Base
     assigned_open_issues_count(force: true)
   end
 
+  def todos_done_count(force: false)
+    Rails.cache.fetch(['users', id, 'todos_done_count'], force: force) do
+      todos.done.count
+    end
+  end
+
+  def todos_pending_count(force: false)
+    Rails.cache.fetch(['users', id, 'todos_pending_count'], force: force) do
+      todos.pending.count
+    end
+  end
+
+  def update_todos_count_cache
+    todos_done_count(force: true)
+    todos_pending_count(force: true)
+  end
+
   private
 
   def projects_union(min_access_level = nil)
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index 5c0c1ba27bf..540bf54b920 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -1,6 +1,6 @@
 # TodoService class
 #
-# Used for creating todos after certain user actions
+# Used for creating/updating todos after certain user actions
 #
 # Ex.
 #   TodoService.new.new_issue(issue, current_user)
@@ -137,6 +137,15 @@ class TodoService
   def mark_pending_todos_as_done(target, user)
     attributes = attributes_for_target(target)
     pending_todos(user, attributes).update_all(state: :done)
+    user.update_todos_count_cache
+  end
+
+  # When user marks some todos as done
+  def mark_todos_as_done(todos, current_user)
+    todos = current_user.todos.where(id: todos.map(&:id)) unless todos.respond_to?(:update_all)
+
+    todos.update_all(state: :done)
+    current_user.update_todos_count_cache
   end
 
   # When user marks an issue as todo
@@ -151,6 +160,7 @@ class TodoService
     Array(users).map do |user|
       next if pending_todos(user, attributes).exists?
       Todo.create(attributes.merge(user_id: user.id))
+      user.update_todos_count_cache
     end
   end
 
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index a09413692d9..b4522536724 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -173,6 +173,48 @@ describe TodoService, services: true do
         expect(first_todo.reload).to be_done
         expect(second_todo.reload).to be_done
       end
+
+      describe 'cached counts' do
+        it 'updates when todos change' do
+          create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author)
+
+          expect(john_doe.todos_done_count).to eq(0)
+          expect(john_doe.todos_pending_count).to eq(1)
+          expect(john_doe).to receive(:update_todos_count_cache).and_call_original
+
+          service.mark_pending_todos_as_done(issue, john_doe)
+
+          expect(john_doe.todos_done_count).to eq(1)
+          expect(john_doe.todos_pending_count).to eq(0)
+        end
+      end
+    end
+
+    describe '#mark_todos_as_done' do
+      it 'marks related todos for the user as done' do
+        first_todo = create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author)
+        second_todo = create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author)
+
+        service.mark_todos_as_done([first_todo, second_todo], john_doe)
+
+        expect(first_todo.reload).to be_done
+        expect(second_todo.reload).to be_done
+      end
+
+      describe 'cached counts' do
+        it 'updates when todos change' do
+          todo = create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author)
+
+          expect(john_doe.todos_done_count).to eq(0)
+          expect(john_doe.todos_pending_count).to eq(1)
+          expect(john_doe).to receive(:update_todos_count_cache).and_call_original
+
+          service.mark_todos_as_done([todo], john_doe)
+
+          expect(john_doe.todos_done_count).to eq(1)
+          expect(john_doe.todos_pending_count).to eq(0)
+        end
+      end
     end
 
     describe '#new_note' do
@@ -395,6 +437,18 @@ describe TodoService, services: true do
     end
   end
 
+  it 'updates cached counts when a todo is created' do
+    issue = create(:issue, project: project, assignee: john_doe, author: author, description: mentions)
+
+    expect(john_doe.todos_pending_count).to eq(0)
+    expect(john_doe).to receive(:update_todos_count_cache)
+
+    service.new_issue(issue, author)
+
+    expect(Todo.where(user_id: john_doe.id, state: :pending).count).to eq 1
+    expect(john_doe.todos_pending_count).to eq(1)
+  end
+
   def should_create_todo(attributes = {})
     attributes.reverse_merge!(
       project: project,