Domain#associations failed to check validity of associated models

[masukomi]

Fixes Issue #335

I'd like a second opinion from someone who knows this codebase better than I if this is the right way to fix Issue #355. If so I'll try and figure out how to write a unit test for it.

note: yes my raise syntax is inconsistent with regards to parens. It's inconsistent in the source and I was trying to emulate the nearby code. Also, please holler if you'd prefer different raise text.

[kerrizor]

After a surface reading, I think this code works, although I'd need to dive in myself to really grok the edge case.

The optional nature of destination is definitely a problem; I'd wager without looking that the (mapping[relationship.destination.name] ||= []) << relationship line was added by a later committer.. but every relationship has 2 sides, the source and the destination... I'd need to dig in and see where the disconnect is happening, but this is a great lead.

Do you have a relationship example that triggers this?

[masukomi]

I intend to make the requested changes + add tests on Friday and update the PR then.

[masukomi]

refactored. rebased. pushed.

re example:
I don't have a simple reproduceable case i can provide but the problem is specifically related to this association in PaperTrail which is coming into play via the Version class
which comes into play when you say class MyModel < PaperTrail::Version

re comments:

• moved check up in check_association_validity and removed from check_polymorphic_association because was no longer needed there.
• refactored check_polymorphic_association to be really minimal. This includes throwing out the || clause that couldn't prove true.

re || clause. I'm assuming it should be added in in the future in some other form but as it wasn't useful here anyway. this simplifies code, and doesn't change behavior until such time as those knowing better than me can figure out how to reinsert it in a more meaningful way.

re tests:
still trying to figure out how to write a test for this. but i can confirm that the new version of the code generated the erd without issue in my codebase with PaperTrail based classes.

[masukomi]

updated tests.

tldr; important one skipped. another one changed. change may be correct or may indicate screw-up in my change. EXPERIENCED EYES NEEDED

---

⚠️ WARNING

in relationship_test.rb

the original
cardinality should be one to one-many for mandatory one to many associations on polymorphic interfaces

is now test "cardinality should be zero-one with invalid polymorphic assoc" do
because defensible doesn't appear to be a valid association. As such the Cardinality in it has been changed to represent what's actually coming out now.

THIS MAY HAVE BEEN A MISTAKE ON MY PART

cardinality should be one to one-many for mandatory one to many associations on polymorphic interfaces

is now defined below cardinality should be zero-one with invalid polymorphic assoc and has a different create_model "Cannon"

things appear to function but i am skipping in because the results don't pass the test and i don't know if i should change the assertion OR if it's protecting us from a real bug.

--- expected
+++ actual
@@ -1 +1 @@
-[#<RailsERD::Domain::Relationship::Cardinality:0xXXXXXX @source_range=1..1 @destination_range=1..Infinity>]
+[#<RailsERD::Domain::Relationship::Cardinality:0xXXXXXX @source_range=1..1 @destination_range=0..Infinity>, #<RailsERD::Domain::Relationship::Cardinality:0xXXXXXX @source_range=0..1 @destination_range=1..Infinity>]