validate_length_of does not work when used with associations

[LukasBarry]

I have been attempting to write a test on a validation for length, and I keep getting the same error. I created a small sample app, as simple as possible, to show the error: Sample Project

Basically, I have a has_many association that must have at least 1, but when I run the test
should validate_length_of(:books).is_at_least(1) I get the following error:

NoMethodError:
       undefined method `each' for "":String

My sample project current models:

class Author < ApplicationRecord
  has_many :books

  validates :books, length: { minimum: 1 }
end

class Book < ApplicationRecord
  belongs_to :author, dependent: :destroy
end

And my current test file:

require 'rails_helper'

describe Author do
  it 'validates length of books' do
    should validate_length_of(:books).is_at_least(1)
  end
end

I believe this is because Shoulda is using a string to test length, but I am attempting to test the length of a collection and not a string. However, while attempting to trace it back through the gem, I found nothing that I could use to fix my issue.

Any help would be greatly appreciated. Thanks!

[mcmire]

Hey @LukasBarry. When using is_at_least, the matcher will build a string with length of <length> - 1 (which in this case is 0), set the attribute (books) to that string, and assert that this causes a failure. You can see that happening here: https://github.com/thoughtbot/shoulda-matchers/blob/master/lib/shoulda/matchers/active_model/validate_length_of_matcher.rb#L340

Regardless of how you are using validate_length_of, however, the matcher does make the assumption that the column you're testing is a string column; it's not designed to work with anything else. I've never used the validation with associations before, but after reading the Rails source code, it does seem like it's a valid use case, so this is new to me. We'll need a PR to fix this. Can you submit one?

[LukasBarry]

Absolutely, I will get one together and submit it. Thanks!

[ysyyork]

Hi, I just came across the same issue and was planning to make changes. But have you already started the change? And is there any update?

[mcmire]

Hey @ysyyork! As you can see here there is a PR here that was submitted. While it's a good start it does need some tests. Unfortunately this issue is low on our priority list at the moment but if you would like to pick it up and supply the tests (submitting a new PR of course) then that would be much appreciated!

[ysyyork]

@mcmire Sure, I will give it a look if I have the time.

[prashantjois]

Hi all, looks like this issue never got resolved. I have a fresh PR open: #1124

[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!

[mcmire]

Reopening since this is dependent on a PR.