Merge pull request #20018 from sikachu/change-column-default-recorder

Add reversible syntax for change_column_default

10 files changed, 72 insertions, 13 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d87e609956..794bb779fa 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,15 @@
+*   Add alternate syntax to make `change_column_default` reversible.
+
+    User can pass in `:from` and `:to` to make `change_column_default` command
+    become reversible.
+
+    Example:
+
+        change_column_default :posts, :status, from: nil, to: "draft"
+        change_column_default :users, authorized, from: true, to: false
+
+    *Prem Sichanugrist*
+
 *   Prevent error when using `force_reload: true` on an unassigned polymorphic
     belongs_to association.
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 158b773e11..d17e272ed1 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -592,10 +592,11 @@ module ActiveRecord
       #
       #  t.change_default(:qualification, 'new')
       #  t.change_default(:authorized, 1)
+      #  t.change_default(:status, from: nil, to: "draft")
       #
       # See SchemaStatements#change_column_default
-      def change_default(column_name, default)
-        @base.change_column_default(name, column_name, default)
+      def change_default(column_name, default_or_changes)
+        @base.change_column_default(name, column_name, default_or_changes)
       end
 
       # Removes the column(s) from the table definition.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 49ffd7ccf0..8bbd96e722 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -460,7 +460,12 @@ module ActiveRecord
       #
       #   change_column_default(:users, :email, nil)
       #
-      def change_column_default(table_name, column_name, default)
+      # Passing a hash containing +:from+ and +:to+ will make this change
+      # reversible in migration:
+      #
+      #   change_column_default(:posts, :state, from: nil, to: "draft")
+      #
+      def change_column_default(table_name, column_name, default_or_changes)
         raise NotImplementedError, "change_column_default is not implemented"
       end
 
@@ -1068,6 +1073,14 @@ module ActiveRecord
           raise ArgumentError, "Index name '#{new_name}' on table '#{table_name}' is too long; the limit is #{allowed_index_name_length} characters"
         end
       end
+
+      def extract_new_default_value(default_or_changes)
+        if default_or_changes.is_a?(Hash) && default_or_changes.has_key?(:from) && default_or_changes.has_key?(:to)
+          default_or_changes[:to]
+        else
+          default_or_changes
+        end
+      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 00e3d2965b..2027492f29 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -629,7 +629,8 @@ module ActiveRecord
         end
       end
 
-      def change_column_default(table_name, column_name, default) #:nodoc:
+      def change_column_default(table_name, column_name, default_or_changes) #:nodoc:
+        default = extract_new_default_value(default_or_changes)
         column = column_for(table_name, column_name)
         change_column table_name, column_name, column.sql_type, :default => default
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index 595c635fc0..d114cad16b 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -410,11 +410,12 @@ module ActiveRecord
         end
 
         # Changes the default value of a table column.
-        def change_column_default(table_name, column_name, default) # :nodoc:
+        def change_column_default(table_name, column_name, default_or_changes) # :nodoc:
           clear_cache!
           column = column_for(table_name, column_name)
           return unless column
 
+          default = extract_new_default_value(default_or_changes)
           alter_column_query = "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} %s"
           if default.nil?
             # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 87129c42cf..7c809b088c 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -422,7 +422,9 @@ module ActiveRecord
         end
       end
 
-      def change_column_default(table_name, column_name, default) #:nodoc:
+      def change_column_default(table_name, column_name, default_or_changes) #:nodoc:
+        default = extract_new_default_value(default_or_changes)
+
         alter_table(table_name) do |definition|
           definition[column_name].default = default
         end
diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb
index b592c004aa..dcc2362397 100644
--- a/activerecord/lib/active_record/migration/command_recorder.rb
+++ b/activerecord/lib/active_record/migration/command_recorder.rb
@@ -72,7 +72,7 @@ module ActiveRecord
 
       [:create_table, :create_join_table, :rename_table, :add_column, :remove_column,
         :rename_index, :rename_column, :add_index, :remove_index, :add_timestamps, :remove_timestamps,
-        :change_column_default, :add_reference, :remove_reference, :transaction,
+        :add_reference, :remove_reference, :transaction,
         :drop_join_table, :drop_table, :execute_block, :enable_extension,
         :change_column, :execute, :remove_columns, :change_column_null,
         :add_foreign_key, :remove_foreign_key
@@ -166,6 +166,16 @@ module ActiveRecord
       alias :invert_add_belongs_to :invert_add_reference
       alias :invert_remove_belongs_to :invert_remove_reference
 
+      def invert_change_column_default(args)
+        table, column, options = *args
+
+        unless options && options.is_a?(Hash) && options.has_key?(:from) && options.has_key?(:to)
+          raise ActiveRecord::IrreversibleMigration, "change_column_default is only reversible if given a :from and :to option."
+        end
+
+        [:change_column_default, [table, column, from: options[:to], to: options[:from]]]
+      end
+
       def invert_change_column_null(args)
         args[2] = !args[2]
         [:change_column_null, args]
diff --git a/activerecord/test/cases/migration/columns_test.rb b/activerecord/test/cases/migration/columns_test.rb
index 5fc7702dfa..ab3f584350 100644
--- a/activerecord/test/cases/migration/columns_test.rb
+++ b/activerecord/test/cases/migration/columns_test.rb
@@ -267,6 +267,13 @@ module ActiveRecord
         assert_nil TestModel.new.first_name
       end
 
+      def test_change_column_default_with_from_and_to
+        add_column "test_models", "first_name", :string
+        connection.change_column_default "test_models", "first_name", from: nil, to: "Tester"
+
+        assert_equal "Tester", TestModel.new.first_name
+      end
+
       def test_remove_column_no_second_parameter_raises_exception
         assert_raise(ArgumentError) { connection.remove_column("funny") }
       end
diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb
index 90b7c6b38a..99f1dc65b0 100644
--- a/activerecord/test/cases/migration/command_recorder_test.rb
+++ b/activerecord/test/cases/migration/command_recorder_test.rb
@@ -169,6 +169,16 @@ module ActiveRecord
         end
       end
 
+      def test_invert_change_column_default_with_from_and_to
+        change = @recorder.inverse_of :change_column_default, [:table, :column, from: "old_value", to: "new_value"]
+        assert_equal [:change_column_default, [:table, :column, from: "new_value", to: "old_value"]], change
+      end
+
+      def test_invert_change_column_default_with_from_and_to_with_boolean
+        change = @recorder.inverse_of :change_column_default, [:table, :column, from: true, to: false]
+        assert_equal [:change_column_default, [:table, :column, from: false, to: true]], change
+      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
diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md
index ad069a112e..0b84001ca5 100644
--- a/guides/source/active_record_migrations.md
+++ b/guides/source/active_record_migrations.md
@@ -423,21 +423,23 @@ change_column :products, :part_number, :text
 ```
 
 This changes the column `part_number` on products table to be a `:text` field.
+Note that `change_column` command is irreversible.
 
 Besides `change_column`, the `change_column_null` and `change_column_default`
-methods are used specifically to change a not null constraint and default values of a
-column.
+methods are used specifically to change a not null constraint and default
+values of a column.
 
 ```ruby
 change_column_null :products, :name, false
-change_column_default :products, :approved, false
+change_column_default :products, :approved, from: true, to: false
 ```
 
 This sets `:name` field on products to a `NOT NULL` column and the default
-value of the `:approved` field to false.
+value of the `:approved` field from true to false.
 
-TIP: Unlike `change_column` (and `change_column_default`), `change_column_null`
-is reversible.
+Note: You could also write the above `change_column_default` migration as
+`change_column_default :products, :approved, false`, but unlike the previous
+example, this would make your migration irreversible.
 
 ### Column Modifiers