Fix `transaction` reverting for migrations

[fatkodima & David Verhasselt]

3 files changed, 40 insertions, 0 deletions

diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb
index 363beb4780..6cf17ac15d 100644
--- a/activerecord/test/cases/invertible_migration_test.rb
+++ b/activerecord/test/cases/invertible_migration_test.rb
@@ -22,6 +22,14 @@ module ActiveRecord
       end
     end
 
+    class InvertibleTransactionMigration < InvertibleMigration
+      def change
+        transaction do
+          super
+        end
+      end
+    end
+
     class InvertibleRevertMigration < SilentMigration
       def change
         revert do
@@ -271,6 +279,14 @@ module ActiveRecord
       assert_not revert.connection.table_exists?("horses")
     end
 
+    def test_migrate_revert_transaction
+      migration = InvertibleTransactionMigration.new
+      migration.migrate :up
+      assert migration.connection.table_exists?("horses")
+      migration.migrate :down
+      assert_not migration.connection.table_exists?("horses")
+    end
+
     def test_migrate_revert_change_column_default
       migration1 = ChangeColumnDefault1.new
       migration1.migrate(:up)
diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb
index 199818fc90..01f8628fc5 100644
--- a/activerecord/test/cases/migration/command_recorder_test.rb
+++ b/activerecord/test/cases/migration/command_recorder_test.rb
@@ -360,6 +360,16 @@ module ActiveRecord
           @recorder.inverse_of :remove_foreign_key, [:dogs]
         end
       end
+
+      def test_invert_transaction_with_irreversible_inside_is_irreversible
+        assert_raises(ActiveRecord::IrreversibleMigration) do
+          @recorder.revert do
+            @recorder.transaction do
+              @recorder.execute "some sql"
+            end
+          end
+        end
+      end
     end
   end
 end
diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
index 69a50674af..017ee7951e 100644
--- a/activerecord/test/cases/migration/compatibility_test.rb
+++ b/activerecord/test/cases/migration/compatibility_test.rb
@@ -127,6 +127,20 @@ module ActiveRecord
         assert_match(/LegacyMigration < ActiveRecord::Migration\[4\.2\]/, e.message)
       end
 
+      def test_legacy_migrations_not_raise_exception_on_reverting_transaction
+        migration = Class.new(ActiveRecord::Migration[5.2]) {
+          def change
+            transaction do
+              execute "select 1"
+            end
+          end
+        }.new
+
+        assert_nothing_raised do
+          migration.migrate(:down)
+        end
+      end
+
       if current_adapter?(:PostgreSQLAdapter)
         class Testing < ActiveRecord::Base
         end