[CE] Support multiple assignees for merge requests

Backports https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/10161
(code out of ee/ folder).

app/assets/javascripts/issuable_bulk_update_actions.js

@@ -81,9 +81,6 @@ export default {
     const formData = {
       update: {
         state_event: this.form.find('input[name="update[state_event]"]').val(),
-        // For Merge Requests
-        assignee_id: this.form.find('input[name="update[assignee_id]"]').val(),
-        // For Issues
         assignee_ids: [this.form.find('input[name="update[assignee_ids][]"]').val()],
         milestone_id: this.form.find('input[name="update[milestone_id]"]').val(),
         issuable_ids: this.form.find('input[name="update[issuable_ids]"]').val(),

app/assets/javascripts/sidebar/components/assignees/assignees.vue

@@ -74,8 +74,7 @@ export default {
       }
       if (!this.users.length) {
-        const emptyTooltipLabel =
-          this.issuableType === 'issue' ? __('Assignee(s)') : __('Assignee');
+        const emptyTooltipLabel = __('Assignee(s)');
         names.push(emptyTooltipLabel);
       }
@@ -90,6 +89,27 @@ export default {
       return counter;
     },
+    mergeNotAllowedTooltipMessage() {
+      const assigneesCount = this.users.length;
+
+      if (this.issuableType !== 'merge_request' || assigneesCount === 0) {
+        return null;
+      }
+
+      const cannotMergeCount = this.users.filter(u => u.can_merge === false).length;
+      const canMergeCount = assigneesCount - cannotMergeCount;
+
+      if (canMergeCount === assigneesCount) {
+        // Everyone can merge
+        return null;
+      } else if (cannotMergeCount === assigneesCount && assigneesCount > 1) {
+        return 'No one can merge';
+      } else if (assigneesCount === 1) {
+        return 'Cannot merge';
+      }
+
+      return `${canMergeCount}/${assigneesCount} can merge`;
+    },
   },
   methods: {
     assignSelf() {
@@ -154,6 +174,15 @@ export default {
       </button>
     </div>
     <div class="value hide-collapsed">
+      <span
+        v-if="mergeNotAllowedTooltipMessage"
+        v-tooltip
+        :title="mergeNotAllowedTooltipMessage"
+        data-placement="left"
+        class="float-right cannot-be-merged"
+      >
+        <i aria-hidden="true" data-hidden="true" class="fa fa-exclamation-triangle"></i>
+      </span>
       <template v-if="hasNoUsers">
         <span class="assign-yourself no-value">
           No assignee

app/assets/stylesheets/pages/merge_requests.scss

@@ -498,6 +498,16 @@
       flex: 1;
     }
+    .issuable-meta {
+      .author-link {
+        display: inline-block;
+      }
+
+      .issuable-comments {
+        height: 18px;
+      }
+    }
+
     .merge-request-title {
       margin-bottom: 2px;

app/assets/stylesheets/pages/projects.scss

@@ -1158,6 +1158,8 @@ pre.light-well {
 .cannot-be-merged:hover {
   color: $red-500;
   margin-top: 2px;
+  position: relative;
+  z-index: 2;
 }
 .private-forks-notice .private-fork-icon {

app/controllers/concerns/issuable_actions.rb

@@ -192,12 +192,7 @@ module IssuableActions
   def bulk_update_params
     permitted_keys_array = permitted_keys.dup
-
-    if resource_name == 'issue'
-      permitted_keys_array << { assignee_ids: [] }
-    else
-      permitted_keys_array.unshift(:assignee_id)
-    end
+    permitted_keys_array << { assignee_ids: [] }
     params.require(:update).permit(permitted_keys_array)
   end

app/controllers/concerns/issuable_collections.rb

@@ -190,15 +190,15 @@ module IssuableCollections
                          end
   end
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def preload_for_collection
+    common_attributes = [:author, :assignees, :labels, :milestone]
     @preload_for_collection ||= case collection_type
                                 when 'Issue'
-                                  [:project, :author, :assignees, :labels, :milestone, project: :namespace]
+                                  common_attributes + [:project, project: :namespace]
                                 when 'MergeRequest'
-                                  [
-                                    :target_project, :author, :assignee, :labels, :milestone,
-                                    source_project: :route, head_pipeline: :project, target_project: :namespace, latest_merge_request_diff: :merge_request_diff_commits
-                                  ]
+                                  common_attributes + [:target_project, source_project: :route, head_pipeline: :project, target_project: :namespace, latest_merge_request_diff: :merge_request_diff_commits]
                                 end
   end
+  # rubocop:enable Gitlab/ModuleWithInstanceVariables
 end

app/controllers/projects/merge_requests/application_controller.rb

@@ -20,7 +20,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
   def merge_request_params_attributes
     [
       :allow_collaboration,
-      :assignee_id,
       :description,
       :force_remove_source_branch,
       :lock_version,
@@ -35,6 +34,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
       :title,
       :discussion_locked,
       label_ids: [],
+      assignee_ids: [],
       update_task: [:index, :checked, :line_number, :line_source]
     ]
   end

app/finders/issuable_finder.rb

@@ -439,22 +439,6 @@ class IssuableFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
-  # rubocop: disable CodeReuse/ActiveRecord
-  def by_assignee(items)
-    if filter_by_no_assignee?
-      items.where(assignee_id: nil)
-    elsif filter_by_any_assignee?
-      items.where('assignee_id IS NOT NULL')
-    elsif assignee
-      items.where(assignee_id: assignee.id)
-    elsif assignee_id? || assignee_username? # assignee not found
-      items.none
-    else
-      items
-    end
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-
   def filter_by_no_assignee?
     # Assignee_id takes precedence over assignee_username
     [NONE, FILTER_NONE].include?(params[:assignee_id].to_s.downcase) || params[:assignee_username].to_s == NONE
@@ -478,6 +462,20 @@ class IssuableFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
+  def by_assignee(items)
+    if filter_by_no_assignee?
+      items.unassigned
+    elsif filter_by_any_assignee?
+      items.assigned
+    elsif assignee
+      items.assigned_to(assignee)
+    elsif assignee_id? || assignee_username? # assignee not found
+      items.none
+    else
+      items
+    end
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def by_milestone(items)
     if milestones?

app/finders/issues_finder.rb

@@ -144,18 +144,4 @@ class IssuesFinder < IssuableFinder
     current_user.blank?
   end
-
-  def by_assignee(items)
-    if filter_by_no_assignee?
-      items.unassigned
-    elsif filter_by_any_assignee?
-      items.assigned
-    elsif assignee
-      items.assigned_to(assignee)
-    elsif assignee_id? || assignee_username? # assignee not found
-      items.none
-    else
-      items
-    end
-  end
 end

app/helpers/boards_helper.rb

@@ -69,7 +69,7 @@ module BoardsHelper
   end
   def board_sidebar_user_data
-    dropdown_options = issue_assignees_dropdown_options
+    dropdown_options = assignees_dropdown_options('issue')
     {
       toggle: 'dropdown',

app/helpers/form_helper.rb

@@ -17,8 +17,8 @@ module FormHelper
     end
   end
-  def issue_assignees_dropdown_options
-    {
+  def assignees_dropdown_options(issuable_type)
+    dropdown_data = {
       toggle_class: 'js-user-search js-assignee-search js-multiselect js-save-user-data',
       title: 'Select assignee',
       filter: true,
@@ -28,8 +28,8 @@ module FormHelper
         first_user: current_user&.username,
         null_user: true,
         current_user: true,
-        project_id: @project&.id,
-        field_name: 'issue[assignee_ids][]',
+        project_id: (@target_project || @project)&.id,
+        field_name: "#{issuable_type}[assignee_ids][]",
         default_label: 'Unassigned',
         'max-select': 1,
         'dropdown-header': 'Assignee',
@@ -39,5 +39,36 @@ module FormHelper
         current_user_info: UserSerializer.new.represent(current_user)
       }
     }
+
+    type = issuable_type.to_s
+
+    if type == 'issue' && issue_supports_multiple_assignees? ||
+        type == 'merge_request' && merge_request_supports_multiple_assignees?
+      dropdown_data = multiple_assignees_dropdown_options(dropdown_data)
+    end
+
+    dropdown_data
+  end
+
+  # Overwritten
+  def issue_supports_multiple_assignees?
+    false
+  end
+
+  # Overwritten
+  def merge_request_supports_multiple_assignees?
+    false
+  end
+
+  private
+
+  def multiple_assignees_dropdown_options(options)
+    new_options = options.dup
+
+    new_options[:title] = 'Select assignee(s)'
+    new_options[:data][:'dropdown-header'] = 'Assignee(s)'
+    new_options[:data].delete(:'max-select')
+
+    new_options
   end
 end

app/helpers/issuables_helper.rb

@@ -15,11 +15,14 @@ module IssuablesHelper
     sidebar_gutter_collapsed? ? _('Expand sidebar') : _('Collapse sidebar')
   end
-  def sidebar_assignee_tooltip_label(issuable)
-    if issuable.assignee
-      issuable.assignee.name
+  def assignees_label(issuable, include_value: true)
+    label = 'Assignee'.pluralize(issuable.assignees.count)
+
+    if include_value
+      sanitized_list = sanitize_name(issuable.assignee_list)
+      "#{label}: #{sanitized_list}"
     else
-      issuable.allows_multiple_assignees? ? _('Assignee(s)') : _('Assignee')
+      label
     end
   end

app/mailers/emails/merge_requests.rb

@@ -24,10 +24,12 @@ module Emails
     end
     # rubocop: disable CodeReuse/ActiveRecord
-    def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id, reason = nil)
+    def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_ids, updated_by_user_id, reason = nil)
       setup_merge_request_mail(merge_request_id, recipient_id)
-      @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
+      @previous_assignees = []
+      @previous_assignees = User.where(id: previous_assignee_ids) if previous_assignee_ids.any?
+
       mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
     end
     # rubocop: enable CodeReuse/ActiveRecord

app/mailers/notify.rb

@@ -4,6 +4,7 @@ class Notify < BaseMailer
   include ActionDispatch::Routing::PolymorphicRoutes
   include GitlabRoutingHelper
   include EmailsHelper
+  include IssuablesHelper
   include Emails::Issues
   include Emails::MergeRequests
@@ -24,6 +25,7 @@ class Notify < BaseMailer
   helper MembersHelper
   helper AvatarsHelper
   helper GitlabRoutingHelper
+  helper IssuablesHelper
   def test_email(recipient_email, subject, body)
     mail(to: recipient_email,

app/models/concerns/deprecated_assignee.rb

+# frozen_string_literal: true
+
+# This module handles backward compatibility for import/export of Merge Requests after
+# multiple assignees feature was introduced. Also, it handles the scenarios where
+# the #26496 background migration hasn't finished yet.
+# Ideally, most of this code should be removed at #59457.
+module DeprecatedAssignee
+  extend ActiveSupport::Concern
+
+  def assignee_ids=(ids)
+    nullify_deprecated_assignee
+    super
+  end
+
+  def assignees=(users)
+    nullify_deprecated_assignee
+    super
+  end
+
+  def assignee_id=(id)
+    self.assignee_ids = Array(id)
+  end
+
+  def assignee=(user)
+    self.assignees = Array(user)
+  end
+
+  def assignee
+    assignees.first
+  end
+
+  def assignee_id
+    assignee_ids.first
+  end
+
+  def assignee_ids
+    if Gitlab::Database.read_only? && pending_assignees_population?
+      return Array(deprecated_assignee_id)
+    end
+
+    update_assignees_relation
+    super
+  end
+
+  def assignees
+    if Gitlab::Database.read_only? && pending_assignees_population?
+      return User.where(id: deprecated_assignee_id)
+    end
+
+    update_assignees_relation
+    super
+  end
+
+  private
+
+  # This will make the background migration process quicker (#26496) as it'll have less
+  # assignee_id rows to look through.
+  def nullify_deprecated_assignee
+    return unless persisted? && Gitlab::Database.read_only?
+
+    update_column(:assignee_id, nil)
+  end
+
+  # This code should be removed in the clean-up phase of the
+  # background migration (#59457).
+  def pending_assignees_population?
+    persisted? && deprecated_assignee_id && merge_request_assignees.empty?
+  end
+
+  # If there's an assignee_id and no relation, it means the background
+  # migration at #26496 didn't reach this merge request yet.
+  # This code should be removed in the clean-up phase of the
+  # background migration (#59457).
+  def update_assignees_relation
+    if pending_assignees_population?
+      transaction do
+        merge_request_assignees.create!(user_id: deprecated_assignee_id, merge_request_id: id)
+        update_column(:assignee_id, nil)
+      end
+    end
+  end
+
+  def deprecated_assignee_id
+    read_attribute(:assignee_id)
+  end
+end

app/models/concerns/issuable.rb

@@ -67,13 +67,6 @@ module Issuable
              allow_nil: true,
              prefix: true
-    delegate :name,
-             :email,
-             :public_email,
-             to: :assignee,
-             allow_nil: true,
-             prefix: true
-
     validates :author, presence: true
     validates :title, presence: true, length: { maximum: 255 }
     validate :milestone_is_valid
@@ -88,6 +81,19 @@ module Issuable
     scope :only_opened, -> { with_state(:opened) }
     scope :closed, -> { with_state(:closed) }
+    # rubocop:disable GitlabSecurity/SqlInjection
+    # The `to_ability_name` method is not an user input.
+    scope :assigned, -> do
+      where("EXISTS (SELECT TRUE FROM #{to_ability_name}_assignees WHERE #{to_ability_name}_id = #{to_ability_name}s.id)")
+    end
+    scope :unassigned, -> do
+      where("NOT EXISTS (SELECT TRUE FROM #{to_ability_name}_assignees WHERE #{to_ability_name}_id = #{to_ability_name}s.id)")
+    end
+    scope :assigned_to, ->(u) do
+      where("EXISTS (SELECT TRUE FROM #{to_ability_name}_assignees WHERE user_id = ? AND #{to_ability_name}_id = #{to_ability_name}s.id)", u.id)
+    end
+    # rubocop:enable GitlabSecurity/SqlInjection
+
     scope :left_joins_milestones,    -> { joins("LEFT OUTER JOIN milestones ON #{table_name}.milestone_id = milestones.id") }
     scope :order_milestone_due_desc, -> { left_joins_milestones.reorder('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date DESC') }
     scope :order_milestone_due_asc,  -> { left_joins_milestones.reorder('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date ASC') }
@@ -104,6 +110,7 @@ module Issuable
     participant :author
     participant :notes_with_associations
+    participant :assignees
     strip_attributes :title
@@ -270,6 +277,10 @@ module Issuable
     end
   end
+  def assignee_or_author?(user)
+    author_id == user.id || assignees.exists?(user.id)
+  end
+
   def today?
     Date.today == created_at.to_date
   end
@@ -314,11 +325,7 @@ module Issuable
       end
       if old_assignees != assignees
-        if self.is_a?(Issue)
-          changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)]
-        else
-          changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs]
-        end
+        changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)]
       end
       if self.respond_to?(:total_time_spent)
@@ -355,10 +362,18 @@ module Issuable
   def card_attributes
     {
       'Author'   => author.try(:name),
-      'Assignee' => assignee.try(:name)
+      'Assignee' => assignee_list
     }
   end
+  def assignee_list
+    assignees.map(&:name).to_sentence
+  end
+
+  def assignee_username_list
+    assignees.map(&:username).to_sentence
+  end
+
   def notes_with_associations
     # If A has_many Bs, and B has_many Cs, and you do
     # `A.includes(b: :c).each { |a| a.b.includes(:c) }`, sadly ActiveRecord

app/models/issue.rb

@@ -49,10 +49,6 @@ class Issue < ApplicationRecord
   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 :with_due_date, -> { where.not(due_date: nil) }
   scope :without_due_date, -> { where(due_date: nil) }
   scope :due_before, ->(date) { where('issues.due_date < ?', date) }
@@ -75,8 +71,6 @@ class Issue < ApplicationRecord
   attr_spammable :title, spam_title: true
   attr_spammable :description, spam_description: true
-  participant :assignees
-
   state_machine :state, initial: :opened do
     event :close do
       transition [:opened] => :closed
@@ -155,22 +149,6 @@ class Issue < ApplicationRecord
     Gitlab::HookData::IssueBuilder.new(self).build
   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}"

app/models/merge_request.rb

@@ -16,6 +16,7 @@ class MergeRequest < ApplicationRecord
   include LabelEventable
   include ReactiveCaching
   include FromUnion
+  include DeprecatedAssignee
   self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
   self.reactive_cache_refresh_interval = 10.minutes
@@ -69,8 +70,7 @@ class MergeRequest < ApplicationRecord
   has_many :suggestions, through: :notes
   has_many :merge_request_assignees
-  # Will be deprecated at https://gitlab.com/gitlab-org/gitlab-ce/issues/59457
-  belongs_to :assignee, class_name: "User"
+  has_many :assignees, class_name: "User", through: :merge_request_assignees
   serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
@@ -79,10 +79,6 @@ class MergeRequest < ApplicationRecord
   after_update :reload_diff_if_branch_changed
   after_save :ensure_metrics
-  # Required until the codebase starts using this relation for single or multiple assignees.
-  # TODO: Remove at gitlab-ee#2004 implementation.
-  after_save :refresh_merge_request_assignees, if: :assignee_id_changed?
-
   # When this attribute is true some MR validation is ignored
   # It allows us to close or modify broken merge requests
   attr_accessor :allow_broken
@@ -188,19 +184,14 @@ class MergeRequest < ApplicationRecord
   end
   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)}
   scope :with_api_entity_associations, -> {
-    preload(:author, :assignee, :notes, :labels, :milestone, :timelogs,
+    preload(:assignees, :author, :notes, :labels, :milestone, :timelogs,
             latest_merge_request_diff: [:merge_request_diff_commits],
             metrics: [:latest_closed_by, :merged_by],
             target_project: [:route, { namespace: :route }],
             source_project: [:route, { namespace: :route }])
   }
-  participant :assignee
-
   after_save :keep_around_commit
   alias_attribute :project, :target_project
@@ -337,31 +328,6 @@ class MergeRequest < ApplicationRecord
     Gitlab::HookData::MergeRequestBuilder.new(self).build
   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
-
-  # These method are needed for compatibility with issues to not mess view and other code
-  def assignees
-    Array(assignee)
-  end
-
-  def assignee_ids
-    Array(assignee_id)
-  end
-
-  def assignee_ids=(ids)
-    write_attribute(:assignee_id, ids.last)
-  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}"
@@ -682,15 +648,6 @@ class MergeRequest < ApplicationRecord
     merge_request_diff || create_merge_request_diff
   end
-  def refresh_merge_request_assignees
-    transaction do
-      # Using it instead relation.delete_all in order to avoid adding a
-      # dependent: :delete_all (we already have foreign key cascade deletion).
-      MergeRequestAssignee.where(merge_request_id: self).delete_all
-      merge_request_assignees.create(user_id: assignee_id) if assignee_id
-    end
-  end
-
   def create_merge_request_diff
     fetch_ref!
@@ -1208,7 +1165,7 @@ class MergeRequest < ApplicationRecord
       variables.append(key: 'CI_MERGE_REQUEST_PROJECT_URL', value: project.web_url)
       variables.append(key: 'CI_MERGE_REQUEST_TARGET_BRANCH_NAME', value: target_branch.to_s)
       variables.append(key: 'CI_MERGE_REQUEST_TITLE', value: title)
-      variables.append(key: 'CI_MERGE_REQUEST_ASSIGNEES', value: assignee.username) if assignee
+      variables.append(key: 'CI_MERGE_REQUEST_ASSIGNEES', value: assignee_username_list) if assignees.any?
       variables.append(key: 'CI_MERGE_REQUEST_MILESTONE', value: milestone.title) if milestone
       variables.append(key: 'CI_MERGE_REQUEST_LABELS', value: label_names.join(',')) if labels.present?
       variables.concat(source_project_variables)

app/models/project.rb

@@ -674,6 +674,10 @@ class Project < ApplicationRecord
     { scope: :project, status: auto_devops&.enabled || Feature.enabled?(:force_autodevops_on_by_default, self) }
   end
+  def multiple_mr_assignees_enabled?
+    Feature.enabled?(:multiple_merge_request_assignees, self)
+  end
+
   def daily_statistics_enabled?
     Feature.enabled?(:project_daily_statistics, self, default_enabled: true)
   end

app/serializers/issuable_sidebar_extras_entity.rb

@@ -11,4 +11,6 @@ class IssuableSidebarExtrasEntity < Grape::Entity
   expose :subscribed do |issuable|
     issuable.subscribed?(request.current_user, issuable.project)
   end
+
+  expose :assignees, using: API::Entities::UserBasic
 end