Fix validate_length_of when used in combination with numericality validation

[mcmire]

It's very common to use a combination of the numericality and length validations to validate an attribute such as a telephone number or postal code (which are usually stored as string columns).

Unfortunately, the validate_length_of matcher uses the string "x" to test the length validation. When combined with the numericality validation, the matcher can never pass. Example:

class User < ApplicationRecord
  # (Assume the users table has a phone_number column)
  validates :phone_number, numericality: true, length: { is: 10 }
end

describe User do
  it { should validate_numericality_of(:phone_number) }
  it { should validate_length_of(:phone_number).is_equal_to(10) }
end

In this case, the numericality matcher will work, but the length matcher will fail with something like:

User did not properly validate that the length of :phone_number is 10.
  After setting :phone_number to ‹"xxxxxxxxx"›, the matcher expected the
  User to be valid, but it was invalid instead, producing these
  validation errors:

  * phone_number: ["is not a number"]

This issue was raised in years past but has never really been addressed, it seems like.

I wonder if the way to solve this would be to add a qualifier to validate_length_of:

it { should validate_length_of(:phone_number).is_equal_to(10).as_a_number }

[yogodoshi]

I've created an app to try to replicate this issue but wasnt able to. The specs passed successfully:

schema.db

ActiveRecord::Schema.define(version: 20161027030409) do
  create_table "users", force: :cascade do |t|
    t.string   "phone"
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
  end
end

user.rb

class User < ActiveRecord::Base
  validates :phone, numericality: true, length: { is: 10 }
end

user_spec.rb

RSpec.describe User do
  it { should validate_numericality_of(:phone) }
  it { should validate_length_of(:phone).is_equal_to(10) }
end

I tried replicating it after I saw this spec that seems to be testing exactly what you said.

Is it already fixed or did I misunderstood the issue? 😕

[jonnyjava]

I have a similar problem: the matcher expects the numericality error message but obviously rails gives the constraint one.
My case is this:

class Remittance < ActiveRecord::Base
  validates :amount, numericality: { greater_than: 0 }
end

it { expect(remittance).to validate_numericality_of(:amount).is_greater_than(0) }

The schema.rb has the following

create_table "remittances", force: :cascade do |t|
  t.decimal  "amount", precision: 8, scale: 2
end

And the failure I receive is this

  1) Remittance should validate that :amount looks like a number greater than 0
     Failure/Error: 
      it { expect(remittance).to validate_numericality_of(:amount).is_greater_than(0) }

       Remittance did not properly validate that :amount looks like a number  
       greater than 0.
         After setting :amount to ‹"abcd"› -- which was read back as
         ‹#<BigDecimal:5c4dee8,'0.0',9(18)>› -- the matcher expected the
         Remittance to be invalid and to produce the validation error "no es un
         número" on :amount. The record was indeed invalid, but it produced
         these validation errors instead:

         * amount: ["mayor que 0"]

It seems that the matcher doesn't know that if I do 'a'.to_i it can never give an error as 'is not a number' because is converted to 0. In fact if I remove the constraint and its validation I get

Remittance did not properly validate that :amount looks like a number.
         After setting :amount to ‹"abcd"› -- which was read back as
         ‹#<BigDecimal:63174b8,'0.0',9(18)>› -- the matcher expected the
         Remittance to be invalid, but it was valid instead.

I'm using shoulda-matchers-3.1.1, rspec-3.4.0 and rails-4.2.5
It happens even in an app created from scratch. Is it my fault because I don't have to test my decimal validations in this way or is an intended behaviour? Should I write my test differently?

[mcmire]

@jonnyjava This is a bug with the numericality matcher. Because your column is a decimal column, it will convert all given values to decimals ("abcd" becomes 0.0). Thus when the matcher reads your attribute, it's already a number, so it's confused.

Do you mind making a new issue for this so I can track it better?

[jonnyjava]

Here it is #976

[kheppenstall]

Just wanted to note that I had a similar problem and replicated this bug.

Validation:

validates :unit_price_in_cents, numericality: true, length: { minimum: 2 }

Tests:

it { should validate_numericality_of(:unit_price_in_cents) }
it { should validate_length_of(:unit_price_in_cents).is_at_least(2) }

Error Message:

[mcmire]

@kheppenstall That's close, but the behavior you've run into isn't a bug. The validate_length_of matcher doesn't support integer columns, only string columns.

[cesc1989]

@mcmire you're right. Although the docs for length validation don't leave that clear(I wasn't aware of this), the ones for numericality does let us infer something:

This helper validates that your attributes have only numeric values

Emphasis mine.

Thanks for pointing it out 👍

[mcmire]

Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!