diff options
author | Robert Speicher <robert@gitlab.com> | 2016-06-09 19:47:15 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2016-06-09 19:47:15 +0000 |
commit | a458211b6569b510593393220a6c1f5b4e25cfb8 (patch) | |
tree | c02730e46e6cd810d224948fdb676bff35024134 | |
parent | d64517c37d1e14dfe8f27eadfb40dcb034a57092 (diff) | |
parent | 0098468dfb5927b4034d38c7faac44ac238b9385 (diff) | |
download | gitlab-ce-a458211b6569b510593393220a6c1f5b4e25cfb8.tar.gz |
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
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/services/todo_service.rb | 11 | ||||
-rw-r--r-- | spec/services/todo_service_spec.rb | 30 |
3 files changed, 38 insertions, 4 deletions
diff --git a/CHANGELOG b/CHANGELOG index 17fc4801a6e..2be7f1568a0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -56,6 +56,7 @@ v 8.9.0 (unreleased) - Improve issuables APIs performance when accessing notes !4471 - External links now open in a new tab - 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) - Ensure branch cleanup regardless of whether the GitHub import process succeeds diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index d8365124175..8e03ff8ddde 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -20,7 +20,7 @@ class TodoService # * mark all pending todos related to the issue for the current user as done # def update_issue(issue, current_user) - create_mention_todos(issue.project, issue, current_user) + update_issuable(issue, current_user) end # When close an issue we should: @@ -53,7 +53,7 @@ class TodoService # * create a todo for each mentioned user on merge request # 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 # When close a merge request we should: @@ -153,6 +153,13 @@ class TodoService create_mention_todos(issuable.project, issuable, author) 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) # Skip system notes, and notes on project snippet return if note.system? || note.for_snippet? diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 6e7ecbd39ba..489c920f19f 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -18,7 +18,7 @@ describe TodoService, services: true do end 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(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) } @@ -101,6 +101,19 @@ describe TodoService, services: true do 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) 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 describe '#close_issue' do @@ -210,7 +223,7 @@ describe TodoService, services: true do end 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) } describe '#new_merge_request' do @@ -253,6 +266,19 @@ describe TodoService, services: true do expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count) 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 describe '#close_merge_request' do |