Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• 

  Rohit Arondekar August 3rd, 2010 @ 12:01 PM

  • Milestone cleared.
  • State changed from “new” to “open”
  • Assigned user set to “Santiago Pastorino”
  • Importance changed from “” to “Low”

  Confirmed.

  Attached is a failing test.

• 

  Rohit Arondekar August 3rd, 2010 @ 12:02 PM

  Sorry, forgot to attach the patch in previous comment.

  • length_of_test.diff 1.5 KB
• 

  Rohit Arondekar August 3rd, 2010 @ 01:42 PM

  Ok I have some weird findings to report.

  First, the test that I provided in the patch above is as follows:

    def test_validates_length_of_custom_errors_for_both_too_short_and_too_long
      Topic.validates_length_of :title, :minimum => 3, :maximum => 5, :too_short => 'too short', :too_long => 'too long'
  
      t = Topic.new(:title => 'a')
      assert t.invalid?
      assert t.errors[:title].any?
      assert_equal ['too short'], t.errors['title']
  
      t = Topic.new(:title => 'aaaaaa')
      assert t.invalid?
      assert t.errors[:title].any?
      assert_equal ['too long'], t.errors['title']
    end
  

  The part of the code in question in length.rb inside AMo is as follows:

        def validate_each(record, attribute, value)
          value = options[:tokenizer].call(value) if value.kind_of?(String)
  
          CHECKS.each do |key, validity_check|
            next unless check_value = options[key]
            default_message = options[MESSAGES[key]]
            options[:message] ||= default_message if default_message
  
            valid_value = if key == :maximum
              value.nil? || value.size.send(validity_check, check_value)
            else
              value && value.size.send(validity_check, check_value)
            end
  
            next if valid_value
  
            record.errors.add(attribute, MESSAGES[key],
                              options.except(*RESERVED_OPTIONS).merge!(:count => check_value))
          end
        end
  

  Note that default_message is set too early, it should be set only if an invalid value is found, i.e after next if valid_value as follows:

        def validate_each(record, attribute, value)
          value = options[:tokenizer].call(value) if value.kind_of?(String)
  
          CHECKS.each do |key, validity_check|
            next unless check_value = options[key]
  
            valid_value = if key == :maximum
              value.nil? || value.size.send(validity_check, check_value)
            else
              value && value.size.send(validity_check, check_value)
            end
  
            next if valid_value
  
  puts options[:message]
  
            default_message = options[MESSAGES[key]]
            options[:message] ||= default_message if default_message
  
  puts "#{value} is #{valid_value} #{key} and #{options[:message]} but message should be #{options[MESSAGES[key]]}"
  
            record.errors.add(attribute, MESSAGES[key],
                              options.except(*RESERVED_OPTIONS).merge!(:count => check_value))
          end
        end
  

  Note that I've added two puts statements. Now if I run my test on this changed code, the test still fails with the original problem and more interestingly I get the following output (truncated):

  ["a"] is false minimum and too short but message should be too short
  too short
  ["a", "a", "a", "a", "a", "a"] is false maximum and too short but message should be too long
  

  Note that in the second run, for testing :too_long message, options[:message] is set to too short. (see second line in above output) How can that be?

  Also reversing the order of my test, that is testing for :too_long first and then :too_short, causes the validation message set to 'too long' instead of 'too short'.

  Running the two sets of asserts in isolation works, if this the desired effect I can provide a patch with the above code fix. However if my test should pass then the problem could be deeper.

• 

  José Valim August 3rd, 2010 @ 01:51 PM

  • Assigned user changed from “Santiago Pastorino” to “José Valim”
  • Importance changed from “Low” to “High”
• 

  Repository August 3rd, 2010 @ 02:36 PM

  (from [621246f997887eccf61c1737c76b1eefc9217263]) Failing test for validates_length_of, when both too_short and too_long messages are set [#5283 state:open]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/621246f997887eccf61c1737c76b1e...

• 

  Repository August 3rd, 2010 @ 02:36 PM

  • State changed from “open” to “resolved”

  (from [f23bc8444b1f1193b2f9135474545436be97b195]) validates_length_of should not change the options hash in place. [#5283 state:resolved] http://github.com/rails/rails/commit/f23bc8444b1f1193b2f91354745454...

• 

  Repository August 3rd, 2010 @ 02:37 PM

  (from [79583ca9b14d7c16e0118de27cd95a0fd6647791]) validates_length_of should not change the options hash in place. [#5283 state:resolved] http://github.com/rails/rails/commit/79583ca9b14d7c16e0118de27cd95a...

• 

  Repository August 3rd, 2010 @ 02:37 PM

  • State changed from “resolved” to “open”

  (from [257e9c4ec46c5045785081e3989bed9bc8d9b37a]) Failing test for validates_length_of, when both too_short and too_long messages are set [#5283 state:open]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/257e9c4ec46c5045785081e3989bed...

• 

  José Valim August 3rd, 2010 @ 02:39 PM

  • State changed from “open” to “resolved”

• You flagged this item as spam.
  

  Jakub Suder August 3rd, 2010 @ 06:48 PM

  Wow, that was fast. Thanks José & Rohit :)

• 

  Ryan Bigg October 9th, 2010 @ 09:47 PM

  • Tag cleared.

  Automatic cleanup of spam.

• 

  Jeremy Kemper October 15th, 2010 @ 11:02 PM

  • Milestone set to “3.0.2”

  [bulk edit]

• You flagged this item as spam.
  

  bingbing March 26th, 2011 @ 02:09 AM

  louis vuitton watches