Skip to content
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

Open
benglass opened this issue Apr 15, 2014 · 3 comments
Open

Add Twig functions for cmf_document_path and cmf_document_url #239

benglass opened this issue Apr 15, 2014 · 3 comments

Comments

@benglass
Copy link
Member

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.

@benglass
Copy link
Member Author

@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

@ElectricMaxxx
Copy link
Member

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.
In my case it shouldn't matter what the reason for that NotFoundRoute is, but the answer does matter: 404 and some extra information for the user. That means SEO and UX in one.
(Btw i won't be able to add that for 1.0 that's planed for 1.1 of SeoBundle)

@dbu
Copy link
Member

dbu commented Apr 16, 2014

@benglass i think it does make sense. its rather special cases, as you should not need to hardcode something like path('/cms/routes/en/test') in your template (since having worked with drupal, i am really wary of mixing content and application. if you need this route to exist and deploy it, make it a core symfony route). however, say i have a list of documents that is supposed to have a route pointing to them, and i loop over them. now if accidentally one has no route, that should not break the application.

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.
if people use it to generate their core routes with this, its their own problem, we can't prevent that. one question is what to return in that case - i guess a configurable url that defaults to "/" would make most sense.

@ElectricMaxxx a nice 404 page is another piece to the puzzle. but here the issue is when generating routes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants