Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Don't enable validations when passing false hash values to ActiveModel.validates #6517

Merged
merged 1 commit into from

2 participants

@purcell

Passing a falsey option value for a validator currently causes that validator to
be enabled, just like "true":

ActiveModel.validates :foo, :presence => false

This is rather counterintuitive, and makes it inconvenient to wrap validates in
methods which may conditionally enable different validators.

As an example, one is currently forced to write:

  def has_slug(source_field, options={:unique => true})
    slugger = Proc.new { |r| r[:slug] = self.class.sluggify(r[source_field]) if r[:slug].blank? }
    before_validation slugger
    validations = { :presence => true, :slug => true }
    if options[:unique]
      validations[:uniqueness] = true
    end
    validates :slug, validations
  end

because the following reasonable-looking alternative fails to work as expected:

  def has_slug(source_field, options={:unique => true})
    slugger = Proc.new { |r| r[:slug] = self.class.sluggify(r[source_field]) if r[:slug].blank? }
    before_validation slugger
    validates :slug, :presence => true, :slug => true, :uniqueness => options[:unique]
  end

(This commit includes a CHANGELOG entry, a test, and all activemodel and activerecord tests pass as before.)

@purcell purcell Don't enable validations when passing false hash values to ActiveMode…
…l.validates

Passing a falsey option value for a validator currently causes that validator to
be enabled, just like "true":

    ActiveModel.validates :foo, :presence => false

This is rather counterintuitive, and makes it inconvenient to wrap `validates` in
methods which may conditionally enable different validators.

As an example, one is currently forced to write:

      def has_slug(source_field, options={:unique => true})
        slugger = Proc.new { |r| r[:slug] = self.class.sluggify(r[source_field]) if r[:slug].blank? }
        before_validation slugger
        validations = { :presence => true, :slug => true }
        if options[:unique]
          validations[:uniqueness] = true
        end
        validates :slug, validations
      end

because the following reasonable-looking alternative fails to work as expected:

      def has_slug(source_field, options={:unique => true})
        slugger = Proc.new { |r| r[:slug] = self.class.sluggify(r[source_field]) if r[:slug].blank? }
        before_validation slugger
        validates :slug, :presence => true, :slug => true, :uniqueness => options[:unique]
      end

(This commit includes a test, and all activemodel and activerecord tests pass as before.)
b3ccd7b
@josevalim josevalim merged commit f5e26bc into from
@purcell

Thanks José!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 28, 2012
  1. @purcell

    Don't enable validations when passing false hash values to ActiveMode…

    purcell authored
    …l.validates
    
    Passing a falsey option value for a validator currently causes that validator to
    be enabled, just like "true":
    
        ActiveModel.validates :foo, :presence => false
    
    This is rather counterintuitive, and makes it inconvenient to wrap `validates` in
    methods which may conditionally enable different validators.
    
    As an example, one is currently forced to write:
    
          def has_slug(source_field, options={:unique => true})
            slugger = Proc.new { |r| r[:slug] = self.class.sluggify(r[source_field]) if r[:slug].blank? }
            before_validation slugger
            validations = { :presence => true, :slug => true }
            if options[:unique]
              validations[:uniqueness] = true
            end
            validates :slug, validations
          end
    
    because the following reasonable-looking alternative fails to work as expected:
    
          def has_slug(source_field, options={:unique => true})
            slugger = Proc.new { |r| r[:slug] = self.class.sluggify(r[source_field]) if r[:slug].blank? }
            before_validation slugger
            validates :slug, :presence => true, :slug => true, :uniqueness => options[:unique]
          end
    
    (This commit includes a test, and all activemodel and activerecord tests pass as before.)
This page is out of date. Refresh to see the latest.
View
2  activemodel/CHANGELOG.md
@@ -1,5 +1,7 @@
## Rails 4.0.0 (unreleased) ##
+* Passing false hash values to `validates` will no longer enable the corresponding validators *Steve Purcell*
+
* `ConfirmationValidator` error messages will attach to `:#{attribute}_confirmation` instead of `attribute` *Brian Cardarella*
* Added ActiveModel::Model, a mixin to make Ruby objects work with AP out of box *Guillermo Iguaran*
View
1  activemodel/lib/active_model/validations/validates.rb
@@ -88,6 +88,7 @@ def validates(*attributes)
defaults.merge!(:attributes => attributes)
validations.each do |key, options|
+ next unless options
key = "#{key.to_s.camelize}Validator"
begin
View
5 activemodel/test/cases/validations_test.rb
@@ -330,6 +330,11 @@ def test_validates_with_bang
end
end
+ def test_validates_with_false_hash_value
+ Topic.validates :title, :presence => false
+ assert Topic.new.valid?
+ end
+
def test_strict_validation_error_message
Topic.validates :title, :strict => true, :presence => true
Something went wrong with that request. Please try again.