Commit bf064215 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Allow multiple todos behind a feature flag

This feature flag allows users to get multiple todos even if they have
an existing pending todo
parent 2503be2f
...@@ -216,7 +216,7 @@ class TodoService ...@@ -216,7 +216,7 @@ class TodoService
def create_todos(users, attributes) def create_todos(users, attributes)
Array(users).map do |user| Array(users).map do |user|
next if pending_todos(user, attributes).exists? next if pending_todos(user, attributes).exists? && Feature.disabled?(:multiple_todos, user)
issue_type = attributes.delete(:issue_type) issue_type = attributes.delete(:issue_type)
track_todo_creation(user, issue_type) track_todo_creation(user, issue_type)
...@@ -278,7 +278,7 @@ class TodoService ...@@ -278,7 +278,7 @@ class TodoService
create_todos(directly_addressed_users, attributes) create_todos(directly_addressed_users, attributes)
# Create Todos for mentioned users # Create Todos for mentioned users
mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users) mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users + directly_addressed_users)
attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note) attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note)
create_todos(mentioned_users, attributes) create_todos(mentioned_users, attributes)
end end
......
---
name: multiple_todos
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47629
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/28355
milestone: '13.8'
type: development
group: group::project management
default_enabled: false
...@@ -88,6 +88,8 @@ RSpec.describe API::Todos do ...@@ -88,6 +88,8 @@ RSpec.describe API::Todos do
end end
it 'returns 304 there already exist a todo on that epic' do it 'returns 304 there already exist a todo on that epic' do
stub_feature_flags(multiple_todos: false)
create(:todo, project: nil, group: group, user: user, target: epic) create(:todo, project: nil, group: group, user: user, target: epic)
subject subject
......
...@@ -3,13 +3,13 @@ ...@@ -3,13 +3,13 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe TodoService do RSpec.describe TodoService do
let(:author) { create(:user, username: 'author') } let_it_be(:author) { create(:user, username: 'author') }
let(:non_member) { create(:user, username: 'non_member') } let_it_be(:non_member) { create(:user, username: 'non_member') }
let(:member) { create(:user, username: 'member') } let_it_be(:member) { create(:user, username: 'member') }
let(:guest) { create(:user, username: 'guest') } let_it_be(:guest) { create(:user, username: 'guest') }
let(:admin) { create(:admin, username: 'administrator') } let_it_be(:admin) { create(:admin, username: 'administrator') }
let(:john_doe) { create(:user, username: 'john_doe') } let_it_be(:john_doe) { create(:user, username: 'john_doe') }
let(:skipped) { create(:user, username: 'skipped') } let_it_be(:skipped) { create(:user, username: 'skipped') }
let(:skip_users) { [skipped] } let(:skip_users) { [skipped] }
let(:service) { described_class.new } let(:service) { described_class.new }
...@@ -21,21 +21,23 @@ RSpec.describe TodoService do ...@@ -21,21 +21,23 @@ RSpec.describe TodoService do
let(:description_mentions) { "- [ ] Task 1\n- [ ] Task 2 FYI: #{mentions}" } let(:description_mentions) { "- [ ] Task 1\n- [ ] Task 2 FYI: #{mentions}" }
let(:description_directly_addressed) { "#{mentions}\n- [ ] Task 1\n- [ ] Task 2" } let(:description_directly_addressed) { "#{mentions}\n- [ ] Task 1\n- [ ] Task 2" }
let(:group) { create(:group) } let_it_be(:group, reload: true) { create(:group) }
let(:epic) { create(:epic, group: group, author: author, description: description_mentions) } let(:epic) { create(:epic, group: group, author: author, description: description_mentions) }
let(:todos_for) { [] } let(:todos_for) { [] }
let(:todos_not_for) { [] } let(:todos_not_for) { [] }
let(:target) { epic } let(:target) { epic }
before do before_all do
stub_licensed_features(epics: true)
group.add_guest(guest) group.add_guest(guest)
group.add_developer(author) group.add_developer(author)
group.add_developer(member) group.add_developer(member)
end end
before do
stub_licensed_features(epics: true)
end
shared_examples_for 'todos creation' do shared_examples_for 'todos creation' do
it 'creates todos for users mentioned' do it 'creates todos for users mentioned' do
if todos_for.count > 0 if todos_for.count > 0
...@@ -169,19 +171,19 @@ RSpec.describe TodoService do ...@@ -169,19 +171,19 @@ RSpec.describe TodoService do
end end
describe '#new_note' do describe '#new_note' do
let!(:first_todo) do
create(:todo, :assigned,
user: john_doe, project: nil, group: group, target: epic, author: author)
end
let!(:second_todo) do
create(:todo, :assigned,
user: john_doe, project: nil, group: group, target: epic, author: author)
end
let(:note) { create(:note, noteable: epic, project: nil, author: john_doe, note: mentions) } let(:note) { create(:note, noteable: epic, project: nil, author: john_doe, note: mentions) }
context 'when a note is created for an epic' do context 'when a note is created for an epic' do
let!(:first_todo) do
create(:todo, :assigned,
user: john_doe, project: nil, group: group, target: epic, author: author)
end
let!(:second_todo) do
create(:todo, :assigned,
user: john_doe, project: nil, group: group, target: epic, author: author)
end
it 'marks pending epic todos for the note author as done' do it 'marks pending epic todos for the note author as done' do
service.new_note(note, john_doe) service.new_note(note, john_doe)
...@@ -189,7 +191,7 @@ RSpec.describe TodoService do ...@@ -189,7 +191,7 @@ RSpec.describe TodoService do
expect(second_todo.reload).to be_done expect(second_todo.reload).to be_done
end end
it 'does not marka pending epic todos for the note author as done for system notes' do it 'does not mark pending epic todos for the note author as done for system notes' do
system_note = create(:system_note, noteable: epic) system_note = create(:system_note, noteable: epic)
service.new_note(system_note, john_doe) service.new_note(system_note, john_doe)
...@@ -208,8 +210,8 @@ RSpec.describe TodoService do ...@@ -208,8 +210,8 @@ RSpec.describe TodoService do
end end
let(:todo_params) { { action: Todo::MENTIONED } } let(:todo_params) { { action: Todo::MENTIONED } }
let(:todos_for) { [author, non_member, member, guest, admin, skipped] } let(:todos_for) { users }
let(:todos_not_for) { [john_doe] } let(:todos_not_for) { [] }
include_examples 'todos creation' include_examples 'todos creation'
end end
...@@ -220,8 +222,8 @@ RSpec.describe TodoService do ...@@ -220,8 +222,8 @@ RSpec.describe TodoService do
end end
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { [author, non_member, member, guest, admin, skipped] } let(:todos_for) { users }
let(:todos_not_for) { [john_doe] } let(:todos_not_for) { [] }
include_examples 'todos creation' include_examples 'todos creation'
end end
...@@ -290,7 +292,6 @@ RSpec.describe TodoService do ...@@ -290,7 +292,6 @@ RSpec.describe TodoService do
# for each valid mentioned user # for each valid mentioned user
should_create_todo(user: john_doe, target: merge_request, action: Todo::MENTIONED) should_create_todo(user: john_doe, target: merge_request, action: Todo::MENTIONED)
should_not_create_todo(user: approver_1, target: merge_request, action: Todo::MENTIONED)
# skip for code owner # skip for code owner
should_not_create_todo(user: code_owner, target: merge_request, action: Todo::APPROVAL_REQUIRED) should_not_create_todo(user: code_owner, target: merge_request, action: Todo::APPROVAL_REQUIRED)
......
...@@ -42,6 +42,8 @@ RSpec.describe 'Creating a todo for the alert' do ...@@ -42,6 +42,8 @@ RSpec.describe 'Creating a todo for the alert' do
context 'todo already exists' do context 'todo already exists' do
before do before do
stub_feature_flags(multiple_todos: false)
create(:todo, :pending, project: project, user: user, target: alert) create(:todo, :pending, project: project, user: user, target: alert)
end end
......
...@@ -302,6 +302,8 @@ RSpec.describe API::Todos do ...@@ -302,6 +302,8 @@ RSpec.describe API::Todos do
end end
it 'returns 304 there already exist a todo on that issuable' do it 'returns 304 there already exist a todo on that issuable' do
stub_feature_flags(multiple_todos: false)
create(:todo, project: project_1, author: author_1, user: john_doe, target: issuable) create(:todo, project: project_1, author: author_1, user: john_doe, target: issuable)
post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", john_doe) post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", john_doe)
......
...@@ -58,6 +58,10 @@ RSpec.describe AlertManagement::Alerts::Todo::CreateService do ...@@ -58,6 +58,10 @@ RSpec.describe AlertManagement::Alerts::Todo::CreateService do
create(:todo, :pending, **todo_params) create(:todo, :pending, **todo_params)
end end
before do
stub_feature_flags(multiple_todos: false)
end
it 'does not create a todo' do it 'does not create a todo' do
expect { result }.not_to change { Todo.count } expect { result }.not_to change { Todo.count }
end end
......
...@@ -100,17 +100,18 @@ RSpec.describe TodoService do ...@@ -100,17 +100,18 @@ RSpec.describe TodoService do
end end
describe 'Issues' do describe 'Issues' do
let(:issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } let(:issue) { create(:issue, project: project, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
let(:addressed_issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } let(:addressed_issue) { create(:issue, project: project, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
let(:assigned_issue) { create(:issue, project: project, assignees: [john_doe]) }
let(:unassigned_issue) { create(:issue, project: project, assignees: []) } let(:unassigned_issue) { create(:issue, project: project, assignees: []) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: mentions) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: mentions) }
let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: directly_addressed) } let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: directly_addressed) }
describe '#new_issue' do describe '#new_issue' do
it 'creates a todo if assigned' do it 'creates a todo if assigned' do
service.new_issue(issue, author) service.new_issue(assigned_issue, author)
should_create_todo(user: john_doe, target: issue, action: Todo::ASSIGNED) should_create_todo(user: john_doe, target: assigned_issue, action: Todo::ASSIGNED)
end end
it 'does not create a todo if unassigned' do it 'does not create a todo if unassigned' do
...@@ -130,7 +131,7 @@ RSpec.describe TodoService do ...@@ -130,7 +131,7 @@ RSpec.describe TodoService do
should_create_todo(user: member, target: issue, action: Todo::MENTIONED) should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
should_create_todo(user: guest, target: issue, action: Todo::MENTIONED) should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
should_create_todo(user: author, target: issue, action: Todo::MENTIONED) should_create_todo(user: author, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED) should_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
end end
...@@ -140,7 +141,7 @@ RSpec.describe TodoService do ...@@ -140,7 +141,7 @@ RSpec.describe TodoService do
should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
end end
...@@ -244,6 +245,8 @@ RSpec.describe TodoService do ...@@ -244,6 +245,8 @@ RSpec.describe TodoService do
end end
it 'does not create a todo if user was already mentioned and todo is pending' do it 'does not create a todo if user was already mentioned and todo is pending' do
stub_feature_flags(multiple_todos: false)
create(:todo, :mentioned, user: member, project: project, target: issue, author: author) create(:todo, :mentioned, user: member, project: project, target: issue, author: author)
expect { service.update_issue(issue, author, skip_users) }.not_to change(member.todos, :count) expect { service.update_issue(issue, author, skip_users) }.not_to change(member.todos, :count)
...@@ -256,6 +259,8 @@ RSpec.describe TodoService do ...@@ -256,6 +259,8 @@ RSpec.describe TodoService do
end end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do
stub_feature_flags(multiple_todos: false)
create(:todo, :directly_addressed, user: member, project: project, target: addressed_issue, author: author) create(:todo, :directly_addressed, user: member, project: project, target: addressed_issue, author: author)
expect { service.update_issue(addressed_issue, author, skip_users) }.not_to change(member.todos, :count) expect { service.update_issue(addressed_issue, author, skip_users) }.not_to change(member.todos, :count)
...@@ -622,6 +627,26 @@ RSpec.describe TodoService do ...@@ -622,6 +627,26 @@ RSpec.describe TodoService do
expect(service.todo_exist?(unassigned_issue, author)).to be_truthy expect(service.todo_exist?(unassigned_issue, author)).to be_truthy
end end
end end
context 'when multiple_todos are enabled' do
before do
stub_feature_flags(multiple_todos: true)
end
it 'creates a todo even if user already has a pending todo' do
create(:todo, :mentioned, user: member, project: project, target: issue, author: author)
expect { service.update_issue(issue, author) }.to change(member.todos, :count)
end
it 'creates multiple todos if a user is assigned and mentioned in a new issue' do
assigned_issue.description = mentions
service.new_issue(assigned_issue, author)
should_create_todo(user: john_doe, target: assigned_issue, action: Todo::ASSIGNED)
should_create_todo(user: john_doe, target: assigned_issue, action: Todo::MENTIONED)
end
end
end end
describe '#reassigned_assignable' do describe '#reassigned_assignable' do
...@@ -664,154 +689,161 @@ RSpec.describe TodoService do ...@@ -664,154 +689,161 @@ RSpec.describe TodoService do
end end
describe 'Merge Requests' do describe 'Merge Requests' do
let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } let(:mentioned_mr) { create(:merge_request, source_project: project, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
let(:addressed_mr_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } let(:addressed_mr) { create(:merge_request, source_project: project, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignees: []) } let(:assigned_mr) { create(:merge_request, source_project: project, author: author, assignees: [john_doe]) }
let(:unassigned_mr) { create(:merge_request, source_project: project, author: author, assignees: []) }
describe '#new_merge_request' do describe '#new_merge_request' do
it 'creates a pending todo if assigned' do it 'creates a pending todo if assigned' do
service.new_merge_request(mr_assigned, author) service.new_merge_request(assigned_mr, author)
should_create_todo(user: john_doe, target: mr_assigned, action: Todo::ASSIGNED) should_create_todo(user: john_doe, target: assigned_mr, action: Todo::ASSIGNED)
end end
it 'does not create a todo if unassigned' do it 'does not create a todo if unassigned' do
should_not_create_any_todo { service.new_merge_request(mr_unassigned, author) } should_not_create_any_todo { service.new_merge_request(unassigned_mr, author) }
end end
it 'does not create a todo if assignee is the current user' do it 'creates a todo if assignee is the current user' do
should_not_create_any_todo { service.new_merge_request(mr_unassigned, john_doe) } service.new_merge_request(assigned_mr, john_doe)
should_create_todo(user: john_doe, target: assigned_mr, author: john_doe, action: Todo::ASSIGNED)
end end
it 'creates a todo for each valid mentioned user' do it 'creates a todo for each valid mentioned user' do
service.new_merge_request(mr_assigned, author) service.new_merge_request(mentioned_mr, author)
should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: member, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
should_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: author, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: john_doe, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mentioned_mr, action: Todo::MENTIONED)
end end
it 'creates a todo for each valid user based on the type of mention' do it 'creates a todo for each valid user based on the type of mention' do
mr_assigned.update!(description: directly_addressed_and_mentioned) mentioned_mr.update!(description: directly_addressed_and_mentioned)
service.new_merge_request(mr_assigned, author) service.new_merge_request(mentioned_mr, author)
should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: member, target: mentioned_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: admin, target: mentioned_mr, action: Todo::MENTIONED)
end end
it 'creates a directly addressed todo for each valid addressed user' do it 'creates a directly addressed todo for each valid addressed user' do
service.new_merge_request(addressed_mr_assigned, author) service.new_merge_request(addressed_mr, author)
should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: author, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: john_doe, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: non_member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end end
end end
describe '#update_merge_request' do describe '#update_merge_request' do
it 'creates a todo for each valid mentioned user not included in skip_users' do it 'creates a todo for each valid mentioned user not included in skip_users' do
service.update_merge_request(mr_assigned, author, skip_users) service.update_merge_request(mentioned_mr, author, skip_users)
should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: member, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
should_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: john_doe, target: mentioned_mr, action: Todo::MENTIONED)
should_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: author, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: skipped, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: skipped, target: mentioned_mr, action: Todo::MENTIONED)
end end
it 'creates a todo for each valid user not included in skip_users based on the type of mention' do it 'creates a todo for each valid user not included in skip_users based on the type of mention' do
mr_assigned.update!(description: directly_addressed_and_mentioned) mentioned_mr.update!(description: directly_addressed_and_mentioned)
service.update_merge_request(mr_assigned, author, skip_users) service.update_merge_request(mentioned_mr, author, skip_users)
should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: member, target: mentioned_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: admin, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: skipped, target: mr_assigned) should_not_create_todo(user: skipped, target: mentioned_mr)
end end
it 'creates a directly addressed todo for each valid addressed user not included in skip_users' do it 'creates a directly addressed todo for each valid addressed user not included in skip_users' do
service.update_merge_request(addressed_mr_assigned, author, skip_users) service.update_merge_request(addressed_mr, author, skip_users)
should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: john_doe, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: author, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: non_member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: skipped, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: skipped, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end end
it 'does not create a todo if user was already mentioned and todo is pending' do it 'does not create a todo if user was already mentioned and todo is pending' do
create(:todo, :mentioned, user: member, project: project, target: mr_assigned, author: author) stub_feature_flags(multiple_todos: false)
create(:todo, :mentioned, user: member, project: project, target: mentioned_mr, author: author)
expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count) expect { service.update_merge_request(mentioned_mr, author) }.not_to change(member.todos, :count)
end end
it 'does not create a todo if user was already mentioned and todo is done' do it 'does not create a todo if user was already mentioned and todo is done' do
create(:todo, :mentioned, :done, user: skipped, project: project, target: mr_assigned, author: author) create(:todo, :mentioned, :done, user: skipped, project: project, target: mentioned_mr, author: author)
expect { service.update_merge_request(mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) expect { service.update_merge_request(mentioned_mr, author, skip_users) }.not_to change(skipped.todos, :count)
end end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do
create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr_assigned, author: author) stub_feature_flags(multiple_todos: false)
create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr, author: author)
expect { service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count) expect { service.update_merge_request(addressed_mr, author) }.not_to change(member.todos, :count)
end end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do
create(:todo, :directly_addressed, user: skipped, project: project, target: addressed_mr_assigned, author: author) create(:todo, :directly_addressed, user: skipped, project: project, target: addressed_mr, author: author)
expect { service.update_merge_request(addressed_mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) expect { service.update_merge_request(addressed_mr, author, skip_users) }.not_to change(skipped.todos, :count)
end end
context 'with a task list' do context 'with a task list' do
it 'does not create todo when tasks are marked as completed' do it 'does not create todo when tasks are marked as completed' do
mr_assigned.update!(description: "- [x] Task 1\n- [X] Task 2 #{mentions}") mentioned_mr.update!(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
service.update_merge_request(mr_assigned, author) service.update_merge_request(mentioned_mr, author)
should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: admin, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: assignee, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: assignee, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: author, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: john_doe, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: member, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mentioned_mr, action: Todo::MENTIONED)
should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
end end
it 'does not create directly addressed todo when tasks are marked as completed' do it 'does not create directly addressed todo when tasks are marked as completed' do
addressed_mr_assigned.update!(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2") addressed_mr.update!(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2")
service.update_merge_request(addressed_mr_assigned, author) service.update_merge_request(addressed_mr, author)
should_not_create_todo(user: admin, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: admin, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: assignee, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: assignee, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: author, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: john_doe, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: non_member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end end
it 'does not raise an error when description not change' do it 'does not raise an error when description not change' do
mr_assigned.update!(title: 'Sample') mentioned_mr.update!(title: 'Sample')
expect { service.update_merge_request(mr_assigned, author) }.not_to raise_error expect { service.update_merge_request(mentioned_mr, author) }.not_to raise_error
end end
end end
end end
describe '#close_merge_request' do describe '#close_merge_request' do
it 'marks related pending todos to the target for the user as done' do it 'marks related pending todos to the target for the user as done' do
first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author) first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author) second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
service.close_merge_request(mr_assigned, john_doe) service.close_merge_request(mentioned_mr, john_doe)
expect(first_todo.reload).to be_done expect(first_todo.reload).to be_done
expect(second_todo.reload).to be_done expect(second_todo.reload).to be_done
...@@ -820,55 +852,55 @@ RSpec.describe TodoService do ...@@ -820,55 +852,55 @@ RSpec.describe TodoService do
describe '#merge_merge_request' do describe '#merge_merge_request' do
it 'marks related pending todos to the target for the user as done' do it 'marks related pending todos to the target for the user as done' do
first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author) first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author) second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
service.merge_merge_request(mr_assigned, john_doe) service.merge_merge_request(mentioned_mr, john_doe)
expect(first_todo.reload).to be_done expect(first_todo.reload).to be_done
expect(second_todo.reload).to be_done expect(second_todo.reload).to be_done
end end
it 'does not create todo for guests' do it 'does not create todo for guests' do
service.merge_merge_request(mr_assigned, john_doe) service.merge_merge_request(mentioned_mr, john_doe)
should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
end end
it 'does not create directly addressed todo for guests' do it 'does not create directly addressed todo for guests' do
service.merge_merge_request(addressed_mr_assigned, john_doe) service.merge_merge_request(addressed_mr, john_doe)
should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end end
end end
describe '#new_award_emoji' do describe '#new_award_emoji' do
it 'marks related pending todos to the target for the user as done' do it 'marks related pending todos to the target for the user as done' do
todo = create(:todo, user: john_doe, project: project, target: mr_assigned, author: author) todo = create(:todo, user: john_doe, project: project, target: mentioned_mr, author: author)
service.new_award_emoji(mr_assigned, john_doe) service.new_award_emoji(mentioned_mr, john_doe)
expect(todo.reload).to be_done expect(todo.reload).to be_done
end end
end end
describe '#merge_request_build_failed' do describe '#merge_request_build_failed' do
let(:merge_participants) { [mr_unassigned.author, admin] } let(:merge_participants) { [unassigned_mr.author, admin] }
before do before do
allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants) allow(unassigned_mr).to receive(:merge_participants).and_return(merge_participants)
end end
it 'creates a pending todo for each merge_participant' do it 'creates a pending todo for each merge_participant' do
service.merge_request_build_failed(mr_unassigned) service.merge_request_build_failed(unassigned_mr)
merge_participants.each do |participant| merge_participants.each do |participant|
should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::BUILD_FAILED) should_create_todo(user: participant, author: participant, target: unassigned_mr, action: Todo::BUILD_FAILED)
end end
end end
end end
describe '#merge_request_push' do describe '#merge_request_push' do
it 'marks related pending todos to the target for the user as done' do it 'marks related pending todos to the target for the user as done' do
first_todo = create(:todo, :build_failed, user: author, project: project, target: mr_assigned, author: john_doe) first_todo = create(:todo, :build_failed, user: author, project: project, target: mentioned_mr, author: john_doe)
second_todo = create(:todo, :build_failed, user: john_doe, project: project, target: mr_assigned, author: john_doe) second_todo = create(:todo, :build_failed, user: john_doe, project: project, target: mentioned_mr, author: john_doe)
service.merge_request_push(mr_assigned, author) service.merge_request_push(mentioned_mr, author)
expect(first_todo.reload).to be_done expect(first_todo.reload).to be_done
expect(second_todo.reload).not_to be_done expect(second_todo.reload).not_to be_done
...@@ -879,24 +911,24 @@ RSpec.describe TodoService do ...@@ -879,24 +911,24 @@ RSpec.describe TodoService do
let(:merge_participants) { [admin, create(:user)] } let(:merge_participants) { [admin, create(:user)] }
before do before do
allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants) allow(unassigned_mr).to receive(:merge_participants).and_return(merge_participants)
end end
it 'creates a pending todo for each merge_participant' do it 'creates a pending todo for each merge_participant' do
mr_unassigned.update!(merge_when_pipeline_succeeds: true, merge_user: admin) unassigned_mr.update!(merge_when_pipeline_succeeds: true, merge_user: admin)
service.merge_request_became_unmergeable(mr_unassigned) service.merge_request_became_unmergeable(unassigned_mr)
merge_participants.each do |participant| merge_participants.each do |participant|
should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::UNMERGEABLE) should_create_todo(user: participant, author: participant, target: unassigned_mr, action: Todo::UNMERGEABLE)
end end
end end
end end
describe '#mark_todo' do describe '#mark_todo' do
it 'creates a todo from a merge request' do it 'creates a todo from a merge request' do
service.mark_todo(mr_unassigned, author) service.mark_todo(unassigned_mr, author)
should_create_todo(user: author, target: mr_unassigned, action: Todo::MARKED) should_create_todo(user: author, target: unassigned_mr, action: Todo::MARKED)
end end
end end
...@@ -913,33 +945,33 @@ RSpec.describe TodoService do ...@@ -913,33 +945,33 @@ RSpec.describe TodoService do
end end
let(:mention) { john_doe.to_reference } let(:mention) { john_doe.to_reference }
let(:diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") } let(:diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: unassigned_mr, author: author, note: "Hey #{mention}") }
let(:addressed_diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "#{mention}, hey!") } let(:addressed_diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: unassigned_mr, author: author, note: "#{mention}, hey!") }
let(:legacy_diff_note_on_merge_request) { create(:legacy_diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") } let(:legacy_diff_note_on_merge_request) { create(:legacy_diff_note_on_merge_request, project: project, noteable: unassigned_mr, author: author, note: "Hey #{mention}") }
it 'creates a todo for mentioned user on new diff note' do it 'creates a todo for mentioned user on new diff note' do
service.new_note(diff_note_on_merge_request, author) service.new_note(diff_note_on_merge_request, author)
should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::MENTIONED, note: diff_note_on_merge_request) should_create_todo(user: john_doe, target: unassigned_mr, author: author, action: Todo::MENTIONED, note: diff_note_on_merge_request)
end end
it 'creates a directly addressed todo for addressed user on new diff note' do it 'creates a directly addressed todo for addressed user on new diff note' do
service.new_note(addressed_diff_note_on_merge_request, author) service.new_note(addressed_diff_note_on_merge_request, author)
should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::DIRECTLY_ADDRESSED, note: addressed_diff_note_on_merge_request) should_create_todo(user: john_doe, target: unassigned_mr, author: author, action: Todo::DIRECTLY_ADDRESSED, note: addressed_diff_note_on_merge_request)
end end
it 'creates a todo for mentioned user on legacy diff note' do it 'creates a todo for mentioned user on legacy diff note' do
service.new_note(legacy_diff_note_on_merge_request, author) service.new_note(legacy_diff_note_on_merge_request, author)
should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::MENTIONED, note: legacy_diff_note_on_merge_request) should_create_todo(user: john_doe, target: unassigned_mr, author: author, action: Todo::MENTIONED, note: legacy_diff_note_on_merge_request)
end end
it 'does not create todo for guests' do it 'does not create todo for guests' do
note_on_merge_request = create :note_on_merge_request, project: project, noteable: mr_assigned, note: mentions note_on_merge_request = create :note_on_merge_request, project: project, noteable: mentioned_mr, note: mentions
service.new_note(note_on_merge_request, author) service.new_note(note_on_merge_request, author)
should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
end end
end end
end end
...@@ -1013,6 +1045,8 @@ RSpec.describe TodoService do ...@@ -1013,6 +1045,8 @@ RSpec.describe TodoService do
end end
it 'does not create a todo if user was already mentioned and todo is pending' do it 'does not create a todo if user was already mentioned and todo is pending' do
stub_feature_flags(multiple_todos: false)
create(:todo, :mentioned, user: member, project: project, target: noteable, author: author) create(:todo, :mentioned, user: member, project: project, target: noteable, author: author)
expect { service.update_note(note, author, skip_users) }.not_to change(member.todos, :count) expect { service.update_note(note, author, skip_users) }.not_to change(member.todos, :count)
...@@ -1025,6 +1059,8 @@ RSpec.describe TodoService do ...@@ -1025,6 +1059,8 @@ RSpec.describe TodoService do
end end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do
stub_feature_flags(multiple_todos: false)
create(:todo, :directly_addressed, user: member, project: project, target: noteable, author: author) create(:todo, :directly_addressed, user: member, project: project, target: noteable, author: author)
expect { service.update_note(addressed_note, author, skip_users) }.not_to change(member.todos, :count) expect { service.update_note(addressed_note, author, skip_users) }.not_to change(member.todos, :count)
...@@ -1038,7 +1074,7 @@ RSpec.describe TodoService do ...@@ -1038,7 +1074,7 @@ RSpec.describe TodoService do
end end
it 'updates cached counts when a todo is created' do it 'updates cached counts when a todo is created' do
issue = create(:issue, project: project, assignees: [john_doe], author: author, description: mentions) issue = create(:issue, project: project, assignees: [john_doe], author: author)
expect(john_doe.todos_pending_count).to eq(0) expect(john_doe.todos_pending_count).to eq(0)
expect(john_doe).to receive(:update_todos_count_cache).and_call_original expect(john_doe).to receive(:update_todos_count_cache).and_call_original
......
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