FluidTYPO3 / vhs

TYPO3 extension VHS: Fluid ViewHelpers
https://fluidtypo3.org
Other
190 stars 229 forks source link

FalViewHelper throw an error when the record is hidden #1714

Open olinox14 opened 3 years ago

olinox14 commented 3 years ago

Issue: an exception No record was found. The "record" or "uid" argument must be specified. is thrown when using FalViewHelper within an hidden content.
To reproduce, create a template:

{namespace v=FluidTYPO3\Vhs\ViewHelpers}
{namespace flux=FluidTYPO3\Flux\ViewHelpers}

<div xmlns="http://www.w3.org/1999/xhtml"
     xmlns:flux="http://fedext.net/ns/flux/ViewHelpers"
     xmlns:v="http://fedext.net/ns/vhs/ViewHelpers"
     xmlns:f="http://typo3.org/ns/fluid/ViewHelpers">

    <f:section name="Configuration">
        <flux:form id="Carousel" label="My Carousel" extensionName="my_extension">
            <flux:field.inline.fal name="images" minItems="2" maxItems="24"/>
        </flux:form>
    </f:section>
    <f:section name="Preview">
            <f:for each="{v:content.resources.fal(field: 'images', uid: '{record.uid}')}"
                   as="image"
                   iteration="iterator">
                    <f:image treatIdAsReference="1"
                             src="{image.id}"
                             title="{image.title}"
                             alt="{image.alternative}"
                    />
            </f:for>
    </f:section>
</div>

Then, add a content of this type to a page in the Typo3 Backend, and hide this content.

An exception is thrown, because the FluidTYPO3\Vhs\ViewHelpers\Resource\Record\AbstractRecordResourceViewHelper->getRecord() can not find any matching record (default restrictions apply to querybuilder here)

A quick workaround is to replace:
{v:content.resources.fal(field: 'images', uid: '{record.uid}')} By
{v:content.resources.fal(field: 'images', record: '{record}')}

However, this remains a bug, and I suggest to remove the 'HiddenRestriction' from the querybuilder in the getRecord() method. Maybe StartTimeRestriction and EndTimeRestriction shall be removed too.

NamelessCoder commented 3 years ago

Since this ViewHelper is designed for frontend usage, it is not sensible to remove the display restrictions (neither hidden nor others). The right solution is actually the workaround you already came up with - passing the record so the ViewHelper does not attempt to load it. In fact this is also recommended practice in frontend for any use case where you have access to the page/content record (Flux will attempt to insert this into template variables but non-Flux usage may not have access to the record). This is also better for performance since you don't cause an unnecessary SQL query (actually, one additional query per relation).

olinox14 commented 3 years ago

I use this VH in the backend to display a preview of the carousel images, shouldn't I? I agree on the fact that it's obviously better to directly pass the record, however I don't think that the sole fact of passing the uid as parameter should be able to crash the backend because of an hidden content... The exception could be catch and managed in the viewhelper, or as I said, the restrictions could be lifted?