Merge pull request #37295 from tekin/fix-belongs-to-validation-context-issue

Fix autosave validation context regression

activerecord/lib/active_record/autosave_association.rb

@@ -272,7 +272,7 @@ def changed_for_autosave?
       # or saved. If +autosave+ is +false+ only new records will be returned,
       # unless the parent is/was a new record itself.
       def associated_records_to_validate_or_save(association, new_record, autosave)
-        if new_record
+        if new_record || custom_validation_context?
           association && association.target
         elsif autosave
           association.target.find_all(&:changed_for_autosave?)
@@ -304,7 +304,7 @@ def nested_records_changed_for_autosave?
       def validate_single_association(reflection)
         association = association_instance_get(reflection.name)
         record      = association && association.reader
-        association_valid?(reflection, record) if record && record.changed_for_autosave?
+        association_valid?(reflection, record) if record && (record.changed_for_autosave? || custom_validation_context?)
       end
 
       # Validate the associated records if <tt>:validate</tt> or
@@ -324,7 +324,7 @@ def validate_collection_association(reflection)
       def association_valid?(reflection, record, index = nil)
         return true if record.destroyed? || (reflection.options[:autosave] && record.marked_for_destruction?)
 
-        context = validation_context unless [:create, :update].include?(validation_context)
+        context = validation_context if custom_validation_context?
 
         unless valid = record.valid?(context)
           if reflection.options[:autosave]
@@ -499,6 +499,10 @@ def save_belongs_to_association(reflection)
         end
       end
 
+      def custom_validation_context?
+        validation_context && [:create, :update].exclude?(validation_context)
+      end
+
       def _ensure_no_duplicate_errors
         errors.messages.each_key do |attribute|
           errors[attribute].uniq!

activerecord/test/cases/autosave_association_test.rb

@@ -34,7 +34,7 @@
 require "models/reply"
 
 class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
-  def test_autosave_validation
+  def test_autosave_does_not_pass_through_non_custom_validation_contexts
     person = Class.new(ActiveRecord::Base) {
       self.table_name = "people"
       validate :should_be_cool, on: :create
@@ -55,8 +55,11 @@ def self.name; "Reference"; end
     }
 
     u = person.create!(first_name: "cool")
-    u.update_attributes!(first_name: "nah") # still valid because validation only applies on 'create'
-    assert_predicate reference.create!(person: u), :persisted?
+    u.first_name = "nah"
+
+    assert_predicate u, :valid?
+    r = reference.new(person: u)
+    assert_predicate r, :valid?
   end
 
   def test_should_not_add_the_same_callbacks_multiple_times_for_has_one
@@ -1683,6 +1686,14 @@ def setup
 
     assert_not_predicate @pirate, :valid?
   end
+
+  def test_validations_still_fire_on_unchanged_association_with_custom_validation_context
+    pirate = FamousPirate.create!(catchphrase: "Avast Ye!")
+    pirate.famous_ships.create!
+
+    assert pirate.valid?
+    assert_not pirate.valid?(:conference)
+  end
 end
 
 class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::TestCase
@@ -1727,6 +1738,13 @@ def setup
     @pirate.non_validated_parrot = Parrot.new(name: "")
     assert_predicate @pirate, :valid?
   end
+
+  def test_validations_still_fire_on_unchanged_association_with_custom_validation_context
+    firm_with_low_credit = Firm.create!(name: "Something", account: Account.new(credit_limit: 50))
+
+    assert firm_with_low_credit.valid?
+    assert_not firm_with_low_credit.valid?(:bank_loan)
+  end
 end
 
 class TestAutosaveAssociationValidationsOnAHABTMAssociation < ActiveRecord::TestCase

activerecord/test/models/account.rb

@@ -21,12 +21,17 @@ def self.destroyed_account_ids
   end
 
   validate :check_empty_credit_limit
+  validate :ensure_good_credit, on: :bank_loan
 
   private
     def check_empty_credit_limit
       errors.add("credit_limit", :blank) if credit_limit.blank?
     end
 
+    def ensure_good_credit
+      errors.add(:credit_limit, "too low") unless credit_limit > 10_000
+    end
+
     def private_method
       "Sir, yes sir!"
     end

activerecord/test/models/pirate.rb

@@ -89,6 +89,6 @@ class DestructivePirate < Pirate
 
 class FamousPirate < ActiveRecord::Base
   self.table_name = "pirates"
-  has_many :famous_ships
+  has_many :famous_ships, inverse_of: :famous_pirate, foreign_key: :pirate_id
   validates_presence_of :catchphrase, on: :conference
 end

activerecord/test/models/ship.rb

@@ -36,6 +36,6 @@ class Prisoner < ActiveRecord::Base
 
 class FamousShip < ActiveRecord::Base
   self.table_name = "ships"
-  belongs_to :famous_pirate
+  belongs_to :famous_pirate, foreign_key: :pirate_id
   validates_presence_of :name, on: :conference
 end