Support relations stack & foreign relations

[shaffe-fr]

Hi,
This PR allows to create many related models at once. It also add support of BelongsTo and MorphTo relationship.

$recipe = RecipeFactory::new()
            ->with(Group::class, 'group')
            ->andWith(Ingredient::class, 'ingredients', 3)
            ->create();

$this->assertCount(1, Group::all());
$this->assertCount(3, Ingredient::all());
$this->assertTrue($recipe->group()->exists());
$this->assertEquals(3, $recipe->ingredients()->count());

To prevent a BC, the relation stack is reset every time with is called.
With this implementation, the same relation doesn't add up and only the last o

Here are some things that can be improved:

• define the expected behaviour of the make creation type,
• externalize the relations build mecanism,
• define relation creation type according to the relation type (and not the method),
• define if the same relation should stack:

$recipe = RecipeFactory::new()
            ->with(Ingredient::class, 'ingredients', 1)
            ->andWith(Ingredient::class, 'ingredients', 3)
            ->create();
$this->assertCount(3, Ingredient::all());
$this->assertCount(4, Ingredient::all()); /* Or */ 

What do you think about this @christophrumpel?

[christophrumpel]

Hey, thanks a lot for the good PR 👍 Here my feedback to the open points:

define the expected behaviour of the make creation type

What are the things we still need to consider?

externalize the relations build mecanism

See review comment.

define relation creation type according to the relation type (and not the method),

You mean that we get rid of the method name when using the "with" method? That as also on my list. would be cool to add that but it would be a breaking change since we cannot make it optional since the "times" variable comes afterwards. 🤔

define if the same relation should stack:

That's a good question. When we only have the "with" method and not the "withAnd" I would assume as the user that relations would be stacked, so in your example that would be 4 ingredients.

[shaffe-fr]

Hey,
Thanks for your feedback.

define the expected behaviour of the make creation type
define relation creation type according to the relation type

That was not very clear in the PR description :)
These two points are related.
The idea is to keep the with method and to adapt the creationType behaviour to each type of relation.
For example, in the FactoryTest::it_lets_you_make_many_related_models_at_once test the current behaviour is a bit funky. The ingredients are not created because the BelongsToMany relation has a saveMany method, but the group are.
Not sure how we can make the behaviour more consistent.

define if the same relation should stack:

The relations are now stacking.

[shaffe-fr]

I pushed a new version with a more consistent make behaviour.

$recipe = RecipeFactory::new()
            ->with(Group::class, 'group')
            ->with(Ingredient::class, 'ingredients', 3)
            ->make();

In this test, the group relation will be set on the recipe and $recipe->group_id will be null.
Do you think it's the expected behaviour?

[christophrumpel]

Thanks, looks good to me. For me this would be the expected behavior for make. 👍
Let me check some details again.

[christophrumpel]

Please remove the docBlock. I saw some other methods have them now too, but probably due some other PRs. I prefer not to use them when using types for parameters and the return.

[shaffe-fr]

The Phpdoc @return $this is needed to help the IDE autocomplete. The static return type keyword will be introduced in PHP 8 and will fill the gap with self.

[shaffe-fr]

Do you prefer if I only use /** @return static */ ?

[christophrumpel]

Which IDE do you use that you need that "@return $this"? In PhpStorm I don't need when using the self return type. But I know there was a problem with this in VS Code so we added the static return in the docblock just for VS Vode. (#24)

[shaffe-fr]

When I tested, I was indeed using VSCode.

[christophrumpel]

Ok, so if it helps with auto-completing in VS code, we can keep it. For PhpStorm I know that the hinted return type self does work currently as long as there is not "static" return type yet.

[christophrumpel]

So I think we are good then to merge it, anything missing in your opinion?

[shaffe-fr]

Do you think we should update the readme to document the new behaviour of the with method?

[christophrumpel]

Oh yeah definitely. Can you adapt it and I will check then?

[shaffe-fr]

Done! Is it ok for you @christophrumpel?

[christophrumpel]

Thanks a lot @shaffe-fr. It took us some time but I think we made it :-) Great improvement.

[christophrumpel]

I just pushed a new release v.1.1.0 👍

[shaffe-fr]

Thanks! Glad I could help!