Made add_foreign_key retriable with safe_by_default and Active Record…

… < 7.1
ankane · Nov 7, 2024 · a1a62e4 · a1a62e4
1 parent 1cf8ba0
commit a1a62e4
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 8 deletions.
diff --git a/lib/strong_migrations/safe_methods.rb b/lib/strong_migrations/safe_methods.rb
@@ -47,9 +47,9 @@ def safe_add_reference(table, reference, *args, **options)
     def safe_add_foreign_key(from_table, to_table, *args, **options)
       @migration.reversible do |dir|
         dir.up do
-          # validate option is ignored for Active Record < 7.1
+          # validate option is unintentionally ignored for Active Record < 7.1
           # https://github.com/rails/rails/pull/45896
-          if ar_version < 7.1 || !connection.foreign_key_exists?(from_table, to_table, **options.merge(validate: false))
+          if !connection.foreign_key_exists?(from_table, to_table, **options.merge(validate: false))
             @migration.add_foreign_key(from_table, to_table, *args, **options.merge(validate: false))
           end
           disable_transaction

diff --git a/test/safe_by_default_test.rb b/test/safe_by_default_test.rb
@@ -146,16 +146,14 @@ def test_add_foreign_key_invalid
 
     user.update!(order_id: nil)
 
+    migrate AddForeignKey
+
     if ActiveRecord::VERSION::STRING.to_f >= 7.1
-      migrate AddForeignKey
-    else
+      # fail if trying to add the same foreign key in a future migration
       assert_raises(ActiveRecord::StatementInvalid) do
         migrate AddForeignKey
       end
-    end
-
-    # fail if trying to add the same foreign key in a future migration
-    assert_raises(ActiveRecord::StatementInvalid) do
+    else
       migrate AddForeignKey
     end