Bug #86635

Plugin/Flexform fields for finisher overrides require the field to be defined in every concrete form configuration instead of falling back to the default value if the field is not set for the form

Added by Stefan P 11 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Must have
Category:
Form Framework
Target version:
-
Start date:
2018-10-12
Due date:
% Done:

100%

TYPO3 Version:
9
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:
Remote Sprint

Description

What I did:
  1. I have a form configuration that uses a custom finisher without options. This works.
  2. I extended the finisher by a new option
  3. I registered the new option also at the FormEngine configuration
  4. I get a PHP warning in the Plugin flexform that has any form using this finisher selected:
    Core: Error handler (BE): PHP Warning: Invalid argument supplied for foreach() in [...]/typo3/sysext/form/Classes/Hooks/DataStructureIdentifierHook.php line 223
    

The warning is thrown because the key options in the finisher configuration in the concrete MyForm.form.yaml is not set (it had no options before, which is valid). It's conceptually wrong that this must be set. The Flexform value in the plugin should be rendered anyways and override the default value of the finisher in this case.

I expect that the override chain is Flexform -> _optional_ Form value -> default from finisher
But currently it's actually Flexform -> _mandatory_ Form value -> default from finisher.

This also has the implication that the default value of the finisher can never have any effect, because any concrete (overridable) form has to have an initial value anways.

Think of this scenario:
Someone has 100 (read: "a lot") Form configurations using a custom finisher. Now this finisher gets a new option with a reasonable default which should (not must!) be overridable in the Flexform. This person now has to edit all 100 Form configurations manually and add an initial value to every Form using this finisher even though the finisher has a default value itself which should actually be used.

Or in other words:
Extending a finisher with a new field should not invalidate the flexform of all forms using this finisher (but currently it does).

Associated revisions

Revision 54851545 (diff)
Added by Ralf Zimmermann 5 months ago

[BUGFIX] Change finisher override display behavior

Show all possible finisher options which can be overridden within the
form plugin even if if they are not part of a form definition.

Resolves: #86635
Resolves: #85033
Releases: master, 9.5
Change-Id: Ie5ce8e2fb97e2c3fde92cbcb405d77818d1c7bda
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577
Tested-by: TYPO3com <>
Tested-by: Mathias Brodala <>
Tested-by: Susanne Moog <>
Reviewed-by: Mathias Brodala <>
Reviewed-by: Susanne Moog <>

Revision 85d7c84b (diff)
Added by Ralf Zimmermann 5 months ago

[BUGFIX] Change finisher override display behavior

Show all possible finisher options which can be overridden within the
form plugin even if if they are not part of a form definition.

Resolves: #86635
Resolves: #85033
Releases: master, 9.5
Change-Id: Ie5ce8e2fb97e2c3fde92cbcb405d77818d1c7bda
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60597
Tested-by: TYPO3com <>
Tested-by: Ralf Zimmermann <>
Tested-by: Susanne Moog <>
Reviewed-by: Ralf Zimmermann <>
Reviewed-by: Susanne Moog <>

History

#1 Updated by Stefan P 11 months ago

Or to state it in a more direct way:

The flexform in the plugin for the finisher overrides should simply show the overridable fields defined in the finisher (finishersDefinition.FinisherKey.FormEngine.elements). Currently it wrongly shows only the intersection between finishersDefinition.FinisherKey.FormEngine.elements and the options already set in the specific selected form (this has the ugly side effect that different forms can show different fields for the same finisher!). And if these options also happen to be empty (because the finisher was later extended by the new field witout adjusting all forms) the PHP warning is shown in addition.

#2 Updated by Ralf Zimmermann 11 months ago

  • Assignee set to Ralf Zimmermann

#3 Updated by Bjoern Jacob 6 months ago

  • Sprint Focus set to Remote Sprint

#4 Updated by Ralf Zimmermann 5 months ago

  • Status changed from New to In Progress

#5 Updated by Gerrit Code Review 5 months ago

  • Status changed from In Progress to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#6 Updated by Gerrit Code Review 5 months ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#7 Updated by Ralf Zimmermann 5 months ago

  • TYPO3 Version changed from 8 to 9

#8 Updated by Gerrit Code Review 5 months ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#9 Updated by Gerrit Code Review 5 months ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#10 Updated by Gerrit Code Review 5 months ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#11 Updated by Gerrit Code Review 5 months ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#12 Updated by Gerrit Code Review 5 months ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#13 Updated by Gerrit Code Review 5 months ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#14 Updated by Gerrit Code Review 5 months ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60577

#15 Updated by Gerrit Code Review 5 months ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60597

#16 Updated by Anonymous 5 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#17 Updated by Benni Mack 5 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF