This project is archived and is in readonly mode.

#6259 ✓resolved
bmihelac (at mihelac)

validate_presence_of does not work with :allow_nil => true

Reported by bmihelac (at mihelac) | January 7th, 2011 @ 08:16 AM

Documentation guide states that :allow_nil and :allow_blank can be used by all validation helpers:

http://edgeguides.rubyonrails.org/active_record_validations_callbac...

This is not the case for PresenceValidator:

class Article < ActiveRecord::Base
  validates_presence_of :subject, :allow_nil => true
end

ruby-1.9.2-p0 > Article.new.valid?
 => false

Comments and changes to this ticket

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer March 7th, 2011 @ 05:58 PM

    • State changed from “new” to “open”
    • Assigned user set to “Jeff Kreeftmeijer”
    • Importance changed from “” to “Low”

    I did a quick test and it seems you're right: https://gist.github.com/858816

    allow_blank doesn't do anything when validating presence, which seems sensible to me. This should be fixed in the docs and maybe even throw a warning to avoid confusion if anybody ever uses it. What do you think?

    allow_nil should just work if you ask me and the example above should return true. I'll look into this one. :)

  • Andrew White

    Andrew White March 7th, 2011 @ 08:10 PM

    The validates_presence_of method has historically ignored the difference between nil and zero lengths strings because any empty fields from a form will be a zero length string. Given this I don't think it'd be wise to change the semantics of its behaviour now.

    The validates_length_of combined with the :allow_nil option can be used to achieved the desired outcome:

      # Allow nil values but not zero length strings
      validates_length_of :title, :minimum => 1, :allow_nil => true
    
      # Allow zero length strings but not nils
      validates_length_of :body, :minimum => 0, :allow_nil => false
    

    You'd probably want to adjust the error messages to something other than the default 'is too short (minimum is {count} characters'.

  • bmihelac (at mihelac)

    bmihelac (at mihelac) March 8th, 2011 @ 08:20 AM

    Andrew, so your proposition is to update the documentation and describe that :allow_nil and :allow_blank in valid_presence_of would throw validation error for empty and nil string? Should use of it be deprecated also?

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer March 8th, 2011 @ 10:46 AM

    :allow_nil and allow_blank now both et ignored on presence validations. Andrew's right, if we want to do some kind of zero-length validation that allows nil for some reason, you should validate length.

    +1 for firing a warning when allow_nil or allow_blank. Any objections? Otherwise I'll add it tonight probably and I'll update the docs. :)

  • Andrew White

    Andrew White March 8th, 2011 @ 10:57 AM

    I'm not sure that it's worth the cpu cycles - validates_presence_of, allow_nil and allow_blank have been around for a while and this is the first bug report I can find that has brought the issue up. I think a document clarification is all that's needed.

  • Josh Kalderimis

    Josh Kalderimis March 8th, 2011 @ 11:14 AM

    • Tag changed from :allow_nil, documentation, presencevalidator, validate_presence_of to documentation, validate_presence_of

    +1 on firing a warning if using :allow_nil or :allow_blank with the presence validator + some documentation to to guides and presence validator about using length validator + allow_nil

  • Josh Kalderimis

    Josh Kalderimis March 8th, 2011 @ 11:28 AM

    Also, I think a warning would help new users as using the presence validator with allow_nil does seem logical to a certain degree.

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer March 8th, 2011 @ 11:46 AM

    While adding a note to the guides telling users not to use allow_nil or allow_blank, I found this:

    Using +:allow_nil+ with +validates_presence_of+ allows for +nil+, but any other +blank?+ value will still be rejected.
    

    That explicitly tells users that allow_nil should work, making me wonder if this is a bug after all. Could that be the case? Otherwise I'll add a note and remove that line. :)

  • Josh Kalderimis

    Josh Kalderimis March 8th, 2011 @ 11:49 AM

    hmmmmmmmm, this make it seem like a bug to me.

    Personally I would like the presence validator to respect allow_nil, otherwise a warning is the very very least it should be doing.

  • Josh Kalderimis

    Josh Kalderimis March 8th, 2011 @ 09:59 PM

    After talking to José and Santaigo, the agreed solution is to update the docs accordingly and note that :allow_blank and :allow_nil do not work with the presence validator and will be ignored.

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer March 9th, 2011 @ 10:21 AM

    I pushed a patch to docrails: https://github.com/lifo/docrails/commit/e05e9979075b37342b7c88e557d...

    What do you think? I'll mark this ticket as resolved when the doc patch is merged into rails and you're all okay with it. :)

  • bmihelac (at mihelac)

    bmihelac (at mihelac) March 9th, 2011 @ 11:03 AM

    I also think warning should be nice.

  • Andrew White

    Andrew White March 9th, 2011 @ 11:07 AM

    • State changed from “open” to “resolved”

    Jeff, in future add [#{num} state:resolved] to your docrails commit and the ticket state will be automatically updated when it's merged into master.

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer March 9th, 2011 @ 12:14 PM

    Will do, somehow I thought I wasn't supposed to do that for docrails patches. Thanks :)

  • Joey

    Joey March 13th, 2011 @ 09:49 AM

    With this change, how would I be able to allow a nil/blank value when using validates_presence_of to verify the existence of an associated record for a belongs_to association only when the id is present?

    class Student
    has_many :students_teachers
    end

    class Teacher
    has_many :students_teachers

    end

    class StudentsTeacher
    belongs_to :student
    belongs_to :teacher
    belongs_to :assistant_teacher, :class_name => 'Teacher'

    validates_associated :student, :teacher, :assistant_teacher
    validates_presence_of :student, :teacher
    validates_presence_of :assistant_teacher, :allow_nil => true #student might not always have an assistant teacher
    end

  • Andrew White

    Andrew White March 13th, 2011 @ 10:12 AM

    It's actually a non-change :)

    I'm not sure what you're to achieve - the validates_associated will ensure :assistant_teacher is valid if it's present. If you're trying to ensure that :assistant_teacher exists if :assistant_teacher_id is not nil then making validates_presence_of work with :allow_nil won't help as the association would still return nil. You can write a custom validation method to achieve what you need:

    class StudentTeacher
      validate :existence_of_associated_teacher, :if => :associated_teacher_id?
    
      def existence_of_associated_teacher
        associated_teacher.present?
      end
    end
    
  • Joey

    Joey March 14th, 2011 @ 05:57 AM

    that works..thanks Andrew

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Pages