-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Twig functions for cmf_document_path and cmf_document_url #239
Comments
@dbu do you still feel it is correct have these functions catch and log exceptions? I am wondering if this is inconsistent with the symfony routing component. The reasoning for me was that if you use twig functions in a cms then the documents can be moved and its likely that the paths may change and result in this kind of error. For us the solution is the have our own custom cms_path and cms_url functions which catch and log these errors. Normal routes can change as well and twig still throws an exception for those so just putting this logic into the cmf versions of the twig functions only solves it for those routes and introduces some inconsistency. For our own implementation I am leaning towards having our cms_path/url twig methods the purposes of which are to catch/log errors and not introducing a separate twig function just for cmf documents. I dont think this is necessary especially if the url generator/route can handle generation for routes outside the current host (which it should be able to using the new candidates strategy to support multiple basepaths combined with a doctrine listener on load for cmf docs that sets their host requirements properly). Interested to hear your thoughts |
I do not really follow the complete discussion, but i can offer an other solution: symfony-cmf/seo-bundle#1 This was my very first issue/enhancement on SeoBundle and it is important for a CMS to have such a handling and it should be the task of the SeoBundle to generate answers for search engines and end users if a route couldn't be found. |
@benglass i think it does make sense. its rather special cases, as you should not need to hardcode something like so i think having those twig functions (basically wrappers with a try-catch around the standard functions) makes sense. we should log the situation as an error, its not something supposed to happen casually but its a data problem. @ElectricMaxxx a nice 404 page is another piece to the puzzle. but here the issue is when generating routes. |
Moved from symfony-cmf/Routing#90
Currently there is support for generating URLs for cmf documents using the symfony path and url twig functions but it would be nice to have a dedicated function that only generates CMF-based routes for the following reasons:
It might be better NOT to throw a RouteNotFoundException for these kind of routes because they are volatile (user's can often edit them and move them) and having this throw exceptions to the template is not desired
In a multi-domain situation these functions would ensure that routes being generated for a site other than the current one would always be generated as absolute although this logic would perhaps live in a DomainAwareGenerator
There is discussion happening around this here #178 but there is nothing stopping us from creating these functions using the current route generator rather than the chain route generator and also implementing the don't throw exceptions approach.
How should exceptions be handled? You could have a $throw boolean method in the constructor for the twig extension that is set based on the kernel.debug parameter (so it would throw exceptions in dev but not production. This might not be ideal because it would make it hard to work on a page that was throwing exceptions. Perhaps it could just log these exceptions in dev as critical errors.
The text was updated successfully, but these errors were encountered: