Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

validates_acceptance_of deep failure ?

#469

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"  

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

State: stale
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. MatthewRudy
    MatthewRudy

    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?

    June 22nd, 2008 @ 06:34 PM

  2. Jérôme
    Jérôme

    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.
    

    June 22nd, 2008 @ 07:20 PM

  3. MatthewRudy
    MatthewRudy

    great,

    so the current functionality is fine, I guess.

    June 22nd, 2008 @ 07:29 PM

  4. Jérôme
    Jérôme

    Errrr no, since the tests fail !

    June 22nd, 2008 @ 08:12 PM

  5. Jérôme
    Jérôme

    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 ?

    June 22nd, 2008 @ 08:39 PM

  6. MatthewRudy
    MatthewRudy

    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.

    June 22nd, 2008 @ 09:44 PM

  7. Lawrence Pit
    Lawrence Pit

    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.

    June 23rd, 2008 @ 12:40 AM

  8. josh
    josh
    • State changed from new to stale
    • Tag set to 2.1, activerecord, bug, validations

    September 30th, 2008 @ 05:50 PM