Order of multiple `conditions` qualifier in `have_many` - v5.3.0

[thedean7]

Not sure if I'm missing something...

Scope in model:

has_many :active_website_resources, -> {
  joins(:resource)
  .where(is_active: true)
  .where(resource: { status: :active }) },
  class_name: 'WebsiteResource'

This test passes:

it { should have_many(:active_website_resources)
  .conditions(resource: { status: :active }) #1 <==
  .conditions(is_active: true) #2 <==
  .class_name('WebsiteResource') }

But when conditions are reversed then it fails:

it { should have_many(:active_website_resources)
  .conditions(is_active: true) #2 <==
  .conditions(resource: { status: :active }) #1 <==
  .class_name('WebsiteResource') }

with message:

Expected Website to have a has_many association called active_website_resources (active_website_resources should have the following conditions: {:resource=>{:status=>:active}})

[matsales28]

Hello! I hope you're doing well. Thank you for raising this issue. After quickly reviewing the source code, I believe you can achieve the desired outcome by combining both where clauses within the conditions method, as shown in the example below.

it { should have_many(:active_website_resources)
  .conditions(resource: { status: :active }, is_active: true)
  .class_name('WebsiteResource') }

Please try using this code and see if it works as expected. Let me know if you need any further assistance!

[thedean7]

Hi!

I tried many combinations, inline and chained but the result is the same. The culprit is nested condition. Order of conditions matters but attribute names and values of nested condition do not.

Only is_active is tested, nested attribute is ignored. Notice attribute changes and order:

# passes
it { should have_many(:active_website_resources)
  .conditions(whatever: { whatever: :whatever }) # <==
  .conditions(is_active: true)
  .class_name('WebsiteResource') }

# fails
it { should have_many(:active_website_resources)
  .conditions(is_active: true)
  .conditions(whatever: { whatever: :whatever })
  .class_name('WebsiteResource') }

Inline conditions:

# passes
it { should have_many(:active_website_resources)
  .conditions(is_active: true, whatever: { whatever: :whatever })
  .class_name('WebsiteResource') }

# fails
it { should have_many(:active_website_resources)
  .conditions(is_active: false, whatever: { whatever: :whatever })
  .class_name('WebsiteResource') }

# passes
it { should have_many(:active_website_resources)
  .conditions(whatever: { whatever: :whatever }, is_active: true)
  .class_name('WebsiteResource') }

# fails
it { should have_many(:active_website_resources)
  .conditions(whatever: { whatever: :whatever }, is_active: false)
  .class_name('WebsiteResource') }

[matsales28]

Yes, I dived into the code now, and we are using the where_values_hash to build a hash of conditions and compare it with the given conditions for the matcher modifier.

https://github.com/thoughtbot/shoulda-matchers/blob/0394a10302a681986df43f1c8830d40ebc5bb507/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb?#L60-L63

Unfortunately, this method only returns conditions for the queried primary table, in this case, "website_resources", so any nested condition will be discarded. So, making this spec using shoulda is impossible for now 😭 . We could implement a new solution to build the hash using some of the methods Rails uses on the where_values_hash, but I'm not sure if that's a nice approach because they're internal and not even documented methods.

https://github.com/rails/rails/blob/254f1d8ded07dfd73462b40d6aaa8c19032259f6/activerecord/lib/active_record/relation.rb#L772-L774

@mcmire I'd love to hear your thoughts on this.

[thedean7]

Thank you, @matsales28. I appreciate the detailed explanation.