diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6e8f15e86ed4be4a6dd3380ba67bf3b591941b2a..f1398f1f966da0f6b70ab8c6ede047b1f2060937 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/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 :validate 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! diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 5d95c4e1da85bcb990cb0c1bef457ea8b05d3753..86dff6c71e6db2f84c3cc210866ca7d02bb9b51f 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/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 diff --git a/activerecord/test/models/account.rb b/activerecord/test/models/account.rb index f28fe83e5c4e9d072e58095e18ce113eba5c2908..55ec35a892447e70d0bb5b3bddc4d5fb9a329bfe 100644 --- a/activerecord/test/models/account.rb +++ b/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 diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index c8617d1cfe715d5e497508d41adb147b59f80570..52cbc20a94182d28482f8df0e32c9aef161f37d1 100644 --- a/activerecord/test/models/pirate.rb +++ b/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 diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index 7973219a7936a1e0a9138dfdd6b100a7edef9160..ab3bbae0fc3f7b50b8f94a412b803d2576416d12 100644 --- a/activerecord/test/models/ship.rb +++ b/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