diff options
author | Sean McGivern <sean@gitlab.com> | 2017-12-08 15:17:22 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2017-12-18 15:23:00 +0300 |
commit | ef454f68e837e4e7360fe1518686dd56adbbb0a9 (patch) | |
tree | 72e67bbf1a2222d8a1007298478350b5cc9c02bd /spec | |
parent | 9429e8ac60a10436a0469d7d206d3f74a2c966c7 (diff) |
Reset todo counters when the target is deleted
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 'spec')
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/issuable/destroy_service_spec.rb | 16 | ||||
-rw-r--r-- | spec/services/notes/destroy_service_spec.rb | 16 | ||||
-rw-r--r-- | spec/services/todo_service_spec.rb | 23 |
5 files changed, 49 insertions, 10 deletions
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 4dbbaecdd6d..b7447f654c4 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -861,7 +861,7 @@ describe Projects::IssuesController do end it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(issue, owner).once + expect_any_instance_of(TodoService).to receive(:destroy_target).with(issue).once delete :destroy, namespace_id: project.namespace, project_id: project, id: issue.iid end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 51d5d6a52b3..97a5018d714 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -468,7 +468,7 @@ describe Projects::MergeRequestsController do end it 'delegates the update of the todos count cache to TodoService' do - expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(merge_request, owner).once + expect_any_instance_of(TodoService).to receive(:destroy_target).with(merge_request).once delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid end diff --git a/spec/services/issuable/destroy_service_spec.rb b/spec/services/issuable/destroy_service_spec.rb index d74d98c6079..0a3647a814f 100644 --- a/spec/services/issuable/destroy_service_spec.rb +++ b/spec/services/issuable/destroy_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Issuable::DestroyService do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, :public) } subject(:service) { described_class.new(project, user) } @@ -19,6 +19,13 @@ describe Issuable::DestroyService do service.execute(issue) end + + it 'updates the todo caches for users with todos on the issue' do + create(:todo, target: issue, user: user, author: user, project: project) + + expect { service.execute(issue) } + .to change { user.todos_pending_count }.from(1).to(0) + end end context 'when issuable is a merge request' do @@ -33,6 +40,13 @@ describe Issuable::DestroyService do service.execute(merge_request) end + + it 'updates the todo caches for users with todos on the merge request' do + create(:todo, target: merge_request, user: user, author: user, project: project) + + expect { service.execute(merge_request) } + .to change { user.todos_pending_count }.from(1).to(0) + end end end end diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb index c9a99a43edb..64445be560e 100644 --- a/spec/services/notes/destroy_service_spec.rb +++ b/spec/services/notes/destroy_service_spec.rb @@ -1,15 +1,25 @@ require 'spec_helper' describe Notes::DestroyService do + set(:project) { create(:project, :public) } + set(:issue) { create(:issue, project: project) } + let(:user) { issue.author } + describe '#execute' do it 'deletes a note' do - project = create(:project) - issue = create(:issue, project: project) note = create(:note, project: project, noteable: issue) - described_class.new(project, note.author).execute(note) + described_class.new(project, user).execute(note) expect(project.issues.find(issue.id).notes).not_to include(note) end + + it 'updates the todo counts for users with todos for the note' do + note = create(:note, project: project, noteable: issue) + create(:todo, note: note, target: issue, user: user, author: user, project: project) + + expect { described_class.new(project, user).execute(note) } + .to change { user.todos_pending_count }.from(1).to(0) + end end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index dc2673abc73..88013acae0a 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -248,11 +248,26 @@ describe TodoService do end end - describe '#destroy_issuable' do - it 'refresh the todos count cache for the user' do - expect(john_doe).to receive(:update_todos_count_cache).and_call_original + describe '#destroy_target' do + it 'refreshes the todos count cache for users with todos on the target' do + create(:todo, target: issue, user: john_doe, author: john_doe, project: issue.project) - service.destroy_issuable(issue, john_doe) + expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original + + service.destroy_target(issue) { } + end + + it 'does not refresh the todos count cache for users with only done todos on the target' do + create(:todo, :done, target: issue, user: john_doe, author: john_doe, project: issue.project) + + expect_any_instance_of(User).not_to receive(:update_todos_count_cache) + + service.destroy_target(issue) { } + end + + it 'yields the target to the caller' do + expect { |b| service.destroy_target(issue, &b) } + .to yield_with_args(issue) end end |