Backport of multiple_assignees_feature [ci skip]

8 files changed, 111 insertions, 30 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 26dbf4d9570..edf4e9e5d78 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -26,7 +26,6 @@ module Issuable
     cache_markdown_field :description, issuable_state_filter_enabled: true
 
     belongs_to :author, class_name: "User"
-    belongs_to :assignee, class_name: "User"
     belongs_to :updated_by, class_name: "User"
     belongs_to :milestone
     has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy do
@@ -65,11 +64,8 @@ module Issuable
     validates :title, presence: true, length: { maximum: 255 }
 
     scope :authored, ->(user) { where(author_id: user) }
-    scope :assigned_to, ->(u) { where(assignee_id: u.id)}
     scope :recent, -> { reorder(id: :desc) }
     scope :order_position_asc, -> { reorder(position: :asc) }
-    scope :assigned, -> { where("assignee_id IS NOT NULL") }
-    scope :unassigned, -> { where("assignee_id IS NULL") }
     scope :of_projects, ->(ids) { where(project_id: ids) }
     scope :of_milestones, ->(ids) { where(milestone_id: ids) }
     scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) }
@@ -92,7 +88,6 @@ module Issuable
     attr_mentionable :description
 
     participant :author
-    participant :assignee
     participant :notes_with_associations
 
     strip_attributes :title
@@ -102,13 +97,6 @@ module Issuable
     after_save :update_assignee_cache_counts, if: :assignee_id_changed?
     after_save :record_metrics, unless: :imported?
 
-    def update_assignee_cache_counts
-      # make sure we flush the cache for both the old *and* new assignees(if they exist)
-      previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was
-      previous_assignee&.update_cache_counts
-      assignee&.update_cache_counts
-    end
-
     # We want to use optimistic lock for cases when only title or description are involved
     # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html
     def locking_enabled?
@@ -237,10 +225,6 @@ module Issuable
     today? && created_at == updated_at
   end
 
-  def is_being_reassigned?
-    assignee_id_changed?
-  end
-
   def open?
     opened? || reopened?
   end
@@ -269,7 +253,11 @@ module Issuable
       # DEPRECATED
       repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
     }
-    hook_data[:assignee] = assignee.hook_attrs if assignee
+    if self.is_a?(Issue)
+      hook_data[:assignees] = assignees.map(&:hook_attrs) if assignees.any?
+    else
+      hook_data[:assignee] = assignee.hook_attrs if assignee
+    end
 
     hook_data
   end
@@ -331,11 +319,6 @@ module Issuable
     false
   end
 
-  def assignee_or_author?(user)
-    # We're comparing IDs here so we don't need to load any associations.
-    author_id == user.id || assignee_id == user.id
-  end
-
   def record_metrics
     metrics = self.metrics || create_metrics
     metrics.record!
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index f449229864d..a3472af5c55 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -40,7 +40,7 @@ module Milestoneish
   def issues_visible_to_user(user)
     memoize_per_user(user, :issues_visible_to_user) do
       IssuesFinder.new(user, issues_finder_params)
-        .execute.where(milestone_id: milestoneish_ids)
+        .execute.includes(:assignees).where(milestone_id: milestoneish_ids)
     end
   end
 
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 0afbca2cb32..538615130a7 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -36,7 +36,7 @@ class GlobalMilestone
     closed = count_by_state(milestones_by_state_and_title, 'closed')
     all = milestones_by_state_and_title.map { |(_, title), _| title }.uniq.count
 
-    { 
+    {
       opened: opened,
       closed: closed,
       all: all
@@ -86,7 +86,7 @@ class GlobalMilestone
   end
 
   def issues
-    @issues ||= Issue.of_milestones(milestoneish_ids).includes(:project, :assignee, :labels)
+    @issues ||= Issue.of_milestones(milestoneish_ids).includes(:project, :assignees, :labels)
   end
 
   def merge_requests
@@ -94,7 +94,7 @@ class GlobalMilestone
   end
 
   def participants
-    @participants ||= milestones.includes(:participants).map(&:participants).flatten.compact.uniq
+    @participants ||= milestones.map(&:participants).flatten.uniq
   end
 
   def labels
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 78bde6820da..27e3ed9bc7f 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -24,10 +24,17 @@ class Issue < ActiveRecord::Base
 
   has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
 
+  has_many :issue_assignees
+  has_many :assignees, class_name: "User", through: :issue_assignees
+
   validates :project, presence: true
 
   scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
 
+  scope :assigned, -> { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') }
+  scope :unassigned, -> { where('NOT EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') }
+  scope :assigned_to, ->(u) { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = ? AND issue_id = issues.id)', u.id)}
+
   scope :without_due_date, -> { where(due_date: nil) }
   scope :due_before, ->(date) { where('issues.due_date < ?', date) }
   scope :due_between, ->(from_date, to_date) { where('issues.due_date >= ?', from_date).where('issues.due_date <= ?', to_date) }
@@ -37,13 +44,15 @@ class Issue < ActiveRecord::Base
 
   scope :created_after, -> (datetime) { where("created_at >= ?", datetime) }
 
-  scope :include_associations, -> { includes(:assignee, :labels, project: :namespace) }
+  scope :include_associations, -> { includes(:labels, project: :namespace) }
 
   after_save :expire_etag_cache
 
   attr_spammable :title, spam_title: true
   attr_spammable :description, spam_description: true
 
+  participant :assignees
+
   state_machine :state, initial: :opened do
     event :close do
       transition [:reopened, :opened] => :closed
@@ -63,10 +72,14 @@ class Issue < ActiveRecord::Base
   end
 
   def hook_attrs
+    assignee_ids = self.assignee_ids
+
     attrs = {
       total_time_spent: total_time_spent,
       human_total_time_spent: human_total_time_spent,
-      human_time_estimate: human_time_estimate
+      human_time_estimate: human_time_estimate,
+      assignee_ids: assignee_ids,
+      assignee_id: assignee_ids.first # This key is deprecated
     }
 
     attributes.merge!(attrs)
@@ -114,6 +127,22 @@ class Issue < ActiveRecord::Base
               "id DESC")
   end
 
+  # Returns a Hash of attributes to be used for Twitter card metadata
+  def card_attributes
+    {
+      'Author'   => author.try(:name),
+      'Assignee' => assignee_list
+    }
+  end
+
+  def assignee_or_author?(user)
+    author_id == user.id || assignees.exists?(user.id)
+  end
+
+  def assignee_list
+    assignees.map(&:name).to_sentence
+  end
+
   # `from` argument can be a Namespace or Project.
   def to_reference(from = nil, full: false)
     reference = "#{self.class.reference_prefix}#{iid}"
@@ -248,7 +277,7 @@ class Issue < ActiveRecord::Base
       true
     elsif confidential?
       author == user ||
-        assignee == user ||
+        assignees.include?(user) ||
         project.team.member?(user, Gitlab::Access::REPORTER)
     else
       project.public? ||
diff --git a/app/models/issue_assignee.rb b/app/models/issue_assignee.rb
new file mode 100644
index 00000000000..b94b55bb1d1
--- /dev/null
+++ b/app/models/issue_assignee.rb
@@ -0,0 +1,29 @@
+class IssueAssignee < ActiveRecord::Base
+  extend Gitlab::CurrentSettings
+
+  belongs_to :issue
+  belongs_to :assignee, class_name: "User", foreign_key: :user_id
+
+  after_create :update_assignee_cache_counts
+  after_destroy :update_assignee_cache_counts
+
+  # EE-specific
+  after_create :update_elasticsearch_index
+  after_destroy :update_elasticsearch_index
+  # EE-specific
+
+  def update_assignee_cache_counts
+    assignee&.update_cache_counts
+  end
+
+  def update_elasticsearch_index
+    if current_application_settings.elasticsearch_indexing?
+      ElasticIndexerWorker.perform_async(
+        :update,
+        'Issue',
+        issue.id,
+        changed_fields: ['assignee_ids']
+      )
+    end
+  end
+end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 12c5481cd6d..35231bab12e 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -17,6 +17,8 @@ class MergeRequest < ActiveRecord::Base
 
   has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
 
+  belongs_to :assignee, class_name: "User"
+
   serialize :merge_params, Hash
 
   after_create :ensure_merge_request_diff, unless: :importing?
@@ -114,8 +116,14 @@ class MergeRequest < ActiveRecord::Base
 
   scope :join_project, -> { joins(:target_project) }
   scope :references_project, -> { references(:target_project) }
+  scope :assigned, -> { where("assignee_id IS NOT NULL") }
+  scope :unassigned, -> { where("assignee_id IS NULL") }
+  scope :assigned_to, ->(u) { where(assignee_id: u.id)}
+
+  participant :assignee
 
   after_save :keep_around_commit
+  after_save :update_assignee_cache_counts, if: :assignee_id_changed?
 
   def self.reference_prefix
     '!'
@@ -177,6 +185,30 @@ class MergeRequest < ActiveRecord::Base
     work_in_progress?(title) ? title : "WIP: #{title}"
   end
 
+  def update_assignee_cache_counts
+    # make sure we flush the cache for both the old *and* new assignees(if they exist)
+    previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was
+    previous_assignee&.update_cache_counts
+    assignee&.update_cache_counts
+  end
+
+  # Returns a Hash of attributes to be used for Twitter card metadata
+  def card_attributes
+    {
+      'Author'   => author.try(:name),
+      'Assignee' => assignee.try(:name)
+    }
+  end
+
+  # This method is needed for compatibility with issues to not mess view and other code
+  def assignees
+    Array(assignee)
+  end
+
+  def assignee_or_author?(user)
+    author_id == user.id || assignee_id == user.id
+  end
+
   # `from` argument can be a Namespace or Project.
   def to_reference(from = nil, full: false)
     reference = "#{self.class.reference_prefix}#{iid}"
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 652b1551928..c06bfe0ccdd 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -21,7 +21,6 @@ class Milestone < ActiveRecord::Base
   has_many :issues
   has_many :labels, -> { distinct.reorder('labels.title') },  through: :issues
   has_many :merge_requests
-  has_many :participants, -> { distinct.reorder('users.name') }, through: :issues, source: :assignee
   has_many :events, as: :target, dependent: :destroy
 
   scope :active, -> { with_state(:active) }
@@ -107,6 +106,10 @@ class Milestone < ActiveRecord::Base
     end
   end
 
+  def participants
+    User.joins(assigned_issues: :milestone).where("milestones.id = ?", id)
+  end
+
   def self.sort(method)
     case method.to_s
     when 'due_date_asc'
diff --git a/app/models/user.rb b/app/models/user.rb
index 2b7ebe6c1a7..a3126cbb644 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -89,6 +89,7 @@ class User < ActiveRecord::Base
   has_many :subscriptions,            dependent: :destroy
   has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id,   class_name: "Event"
   has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy
+
   has_one  :abuse_report,             dependent: :destroy, foreign_key: :user_id
   has_many :reported_abuse_reports,   dependent: :destroy, foreign_key: :reporter_id, class_name: "AbuseReport"
   has_many :spam_logs,                dependent: :destroy
@@ -99,6 +100,10 @@ class User < ActiveRecord::Base
   has_many :award_emoji,              dependent: :destroy
   has_many :triggers,                 dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id
 
+  has_many :issue_assignees
+  has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue
+  has_many :assigned_merge_requests,  dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest"
+
   # Issues that a user owns are expected to be moved to the "ghost" user before
   # the user is destroyed. If the user owns any issues during deletion, this
   # should be treated as an exceptional condition.