Add snippet_selection Resolver #32

TheCadien · 2020-06-05T19:39:37Z

See #3

amb-jarek · 2020-06-09T11:17:27Z

@TheCadien thx! It solved big problem in my project.
I just noticed one place which could be improved. I mean image urls. Right now if snippet contains bunch of images then each of them is returned as a list of all available formats, like:

                        "thumbnails": {
                            "sulu-400x400": "/uploads/media/sulu-400x400/02/22-l2.png?v=3-0",
                            "sulu-400x400.png": "/uploads/media/sulu-400x400/02/22-l2.png?v=3-0",
                            "sulu-400x400.jpg": "/uploads/media/sulu-400x400/02/22-l2.jpg?v=3-0",
                            "sulu-400x400.gif": "/uploads/media/sulu-400x400/02/22-l2.gif?v=3-0",
                            "sulu-400x400.webp": "/uploads/media/sulu-400x400/02/22-l2.webp?v=3-0",
                            "sulu-260x": "/uploads/media/sulu-260x/02/22-l2.png?v=3-0",
                            "sulu-260x.png": "/uploads/media/sulu-260x/02/22-l2.png?v=3-0",
                            "sulu-260x.jpg": "/uploads/media/sulu-260x/02/22-l2.jpg?v=3-0",
                            "sulu-260x.gif": "/uploads/media/sulu-260x/02/22-l2.gif?v=3-0",
                            "sulu-260x.webp": "/uploads/media/sulu-260x/02/22-l2.webp?v=3-0",
                            "sulu-170x170": "/uploads/media/sulu-170x170/02/22-l2.png?v=3-0",
                            "sulu-170x170.png": "/uploads/media/sulu-170x170/02/22-l2.png?v=3-0",
                            "sulu-170x170.jpg": "/uploads/media/sulu-170x170/02/22-l2.jpg?v=3-0",
                            "sulu-170x170.gif": "/uploads/media/sulu-170x170/02/22-l2.gif?v=3-0",
                            "sulu-170x170.webp": "/uploads/media/sulu-170x170/02/22-l2.webp?v=3-0",
                            "sulu-100x100-inset": "/uploads/media/sulu-100x100-inset/02/22-l2.png?v=3-0",
                            "sulu-100x100-inset.png": "/uploads/media/sulu-100x100-inset/02/22-l2.png?v=3-0",
                            "sulu-100x100-inset.jpg": "/uploads/media/sulu-100x100-inset/02/22-l2.jpg?v=3-0",
                            "sulu-100x100-inset.gif": "/uploads/media/sulu-100x100-inset/02/22-l2.gif?v=3-0",
                            "sulu-100x100-inset.webp": "/uploads/media/sulu-100x100-inset/02/22-l2.webp?v=3-0",
                            "sulu-50x50": "/uploads/media/sulu-50x50/02/22-l2.png?v=3-0",
                            "sulu-50x50.png": "/uploads/media/sulu-50x50/02/22-l2.png?v=3-0",
                            "sulu-50x50.jpg": "/uploads/media/sulu-50x50/02/22-l2.jpg?v=3-0",
                            "sulu-50x50.gif": "/uploads/media/sulu-50x50/02/22-l2.gif?v=3-0",
                            "sulu-50x50.webp": "/uploads/media/sulu-50x50/02/22-l2.webp?v=3-0",
                            "sulu-25x25": "/uploads/media/sulu-25x25/02/22-l2.png?v=3-0",
                            "sulu-25x25.png": "/uploads/media/sulu-25x25/02/22-l2.png?v=3-0",
                            "sulu-25x25.jpg": "/uploads/media/sulu-25x25/02/22-l2.jpg?v=3-0",
                            "sulu-25x25.gif": "/uploads/media/sulu-25x25/02/22-l2.gif?v=3-0",
                            "sulu-25x25.webp": "/uploads/media/sulu-25x25/02/22-l2.webp?v=3-0",
                            "sulu-40x40": "/uploads/media/sulu-40x40/02/22-l2.png?v=3-0",
                            "sulu-40x40.png": "/uploads/media/sulu-40x40/02/22-l2.png?v=3-0",
                            "sulu-40x40.jpg": "/uploads/media/sulu-40x40/02/22-l2.jpg?v=3-0",
                            "sulu-40x40.gif": "/uploads/media/sulu-40x40/02/22-l2.gif?v=3-0",
                            "sulu-40x40.webp": "/uploads/media/sulu-40x40/02/22-l2.webp?v=3-0",
                            "sulu-240x": "/uploads/media/sulu-240x/02/22-l2.png?v=3-0",
                            "sulu-240x.png": "/uploads/media/sulu-240x/02/22-l2.png?v=3-0",
                            "sulu-240x.jpg": "/uploads/media/sulu-240x/02/22-l2.jpg?v=3-0",
                            "sulu-240x.gif": "/uploads/media/sulu-240x/02/22-l2.gif?v=3-0",
                            "sulu-240x.webp": "/uploads/media/sulu-240x/02/22-l2.webp?v=3-0",
                            "sulu-400x400-inset": "/uploads/media/sulu-400x400-inset/02/22-l2.png?v=3-0",
                            "sulu-400x400-inset.png": "/uploads/media/sulu-400x400-inset/02/22-l2.png?v=3-0",
                            "sulu-400x400-inset.jpg": "/uploads/media/sulu-400x400-inset/02/22-l2.jpg?v=3-0",
                            "sulu-400x400-inset.gif": "/uploads/media/sulu-400x400-inset/02/22-l2.gif?v=3-0",
                            "sulu-400x400-inset.webp": "/uploads/media/sulu-400x400-inset/02/22-l2.webp?v=3-0",
                            "sulu-100x100": "/uploads/media/sulu-100x100/02/22-l2.png?v=3-0",
                            "sulu-100x100.png": "/uploads/media/sulu-100x100/02/22-l2.png?v=3-0",
                            "sulu-100x100.jpg": "/uploads/media/sulu-100x100/02/22-l2.jpg?v=3-0",
                            "sulu-100x100.gif": "/uploads/media/sulu-100x100/02/22-l2.gif?v=3-0",
                            "sulu-100x100.webp": "/uploads/media/sulu-100x100/02/22-l2.webp?v=3-0",
                           ...
                        },

If you have lots of images in your snippet then response payload size increases singnificantly.

Other resolvers deal with media files like this:

itemSource": {
                    "id": 16,
                    "locale": "pl",
                    "collection": 3,
                    "size": 2354045,
                    "mimeType": "image/jpeg",
                    "title": "1full hd",
                    "version": 1,
                    "subVersion": 0,
                    "name": "1full hd.jpg",
                    "type": {
                        "name": "image",
                        "id": 2
                    },
                    "isImage": true,
                    "isVideo": false,
                    "isAudio": false,
                    "isDocument": false,
                    "publishLanguages": [],
                    "contentLanguages": [],
                    "tags": [],
                    "url": "/media/16/download/1full%20hd.jpg?v=1",
                    "changed": "2020-05-07T11:07:44",
                    "changer": "Adam Ministrator",
                    "created": "2020-05-07T11:07:44",
                    "creator": "Adam Ministrator",
                    "properties": [],
                    "categories": [],
                    "targetGroups": [],
                    "formatUri": "/uploads/media/{format}/06/16-1full%20hd.jpg?v=1-0"
                },

Using formatUri property consumer may fetcha any format of the image.

But even though it is really importand PR for me and I can finally use snippets in my headless SULU. Thx for that!

TheCadien · 2020-06-09T11:20:11Z

@amb-jarek I will check that,I have to do unit tests anyway until we can finish that MR

alexander-schranz · 2020-06-09T11:24:24Z

Content/ContentTypeResolver/SnippetSelectionResolver.php

+            return new ContentView([]);
+        }
+
+        $content = $this->snippetResolver->resolve($data, $property, $locale, $attributes);


I don't think that we can use the SnippetResolver here as we need to resolve every property itself e.g. the media need to be resolved over the MediaResolver and so on, but could not have a deeper look into just the first think which did come to my mind when scrolled over this PR.

i think your right !
change a lot to add the correct structure Resolver

TheCadien · 2020-06-11T19:46:29Z

@amb-jarek think the image output is much better now

amb-grzegorz · 2020-06-16T11:06:19Z

@TheCadien Did you test this solution with smart content?
When I'm trying to use these changes in situation where:

I have a snippet with smart content
Content which is matched have field "single_media_selection"
I get error says "second argument of array_key_exists() has to be array, but got string.

I found that error is throwed by from SingleMediaSelectionResolver line: 42 where variable $data is string instead of array.

alexandersch · 2020-07-10T07:42:50Z

Hi, I've tested these changes and it works for normal snippet selection. But when you use a default snippet it does not resolve the default snippet unfortunately.

TheCadien · 2020-07-10T14:52:31Z

@alexandersch could you explain what you mean with "default snippet" and step by step way to reproduce ? Thanks ! :)

alexandersch · 2020-07-10T18:57:16Z

Yes ofcourse.

I've created a default snippet template based on the documentation. https://docs.sulu.io/en/2.0/cookbook/default-snippets.html. For example a 'footer' snippet with area 'footer'.

Next, I've defined a 'snippet_selection' property in my page template. With parameters type='default' and default='footer'.

Final step is to create a new footer snippet in the admin interface and then select it as a default snippet in the webspace's default snippets settings.

Normally you would then get at least the default snippet in your twig template when you do a dump of 'content.footer' for example.

I've made some changes to the StructureResolver as a workaround, but maybe that is not the correct solution for this.

niklasnatter · 2020-07-13T12:00:37Z

hey @TheCadien - first of all, thanks for your work on this!
I am going to have a look at the default snippet thing today. Hope it is okay for you if I build upon your existing code 🙂

TheCadien · 2020-07-13T12:03:50Z

@nnatter sure !

niklasnatter · 2020-07-13T15:10:27Z

@TheCadien Did you test this solution with smart content?
When I'm trying to use these changes in situation where:

I have a snippet with smart content

Content which is matched have field "single_media_selection"
I get error says "second argument of array_key_exists() has to be array, but got string.

I found that error is throwed by from SingleMediaSelectionResolver line: 42 where variable $data is string instead of array.

Just tested this. The problem is not related to this PR. It would be nice if you could create new issue if the problem still affects you.

niklasnatter · 2020-07-13T15:19:23Z

Just pushed some changes that should hopefully fix the default-snippet issue and added some test cases. Would be nice to get some feedback regarding to the default-snippet before merging it 🙂

Thanks again for working on this @TheCadien

alexander-schranz · 2020-07-13T15:20:11Z

I found that error is throwed by from SingleMediaSelectionResolver line: 42 where variable $data is string instead of array.

@nnatter is this not what was fixed in #31?

niklasnatter · 2020-07-13T15:24:10Z

@nnatter is this not what was fixed in #31?

Unfortunately only partially. It looks like resolving properties via the properties param is not working correctly for the page_selection and the smart_content to me. But its definitely not related to this PR 🙂

alexandersch · 2020-07-13T16:22:47Z

I'll test the default snippet bit tonight.

EDIT:
Somehow I'm getting this error for two of my templates, not sure if this is related?

InvalidArgumentException  TemplateNotFoundException
Unknown child "article" in structure "default". Children: "title", "url", "header_image"

If I add the article property to these templates, it works as expected. The default snippet is outputted.

niklasnatter · 2020-07-15T13:57:46Z

@alexandersch Can you provide some context where this exception is thrown?

alexander-schranz · 2020-07-23T14:34:55Z

@TheCadien @nnatter Thank you for providing this 👍

alexander-schranz reviewed Jun 9, 2020

View reviewed changes

TheCadien and others added 5 commits July 13, 2020 15:49

Add snippet_selection Resolver

a39950d

Add own Resolver function

dbd1e8b

Add simple Unit tests

02f600d

Update PHP CS

13c94d1

Implement default snippet handling

18b38ff

Add test cases for SnippetSelectionResolver

b12f112

alexander-schranz added the Feature New functionality not yet included in the Bundle label Jul 13, 2020

niklasnatter assigned alexander-schranz Jul 23, 2020

alexander-schranz merged commit f556be6 into sulu:master Jul 23, 2020

wachterjohannes mentioned this pull request Jul 24, 2020

Missing ContentType resolver #3

Open

50 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add snippet_selection Resolver #32

Add snippet_selection Resolver #32

TheCadien commented Jun 5, 2020

amb-jarek commented Jun 9, 2020 •

edited

Loading

TheCadien commented Jun 9, 2020 •

edited

Loading

alexander-schranz Jun 9, 2020

TheCadien Jun 11, 2020 •

edited

Loading

TheCadien commented Jun 11, 2020

amb-grzegorz commented Jun 16, 2020

alexandersch commented Jul 10, 2020

TheCadien commented Jul 10, 2020

alexandersch commented Jul 10, 2020 •

edited

Loading

niklasnatter commented Jul 13, 2020

TheCadien commented Jul 13, 2020

niklasnatter commented Jul 13, 2020

niklasnatter commented Jul 13, 2020

alexander-schranz commented Jul 13, 2020

niklasnatter commented Jul 13, 2020

alexandersch commented Jul 13, 2020 •

edited

Loading

niklasnatter commented Jul 15, 2020

alexander-schranz commented Jul 23, 2020

Add snippet_selection Resolver #32

Add snippet_selection Resolver #32

Conversation

TheCadien commented Jun 5, 2020

amb-jarek commented Jun 9, 2020 • edited Loading

TheCadien commented Jun 9, 2020 • edited Loading

alexander-schranz Jun 9, 2020

Choose a reason for hiding this comment

TheCadien Jun 11, 2020 • edited Loading

Choose a reason for hiding this comment

TheCadien commented Jun 11, 2020

amb-grzegorz commented Jun 16, 2020

alexandersch commented Jul 10, 2020

TheCadien commented Jul 10, 2020

alexandersch commented Jul 10, 2020 • edited Loading

niklasnatter commented Jul 13, 2020

TheCadien commented Jul 13, 2020

niklasnatter commented Jul 13, 2020

niklasnatter commented Jul 13, 2020

alexander-schranz commented Jul 13, 2020

niklasnatter commented Jul 13, 2020

alexandersch commented Jul 13, 2020 • edited Loading

niklasnatter commented Jul 15, 2020

alexander-schranz commented Jul 23, 2020

amb-jarek commented Jun 9, 2020 •

edited

Loading

TheCadien commented Jun 9, 2020 •

edited

Loading

TheCadien Jun 11, 2020 •

edited

Loading

alexandersch commented Jul 10, 2020 •

edited

Loading

alexandersch commented Jul 13, 2020 •

edited

Loading