From 0e928de345a99a6173efbaa5c87e472dd86e4110 Mon Sep 17 00:00:00 2001
From: Yves Senn <yves.senn@gmail.com>
Date: Mon, 15 Jun 2015 09:12:55 +0200
Subject: [PATCH] make `remove_index :table, :column` reversible.

This used to raise a `IrreversibleMigration` error (since #10437).
However since `remove_index :table, :column` is probably the most basic
use-case we should make it reversible again.

Conflicts:
	activerecord/CHANGELOG.md
---
 activerecord/CHANGELOG.md                        |  4 ++++
 .../abstract/schema_statements.rb                |  2 +-
 .../active_record/migration/command_recorder.rb  | 16 +++++++++-------
 .../test/cases/invertible_migration_test.rb      | 16 ++++++++++------
 .../cases/migration/command_recorder_test.rb     |  5 +++++
 5 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index a7cd6d4d54..d570f8e965 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Make `remove_index :table, :column` reversible.
+
+    *Yves Senn*
+
 *   Fixed an error which would occur in dirty checking when calling
     `update_attributes` from a getter.
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index c8be038d76..49ffd7ccf0 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -587,7 +587,7 @@ def add_index(table_name, column_name, options = {})
       #
       # Removes the +index_accounts_on_column+ in the +accounts+ table.
       #
-      #   remove_index :accounts, :column
+      #   remove_index :accounts, :branch_id
       #
       # Removes the index named +index_accounts_on_branch_id+ in the +accounts+ table.
       #
diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb
index ee4545ed71..b592c004aa 100644
--- a/activerecord/lib/active_record/migration/command_recorder.rb
+++ b/activerecord/lib/active_record/migration/command_recorder.rb
@@ -151,14 +151,16 @@ def invert_add_index(args)
       end
 
       def invert_remove_index(args)
-        table, options = *args
-
-        unless options && options.is_a?(Hash) && options[:column]
-          raise ActiveRecord::IrreversibleMigration, "remove_index is only reversible if given a :column option."
+        table, options_or_column = *args
+        if (options = options_or_column).is_a?(Hash)
+          unless options[:column]
+            raise ActiveRecord::IrreversibleMigration, "remove_index is only reversible if given a :column option."
+          end
+          options = options.dup
+          [:add_index, [table, options.delete(:column), options]]
+        elsif (column = options_or_column).present?
+          [:add_index, [table, column]]
         end
-
-        options = options.dup
-        [:add_index, [table, options.delete(:column), options]]
       end
 
       alias :invert_add_belongs_to :invert_add_reference
diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb
index 8144f3e5c5..99230aa3d5 100644
--- a/activerecord/test/cases/invertible_migration_test.rb
+++ b/activerecord/test/cases/invertible_migration_test.rb
@@ -144,13 +144,17 @@ def test_no_reverse
     end
 
     def test_exception_on_removing_index_without_column_option
-      RemoveIndexMigration1.new.migrate(:up)
-      migration = RemoveIndexMigration2.new
-      migration.migrate(:up)
+      index_definition = ["horses", [:name, :color]]
+      migration1 = RemoveIndexMigration1.new
+      migration1.migrate(:up)
+      assert migration1.connection.index_exists?(*index_definition)
 
-      assert_raises(IrreversibleMigration) do
-        migration.migrate(:down)
-      end
+      migration2 = RemoveIndexMigration2.new
+      migration2.migrate(:up)
+      assert_not migration2.connection.index_exists?(*index_definition)
+
+      migration2.migrate(:down)
+      assert migration2.connection.index_exists?(*index_definition)
     end
 
     def test_migrate_up
diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb
index 24d3c085a7..90b7c6b38a 100644
--- a/activerecord/test/cases/migration/command_recorder_test.rb
+++ b/activerecord/test/cases/migration/command_recorder_test.rb
@@ -206,6 +206,11 @@ def test_invert_add_index_with_no_options
       end
 
       def test_invert_remove_index
+        add = @recorder.inverse_of :remove_index, [:table, :one]
+        assert_equal [:add_index, [:table, :one]], add
+      end
+
+      def test_invert_remove_index_with_column
         add = @recorder.inverse_of :remove_index, [:table, {column: [:one, :two], options: true}]
         assert_equal [:add_index, [:table, [:one, :two], options: true]], add
       end
-- 
GitLab