wrong message used in validates_length_of

Sorry, forgot to attach the patch in previous comment.

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.

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

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

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

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

louis vuitton watches