vierge-noire / cakephp-fixture-factories

CakePHP Fixture Factories
https://vierge-noire.github.io/
MIT License
83 stars 21 forks source link

Some thoughts/improvements #241

Open dereuromark opened 9 months ago

dereuromark commented 9 months ago

Factory defaults

When baking the factory, it by schema knows already required fields. For now it is empty by default, even though maybe 3-4 fields are required:

    $this->setDefaultData(function (Generator $faker) {
        return [
        ];
    });

It could actually already autocomplete those into the array:

    $this->setDefaultData(function (Generator $faker) {
        return [
            'name' => $faker->firstName(),
            'email' => $faker->email(),
            'summary' => $faker->text(40), // max length from VARCHAR(40) limit
            'foo' => // Add faker data
        ];
    });

It can even auto-guess the type or name if it matches to available faker methods somehow:

For unknown name matching, e.g. general string types, it could use the random generator per type by default. Or just leave the comment in as above to manually finish up.

It would easy enough to remove again but could be a quick start to have a valid and complete default data set per entity. What do you think?

Factory naming

We have UsersFixture, UsersTable, .. which deal with the User entity So the UserFactory should actually also be plural: UsersFactory

A consideration for a future version maybe? The bake script should easily be adjustable and it seems quite BC.

LordSimal commented 8 months ago

Default fields

Agreed. That would be a very cool feature to decrease the amount of setup in the factory.

Also, the already defined associations in the table class (if already found) could be used to generate the withXYZ() methods used to load associations into the factory/entities.

Factory naming

I personally prefer the singular name because the plural name is always associated to the table instance or the whole table logic in total.

Sure in the core we create entities via the table instance like $this->Articles->newEmptyEntity() but from a factory pattern perspective I prefer the singular name because we don't create table instances - we create entities.

Yes, there are table-like methods on the factory available get(), find(), first() etc. but the focus is still creating entities.

dereuromark commented 8 months ago

I started the default fields stuff: https://github.com/vierge-noire/cakephp-fixture-factories/pull/243 Feel free to chime in for more cool things.

dereuromark commented 8 months ago

I am not sold on the singular part however

bin/cake bake fixture_factory Events

It also uses plural input (table, sure), and it creates a factory that can create multiple ones. So similar like a table it handles 1...n of such entities. Therefore it would only make sense to keep this in sync with the rest of the ecosystem/framework, and only have the entity itself be singular.

See also the getRootTableRegistryName() part:

class AccountingFactory extends CakephpBaseFactory
{
    /**
     * Defines the Table Registry used to generate entities with
     *
     * @return string
     */
    protected function getRootTableRegistryName(): string
    {
        return 'Accountings';
    }

So instead of by default being in sync with table (which knows the entity by design), we need to go from Factory => Table => Entity instead. Seems overly complicated to me.