search

kbond / foundry-next

MIT License
0 stars 1 forks source link

feat: handle when an entity have two relationships to the same entity #17

Closed nikophil closed 8 months ago

nikophil commented 8 months ago

I'm struggling with this test

Actually, I managed to get feature parity with foundry 1, but I've discovered a bug (in foundry 1) and I don't know how to mitigate it, it is pretty complex.

in this test, if Post::$category has a default in PostFactory (something like 'category' => CategoryFactory::new()) the test fails. It fails because foundry creates a new category for each secondaryPosts.category :shrug:

And in Foundry 2, the equivalent field (Contact::$category) is mandatory, so... we always have as many category created as new "secondary contact" created.

https://github.com/kbond/foundry-next/actions/runs/7775788405/job/21202156474?pr=17#step:6:66

I think a (complex) solution would be to check all relationships in Contact which targets Category, and hydrates it with the main category, only if the field is mandatory

I have mixed feelings towards this: I'm quite surprised that this bug has not been spotted, this is not really a strange case. On the other hand, because nobody yelled about it, I'm tempted to make Contact::$category nullable in Foundry tests 2 and... voilà! and maybe open an issue

I have separated the PR in two commits: in the second one, I only set Contact::$category nullable, which makes the tests pass, and hides the problem under the rug :innocent:

WDYT?

kbond commented 8 months ago

I agree it's weird it hasn't come up and have mixed feelings also.

What about isolating the tests that fail and marking them as incomplete?