Create a pending task when a user is mentioned on issue/mr

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