topclaudy / compoships

Multi-columns relationships for Laravel's Eloquent ORM
MIT License
1.12k stars 130 forks source link

Add support for factory relationships #155

Closed didac-adria closed 1 year ago

didac-adria commented 1 year ago

Related to issue #154

The idea of this PR is to add support for factory relationships. Right now methods such as ->has() or ->for() don't work for factories related to compoship models.

For the first version of this PR, I have added support for the ->has() method only. I have tested it locally and it works. I have tried to follow the same patterns of usage and files organization as the current package. Once we agree on that, and if we agree that this PR will be merged, we can work on the implementation for the other type of relationship allowed by this package that has a related method in the Factory class: the ->for() method. I left it for now because that's a little bit more tricky and I wanna make sure I do it only if the idea is merging this PR. I hope you understand ;)

topclaudy commented 1 year ago

Thanks @didac-adria. Can you please add tests?

didac-adria commented 1 year ago

@topclaudy sorry for the late reply. I have added the tests

topclaudy commented 1 year ago

@didac-adria We have some checks errors. Can you please address them? Thanks

didac-adria commented 1 year ago

@topclaudy thank you for running the tests. @erikn69 made a PR to fix it and looks excellent to my eyes. Can you run the tests again?

By the way, as mentioned in the PR description, this gives support to the ->has() method of the factories. However, the implementation of the ->for() method is not done. But I find this method useless because you can already use the ->recycle() method or just add the id of the parent as state instead. Additionally, it's a bit more complicated, but @erikn69 if you wanna go ahead, all yours.

didac-adria commented 1 year ago

@topclaudy can you run the tests again?

didac-adria commented 1 year ago

@parallels999, done. I think we can go ahead

parallels999 commented 1 year ago

LGTM 👍

topclaudy commented 1 year ago

Thanks @didac-adria. Released in 2.2.3.

didac-adria commented 1 year ago

Excellent thank you! We will be pulling that release then!