Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Validates_presence_of associated object marked for destruction #6827

Merged
merged 1 commit into from

4 participants

@zephyr-dev

This allows us to mark the parent object as invalid if all associated objects
in a presence validated association are marked for destruction.

See: #6812

activerecord/lib/active_record/validations/presence.rb
@@ -0,0 +1,22 @@
+module ActiveRecord
+ module Validations
+ class PresenceValidator < ActiveModel::Validations::PresenceValidator
+ def validate(record)
+ super(record)
@rafaelfranca Owner

super without arguments will pass the same arguments of the method. We don't need to pass it.

Good call, fixed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
activerecord/lib/active_record/validations/presence.rb
@@ -0,0 +1,22 @@
+module ActiveRecord
+ module Validations
+ class PresenceValidator < ActiveModel::Validations::PresenceValidator
+ def validate(record)
+ super
+ attributes.each do |attribute|
+ next unless record.class.reflect_on_association(attribute)
+ value = record.send(attribute)
+ if Array.wrap(value).all? { |r| r.marked_for_destruction? }

I think there's no need for Array.wrap, could just use Array() instead?

Sure, fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva

I'm ok with the idea, I just think there's a possibility of breaking current applications that may be relying on the current presence validation not ignoring records marked for destruction (even if it's wrong or not).

For that, I think we need a changelog entry, and some docs in this file explaining why this presence validator exists and what makes it different from the Active Model one. Guide updates are also welcome.

Thanks!

@zephyr-dev

Thanks, @carlosantoniodasilva. We'll work on these and update the pull request.

@zephyr-dev

Documentation added.

@rafaelfranca

This pull request cannot be automatically merged. Also, please squash your commits.

@zephyr-dev

Squashed and rebased, thanks!

@zephyr-dev

Hi all. Any update on this?

@rafaelfranca
Owner

@tenderlove @josevalim what do you think?

Brent Wheeldon & Nick Monje AR has a subclass of AM:PresenceValidator.
This allows us to mark the parent object as invalid if all associated objects
in a presence validated association are marked for destruction.

See: rails/rails#6812
9feda92
@zephyr-dev

Is there an update on this?

@josevalim
Owner

This looks good to me.

@josevalim josevalim merged commit 1fab518 into from
@zephyr-dev

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 20, 2012
  1. AR has a subclass of AM:PresenceValidator.

    Brent Wheeldon & Nick Monje authored Ben Moss committed
    This allows us to mark the parent object as invalid if all associated objects
    in a presence validated association are marked for destruction.
    
    See: rails/rails#6812
This page is out of date. Refresh to see the latest.
View
7 activerecord/CHANGELOG.md
@@ -31,6 +31,13 @@
*kennyj*
+* Changed validates_presence_of on an association so that children objects
+ do not validate as being present if they are marked for destruction. This
+ prevents you from saving the parent successfully and thus putting the parent
+ in an invalid state.
+
+ *Nick Monje & Brent Wheeldon*
+
* `FinderMethods#exists?` now returns `false` with the `false` argument.
*Egor Lynko*
View
1  activerecord/lib/active_record/validations.rb
@@ -81,3 +81,4 @@ def perform_validations(options={})
require "active_record/validations/associated"
require "active_record/validations/uniqueness"
+require "active_record/validations/presence"
View
64 activerecord/lib/active_record/validations/presence.rb
@@ -0,0 +1,64 @@
+module ActiveRecord
+ module Validations
+ class PresenceValidator < ActiveModel::Validations::PresenceValidator
+ def validate(record)
+ super
+ attributes.each do |attribute|
+ next unless record.class.reflect_on_association(attribute)
+ value = record.send(attribute)
+ if Array(value).all? { |r| r.marked_for_destruction? }
+ record.errors.add(attribute, :blank, options)
+ end
+ end
+ end
+ end
+
+ module ClassMethods
+ # Validates that the specified attributes are not blank (as defined by
+ # Object#blank?), and, if the attribute is an association, that the
+ # associated object is not marked for destruction. Happens by default
+ # on save.
+ #
+ # class Person < ActiveRecord::Base
+ # has_one :face
+ # validates_presence_of :face
+ # end
+ #
+ # The face attribute must be in the object and it cannot be blank or marked
+ # for destruction.
+ #
+ # If you want to validate the presence of a boolean field (where the real values
+ # are true and false), you will want to use
+ # <tt>validates_inclusion_of :field_name, :in => [true, false]</tt>.
+ #
+ # This is due to the way Object#blank? handles boolean values:
+ # <tt>false.blank? # => true</tt>.
+ #
+ # This validator defers to the ActiveModel validation for presence, adding the
+ # check to see that an associated object is not marked for destruction. This
+ # prevents the parent object from validating successfully and saving, which then
+ # deletes the associated object, thus putting the parent object into an invalid
+ # state.
+ #
+ # Configuration options:
+ # * <tt>:message</tt> - A custom error message (default is: "can't be blank").
+ # * <tt>:on</tt> - Specifies when this validation is active. Runs in all
+ # validation contexts by default (+nil+), other options are <tt>:create</tt>
+ # and <tt>:update</tt>.
+ # * <tt>:if</tt> - Specifies a method, proc or string to call to determine if
+ # the validation should occur (e.g. <tt>:if => :allow_validation</tt>, or
+ # <tt>:if => Proc.new { |user| user.signup_step > 2 }</tt>). The method, proc
+ # or string should return or evaluate to a true or false value.
+ # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine
+ # if the validation should not occur (e.g. <tt>:unless => :skip_validation</tt>,
+ # or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The method,
+ # proc or string should return or evaluate to a true or false value.
+ # * <tt>:strict</tt> - Specifies whether validation should be strict.
+ # See <tt>ActiveModel::Validation#validates!</tt> for more information.
+ #
+ def validates_presence_of(*attr_names)
+ validates_with PresenceValidator, _merge_attributes(attr_names)
+ end
+ end
+ end
+end
View
44 activerecord/test/cases/validations/presence_validation_test.rb
@@ -0,0 +1,44 @@
+# encoding: utf-8
+require "cases/helper"
+require 'models/man'
+require 'models/face'
+require 'models/interest'
+
+class PresenceValidationTest < ActiveRecord::TestCase
+ class Boy < Man; end
+
+ repair_validations(Boy)
+
+ def test_validates_presence_of_non_association
+ Boy.validates_presence_of(:name)
+ b = Boy.new
+ assert b.invalid?
+
+ b.name = "Alex"
+ assert b.valid?
+ end
+
+ def test_validates_presence_of_has_one_marked_for_destruction
+ Boy.validates_presence_of(:face)
+ b = Boy.new
+ f = Face.new
+ b.face = f
+ assert b.valid?
+
+ f.mark_for_destruction
+ assert b.invalid?
+ end
+
+ def test_validates_presence_of_has_many_marked_for_destruction
+ Boy.validates_presence_of(:interests)
+ b = Boy.new
+ b.interests << [i1 = Interest.new, i2 = Interest.new]
+ assert b.valid?
+
+ i1.mark_for_destruction
+ assert b.valid?
+
+ i2.mark_for_destruction
+ assert b.invalid?
+ end
+end
View
2  guides/source/active_record_validations_callbacks.textile
@@ -373,6 +373,8 @@ class LineItem < ActiveRecord::Base
end
</ruby>
+If you validate the presence of an object associated via a +has_one+ or +has_many+ relationship, it will check that the object is neither +blank?+ nor +marked_for_destruction?+.
+
Since +false.blank?+ is true, if you want to validate the presence of a boolean field you should use <tt>validates :field_name, :inclusion => { :in => [true, false] }</tt>.
The default error message is "_can't be empty_".
Something went wrong with that request. Please try again.