diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/controllers/dashboard/todos_controller.rb | 4 | ||||
| -rw-r--r-- | app/helpers/todos_helper.rb | 4 | ||||
| -rw-r--r-- | app/models/user.rb | 4 | ||||
| -rw-r--r-- | app/services/todo_service.rb | 3 | ||||
| -rw-r--r-- | lib/api/todos.rb | 8 | ||||
| -rw-r--r-- | spec/requests/api/todos_spec.rb | 12 | ||||
| -rw-r--r-- | spec/services/todo_service_spec.rb | 36 |
8 files changed, 60 insertions, 12 deletions
diff --git a/CHANGELOG b/CHANGELOG index c6c2220b133..008c5c64284 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -275,6 +275,7 @@ v 8.10.0 - Fix new snippet style bug (elliotec) - Instrument Rinku usage - Be explicit to define merge request discussion variables + - Use cache for todos counter calling TodoService - Metrics for Rouge::Plugins::Redcarpet and Rouge::Formatters::HTMLGitlab - RailsCache metris now includes fetch_hit/fetch_miss and read_hit/read_miss info. - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w) diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 19a76a5b5d8..1243bb96d4d 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -37,8 +37,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController def todos_counts { - count: TodosFinder.new(current_user, state: :pending).execute.count, - done_count: TodosFinder.new(current_user, state: :done).execute.count + count: current_user.todos_pending_count, + done_count: current_user.todos_done_count } end end diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index e3a208f826a..0465327060e 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -1,10 +1,10 @@ module TodosHelper def todos_pending_count - @todos_pending_count ||= TodosFinder.new(current_user, state: :pending).execute.count + @todos_pending_count ||= current_user.todos_pending_count end def todos_done_count - @todos_done_count ||= TodosFinder.new(current_user, state: :done).execute.count + @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 73368be7b1b..87a2d999843 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -809,13 +809,13 @@ class User < ActiveRecord::Base def todos_done_count(force: false) Rails.cache.fetch(['users', id, 'todos_done_count'], force: force) do - todos.done.count + TodosFinder.new(self, state: :done).execute.count end end def todos_pending_count(force: false) Rails.cache.fetch(['users', id, 'todos_pending_count'], force: force) do - todos.pending.count + TodosFinder.new(self, state: :pending).execute.count end end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 6b48d68cccb..eb833dd82ac 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -144,8 +144,9 @@ class TodoService 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) + marked_todos = todos.update_all(state: :done) current_user.update_todos_count_cache + marked_todos end # When user marks an issue as todo diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 26c24c3baff..19df13d8aac 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -61,9 +61,9 @@ module API # delete ':id' do 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 # Mark all todos as done @@ -73,9 +73,7 @@ module API # delete do todos = find_todos - todos.each(&:done) - - todos.length + TodoService.new.mark_todos_as_done(todos, current_user) end end end diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 3ccd0af652f..887a2ba5b84 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -117,6 +117,12 @@ describe API::Todos, api: true do expect(response.status).to eq(200) expect(pending_1.reload).to be_done end + + it 'updates todos cache' do + expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original + + delete api("/todos/#{pending_1.id}", john_doe) + end end end @@ -139,6 +145,12 @@ describe API::Todos, api: true do expect(pending_2.reload).to be_done expect(pending_3.reload).to be_done end + + it 'updates todos cache' do + expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original + + delete api("/todos", john_doe) + end end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 34d8ea9090e..6c3cbeae13c 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -472,6 +472,42 @@ describe TodoService, services: true do expect(john_doe.todos_pending_count).to eq(1) 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 = {}) attributes.reverse_merge!( project: project, |