Merge branch 'remove_assignee_id' into 'master'

Remove issues.assignee_id column

Closes #30453

See merge request gitlab-org/gitlab-ce!11637

6 files changed, 90 insertions, 4 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 33db197e612..bbda848c39d 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -10,6 +10,9 @@ class Issue < ActiveRecord::Base
   include RelativePositioning
   include TimeTrackable
   include ThrottledTouch
+  include IgnorableColumn
+
+  ignore_column :assignee_id
 
   DueDateStruct = Struct.new(:title, :name).freeze
   NoDueDate     = DueDateStruct.new('No Due Date', '0').freeze
diff --git a/db/post_migrate/20170523073948_remove_assignee_id_from_issue.rb b/db/post_migrate/20170523073948_remove_assignee_id_from_issue.rb
new file mode 100644
index 00000000000..006d17b4d62
--- /dev/null
+++ b/db/post_migrate/20170523073948_remove_assignee_id_from_issue.rb
@@ -0,0 +1,48 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveAssigneeIdFromIssue < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+  # "add_column_with_default" you must disable the use of transactions
+  # as these methods can not run in an existing transaction.
+  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+  # that either of them is the _only_ method called in the migration,
+  # any other changes should go in a separate migration.
+  # This ensures that upon failure _only_ the index creation or removing fails
+  # and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  disable_ddl_transaction!
+
+  class Issue < ActiveRecord::Base
+    self.table_name = 'issues'
+
+    include ::EachBatch
+  end
+
+  def up
+    remove_column :issues, :assignee_id
+  end
+
+  def down
+    add_column :issues, :assignee_id, :integer
+    add_concurrent_index :issues, :assignee_id
+
+    update_value = Arel.sql('(SELECT user_id FROM issue_assignees WHERE issue_assignees.issue_id = issues.id LIMIT 1)')
+
+    # This is only used in the down step, so we can ignore the RuboCop warning
+    # about large tables, as this is very unlikely to be run on GitLab.com
+    update_column_in_batches(:issues, :assignee_id, update_value) # rubocop:disable Migration/UpdateLargeTable
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index c0a141885ad..f0b1da16d53 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -841,7 +841,6 @@ ActiveRecord::Schema.define(version: 20171206221519) do
 
   create_table "issues", force: :cascade do |t|
     t.string "title"
-    t.integer "assignee_id"
     t.integer "author_id"
     t.integer "project_id"
     t.datetime "created_at"
@@ -867,7 +866,6 @@ ActiveRecord::Schema.define(version: 20171206221519) do
     t.datetime_with_timezone "closed_at"
   end
 
-  add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
   add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree
   add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree
   add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree
diff --git a/features/steps/groups.rb b/features/steps/groups.rb
index a2d9a0332e0..753694a5392 100644
--- a/features/steps/groups.rb
+++ b/features/steps/groups.rb
@@ -138,7 +138,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
   private
 
   def assigned_to_me(key)
-    project.send(key).where(assignee_id: current_user.id)
+    project.send(key).assigned_to(current_user)
   end
 
   def project
diff --git a/spec/migrations/remove_assignee_id_from_issue_spec.rb b/spec/migrations/remove_assignee_id_from_issue_spec.rb
new file mode 100644
index 00000000000..2c6f992d3ae
--- /dev/null
+++ b/spec/migrations/remove_assignee_id_from_issue_spec.rb
@@ -0,0 +1,37 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170523073948_remove_assignee_id_from_issue.rb')
+
+describe RemoveAssigneeIdFromIssue, :migration do
+  let(:issues) { table(:issues) }
+  let(:issue_assignees) { table(:issue_assignees) }
+  let(:users) { table(:users) }
+
+  let!(:user_1) { users.create(email: 'email1@example.com') }
+  let!(:user_2) { users.create(email: 'email2@example.com') }
+  let!(:user_3) { users.create(email: 'email3@example.com') }
+
+  def create_issue(assignees:)
+    issues.create.tap do |issue|
+      assignees.each do |assignee|
+        issue_assignees.create(issue_id: issue.id, user_id: assignee.id)
+      end
+    end
+  end
+
+  let!(:issue_single_assignee) { create_issue(assignees: [user_1]) }
+  let!(:issue_no_assignee) { create_issue(assignees: []) }
+  let!(:issue_multiple_assignees) { create_issue(assignees: [user_2, user_3]) }
+
+  describe '#down' do
+    it 'sets the assignee_id to a random matching assignee from the assignees table' do
+      migrate!
+      disable_migrations_output { described_class.new.down }
+
+      expect(issue_single_assignee.reload.assignee_id).to eq(user_1.id)
+      expect(issue_no_assignee.reload.assignee_id).to be_nil
+      expect(issue_multiple_assignees.reload.assignee_id).to eq(user_2.id).or(user_3.id)
+
+      disable_migrations_output { described_class.new.up }
+    end
+  end
+end
diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb
index 2d04d824180..d4ef31c0c74 100644
--- a/spec/services/members/authorized_destroy_service_spec.rb
+++ b/spec/services/members/authorized_destroy_service_spec.rb
@@ -45,7 +45,7 @@ describe Members::AuthorizedDestroyService do
       expect { described_class.new(member, member_user).execute }
         .to change { number_of_assigned_issuables(member_user) }.from(4).to(2)
 
-      expect(issue.reload.assignee_id).to be_nil
+      expect(issue.reload.assignee_ids).to be_empty
       expect(merge_request.reload.assignee_id).to be_nil
     end
   end