From a7fad65792b37a3f8643149ebbee64cfabfbcea8 Mon Sep 17 00:00:00 2001 From: David Workman Date: Mon, 23 May 2011 13:54:38 +0100 Subject: Simple fix for correctly inverting an add_index migration when a name has been provided --- activerecord/lib/active_record/migration/command_recorder.rb | 8 ++++++-- activerecord/test/cases/migration/command_recorder_test.rb | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index c9d57ce812..5f4dd798a7 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -79,8 +79,12 @@ module ActiveRecord end def invert_add_index(args) - table, columns, _ = *args - [:remove_index, [table, {:column => columns}]] + table, columns, options = *args + if options && options[:name] + [:remove_index, [table, {:name => options[:name]}]] + else + [:remove_index, [table, {:column => columns}]] + end end def invert_remove_timestamps(args) diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index ae531ebb4c..3e404eaf70 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -86,6 +86,12 @@ module ActiveRecord assert_equal [:remove_index, [:table, {:column => [:one, :two]}]], remove end + def test_invert_add_index_with_name + @recorder.record :add_index, [:table, [:one, :two], {:name => "new_index"}] + remove = @recorder.inverse.first + assert_equal [:remove_index, [:table, {:name => "new_index"}]], remove + end + def test_invert_rename_index @recorder.record :rename_index, [:old, :new] rename = @recorder.inverse.first -- cgit v1.2.3 From 8588dd431b03616ed815c8ee82a387540a6e571b Mon Sep 17 00:00:00 2001 From: David Workman Date: Mon, 23 May 2011 15:03:42 +0100 Subject: Neatened up the invert_add_index method as per suggeston --- activerecord/lib/active_record/migration/command_recorder.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 5f4dd798a7..5923993229 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -81,10 +81,11 @@ module ActiveRecord def invert_add_index(args) table, columns, options = *args if options && options[:name] - [:remove_index, [table, {:name => options[:name]}]] + options_hash = {:name => options[:name]} else - [:remove_index, [table, {:column => columns}]] + options_hash = {:column => columns} end + [:remove_index, [table, options_hash]] end def invert_remove_timestamps(args) -- cgit v1.2.3 From 86afbf7464e96dd156895eba2fa44b41678cdd6e Mon Sep 17 00:00:00 2001 From: David Workman Date: Mon, 23 May 2011 22:33:03 +0100 Subject: Using .try to test for the existence of a method option in a nil-resistent manner. Inlined the determination of the options hash for reversing using a ternary operator. Shortens the method in a way that keeps the code neat --- activerecord/lib/active_record/migration/command_recorder.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 5923993229..88752aa013 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -80,11 +80,8 @@ module ActiveRecord def invert_add_index(args) table, columns, options = *args - if options && options[:name] - options_hash = {:name => options[:name]} - else - options_hash = {:column => columns} - end + index_name = options.try(:[], :name) + options_hash = index_name ? {:name => index_name} : {:column => columns} [:remove_index, [table, options_hash]] end -- cgit v1.2.3 From 06436b2cade183db3a231150555c4c999ca2827a Mon Sep 17 00:00:00 2001 From: David Workman Date: Tue, 24 May 2011 17:06:40 +0100 Subject: Added a test to check for correct behaviour with no options in add_index command recorder --- activerecord/test/cases/migration/command_recorder_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 3e404eaf70..0f79c99e1a 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -92,6 +92,12 @@ module ActiveRecord assert_equal [:remove_index, [:table, {:name => "new_index"}]], remove end + def test_invert_add_index_with_no_options + @recorder.record :add_index, [:table, [:one, :two]] + remove = @recorder.inverse.first + assert_equal [:remove_index, [:table, {:column => [:one, :two]}]], remove + end + def test_invert_rename_index @recorder.record :rename_index, [:old, :new] rename = @recorder.inverse.first -- cgit v1.2.3