diff options
author | Sean McGivern <sean@gitlab.com> | 2017-12-08 12:17:22 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2017-12-18 12:23:00 +0000 |
commit | ef454f68e837e4e7360fe1518686dd56adbbb0a9 (patch) | |
tree | 72e67bbf1a2222d8a1007298478350b5cc9c02bd /app | |
parent | 9429e8ac60a10436a0469d7d206d3f74a2c966c7 (diff) | |
download | gitlab-ce-ef454f68e837e4e7360fe1518686dd56adbbb0a9.tar.gz |
Reset todo counters when the target is deleted40871-todo-notification-count-shows-notification-without-having-a-todo
When the target is deleted, todos are destroyed, but we did not reset the todo
cache for users with todos on the deleted target. This would only update after
the next time the todo cache was updated for that user.
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/concerns/issuable_actions.rb | 1 | ||||
-rw-r--r-- | app/services/issuable/destroy_service.rb | 6 | ||||
-rw-r--r-- | app/services/notes/destroy_service.rb | 4 | ||||
-rw-r--r-- | app/services/todo_service.rb | 16 |
4 files changed, 19 insertions, 8 deletions
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 744e448e8df..141c34ee5ee 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -55,7 +55,6 @@ module IssuableActions def destroy Issuable::DestroyService.new(issuable.project, current_user).execute(issuable) - TodoService.new.destroy_issuable(issuable, current_user) name = issuable.human_class_name flash[:notice] = "The #{name} was successfully deleted." diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb index 0610b401213..7197a426a72 100644 --- a/app/services/issuable/destroy_service.rb +++ b/app/services/issuable/destroy_service.rb @@ -1,8 +1,10 @@ module Issuable class DestroyService < IssuableBaseService def execute(issuable) - if issuable.destroy - issuable.update_project_counter_caches + TodoService.new.destroy_target(issuable) do |issuable| + if issuable.destroy + issuable.update_project_counter_caches + end end end end diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index b819bd17039..fb78420d324 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -1,7 +1,9 @@ module Notes class DestroyService < BaseService def execute(note) - note.destroy + TodoService.new.destroy_target(note) do |note| + note.destroy + end end end end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 575853fd66b..c2ca404b179 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -31,12 +31,20 @@ class TodoService mark_pending_todos_as_done(issue, current_user) end - # When we destroy an issuable we should: + # When we destroy a todo target we should: # - # * refresh the todos count cache for the current user + # * refresh the todos count cache for all users with todos on the target # - def destroy_issuable(issuable, user) - user.update_todos_count_cache + # This needs to yield back to the caller to destroy the target, because it + # collects the todo users before the todos themselves are deleted, then + # updates the todo counts for those users. + # + def destroy_target(target) + todo_users = User.where(id: target.todos.pending.select(:user_id)).to_a + + yield target + + todo_users.each(&:update_todos_count_cache) end # When we reassign an issue we should: |