Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Make Database Factory macroable #36380

Merged
merged 1 commit into from
Feb 25, 2021
Merged

[8.x] Make Database Factory macroable #36380

merged 1 commit into from
Feb 25, 2021

Conversation

cmgmyr
Copy link
Contributor

@cmgmyr cmgmyr commented Feb 24, 2021

My company is looking to upgrade to Laravel 8 soon, and while doing some due diligence, we noticed that we were going to have an issue with how we are currently using factory states and how they are handled in L8. While we're looking forward to using the new class-based factories, we have hundreds of states, thousands of tests, and a home-grown factory/state builder, which is used to create complex user stories. These references are using the string-based versions of states but accumulated over all of the builder calls and finally executed with the L7 ->states(...) method call.

It would be a huge undertaking to make adjustments to the builder and delay the upgrade to L8. So, I'd like to add a macro to get the states() method back onto the database factory class. This way, we'd be able to temporarily leverage the macro and delete it when we're done with the changes we need to make to our builder. We might have to add some other macros as well, but this is to be determined.

The initial idea is to do something like this for the states() method.

use Illuminate\Database\Eloquent\Factories\Factory;

Factory::macro('states', function (array $states = []) {
    $factory = $this;
    foreach ($states as $state) {
        $stateMethod = Str::camel($state);
        $factory = $factory->{$stateMethod}();
    }

    return $factory;
});

@taylorotwell
Copy link
Member

What were you wanting to add to it?

@cmgmyr
Copy link
Contributor Author

cmgmyr commented Feb 25, 2021

Hey @taylorotwell, I updated the PR description for you.

@taylorotwell taylorotwell merged commit aa19ecd into laravel:8.x Feb 25, 2021
@jasonmccreary
Copy link
Contributor

@cmgmyr, Shift has automation tools for converting to class-based factories. May be something to consider when you make the eventual upgrade.

@cmgmyr cmgmyr deleted the macroable-eloquent-factory branch March 2, 2021 20:52
@cmgmyr
Copy link
Contributor Author

cmgmyr commented Mar 2, 2021

@jasonmccreary yes, we're looking forward to using shift! We've used it on all of our other upgrades. The problem with our use of states is that we're passing them around in an unconventional way and I'm not sure if Shift will be able to convert those automatically for us. I can loop you in when we kick off the shift and see what happens 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants