Add VHM support to @search #508
Conversation
|
@sneridagh what's the status here? |
| vhm_physical_path = self.request.get('VirtualRootPhysicalPath') | ||
| if vhm_physical_path: | ||
| path = query['path'].get('query') | ||
| if path: |
There was a problem hiding this comment.
Could we change this to something along the lines of if vhm_physical_path and not path.startswith('/'.join(vhm_physical_path)):?
(Only prepend VirtualRootPhysicalPath if client didn't already supply it).
That would keep backwards compatibility with clients that for some reason already did know the Plone site ID (maybe communicated out-of-band), and still assist clients that don't know it.
There was a problem hiding this comment.
I thought about that. This will fail if you have a folder in the site root with the same id as the site. As it is not a dev who decides that it i did not implement it.
I don't think we should make this behavior optional and the old behavior the default. So if backwards compatibility is a blocker it should go into a 2.0alpha release.
There was a problem hiding this comment.
@csenger yeah, I thought about that case too. I was thinking that with Plone and Acquisition you're not going to have a good time naming an object the same as the Plone site ID - but you're right, it's possible, though probably rare.
I don't think we should make this behavior optional and the old behavior the default. So if backwards compatibility is a blocker it should go into a 2.0alpha release.
Yeah, I think you're right - offering two code paths here is probably more confusing than it helps 👍
There was a problem hiding this comment.
@lukasgraf @csenger 2.0.0 is around the corner anyways. I don't mind just including that one in a new major release as a breaking change. Though, if this is a breaking change, we have to document it in our upgrade guide: http://plonerestapi.readthedocs.io/en/latest/upgrade-guide.html
Solves issue #186
If called via VHM it will include the VirtualHostRoot path in a path search