From c8bbe9aefae3fdee1eebc8c3e95f5009f5148034 Mon Sep 17 00:00:00 2001 From: Kassio Borges Date: Sun, 8 Nov 2015 10:57:25 -0200 Subject: [PATCH] Improve support for non Active Record objects on `validates_associated` Skipping `marked_for_destruction?` when the associated object does not responds to it make easier to validate virtual associations built on top of Active Model objects and/or serialized objects that implement a `valid?` instance method. --- activerecord/CHANGELOG.md | 8 ++++++++ .../lib/active_record/validations/associated.rb | 10 ++++++++-- .../cases/validations/association_validation_test.rb | 12 ++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c0e3840b40..840ad0d7ce 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Improve support for non Active Record objects on `validates_associated` + + Skipping `marked_for_destruction?` when the associated object does not responds + to it make easier to validate virtual associations built on top of Active Model + objects and/or serialized objects that implement a `valid?` instance method. + + *Kassio Borges*, *Lucas Mazza* + * Change connection management middleware to return a new response with a body proxy, rather than mutating the original. diff --git a/activerecord/lib/active_record/validations/associated.rb b/activerecord/lib/active_record/validations/associated.rb index 32fbaf0a91..b14db85167 100644 --- a/activerecord/lib/active_record/validations/associated.rb +++ b/activerecord/lib/active_record/validations/associated.rb @@ -2,10 +2,16 @@ module ActiveRecord module Validations class AssociatedValidator < ActiveModel::EachValidator #:nodoc: def validate_each(record, attribute, value) - if Array.wrap(value).reject {|r| r.marked_for_destruction? || r.valid?}.any? - record.errors.add(attribute, :invalid, options.merge(:value => value)) + if Array(value).reject { |r| valid_object?(r) }.any? + record.errors.add(attribute, :invalid, options.merge(value: value)) end end + + private + + def valid_object?(record) + (record.respond_to?(:marked_for_destruction?) && record.marked_for_destruction?) || record.valid? + end end module ClassMethods diff --git a/activerecord/test/cases/validations/association_validation_test.rb b/activerecord/test/cases/validations/association_validation_test.rb index bff5ffa65e..584a3dc0d8 100644 --- a/activerecord/test/cases/validations/association_validation_test.rb +++ b/activerecord/test/cases/validations/association_validation_test.rb @@ -45,6 +45,18 @@ class AssociationValidationTest < ActiveRecord::TestCase assert t.valid? end + def test_validates_associated_without_marked_for_destruction + reply = Class.new do + def valid? + true + end + end + Topic.validates_associated(:replies) + t = Topic.new + t.define_singleton_method(:replies) { [reply.new] } + assert t.valid? + end + def test_validates_associated_with_custom_message_using_quotes Reply.validates_associated :topic, :message=> "This string contains 'single' and \"double\" quotes" Topic.validates_presence_of :content