Merge branch 'fix_assignee_yourick_approach' into 'master'

Improve migration for Multipple issue assignee feature

See merge request !11417

8 files changed, 207 insertions, 55 deletions

diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb
index a85b9465c84..cd16ab7e52f 100644
--- a/app/services/members/authorized_destroy_service.rb
+++ b/app/services/members/authorized_destroy_service.rb
@@ -29,7 +29,7 @@ module Members
         issue_ids = IssuesFinder.new(user, group_id: member.source_id, assignee_id: member.user_id).
           execute.pluck(:id)
 
-        IssueAssignee.destroy_all(issue_id: issue_ids, user_id: member.user_id)
+        IssueAssignee.delete_all(issue_id: issue_ids, user_id: member.user_id)
 
         MergeRequestsFinder.new(user, group_id: member.source_id, assignee_id: member.user_id).
           execute.
@@ -37,7 +37,7 @@ module Members
       else
         project = member.source
 
-        IssueAssignee.destroy_all(
+        IssueAssignee.delete_all(
           user_id: member.user_id,
           issue_id: project.issues.opened.assigned_to(member.user).select(:id)
         )
diff --git a/db/migrate/20170320171632_create_issue_assignees_table.rb b/db/migrate/20170320171632_create_issue_assignees_table.rb
deleted file mode 100644
index 23b8da37b6d..00000000000
--- a/db/migrate/20170320171632_create_issue_assignees_table.rb
+++ /dev/null
@@ -1,40 +0,0 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class CreateIssueAssigneesTable < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  INDEX_NAME = 'index_issue_assignees_on_issue_id_and_user_id'
-
-  # 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" 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" make sure that this
-  # method 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 fails and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-
-  def up
-    create_table :issue_assignees do |t|
-      t.references :user, foreign_key: { on_delete: :cascade }, index: true, null: false
-      t.references :issue, foreign_key: { on_delete: :cascade }, null: false
-    end
-
-    add_index :issue_assignees, [:issue_id, :user_id], unique: true, name: INDEX_NAME
-  end
-
-  def down
-    drop_table :issue_assignees
-  end
-end
diff --git a/db/migrate/20170320173259_migrate_assignees.rb b/db/migrate/20170320173259_migrate_assignees.rb
index ba8edbd7d32..23e7500a32d 100644
--- a/db/migrate/20170320173259_migrate_assignees.rb
+++ b/db/migrate/20170320173259_migrate_assignees.rb
@@ -37,16 +37,8 @@ class MigrateAssignees < ActiveRecord::Migration
           users.project("true").where(users[:id].eq(table[:assignee_id])).exists.not
         ))
     end
-
-    execute <<-EOF
-      INSERT INTO issue_assignees(issue_id, user_id)
-      SELECT id, assignee_id FROM issues WHERE assignee_id IS NOT NULL
-    EOF
   end
 
   def down
-    execute <<-EOF
-      DELETE FROM issue_assignees
-    EOF
   end
 end
diff --git a/db/migrate/20170516153305_migrate_assignee_to_separate_table.rb b/db/migrate/20170516153305_migrate_assignee_to_separate_table.rb
new file mode 100644
index 00000000000..f269ca7fc34
--- /dev/null
+++ b/db/migrate/20170516153305_migrate_assignee_to_separate_table.rb
@@ -0,0 +1,83 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class MigrateAssigneeToSeparateTable < 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!
+
+  def up
+    drop_table(:issue_assignees) if table_exists?(:issue_assignees)
+
+    if Gitlab::Database.mysql?
+      execute <<-EOF
+        CREATE TABLE issue_assignees AS
+          SELECT assignee_id AS user_id, id AS issue_id FROM issues WHERE assignee_id IS NOT NULL
+      EOF
+    else
+      ActiveRecord::Base.transaction do
+        execute('LOCK TABLE issues IN EXCLUSIVE MODE')
+
+        execute <<-EOF
+          CREATE TABLE issue_assignees AS
+            SELECT assignee_id AS user_id, id AS issue_id FROM issues WHERE assignee_id IS NOT NULL
+        EOF
+
+        execute <<-EOF
+            CREATE OR REPLACE FUNCTION replicate_assignee_id()
+            RETURNS trigger AS
+            $BODY$
+            BEGIN
+              if OLD.assignee_id IS NOT NULL THEN
+                  DELETE FROM issue_assignees WHERE issue_id = OLD.id;
+              END IF;
+
+              if NEW.assignee_id IS NOT NULL THEN
+                  INSERT INTO issue_assignees (user_id, issue_id) VALUES (NEW.assignee_id, NEW.id);
+              END IF;
+
+              RETURN NEW;
+            END;
+            $BODY$
+            LANGUAGE 'plpgsql'
+            VOLATILE;
+
+            CREATE TRIGGER replicate_assignee_id
+            BEFORE INSERT OR UPDATE OF assignee_id
+            ON issues
+            FOR EACH ROW EXECUTE PROCEDURE replicate_assignee_id();
+        EOF
+      end
+    end
+  end
+
+  def down
+    drop_table(:issue_assignees) if table_exists?(:issue_assignees)
+
+    if Gitlab::Database.postgresql?
+      execute <<-EOF
+        DROP TRIGGER IF EXISTS replicate_assignee_id ON issues;
+        DROP FUNCTION IF EXISTS replicate_assignee_id();
+      EOF
+    end
+  end
+end
diff --git a/db/migrate/20170516183131_add_indices_to_issue_assignees.rb b/db/migrate/20170516183131_add_indices_to_issue_assignees.rb
new file mode 100644
index 00000000000..a1f064c6848
--- /dev/null
+++ b/db/migrate/20170516183131_add_indices_to_issue_assignees.rb
@@ -0,0 +1,41 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIndicesToIssueAssignees < 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!
+
+  def up
+    add_concurrent_index :issue_assignees, [:issue_id, :user_id], unique: true, name: 'index_issue_assignees_on_issue_id_and_user_id'
+    add_concurrent_index :issue_assignees, :user_id, name: 'index_issue_assignees_on_user_id'
+    add_concurrent_foreign_key :issue_assignees, :users, column: :user_id, on_delete: :cascade
+    add_concurrent_foreign_key :issue_assignees, :issues, column: :issue_id, on_delete: :cascade
+  end
+
+  def down
+    remove_foreign_key :issue_assignees, column: :user_id
+    remove_foreign_key :issue_assignees, column: :issue_id
+    remove_concurrent_index :issue_assignees, [:issue_id, :user_id] if index_exists?(:issue_assignees, [:issue_id, :user_id])
+    remove_concurrent_index :issue_assignees, :user_id if index_exists?(:issue_assignees, :user_id)
+  end
+end
diff --git a/db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb b/db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb
new file mode 100644
index 00000000000..378fe5603c3
--- /dev/null
+++ b/db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb
@@ -0,0 +1,39 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CleanupTriggerForIssues < 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!
+
+  def up
+    if Gitlab::Database.postgresql?
+      execute <<-EOF
+        DROP TRIGGER IF EXISTS replicate_assignee_id ON issues;
+        DROP FUNCTION IF EXISTS replicate_assignee_id();
+      EOF
+    end
+  end
+
+  def down
+  end
+end
diff --git a/db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb b/db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb
new file mode 100644
index 00000000000..2aab1f4d14f
--- /dev/null
+++ b/db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb
@@ -0,0 +1,37 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddConstraintsToIssueAssigneesTable < 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!
+
+  def up
+    change_column :issue_assignees, :issue_id, :integer, null: false
+    change_column :issue_assignees, :user_id, :integer, null: false
+  end
+
+  def down
+    change_column :issue_assignees, :issue_id, :integer
+    change_column :issue_assignees, :user_id, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index aa76017d759..0db78259319 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,8 +11,8 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170506185517) do
-
+ActiveRecord::Schema.define(version: 20170516183131) do
+

   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
   enable_extension "pg_trgm"
@@ -457,7 +457,7 @@ ActiveRecord::Schema.define(version: 20170506185517) do
 
   add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree
 
-  create_table "issue_assignees", force: :cascade do |t|
+  create_table "issue_assignees", id: false, force: :cascade do |t|
     t.integer "user_id", null: false
     t.integer "issue_id", null: false
   end
@@ -1417,8 +1417,8 @@ ActiveRecord::Schema.define(version: 20170506185517) do
   add_foreign_key "ci_trigger_requests", "ci_triggers", column: "trigger_id", name: "fk_b8ec8b7245", on_delete: :cascade
   add_foreign_key "ci_triggers", "users", column: "owner_id", name: "fk_e8e10d1964", on_delete: :cascade
   add_foreign_key "container_repositories", "projects"
-  add_foreign_key "issue_assignees", "issues", on_delete: :cascade
-  add_foreign_key "issue_assignees", "users", on_delete: :cascade
+  add_foreign_key "issue_assignees", "issues", name: "fk_b7d881734a", on_delete: :cascade
+  add_foreign_key "issue_assignees", "users", name: "fk_5e0c8d9154", on_delete: :cascade
   add_foreign_key "issue_metrics", "issues", on_delete: :cascade
   add_foreign_key "label_priorities", "labels", on_delete: :cascade
   add_foreign_key "label_priorities", "projects", on_delete: :cascade