Duplicated fos_* routes as sonata_* (@SonataUserBundle) breaks page edition

rbs-asimon commented 7 years ago

Environment

Sonata packages

sonata-project/admin-bundle              3.10.3          The missing Symfony Admin Generator
sonata-project/block-bundle              3.2.0           Symfony SonataBlockBundle
sonata-project/cache                     1.0.7           Cache library
sonata-project/cache-bundle              2.3.1           This bundle provides caching services
sonata-project/classification-bundle     3.1.0           Symfony SonataClassificationBundle
sonata-project/core-bundle               3.1.2           Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.x-dev 60e0eac Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2           Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.1.2           Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/easy-extends-bundle       2.1.10          Symfony SonataEasyExtendsBundle
sonata-project/exporter                  1.7.0           Lightweight Exporter library
sonata-project/formatter-bundle          3.1.0           Symfony SonataFormatterBundle
sonata-project/google-authenticator      1.0.2           Library to integrate Google Authenticator into a PHP project
sonata-project/intl-bundle               2.2.4           Symfony SonataIntlBundle
sonata-project/media-bundle              3.3.0           Symfony SonataMediaBundle
sonata-project/notification-bundle       3.0.0           Symfony SonataNotificationBundle
sonata-project/page-bundle               3.2.0           This bundle provides a Site and Page management through container and block services
sonata-project/seo-bundle                2.0.2           Symfony SonataSeoBundle
sonata-project/timeline-bundle           3.1.0           Integrates SpyTimelineBundle into Sonata
sonata-project/translation-bundle        2.0.2           SonataTranslationBundle
sonata-project/user-bundle               3.2.0           Symfony SonataUserBundle

Symfony packages

symfony/assetic-bundle     v2.8.1  Integrates Assetic into Symfony2
symfony/monolog-bundle     2.12.0  Symfony MonologBundle
symfony/polyfill-apcu      v1.3.0  Symfony polyfill backporting apcu_* functions to lower PHP versions
symfony/polyfill-intl-icu  v1.3.0  Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring  v1.3.0  Symfony polyfill for the Mbstring extension
symfony/polyfill-php54     v1.3.0  Symfony polyfill backporting some PHP 5.4+ features to lower PHP versions
symfony/polyfill-php55     v1.3.0  Symfony polyfill backporting some PHP 5.5+ features to lower PHP versions
symfony/polyfill-php56     v1.3.0  Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70     v1.3.0  Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-util      v1.3.0  Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.0  Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v2.4.2  Symfony SwiftmailerBundle
symfony/symfony            v2.8.15 The Symfony PHP framework

PHP version

PHP 5.6.28-0+deb8u1 (cli) (built: Nov 15 2016 06:01:37) 
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies

Subject

When you install SonataUserBundle and use Sonata Controllers instead of FOS one, (Cf. https://github.com/sonata-project/SonataUserBundle/blob/3.x/Resources/doc/reference/installation.rst#use-custom-sonatauser-controllers-and-templates-instead-of-fosuser-ones), you get multiple pages sharing the same path like the screenshot of the official Sonata demo shows :

It should not be a problem except it breaks totally the possibility to edit one of these pages (BasePage.routeName property is unique).

My understanding is that the sonata_* routes are only required to do some clean separation for a project using these routes or not. So it is possible to ignore the routes during the page creation process (./bin/console sonata:page:update-core-routes)

Steps to reproduce

Install SonataUserBundle and activate Sonata Controllers like described here : https://github.com/sonata-project/SonataUserBundle/blob/3.x/Resources/doc/reference/installation.rst#use-custom-sonatauser-controllers-and-templates-instead-of-fosuser-ones

See the multiple duplicates in admin :

Try to edit one of these pages and cry because it is not possible (crying is optional ;) ).

Expected results

There should not be any duplicated routes : Sonata\PageBundle\Route\RoutePageGenerator should ignore the routes that are already defined (the Symfony router retains the first one).
fos_* routes like fos_user_security_login or fos_user_change_password should be editable

Actual results

Many duplicated and useless pages (15)
Page edition is broken for these elements

Neodork commented 7 years ago

I filter the sonata_user_security_* and sonata_user_registration_* in the sonata_page.yml as workaround. You end up with only FOS routes. However the Sonata functionality stays intact.

sonata_page:
    ignore_route_patterns:
        ...
        - ^sonata_user_security(.*) # Ignore sonata user routes
        - ^sonata_user_registration(.*) # Ignore sonata user routes
        ...

rbs-asimon commented 7 years ago

@Neodork the code modification does not care about FOS or not FOS. In fact the first (ordered by $router->getRouteCollection()->all() ) route declaration wins. And if I remember well, that is the way the Symfony router handle things.

@core23 I'm sorry I really had no time to add unit testing or what else. I needed this bundle to work for my company project and, for now, this project takes me too much time to be "gentle".

Neodork commented 7 years ago

@rbs-asimon Yes so if I assume it's ~~ordered alphabetically~~ ordered by the order of routes getting loaded, sonata routes will always be second compared to the FOS ones. So ignoring the sonata routes will get rid of the duplicate routes in the CMS and keep functionality. It is however prone to break and I think your PR would be ideal to fix the problem once and for all.

Like I mentioned it's only a workaround.

jordisala1991 commented 7 years ago

I do not recommend using sonata_user routes instead of fos_user for your custom login page, since I am trying to remove a lot of duplicated code between FOSUser and Sonata.

Look at this PR: https://github.com/sonata-project/SonataUserBundle/pull/869

stale[bot] commented 4 years ago

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.

sonata-project / SonataPageBundle