sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Deprecate or not `sonata_urlsafeid` #7068

Closed franmomu closed 3 years ago

franmomu commented 3 years ago

Feature Request

Ref: https://github.com/sonata-project/SonataAdminBundle/issues/6963

We have registered sonata_urlsafeid twig filter in two places:

I guess depending on which one is registered last, is the one used.

IMO we could deprecate the one from twig-extensions since it calls directly the persistence layer through sonata-extensions using a modelAdapter, the problem I see is that the way it is implemented right now we have to manually add the persistence layers in https://github.com/sonata-project/sonata-doctrine-extensions/blob/3a9f9086f3d94a68dea27d3db487355375139494/src/Bridge/Symfony/DependencyInjection/Compiler/AdapterCompilerPass.php.

The one from this bundle uses the admin service:

https://github.com/sonata-project/SonataAdminBundle/blob/04b8b38fcbba8ca04754ec4424d3c08bd6b4d52e/src/Twig/Extension/SonataAdminExtension.php#L575-L588

In this case we could deprecate passing the admin instance as suggested in https://github.com/sonata-project/SonataAdminBundle/pull/7055#discussion_r612696910 or completely, but that would require more work in our bundles using it.

VincentLanglet commented 3 years ago

Looking at the usage https://github.com/search?q=org%3Asonata-project+sonata_urlsafeid&type=code It's used by

I would say the sonata admin filter should be prefer.

So I would say we could deprecate the TemplateExtension of the twig-extensions project https://github.com/sonata-project/twig-extensions/blob/ccb5dce1012f58e607c38978cdea2c00814c9e93/src/Extension/TemplateExtension.php

Do you agree @sonata-project/contributors ?