Project

General

Profile

Actions

Bug #105133

open

FlexFormProcessor do not attach data correctly in processAdditionalDataProcessors

Added by Maxime Lafontaine about 2 months ago. Updated 23 days ago.

Status:
Under Review
Priority:
Should have
Assignee:
-
Category:
Frontend
Target version:
-
Start date:
2024-09-26
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
11
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

In the FlexFormProcessor, the data is added to an array before passing it to the CObj start method.

$contentObjectRenderer->start([$data]);
Should be
$contentObjectRenderer->start($data);

Else the sub-processor need to get the data from the first element of the array, and not directly.
That can work with DatabaseQueryProcessor (my case) with, per example, uidInList.data = field : 0|my_flexform_field .
But is a problem for the processors using only "fieldName", as CommaSeparatedValueProcessor

Actions #1

Updated by Gerrit Code Review about 2 months ago

  • Status changed from New to Under Review

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

Actions #2

Updated by Garvin Hicking about 2 months ago

  • Status changed from Under Review to Needs Feedback

I've tried your patch suggestion and the tests will actually report that this leads to consequences where expectations are no longer met:

https://git.typo3.org/typo3/CI/cms/-/jobs/3803169

Maybe that helps you to further work on a patch/investigation?

Actions #3

Updated by Garvin Hicking about 2 months ago

Copied here in case the test artifacts get removed:

There were 2 failures:
1) TYPO3\CMS\Frontend\Tests\Unit\DataProcessing\FlexFormProcessorTest::subDataProcessorIsResolved
Expectation failed for method name is "start" when invoked 1 time
Parameter 0 for invocation TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::start([...], '') does not match expected value.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => [...]
+    'options' => [...]
 )
/builds/typo3/CI/cms/typo3/sysext/frontend/Classes/DataProcessing/FlexFormProcessor.php:140
/builds/typo3/CI/cms/typo3/sysext/frontend/Classes/DataProcessing/FlexFormProcessor.php:101
/builds/typo3/CI/cms/typo3/sysext/frontend/Tests/Unit/DataProcessing/FlexFormProcessorTest.php:214
/builds/typo3/CI/cms/bin/phpunit:122
2) TYPO3\CMS\Frontend\Tests\Unit\DataProcessing\FlexFormProcessorTest::subDataProcessorIsResolved
tearDown() integrity check found left over instances in GeneralUtility::makeInstance() instance list. Always consume instances added via GeneralUtility::addInstance() in your test by the test subject.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
+    0 => MockObject_ContentDataProcessor_ee0aac71 Object (...)
 )
/builds/typo3/CI/cms/vendor/typo3/testing-framework/Classes/Core/Unit/UnitTestCase.php:180
/builds/typo3/CI/cms/bin/phpunit:122
FAILURES!
Tests: 11721, Assertions: 49873, Failures: 2.
Actions #4

Updated by Gerrit Code Review about 1 month ago

  • Status changed from Needs Feedback to Under Review

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

Actions #5

Updated by Maxime Lafontaine 23 days ago

Yes, the change break the test because the test display exactly the problem:
The data of the flexform is added to an array as the first (0) element.
The array should be the flexform data. I'm not sure what was the idea to put it in the array, but it break the chaining of processors.
With the DatabaseQueryProcessor I have make it work since I could transform the data used (has display in the bug description), but will not work for other processors that take the data directly (those that use a fieldName as CommaSeparatedValueProcessor for example).
Maybe we can add a property to change the behaviour, or just have it in the array (like now) AND merge it with the flexform data, if really needed.
Yes, it's a breaking change to how it work now.

Actions

Also available in: Atom PDF