Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7383 (closed enhancement: fixed)

Opened 2 years ago

Last modified 1 year ago

[PATCH] Add allow_blank (or similar) to validations

Reported by: nappin Assigned to: nzkoz
Priority: normal Milestone: 1.x
Component: ActiveRecord Version: edge
Severity: minor Keywords: validates_length_of, allow_nil, empty_string
Cc: jnoon, brett, demisone, carlivar, gvenk, tarmo, shane, kuahyeow

Description

I have the following line in my code 

validates_length_of :phone, :minimum =>10, :message => "must be at least 10 digits long", :allow_nil => true

Submitting a blank text field for phone does not pass validations. The problem seems to be that the text field is treated as an empty string "" instead of a nil value.

If I modify the validation to this, the validation is not run when submitting a blank text field. 

  validates_length_of :phone, :minimum =>10, :message => "must be at least 10 digits long", :if => Proc.new{|customer| customer.phone != ''}

Attachments

add_allow_blank_to_validates_each_patch.diff (6.0 kB) - added by jnoon on 02/27/07 07:58:37.
Attaching patch for suggested addition of :allow_blank in #validates_each. Added :allow_blank to DEFAULT_VALIDATION_OPTIONS. Added documentation for :allow_blank to relevant methods. refs #3375 and this ticket #7383.

Change History

(in reply to: ↑ description ) 01/31/07 00:45:36 changed by nzjames

Replying to nappin:

This issue resides with validates_each(..) in source:/trunk/activerecord/lib/active_record/validations.rb.
It affects all the methods that rely on this for the :allow_nil option.

+1 for changing functionality of :allow_nil to also accept empty strings ("") as this is probably more usable given the nature of form submission.

01/31/07 09:31:37 changed by dylans

:allow_nil should only allow nil, and it should probably stay this way... for backwards compatibilities sake, and, well, nil means nil.

:allow_blank however, would be a nice addition.

01/31/07 23:11:17 changed by nzjames

  • type changed from defect to enhancement.
  • severity changed from normal to minor.
  • summary changed from validates_length_of allow_nil option not working to Add allow_blank (or similar) to validations.

I agree.

This would better suit the every day use of the validation functions.

Ideally this could be achieved by adding an :allow_blank catch to validates_each(..).

02/27/07 06:55:22 changed by jnoon

  • cc set to jnoon.

02/27/07 06:56:59 changed by jnoon

  • summary changed from Add allow_blank (or similar) to validations to [PATCH] Add allow_blank (or similar) to validations.

02/27/07 07:58:37 changed by jnoon

  • attachment add_allow_blank_to_validates_each_patch.diff added.

Attaching patch for suggested addition of :allow_blank in #validates_each. Added :allow_blank to DEFAULT_VALIDATION_OPTIONS. Added documentation for :allow_blank to relevant methods. refs #3375 and this ticket #7383.

05/01/07 21:57:02 changed by brett

  • cc changed from jnoon to jnoon, brett.

05/10/07 21:30:24 changed by demisone

  • cc changed from jnoon, brett to jnoon, brett, demisone.

06/15/07 06:48:23 changed by carlivar

  • cc changed from jnoon, brett, demisone to jnoon, brett, demisone, carlivar.

Workaround in the meantime (in model): 

before_validation { |m|
  m.field = nil if m.field.blank?
}

06/15/07 07:11:45 changed by jnoon

possibly.. however nil is not "", which is the point of the patch. They mean entirely different things in the db, and assuming any empty string should be nil just doesnt sound good to me.

06/15/07 07:24:17 changed by carlivar

Valid point. So a warning: treating nil and blank the same like this should be examined on a case-by-case basis. This is my particular workaround to avoid monkey-patching while I wait for the patch to be committed.

I suppose you could store the original value in the before_validation and later restore it in after_validation. But then you might as well just make a custom validation method and avoid the built-ins. Ugh. I hope this gets committed soon :)

06/15/07 07:50:41 changed by jnoon

If it helps, the workaround I've been using is somewhat similiar to yours, but instead is not destructive and just part of each validation:

validates_length_of :something, :is => 5,

:if => Proc.new { |at| !at.something.blank? }

07/09/07 10:28:32 changed by nzkoz

  • owner changed from core to nzkoz.
  • status changed from new to assigned.

08/11/07 22:52:22 changed by gvenk

  • cc changed from jnoon, brett, demisone, carlivar to jnoon, brett, demisone, carlivar, gvenk.

08/11/07 23:55:39 changed by tarmo

  • cc changed from jnoon, brett, demisone, carlivar, gvenk to jnoon, brett, demisone, carlivar, gvenk, tarmo.

08/24/07 03:57:03 changed by shane

  • cc changed from jnoon, brett, demisone, carlivar, gvenk, tarmo to jnoon, brett, demisone, carlivar, gvenk, tarmo, shane.

Is this waiting for another reviewer? If so, consider it done. Since most validations are against strings anyway, this seems like a very logical enhancement.

09/04/07 22:56:52 changed by kuahyeow

  • cc changed from jnoon, brett, demisone, carlivar, gvenk, tarmo, shane to jnoon, brett, demisone, carlivar, gvenk, tarmo, shane, kuahyeow.

09/04/07 23:08:50 changed by jnoon

Any reason why this is still being held up? Or is it just lost among the many tickets?

09/05/07 05:37:57 changed by nzkoz

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [7407]) Add :allow_blank to validations. Like allow_nil, but for values which are +blank?+. [jnoon] Closes #7383