terminal42 / contao-mp_forms

Real step separation in the form generator of the Contao Open Source CMS
25 stars 13 forks source link

Keep form fields array key intact #65

Closed tsarma closed 1 year ago

tsarma commented 1 year ago

I am using Contao Hook, prepareFormData. When I install contao-mp_forms, the 3rd argument passed to the hook function $fields which originally has array keys is removed.

mp-form

Toflar commented 1 year ago

Can you check if the current development version would fix this issue? https://github.com/terminal42/contao-mp_forms/pull/66

tsarma commented 1 year ago

Thanks for the new version. I tried to install the dev_develop branch but did not succeed.

The problem shown is as follows.

  Problem 1
    - Root composer.json requires terminal42/contao-mp_forms dev-develop -> satisfiable by terminal42/contao-mp_forms[dev-develop].
    - terminal42/contao-mp_forms dev-develop requires codefog/contao-haste ^5.0 -> found codefog/contao-haste[5.0.0, ..., 5.0.4] but these were not loaded, likely because it conflicts with another require.

It may be caused by some other extensions which also requires contao-haste.

  1. contao-changelanguage: "codefog/contao-haste": "^4.13"
  2. contao-leads: "codefog/contao-haste": "^4.10.0"
  3. dcwizard: "codefog/contao-haste": "4.*"
  4. notification-center: "codefog/contao-haste":"^4.14.1"

My Env: Contao 4.13.14 PHP: 8.1.11

Toflar commented 1 year ago

Yes, this version is only compatible with haste 5. You cannot use it together with other extensions that still require haste 4.

Remarks:

  1. Changelanguage has a new dev version too (main branch) which does no longer require haste. Should be released soon too.
  2. A new version of leads should also be released in the upcoming weeks/months.
  3. dcawizard has a new release that supports haste 5.
  4. NC has a new release that supports haste 5.
tsarma commented 1 year ago

Now I thought of testing it on a fresh Contao installation without any external extensions. But it won't go as expected, it stops with PHP Fatal error and I don't know if it is due to my mistake or something else.

PHP Fatal error:  During inheritance of Terminal42\MultipageFormsBundle\Controller\FrontendModule\StepsController, while autoloading Contao\CoreBundle\Twig\FragmentTemplate: Uncaught ReflectionException: Class "Contao\CoreBundle\Twig\FragmentTemplate" not found while loading "Terminal42\MultipageFormsBundle\Controller\FrontendModule\StepsController". in /Volumes/Scratch/projects/_contao/contao41314/vendor/composer/ClassLoader.php:571

Screenshot 2022-12-19 at 09 37 52

Toflar commented 1 year ago

Ah, you're trying with 4.13, very good :) Fixed.

tsarma commented 1 year ago

Thank you. Now the installation went smoothly. But the original problem remains i.e. the array keys are removed with contao-mp_forms installed. I've tested with the below code, once with and without contao-mp_forms


<?php
// src/EventListener/PrepareFormDataListener.php
namespace App\EventListener;

use Contao\CoreBundle\ServiceAnnotation\Hook;
use Contao\Form;

/**
 * @Hook("prepareFormData")
 */
class PrepareFormDataListener
{
    public function __invoke(array &$submittedData, array $labels, array $fields, Form $form): void
    {
        dd($fields);
    }
}
Toflar commented 1 year ago

I've refactored a lot again in https://github.com/terminal42/contao-mp_forms/pull/66/commits/189cce7fc5183e67532837fa3fa1f177243906af.

It's now also covered by unit tests here: https://github.com/terminal42/contao-mp_forms/pull/66/commits/189cce7fc5183e67532837fa3fa1f177243906af#diff-98228ed7bd5a1e49bac64332a36d033ebbb9b40e8dbc1780766a70d9f79137f5R89

So it should deifinitely be a key-value array now - otherwise please try to debug why it's not in your case.

tsarma commented 1 year ago

The array $formFields returned from FromManager::getFieldsWithoutPageBreaks() does not have keys anymore. It is removed from there.

I think this can be avoided if you pass $formFields as a parameter to the $manager->getFieldsWithoutPageBreaks() inside class CompileFormFieldsListener, work on the same $formFields instead of getting fields from formModels again.

Toflar commented 1 year ago

Ah, now I see the problem. Your initial description was very misleading because it always pointed to the prepareFormData hook when in fact the issue is in the compileFormFields process. https://github.com/terminal42/contao-mp_forms/commit/29b53a1aa0b01a16cac720b1016f42e215753700 should keep things consistent, now :)

tsarma commented 1 year ago

Thank you. It works now in a consistent manner with the core. You may close this issue from my side.