Rails Ruby on Rails

Comments and changes to this ticket

• 

  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 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'.

• You flagged this item as spam.
  

  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 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 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 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 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 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 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 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 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. :)

• You flagged this item as spam.
  

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

  I also think warning should be nice.

• 

  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 March 9th, 2011 @ 12:14 PM

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

• You flagged this item as spam.
  

  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 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
  

• You flagged this item as spam.
  

  Joey March 14th, 2011 @ 05:57 AM

  that works..thanks Andrew