Account for composite primary key in foreign key

[natematykiewicz]

Fixes [#1392]

[natematykiewicz]

@vsppedro what do you think of this?

Things get a little weird in symbols vs strings. When options[:foreign_key] fails, the value you passed in is used as-is. If that succeeds and it looks up the actual foreign_key, the value is normalized to a string (though, I stopped it from turning [:a, :b] into a string of "[:a, :b]" and instead turned it into ["a", "b"]).

I think it solves the problem, though it looks like primary key has the same original issue (no error message in the options check). I also think some DRYing could happen here between primary and foreign key checks.

I'm also not really sure what level of support is expected for Composite Primary Keys. shoulda-matchers doesn't mention that it supports CPK, but it also did work until the most recent version.

I'm open to any and all feedback. One thing that's nice here is that it actually asserts that the configured foreign key's corresponding column(s) exist when passing in an option. This is a nice additional check that it wasn't doing in 4.4.0. It would be good to do the same for the primary key in a future PR.

[vsppedro]

I should have asked if you would like to make the contribution. I'm sorry.

Thank you very much!

LGTM!

[natematykiewicz]

You're totally fine!

[vsppedro]

I also think some DRYing could happen here between primary and foreign key checks.

I agree with you, but I think we can do that on another PR later. Let's fix this first. 👍

[vsppedro]

@mcmire, what do you think of this?

I'm thinking of releasing a fix for this - v4.5.1.

[mcmire]

Makes sense to me!

[vsppedro]

Thanks! Merging!

[vsppedro]

Hi @natematykiewicz, I'm sending this message just to let you know that we now have a new version of SholdaMatchers with your contribution! Thank you again!

[natematykiewicz]

I just upgraded. Everything works and the additional testing showed me that I had an incorrect :inverse_of on an association. 😬 Thanks for this!

[voxik]

Just FTR, I think there is something about the symbol vs strings, because testing against Rails 6.1, there are two test failures:

  1) Shoulda::Matchers::ActiveRecord::AssociationMatcher have_one rejects an association with a valid :class_name and :foreign_key option (CPK), but no columns
     Failure/Error:
       expect do
         have_one(:detail).class_name('PersonDetail').
           with_foreign_key(%i[company_id person_detail_id])
       end.not_to match_against(Person.new).and_fail_with(expected_message)
     
       Expected the positive version of the matcher not to match and for the failure
       message to be:
     
       ------------------------------------------------------------------------
       Expected Person to have a has_one association called detail (PersonDetail does not have a ["company_id", "person_detail_id"] foreign key.)
       ------------------------------------------------------------------------
     
       However, it was:
     
       ------------------------------------------------------------------------
       Expected Person to have a has_one association called detail (PersonDetail does not have a [:company_id, :person_detail_id] foreign key.)
       ------------------------------------------------------------------------
     
       Diff:
     # ./spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb:1455:in `block (3 levels) in <top (required)>'
     # /usr/share/gems/gems/bundler-2.2.3/libexec/bundle:49:in `block in <top (required)>'
     # /usr/share/gems/gems/bundler-2.2.3/libexec/bundle:37:in `<top (required)>'

  2) Shoulda::Matchers::ActiveRecord::AssociationMatcher have_one accepts an association with a valid :class_name and :foreign_key option (CPK)
     Failure/Error:
       expect(Person.new).
         to have_one(:detail).
         class_name('PersonDetail').
         with_foreign_key(%i[company_id person_detail_id])
     
       Expected Person to have a has_one association called detail (PersonDetail does not have a [:company_id, :person_detail_id] foreign key.)
     # ./spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb:1467:in `block (3 levels) in <top (required)>'
     # /usr/share/gems/gems/bundler-2.2.3/libexec/bundle:49:in `block in <top (required)>'
     # /usr/share/gems/gems/bundler-2.2.3/libexec/bundle:37:in `<top (required)>'