Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Fix localization record merging logic #2198

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

monosize
Copy link
Member

@monosize monosize commented Dec 4, 2024

Updated the logic to correctly merge localized records by checking for the existence of '_LOCALIZED_UID' in the $contentObject data. The $record was missing the '_LOCALIZED_UID' field, and therefore proper data localization never occurred.

This change ensures that records are only merged if a valid localized version is available.

I extracted $this->recoredService->getSingle(...) from the array_merge function because on my system this never merged the localized record. Therefore, getSingle is stored in a separate variable and then, if available, merged.

Update the logic to correctly merge localized records by checking the presence of a '_LOCALIZED_UID' in the content object's data. This change ensures that records are only merged when a valid localized version is available.

I extracted $this->recoredService->getSingle(...) from the array_merge function because it had never merged the localised record on my system here.
Therefore, getSingle is saved in an extra variable and then merged if it exists.
@monosize
Copy link
Member Author

monosize commented Dec 4, 2024

Tested with TYPO3 12.4.23

@monosize
Copy link
Member Author

monosize commented Dec 4, 2024

Maybe the content from the $contentObject can be used completely – without array_merge; I haven't tested that yet.

@monosize
Copy link
Member Author

monosize commented Dec 4, 2024

I ran a test that uses $contentObject->data.
It worked for me. Is there any reason not to use it directly?

        if ($contentObject->data['_LOCALIZED_UID'] ?? false) {
            return $contentObject->data;
//            $test = $this->recordService->getSingle(
//                (string) $this->getFluxTableName(),
//                '*',
//                $contentObject->data['_LOCALIZED_UID']
//            );
//            $record = array_merge(
//                $record,
//                $test ?: $record
//            );
        }
        return $record;

Copy link
Member

@NamelessCoder NamelessCoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason not to use it (cObj->data) directly?

This may be different on TYPO3 versions, but in at least some of my testing back when writing the method, cObj->data was not trustworthy. It behaved differently in content vs. pages, and in INT and cached contexts. In some cases it even contained the data of a totally different element - which would be really bad.

So we do need to use $record and we do need to perform the overlay. At least for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants