Commit f8b53ba2 authored by Paco Guzman's avatar Paco Guzman

Recover usage of Todos counter cache

We’re being kept up to date the counter data but we’re not using it.
The only thing which is not real if is the number of projects that the 
user read changes the number of todos can be stale for some time.

The counters will be sync just after the user receives a new todo or mark any as done
parent 1f225354
...@@ -37,8 +37,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -37,8 +37,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController
def todos_counts def todos_counts
{ {
count: TodosFinder.new(current_user, state: :pending).execute.count, count: current_user.todos_pending_count,
done_count: TodosFinder.new(current_user, state: :done).execute.count done_count: current_user.todos_done_count
} }
end end
end end
module TodosHelper module TodosHelper
def todos_pending_count def todos_pending_count
@todos_pending_count ||= TodosFinder.new(current_user, state: :pending).execute.count @todos_pending_count ||= current_user.todos_pending_count
end end
def todos_done_count def todos_done_count
@todos_done_count ||= TodosFinder.new(current_user, state: :done).execute.count @todos_done_count ||= current_user.todos_done_count
end end
def todo_action_name(todo) def todo_action_name(todo)
......
...@@ -144,8 +144,9 @@ class TodoService ...@@ -144,8 +144,9 @@ class TodoService
def mark_todos_as_done(todos, current_user) def mark_todos_as_done(todos, current_user)
todos = current_user.todos.where(id: todos.map(&:id)) unless todos.respond_to?(:update_all) todos = current_user.todos.where(id: todos.map(&:id)) unless todos.respond_to?(:update_all)
todos.update_all(state: :done) marked_todos = todos.update_all(state: :done)
current_user.update_todos_count_cache current_user.update_todos_count_cache
marked_todos
end end
# When user marks an issue as todo # When user marks an issue as todo
......
...@@ -61,9 +61,9 @@ module API ...@@ -61,9 +61,9 @@ module API
# #
delete ':id' do delete ':id' do
todo = current_user.todos.find(params[:id]) todo = current_user.todos.find(params[:id])
todo.done TodoService.new.mark_todos_as_done([todo], current_user)
present todo, with: Entities::Todo, current_user: current_user present todo.reload, with: Entities::Todo, current_user: current_user
end end
# Mark all todos as done # Mark all todos as done
...@@ -74,8 +74,6 @@ module API ...@@ -74,8 +74,6 @@ module API
delete do delete do
todos = find_todos todos = find_todos
TodoService.new.mark_todos_as_done(todos, current_user) TodoService.new.mark_todos_as_done(todos, current_user)
todos.length
end end
end end
end end
......
...@@ -472,6 +472,42 @@ describe TodoService, services: true do ...@@ -472,6 +472,42 @@ describe TodoService, services: true do
expect(john_doe.todos_pending_count).to eq(1) expect(john_doe.todos_pending_count).to eq(1)
end end
describe '#mark_todos_as_done' do
let(:issue) { create(:issue, project: project, author: author, assignee: john_doe) }
it 'marks a relation of todos as done' do
create(:todo, :mentioned, user: john_doe, target: issue, project: project)
todos = TodosFinder.new(john_doe, {}).execute
expect { TodoService.new.mark_todos_as_done(todos, john_doe) }
.to change { john_doe.todos.done.count }.from(0).to(1)
end
it 'marks an array of todos as done' do
todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project)
expect { TodoService.new.mark_todos_as_done([todo], john_doe) }
.to change { todo.reload.state }.from('pending').to('done')
end
it 'returns the number of updated todos' do # Needed on API
todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project)
expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq(1)
end
it 'caches the number of todos of a user', :caching do
create(:todo, :mentioned, user: john_doe, target: issue, project: project)
todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project)
TodoService.new.mark_todos_as_done([todo], john_doe)
expect_any_instance_of(TodosFinder).not_to receive(:execute)
expect(john_doe.todos_done_count).to eq(1)
expect(john_doe.todos_pending_count).to eq(1)
end
end
def should_create_todo(attributes = {}) def should_create_todo(attributes = {})
attributes.reverse_merge!( attributes.reverse_merge!(
project: project, project: project,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment