Commit fcd9f906 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'fix-toggling-task-should-not-generate-todo' into 'master'

Fix error when editing an issuable with a task list

Closes #18712

See merge request !4751
parents ed5f17cc b83f9a55
...@@ -161,11 +161,16 @@ class TodoService ...@@ -161,11 +161,16 @@ class TodoService
def update_issuable(issuable, author) def update_issuable(issuable, author)
# Skip toggling a task list item in a description # Skip toggling a task list item in a description
return if issuable.tasks? && issuable.updated_tasks.any? return if toggling_tasks?(issuable)
create_mention_todos(issuable.project, issuable, author) create_mention_todos(issuable.project, issuable, author)
end end
def toggling_tasks?(issuable)
issuable.previous_changes.include?('description') &&
issuable.tasks? && issuable.updated_tasks.any?
end
def handle_note(note, author) def handle_note(note, author)
# Skip system notes, and notes on project snippet # Skip system notes, and notes on project snippet
return if note.system? || note.for_snippet? return if note.system? || note.for_snippet?
......
...@@ -108,17 +108,25 @@ describe TodoService, services: true do ...@@ -108,17 +108,25 @@ describe TodoService, services: true do
should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED) should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
end end
it 'does not create todo when when tasks are marked as completed' do context 'issues with a task list' do
issue.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}") it 'does not create todo when tasks are marked as completed' do
issue.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
service.update_issue(issue, author)
should_not_create_todo(user: admin, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: assignee, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: member, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
end
service.update_issue(issue, author) it 'does not raise an error when description not change' do
issue.update(title: 'Sample')
should_not_create_todo(user: admin, target: issue, action: Todo::MENTIONED) expect { service.update_issue(issue, author) }.not_to raise_error
should_not_create_todo(user: assignee, target: issue, action: Todo::MENTIONED) end
should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: member, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
end end
end end
...@@ -285,17 +293,25 @@ describe TodoService, services: true do ...@@ -285,17 +293,25 @@ describe TodoService, services: true do
expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count) expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count)
end end
it 'does not create todo when when tasks are marked as completed' do context 'with a task list' do
mr_assigned.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}") it 'does not create todo when tasks are marked as completed' do
mr_assigned.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
service.update_merge_request(mr_assigned, author) service.update_merge_request(mr_assigned, author)
should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED)
should_not_create_todo(user: assignee, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: assignee, target: mr_assigned, action: Todo::MENTIONED)
should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
should_not_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
end
it 'does not raise an error when description not change' do
mr_assigned.update(title: 'Sample')
expect { service.update_merge_request(mr_assigned, author) }.not_to raise_error
end
end end
end end
......
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