ToImagePreview errors if source empty - fails transform

pkbullock commented 5 years ago

Problem Area

[ ] Page Transformation: Error during the setup/use of the Page Transformation UI solution (did you check our troubleshooting guide?)
[X] Page Transformation: Error during the use of page transformation from PnP PowerShell
[ ] Page Transformation: Error during the use of page transformation from .Net
[ ] Page Transformation: Page is not looking correct after transformation
[ ] Modernization Scanner: something went wrong...

Expected or Desired Behavior

Handles empty or no content if the source field used in the function is null or empty and defaults to the standard /_layouts/15/images/sitepagethumbnail.png file.

Observed Behavior

In this scenario, if the field is empty the transform fails e.g. returns no transform path. In addition, the logging does NOT pick this up and no logs are produced even when running cmdlet for saving logs.

I discovered this by trial and error with the mapping then discovered field didn't already contain a value.

Steps to Reproduce

Create a page with a empty field and use article: https://docs.microsoft.com/en-us/sharepoint/dev/transform/modernize-userinterface-site-pages-model-publishing#can-i-control-the-page-preview-image

to map to field.

Transform. Nothing is returned. No errors and no resulting transform URL.

For those that might encounter this, workaround for now is two mapping files and switch between them removing the ToPreviewImage mapping when source metadata is empty.

jansenbe commented 5 years ago

Hi @pkbullock,

Will do some testing, but the idea is that if there's no value set by page transformation (BannerImageUrl = "") then the default logic in the page transformation API tries to come up with a banner image.

So you're saying the transformation ends in this case without any error?

pkbullock commented 5 years ago

Yeap, in PnP Powershell just returns nothing after running this, couldn't get the log to write when using SkipLogFlush then the Save-PnPConversionLog to find out why. Might of missing something but the error appeared to be silent fail.

jansenbe commented 5 years ago

Did try to repro this, but without success. When the source field value is null or empty the field value is not considered in the metadata handling: https://github.com/SharePoint/sp-dev-modernization/blob/dev/Tools/SharePoint.Modernization/SharePointPnP.Modernization.Framework/Publishing/PublishingMetadataTransformator.cs#L112

In my test the result of providing an empty value is that the preview image will be set to it's default value (page header image in this case).

Could it be that there's some malformed value in your input?

pkbullock commented 5 years ago

I'll get back to you on this one with more detail.

jansenbe commented 5 years ago

Hey @pkbullock ,

Did you manage to still repro this? It's still listed for the September release, but without clear issue repro not much can be done.

pkbullock commented 5 years ago

Hi @jansenbe, sorry just back from holiday, can we move this fix to Oct release? I need to do more investigate but likely wont make it for Sept release.

pkbullock commented 5 years ago

@jansenbe if i discover why/if then i'm happy to submit a PR for a fix 😊

jansenbe commented 5 years ago

Moved to October release

pkbullock commented 5 years ago

I'm going to close this one, i didn't get a chance to repo this on clients systems, the project has come to an end.

pnp / modernization