Task #86141

Remove superfluous database contraint in DataMapProcessor

Added by Nicole Cordes 10 months ago. Updated 6 months ago.

Status:
Under Review
Priority:
Should have
Assignee:
Category:
-
Target version:
-
Start date:
2018-09-04
Due date:
% Done:

0%

TYPO3 Version:
8
PHP Version:
Tags:
Complexity:
Sprint Focus:

Description

The database query to fetch dependent elements in \TYPO3\CMS\Core\DataHandling\Localization\DataMapProcessor::fetchDependentElements uses an IN constraint as well as an gt() (greater than) constraint. Having the IN constraint is sufficient and the gt() constraint should be superfluous.

Removing the gt() constraint reveals some more flaws as the API returns 0 id as ancestor id. This has to be fixed as well to prevent wrong elements.

issue86141_0.1.0_201809092229.zip (5.44 KB) Nicole Cordes, 2018-09-09 22:30


Related issues

Related to TYPO3 Core - Bug #78059: Checks in DataHandler localize mismatch with new Localization Wizard Closed 2016-09-25
Related to TYPO3 Core - Bug #86231: Distinguish between free-mode localization and chained translation New 2018-09-12

History

#1 Updated by Gerrit Code Review 10 months ago

  • Status changed from New to Under Review

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/58049

#2 Updated by Gerrit Code Review 10 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/58049

#3 Updated by Nicole Cordes 9 months ago

Steps to reproduce the current pending patch and its bugfixes:

- install attached extension "issue86141"
- add a tt_content element in default language as well as a content element for "Related Content" (tab "Extended")
- translate the default record to language_1
- open the new content element of language_1 and remove the attached record in "Related Content", switch from "Value of default language" to "Custom value" and add another new record in that field
- translate the language_1 record to language_2
- add a new "Related Content" element to language_1 record
- translate the language_2 record to language_3

Expected behavior:

- Two new tt_content elements should be created for language_3

Actual behavior:

- An exception ("Child record was not processed", 1486233164) is thrown and prevents the overlay from being removed

#4 Updated by Gerrit Code Review 9 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/58049

#5 Updated by Oliver Hader 9 months ago

The issue is a bit more complex... in the first run I was wondering, why there are localized records that are not having any language parent-relation (l10n_parent).

Since https://review.typo3.org/#/c/50022/ (which allowed chained translations from an existing localized record), l10n_parent is only set for the default language.
In a consequence, records that are only available for a given language and are localized from there into a different language, l10n_parent will stay empty.
The behavior for free-mode localizations (copyToLanguage command in DataHandler) have a similar effect, l10n_parent is set to 0.

Great, in this case we cannot distinguish between connected relations (that were not created from the default language) and free-mode localizations anymore. That's the first issue...

DataMapProcessor can only handle items that are connected (thus, has accordant ancestor relationship - in terms of a chain of translations) - synchronizing values between items, that are not connected to each other, is simply wrong.

Awesome, DataMapProcessor is currently written with the assumption that connected elements always have l10n_parent defined - which is not always the case as written above. That's the second issue...

In order to resolve connected translations chains in a reliable way, the l10n_source values have to be resolved to their relative ancestors. However, since there is no way to distinguish between connected and free-mode localizations that have not been created from some record in default language the information simply is not available.

Great, this issue cannot be solved without enriching the information that is persisted currently. Besides that, l10n_source is not available for all tables. Thus, this is not generic. And that's the third and most severe issue...

Conclusion: Cannot be solved in a generic way with the current information that is available in the database.

#6 Updated by Oliver Hader 9 months ago

  • Related to Bug #78059: Checks in DataHandler localize mismatch with new Localization Wizard added

#7 Updated by Gerrit Code Review 9 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/58049

#8 Updated by Gerrit Code Review 9 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/58049

#9 Updated by Tymoteusz Motylewski 9 months ago

@Olly, you mean that there is no way to detect default language ancestor except for walkin the chain up?
Can you sum up which combinations are/can be supported and which not (in terms of chains of translations)?

#10 Updated by Oliver Hader 9 months ago

@Tymek I'll do and create a new ticket...

#11 Updated by Oliver Hader 9 months ago

  • Related to Bug #86231: Distinguish between free-mode localization and chained translation added

#13 Updated by Gerrit Code Review 9 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/58049

#14 Updated by Gerrit Code Review 9 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/58049

#15 Updated by Gerrit Code Review 9 months ago

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

#16 Updated by Gerrit Code Review 9 months ago

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

#17 Updated by Gerrit Code Review 6 months ago

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

Also available in: Atom PDF