mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
make change_column_comment and change_table_comment invertible
We can revert migrations using `change_column_comment` or `change_table_comment` at current master. However, results are not what we expect: comments are remained in new status. This change tells previous comment to these methods in a way like `change_column_default`.
This commit is contained in:
parent
3e66ba91d5
commit
1fe71ebd04
9 changed files with 182 additions and 7 deletions
|
@ -1,3 +1,8 @@
|
|||
* `change_column_comment` and `change_table_comment` are invertible only if
|
||||
`to` and `from` options are specified.
|
||||
|
||||
*Yoshiyuki Kinjo*
|
||||
|
||||
* Don't call commit/rollback callbacks despite a record isn't saved.
|
||||
|
||||
Fixes #29747.
|
||||
|
|
|
@ -1185,12 +1185,22 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
# Changes the comment for a table or removes it if +nil+.
|
||||
def change_table_comment(table_name, comment)
|
||||
#
|
||||
# Passing a hash containing +:from+ and +:to+ will make this change
|
||||
# reversible in migration:
|
||||
#
|
||||
# change_table_comment(:posts, from: "old_comment", to: "new_comment")
|
||||
def change_table_comment(table_name, comment_or_changes)
|
||||
raise NotImplementedError, "#{self.class} does not support changing table comments"
|
||||
end
|
||||
|
||||
# Changes the comment for a column or removes it if +nil+.
|
||||
def change_column_comment(table_name, column_name, comment)
|
||||
#
|
||||
# Passing a hash containing +:from+ and +:to+ will make this change
|
||||
# reversible in migration:
|
||||
#
|
||||
# change_column_comment(:posts, :state, from: "old_comment", to: "new_comment")
|
||||
def change_column_comment(table_name, column_name, comment_or_changes)
|
||||
raise NotImplementedError, "#{self.class} does not support changing column comments"
|
||||
end
|
||||
|
||||
|
@ -1374,6 +1384,7 @@ module ActiveRecord
|
|||
default_or_changes
|
||||
end
|
||||
end
|
||||
alias :extract_new_comment_value :extract_new_default_value
|
||||
|
||||
def can_remove_index_by_name?(options)
|
||||
options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty?
|
||||
|
|
|
@ -285,7 +285,8 @@ module ActiveRecord
|
|||
SQL
|
||||
end
|
||||
|
||||
def change_table_comment(table_name, comment) #:nodoc:
|
||||
def change_table_comment(table_name, comment_or_changes) # :nodoc:
|
||||
comment = extract_new_comment_value(comment_or_changes)
|
||||
comment = "" if comment.nil?
|
||||
execute("ALTER TABLE #{quote_table_name(table_name)} COMMENT #{quote(comment)}")
|
||||
end
|
||||
|
@ -341,7 +342,8 @@ module ActiveRecord
|
|||
change_column table_name, column_name, nil, null: null
|
||||
end
|
||||
|
||||
def change_column_comment(table_name, column_name, comment) #:nodoc:
|
||||
def change_column_comment(table_name, column_name, comment_or_changes) # :nodoc:
|
||||
comment = extract_new_comment_value(comment_or_changes)
|
||||
change_column table_name, column_name, nil, comment: comment
|
||||
end
|
||||
|
||||
|
|
|
@ -418,14 +418,16 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
# Adds comment for given table column or drops it if +comment+ is a +nil+
|
||||
def change_column_comment(table_name, column_name, comment) # :nodoc:
|
||||
def change_column_comment(table_name, column_name, comment_or_changes) # :nodoc:
|
||||
clear_cache!
|
||||
comment = extract_new_comment_value(comment_or_changes)
|
||||
execute "COMMENT ON COLUMN #{quote_table_name(table_name)}.#{quote_column_name(column_name)} IS #{quote(comment)}"
|
||||
end
|
||||
|
||||
# Adds comment for given table or drops it if +comment+ is a +nil+
|
||||
def change_table_comment(table_name, comment) # :nodoc:
|
||||
def change_table_comment(table_name, comment_or_changes) # :nodoc:
|
||||
clear_cache!
|
||||
comment = extract_new_comment_value(comment_or_changes)
|
||||
execute "COMMENT ON TABLE #{quote_table_name(table_name)} IS #{quote(comment)}"
|
||||
end
|
||||
|
||||
|
|
|
@ -14,6 +14,8 @@ module ActiveRecord
|
|||
# * change_column
|
||||
# * change_column_default (must supply a :from and :to option)
|
||||
# * change_column_null
|
||||
# * change_column_comment (must supply a :from and :to option)
|
||||
# * change_table_comment (must supply a :from and :to option)
|
||||
# * create_join_table
|
||||
# * create_table
|
||||
# * disable_extension
|
||||
|
@ -35,7 +37,8 @@ module ActiveRecord
|
|||
:change_column_default, :add_reference, :remove_reference, :transaction,
|
||||
:drop_join_table, :drop_table, :execute_block, :enable_extension, :disable_extension,
|
||||
:change_column, :execute, :remove_columns, :change_column_null,
|
||||
:add_foreign_key, :remove_foreign_key
|
||||
:add_foreign_key, :remove_foreign_key,
|
||||
:change_column_comment, :change_table_comment
|
||||
]
|
||||
include JoinTable
|
||||
|
||||
|
@ -244,6 +247,26 @@ module ActiveRecord
|
|||
[:add_foreign_key, reversed_args]
|
||||
end
|
||||
|
||||
def invert_change_column_comment(args)
|
||||
table, column, options = *args
|
||||
|
||||
unless options && options.is_a?(Hash) && options.has_key?(:from) && options.has_key?(:to)
|
||||
raise ActiveRecord::IrreversibleMigration, "change_column_comment is only reversible if given a :from and :to option."
|
||||
end
|
||||
|
||||
[:change_column_comment, [table, column, from: options[:to], to: options[:from]]]
|
||||
end
|
||||
|
||||
def invert_change_table_comment(args)
|
||||
table, options = *args
|
||||
|
||||
unless options && options.is_a?(Hash) && options.has_key?(:from) && options.has_key?(:to)
|
||||
raise ActiveRecord::IrreversibleMigration, "change_table_comment is only reversible if given a :from and :to option."
|
||||
end
|
||||
|
||||
[:change_table_comment, [table, from: options[:to], to: options[:from]]]
|
||||
end
|
||||
|
||||
def respond_to_missing?(method, _)
|
||||
super || delegate.respond_to?(method)
|
||||
end
|
||||
|
|
|
@ -27,6 +27,16 @@ module ActiveRecord
|
|||
def invert_transaction(args, &block)
|
||||
[:transaction, args, block]
|
||||
end
|
||||
|
||||
def invert_change_column_comment(args)
|
||||
table_name, column_name, comment = args
|
||||
[:change_column_comment, [table_name, column_name, from: comment, to: comment]]
|
||||
end
|
||||
|
||||
def invert_change_table_comment(args)
|
||||
table_name, comment = args
|
||||
[:change_table_comment, [table_name, from: comment, to: comment]]
|
||||
end
|
||||
end
|
||||
|
||||
def create_table(table_name, **options)
|
||||
|
|
|
@ -103,6 +103,32 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
class ChangeColumnComment1 < SilentMigration
|
||||
def change
|
||||
create_table("horses") do |t|
|
||||
t.column :name, :string, comment: "Sekitoba"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
class ChangeColumnComment2 < SilentMigration
|
||||
def change
|
||||
change_column_comment :horses, :name, from: "Sekitoba", to: "Diomed"
|
||||
end
|
||||
end
|
||||
|
||||
class ChangeTableComment1 < SilentMigration
|
||||
def change
|
||||
create_table("horses", comment: "Sekitoba")
|
||||
end
|
||||
end
|
||||
|
||||
class ChangeTableComment2 < SilentMigration
|
||||
def change
|
||||
change_table_comment :horses, from: "Sekitoba", to: "Diomed"
|
||||
end
|
||||
end
|
||||
|
||||
class DisableExtension1 < SilentMigration
|
||||
def change
|
||||
enable_extension "hstore"
|
||||
|
@ -290,6 +316,7 @@ module ActiveRecord
|
|||
def test_migrate_revert_change_column_default
|
||||
migration1 = ChangeColumnDefault1.new
|
||||
migration1.migrate(:up)
|
||||
Horse.reset_column_information
|
||||
assert_equal "Sekitoba", Horse.new.name
|
||||
|
||||
migration2 = ChangeColumnDefault2.new
|
||||
|
@ -302,6 +329,38 @@ module ActiveRecord
|
|||
assert_equal "Sekitoba", Horse.new.name
|
||||
end
|
||||
|
||||
if ActiveRecord::Base.connection.supports_comments?
|
||||
def test_migrate_revert_change_column_comment
|
||||
migration1 = ChangeColumnComment1.new
|
||||
migration1.migrate(:up)
|
||||
Horse.reset_column_information
|
||||
assert_equal "Sekitoba", Horse.columns_hash["name"].comment
|
||||
|
||||
migration2 = ChangeColumnComment2.new
|
||||
migration2.migrate(:up)
|
||||
Horse.reset_column_information
|
||||
assert_equal "Diomed", Horse.columns_hash["name"].comment
|
||||
|
||||
migration2.migrate(:down)
|
||||
Horse.reset_column_information
|
||||
assert_equal "Sekitoba", Horse.columns_hash["name"].comment
|
||||
end
|
||||
|
||||
def test_migrate_revert_change_table_comment
|
||||
connection = ActiveRecord::Base.connection
|
||||
migration1 = ChangeTableComment1.new
|
||||
migration1.migrate(:up)
|
||||
assert_equal "Sekitoba", connection.table_comment("horses")
|
||||
|
||||
migration2 = ChangeTableComment2.new
|
||||
migration2.migrate(:up)
|
||||
assert_equal "Diomed", connection.table_comment("horses")
|
||||
|
||||
migration2.migrate(:down)
|
||||
assert_equal "Sekitoba", connection.table_comment("horses")
|
||||
end
|
||||
end
|
||||
|
||||
if current_adapter?(:PostgreSQLAdapter)
|
||||
def test_migrate_enable_and_disable_extension
|
||||
migration1 = InvertibleMigration.new
|
||||
|
|
|
@ -182,6 +182,40 @@ module ActiveRecord
|
|||
assert_equal [:change_column_default, [:table, :column, from: false, to: true]], change
|
||||
end
|
||||
|
||||
if ActiveRecord::Base.connection.supports_comments?
|
||||
def test_invert_change_column_comment
|
||||
assert_raises(ActiveRecord::IrreversibleMigration) do
|
||||
@recorder.inverse_of :change_column_comment, [:table, :column, "comment"]
|
||||
end
|
||||
end
|
||||
|
||||
def test_invert_change_column_comment_with_from_and_to
|
||||
change = @recorder.inverse_of :change_column_comment, [:table, :column, from: "old_value", to: "new_value"]
|
||||
assert_equal [:change_column_comment, [:table, :column, from: "new_value", to: "old_value"]], change
|
||||
end
|
||||
|
||||
def test_invert_change_column_comment_with_from_and_to_with_nil
|
||||
change = @recorder.inverse_of :change_column_comment, [:table, :column, from: nil, to: "new_value"]
|
||||
assert_equal [:change_column_comment, [:table, :column, from: "new_value", to: nil]], change
|
||||
end
|
||||
|
||||
def test_invert_change_table_comment
|
||||
assert_raises(ActiveRecord::IrreversibleMigration) do
|
||||
@recorder.inverse_of :change_column_comment, [:table, :column, "comment"]
|
||||
end
|
||||
end
|
||||
|
||||
def test_invert_change_table_comment_with_from_and_to
|
||||
change = @recorder.inverse_of :change_table_comment, [:table, from: "old_value", to: "new_value"]
|
||||
assert_equal [:change_table_comment, [:table, from: "new_value", to: "old_value"]], change
|
||||
end
|
||||
|
||||
def test_invert_change_table_comment_with_from_and_to_with_nil
|
||||
change = @recorder.inverse_of :change_table_comment, [:table, from: nil, to: "new_value"]
|
||||
assert_equal [:change_table_comment, [:table, from: "new_value", to: nil]], change
|
||||
end
|
||||
end
|
||||
|
||||
def test_invert_change_column_null
|
||||
add = @recorder.inverse_of :change_column_null, [:table, :column, true]
|
||||
assert_equal [:change_column_null, [:table, :column, false]], add
|
||||
|
|
|
@ -220,6 +220,35 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
if ActiveRecord::Base.connection.supports_comments?
|
||||
def test_change_column_comment_can_be_reverted
|
||||
migration = Class.new(ActiveRecord::Migration[5.2]) {
|
||||
def migrate(x)
|
||||
revert do
|
||||
change_column_comment(:testings, :foo, "comment")
|
||||
end
|
||||
end
|
||||
}.new
|
||||
|
||||
ActiveRecord::Migrator.new(:up, [migration]).migrate
|
||||
assert connection.column_exists?(:testings, :foo, comment: "comment")
|
||||
end
|
||||
|
||||
def test_change_table_comment_can_be_reverted
|
||||
migration = Class.new(ActiveRecord::Migration[5.2]) {
|
||||
def migrate(x)
|
||||
revert do
|
||||
change_table_comment(:testings, "comment")
|
||||
end
|
||||
end
|
||||
}.new
|
||||
|
||||
ActiveRecord::Migrator.new(:up, [migration]).migrate
|
||||
|
||||
assert_equal "comment", connection.table_comment("testings")
|
||||
end
|
||||
end
|
||||
|
||||
if current_adapter?(:PostgreSQLAdapter)
|
||||
class Testing < ActiveRecord::Base
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue