Possible bug in Admingenerator\GeneratorBundle\DependencyInjection\AdmingeneratorGeneratorExtension::registerFormsServicesFromGenerator

[TheCelavi]

Every auto-generated form is registered in: Admingenerator\GeneratorBundle\DependencyInjection\AdmingeneratorGeneratorExtension::registerFormsServicesFromGenerator

Pattern is:

$container->setDefinition('admingen_generator_' . strtolower($model) . '_edit', $editDefinition);

Note that _edit can be _new and _filter as well.

strtolower($model) is used, and model is class name without namespace prefix.

That means that, per example, classes:

• \One\ModelX
• \Two\ModelX

Would have generated form type service under the same name:

• admingen_generator_modelx_edit, and so on...

That means that in case when we have models with same name under different namespaces, form type services gets overridden.

This is easy fix, can someone confirm this, just another pair of eyes, in case I am missing something?

[ioleo]

@TheCelavi yes, this is obviously a bug, could you submit a PR to fix it?
However, changeing the service names would introduce BC break to anyone already useing these forms (@sescandell ? ). We should also provide an alias (old service name) to the first/latest defininion (whichever was in current behaviour).

[TheCelavi]

Of course, assign it to me. Agree with alias.

Should I update this file

https://github.com/symfony2admingenerator/GeneratorBundle/blob/master/UPGRADE.MD

with info as well, is there any log in which change should be noted?

[ioleo]

There is no changelog, only what you mentioned - upgrade notes.

[sescandell]

Good one! Thanks @TheCelavi

However, changeing the service names would introduce BC break to anyone already useing these forms (@sescandell ? ).

Tru... but I'm not sure actually people use the service name itself: they are not necessary anymore if I'm not wrong... actually... not sure about that...

[TheCelavi]

@sescandell If someone used compiler pass to add additional dependencies to generated form type service (as we used to do so), new name without alias would be a BC break.

I don't know what is your practice in development, but I have added TODO comment as reminder for alias to be removed in next major release when BC breaks are allowed.

[sescandell]

@TheCelavi 👍