minor: add $persistentManagerName param

[nikophil]

fixes #447

I've not provided tests, because the code is trivial, but this would need to add a new document manager in the test 🤷

/cc @kerstvo

EDIT: I just realized that adding a param to ModelFactory::repository() will create an error in psalm in userland, because of the @phpstan-method annotation 😞

https://github.com/zenstruck/foundry/actions/runs/4707671150/jobs/8349671474?pr=450#step:8:18

[kbond]

I just realized that adding a param to ModelFactory::repository() will create an error in psalm in userland

You mean on existing factories? I guess we should we update the maker/docs?

[nikophil]

You mean on existing factories?

yes, all existing factories will create an error with psalm if we make this move... We can update update maker and docs, but still, this could be pretty annoying...

[kbond]

What about phpstan?

[nikophil]

even in max level, phpstan does not complain. But psalm is actually true: one param is missing in the methods prototype in the phpdoc

[kbond]

Huh, well I don't want to not add a feature because it breaks static analysis, but that's just my opinion.

[kbond]

What about starting a UPGRADE.md with a note for the next feature release?

[nikophil]

ok, this seems reasonable :)

I feel the same, otherwise it would restrict too much further updates... see this issue as well #445 (comment)

[kerstvo]

It seems that problem is more complicated.
I apply your changes in my project and got error

[kbond]

I think we need to have a Factory::withManagerName() where you actually configure the alternate manager name per factory.

This would remove the need for $persistentManagerName in ModelFactory::repository() (and not break static analysis for existing factories).

[nikophil]

this seems not possible because ModelFactory::repository() is static...

I think this needs to be hardcoded directly in the class, and then have a factory class which is bound to a given manager. ex:

class SomeFactoryForSpecificManager
{
    protected static function getClass(): string
    {
        return SomeEntity::class;
    }

    protected static function getManagerName(): ?string
    {
        return 'specific-manager;
    }
}

@kerstvo would this fix your problem?

[kbond]

So ModelFactory would look like?

abstract class ModelFactory extends Factory
{
    // ...
    
    protected static function getManagerName(): ?string
    {
        return null;
    }
}

[nikophil]

That's what I'm suggesting :)

[kbond]

Since we're deprecating ModelFactory soon, maybe we should just add this feature to #452?

We'll need to update some function parameters and the anon. proxy generator.

[nikophil]

I'm closing this PR, which targets 1.x and is full of conflicts. The issue stays opens, though