Permalink
Browse files

Merge pull request #10437 from neerajdotname/10419

raise IrreversibleMigration if no column given
  • Loading branch information...
2 parents 32b4abd + 3771e4d commit 4be5bce125a19a5bc0a60b33e08fdaf2066fa4d9 @rafaelfranca rafaelfranca committed May 6, 2013
View
@@ -1,3 +1,20 @@
+* While removing index if column option is missing then raise IrreversibleMigration exception.
+
+ Following code should raise `IrreversibleMigration`. But the code was
+ failing since options is an array and not a hash.
+
+ def change
+ change_table :users do |t|
+ t.remove_index [:name, :email]
+ end
+ end
+
+ Fix was to check if the options is a Hash before operating on it.
+
+ Fixes #10419.
+
+ *Neeraj Singh*
+
* Do not overwrite manually built records during one-to-one nested attribute assignment
For one-to-one nested associations, if you build the new (in-memory)
@@ -144,7 +144,10 @@ def invert_add_index(args)
def invert_remove_index(args)
table, options = *args
- raise ActiveRecord::IrreversibleMigration, "remove_index is only reversible if given a :column option." unless options && options[:column]
+
+ unless options && options.is_a?(Hash) && 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]]
@@ -58,6 +58,24 @@ def change
end
end
+ class RemoveIndexMigration1 < SilentMigration
+ def self.up
+ create_table("horses") do |t|
+ t.column :name, :text
+ t.column :color, :text
+ t.index [:name, :color]
+ end
+ end
+ end
+
+ class RemoveIndexMigration2 < SilentMigration
+ def change
+ change_table("horses") do |t|
+ t.remove_index [:name, :color]
+ end
+ end
+ end
+
class LegacyMigration < ActiveRecord::Migration
def self.up
create_table("horses") do |t|
@@ -104,6 +122,16 @@ def test_no_reverse
end
end
+ def test_exception_on_removing_index_without_column_option
+ RemoveIndexMigration1.new.migrate(:up)
+ migration = RemoveIndexMigration2.new
+ migration.migrate(:up)
+
+ assert_raises(IrreversibleMigration) do
+ migration.migrate(:down)
+ end
+ end
+
def test_migrate_up
migration = InvertibleMigration.new
migration.migrate(:up)

0 comments on commit 4be5bce

Please sign in to comment.