diff options
-rw-r--r-- | app/models/task.rb | 2 | ||||
-rw-r--r-- | app/services/task_service.rb | 32 | ||||
-rw-r--r-- | spec/services/task_service_spec.rb | 96 |
3 files changed, 94 insertions, 36 deletions
diff --git a/app/models/task.rb b/app/models/task.rb index b9e5152b819..c9881991e38 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -16,6 +16,7 @@ class Task < ActiveRecord::Base ASSIGNED = 1 + MENTIONED = 2 belongs_to :author, class_name: "User" belongs_to :project @@ -43,6 +44,7 @@ class Task < ActiveRecord::Base def action_name case action when ASSIGNED then 'assigned' + when MENTIONED then 'mentioned on' end end diff --git a/app/services/task_service.rb b/app/services/task_service.rb index 4c040ee0fff..fa615f4cfcf 100644 --- a/app/services/task_service.rb +++ b/app/services/task_service.rb @@ -67,8 +67,7 @@ class TaskService # * mark all pending tasks related to the target for the user as done # def mark_pending_tasks_as_done(target, user) - pending_tasks = pending_tasks_for(user, target.project, target) - pending_tasks.update_all(state: :done) + pending_tasks(user, target.project, target).update_all(state: :done) end # When create a note we should: @@ -108,13 +107,34 @@ class TaskService Task.create(attributes) end - def pending_tasks_for(user, project, target) + def build_mentioned_users(project, target, author) + mentioned_users = target.mentioned_users.select do |user| + user.can?(:read_project, project) + end + + mentioned_users.delete(author) + mentioned_users.delete(target.assignee) if target.respond_to?(:assignee) + + mentioned_users.uniq + end + + def pending_tasks(user, project, target) user.tasks.pending.where(project: project, target: target) end - def new_issuable(issuable, user) - if issuable.is_assigned? && issuable.assignee != user - create_task(issuable.project, issuable, user, issuable.assignee, Task::ASSIGNED) + def new_issuable(issuable, current_user) + project = issuable.project + target = issuable + author = issuable.author + + if target.is_assigned? && target.assignee != current_user + create_task(project, target, author, target.assignee, Task::ASSIGNED) + end + + mentioned_users = build_mentioned_users(project, target, author) + + mentioned_users.each do |mentioned_user| + create_task(project, target, author, mentioned_user, Task::MENTIONED) end end diff --git a/spec/services/task_service_spec.rb b/spec/services/task_service_spec.rb index 0cd0011b041..b35715c8b7a 100644 --- a/spec/services/task_service_spec.rb +++ b/spec/services/task_service_spec.rb @@ -2,46 +2,61 @@ require 'spec_helper' describe TaskService, services: true do let(:author) { create(:user) } - let(:john_doe) { create(:user) } + let(:john_doe) { create(:user, username: 'john_doe') } + let(:michael) { create(:user, username: 'michael') } + let(:stranger) { create(:user, username: 'stranger') } let(:project) { create(:project) } + let(:mentions) { [author.to_reference, john_doe.to_reference, michael.to_reference, stranger.to_reference].join(' ') } let(:service) { described_class.new } before do project.team << [author, :developer] project.team << [john_doe, :developer] + project.team << [michael, :developer] end describe 'Issues' do - let(:assigned_issue) { create(:issue, project: project, assignee: john_doe) } + let(:issue) { create(:issue, project: project, assignee: john_doe, author: author) } let(:unassigned_issue) { create(:issue, project: project, assignee: nil) } describe '#new_issue' do - it 'creates a pending task if assigned' do - service.new_issue(assigned_issue, author) + it 'creates a task if assigned' do + service.new_issue(issue, author) - is_expected_to_create_pending_task(user: john_doe, target: assigned_issue, action: Task::ASSIGNED) + should_create_task(user: john_doe, target: issue, action: Task::ASSIGNED) end it 'does not create a task if unassigned' do - is_expected_to_not_create_task { service.new_issue(unassigned_issue, author) } + should_not_create_any_task { service.new_issue(unassigned_issue, author) } end it 'does not create a task if assignee is the current user' do - is_expected_to_not_create_task { service.new_issue(unassigned_issue, john_doe) } + should_not_create_any_task { service.new_issue(unassigned_issue, john_doe) } + end + + it 'creates a task for each valid mentioned user' do + issue.update_attribute(:description, mentions) + + service.new_issue(issue, author) + + should_create_task(user: michael, target: issue, action: Task::MENTIONED) + should_not_create_task(user: author, target: issue, action: Task::MENTIONED) + should_not_create_task(user: john_doe, target: issue, action: Task::MENTIONED) + should_not_create_task(user: stranger, target: issue, action: Task::MENTIONED) end end describe '#close_issue' do let!(:first_pending_task) do - create(:pending_assigned_task, user: john_doe, project: project, target: assigned_issue, author: author) + create(:pending_assigned_task, user: john_doe, project: project, target: issue, author: author) end let!(:second_pending_task) do - create(:pending_assigned_task, user: john_doe, project: project, target: assigned_issue, author: author) + create(:pending_assigned_task, user: john_doe, project: project, target: issue, author: author) end it 'marks related pending tasks to the target for the user as done' do - service.close_issue(assigned_issue, john_doe) + service.close_issue(issue, john_doe) expect(first_pending_task.reload).to be_done expect(second_pending_task.reload).to be_done @@ -53,27 +68,27 @@ describe TaskService, services: true do unassigned_issue.update_attribute(:assignee, john_doe) service.reassigned_issue(unassigned_issue, author) - is_expected_to_create_pending_task(user: john_doe, target: unassigned_issue, action: Task::ASSIGNED) + should_create_task(user: john_doe, target: unassigned_issue, action: Task::ASSIGNED) end it 'does not create a task if unassigned' do - assigned_issue.update_attribute(:assignee, nil) + issue.update_attribute(:assignee, nil) - is_expected_to_not_create_task { service.reassigned_issue(assigned_issue, author) } + should_not_create_any_task { service.reassigned_issue(issue, author) } end end describe '#mark_pending_tasks_as_done' do let!(:first_pending_task) do - create(:pending_assigned_task, user: john_doe, project: project, target: assigned_issue, author: author) + create(:pending_assigned_task, user: john_doe, project: project, target: issue, author: author) end let!(:second_pending_task) do - create(:pending_assigned_task, user: john_doe, project: project, target: assigned_issue, author: author) + create(:pending_assigned_task, user: john_doe, project: project, target: issue, author: author) end it 'marks related pending tasks to the target for the user as done' do - service.mark_pending_tasks_as_done(assigned_issue, john_doe) + service.mark_pending_tasks_as_done(issue, john_doe) expect(first_pending_task.reload).to be_done expect(second_pending_task.reload).to be_done @@ -82,16 +97,16 @@ describe TaskService, services: true do describe '#new_note' do let!(:first_pending_task) do - create(:pending_assigned_task, user: john_doe, project: project, target: assigned_issue, author: author) + create(:pending_assigned_task, user: john_doe, project: project, target: issue, author: author) end let!(:second_pending_task) do - create(:pending_assigned_task, user: john_doe, project: project, target: assigned_issue, author: author) + create(:pending_assigned_task, user: john_doe, project: project, target: issue, author: author) end - let(:note) { create(:note, project: project, noteable: assigned_issue, author: john_doe) } - let(:award_note) { create(:note, :award, project: project, noteable: assigned_issue, author: john_doe, note: 'thumbsup') } - let(:system_note) { create(:system_note, project: project, noteable: assigned_issue) } + let(:note) { create(:note, project: project, noteable: issue, author: john_doe) } + let(:award_note) { create(:note, :award, project: project, noteable: issue, author: john_doe, note: 'thumbsup') } + let(:system_note) { create(:system_note, project: project, noteable: issue) } it 'mark related pending tasks to the noteable for the note author as done' do service.new_note(note) @@ -117,22 +132,33 @@ describe TaskService, services: true do end describe 'Merge Requests' do - let(:mr_assigned) { create(:merge_request, source_project: project, assignee: john_doe) } - let(:mr_unassigned) { create(:merge_request, source_project: project, assignee: nil) } + let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe) } + let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignee: nil) } describe '#new_merge_request' do it 'creates a pending task if assigned' do service.new_merge_request(mr_assigned, author) - is_expected_to_create_pending_task(user: john_doe, target: mr_assigned, action: Task::ASSIGNED) + should_create_task(user: john_doe, target: mr_assigned, action: Task::ASSIGNED) end it 'does not create a task if unassigned' do - is_expected_to_not_create_task { service.new_merge_request(mr_unassigned, author) } + should_not_create_any_task { service.new_merge_request(mr_unassigned, author) } end it 'does not create a task if assignee is the current user' do - is_expected_to_not_create_task { service.new_merge_request(mr_unassigned, john_doe) } + should_not_create_any_task { service.new_merge_request(mr_unassigned, john_doe) } + end + + it 'creates a task for each valid mentioned user' do + mr_assigned.update_attribute(:description, mentions) + + service.new_merge_request(mr_assigned, author) + + should_create_task(user: michael, target: mr_assigned, action: Task::MENTIONED) + should_not_create_task(user: author, target: mr_assigned, action: Task::MENTIONED) + should_not_create_task(user: john_doe, target: mr_assigned, action: Task::MENTIONED) + should_not_create_task(user: stranger, target: mr_assigned, action: Task::MENTIONED) end end @@ -158,13 +184,13 @@ describe TaskService, services: true do mr_unassigned.update_attribute(:assignee, john_doe) service.reassigned_merge_request(mr_unassigned, author) - is_expected_to_create_pending_task(user: john_doe, target: mr_unassigned, action: Task::ASSIGNED) + should_create_task(user: john_doe, target: mr_unassigned, action: Task::ASSIGNED) end it 'does not create a task if unassigned' do mr_assigned.update_attribute(:assignee, nil) - is_expected_to_not_create_task { service.reassigned_merge_request(mr_assigned, author) } + should_not_create_any_task { service.reassigned_merge_request(mr_assigned, author) } end end @@ -186,7 +212,7 @@ describe TaskService, services: true do end end - def is_expected_to_create_pending_task(attributes = {}) + def should_create_task(attributes = {}) attributes.reverse_merge!( project: project, author: author, @@ -196,7 +222,17 @@ describe TaskService, services: true do expect(Task.where(attributes).count).to eq 1 end - def is_expected_to_not_create_task + def should_not_create_task(attributes = {}) + attributes.reverse_merge!( + project: project, + author: author, + state: :pending + ) + + expect(Task.where(attributes).count).to eq 0 + end + + def should_not_create_any_task expect { yield }.not_to change(Task, :count) end end |