Commit f6bfa46d authored by Paco Guzman's avatar Paco Guzman

Cache todo counters (pending/done)

- 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
parent fcd9f906
...@@ -123,6 +123,7 @@ v 8.9.0 (unreleased) ...@@ -123,6 +123,7 @@ v 8.9.0 (unreleased)
- Update tanuki logo highlight/loading colors - Update tanuki logo highlight/loading colors
- Use Git cached counters for branches and tags on project page - Use Git cached counters for branches and tags on project page
- Filter parameters for request_uri value on instrumented transactions. - Filter parameters for request_uri value on instrumented transactions.
- Cache user todo counts from TodoService
v 8.8.5 v 8.8.5
- Import GitHub repositories respecting the API rate limit !4166 - Import GitHub repositories respecting the API rate limit !4166
......
...@@ -6,7 +6,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -6,7 +6,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
end end
def destroy def destroy
todo.done TodoService.new.mark_todos_as_done([todo], current_user)
todo_notice = 'Todo was successfully marked as done.' todo_notice = 'Todo was successfully marked as done.'
...@@ -14,20 +14,20 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -14,20 +14,20 @@ class Dashboard::TodosController < Dashboard::ApplicationController
format.html { redirect_to dashboard_todos_path, notice: todo_notice } format.html { redirect_to dashboard_todos_path, notice: todo_notice }
format.js { head :ok } format.js { head :ok }
format.json do 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 end
end end
def destroy_all def destroy_all
@todos.each(&:done) TodoService.new.mark_todos_as_done(@todos, current_user)
respond_to do |format| respond_to do |format|
format.html { redirect_to dashboard_todos_path, notice: 'All todos were marked as done.' } format.html { redirect_to dashboard_todos_path, notice: 'All todos were marked as done.' }
format.js { head :ok } format.js { head :ok }
format.json do format.json do
find_todos 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 end
end end
......
...@@ -4,7 +4,7 @@ class Projects::TodosController < Projects::ApplicationController ...@@ -4,7 +4,7 @@ class Projects::TodosController < Projects::ApplicationController
render json: { render json: {
todo: todos, todo: todos,
count: current_user.todos.pending.count, count: current_user.todos_pending_count,
} }
end end
...@@ -12,7 +12,7 @@ class Projects::TodosController < Projects::ApplicationController ...@@ -12,7 +12,7 @@ class Projects::TodosController < Projects::ApplicationController
current_user.todos.find_by_id(params[:id]).update(state: :done) current_user.todos.find_by_id(params[:id]).update(state: :done)
render json: { render json: {
count: current_user.todos.pending.count, count: current_user.todos_pending_count,
} }
end end
......
module TodosHelper module TodosHelper
def todos_pending_count def todos_pending_count
current_user.todos.pending.count current_user.todos_pending_count
end end
def todos_done_count def todos_done_count
current_user.todos.done.count current_user.todos_done_count
end end
def todo_action_name(todo) def todo_action_name(todo)
......
...@@ -827,6 +827,23 @@ class User < ActiveRecord::Base ...@@ -827,6 +827,23 @@ class User < ActiveRecord::Base
assigned_open_issues_count(force: true) assigned_open_issues_count(force: true)
end 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 private
def projects_union(min_access_level = nil) def projects_union(min_access_level = nil)
......
# TodoService class # TodoService class
# #
# Used for creating todos after certain user actions # Used for creating/updating todos after certain user actions
# #
# Ex. # Ex.
# TodoService.new.new_issue(issue, current_user) # TodoService.new.new_issue(issue, current_user)
...@@ -137,6 +137,15 @@ class TodoService ...@@ -137,6 +137,15 @@ class TodoService
def mark_pending_todos_as_done(target, user) def mark_pending_todos_as_done(target, user)
attributes = attributes_for_target(target) attributes = attributes_for_target(target)
pending_todos(user, attributes).update_all(state: :done) 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 end
# When user marks an issue as todo # When user marks an issue as todo
...@@ -151,6 +160,7 @@ class TodoService ...@@ -151,6 +160,7 @@ class TodoService
Array(users).map do |user| Array(users).map do |user|
next if pending_todos(user, attributes).exists? next if pending_todos(user, attributes).exists?
Todo.create(attributes.merge(user_id: user.id)) Todo.create(attributes.merge(user_id: user.id))
user.update_todos_count_cache
end end
end end
......
...@@ -173,6 +173,48 @@ describe TodoService, services: true do ...@@ -173,6 +173,48 @@ describe TodoService, services: true do
expect(first_todo.reload).to be_done expect(first_todo.reload).to be_done
expect(second_todo.reload).to be_done expect(second_todo.reload).to be_done
end 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 end
describe '#new_note' do describe '#new_note' do
...@@ -395,6 +437,18 @@ describe TodoService, services: true do ...@@ -395,6 +437,18 @@ describe TodoService, services: true do
end end
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 = {}) 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