This project is archived and is in readonly mode.

#469 ✓stale
Jérôme

validates_acceptance_of deep failure ?

Reported by Jérôme | June 22nd, 2008 @ 06:27 PM

Hi

It looks like validates_acceptance_of doesn't work wether it uses a db column or just a regular instance attribute.

$ rails test ; cd test
$ ./script/generate model User name:string terms_of_service:boolean
$ ./script/generate model Person name:string
$ rake db:migrate
class User < ActiveRecord::Base
  validates_acceptance_of :terms_of_service
end

class Person < ActiveRecord::Base
  attr_accessor :terms_of_service
  validates_acceptance_of :terms_of_service
end

class UserTest < ActiveSupport::TestCase
  def test_new_user_be_valid_since_boolean_is_nil
    assert_valid User.new(:name => "Mister New")
  end

  def test_nice_user_should_be_valid
    assert_valid User.new(:name => "Mister Yes", :terms_of_service => true)
  end

  def test_bad_user_should_not_be_valid
    user = User.create(:name => "Mister No", :terms_of_service => false)
    assert_equal user.errors.on(:terms_of_service), "must be accepted"
  end
end

class PersonTest < ActiveSupport::TestCase
  def test_new_person_be_valid_since_boolean_is_nil
    assert_valid Person.new(:name => "Mister New")
  end

  def test_nice_person_should_be_valid
    assert_valid Person.new(:name => "Mister Yes", :terms_of_service => true)
  end

  def test_bad_person_should_not_be_valid
    person = Person.create(:name => "Mister No", :terms_of_service => false)
    assert_equal person.errors.on(:terms_of_service), "must be accepted"
  end
end
$ rake
(in /Users/jerome/tmp/test)
/opt/local/bin/ruby -Ilib:test "/opt/local/lib/ruby/gems/1.8/gems/rake-0.8.1/lib/rake/rake_test_loader.rb" "test/unit/person_test.rb" "test/unit/user_test.rb" 
Loaded suite /opt/local/lib/ruby/gems/1.8/gems/rake-0.8.1/lib/rake/rake_test_loader
Started
..F..F
Finished in 0.327487 seconds.

  1) Failure:
test_nice_person_should_be_valid(PersonTest)
    [./test/unit/person_test.rb:9:in `test_nice_person_should_be_valid'
     /opt/local/lib/ruby/gems/1.8/gems/activesupport-2.1.0/lib/active_support/testing/setup_and_teardown.rb:33:in `__send__'
     /opt/local/lib/ruby/gems/1.8/gems/activesupport-2.1.0/lib/active_support/testing/setup_and_teardown.rb:33:in `run']:
Terms of service must be accepted.
<false> is not true.

  2) Failure:
test_nice_user_should_be_valid(UserTest)
    [./test/unit/user_test.rb:9:in `test_nice_user_should_be_valid'
     /opt/local/lib/ruby/gems/1.8/gems/activesupport-2.1.0/lib/active_support/testing/setup_and_teardown.rb:33:in `__send__'
     /opt/local/lib/ruby/gems/1.8/gems/activesupport-2.1.0/lib/active_support/testing/setup_and_teardown.rb:33:in `run']:
Terms of service must be accepted.
<false> is not true.

6 tests, 6 assertions, 2 failures, 0 errors
/opt/local/bin/ruby -Ilib:test "/opt/local/lib/ruby/gems/1.8/gems/rake-0.8.1/lib/rake/rake_test_loader.rb"  
/opt/local/bin/ruby -Ilib:test "/opt/local/lib/ruby/gems/1.8/gems/rake-0.8.1/lib/rake/rake_test_loader.rb"  

Comments and changes to this ticket

  • MatthewRudy

    MatthewRudy June 22nd, 2008 @ 06:34 PM

    I think this is because validates_acceptance_of has a default of :allow_nil => true,

          def validates_acceptance_of(*attr_names)
            configuration = { :message => ActiveRecord::Errors.default_error_messages[:accepted], :on => :save, :allow_nil => true, :accept => "1" }
            configuration.update(attr_names.extract_options!)
    
            db_cols = begin
              column_names
            rescue ActiveRecord::StatementInvalid
              []
            end
            names = attr_names.reject { |name| db_cols.include?(name.to_s) }
            attr_accessor(*names)
    
            validates_each(attr_names,configuration) do |record, attr_name, value|
              record.errors.add(attr_name, configuration[:message]) unless value == configuration[:accept]
            end
          end
    

    but it does seem to default to only accept "1" as a valid value...

    should probably be "1" or true?

          def validates_acceptance_of(*attr_names)
            configuration = { :message => ActiveRecord::Errors.default_error_messages[:accepted], :on => :save, :allow_nil => true, :accept => ["1", true] }
            configuration.update(attr_names.extract_options!)
    
            db_cols = begin
              column_names
            rescue ActiveRecord::StatementInvalid
              []
            end
            names = attr_names.reject { |name| db_cols.include?(name.to_s) }
            attr_accessor(*names)
    
            validates_each(attr_names,configuration) do |record, attr_name, value|
              record.errors.add(attr_name, configuration[:message]) unless safe_to_array(configuration[:accept]).include?(value)
            end
          end
    

    perhaps that works better?

  • Jérôme

    Jérôme June 22nd, 2008 @ 07:20 PM

    but the documentation says:

    :accept - Specifies value that is considered accepted. The default value is a string "1", which makes it easy to relate to an HTML checkbox. This should be set to true if you are validating a database column, since the attribute is typecast from "1" to true before validation.
    
  • MatthewRudy

    MatthewRudy June 22nd, 2008 @ 07:29 PM

    great,

    so the current functionality is fine, I guess.

  • Jérôme

    Jérôme June 22nd, 2008 @ 08:12 PM

    Errrr no, since the tests fail !

  • Jérôme

    Jérôme June 22nd, 2008 @ 08:39 PM

    Ok so I misunderstanded the doc. The typecasting explanation was unclear for me. Setting :accept => true is a better option since it validates "1", 1 and true, while the default :accept => "1" only validates "1"...

    What about changing this default value then ?

  • MatthewRudy

    MatthewRudy June 22nd, 2008 @ 09:44 PM

    well, that was my suggestion,

    and what my code did was...

    default to "1" or true

    But, need to get a core team guy on here to decide what "should" be the default.

  • Lawrence Pit

    Lawrence Pit June 23rd, 2008 @ 12:40 AM

    I brought this up last month on the core list. See

    http://groups.google.com.au/grou...

    It does work, though not as advertised. You do have to add a hidden checkbox with a default value.

  • josh

    josh September 30th, 2008 @ 05:50 PM

    • State changed from “new” to “stale”
    • Tag set to 2.1, activerecord, bug, validations

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>

People watching this ticket

Pages