solspace / craft-freeform

Freeform for Craft: The most reliable form builder that's ready for wherever your project takes you.
https://docs.solspace.com/craft/freeform/v5/
Other
47 stars 60 forks source link

[5.1] Cannot assign string to property Solspace\Freeform\Fields\AbstractField::$id of type ?int #1214

Closed rob-baker-ar closed 5 months ago

rob-baker-ar commented 6 months ago

Describe the bug or issue you're experiencing

I get a TypeError exception of "Cannot assign string to property Solspace\Freeform\Fields\AbstractField::$id of type ?int" when trying to call the render() method (with no parameters) on a "Freeform form" field attached to a Matrix block.

This is on an install that has just been upgraded from v4. This is in our DEV environment - Freeform v5 has not been put live yet.

I am not seeing any issues in the CMS / control panel screens for the form.

The form is fairly simple (9 text fields, 1 email, 1 drop down, 1checkboxes field, a hidden field and a button), no integrations,

Steps to reproduce

  1. Un-altered template code from v4 install that worked fine previously (relevant part: {{ formField.render() }})
  2. Upgrade Freeform to v5
  3. Attempt to view same form on the front-end

Expected behavior

The form renders as before.

Versions

Redacted Stack Trace

TypeError: Cannot assign string to property Solspace\Freeform\Fields\AbstractField::$id of type ?int in [...]/vendor/solspace/craft-freeform/packages/plugin/src/Library/Processors/AbstractOptionProcessor.php:87
Stack trace:
#0 [...]/vendor/solspace/craft-freeform/packages/plugin/src/Library/Processors/AbstractOptionProcessor.php(87): ReflectionProperty->setValue()
#1 [...]/vendor/solspace/craft-freeform/packages/plugin/src/Library/Processors/FieldRenderOptionProcessor.php(20): Solspace\Freeform\Library\Processors\AbstractOptionProcessor->processPropertyValue()
#2 [...]/vendor/solspace/craft-freeform/packages/plugin/src/Bundles/Fields/FieldRenderOptionsBundle.php(149): Solspace\Freeform\Library\Processors\FieldRenderOptionProcessor->processProperties()
#3 [internal function]: Solspace\Freeform\Bundles\Fields\FieldRenderOptionsBundle->onSetParameters()
#4 [...]/vendor/yiisoft/yii2/base/Event.php(312): call_user_func()
#5 [...]/vendor/solspace/craft-freeform/packages/plugin/src/Fields/AbstractField.php(520): yii\base\Event::trigger()
#6 [...]/vendor/solspace/craft-freeform/packages/plugin/src/Fields/AbstractField.php(277): Solspace\Freeform\Fields\AbstractField->setParameters()
#7 [...]/vendor/twig/twig/src/Extension/CoreExtension.php(1635): Solspace\Freeform\Fields\AbstractField->renderInput()
#8 [...]/vendor/craftcms/cms/src/helpers/Template.php(129): twig_get_attribute()
#9 [...]/storage/runtime/compiled_templates/71/7130c5ebf53efcdf7f2c5ff502e309e8.php(414): craft\helpers\Template::attribute()
#10 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_fca0970c6460cdea21bbef67f2ac03ef->doDisplay()
#11 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#12 [...]/vendor/twig/twig/src/Template.php(379): Twig\Template->display()
#13 [...]/vendor/twig/twig/src/TemplateWrapper.php(38): Twig\Template->render()
#14 [...]/vendor/craftcms/cms/src/web/View.php(576): Twig\TemplateWrapper->render()
#15 [...]/vendor/solspace/craft-freeform/packages/plugin/src/Services/FormsService.php(346): craft\web\View->renderString()
#16 [...]/vendor/solspace/craft-freeform/packages/plugin/src/Form/Form.php(597): Solspace\Freeform\Services\FormsService->renderFormTemplate()
#17 [...]/vendor/twig/twig/src/Extension/CoreExtension.php(1635): Solspace\Freeform\Form\Form->render()
#18 [...]/vendor/craftcms/cms/src/helpers/Template.php(129): twig_get_attribute()
#19 [...]/storage/runtime/compiled_templates/28/2896052b14ba5fd8cd31d153ab21dd58.php(64): craft\helpers\Template::attribute()
#20 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_c3046b988ca203d6ec8e61e7bb3e7b06->doDisplay()
#21 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#22 [...]/storage/runtime/compiled_templates/37/37f25de7ccf10dd92c1625eaad426917.php(446): Twig\Template->display()
#23 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_988bda3b292482bcd0d15b51a1ed8b61->doDisplay()
#24 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#25 [...]/storage/runtime/compiled_templates/e5/e52208b4676f1b0b6dc8206ec691d57b.php(61): Twig\Template->display()
#26 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_5c9bf1e69279694ae5cf7845bc32b420->doDisplay()
#27 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#28 [...]/storage/runtime/compiled_templates/de/def0afe9c340c382d5375b54822cb6ae.php(331): Twig\Template->display()
#29 [...]/vendor/twig/twig/src/Template.php(171): __TwigTemplate_aa7cb353ec076aca5732d7402bc4277f___834627292->block_main_content()
#30 [...]/storage/runtime/compiled_templates/9e/9e08e97366c5debc7376a0a1849c85bc.php(63): Twig\Template->displayBlock()
#31 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_bea73408f564b96f5a3fade28013a6d7->doDisplay()
#32 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#33 [...]/vendor/twig/twig/src/TemplateWrapper.php(45): Twig\Template->display()
#34 [...]/storage/runtime/compiled_templates/de/def0afe9c340c382d5375b54822cb6ae.php(282): Twig\TemplateWrapper->display()
#35 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_aa7cb353ec076aca5732d7402bc4277f___834627292->doDisplay()
#36 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#37 [...]/storage/runtime/compiled_templates/de/def0afe9c340c382d5375b54822cb6ae.php(103): Twig\Template->display()
#38 [...]/vendor/twig/twig/src/Template.php(171): __TwigTemplate_aa7cb353ec076aca5732d7402bc4277f->block_content()
#39 [...]/storage/runtime/compiled_templates/8d/8dfe94ea97e5ba0ba9fc9006f09db26f.php(220): Twig\Template->displayBlock()
#40 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_fe0a1bf6b8428849a3142d0d21699508->doDisplay()
#41 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#42 [...]/storage/runtime/compiled_templates/de/def0afe9c340c382d5375b54822cb6ae.php(85): Twig\Template->display()
#43 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_aa7cb353ec076aca5732d7402bc4277f->doDisplay()
#44 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#45 [...]/vendor/twig/twig/src/TemplateWrapper.php(45): Twig\Template->display()
#46 [...]/storage/runtime/compiled_templates/96/967d447b9d4ace441a3fdaf11f2e7428.php(44): Twig\TemplateWrapper->display()
#47 [...]/vendor/twig/twig/src/Template.php(394): __TwigTemplate_8800b5c23dca32002e3572e9fc3f75d6->doDisplay()
#48 [...]/vendor/twig/twig/src/Template.php(367): Twig\Template->displayWithErrorHandling()
#49 [...]/vendor/twig/twig/src/Template.php(379): Twig\Template->display()
#50 [...]/vendor/twig/twig/src/TemplateWrapper.php(38): Twig\Template->render()
#51 [...]/vendor/twig/twig/src/Environment.php(280): Twig\TemplateWrapper->render()
#52 [...]/vendor/craftcms/cms/src/web/View.php(482): Twig\Environment->render()
#53 [...]/vendor/craftcms/cms/src/web/View.php(535): craft\web\View->renderTemplate()
#54 [...]/vendor/craftcms/cms/src/web/TemplateResponseFormatter.php(57): craft\web\View->renderPageTemplate()
#55 [...]/vendor/yiisoft/yii2/web/Response.php(1100): craft\web\TemplateResponseFormatter->format()
#56 [...]/vendor/craftcms/cms/src/web/Response.php(337): yii\web\Response->prepare()
#57 [...]/vendor/yiisoft/yii2/web/Response.php(340): craft\web\Response->prepare()
#58 [...]/vendor/yiisoft/yii2/base/Application.php(390): yii\web\Response->send()
#59 [...]/web/index.php(11): yii\base\Application->run()
#60 {main}
rob-baker-ar commented 6 months ago

This issue also occurs if I leave out the fom field form the Matrix block and call this directly:

{{ freeform.form("[formHandle]").render() }}
kjmartens commented 6 months ago

Hi @rob-baker-ar,

Sorry for the delay and the trouble you're experiencing.

I have tested this out on my end, and it seems to work alright. I am using a sample Freeform formatting template like Flexbox or Basic Light and the template code I have for the entry + Matrix field is:

<div class="entry">
    <h1>{{ entry.title }}</h1>
    {{ entry.body }}

    {% for block in entry.myMatrixField.all() %}

        {% if block.type == "form" %}

            <h3>{{ block.myFreeformForm.name }}</h3>
            {{ block.myFreeformForm.render }}

            <p>{{ block.someOtherField }}</p>

        {% elseif block.type == "whatever" %}

            <p>Whatever</p>

        {% endif %}

    {% endfor %}
</div>

It displays correctly and seems to submit correctly.

Are you doing something different from what I am doing? And does it work for you if you adjust your template code to something similar to mine and use a sample Freeform formatting template such as Flexbox? 🙂

My best guess is that you are using a custom formatting template and something in there needs to be adjusted. If that's the case, can you please share it with me and I can help with that? 🙂

rob-baker-ar commented 6 months ago

Hi @kjmartens,

Thanks for the update.

If I use a built in / example template for rendering the form, it works OK. It has been hard to narrow down exactly what is at the rout of this due to #1224 but I think I narrowed it down:

In my (old) template there was something like this:

{% for row in form %}
    ...
    {% for field in row %}
        ...
        {{ field.renderInput({
            id: "field_id",
            class: "field_class"
        }) }}
        ...
    {% endfor %}
    ...
{% endfor %}

This generates the previously referred to error. I have been trying to work out what options to pass in to that method to replicate the old functionality as it's not 100% clear from the code or docs and came up with this:

{% for row in form %}
    ...
    {% for field in row %}
        ...
        {{ field.renderInput({
            attributes: {
                input: {
                    id: "field_id",
                    class: "field_class"
                }
            }
        }) }}
        ...
    {% endfor %}
    ...
{% endfor %}

But this is not 100% working either. Seems to behave differently depending on whether the form is rendered from a standard Twig context or from a controller which delivers the HTML via AJAX to the front-end (some forms we load just in time when users are scrolling through long pages, which helps with DOM complexity and CSRF as we use front-end static caching).

I realise this is not necessarily a "normal" implementation / approach, but it works (worked?) in v4 and suited us quite well. I may have to think again on how this bit works.

The PHP code to render the form in the AJAX context looks like:

$options = []; // these options are in fact identical to those passed in when in the Twig context
$form = Freeform::getInstance()->forms->getFormByHandle($form_handle);
$html = (string) $form->render($options);
kjmartens commented 6 months ago

Hi @rob-baker-ar,

I have tested the Freeform 5 approach like this, and it works correctly in my testing:

 {{ field.renderInput({
      attributes: {
          input: {
              id: "zesty",
              novalidate: true,
              class: "input-element",
          },
      }
 }) }}

However, you mention you're using AJAX and not a normal implementation. Can I ask, are you using a formatting template for your form? Or is this coded directly into the (regular) template that the form is being loaded into?

rob-baker-ar commented 6 months ago

@kjmartens The snippet above is in a custom formatting template - defined in Freeform > Settings > Formatting Templates. My code does not interfere with this, so it should be using the same formatting template for rendering in both contexts.

The important bits from the controller I have that renders the form for AJAX:

v4 version:

$form_model = Freeform::getInstance()->forms->getFormByHandle($form_handle);
$form = $form_model->getForm();
$html = $form->render($options);

v5 version

$form = Freeform::getInstance()->forms->getFormByHandle($form_handle);
$html = $form->render($options);

$options contains dynamicNotification, submitClass & overrideValues for v4 and fields to do the same thing in v5.

kjmartens commented 6 months ago

Thanks for the additional information, @rob-baker-ar. I've queued this up for @gustavs-gutmanis to have a peek at and see what insight he can offer here. 🙂

kjmartens commented 6 months ago

@rob-baker-ar,

Is it possible you still are calling id directly instead of through attributes somewhere else in the template? Is there any chance we could see your complete formatting template and controller? 🙂

rob-baker-ar commented 6 months ago

I can't share the whole template / controller, but in the version I had, as can be seen above, the template call for input render looked like:

{{ field.renderInput({
     id: "field_id",
     class: "field_class"
}) }}

Which makes sense when you look at the headline error: It's trying to assign a string to a field that expects an int. In the old code that would have made sense, but in the new, because it's not wrapped in an attributes key, I'm guessing it's trying to overwrite the field id from the database on the abstract field class, which would of course need an int opposed to a string. It's arguable whether it should be possible at all to manipulate that id from a Twig / render context.

In the controller, having checked again, there was nothing more than what's above, but was, generally speaking, doing the old way of overriding values, so this may all come down to that in the end. It would be helpful if there were examples in the docs of exactly what to pass in to the various different rendering methods (i.e. field.render(), field.renderInput(), etc) as well as the top-level form.render().

gustavs-gutmanis commented 6 months ago

Hi @rob-baker-ar,

You should be passing these as attributes to the specific target element (probably input in this case).

But you're completely on point regarding docs clarity and the fact that it shouldn't even be possible to override an ID. That is an oversight on our part and will be corrected, sorry for the inconvenience.

We will also address docs examples, of course.