diff --git a/CHANGELOG b/CHANGELOG index 374bb153c56..e16f5c331e5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -125,6 +125,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,