Shoulda-matchers: undefined method `type' for nil:NilClass when using validate_uniqueness_of(...).scoped_to(AnArray)

[gmmcal]

When using a validation like

validates :a_field, uniqueness: { scope: [:an_association, :another_association]

and running the test as

it { should validate_uniqueness_of(:a_field).scoped_to([:an_association, :another_association]) }

produces an error

Failure/Error: it { should validate_uniqueness_of(:a_field).scoped_to([:an_association, :another_association]) }
NoMethodError:
  undefined method `type' for nil:NilClass

  # shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:473:in `dummy_scalar_value_for'
  # shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:468:in `dummy_value_for'
  # shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:441:in `block in validate_after_scope_change?'
  # shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:436:in `each'
  # shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:436:in `all?'
  # shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:436:in `validate_after_scope_change?'
  # shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:266:in `matches?'

[mcmire]

The uniqueness matcher doesn't support using associations as scopes. I'm not really sure this is what you want to do, anyway -- don't you want to use id columns instead?

[gmmcal]

Hi @mcmire,

this validation is when a field has to be unique, but not related to the model itself, but the other models

i.e.: User has many Profiles and Profile has an attribute name. Name has to be unique for a user, but another user can have the same name.

In this particular error, the uniqueness of the scope is related to two models, not only one, as my example says.

[mcmire]

Okay, I didn't know the uniqueness validation could be used this way, so this is definitely a bug.

[This is related to (but not quite the same as) #535.]

[coreyleveen]

@mcmire I am receiving this same error with the following

validates_uniqueness_of :a_field, scope: :association

with test:

subject { FactoryGirl.build(:record) }
before { FactoryGirl.create(:record) }
it { is_expected.to validate_uniqueness_of(:a_field).scoped_to(:association) }

Strangely, this only seems to be occurring for me when scoping to a single association, not an Array

[oniofchaos]

@mntj I'm having the same error you are with a single association.

[KeirSurvival]

+1

Just run into this after upgrading to v3.

[mcmire]

Ah, I didn't realize this was a regression. I'll bump up the priority on this.

[mcmire]

I just reproduced this and this doesn't appear to be a regression -- I encountered the same issue on 2.8.0.

I'm working on a fix for this, but I'm going to hold off on including it in the next release.

[maxcodes]

Any updates on this?

[rlugge]

I'm running into this same bug with a validation intended to prevent accidentally creating two identical relationships. Wound up just checking the IDs rather than the related object to work around.

[brightchimp]

Same here... we had worked around this in 2.x by using association_id:

it { is_expected.to validate_uniqueness_of(:a_field).scoped_to(:association_id).case_insensitive}

However, moving to 3.1.1 just now has removed that:

YourModel did not properly validate that :a_field is
case-insensitively unique within the scope of :association_id.
Expected the validation to be scoped to :association_id, but it was
scoped to :association instead.

[mneumark]

+1 Having the exact same issue as the others. I had a Active Record 5 related issue so I thought this might be related to using Active Record 5, but it seems that this is not the case.

Any chance of a release soon? Been about a half a year since the last update....

[michaeldever]

Having the same issue here.

[oddlyfunctional]

+1

[Sheval]

+1

[mikemerritt]

+1

[dsgn1graphics]

+1

[mikgry]

+1

[elisoncampos]

+1

[a118n]

+1

[maxcodes]

How can I help to get this resolved? There seems to be enough interest in this.

[mcmire]

@maxcodes If you can fork this repo, open up spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb, and write a failing test, then you'll at least be able to reproduce this behavior and isolate the problem. This should also lead you to why this error is happening in the first place, and what a potential fix for this could be.

[jonathanpa]

+1

[norbertmaleckii]

+1

[vonchristian]

Just use the association_id
validates_uniqueness_of :a_field, scope: :association_id

[carlows]

+1

[esbanarango]

+1

[Drenmi]

+1

[Wei-LiangChew]

+1

[Ana06]

+1

[ghost]

i think the problem comes from these functions:

        def dummy_value_for(scope)
          column = column_for(scope)

          if column.respond_to?(:array) && column.array
            [ dummy_scalar_value_for(column) ]
          else
            dummy_scalar_value_for(column)
          end
        end

        def dummy_scalar_value_for(column)
          Shoulda::Matchers::Util.dummy_value_for(column.type)
        end

( /lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb)

the column in dummy type needs to checked for a reference, if so the column needs to become 'column' + '_id'

just an idea

[ghost]

if someone needs a temporary workaround for that:

• using rspec, factory_bot (feel free to use something different)
• parent is the referenced model
• referenced model and name have to be unique

require 'rails_helper'

RSpec.describe SomeRecord, type: :model do
  ...
  describe 'extended validation' do
    let!(:some_record_parent) { create(:some_record_parent) }
    ... 
    context 'some_record already exists' do
      let(:some_record) {
        create(:some_record, some_record_parent: some_record_parent)
      }

      it 'fails when adding some_record twice' do
        expect { create(:some_record, some_record_parent: some_record_parent, name: some_record.name) }
          .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Name has already been taken')
      end
    end
  end
end

[edudepetris]

+1

[epbarger]

Just experienced this, bit of a bummer 👍

[mcmire]

Unfortunately, there's so much stuff that we are including in the next release that we've decided to hold off on fixing this for now. I'll keep this open for now but we'll have to swing back around and include a fix for this in a future release.

[glm4]

+1

[VSPPedro]

+1

[kimmy]

+1

[Jelkster]

+1

[bbugh]

I am seeing this even without scoped_to on any association, just a basic uniqueness validation:

# model
validates :user, uniqueness: true

# test
it { should validate_uniqueness_of(:user) }

Failure/Error: it { should validate_uniqueness_of(:user) }

NoMethodError:
  undefined method `type' for nil:NilClass
# /Users/bbugh/.rvm/gems/ruby-2.6.2/gems/shoulda-matchers-4.0.1/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:809:in `dummy_scalar_value_for'
# /Users/bbugh/.rvm/gems/ruby-2.6.2/gems/shoulda-matchers-4.0.1/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:804:in `dummy_value_for'
# /Users/bbugh/.rvm/gems/ruby-2.6.2/gems/shoulda-matchers-4.0.1/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:567:in `arbitrary_non_blank_value'
# /Users/bbugh/.rvm/gems/ruby-2.6.2/gems/shoulda-matchers-4.0.1/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:684:in `matches_uniqueness_without_scopes?'
# /Users/bbugh/.rvm/gems/ruby-2.6.2/gems/shoulda-matchers-4.0.1/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:330:in `matches?'
# ./spec/models/organization_user_spec.rb:14:in `block (3 levels) in <top (required)>'

[mcmire]

@bbugh I think you might be right about that, I have definitely noticed some oddities in the past. I will put this in the queue for the next version.

[xBartu]

I have the same issue, any update?

[kleytonmr]

+1

[the-codetrane]

I was able to get this to work when using attributes on the table itself:

# app/models/saved_view.rb
  validates :name, presence: true, uniqueness: { scope: :user_id }

# spec/models.saved_view.rb
    it { is_expected.to validate_uniqueness_of(:name).scoped_to(:user_id) }

If you're willing to live with the attributes v. association object, this will pass

[gmmcal]

I'm not even sure if this issue still occurs. I've first opened this error because it failed to validate scoped to more than one association at the same time. I do vaguely remember that scoping to a single association worked at the moment in time. I'll try to create a demo app that reproduces this error, so we are clear in it. Based on comments, I think we potentially have 2 issues in a single place. I'll come back with demo repository whenever I have it.

[mcmire]

@gmmcal Sounds good, thank you!

[gmmcal]

I've created the simplest example app to show the progress of this issue. on a certain view, this issue seems fixed to me. But, personally, I don't like the limitation on the solution.

I'd like to me able to use shoulda these ways
https://github.com/gmmcal/shoulda-uniqueness-issue-814/blob/master/spec/models/profile_spec.rb#L6
https://github.com/gmmcal/shoulda-uniqueness-issue-814/blob/master/spec/models/company_spec.rb#L6

But the lines above does work and this test suite passes all tests. Model itself has to be written in a given way, but, for rails, either way is a valid validation process
https://github.com/gmmcal/shoulda-uniqueness-issue-814/blob/master/app/models/profile.rb
https://github.com/gmmcal/shoulda-uniqueness-issue-814/blob/master/app/models/company.rb

@mcmire if you don't think that validates :name, uniqueness: { scope: :user } is a valid input for this gem, I'm 100% ok on closing this issue. If so, I think that README should be improved to point this "limitation".

[mcmire]

I've been noodling on this the past few days. The reason why we haven't tackled this problem yet is it's a difficult problem, and it's difficult because because associations are way more variable than non-associations. The most complicated part of the uniqueness matcher is that it needs to generate unique values for the attributes that are involved in the validation (or at least the ones you care about), so that it can test the validation thoroughly. For instance, if you have a Post model and it validates uniqueness of slug scoped to blog_id, it tests these scenarios:

• What happens if you have two posts with the same slug and blog_id — does the validation fail?
• What happens if you have two posts with a different slug but the same blog_id — does the validation pass?
• What happens if you have two posts with the same slug but a different blog_id — does the validation pass?

To do this, the uniqueness matcher needs to know how to generate a record with a "different" (i.e. unique) value for one or more attributes. This is relatively easy to do for non-associations: if the attribute is a string column, call .next on the string; if it's a number column, add 1; if it's a time column, add 1 second; etc. But associations are totally separate ActiveRecord objects, and while the simplest way to create a unique version of another record is to .dup it and save it to get an object with a different id, that's probably not going to work, especially if _that_ model has a uniqueness validation on it. Maybe the matcher could do something like look inside that model, see which attributes are unique, and try to generate unique values for _those_ attributes — but that would add complexity I don't think we need. It's probably less code and a better experience in the long run to have you as the developer provide unique values for association attributes through another qualifier. I'm thinking that this could look something like this:

# Presumably this would already have a blog
subject { FactoryGirl.build(:post) }

it do
  should validate_uniqueness_of(:slug)
    .scoped_to(:blog)
    .using_unique_value_for(:blog) { FactoryGirl.create(:blog) }
end

and if you had multiple association scopes:

# Presumably this would already have a blog
subject { FactoryGirl.build(:company) }

it do
  should validate_uniqueness_of(:name)
    .scoped_to(:profile, :user)
    .using_unique_value_for(:profile) { FactoryGirl.create(:profile) }
    .using_unique_value_for(:user) { FactoryGirl.create(:user) }
end

I don't have any code written to support this yet as I'm still playing around with this. Would this seem like a good solution, though?

[gmmcal]

TBH, not sure how I feel with this approach compared to doing this myself without using this gem. A test itself would not be that much different than your dummy code, and I think it might even be easier to understand in the future. Taking your example:

let(:profile) { FactoryGirl.create(:profile) }
let(:user) { FactoryGirl.create(:user) }
let(:blog) { FactoryGirl.build(:blog, user: user, profile: profile) }

before do
  FactoryGirl.create(:blog, user: user, profile: profile) # make sure one exists
end

it do
  expect(blog.valid?).to be_false # add more extra checks on scope matches
end

[mcmire]

@gmmcal Coming back to this again. Based on the feedback, it sounds like this issue is not really solvable, or at least that the solution would not be that useful. Do we need to add some documentation around this? Or maybe the gem should raise an explicit error if an attempt is made to use an association for the attribute or scope with an example of the test that should be written instead?

[gmmcal]

@mcmire I think a change on README and/or code documentation explicitly stating this scenario is not supported and point to this issue would be enough. Making the code raise an exception for this could be easier to understand, for sure, but I'm not sure how many people face this on a daily basis and if it is worth the effort, unless you could build this within a few minutes (I have not checked that tbh). Your call on that.

I can open a PR with text changes for this case.

[mcmire]

@gmmcal Okay, yeah, I'm not sure adding an exception would be a 5-minute job so updating the documentation makes sense to me.

[gmmcal]

Will create a PR then