plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
447 stars 607 forks source link

Listing block shows the current object #1166

Closed sneridagh closed 4 years ago

sneridagh commented 4 years ago

It seems odd that the current object where the listing lives shows itself as a result.

Either we make the filtering in the endpoint, so we never return the context (then we should make the @querystringsearch endpoint context aware).

Or we just patch the listing block for doing that.

sneridagh commented 4 years ago

@pnicolli what do you think?

tisto commented 4 years ago

I'd patch the listing block and filter that out on React level.

pnicolli commented 4 years ago

@sneridagh @tisto Actually I feel like it would be better to change p.a.querystring or restapi, for a couple reasons:

On the other hand, this would be a breaking change.

tisto commented 4 years ago

@sneridagh so the plan would be to make querystring-search context aware, so we can automatically filter out the current object from the search results, correct? We would need to:

a) create a PR to plone.restapi to make querystring-search context aware (not a breaking change since we just add functionality here) b) create a PR to Volto to call querystring-search on the context where it is currently used (the only difference would be to filter out the current context in the results, right? I mean we do not automatically take the path into account when calling querystring-search, is that correct?)

tisto commented 4 years ago

Note: we will go with the context-aware querystring search. the only thing it does it filter out the context. we live with the fact that you can call it context-aware but that you have to pass a path to actually take the context/path into account when doing the search. we should document that properly.

pnicolli commented 4 years ago

I think that making it context aware would also be helpful for relative path searches. At the moment we have this hack to make that work https://github.com/plone/volto/blob/master/src/actions/querystringsearch/querystringsearch.js#L12

tisto commented 4 years ago

@pnicolli just to be clear. context-aware means just that we filter out the context object itself. this has no consequences for the query path (relative or not). If we would make querystring-search context aware in that sense, we would duplicate functionality between the context and the query. Say you call querystring-search on a context and provide a location path criterion. Which one would win?

sneridagh commented 4 years ago

Support for this has been added, ZCatalog version should be pinned to 5.1 or above.