Commit a458211b authored by Robert Speicher's avatar Robert Speicher

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

Toggling a task in a description with mentions doesn't creates a Todo

When user toggle a task list item in a description with a mention it
does not create an unnecessary Todo for that mention.

Closes #14116

See merge request !4568
parents d64517c3 0098468d
...@@ -56,6 +56,7 @@ v 8.9.0 (unreleased) ...@@ -56,6 +56,7 @@ v 8.9.0 (unreleased)
- Improve issuables APIs performance when accessing notes !4471 - Improve issuables APIs performance when accessing notes !4471
- External links now open in a new tab - External links now open in a new tab
- Markdown editor now correctly resets the input value on edit cancellation !4175 - Markdown editor now correctly resets the input value on edit cancellation !4175
- Toggling a task list item in a issue/mr description does not creates a Todo for mentions
v 8.8.4 (unreleased) v 8.8.4 (unreleased)
- Ensure branch cleanup regardless of whether the GitHub import process succeeds - Ensure branch cleanup regardless of whether the GitHub import process succeeds
......
...@@ -20,7 +20,7 @@ class TodoService ...@@ -20,7 +20,7 @@ class TodoService
# * mark all pending todos related to the issue for the current user as done # * mark all pending todos related to the issue for the current user as done
# #
def update_issue(issue, current_user) def update_issue(issue, current_user)
create_mention_todos(issue.project, issue, current_user) update_issuable(issue, current_user)
end end
# When close an issue we should: # When close an issue we should:
...@@ -53,7 +53,7 @@ class TodoService ...@@ -53,7 +53,7 @@ class TodoService
# * create a todo for each mentioned user on merge request # * create a todo for each mentioned user on merge request
# #
def update_merge_request(merge_request, current_user) def update_merge_request(merge_request, current_user)
create_mention_todos(merge_request.project, merge_request, current_user) update_issuable(merge_request, current_user)
end end
# When close a merge request we should: # When close a merge request we should:
...@@ -153,6 +153,13 @@ class TodoService ...@@ -153,6 +153,13 @@ class TodoService
create_mention_todos(issuable.project, issuable, author) create_mention_todos(issuable.project, issuable, author)
end end
def update_issuable(issuable, author)
# Skip toggling a task list item in a description
return if issuable.tasks? && issuable.updated_tasks.any?
create_mention_todos(issuable.project, issuable, author)
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?
......
...@@ -18,7 +18,7 @@ describe TodoService, services: true do ...@@ -18,7 +18,7 @@ describe TodoService, services: true do
end end
describe 'Issues' do describe 'Issues' do
let(:issue) { create(:issue, project: project, assignee: john_doe, author: author, description: mentions) } let(:issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
let(:unassigned_issue) { create(:issue, project: project, assignee: nil) } let(:unassigned_issue) { create(:issue, project: project, assignee: nil) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) }
...@@ -101,6 +101,19 @@ describe TodoService, services: true do ...@@ -101,6 +101,19 @@ describe TodoService, services: true do
should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED) should_create_todo(user: admin, 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) 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
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
end end
describe '#close_issue' do describe '#close_issue' do
...@@ -210,7 +223,7 @@ describe TodoService, services: true do ...@@ -210,7 +223,7 @@ describe TodoService, services: true do
end end
describe 'Merge Requests' do describe 'Merge Requests' do
let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: mentions) } let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignee: nil) } let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignee: nil) }
describe '#new_merge_request' do describe '#new_merge_request' do
...@@ -253,6 +266,19 @@ describe TodoService, services: true do ...@@ -253,6 +266,19 @@ 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
mr_assigned.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
service.update_merge_request(mr_assigned, author)
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: 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: member, target: mr_assigned, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
end
end end
describe '#close_merge_request' do describe '#close_merge_request' do
......
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