Merge branch 'better-commit-mentions' into 'master'

Clean up code around commit mentions.

Builds upon !527.

See merge request !528

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  = {

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
+    
+    users.push *self.mentioned_users(current_user, project)
+
+    notes(project).each do |note|
+      users << note.author
+      users.push *note.mentioned_users(current_user, project)
+    end
+
+    users.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

app/models/concerns/issuable.rb

@@ -122,15 +122,15 @@ module Issuable
     users = []
     users << author
     users << assignee if is_assigned?
-    mentions = []
-    mentions << self.mentioned_users(current_user)
+
+    users.push *self.mentioned_users(current_user)
 
     notes.each do |note|
       users << note.author
-      mentions << note.mentioned_users(current_user)
+      users.push *note.mentioned_users(current_user)
     end
 
-    users.concat(mentions.reduce([], :|)).uniq
+    users.uniq
   end
 
   def subscribed?(user)

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

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

app/models/snippet.rb

@@ -87,6 +87,18 @@ class Snippet < ActiveRecord::Base
     visibility_level
   end
 
+  def participants(current_user = self.author)
+    users = []
+    users << author
+    
+    notes.each do |note|
+      users << note.author
+      users.push *note.mentioned_users(current_user)
+    end
+
+    users.uniq
+  end
+
   class << self
     def search(query)
       where('(title LIKE :query OR file_name LIKE :query)', query: "%#{query}%")

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)
-        target.participants
-      elsif target.is_a?(Commit)
-        author_ids = Note.for_commit_id(target.id).pluck(:author_id).uniq
-        User.where(id: author_ids)
+      if target.is_a?(Commit)
+        target.participants(note.project, note.author)
+      elsif target.respond_to?(:participants)
+        target.participants(note.author)
       else
         note.mentioned_users
       end

app/services/projects/participants_service.rb

@@ -13,19 +13,21 @@ module Projects
     end
 
     def participants_in(type, id)
-      users = case type
-              when "Issue"
-                issue = project.issues.find_by_iid(id)
-                issue ? issue.participants(current_user) : []
-              when "MergeRequest"
-                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)
-              else
-                []
-              end
+      users = 
+        case type
+        when "Issue"
+          issue = project.issues.find_by_iid(id)
+          issue.participants(current_user) if issue
+        when "MergeRequest"
+          merge_request = project.merge_requests.find_by_iid(id)
+          merge_request.participants(current_user) if merge_request
+        when "Commit"
+          commit = project.repository.commit(id)
+          commit.participants(project, current_user) if commit
+        end
+
+      return [] unless users
+      
       sorted(users)
     end
 

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

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(@u_committer)
       end
 
       describe :new_note do