Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

raise IrreversibleMigration if no column given #10437

Merged
merged 1 commit into from

2 participants

Neeraj Singh Rafael Mendonça França
Neeraj Singh
Collaborator

fixes #10419

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.

Rafael Mendonça França
Owner

Seems good. Could you add a CHANGELOG entry?

Neeraj Singh neerajdotname raise IrreversibleMigration if no column given
fixes #10419

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.
3771e4d
Neeraj Singh
Collaborator

@rafaelfranca changelog added.

Rafael Mendonça França rafaelfranca merged commit 4be5bce into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 6, 2013
  1. Neeraj Singh

    raise IrreversibleMigration if no column given

    neerajdotname authored
    fixes #10419
    
    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.
This page is out of date. Refresh to see the latest.
17 activerecord/CHANGELOG.md
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)
5 activerecord/lib/active_record/migration/command_recorder.rb
View
@@ -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]]
28 activerecord/test/cases/invertible_migration_test.rb
View
@@ -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)
Something went wrong with that request. Please try again.