Skip to content
Browse files

Merge pull request #3603 from vijaydev/change_table_without_block_arg

Modify change_table to remove the need for the block argument
  • Loading branch information...
2 parents 23101de + 6e112e4 commit 81fad6a270ec3cbbb88553c9f2e8200c34fd4d13 @josevalim josevalim committed Nov 10, 2011
View
19 activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -241,14 +241,19 @@ def create_table(table_name, options = {}, &blk)
#
# See also Table for details on
# all of the various column transformation
- def change_table(table_name, options = {})
- if supports_bulk_alter? && options[:bulk]
- recorder = ActiveRecord::Migration::CommandRecorder.new(self)
- yield Table.new(table_name, recorder)
- bulk_change_table(table_name, recorder.commands)
- else
- yield Table.new(table_name, self)
+ def change_table(table_name, options = {}, &blk)
+ bulk_change = supports_bulk_alter? && options[:bulk]
+ recorder = bulk_change ? ActiveRecord::Migration::CommandRecorder.new(self) : self
+ table = Table.new(table_name, recorder)
+
+ if block_given?
+ if blk.arity == 1
+ yield table
+ else
+ table.instance_eval(&blk)
+ end
end
+ bulk_change_table(table_name, recorder.commands) if bulk_change
end
# Renames a table.
View
52 activerecord/test/cases/migration_test.rb
@@ -1757,6 +1757,58 @@ def test_create_table_should_not_have_mixed_syntax
end
end
end
+
+ def test_change_table_without_block_parameter_no_bulk
+ Person.connection.create_table :testings, :force => true do
+ string :foo
+ end
+ assert Person.connection.column_exists?(:testings, :foo, :string)
+
+ Person.connection.change_table :testings do
+ remove :foo
+ integer :bar
+ end
+
+ assert_equal %w(bar id), Person.connection.columns(:testings).map { |c| c.name }.sort
+ ensure
+ Person.connection.drop_table :testings rescue nil
+ end
+
+ def test_change_table_without_block_parameter_with_bulk
+ Person.connection.create_table :testings, :force => true do
+ string :foo
+ end
+ assert Person.connection.column_exists?(:testings, :foo, :string)
+
+ assert_queries(1) do
+ Person.connection.change_table(:testings, :bulk => true) do
+ integer :bar
+ string :foo_bar
+ end
+ end
+
+ assert_equal %w(bar foo foo_bar id), Person.connection.columns(:testings).map { |c| c.name }.sort
+ ensure
+ Person.connection.drop_table :testings rescue nil
+ end
+
+ def test_change_table_should_not_have_mixed_syntax
+ Person.connection.create_table :testings, :force => true do
+ string :foo
+ end
+ assert_raise(NoMethodError) do
+ Person.connection.change_table :testings do |t|
+ t.remove :foo
+ integer :bar
+ end
+ end
+ assert_raise(NameError) do
+ Person.connection.change_table :testings do
+ t.remove :foo
+ integer :bar
+ end
+ end
+ end
end # SexierMigrationsTest
class MigrationLoggerTest < ActiveRecord::TestCase

0 comments on commit 81fad6a

Please sign in to comment.
Something went wrong with that request. Please try again.