Merge branch 'fix_assignee_yourick_approach' into 'master'

Improve migration for Multipple issue assignee feature

See merge request !11417

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)
         )

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

db/migrate/20170516153305_migrate_assignee_to_separate_table.rb

+# 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

db/migrate/20170516183131_add_indices_to_issue_assignees.rb

+# 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

db/post_migrate/20170516165238_cleanup_trigger_for_issues.rb

+# 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

db/migrate/20170320171632_create_issue_assignees_table.rb → db/post_migrate/20170516181025_add_constraints_to_issue_assignees_table.rb

 # 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
+class AddConstraintsToIssueAssigneesTable < 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
 
@@ -14,27 +12,26 @@ class CreateIssueAssigneesTable < ActiveRecord::Migration
   # 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.
+  # 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
-    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
+    change_column :issue_assignees, :issue_id, :integer, null: false
+    change_column :issue_assignees, :user_id, :integer, null: false
   end
 
   def down
-    drop_table :issue_assignees
+    change_column :issue_assignees, :issue_id, :integer
+    change_column :issue_assignees, :user_id, :integer
   end
 end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170511101000) do
+ActiveRecord::Schema.define(version: 20170516183131) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -459,7 +459,7 @@ ActiveRecord::Schema.define(version: 20170511101000) 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
@@ -1423,8 +1423,8 @@ ActiveRecord::Schema.define(version: 20170511101000) do
   add_foreign_key "ci_triggers", "users", column: "owner_id", name: "fk_e8e10d1964", on_delete: :cascade
   add_foreign_key "ci_variables", "projects", name: "fk_ada5eb64b3", 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