adobe / asset-share-commons

A modern, open-source asset share reference implementation built on Adobe Experience Manager (AEM)
https://opensource.adobe.com/asset-share-commons/
Apache License 2.0
88 stars 107 forks source link

For details page components like image, empty placeholder template does not work if placeholders are missing #182

Closed kanika1901kapoor closed 6 years ago

kanika1901kapoor commented 6 years ago

Pages like /en/home/details/image.html, /en/home/details/video.html resolve to placeholder asset from page properties. If that is not present, they fallback to default placeholder at /apps/asset-share-commons/resources/placeholder.png .

Expected - If the default placeholder is not present , a warning should be thrown in logs and the empty placeholder template as mentioned in core/wcm/components/commons/v1/templates.html should be displayed in author mode.

Actual - An exception is seen in logs and in author mode, the component is not visible for authoring unless it is hovered over.

> Caused by: org.apache.sling.scripting.sightly.java.compiler.SightlyJavaCompilerException: data-sly-call: expression evaluates to null. at org.apache.sling.scripting.sightly.java.compiler.RenderUnit.callUnit(RenderUnit.java:70)

Fix - the sly tags with data-sly-use.placeholderTemplate needs to wrap properly

davidjgonzalez commented 6 years ago

I think we need to be careful about this one (IIRC this was brought up previously, though dont recall if logged) .. the reason for this is:

The asset resolution (ie. using the placeholder as fallback) drives ~all components on the Asset Details page. So... on author, it might nice to display some clean error handling, but on AEM Publish, if the requested asset cannot be found, it should return a non-200 response as its an invalid request.

My hesitation is leaking a bunch of conditional (auth v pub) complexity across all these components to resolve this often "1 time" issue.

Perhaps a solution would be to show a Resource Status Provider [1] Bar on AEM Author pages when it's not configured altering the viewer that the search page requires configuring..?

[1] https://github.com/Adobe-Consulting-Services/acs-aem-samples/blob/master/bundle/src/main/java/com/adobe/acs/samples/resourcestatus/impl/SampleEditorResourceStatusProvider.java

kanika1901kapoor commented 6 years ago

@davidjgonzalez - I was referring to a minor issue - the empty placeholder (/apps/core/wcm/components/commons/v1/templates.html) not getting displayed if you do not have a placeholder configured in page properties

Expected - Empty placeholders image

Actual - This is how the page (/content/asset-share-commons/en/light/details/image.html) gets displayed as of today image

davidjgonzalez commented 6 years ago

I think using Resource Status Providers is a simple (and most importantly unobtrusive) solution to this ... Modifying the core logic of components to handle these cases seems like adding unnecessary complexity for something that should be a "one time" issue (ie. once you fix these configurations, they probably wont happen again).

Search Page - Missing Results component image

Asset Details Page - Missing Placeholder Asset configuration image