From d86c6666623a90d46fa4cfe624c67e86c6ad235f Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 1 Sep 2016 19:12:05 -0300
Subject: [PATCH] Refresh todos count cache when an Issue/MR is deleted

---
 app/controllers/concerns/issuable_actions.rb  |  2 ++
 app/services/todo_service.rb                  | 20 +++++++++++++++++++
 .../projects/issues_controller_spec.rb        |  6 ++++++
 .../merge_requests_controller_spec.rb         |  6 ++++++
 spec/services/todo_service_spec.rb            | 16 +++++++++++++++
 5 files changed, 50 insertions(+)

diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 77b4efffd7f..bb32bc502e6 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -8,6 +8,8 @@ module IssuableActions
 
   def destroy
     issuable.destroy
+    destroy_method = "destroy_#{issuable.class.name.underscore}".to_sym
+    TodoService.new.public_send(destroy_method, issuable, current_user)
 
     name = issuable.class.name.titleize.downcase
     flash[:notice] = "The #{name} was successfully deleted."
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index 2aab8c736d6..776530ac0a5 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -31,6 +31,14 @@ class TodoService
     mark_pending_todos_as_done(issue, current_user)
   end
 
+  # When we destroy an issue we should:
+  #
+  #  * refresh the todos count cache for the current user
+  #
+  def destroy_issue(issue, current_user)
+    destroy_issuable(issue, current_user)
+  end
+
   # When we reassign an issue we should:
   #
   #  * create a pending todo for new assignee if issue is assigned
@@ -64,6 +72,14 @@ class TodoService
     mark_pending_todos_as_done(merge_request, current_user)
   end
 
+  # When we destroy a merge request we should:
+  #
+  #  * refresh the todos count cache for the current user
+  #
+  def destroy_merge_request(merge_request, current_user)
+    destroy_issuable(merge_request, current_user)
+  end
+
   # When we reassign a merge request we should:
   #
   #  * creates a pending todo for new assignee if merge request is assigned
@@ -187,6 +203,10 @@ class TodoService
     create_mention_todos(issuable.project, issuable, author)
   end
 
+  def destroy_issuable(issuable, user)
+    user.update_todos_count_cache
+  end
+
   def toggling_tasks?(issuable)
     issuable.previous_changes.include?('description') &&
       issuable.tasks? && issuable.updated_tasks.any?
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 16929767ddf..90419368f22 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -370,6 +370,12 @@ describe Projects::IssuesController do
         expect(response).to have_http_status(302)
         expect(controller).to set_flash[:notice].to(/The issue was successfully deleted\./).now
       end
+
+      it 'delegates the update of the todos count cache to TodoService' do
+        expect_any_instance_of(TodoService).to receive(:destroy_issue).with(issue, owner).once
+
+        delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: issue.iid
+      end
     end
   end
 
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index a219400d75f..94c9edc91fe 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -320,6 +320,12 @@ describe Projects::MergeRequestsController do
         expect(response).to have_http_status(302)
         expect(controller).to set_flash[:notice].to(/The merge request was successfully deleted\./).now
       end
+
+      it 'delegates the update of the todos count cache to TodoService' do
+        expect_any_instance_of(TodoService).to receive(:destroy_merge_request).with(merge_request, owner).once
+
+        delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
+      end
     end
   end
 
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index cafcad3e3c0..b41f6f14fbd 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -145,6 +145,14 @@ describe TodoService, services: true do
       end
     end
 
+    describe '#destroy_issue' do
+      it 'refresh the todos count cache for the user' do
+        expect(john_doe).to receive(:update_todos_count_cache).and_call_original
+
+        service.destroy_issue(issue, john_doe)
+      end
+    end
+
     describe '#reassigned_issue' do
       it 'creates a pending todo for new assignee' do
         unassigned_issue.update_attribute(:assignee, john_doe)
@@ -394,6 +402,14 @@ describe TodoService, services: true do
       end
     end
 
+    describe '#destroy_merge_request' do
+      it 'refresh the todos count cache for the user' do
+        expect(john_doe).to receive(:update_todos_count_cache).and_call_original
+
+        service.destroy_merge_request(mr_assigned, john_doe)
+      end
+    end
+
     describe '#reassigned_merge_request' do
       it 'creates a pending todo for new assignee' do
         mr_unassigned.update_attribute(:assignee, john_doe)