refactor common features into RoutingBundle #98
Conversation
| /** | ||
| * provides multi language support when using MultilangRouteProvider | ||
| */ | ||
| class MultilangRedirectRoute extends RedirectRoute |
There was a problem hiding this comment.
we need to note these in the Changelog and Upgrade and explain how to migrate, i guess. or did anybody ever even use these?
|
@lsmith77 can you review this one too? the BC stuff works to at least make the sandbox still happy. though as there are some BC issues anyways, i wonder if we can simply drop routing configuration from simple cms completely. |
|
whats missing: update the PR on the docs again, and a small PR on CoreBundle to prepend the right paths. |
|
hmm would be easier to review such a PR with a branch of the sandbox that integrates all the changes |
|
there you go symfony-cmf/cmf-sandbox#244 would be great to get some input. i will wrap up tonight and then we can |
|
|
|
how does the multilang support work now? as soon as I add a locale to the page, it will add the |
|
either you manually (or maybe in the admin?) set the add_locale_pattern option, or you enable the auto_locale_pattern option on routing bundle. this is however global and will affect all routes, also those from other routes. though it should not affect routes that already have a locale in them, though that might still be incorrect. |
| { | ||
| public $node; | ||
| /** |
There was a problem hiding this comment.
generally i dont think we should map things like node and uuid by default. there is a bit of overhead involved and it exposes things that people on the ODM level should not worry about. then again, we already exposed this here before.
There was a problem hiding this comment.
yeah it would be a BC break to stop exposing it. but we can also remove
it, there is always getNodeByDocument on the documentmanager.
|
symfony 2.5 will probably fail until sonata-project/SonataAdminBundle#2027 is merged. |
| persistence: | ||
| phpcr: | ||
| route_basepaths: | ||
| - "/test/page" |
There was a problem hiding this comment.
this is actually a problem. we configure that path twice, once on simplecms and once on routing. when its the default path, the core bundle prepends it everywhere. but if we need something different, we duplicate the config. what could we do? could prepend configuration work in that case? what happens if the cmf_routing has route_basepaths configured, will the list be merged or replaced?
There was a problem hiding this comment.
seems we already prepend a bit of config from simplecms to the routing, so if it works it would be no big deal.
| @@ -32,9 +32,32 @@ class CmfSimpleCmsExtension extends Extension implements PrependExtensionInterfa | |||
| */ | |||
| public function prepend(ContainerBuilder $container) | |||
| { | |||
| $prependConfig = array('persistence' => array('phpcr' => (array('enabled' => true)))); | |||
| $container->prependExtensionConfig('cmf_menu', $prependConfig); | |||
| $prependConfig = array('dynamic' => $prependConfig); | |||
There was a problem hiding this comment.
i remove this part as the corebundle already does it, and we depend on it
dbu
changed the title
|
alright, i updated this and i think its ready now |
| @@ -45,7 +51,7 @@ public function testPage() | |||
| $page = new Page; | |||
| $refl = new \ReflectionClass($page); | |||
There was a problem hiding this comment.
@lsmith77 ok if we just drop this reflection stuff and use the normal way of creating a page through the dm?
There was a problem hiding this comment.
+1 this is really confusing code
|
okay, updated the test, should be good now. 2.5 will still fail as the PR on sonata admin is not merged yet. |
|
looks good .. but we probably need to bump the CoreBundle requirement |
refactor common features into RoutingBundle
adjust to symfony-cmf/routing-bundle#210