WIP Enhance relation field serialization for image content types #1442
Conversation
|
@reebalazs 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! |
✅ Deploy Preview for plone-restapi canceled.
|
reebalazs
force-pushed
the
ree-enhance-relation-field-for-images
branch
from
b3a2b81
to
608b8d3
Compare
- In better support of the preview image link - Reuses serialization from existing field serializers - Flexible adapation allows new types to be added in the future WIP until preview_image_link has been merged into plone.volto
reebalazs
force-pushed
the
ree-enhance-relation-field-for-images
branch
from
608b8d3
to
7adb2a4
Compare
There was a problem hiding this comment.
Take a look at my comments.
/cc @ericof @tisto @tiberiuichim I'd like to know your point of view too.
| @@ -18,6 +18,7 @@ parts = | |||
| develop = . | |||
| sources-dir = extras | |||
| auto-checkout = | |||
| plone.volto | |||
There was a problem hiding this comment.
We don't really need to tie this to the plone.volto change. So I'd remove this for the merge.
|
|
||
|
|
||
| @adapter(IImage, IRelationChoice, IDexterityContent, Interface) | ||
| class ImageRelationObjectSerializer(FieldRelationObjectSerializer): |
There was a problem hiding this comment.
Interesting approach, wondering how this will scale in the case that we have a custom content type and we want to return, let's say two images.
There was a problem hiding this comment.
or a combination of fields. Not sure if make a 1:1 content type => field serialised I would like to include in the final serialization is a good choice.
|
@tiberiuichim @avoinea this is what I was talking about yesterday. The idea would be that once we have this Relation serializers, we could use them in the smart fields (href, teaser's one) so if what's behind it's really an image, then pull off the enhanced serialization. |
There was a problem hiding this comment.
I am not sure I understand the motivation for this feature.
If I understand correctly, it adds a way to customize the serialization of a related item from a RelationChoice field, with the ability to use a different adapter based on what type of object is related.
But it seems strange to me that we would want different fields included in the serialization when an object was found via a RelationChoice field compared to some other way like a catalog query or getting the actual content path.
For the use case of images in particular, I think it is now possible to include image scale sizes in the image summary serialization generated from catalog brains, and that could be done whether or not the image was referenced from a relation field.
|
+1 - this would speed up usage of referenced images a lot. The particular implementation is maybe not exact how it should be. The core idea form Beethoven sprint is to have a general adapter in Plone core on field level. This code here then should call it or retrieve the data from the catalog. |
WIP until preview_image_link has been merged into plone.volto