Use participants from database table instead of parsing all commentsparticipants-table

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

14 files changed, 47 insertions, 75 deletions

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index bfafdeeb1fb..33a12cfb192 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -55,7 +55,7 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def show
-    @participants = @issue.participants(current_user, @project)
+    @participants = @issue.participants
     @note = @project.notes.new(noteable: @issue)
     @notes = @issue.notes.inc_author.fresh
     @noteable = @issue
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index d1265198318..623f9a32141 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -246,7 +246,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def define_show_vars
-    @participants = @merge_request.participants(current_user, @project)
+    @participants = @merge_request.participants
 
     # Build a note object for comment form
     @note = @project.notes.new(noteable: @merge_request)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index aff329d71fa..b9569c00421 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -8,7 +8,6 @@ class Commit
   include StaticModel
 
   attr_mentionable :safe_message
-  participant :author, :committer, :notes, :mentioned_users
 
   attr_accessor :project
 
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 8c1c8851f52..4f7b8c6419c 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -17,7 +17,6 @@ module Issuable
     has_many :label_links, as: :target, dependent: :destroy
     has_many :labels, through: :label_links
     has_many :subscriptions, dependent: :destroy, as: :subscribable
-    # has_many :participants, dependent: :destroy, as: :target
 
     validates :author, presence: true
     validates :title, presence: true, length: { within: 0..255 }
@@ -47,7 +46,6 @@ module Issuable
              prefix: true
 
     attr_mentionable :title, :description
-    participant :author, :assignee, :notes, :mentioned_users
   end
 
   module ClassMethods
@@ -127,7 +125,7 @@ module Issuable
       return subscription.subscribed
     end
 
-    participants(user).include?(user)
+    participants.include?(user)
   end
 
   def toggle_subscription(user)
diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb
index 7c9597333dd..c81e6bbf567 100644
--- a/app/models/concerns/participable.rb
+++ b/app/models/concerns/participable.rb
@@ -9,10 +9,6 @@
 #
 #     class Issue < ActiveRecord::Base
 #       include Participable
-#
-#       # ...
-#
-#       participant :author, :assignee, :mentioned_users, :notes
 #     end
 #
 #     issue = Issue.last
@@ -23,53 +19,14 @@
 #     # since Note implements Participable as well.
 #
 module Participable
-  extend ActiveSupport::Concern
-
-  module ClassMethods
-    def participant(*attrs)
-      participant_attrs.concat(attrs.map(&:to_s))
-    end
-
-    def participant_attrs
-      @participant_attrs ||= []
-    end
-  end
-
-  # Be aware that this method makes a lot of sql queries.
-  # Save result into variable if you are going to reuse it inside same request
-  def participants(current_user = self.author, project = self.project)
-    participants = self.class.participant_attrs.flat_map do |attr|
-      meth = method(attr)
-
-      value =
-        if meth.arity == 1 || meth.arity == -1
-          meth.call(current_user)
-        else
-          meth.call
-        end
-
-      participants_for(value, current_user, project)
-    end.compact.uniq
-
-    if project
-      participants.select! do |user|
-        user.can?(:read_project, project)
+  def participants
+    @participants ||=
+      begin
+        user_ids = Participant.
+          where(target_id: id.to_s, target_type: self.class.name).
+          pluck(:user_id)
+
+        User.where(id: user_ids)
       end
-    end
-
-    participants
-  end
-
-  private
-
-  def participants_for(value, current_user = nil, project = nil)
-    case value
-    when User
-      [value]
-    when Enumerable, ActiveRecord::Relation
-      value.flat_map { |v| participants_for(v, current_user, project) }
-    when Participable
-      value.participants(current_user, project)
-    end
   end
 end
diff --git a/app/models/note.rb b/app/models/note.rb
index 68b9d433ae0..00ac2534039 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -23,12 +23,10 @@ require 'file_size_validator'
 class Note < ActiveRecord::Base
   include Mentionable
   include Gitlab::CurrentSettings
-  include Participable
 
   default_value_for :system, false
 
   attr_mentionable :note
-  participant :author, :mentioned_users
 
   belongs_to :project
   belongs_to :noteable, polymorphic: true, touch: true
diff --git a/app/models/participant.rb b/app/models/participant.rb
index 320bd260c60..0e192155683 100644
--- a/app/models/participant.rb
+++ b/app/models/participant.rb
@@ -2,7 +2,8 @@ class Participant < ActiveRecord::Base
   belongs_to :user
   belongs_to :target, polymorphic: true
 
-  validates :target, presence: true
+  validates :target_id, presence: true
+  validates :target_type, presence: true
   validates :user_id,
             uniqueness: { scope: [:target_id, :target_type] },
             presence: true
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index b0831982aa7..998d24d3cb4 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -49,8 +49,6 @@ class Snippet < ActiveRecord::Base
   scope :expired, -> { where(["expires_at IS NOT NULL AND expires_at < ?", Time.current]) }
   scope :non_expired, -> { where(["expires_at IS NULL OR expires_at > ?", Time.current]) }
 
-  participant :author, :notes
-
   def self.reference_prefix
     '$'
   end
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index 482c0444049..de5c5504d03 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -7,6 +7,7 @@ module Notes
 
       if note.save
         notification_service.new_note(note)
+        update_participants(note)
 
         # Skip system notes, like status changes and cross-references.
         unless note.system
@@ -34,5 +35,25 @@ module Notes
       note.project.execute_hooks(note_data, :note_hooks)
       note.project.execute_services(note_data, :note_hooks)
     end
+
+    def update_participants(note)
+      users = [note.author, note.mentioned_users].flatten
+      noteable = note.noteable
+
+      # Add users as participants only if they have access to project
+      users.select! do |user|
+        user.can?(:read_project, note.project)
+      end
+
+      users.each do |user|
+        if Participant.where(user_id: user, target_id: noteable.id.to_s, target_type: noteable.class.name).blank?
+          participant = Participant.new
+          participant.target_id = noteable.id
+          participant.target_type = noteable.class.name
+          participant.user = user
+          participant.save
+        end
+      end
+    end
   end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 312b56eb87b..8880d16387c 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -132,9 +132,9 @@ class NotificationService
     recipients = []
 
     # Add all users participating in the thread (author, assignee, comment authors)
-    participants = 
+    participants =
       if target.respond_to?(:participants)
-        target.participants(note.author)
+        target.participants
       else
         note.mentioned_users
       end
@@ -344,7 +344,7 @@ class NotificationService
 
   def reject_unsubscribed_users(recipients, target)
     return recipients unless target.respond_to? :subscriptions
-    
+
     recipients.reject do |user|
       subscription = target.subscriptions.find_by_user_id(user.id)
       subscription && !subscription.subscribed
@@ -362,7 +362,7 @@ class NotificationService
       recipients
     end
   end
-  
+
   def new_resource_email(target, project, method)
     recipients = build_recipients(target, project)
     recipients.delete(target.author)
diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb
index 0004a399f47..657bbcffc9b 100644
--- a/app/services/projects/participants_service.rb
+++ b/app/services/projects/participants_service.rb
@@ -13,7 +13,7 @@ module Projects
     end
 
     def participants_in(type, id)
-      target = 
+      target =
         case type
         when "Issue"
           project.issues.find_by_iid(id)
@@ -22,21 +22,21 @@ module Projects
         when "Commit"
           project.commit(id)
         end
-        
+
       return [] unless target
 
-      users = target.participants(current_user)
+      users = target.participants
       sorted(users)
     end
 
     def sorted(users)
-      users.uniq.to_a.compact.sort_by(&:username).map do |user| 
+      users.uniq.to_a.compact.sort_by(&:username).map do |user|
         { username: user.username, name: user.name }
       end
     end
 
     def groups
-      current_user.authorized_groups.sort_by(&:path).map do |group| 
+      current_user.authorized_groups.sort_by(&:path).map do |group|
         count = group.users.count
         { username: group.path, name: group.name, count: count }
       end
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 0a77f200f56..4a1009686c6 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -21,7 +21,7 @@
           - if member
             %span.note-role.label
               = member.human_access
-              
+
         - if note.system
           = link_to user_path(note.author) do
             = image_tag avatar_icon(note.author_email), class: 'avatar s16', alt: ''
diff --git a/db/migrate/20150625153454_create_participants.rb b/db/migrate/20150625153454_create_participants.rb
index ca680271236..c5ecad6969e 100644
--- a/db/migrate/20150625153454_create_participants.rb
+++ b/db/migrate/20150625153454_create_participants.rb
@@ -1,7 +1,7 @@
 class CreateParticipants < ActiveRecord::Migration
   def change
     create_table :participants do |t|
-      t.integer :target_id, null: false
+      t.string :target_id, null: false
       t.string :target_type, null: false
       t.integer :user_id, null: false
 
diff --git a/db/schema.rb b/db/schema.rb
index e7a55197f42..3aeb8ce0d31 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -331,7 +331,7 @@ ActiveRecord::Schema.define(version: 20150625153454) do
   add_index "oauth_applications", ["uid"], name: "index_oauth_applications_on_uid", unique: true, using: :btree
 
   create_table "participants", force: true do |t|
-    t.integer  "target_id",   null: false
+    t.string   "target_id",   null: false
     t.string   "target_type", null: false
     t.integer  "user_id",     null: false
     t.datetime "created_at"
@@ -504,12 +504,12 @@ ActiveRecord::Schema.define(version: 20150625153454) do
     t.string   "bitbucket_access_token"
     t.string   "bitbucket_access_token_secret"
     t.string   "location"
+    t.string   "public_email",                  default: "",    null: false
     t.string   "encrypted_otp_secret"
     t.string   "encrypted_otp_secret_iv"
     t.string   "encrypted_otp_secret_salt"
     t.boolean  "otp_required_for_login",        default: false, null: false
     t.text     "otp_backup_codes"
-    t.string   "public_email",                  default: "",    null: false
     t.integer  "dashboard",                     default: 0
   end