[Multiple issue assignees] Fix a buch of issues

app/controllers/projects/issues_controller.rb

@@ -157,7 +157,7 @@ class Projects::IssuesController < Projects::ApplicationController
         if @issue.valid?
           render json: @issue.to_json(methods: [:task_status, :task_status_short],
                                       include: { milestone: {},
-                                                 assignee: { only: [:name, :username], methods: [:avatar_url] },
+                                                 assignees: { only: [:name, :username], methods: [:avatar_url] },
                                                  labels: { methods: :text_color } })
         else
           render json: { errors: @issue.errors.full_messages }, status: :unprocessable_entity

app/finders/issuable_finder.rb

@@ -231,17 +231,6 @@ class IssuableFinder
     klass.all
   end
 
-  def by_scope(items)
-    case params[:scope]
-    when 'created-by-me', 'authored'
-      items.where(author_id: current_user.id)
-    when 'assigned-to-me'
-      items.where(assignee_id: current_user.id)
-    else
-      items
-    end
-  end
-
   def by_state(items)
     case params[:state].to_s
     when 'closed'

app/finders/issues_finder.rb

@@ -38,6 +38,17 @@ class IssuesFinder < IssuableFinder
     items
   end
 
+  def by_scope(items)
+    case params[:scope]
+    when 'created-by-me', 'authored'
+      items.where(author_id: current_user.id)
+    when 'assigned-to-me'
+      items.where("issue_assignees.user_id = ?", current_user.id)
+    else
+      items
+    end
+  end
+
   def self.not_restricted_by_confidentiality(user)
     issues = Issue.with_assignees
 

app/finders/merge_requests_finder.rb

@@ -23,6 +23,17 @@ class MergeRequestsFinder < IssuableFinder
 
   private
 
+  def by_scope(items)
+    case params[:scope]
+    when 'created-by-me', 'authored'
+      items.where(author_id: current_user.id)
+    when 'assigned-to-me'
+      items.where(assignee_id: current_user.id)
+    else
+      items
+    end
+  end
+
   def item_project_ids(items)
     items&.reorder(nil)&.select(:target_project_id)
   end

app/models/concerns/issuable.rb

@@ -63,11 +63,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 }) }
@@ -99,16 +96,8 @@ module Issuable
 
     acts_as_paranoid
 
-    after_save :update_assignee_cache_counts, if: :assignee_id_changed?
     after_save :record_metrics
 
-    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?
@@ -245,10 +234,6 @@ module Issuable
     today? && created_at == updated_at
   end
 
-  def is_being_reassigned?
-    assignee_id_changed?
-  end
-
   def open?
     opened? || reopened?
   end

app/models/issue.rb

@@ -29,14 +29,18 @@ class Issue < ActiveRecord::Base
 
   has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
 
-  has_and_belongs_to_many :assignees, class_name: "User", join_table: :issue_assignees
+  has_many :issue_assignees
+  has_many :assignees, class_name: "User", through: :issue_assignees
 
   validates :project, presence: true
 
   scope :cared, ->(user) { with_assignees.where("issue_assignees.user_id IN(?)", user.id) }
   scope :open_for, ->(user) { opened.assigned_to(user) }
   scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
-  scope :with_assignees, -> { joins('LEFT JOIN issue_assignees ON issue_id = issues.id') }
+  scope :with_assignees, -> { joins("LEFT JOIN issue_assignees ON issue_id = issues.id") }
+  scope :assigned, -> { with_assignees.where('issue_assignees.user_id IS NOT NULL') }
+  scope :unassigned, -> { with_assignees.where('issue_assignees.user_id IS NULL') }
+  scope :assigned_to, ->(u) { with_assignees.where('issue_assignees.user_id = ?', u.id)}
 
   scope :without_due_date, -> { where(due_date: nil) }
   scope :due_before, ->(date) { where('issues.due_date < ?', date) }
@@ -49,7 +53,7 @@ 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) }
 
   attr_spammable :title, spam_title: true
   attr_spammable :description, spam_description: true
@@ -132,6 +136,14 @@ class Issue < ActiveRecord::Base
               "id DESC")
   end
 
+  def update_assignee_cache_counts
+    return true # TODO implement it properly
+    # 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
     {
@@ -148,12 +160,6 @@ class Issue < ActiveRecord::Base
     assignees.map(&:name).to_sentence
   end
 
-  # TODO: This method will help us to find some silent failures.
-  # We should remove it before merging to master
-  def assignee_id
-    raise "assignee_id is deprecated"
-  end
-
   # `from` argument can be a Namespace or Project.
   def to_reference(from = nil, full: false)
     reference = "#{self.class.reference_prefix}#{iid}"

app/models/issue_assignee.rb

+class IssueAssignee < ActiveRecord::Base
+	belongs_to :issue
+	belongs_to :assignee, class_name: "User", foreign_key: :user_id
+end
\ No newline at end of file

app/models/merge_request.rb

@@ -121,11 +121,15 @@ class MergeRequest < ActiveRecord::Base
   scope :from_source_branches, ->(branches) { where(source_branch: branches) }
   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 :approvers_left
   participant :assignee
 
   after_save :keep_around_commit
+  after_save :update_assignee_cache_counts, if: :assignee_id_changed?
 
   def self.reference_prefix
     '!'
@@ -185,6 +189,17 @@ class MergeRequest < ActiveRecord::Base
     work_in_progress?(title) ? title : "WIP: #{title}"
   end
 
+  def is_being_reassigned?
+    assignee_id_changed?
+  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
     {
@@ -193,9 +208,9 @@ class MergeRequest < ActiveRecord::Base
     }
   end
 
-  # This method is needed for compatibility with issues
+  # This method is needed for compatibility with issues to not mess view and other code
   def assignees
-    [assignee]
+    assignee ? [assignee] : []
   end
 
   def assignee_or_author?(user)

app/models/milestone.rb

@@ -23,7 +23,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) }
@@ -109,6 +108,10 @@ class Milestone < ActiveRecord::Base
     end
   end
 
+  def participants
+    User.joins(assigned_issues: :milestone).where("milestones.id = ?", id)
+  end
+
   ##
   # Returns the String necessary to reference this Milestone in Markdown
   #

app/models/user.rb

@@ -88,9 +88,7 @@ class User < ActiveRecord::Base
   has_many :merge_requests,           dependent: :destroy, foreign_key: :author_id
   has_many :events,                   dependent: :destroy, foreign_key: :author_id
   has_many :subscriptions,            dependent: :destroy
-  has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id,   class_name: "Event"
-  has_many :assigned_issues,          dependent: :destroy, foreign_key: :assignee_id, class_name: "Issue"
-  has_many :assigned_merge_requests,  dependent: :destroy, foreign_key: :assignee_id, class_name: "MergeRequest"
+  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_many :approvals,                dependent: :destroy
   has_many :approvers,                dependent: :destroy
@@ -108,7 +106,8 @@ class User < ActiveRecord::Base
   has_many :protected_branch_push_access_levels, dependent: :destroy, class_name: ProtectedBranch::PushAccessLevel
   has_many :triggers,                 dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id
 
-  has_many :assigned_issues,          dependent: :nullify, foreign_key: :assignee_id, class_name: "Issue"
+  has_many :issue_assignees, dependent: :destroy
+  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

app/views/shared/milestones/_issuable.html.haml

 -# @project is present when viewing Project's milestone
 - project = @project || issuable.project
-- assignee = issuable.assignee
+- assignees = issuable.assignees
 - issuable_type = issuable.class.table_name
 - base_url_args = [project.namespace.becomes(Namespace), project, issuable_type]
 - can_update = can?(current_user, :"update_#{issuable.to_ability_name}", issuable)
@@ -23,7 +23,7 @@
         - render_colored_label(label)
 
     %span.assignee-icon
-      - if assignee
-        = link_to polymorphic_path(base_url_args, { milestone_title: @milestone.title, assignee_id: issuable.assignee_id, state: 'all' }),
+      - assignees.each do |assignee|
+        = link_to polymorphic_path(base_url_args, { milestone_title: @milestone.title, assignee_id: assignee.id, state: 'all' }),
                   class: 'has-tooltip', title: "Assigned to #{assignee.name}", data: { container: 'body' } do
-          - image_tag(avatar_icon(issuable.assignee, 16), class: "avatar s16", alt: '')
+          - image_tag(avatar_icon(assignee, 16), class: "avatar s16", alt: '')

spec/controllers/projects/issues_controller_spec.rb

@@ -161,7 +161,7 @@ describe Projects::IssuesController do
           namespace_id: project.namespace.to_param,
           project_id: project,
           id: issue.iid,
-          issue: { assignee_id: assignee.id },
+          issue: { assignee_ids: assignee.id.to_s },
           format: :json
         body = JSON.parse(response.body)
 

spec/services/issues/update_service_spec.rb

@@ -13,7 +13,7 @@ describe Issues::UpdateService, services: true do
 
   let(:issue) do
     create(:issue, title: 'Old title',
-                   assignee_id: user3.id,
+                   assignees: [user3],
                    project: project)
   end
 
@@ -39,7 +39,7 @@ describe Issues::UpdateService, services: true do
         {
           title: 'New title',
           description: 'Also please fix',
-          assignee_id: user2.id,
+          assignee_ids: "#{user2.id}, #{user3.id}",
           state_event: 'close',
           label_ids: [label.id],
           due_date: Date.tomorrow
@@ -52,15 +52,15 @@ describe Issues::UpdateService, services: true do
         expect(issue).to be_valid
         expect(issue.title).to eq 'New title'
         expect(issue.description).to eq 'Also please fix'
-        expect(issue.assignee).to eq user2
+        expect(issue.assignees).to eq [user2, user3]
         expect(issue).to be_closed
         expect(issue.labels).to match_array [label]
         expect(issue.due_date).to eq Date.tomorrow
       end
 
       it 'sorts issues as specified by parameters' do
-        issue1 = create(:issue, project: project, assignee_id: user3.id)
-        issue2 = create(:issue, project: project, assignee_id: user3.id)
+        issue1 = create(:issue, project: project, assignees: [user3])
+        issue2 = create(:issue, project: project, assignees: [user3])
 
         [issue, issue1, issue2].each do |issue|
           issue.move_to_end
@@ -86,7 +86,7 @@ describe Issues::UpdateService, services: true do
           expect(issue).to be_valid
           expect(issue.title).to eq 'New title'
           expect(issue.description).to eq 'Also please fix'
-          expect(issue.assignee).to eq user3
+          expect(issue.assignees).to eq [user3]
           expect(issue.labels).to be_empty
           expect(issue.milestone).to be_nil
           expect(issue.due_date).to be_nil
@@ -136,7 +136,7 @@ describe Issues::UpdateService, services: true do
         {
           title: 'New title',
           description: 'Also please fix',
-          assignee_id: user2.id,
+          assignee_ids: [user2],
           state_event: 'close',
           label_ids: [label.id],
           confidential: true
@@ -163,11 +163,11 @@ describe Issues::UpdateService, services: true do
         project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
         update_issue(confidential: true)
         non_member        = create(:user)
-        original_assignee = issue.assignee
+        original_assignees = issue.assignees
 
-        update_issue(assignee_id: non_member.id)
+        update_issue(assignee_ids: non_member.id.to_s)
 
-        expect(issue.reload.assignee_id).to eq(original_assignee.id)
+        expect(issue.reload.assignees).to eq(original_assignees)
       end
     end