From 7c87723f8647638ad0919810373d3d86c846bb4d Mon Sep 17 00:00:00 2001
From: fatkodima <fatkodima123@gmail.com>
Date: Tue, 27 Aug 2024 21:24:04 +0300
Subject: [PATCH] Fix `MissingNonNullConstraint` to work with
 `ActiveRecord.belongs_to_required_validates_foreign_key`

---
 .../detectors/missing_non_null_constraint.rb  | 18 +++++++++++++--
 ...orrect_boolean_presence_validation_test.rb |  9 ++++++++
 .../missing_non_null_constraint_test.rb       | 22 +++++++++++++++++++
 test/setup.rb                                 |  7 ++++++
 4 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/lib/active_record_doctor/detectors/missing_non_null_constraint.rb b/lib/active_record_doctor/detectors/missing_non_null_constraint.rb
index 9308c80..fe06a7f 100644
--- a/lib/active_record_doctor/detectors/missing_non_null_constraint.rb
+++ b/lib/active_record_doctor/detectors/missing_non_null_constraint.rb
@@ -67,14 +67,28 @@ def required_presence_validators(model)
         model.validators.select do |validator|
           validator.is_a?(ActiveRecord::Validations::PresenceValidator) &&
             !validator.options[:allow_nil] &&
-            !validator.options[:if] &&
-            !validator.options[:unless]
+            (rails_belongs_to_presence_validator?(validator) || !conditional_validator?(validator))
         end
       end
 
       def sti_column?(models, column_name)
         models.any? { |model| model.inheritance_column == column_name }
       end
+
+      def rails_belongs_to_presence_validator?(validator)
+        ActiveRecord.version >= Gem::Version.new("7.1") &&
+          !ActiveRecord.belongs_to_required_validates_foreign_key &&
+          validator.options[:message] == :required &&
+          proc_from_activerecord?(validator.options[:if])
+      end
+
+      def conditional_validator?(validator)
+        validator.options[:if] || validator.options[:unless]
+      end
+
+      def proc_from_activerecord?(object)
+        object.is_a?(Proc) && object.binding.receiver.name.start_with?("ActiveRecord")
+      end
     end
   end
 end
diff --git a/test/active_record_doctor/detectors/incorrect_boolean_presence_validation_test.rb b/test/active_record_doctor/detectors/incorrect_boolean_presence_validation_test.rb
index d6c92c7..2f2688c 100644
--- a/test/active_record_doctor/detectors/incorrect_boolean_presence_validation_test.rb
+++ b/test/active_record_doctor/detectors/incorrect_boolean_presence_validation_test.rb
@@ -12,8 +12,17 @@ def test_presence_true_is_reported_on_boolean_only
       validates :email, :active, presence: true
     end
 
+    # Another model is defined in a different database to ensure that all
+    # databases are checked.
+    SecondaryContext.create_table(:users) do |t|
+      t.boolean :active, null: false
+    end.define_model do
+      validates :active, presence: true
+    end
+
     assert_problems(<<~OUTPUT)
       replace the `presence` validator on Context::User.active with `inclusion` - `presence` can't be used on booleans
+      replace the `presence` validator on SecondaryContext::User.active with `inclusion` - `presence` can't be used on booleans
     OUTPUT
   end
 
diff --git a/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb b/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb
index af2e992..580a868 100644
--- a/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb
+++ b/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb
@@ -39,6 +39,28 @@ def test_optional_columns_with_required_polymorphic_association_are_disallowed
     OUTPUT
   end
 
+  def test_optional_foreign_keys_with_required_with_if_association_are_disallowed
+    skip("ActiveRecord < 7.1 doesn't support optimized association presence validation") if ActiveRecord::VERSION::STRING < "7.1"
+
+    begin
+      previous = ActiveRecord.belongs_to_required_validates_foreign_key
+      ActiveRecord.belongs_to_required_validates_foreign_key = false
+
+      Context.create_table(:companies)
+      Context.create_table(:users) do |t|
+        t.references :company, null: true
+      end.define_model do
+        belongs_to :company, required: true
+      end
+
+      assert_problems(<<~OUTPUT)
+        add `NOT NULL` to users.company_id - models validates its presence but it's not non-NULL in the database
+      OUTPUT
+    ensure
+      ActiveRecord.belongs_to_required_validates_foreign_key = previous
+    end
+  end
+
   def test_required_columns_with_required_polymorphic_association_are_allowed
     Context.create_table(:comments) do |t|
       t.references :commentable, polymorphic: true, null: false
diff --git a/test/setup.rb b/test/setup.rb
index 4ede3e8..f7480e1 100644
--- a/test/setup.rb
+++ b/test/setup.rb
@@ -99,6 +99,13 @@ class SecondaryRecord < ApplicationRecord
   SecondaryContext = TransientRecord.context_for SecondaryRecord
 end
 
+if ActiveRecord.version >= Gem::Version.new("7.1")
+  # See https://github.com/rails/rails/pull/46522 for details.
+  # When `false` (default in Rails 7.1) - validate presence only when the foreign key changed.
+  # When `true` - always validate the association presence.
+  ActiveRecord.belongs_to_required_validates_foreign_key = true
+end
+
 # Prepare the test class.
 class Minitest::Test
   def setup