From fc016e4940a3524b3324a3af95868712d599eaad Mon Sep 17 00:00:00 2001
From: fatkodima <fatkodima123@gmail.com>
Date: Fri, 6 Dec 2024 01:55:03 +0200
Subject: [PATCH] Fix `add_foreign_key` when referencing same table via
 different columns

---
 CHANGELOG.md                                |  1 +
 lib/online_migrations/schema_statements.rb  | 15 +++++++--------
 test/schema_statements/foreign_keys_test.rb |  9 +++++++++
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c68d66b..6ab84c8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## master (unreleased)
 
+- Fix `add_foreign_key` when referencing same table via different columns
 - Make `validate_not_null_constraint`, `remove_foreign_key` and `remove_check_constraint` idempotent
 
 - Add helpers for validating constraints in background
diff --git a/lib/online_migrations/schema_statements.rb b/lib/online_migrations/schema_statements.rb
index ff4600b..50ee806 100644
--- a/lib/online_migrations/schema_statements.rb
+++ b/lib/online_migrations/schema_statements.rb
@@ -813,13 +813,13 @@ def index_name(table_name, options)
     # @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_foreign_key
     #
     def add_foreign_key(from_table, to_table, **options)
-      # Do not consider validation for idempotency.
-      if foreign_key_exists?(from_table, to_table, **options.except(:validate))
-        message = +"Foreign key was not created because it already exists " \
-                   "(this can be due to an aborted migration or similar): from_table: #{from_table}, to_table: #{to_table}"
-        message << ", #{options.inspect}" if options.any?
+      options = foreign_key_options(from_table, to_table, options)
 
-        Utils.say(message)
+      if foreign_key_exists?(from_table, to_table, **options.slice(:column, :primary_key))
+        Utils.say(<<~MSG.squish)
+          Foreign key was not created because it already exists (this can be due to an aborted migration or similar).
+          from_table: #{from_table}, to_table: #{to_table}, options: #{options.inspect}
+        MSG
       else
         super
       end
@@ -859,8 +859,7 @@ def validate_foreign_key(from_table, to_table = nil, **options)
     # @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-remove_foreign_key
     #
     def remove_foreign_key(from_table, to_table = nil, **options)
-      # Do not consider validation for idempotency.
-      if foreign_key_exists?(from_table, to_table, **options.except(:validate))
+      if foreign_key_exists?(from_table, to_table, **options.slice(:name, :to_table, :column))
         super
       else
         Utils.say(<<~MSG.squish)
diff --git a/test/schema_statements/foreign_keys_test.rb b/test/schema_statements/foreign_keys_test.rb
index 605b4ba..1164535 100644
--- a/test/schema_statements/foreign_keys_test.rb
+++ b/test/schema_statements/foreign_keys_test.rb
@@ -15,6 +15,7 @@ def setup
       @connection.create_table(:milestones, force: true) do |t|
         t.bigint :owner_id
         t.bigint :project_id
+        t.bigint :parent_project_id
       end
     end
 
@@ -30,6 +31,14 @@ def test_add_foreign_key_is_idempotent
       assert connection.foreign_key_exists?(:milestones, :projects)
     end
 
+    def test_add_foreign_key_referencing_same_table_by_different_columns
+      connection.add_foreign_key :milestones, :projects, column: :parent_project_id
+      assert_equal 1, connection.foreign_keys(:milestones).size
+
+      connection.add_foreign_key :milestones, :projects
+      assert_equal 2, connection.foreign_keys(:milestones).size
+    end
+
     def test_validate_non_existing_foreign_key
       assert_raises_with_message(ArgumentError, "has no foreign key") do
         connection.validate_foreign_key :milestones, :non_existing