Issue with validates uniqueness and STI/polymorphic models on rails 6?

[mjankowski]

This feels similar to this long-ago bug - #203 - which was fixed here - #592

Model of:

class Classification < ApplicationRecord
  belongs_to :classifiable, polymorphic: true
  belongs_to :topic

  validates :classifiable_id, :uniqueness => { :scope=> [:topic_id, :classifiable_type] }
end

Spec of:

require "rails_helper"

describe Classification do
  it { should belong_to(:classifiable) }
  it { should belong_to(:topic) }

  it "ensures uniqueness" do
    create(:classification)
    should validate_uniqueness_of(:classifiable_id).scoped_to([:topic_id, :classifiable_type])
  end
end

Relevant output is:

Failures:

  1) Classification ensures uniqueness
     Failure/Error: should validate_uniqueness_of(:classifiable_id).scoped_to([:topic_id, :classifiable_type])
     
     ActiveRecord::SubclassNotFound:
       Invalid single-table inheritance type: Show is not a subclass of Shoulda::Matchers::ActiveRecord::Uniqueness::TestModels::Producu
     # ./spec/models/classification_spec.rb:9:in `block (2 levels) in <top (required)>'

That ::Producu on the very end there in the error makes me suspect that we're calling next on a string with Product (or something?) somewhere in the codebase.

[diebas]

This is happening to me as well

[ngouy]

it's likely due to :

# shoulda-matchers-4.1.2/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb

        def next_scalar_value_for(scope, previous_value)
          column = column_for(scope)

         # scope == :polymorphic_type,
         # `previous_value` is an instance of the model we are testing,
         # some dumb values and a real existing `polymorphic_type` such as `MySTIType`
         # previous_value == MyModel(title: "dummy", polymorphic_type: MySTIType, ...)
         # which the #create method will replace with the incirminated value : 
         # Shoulda::Matchers::ActiveRecord::Uniqueness::Namespace::MySTITypf]

          if column.type == :uuid
          # ...
          elsif polymorphic_type_attribute?(scope, previous_value)
            Uniqueness::TestModels.create(previous_value).to_s
          # ...
          end
        end

It triggers a TestModelCreator.create, which triggers a model_name#next, which triggers "MySTIType".next => MySTITypf (or For exemple "Product".next for @mjankowski)

I don't know why. I just tracked it down to this. I have the exact same problem, my Form become a Forn

Edit Apparently, it is meant to be this way

[mcmire]

Yes, that behavior is intentional. The uniqueness matcher will:

• take the record you provide and ensure that it is saved to the database (this record is now the "existing" record)
• build a new instance of the record (without saving) by going through each of the attributes that you specify (attribute that you're testing + scopes you specify) and copying them from the existing record
• assert that this new record is invalid and fails the uniqueness validation
• go through each of the attributes that you specify, change one of the values in the new record and assert that the record is valid again (because now it's unique again)

It's this last step that's tricky. What do I mean by "changing" a value? I mean that the matcher will look at the existing value for the attribute, whatever it happens to be, and create another value based on it which is different. How this works differs based on the type of the value. For "simple" data types like strings, numbers, etc., this is easy: call .succ on it. For other values, this is not so easy. For polymorphic attributes, you have an _id and _type attribute. The _type attribute is usually a class. In that case, the matcher needs to set the _type attribute to another class, but one which the parent model will accept. How do you do that?? Well, as @ngouy pointed out, the matcher is very clever, and instead of creating a new class, it merely makes a new alias for the existing class. You can kind of see that happening here. For instance, if your _type attribute was "Product", then it would create a Producu class (scoped to a module within the gem) which is merely equivalent to Product. In other words, it basically does this:

Shoulda::Matchers::ActiveRecord::Uniqueness::TestModels::Producu = Product

@mjankowski @diebas Is there something specific about your models that would cause the error that you're seeing? Something you've done differently?

[mjankowski]

My issue was happening in a rails six upgrade of branch of Upcase, which is a public repo, so I can actually link you to the "fix" I came up with.

Prior to this commit - thoughtbot/upcase@9f0792b - I was seeing the Invalid single-table inheritance type: Show is not a subclass of Shoulda::Matchers::ActiveRecord::Uniqueness::TestModels::Producu error. After that commit, things pass as expected.

In this specific case, I'm not actually sure why the validation and spec were written the way they were to begin with ... so the change both made logical common sense to me as a better way to state the same thing, but it also fixed the shoulda matcher generated error.

[ngouy]

I think it's a rails "issue". Maybe in rails (ActiveRecord) 5, models weren't checking, or not the same way, what was put in the type column of an STI'ed model instance. I think the error pop outside of shoulda_matchers, exactly BECAUSE whatever Shoulda::Matchers::ActiveRecord::Uniqueness::TestModels::PutWhatYouWantHere is not a child of the STI parent.

Or maybe we can make the matcher inherits from the STI parent ?

[ngouy]

not sure what side effect it has, but removing the .dup from
lib/shoulda/matchers/active_record/uniqueness/model.rb#L31

          def symlink_to(parent)
            namespace.set(name, parent.dup)
          end

fix the issue (as the Shoulda::Matchers::ActiveRecord::Uniqueness::TestModels::PutWhatYouWantHere point to the true STI root class (and not a dup)

[mcmire]

@ngouy Hmm.... I wonder why I ended up doing that. Thanks for the tip. I'll try this out and see if it breaks anything.

[HeshamaMohamed]

Hey, thanks for your efforts!
I am facing this issue as well.
Is it going to be merged soon?