Commit 98eef887 authored by Robert Speicher's avatar Robert Speicher

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
parents 34a472f6 7629dc99
...@@ -148,7 +148,8 @@ class TodoService ...@@ -148,7 +148,8 @@ class TodoService
def mark_todos_as_done_by_ids(ids, current_user) def mark_todos_as_done_by_ids(ids, current_user)
todos = current_user.todos.where(id: ids) 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 current_user.update_todos_count_cache
marked_todos marked_todos
end end
......
...@@ -41,6 +41,27 @@ describe 'Dashboard Todos', feature: true do ...@@ -41,6 +41,27 @@ describe 'Dashboard Todos', feature: true do
expect(page).to have_content("You're all done!") expect(page).to have_content("You're all done!")
end end
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 end
context 'User has Todos with labels spanning multiple projects' do context 'User has Todos with labels spanning multiple projects' do
......
...@@ -496,6 +496,7 @@ describe TodoService, services: true do ...@@ -496,6 +496,7 @@ describe TodoService, services: true do
describe '#mark_todos_as_done' do describe '#mark_todos_as_done' do
let(:issue) { create(:issue, project: project, author: author, assignee: john_doe) } 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 it 'marks a relation of todos as done' do
create(:todo, :mentioned, user: john_doe, target: issue, project: project) create(:todo, :mentioned, user: john_doe, target: issue, project: project)
...@@ -518,6 +519,26 @@ describe TodoService, services: true do ...@@ -518,6 +519,26 @@ describe TodoService, services: true do
expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq(1) expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq(1)
end 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 it 'caches the number of todos of a user', :caching do
create(:todo, :mentioned, user: john_doe, target: issue, project: project) create(:todo, :mentioned, user: john_doe, target: issue, project: project)
todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) todo = create(:todo, :mentioned, user: john_doe, target: issue, 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