Clean up code around commit mentions.

8 files changed, 31 insertions, 25 deletions

diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 87e39f1363a..894cf93b9ae 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -10,11 +10,11 @@ class Projects::CommitController < Projects::ApplicationController
   def show
     return git_not_found! unless @commit
 
-    @line_notes = @project.notes.for_commit_id(commit.id).inline
+    @line_notes = commit.notes(@project).inline
     @diffs = @commit.diffs
     @note = @project.build_commit_note(commit)
-    @notes_count = @project.notes.for_commit_id(commit.id).count
-    @notes = @project.notes.for_commit_id(@commit.id).not_inline.fresh
+    @notes_count = commit.notes(@project).count
+    @notes = commit.notes(@project).not_inline.fresh
     @noteable = @commit
     @comments_allowed = @reply_allowed = true
     @comments_target  = {
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 006fa62c8f9..f54696c378c 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -134,6 +134,25 @@ class Commit
     User.find_for_commit(committer_email, committer_name)
   end
 
+  def participants(project, current_user = nil)
+    users = []
+    users << author
+    users << committer
+    mentions = []
+    mentions << self.mentioned_users(current_user, project)
+
+    notes(project).each do |note|
+      users << note.author
+      mentions << note.mentioned_users(current_user, project)
+    end
+
+    users.concat(mentions.reduce([], :|)).uniq
+  end
+
+  def notes(project)
+    project.notes.for_commit_id(self.id)
+  end
+
   def method_missing(m, *args, &block)
     @raw.send(m, *args, &block)
   end
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index b7882a2bb16..acd9a1edc48 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -42,10 +42,10 @@ module Mentionable
     Note.cross_reference_exists?(target, local_reference)
   end
 
-  def mentioned_users(current_user = nil)
+  def mentioned_users(current_user = nil, p = project)
     return [] if mentionable_text.blank?
 
-    ext = Gitlab::ReferenceExtractor.new(self.project, current_user)
+    ext = Gitlab::ReferenceExtractor.new(p, current_user)
     ext.analyze(mentionable_text)
     ext.users.uniq
   end
diff --git a/app/models/note.rb b/app/models/note.rb
index fdab4517df3..8ebeb371352 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -327,14 +327,6 @@ class Note < ActiveRecord::Base
     current_application_settings.max_attachment_size.megabytes.to_i
   end
 
-  def commit_author
-    @commit_author ||=
-      project.team.users.find_by(email: noteable.author_email) ||
-      project.team.users.find_by(name: noteable.author_name)
-  rescue
-    nil
-  end
-
   def cross_reference?
     note.start_with?(self.class.cross_reference_note_prefix)
   end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index cfed7964c37..1337dca8354 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -127,17 +127,12 @@ class NotificationService
 
     recipients = []
 
-    if note.commit_id.present?
-      recipients << note.commit_author
-    end
-    
     # Add all users participating in the thread (author, assignee, comment authors)
     participants = 
-      if target.respond_to?(:participants)
+      if target.is_a?(Commit)
+        target.participants(note.project)
+      elsif target.respond_to?(:participants)
         target.participants
-      elsif target.is_a?(Commit)
-        author_ids = Note.for_commit_id(target.id).pluck(:author_id).uniq
-        User.where(id: author_ids)
       else
         note.mentioned_users
       end
diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb
index ae6260bcdab..dcbf348153d 100644
--- a/app/services/projects/participants_service.rb
+++ b/app/services/projects/participants_service.rb
@@ -21,8 +21,8 @@ module Projects
                 merge_request = project.merge_requests.find_by_iid(id)
                 merge_request ? merge_request.participants(current_user) : []
               when "Commit"
-                author_ids = Note.for_commit_id(id).pluck(:author_id).uniq
-                User.where(id: author_ids)
+                commit = project.repository.commit(id)
+                commit ? commit.participants(project, current_user) : []
               else
                 []
               end
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index c6026f96804..8e1aaa4d051 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -12,7 +12,7 @@
       - if @note_counts
         - note_count = @note_counts.fetch(commit.id, 0)
       - else
-        - notes = project.notes.for_commit_id(commit.id)
+        - notes = commit.notes(project)
         - note_count = notes.user.count
 
       - if note_count > 0
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index bfca2c88264..0faff48c059 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -149,7 +149,7 @@ describe NotificationService do
 
       before do
         build_team(note.project)
-        note.stub(:commit_author => @u_committer)
+        allow_any_instance_of(Commit).to receive(:author).and_return()
       end
 
       describe :new_note do