composite-primary-keys / composite_primary_keys

Composite Primary Keys support for Active Record
1.03k stars 350 forks source link

Add test for #491 #523

Closed f-mer closed 3 years ago

f-mer commented 4 years ago

This is a first step to fix #491.

Replacing if source_reflection.klass.composite? with if !source_reflection.polymorphic? && source_reflection.klass.composite? as suggested by @moiristo makes the tests pass but I'm feeling like this could causes different problems.

@cfis you've got an idea here? :see_no_evil:

cfis commented 4 years ago

Thanks for the test. So if this gets merged in, the test will fail (which is ok, just verifying).

Can you add a little bit of tests data so this through association has a bit of data on the other side?

As for a fix - good question. I've never actually used polymorphic associations, so not sure.

cfis commented 3 years ago

Thanks for the inspiration. I decided to remove the hack table, so did not merge this test. Instead, I updated the data model a bit to link comments to articles and then I could implement a similar test.

Thanks for the help on this!