Sonataadminbundle: Decide what to do with unused id parameter

[greg0ire]

Several actions (CrudController::editAction, CRUDController::showAction) have an unused id parameter, a legacy kept for BC. We should decide what to do on next major.

Several possibilities :

• drop the parameter
• introduce a childId parameter and remove the getIdParameter method, but then we would have to take childId when not null, id otherwise to get the current object id, which is not very handy
• @Soullivaneuh talked about ParamConverters, can you please develop ?

[curry684]

Given that PHP silently ignores extra parameters and there are no plans to change that during 7.x lifetime keeping them around is far more harmful (it implies it should work, https://github.com/sonata-project/SonataAdminBundle/pull/3961) than removing (everything will keep working yet show IDE warnings to consumers).

[greg0ire]

It's indeed worth noting that in this particular case, the parameter can be removed without causing BC-breaks, even when extending the class. This seems to be because of the = null part, which makes the child class callable just like the parent class would be: without any argument.

[greg0ire]

Thinking more about it, the getIdParameter was probably introduced because routes in a child admin use id and childId, while the could have used parentId and id.

[soullivaneuh]

@Soullivaneuh talked about ParamConverters, can you please develop ?

Having directly the object on the action parameters instead of the ID thanks to this:

But this has some inconvenient:

• This seems to require annotations enable. Have to see if we can use it without them.
• Not sure if we can use it for an not determined object type.

[greg0ire]

This seems to require annotations enable. Have to see if we can use it without them
Could be doable by setting the _converters attribute of a request, but this feels ugly.

Not sure if we can use it for an not determined object type.

The param converter would have to know about the current admin to do that, so… why not.

Anyway, unless you're willing to work on this in the very near future, I propose to get rid of the parameter. It won't prevent you to add ParamConverters later if you want to.

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[VincentLanglet]

@greg0ire Should we close this, thanks to https://github.com/sonata-project/SonataAdminBundle/pull/5807 ?