In @vocabulary endpoint, call the vocabulary factory with the 'title'… #1807
Conversation
… field from the request form
|
@tiberiuichim thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
| @@ -86,7 +86,8 @@ def reply(self): | |||
| 404, "Not Found", f"The vocabulary '{vocabulary_name}' does not exist" | |||
| ) | |||
|
|
|||
| vocabulary = factory(self.context) | |||
| query = self.request.form.get("title", "") | |||
| vocabulary = factory(self.context, query=query) | |||
There was a problem hiding this comment.
@tiberiuichim This won't work with many vocabularies, which is why a lot of tests broke.
The IVocabularyFactory interface specifies that a vocab factory is called with a context, but not other parameters: https://github.com/zopefoundation/zope.schema/blob/master/src/zope/schema/interfaces.py#L1087
The addition of the query parameter seems to be something that was made up for the vocabulary factories in plone.app.vocabularies.principals without following any formal interface. This makes it hard to detect here how the factory is supposed to be called.
To really sort this out, we need to define a new interface like IQueryableVocabularyFactory. Then plone.app.vocabularies can define that these vocabulary factories implement that interface, and plone.restapi can be updated to pass the query to factories which provide that interface.
|
@davisagli
Low-tech approach doesn't work. |
Is there some mechanism to make sure that it is only sent in the request for the vocabularies that support it? |
|
No, the simple guard doesn't work, we have to implement your approach. I was hoping to limit the "damage" to plone.app.restapi, but it looks like it will need work in plone.app.vocabularies as well |
|
@tiberiuichim My approach is probably possible to do in plone.restapi only, if it imports and applies a custom interface to these vocabularies at import time, and checks for it at runtime. |
|
I'll try a different approach. I'll implement my own custom vocabulary, based on the p.a.v.Principals, which would properly use the serializer API approach in plone.restapi. This would mean that this PR wouldn't be needed anymore |
|
@tiberiuichim Okay, I'll close this for now |
… field from the request form
Fixes #1806
📚 Documentation preview 📚: https://plonerestapi--1807.org.readthedocs.build/