Backport of multiple_assignees_feature [ci skip]

6 files changed, 184 insertions, 119 deletions

diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 3ecba2e9687..27890e33b49 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -10,7 +10,6 @@ describe Issuable do
 
     it { is_expected.to belong_to(:project) }
     it { is_expected.to belong_to(:author) }
-    it { is_expected.to belong_to(:assignee) }
     it { is_expected.to have_many(:notes).dependent(:destroy) }
     it { is_expected.to have_many(:todos).dependent(:destroy) }
 
@@ -66,60 +65,6 @@ describe Issuable do
     end
   end
 
-  describe 'assignee_name' do
-    it 'is delegated to assignee' do
-      issue.update!(assignee: create(:user))
-
-      expect(issue.assignee_name).to eq issue.assignee.name
-    end
-
-    it 'returns nil when assignee is nil' do
-      issue.assignee_id = nil
-      issue.save(validate: false)
-
-      expect(issue.assignee_name).to eq nil
-    end
-  end
-
-  describe "before_save" do
-    describe "#update_cache_counts" do
-      context "when previous assignee exists" do
-        before do
-          assignee = create(:user)
-          issue.project.team << [assignee, :developer]
-          issue.update(assignee: assignee)
-        end
-
-        it "updates cache counts for new assignee" do
-          user = create(:user)
-
-          expect(user).to receive(:update_cache_counts)
-
-          issue.update(assignee: user)
-        end
-
-        it "updates cache counts for previous assignee" do
-          old_assignee = issue.assignee
-          allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee)
-
-          expect(old_assignee).to receive(:update_cache_counts)
-
-          issue.update(assignee: nil)
-        end
-      end
-
-      context "when previous assignee does not exist" do
-        before{ issue.update(assignee: nil) }
-
-        it "updates cache count for the new assignee" do
-          expect_any_instance_of(User).to receive(:update_cache_counts)
-
-          issue.update(assignee: user)
-        end
-      end
-    end
-  end
-
   describe ".search" do
     let!(:searchable_issue) { create(:issue, title: "Searchable issue") }
 
@@ -307,7 +252,20 @@ describe Issuable do
     end
 
     context "issue is assigned" do
-      before { issue.update_attribute(:assignee, user) }
+      before { issue.assignees << user }
+
+      it "returns correct hook data" do
+        expect(data[:assignees].first).to eq(user.hook_attrs)
+      end
+    end
+
+    context "merge_request is assigned" do
+      let(:merge_request) { create(:merge_request) }
+      let(:data) { merge_request.to_hook_data(user) }
+
+      before do
+        merge_request.update_attribute(:assignee, user)
+      end
 
       it "returns correct hook data" do
         expect(data[:object_attributes]['assignee_id']).to eq(user.id)
@@ -329,24 +287,6 @@ describe Issuable do
     include_examples 'deprecated repository hook data'
   end
 
-  describe '#card_attributes' do
-    it 'includes the author name' do
-      allow(issue).to receive(:author).and_return(double(name: 'Robert'))
-      allow(issue).to receive(:assignee).and_return(nil)
-
-      expect(issue.card_attributes).
-        to eq({ 'Author' => 'Robert', 'Assignee' => nil })
-    end
-
-    it 'includes the assignee name' do
-      allow(issue).to receive(:author).and_return(double(name: 'Robert'))
-      allow(issue).to receive(:assignee).and_return(double(name: 'Douwe'))
-
-      expect(issue.card_attributes).
-        to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' })
-    end
-  end
-
   describe '#labels_array' do
     let(:project) { create(:empty_project) }
     let(:bug) { create(:label, project: project, title: 'bug') }
@@ -475,27 +415,6 @@ describe Issuable do
     end
   end
 
-  describe '#assignee_or_author?' do
-    let(:user) { build(:user, id: 1) }
-    let(:issue) { build(:issue) }
-
-    it 'returns true for a user that is assigned to an issue' do
-      issue.assignee = user
-
-      expect(issue.assignee_or_author?(user)).to eq(true)
-    end
-
-    it 'returns true for a user that is the author of an issue' do
-      issue.author = user
-
-      expect(issue.assignee_or_author?(user)).to eq(true)
-    end
-
-    it 'returns false for a user that is not the assignee or author' do
-      expect(issue.assignee_or_author?(user)).to eq(false)
-    end
-  end
-
   describe '#spend_time' do
     let(:user) { create(:user) }
     let(:issue) { create(:issue) }
diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb
index 68e4c0a522b..675b730c557 100644
--- a/spec/models/concerns/milestoneish_spec.rb
+++ b/spec/models/concerns/milestoneish_spec.rb
@@ -11,13 +11,13 @@ describe Milestone, 'Milestoneish' do
   let(:milestone) { create(:milestone, project: project) }
   let!(:issue) { create(:issue, project: project, milestone: milestone) }
   let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone) }
-  let!(:security_issue_2) { create(:issue, :confidential, project: project, assignee: assignee, milestone: milestone) }
+  let!(:security_issue_2) { create(:issue, :confidential, project: project, assignees: [assignee], milestone: milestone) }
   let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) }
   let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) }
   let!(:closed_security_issue_1) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) }
-  let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) }
+  let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) }
   let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) }
-  let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) }
+  let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) }
   let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) }
 
   before do
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index 8c90a538f57..e5954d80f00 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -92,8 +92,8 @@ describe Event, models: true do
     let(:author) { create(:author) }
     let(:assignee) { create(:user) }
     let(:admin) { create(:admin) }
-    let(:issue) { create(:issue, project: project, author: author, assignee: assignee) }
-    let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
+    let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) }
+    let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) }
     let(:note_on_commit) { create(:note_on_commit, project: project) }
     let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) }
     let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) }
diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb
index d8aed25c041..93c2c538e10 100644
--- a/spec/models/issue_collection_spec.rb
+++ b/spec/models/issue_collection_spec.rb
@@ -28,7 +28,7 @@ describe IssueCollection do
       end
 
       it 'returns the issues the user is assigned to' do
-        issue1.assignee = user
+        issue1.assignees << user
 
         expect(collection.updatable_by_user(user)).to eq([issue1])
       end
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 8748b98a4e3..725f5c2311f 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
 describe Issue, models: true do
   describe "Associations" do
     it { is_expected.to belong_to(:milestone) }
+    it { is_expected.to have_many(:assignees) }
   end
 
   describe 'modules' do
@@ -37,6 +38,64 @@ describe Issue, models: true do
     end
   end
 
+  describe "before_save" do
+    describe "#update_cache_counts when an issue is reassigned" do
+      let(:issue) { create(:issue) }
+      let(:assignee) { create(:user) }
+
+      context "when previous assignee exists" do
+        before do
+          issue.project.team << [assignee, :developer]
+          issue.assignees << assignee
+        end
+
+        it "updates cache counts for new assignee" do
+          user = create(:user)
+
+          expect(user).to receive(:update_cache_counts)
+
+          issue.assignees << user
+        end
+
+        it "updates cache counts for previous assignee" do
+          issue.assignees.first
+
+          expect_any_instance_of(User).to receive(:update_cache_counts)
+
+          issue.assignees.destroy_all
+        end
+      end
+
+      context "when previous assignee does not exist" do
+        it "updates cache count for the new assignee" do
+          issue.assignees = []
+
+          expect_any_instance_of(User).to receive(:update_cache_counts)
+
+          issue.assignees << assignee
+        end
+      end
+    end
+  end
+
+  describe '#card_attributes' do
+    it 'includes the author name' do
+      allow(subject).to receive(:author).and_return(double(name: 'Robert'))
+      allow(subject).to receive(:assignees).and_return([])
+
+      expect(subject.card_attributes).
+        to eq({ 'Author' => 'Robert', 'Assignee' => '' })
+    end
+
+    it 'includes the assignee name' do
+      allow(subject).to receive(:author).and_return(double(name: 'Robert'))
+      allow(subject).to receive(:assignees).and_return([double(name: 'Douwe')])
+
+      expect(subject.card_attributes).
+        to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' })
+    end
+  end
+
   describe '#closed_at' do
     after do
       Timecop.return
@@ -124,13 +183,24 @@ describe Issue, models: true do
     end
   end
 
-  describe '#is_being_reassigned?' do
-    it 'returns true if the issue assignee has changed' do
-      subject.assignee = create(:user)
-      expect(subject.is_being_reassigned?).to be_truthy
+  describe '#assignee_or_author?' do
+    let(:user) { create(:user) }
+    let(:issue) { create(:issue) }
+
+    it 'returns true for a user that is assigned to an issue' do
+      issue.assignees << user
+
+      expect(issue.assignee_or_author?(user)).to be_truthy
     end
-    it 'returns false if the issue assignee has not changed' do
-      expect(subject.is_being_reassigned?).to be_falsey
+
+    it 'returns true for a user that is the author of an issue' do
+      issue.update(author: user)
+
+      expect(issue.assignee_or_author?(user)).to be_truthy
+    end
+
+    it 'returns false for a user that is not the assignee or author' do
+      expect(issue.assignee_or_author?(user)).to be_falsey
     end
   end
 
@@ -383,14 +453,14 @@ describe Issue, models: true do
       user1 = create(:user)
       user2 = create(:user)
       project = create(:empty_project)
-      issue = create(:issue, assignee: user1, project: project)
+      issue = create(:issue, assignees: [user1], project: project)
       project.add_developer(user1)
       project.add_developer(user2)
 
       expect(user1.assigned_open_issues_count).to eq(1)
       expect(user2.assigned_open_issues_count).to eq(0)
 
-      issue.assignee = user2
+      issue.assignees = [user2]
       issue.save
 
       expect(user1.assigned_open_issues_count).to eq(0)
@@ -676,6 +746,11 @@ describe Issue, models: true do
       expect(attrs_hash).to include(:human_total_time_spent)
       expect(attrs_hash).to include('time_estimate')
     end
+
+    it 'includes assignee_ids and deprecated assignee_id' do
+      expect(attrs_hash).to include(:assignee_id)
+      expect(attrs_hash).to include(:assignee_ids)
+    end
   end
 
   describe '#check_for_spam' do
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 8b72125dd5d..6cf3dd30ead 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -9,6 +9,7 @@ describe MergeRequest, models: true do
     it { is_expected.to belong_to(:target_project).class_name('Project') }
     it { is_expected.to belong_to(:source_project).class_name('Project') }
     it { is_expected.to belong_to(:merge_user).class_name("User") }
+    it { is_expected.to belong_to(:assignee) }
     it { is_expected.to have_many(:merge_request_diffs).dependent(:destroy) }
   end
 
@@ -86,6 +87,86 @@ describe MergeRequest, models: true do
     end
   end
 
+  describe "before_save" do
+    describe "#update_cache_counts when a merge request is reassigned" do
+      let(:project) { create :project }
+      let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+      let(:assignee) { create :user }
+
+      context "when previous assignee exists" do
+        before do
+          project.team << [assignee, :developer]
+          merge_request.update(assignee: assignee)
+        end
+
+        it "updates cache counts for new assignee" do
+          user = create(:user)
+
+          expect(user).to receive(:update_cache_counts)
+
+          merge_request.update(assignee: user)
+        end
+
+        it "updates cache counts for previous assignee" do
+          old_assignee = merge_request.assignee
+          allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee)
+
+          expect(old_assignee).to receive(:update_cache_counts)
+
+          merge_request.update(assignee: nil)
+        end
+      end
+
+      context "when previous assignee does not exist" do
+        it "updates cache count for the new assignee" do
+          merge_request.update(assignee: nil)
+
+          expect_any_instance_of(User).to receive(:update_cache_counts)
+
+          merge_request.update(assignee: assignee)
+        end
+      end
+    end
+  end
+
+  describe '#card_attributes' do
+    it 'includes the author name' do
+      allow(subject).to receive(:author).and_return(double(name: 'Robert'))
+      allow(subject).to receive(:assignee).and_return(nil)
+
+      expect(subject.card_attributes).
+        to eq({ 'Author' => 'Robert', 'Assignee' => nil })
+    end
+
+    it 'includes the assignee name' do
+      allow(subject).to receive(:author).and_return(double(name: 'Robert'))
+      allow(subject).to receive(:assignee).and_return(double(name: 'Douwe'))
+
+      expect(subject.card_attributes).
+        to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' })
+    end
+  end
+
+  describe '#assignee_or_author?' do
+    let(:user) { create(:user) }
+
+    it 'returns true for a user that is assigned to a merge request' do
+      subject.assignee = user
+
+      expect(subject.assignee_or_author?(user)).to eq(true)
+    end
+
+    it 'returns true for a user that is the author of a merge request' do
+      subject.author = user
+
+      expect(subject.assignee_or_author?(user)).to eq(true)
+    end
+
+    it 'returns false for a user that is not the assignee or author' do
+      expect(subject.assignee_or_author?(user)).to eq(false)
+    end
+  end
+
   describe '#cache_merge_request_closes_issues!' do
     before do
       subject.project.team << [subject.author, :developer]
@@ -295,16 +376,6 @@ describe MergeRequest, models: true do
     end
   end
 
-  describe '#is_being_reassigned?' do
-    it 'returns true if the merge_request assignee has changed' do
-      subject.assignee = create(:user)
-      expect(subject.is_being_reassigned?).to be_truthy
-    end
-    it 'returns false if the merge request assignee has not changed' do
-      expect(subject.is_being_reassigned?).to be_falsey
-    end
-  end
-
   describe '#for_fork?' do
     it 'returns true if the merge request is for a fork' do
       subject.source_project = build_stubbed(:empty_project, namespace: create(:group))