Open benglass opened 11 years ago
@dbu It would be be able to handle both regular symfony routes and phpcr routes but I feel the client use case is primary to insert links to other cms pages. Inserting dynamic routes with parameters would require a complicated user interface wherein you could select possible parameters. This part of the feature would be most used by developers and perhaps could be achieved by allowing some subset of twig (for example path() or url() calls could be evaluated). Perhaps this could be used as the approach for both kinds of url expansion (phpcr and symfony routes). Then the functionality @uwej711 mentioned in https://github.com/symfony-cmf/SimpleCmsBundle/issues/77#issuecomment-25064922 could perhaps be made more robust so it could handle uuid instead of a phpcr path which is volatile.
In terms of handling situations where the cms user delete a page and recreates it elsewhere im wondering whether this is should be wrapped in with this feature or considered a separate enhancement as I do not think this is something most cms' handle.
See also the RoutingAutoBundle for a different approach to this problem:
http://symfony.com/doc/current/cmf/bundles/routing_auto.html
Basically that bundle will automatically create routes based on given rules for a given Document (e.g. the Page). You can then, for instance, tell it to leave Redirect routes when the page is moved.
As an aside, I was thinking that it would be quite simple to create a "SimpleCMS" using the RoutingAutoBundle and the original routing system (i.e. not having a custom RouteProvider etc).
Interesting, it seems the RoutingAutoBundle could potentially already address the requirement of automatically redirecting when a document is moved provided that the link pointed at the auto route and not the original document. Presumably AutoRoutes have uuids and could therefore be referenced in a link and parsed out in the way as any other phpcr document. We would just need perhaps a service that provides a list of pages from which the user can select when inserting a link in the wysiwyg. Whether this service loads autoroutes or documents would be up to the configuration, perhaps some kind of Interface could be provided for this and a few base implementations although it may be enough to simply allow the basepath to be configurable on a generic provider, which would load all of the routes found at that basepath.
Not sure I understand fully - in what scenario would you want to "parse out" an auto routes UUID from a link?
On Wed, Sep 25, 2013 at 12:12:03PM -0700, Ben Glassman wrote:
Interesting, it seems the RoutingAutoBundle could potentially already address the requirement of automatically redirecting when a document is moved provided that the link pointed at the auto route and not the original document. Presumably AutoRoutes have uuids and could therefore be referenced in a link and parsed out in the way as any other phpcr document. We would just need perhaps a service that provides a list of pages from which the user can select when inserting a link in the wysiwyg. Whether this service loads autoroutes or documents would be up to the configuration, perhaps some kind of Interface could be provided for this and a few base implementations although it may be enough to simply allow the basepath to be configurable on a generic provider, which would load all of the routes found at that basepath.
— Reply to this email directly or [1]view it on GitHub.
References
Visible links
I havent used the auto routing so perhaps im misunderstanding but i was thinking that if a user was editing a page in the backend and inserts a link they would be able to choose from a list of pages/routes generated by the cmf. When selecting one, it would insert some special format
<a href="%%%path55%%%">Link to doc 55</a>
I was thinking 55 would be the uuid of the document to link to in phpcr. If you are using SimpleCms the uuid might be an instance of Page from /cms/simple. If you are using AutoRouting perhaps its the uuid of an AutoRoute. What options are presented to the user in terms of links they can insert to existing documents would be determined by a basepath configuration or perhaps a service that provides a Tree of documents or something similar would could be used to build a select dropdown.
Ah yes. Well, in that case if the original URL was "/blog/post-1" and it was moved to "/blog/post-2" then the routing auto bundle would have just dropped a redirect route at "/blog/post-1" (redirecting to "blog/post-2"). So with that approach there would be no need to have a subsitute.
But while thats great for URLs sent out by email etc. It is les good for internal articles that we render, in which case some sort of thing like you propose would indeed make sense.
On Wed, Sep 25, 2013 at 01:11:06PM -0700, Ben Glassman wrote:
I havent used the auto routing so perhaps im misunderstanding but i was thinking that if a user was editing a page in the backend and inserts a link they would be able to choose from a list of pages/routes generated by the cmf. When selecting one, it would insert some special format
I was thinking 55 would be the uuid of the document to link to in phpcr. If you are using SimpleCms the uuid might be an instance of Page from /cms/simple. If you are using AutoRouting perhaps its the uuid of an AutoRoute. What options are presented to the user in terms of links they can insert to existing documents would be determined by a basepath configuration or perhaps a service that provides a Tree of documents or something similar would could be used to build a select dropdown.
— Reply to this email directly or [1]view it on GitHub.
References
Visible links
i would propose that instead (or in addition to) an internal link target selector, we should also have a doctrine listener that finds link to the site domain and resolves the target path of a normal <a href="...
with the router to detect the route / content document and replaces the absolute link with the phpcr path and the uuid.
having the user put a uuid or something as link target will not work in frontend editng with CreateBundle, as the filter is then already applied and the link transformed into the real link. on the next save in frontend, the id would get overwritten.
@dbu good point and i can think of other ways in which front end editing could cause issues like.
For example on our cms architecture we are not currently using the concept of a page being composed of multiple blocks (same with simple cms) so I implemented a twig filter which inserts dynamic calls to blocks based on a format.
<p>Some static content</p>
%%%sonata.block.service.foo &bar=`baz`%%%
<p>Some more static content
The %%%foo &bar=baz
%%% would be replaced with the sonata.block.service.foo block service and called with the options array('bar' => 'baz').
This would be rendered by create as the final HTML output of the block and would be broken by front end editing. This approach is actually based on one that is written about in the docs of cmf block bundle at http://symfony.com/doc/current/cmf/cookbook/using_blockbundle_and_contentbundle.html#tutorial-block-embed
It makes me wonder about the approach of allowing users to editing filtered content through the front end as it could have a lot of potential issues similar to these which we cannot predict. Making it so users are editing the content that is stored in the phpcr (instead of the rendered content) may have other issues (we'd have to save the content and re-filter it and inject it into the space after editing).
<span class="no-editable">
The dynamically generated html
</span>
Not even sure this would help unless we then matched up the no-editable sections with the original content which seems messy and error prone.
Putting all your content into a collection of child blocks can sidestep the issues of the dynamic content a bit.
For me I think it would be an acceptable solution if I could just disable frontend editing of some pages and if the edit button just took users to sonata admin page edit for those pages but id love to hear other folks' thoughts.
i think the url is a very basic and thus special case where a back-and-forth conversion would make sense.
setting a part non-editable will not help, its still part of your wysiwyg blob that gets stored back into the db. what the embed block thing could do would be to have the filter render something around the magic tag that allows a listener to remove the rendered stuff and restore the call (unless the frontend editor goes destroying that something while editing, but then screw him). something like <div cmf-block-render="{eventual:parameters}">...
. this concept could be applied to any filter thing somebody comes up with: if you provide a filter, you should provide a listener as well.
each filter you activate is going to cost you. listening on save operations should be comparably cheap as its not happening that often. and anyway is not more expensive than rendering the page.
deactivating frontend editing on a per page basis should become possible at some point. you definitely can do it on a per template basis by not rendering the createjs annotations. but once we have fine grained permission management, we need more. right now we just don't render the create.js inclusion if you are not allowed to edit (based on a role). anyway, this topic belongs into CreateBundle... for the OT i would vote for listeners to convert things back.
btw, as we already have the block twig filter, can you open an issue on BlockBundle explaining what was missing from the provided filter? could we extend the filter to be able to do what you want? or is it a very special case?
@dbu perhaps this can go into block bundle but the reason I created a separate filter was that I am not embedding blocks that are stored in the database. These are service oriented blocks that are not about there being editable content stored in the cmf. They are basically just standard sonata blocks and I want to render them inside a cmf doc.
The main missing feature (which could certainly be added to block bundle) would be the ability to specify parameters in the embedded call. This is key for my use case where you are listing projects (or perhaps showing 3 featured products in which case we want the options of feature=1
and limit=3
in the embedded call). Ill move the discussion over there and see if there could be some enhancement to block bundle to suit this.
@dbu I have moved the block discussion to a new issue https://github.com/symfony-cmf/BlockBundle/issues/143
@dbu so at this point it looks like we can move forward with adding the following
A twig filter that converts uuids to URLs
What format should it be looking for? I like [~uuid~] as its very simple. It seems it might be worthwhile to come up with a standard format for dynamic content like this in documents which may be replaced. MODx uses brackets as does wordpress. Alternately it could be something that looks like like twig {{ path('', { phpcr_uuid: 55 }) }} and then we could actually parse it out and call the path function which could be made to support phpcr_uuids which would have other advantages.
I would say the other items we discussed such as parsing dynamic sections should be treated as a separate issue.
magnolia cms stores not only the uuid but also the path. this has the benefit that after migrating content or destroying and rebuilding a content, things still work. the uuid has the benefit that after moving, things still work. so ideally we would also store both path and uuid to have a way to survive as much as possible.
the filter for dynamic sections should go to BlockBundle, created https://github.com/symfony-cmf/BlockBundle/issues/144 for that.
so the magic thing could be [~uuid:~
. could we use some other separator that is easily escepable in the path or not valid in a phpcr path in the first place? *
might be a good path invalid character. [*uuid:<uuid>|path:<path>*]
@dbu I am not grasping the magnoa cms/storing the path in addition to the uuid requirement, it seems like redundant data. Can you please explain further? You are talking about if the user deletes page uuid 1 at path /cms/about and then creates page uuid 2 at path /cms/about (the same path)? I suppose that would be a nice feature to support but it seems like a bit of an edge case that would add additional complexity. Perhaps handling paths as well to survive delete/recreate scenarios could be an extra enhancement.
I have no problem with [uuid:55] as a format. Perhaps we should consider using 2 brackets. My reasoning is that in MODx for example you can embed different kinds of things using these special tags, whether they are URLs or system settings. Which type of thing is loaded depends on the special character used in addition to the brackets so
[[55]]
Might be a url, whereas
[[#something.foo.bar#]]
Might be a container parameter that would be automatically injected.
ah, i see. well that sounds like the ultra dense wiki syntax that then are hard to read if you don't know them very well. i would prefer to be explicit. for the url thing i was assuming the base wrapper is that its href="<our thing>"
that lets us detect it. oh and src=""
for images i guess. but indeed thinking about this more generally might be a good idea.
the delete and recreate scenario is indeed an edge case. but dumping the repository and re-importing it is rather standard, e.g. to see the data on a stage or local dev env. re-importing can lead to uuid changing. depending on your settings when importing they either change for sure or only in case of conflicts. that scenario is imho less of an edge case.
adding path support does sound like a good idea and it doesnt really seem particularly more complex from an implementation standpoint (just falling back to load by path if uuid is not found).
regarding the format i think this is a feature for developers more than non-technical users (clients would insert these links via some kind of wysiwyg editor function). Having clients understand the concept of uuids and paths is something we probably want to avoid regardless of the format of this.
I think it would be best not to assume these placeholders would appear in only certain places like href="" or src="" because users may want to use them to populate javascript variables or other things we may not think of.
Having a single format like
[uuid:55 path:55]
may be just fine and could be expanded to handle other things like parameters via
[parameter:key]
This would be more explicit
Perhaps the format should be more like
[_filterprovider param1:value param2:value]
So we'd have
[url uuid:55 path:/cms/about]
and maybe later on
[parameter name:foo.param.name]
This meets the requirements of being explicit and also establishes a pattern that could be used for other things. We could potentially treat it like MenuProviders and register services via a tag that would be used by the filter to provide expansion of this snippet format based on the filter_provider. By default we could provide a phpcr_url_filter_provider
I am wondering if we should consider the name url vs path. In symfony those are used in templates to differentiate between absolute and relative.
This could also dovetail with the way that the cmf_embed_block works. Instead of having a separate twig filter that bundle would just register a phpcr_block_filter_provider. That way the concept or parsing options from the string would be all handled in one place.
very interesting ideas. having one common format sounds like a good idea. for the syntax we have to be careful, as phpcr paths can contain spaces and colons...
for the reverse transform, a url is special, it can not have a tag around itself as its part of a tag. where the blog can well have some tagging around. so we still have somewhat two cases at least for the storage filter. or the standard case of blocks and the special case of urls.
supporting both path and url makes sense. we can also detect that when we see if href / src has a domain or not. btw, not sure how you would parse out paths occuring at random locations. for urls it could actually work.
im thinking we may want to wrap the values in backticks to address them being able to contain anything, is backtick allowed in a phpcr path?
[*url uuid=`55` path=`/path:to/containing=equals`*]
In terms of parsing them out in random places I was envisioning that we are just going to be like the existing embedBlocks functionality https://github.com/symfony-cmf/BlockBundle/blob/master/Templating/Helper/CmfBlockHelper.php#L54
I am thinking perhaps a more unique delimter than [* may be required as if the content contained
<p>We offer everything* you can image [*well not everything] go look at <a href="[*url uuid=`55`*]">this</a></p>
It could create false positives. Seems like an unlikely situation but making the delimeter very unique would prevent it even as a possibility so perhaps having 2 brackets.
[[*url uuid=`55` path=`/path:to/containing=equals`*]]
I prefer the equals syntax to the colon as it seems more intuitive. We could also allow configuration of the delimeter just as the block bundle does.
a *
is about the only thing that can really not occur in a phpcr path.
the alternative would be blank space and encode spaces with %20. well,
we could encode any delimiter we use with its character code that way.
making delimiters configurable sounds right, then people have a way out
for special cases. the uniqueness is why the block tag has this whole %
and a name business.
Just an idea, why not storing that information in a data attribute of the a tag (data-target-uuid="...")? It should be easy to do that with ckeditor (it has some add link functionality) with the added benefit to have it for frontend and backend editing.
@dbu im not seeing an easy solution to picking delimeters that wont run into possible encoding issues in terms of values and this should be able to support any string value so im voting for configurable delimeters that are escaped in any values that use them. This means we will probably have a Helper class which can do this encoding/decoding.
So i guess im saying a configurable delimeter (wondering if this should be configurable in the cmf_core because what im advocating is a general cmf twig filter that uses a provider concept to handle filtering the content based on the embed_filter_key. The filter providers would be tagged and gathered in a compiler pass and injected into the twig filter helper.
In the config
cmf_core:
twig_embed_filter_delimiter: [[
In your document content
[[*url uuid=`55` path=`/path:to/containing=equals`*]]
This would be converted to a url by a service tagged as an embed_filter_provider, example service configuration
services:
cmf_router.embed_filter_provider.url:
class: Cmf\RouterBundle\Templating\Provider\EmbedFilterUrlProvider
arguments:
- @the.document.manager
tags:
- { name: cmf_embed_filter_provider }
Pseudo code in the twig filter (see also https://github.com/symfony-cmf/BlockBundle/blob/master/Templating/Helper/CmfBlockHelper.php#L48)
public function processedEmbedded($text)
{
// do a preg_replace_callback on the text looking for delimeter(embed_filter_name option1key=`option1value`)delimeter and calling embeddedRender
}
public function embededRender($embeddedMatches)
{
// Parse out the embedded_filter_name and the options into an array of key => value pairs
// Loop through the providers from the cmf_embed_filter_provider tag and call $provider->has($embedded_filter_name)
// If it returns true, then call $provider->get($embedded_filter_name, $options) which returns the replaced string
}
@dbu We are looking at this again for our CMS. We discussed possibly using a twig sandbox as it would provide a clear extension point. We would potentially provide config parameters for enabled methods/functions, etc. in the sandbox to make it easy to configure, something along the lines of:
security_policy:
tags: []
filters: []
methods: []
properties: []
functions: []
sandbox:
sandboxed_globally: false
so basically this would mean to evaluate embedded twig in the content, but evaluate it in a sandbox rather than the full application context? is that already done somewhere? it could make sense, just sounds like a complex undertaking to me :-)
and how will that help us with converting things back from the rendered html to the storage format, in case we used a wysiwyg editor in the frontend?
@dbu Setting up the sandbox approach is actually pretty simple (ill post some code) and gains you the ability to embed any kind of content you might want into your document content which is a very powerful bonus. On the other hand it also introduces potential for abuse if you are too liberal with your sandbox policy. It also prevents having to do string parsing and introduce a new way to embed wiki-style tags in content and you can allow the path() twig function and take care of url generator. It does require the load_template_from_string twig extension/function as the usage is something like.
{% sandbox %}
{% include load_template_from_string(cmfMainContent.content) %}
{% endsandbox %}
However I have discovered a few issues with using the existing path() function in this way (to insert links to cms pages).
I think for my use case I will probably create a separate twig function for generating phpcr links that has the following additional behavior
This is similar to a PR I submitted to the menu bundle to allow absolute and relative paths to be passed to knp_menu_get/render so let me know if you think its something you'd like to have in the cmf or if you can see a way to integrate it into the existing path() function with causing breaking changes (i can't see how the relative path part would work beause it would mean you could never have a node in the phpcr with a name that is the same as a static route ie. a page at /foo/bar/about or /biz/baz/about would supercede a route named about). Also throwing an exception has to be caught/suppressed for a twig function that could easily fail like this (the document moved for example). This wouldn't be a problem if you have auto routing perhaps to set up 301 redirects but even then url generation code like this is somewhat volatile within a cms and can't throw an exception that could break the template.
for the question at hand, i think we want a specific thing that uses extra logic to figure out where to link to. in the sandbox, we could define a cmf_document_path funktion that accepts both path and uuid so that we can build the link from either repo path or uuid.
to get the uuid, we would either need to map the jcr:uuid field in Resources/config/doctrine-phpcr/Route.phpcr.xml and add a property and getter to the document class, or add something to phpcr-odm. the unit of work currently has getDocumentId, having getDocumentUuid would make sense to me.
so the relative paths would mean links inside the same domain? that would mean that the uuid could be wrong, if it is from a route of a different domain. for the routes outside of the domain, we would need an extra route generator i think, something like a "domain aware generator" that realizes if we create a crossdomain link and make sure the domain name is output as well. it would need to take care of configuring the right prefix as well, to make the route building successful. do you see what i mean? it looks to me like that is one, if not the bit we need to support multi-domain sites. btw, it would be great if you could share how you set the basepath based on the domain. i would love to have a cookbook entry in the cmf doc explaining how to do multidomain with the cmf.
regarding hiding system routes, that depends on the order you configure your routers, but of course having multiple routers you always have the potential of one hiding the other.
by the way: here we talk about links in the content, right? a template loading routes stored in the repository by using their repository path would be flaky. you don't want to hardcode database paths in your templates, that would break as soon as an editor changes the path. if its something that must exist for sure, make it a static route. if it has to be editable, give it a special type so that you can find it dynamically.
regarding failing to create links, a thing that i think we should add to the corebundle twig helper is a function cmf_is_linkable
public function isLinkable($document)
{
return $document instanceof RouteReferrersReadInterface && count($document->getRoutes()) > 0;
}
@dbu
A step in this direction would be identifying classes that handle basepaths, these are the ones we have been concerned with in order to handle multi-website routes
Any insight into other services that fall into this category would be great.
Regarding the behavior of a DomainAwareGenerator it would basically determine if the route being generated was on the current domain/website and if not it would always return an absolute url.
I would be willing to post example code but our implementation is not substantially different from PrestaCMSCoreBundle
A request listener to detect the website based on host matching https://github.com/prestaconcept/PrestaCMSCoreBundle/blob/master/EventListener/WebsiteListener.php#L58
Which dispatches to a WebsiteManager to actually do the work of configuring the services that handle basepaths (and anything else that has to change based on the website). https://github.com/prestaconcept/PrestaCMSCoreBundle/blob/master/Model/WebsiteManager.php#L255 https://github.com/prestaconcept/PrestaCMSCoreBundle/blob/master/Model/WebsiteManager.php#L188
@dbu just to elaborate on why i think this listener based approach doesnt work in my experience with our implementation here is a scenario
This is the reason Im thinking having the prefix listener and other phpcr related services that know about a prefix be able to handle multiple prefixes would be better than trying to keep track of a single prefix which starts to feel like global state.
Another issue is when you have a cms admin area for managing multiple websites (sonata admin) and the user could be logged on any number of domains and switching the current website they are managing (tracked via the session since you cant determine it from the url). Then lets say they edit a page on website A then switch to website B then hit the back button twice to go back to the edit page. Now your session says they are editing B but they are trying to load a page from A from the phpcr which is going to throw that IdPrefixListener exception.
ah, i see. you have a point here yes. sure, +1 for cmf_document_url as well. and for defining that if either path or uuid is found, that is used rather than complain if a path was specified but not found. i guess it would be path = $name and uuid a field in the options array.
local paths those make assumptions on how your routes are named. what if i want the route for about to be called about-us? i would either have a menu for such things (a secondary navigation having the about, impressum and a contact for example) - then you do not need to hardcode the route. if you need to link to about from various places, maybe link it from the "website" document and get it from there?
multiple base_paths: it seems we indeed could use them, but i think it still should only be used for paths that lie within your current site. if you tell the route provider routes can be at /routes/site-a or /routes/site-b you can't determine anymore if you need absolute urls or not. and even worse, you would serve urls from site-a under domain site-b which is not a good thing. so i think what we should have:
@dbu regarding multiple base_paths I was assuming there would be a single basepath PER website or domain. The you would be able to tell if someone asks for /routes/site-a that this is in site a and sita a is the current site and if they ask for /routes/site-b you could tell thats not in the current site and always return an absolute URL
This does assume there is a service which knows what the current website is. There would have to be a way to map a host pattern to a single basepath (even a set of basepaths would be ok). This would allow us to always know whether the requested basepath was within the current website or not and for this I think the approach of a request listener is fine. It would match the current site based on a host pattern and update a service.
Our service is called WebsiteContext and it just has a setWebsite and getWebsite method.
If we wanted to generalize this could be a HostAwareBasepathProvider service which would perhaps have methods like all and getByHost.
not sure if i should reply here or in the symfony-cmf issue you opened... but i think having a router that looks at routes from other domains for route matching would be wrong. and building all the logic into the route generator we have seems to not well handle separation of concerns. imho we should create a separate route generator that knows about the multisite stuff and what is the current site and handle that - see my comment in https://github.com/symfony-cmf/symfony-cmf/issues/182
There should be a built in way to embed links/urls to other routes in the CMF into the body of a PHPCR document that can be resolved to the correct URL for the page whether the page has been moved or renamed. This is a feature that other CMS's have, for example in MODx you can have something like in your content
And the CMS will expand that to the correct URL for the page of that id. This allows the end user to change page URLs without having to update links throughout their site. This may be possible for any documents that use the referenceable mixin as they have a unique id.
Original discussion at https://github.com/symfony-cmf/SimpleCmsBundle/issues/77