Bug #65126

Refindex not correctly updated for flexform fields

Added by Dominique Kreemers almost 6 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Workspaces
Target version:
-
Start date:
2015-02-17
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
medium
Is Regression:
No
Sprint Focus:

Description

This occurs in TYPO3 6.2.9.

When saving content elements with flexform fields the refindex isn't updated.
Therefore the grouping in the workspace module is not working as expected.

This seems like a wrong method call in typo3/sysext/core/Classes/Database/ReferenceIndex.php -> getRelations_flexFormCallBack on line 494.
https://forge.typo3.org/projects/typo3cms-core/repository/revisions/6.2.9/entry/typo3/sysext/core/Classes/Database/ReferenceIndex.php#L494

$resultsFromDatabase = $this->getRelations_procDB($dataValue, $dsConf, $uid, $field);

The method is defined like this:

/**
 * Check field configuration if it is a DB relation field and extract DB relations if any
 *
 * @param string $value Field value
 * @param array $conf Field configuration array of type "TCA/columns
 * @param integer $uid Field uid
 * @param string $table Table name
 * @param string $field Field name
 * @return array If field type is OK it will return an array with the database relations. Else FALSE
 * @todo Define visibility
 */
public function getRelations_procDB($value, $conf, $uid, $table = '', $field = '') {

After changing the call to

$resultsFromDatabase = $this->getRelations_procDB($dataValue, $dsConf, $uid, $table, $field);

the generated sys_refindex records looked very promising, but unfortunately the grouping in the workspace module is still not working for related records (e.g. sys_file_reference records).


Files

flexform_test.zip (3.69 KB) flexform_test.zip Dominique Kreemers, 2015-03-16 18:15
Bildschirmfoto 2015-03-16 um 18.07.59.png (60.6 KB) Bildschirmfoto 2015-03-16 um 18.07.59.png Dominique Kreemers, 2015-03-16 18:15
patch.diff (2.96 KB) patch.diff Dominique Kreemers, 2015-03-16 18:15
patch.diff (2.97 KB) patch.diff Dominique Kreemers, 2015-03-17 09:43
without-patch-file-reference-is-not-grouped-with-content.png (33.7 KB) without-patch-file-reference-is-not-grouped-with-content.png without patch file reference is not grouped with content Stephan Großberndt, 2015-03-31 20:02
with-patch-file-reference-is-grouped-with-content.png (41.5 KB) with-patch-file-reference-is-grouped-with-content.png with patch file reference is grouped with content Stephan Großberndt, 2015-03-31 20:02

Related issues

Related to TYPO3 Core - Bug #60226: Content Element header_link to a page also adds a references to the file with same idClosedNicole Cordes2014-07-10

Actions
Related to TYPO3 Core - Bug #70921: Really resolve meaning of FlexForm fields in version dependency resolverAcceptedOliver Hader2015-10-21

Actions
Related to TYPO3 Core - Bug #68463: Infinite Recursion blocks the Live WorkspaceAccepted2015-07-22

Actions
#1

Updated by Dominique Kreemers almost 6 years ago

Found the error for the grouping in the workspace module in typo3/sysext/core/Classes/DataHandling/DataHandler.php -> getInlineFieldType on line 6787.
https://forge.typo3.org/projects/typo3cms-core/repository/revisions/6.2.9/entry/typo3/sysext/core/Classes/DataHandling/DataHandler.php#L6786
If the given field in the $conf parameter is a "flex" field the method will return FALSE.

#2

Updated by Dominique Kreemers almost 6 years ago

Found one more error in typo3/sysext/core/Classes/Database/ReferenceIndex.php -> getRelations_procDB on line 616.
https://forge.typo3.org/projects/typo3cms-core/repository/revisions/6.2.9/entry/typo3/sysext/core/Classes/Database/ReferenceIndex.php#L616

} elseif ($conf['type'] == 'input' && isset($conf['wizards']['link']) && trim($value)) {
    try {
        $file = \TYPO3\CMS\Core\Resource\ResourceFactory::getInstance()->retrieveFileOrFolderObject($value);
    } catch (\Exception $e) {

    }
    if ($file instanceof \TYPO3\CMS\Core\Resource\FileInterface) {
        return array(
            0 => array(
                'table' => 'sys_file',
                'id' => $file->getUid()
            )
        );
    }
}

If you select a page with a link wizard, the page uid is saved in the input field. The ReferenceIndex will interpret this field as a uid to a sys_file record and create a wrong sys_refindex record.
After changing the elseif condition to
} elseif ($conf['type'] == 'input' && isset($conf['wizards']['link']) && trim($value) && GeneralUtility::isFirstPartOfStr($input, 'file:')) {

this seems to work.

#3

Updated by Stephan Großberndt almost 6 years ago

This is another bug and is currently being fixed in https://review.typo3.org/#/c/37016.

#4

Updated by Markus Klein almost 6 years ago

Maybe you can push a patch for the DataHandler bug?

#5

Updated by Stephan Großberndt over 5 years ago

Dominique Kreemers wrote:

Found the error for the grouping in the workspace module in typo3/sysext/core/Classes/DataHandling/DataHandler.php -> getInlineFieldType on line 6787.
https://forge.typo3.org/projects/typo3cms-core/repository/revisions/6.2.9/entry/typo3/sysext/core/Classes/DataHandling/DataHandler.php#L6786
If the given field in the $conf parameter is a "flex" field the method will return FALSE.

Which call leads to getInlineFieldType()? Is it from copyRecord_flexFormCallBack()

    $dataValue = $this->copyRecord_procFilesRefs($dsConf, $uid, $dataValue);
    // If references are set for this field, set flag so they can be corrected later (in ->remapListedDBRecords())
    if (($this->isReferenceField($dsConf) || $this->getInlineFieldType($dsConf) !== FALSE) && (string)$dataValue !== '') {

Thats the only occurrence I can find is there. $conf['type'] = 'flex' is neither db reference field nor an inline field. So it cannot be integrated there. But I wonder: copyRecord_procFilesRefs() will return '' for a $conf['type'] = 'flex', so $dataValue === ''. Are you sure simply adding

    if (($this->isReferenceField($dsConf) || $dsConf['type'] === 'flex' || $this->getInlineFieldType($dsConf) !== FALSE) && (string)$dataValue !== '') {

fixes your problem?

Can you please add a description how exactly to reproduce the problem?

#6

Updated by Gerrit Code Review over 5 years ago

  • Status changed from New 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 http://review.typo3.org/37767

#7

Updated by Gerrit Code Review over 5 years ago

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/37796

#8

Updated by Stephan Großberndt over 5 years ago

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

Updated by Stephan Großberndt over 5 years ago

  • Status changed from Resolved to Needs Feedback

As I wrote before in Note 5 :

Dominique, could you please add a description how exactly to reproduce the problem with grouping in the workspace module?

The first problem you mentioned is fixed in core and will be part of the next update.

#10

Updated by Dominique Kreemers over 5 years ago

I attached a little extension so you can reproduce the error.

Just install the flexform_test extension, go to a custom workspace and create a new content element "Flexform Test". Switch to the "Flexform Test" tab and add a new relation to an image.
After switching to the workspace module you will see 2 database records that should be grouped because the file reference is a child of the content element.

With the attached patch the grouping seems to work properly but there is still an error in the publishing process. After you publish the content element, the image reference is still in the workspace on state "ready to publish".

#11

Updated by Gerrit Code Review over 5 years ago

  • Status changed from Needs Feedback 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 http://review.typo3.org/37891

#12

Updated by Stephan Großberndt over 5 years ago

  • Category set to Workspaces
  • Status changed from Under Review to Accepted
  • % Done changed from 100 to 50

Thanks for supplying that extension!

So far I have developed and tested this on 7.2.0-dev only. I can confirm the bug that the file changes are not grouped with the content element change.

On 7.2.0-dev only your patch for ElementEntityProcessor.php is necessary. Your change for ElementEntity.php will always select the else case because $table in $GLOBALS['TCA'][$table]['ctrl']['versioningWS'] is not defined. But that change is not necessary, it works without it: If you publish the change by only checking the checkbox of the content element still the image reference vanishes from the workspace view too.

I'll try testing with 6.2 at a later time.

#13

Updated by Dominique Kreemers over 5 years ago

Stephan Großberndt wrote:

On 7.2.0-dev only your patch for ElementEntityProcessor.php is necessary. Your change for ElementEntity.php will always select the else case because $table in $GLOBALS['TCA'][$table]['ctrl']['versioningWS'] is not defined. But that change is not necessary, it works without it: If you publish the change by only checking the checkbox of the content element still the image reference vanishes from the workspace view too.

You're right, the $table variable was the problem. I changed it to $this->getTable() and now the publishing works as expected (in 6.2.11-dev).

#14

Updated by Markus Klein over 5 years ago

@Dominique: Please push you patch suggestions to our review system directly. Thanks a lot

#15

Updated by Stephan Großberndt over 5 years ago

Did you also try my patch? Are you sure your addition is needed?

#16

Updated by Dominique Kreemers over 5 years ago

Stephan Großberndt wrote:

Did you also try my patch? Are you sure your addition is needed?

Sorry, i overlooked the patch because i'm just getting started with this code review system.
Your patch seems to work just fine with TYPO3 6.2.11.

Thanks for your help.

#17

Updated by Stephan Großberndt over 5 years ago

  • Status changed from Accepted to Under Review
  • % Done changed from 50 to 100
  • Complexity set to medium

Thanks for verifying the functionality of the patch in TYPO3 CMS 6.2.

We now have to verify the changes to not break any other functionality so this can become part of 6.2.12

#18

Updated by Gerrit Code Review over 5 years ago

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

#19

Updated by Markus Klein over 5 years ago

Please votes again, I had to change the commit message.

#21

Updated by Gerrit Code Review over 5 years ago

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

#22

Updated by Gerrit Code Review over 5 years ago

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/38520

#23

Updated by Stephan Großberndt over 5 years ago

  • Status changed from Under Review to Resolved
#24

Updated by Benni Mack about 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF