diff options
author | Robert Speicher <robert@gitlab.com> | 2016-08-25 19:04:16 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2016-08-25 19:04:16 +0000 |
commit | 98eef887a4e80e1c1711d45f8507828f19dd3b0b (patch) | |
tree | b834afc3db319dd534d38e4f018826e21e9fb189 | |
parent | 34a472f674fa0d26deea1b248b54a44354f889ba (diff) | |
parent | 7629dc9982f5559972acf9d9b9d98f78ad53e54c (diff) | |
download | gitlab-ce-98eef887a4e80e1c1711d45f8507828f19dd3b0b.tar.gz |
Merge branch '19730-mark-as-done' into 'master'
If the received id is still a pending todo mark it as done
## What does this MR do?
Just return properly on stale todos, for me is annoying to mark as done a todo that is already done and the done hung there forever, so I have to refresh the page. I decided to resolve the issue myself.
## What are the relevant issue numbers?
Closes #19730
See merge request !5795
-rw-r--r-- | app/services/todo_service.rb | 3 | ||||
-rw-r--r-- | spec/features/todos/todos_spec.rb | 21 | ||||
-rw-r--r-- | spec/services/todo_service_spec.rb | 21 |
3 files changed, 44 insertions, 1 deletions
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index e0ccb654590..2aab8c736d6 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -148,7 +148,8 @@ class TodoService def mark_todos_as_done_by_ids(ids, current_user) todos = current_user.todos.where(id: ids) - marked_todos = todos.update_all(state: :done) + # Only return those that are not really on that state + marked_todos = todos.where.not(state: :done).update_all(state: :done) current_user.update_todos_count_cache marked_todos end diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 0342f4f1d97..32544f3f538 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -41,6 +41,27 @@ describe 'Dashboard Todos', feature: true do expect(page).to have_content("You're all done!") end end + + context 'todo is stale on the page' do + before do + todos = TodosFinder.new(user, state: :pending).execute + TodoService.new.mark_todos_as_done(todos, user) + end + + describe 'deleting the todo' do + before do + first('.done-todo').click + end + + it 'is removed from the list' do + expect(page).not_to have_selector('.todos-list .todo') + end + + it 'shows "All done" message' do + expect(page).to have_content("You're all done!") + end + end + end end context 'User has Todos with labels spanning multiple projects' do diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 296fd1bd5a4..cafcad3e3c0 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -496,6 +496,7 @@ describe TodoService, services: true do describe '#mark_todos_as_done' do let(:issue) { create(:issue, project: project, author: author, assignee: john_doe) } + let(:another_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) @@ -518,6 +519,26 @@ describe TodoService, services: true do expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq(1) end + context 'when some of the todos are done already' do + before do + create(:todo, :mentioned, user: john_doe, target: issue, project: project) + create(:todo, :mentioned, user: john_doe, target: another_issue, project: project) + end + + it 'returns the number of those still pending' do + TodoService.new.mark_pending_todos_as_done(issue, john_doe) + + expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq(1) + end + + it 'returns 0 if all are done' do + TodoService.new.mark_pending_todos_as_done(issue, john_doe) + TodoService.new.mark_pending_todos_as_done(another_issue, john_doe) + + expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq(0) + end + 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) |