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.

Sign up for GitHub

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

IBX-6494: Fixed copying of non-translatable fields to later versions #394

Merged
merged 3 commits into from
May 7, 2024
Merged

IBX-6494: Fixed copying of non-translatable fields to later versions #394

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38d2838
IBX-6494: Copy nontranslatable fields to later versions
barw4 Nov 14, 2023
a257944
IBX-6494: Refactored solution
barw4 Apr 15, 2024
ceb8c7a
IBX-6494: CS
barw4 Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions eZ/Publish/Core/Repository/ContentService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1517,10 +1517,7 @@ protected function copyNonTranslatableFieldsFromPublishedVersion(APIContent $cur
$publishedContent = $this->internalLoadContentById($versionInfo->getContentInfo()->getId());
$publishedVersionInfo = $publishedContent->getVersionInfo();

if (
!$publishedVersionInfo->isPublished()
Copy link
Contributor

Choose a reason for hiding this comment

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

Isnt that going to break some cases when things are not copied for translations (\Ibexa\Core\Repository\ContentService::copyTranslationsFromPublishedVersion) and then some other fields will be copied here?

Or isnt it going to break publishing older version, with overriding all fields with current ones?

Copy link
Member Author

@barw4 barw4 Nov 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@ViniTou

Isnt that going to break some cases when things are not copied for translations (\Ibexa\Core\Repository\ContentService::copyTranslationsFromPublishedVersion) and then some other fields will be copied here?

Hmm, I cannot see why removing this condition would break that. The updated method only deals with nontranslatable fields and those are not handled in copyTranslationsFromPublishedVersion.

Or isnt it going to break publishing older version, with overriding all fields with current ones?

I've just tested this and it seems that version 2 (3 is published) properly overridden version 3 so values from version 3 weren't copied into earlier version 2 during publishing if that's what you had in mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

The updated method only deals with nontranslatable fields and those are not handled in copyTranslationsFromPublishedVersion.

Yes, my point was, that right now some fields could be copied/not copied there, and be copied/not copied here as the logic now differs.

But I guess, maybe QA will stumble on specific case as I cant come up with proper testing scenario for it.

|| ($versionInfo->versionNo >= $publishedVersionInfo->versionNo)
alongosz marked this conversation as resolved.
Show resolved Hide resolved
) {
if (!$publishedVersionInfo->isPublished()) {
return;
}

Expand Down
56 changes: 56 additions & 0 deletions ...tion/Core/Repository/ContentService/CopyNonTranslatableFieldsFromPublishedVersionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,62 @@ public function testCopyNonTranslatableFieldsFromPublishedVersionToDraft(): void
self::assertSame($expectedBodyValue, $bodyFieldValue->text);
}

/**
* @throws \eZ\Publish\API\Repository\Exceptions\Exception
*/
public function testCopyNonTranslatableFieldsFromPublishedVersionToLatestVersion(): void
{
$this->createNonTranslatableContentType();

$contentService = self::getContentService();
$contentTypeService = self::getContentTypeService();
$locationService = self::getLocationService();

// Creating start content in eng-US language
$contentType = $contentTypeService->loadContentTypeByIdentifier(self::CONTENT_TYPE_IDENTIFIER);
$mainLanguageCode = self::ENG_US;
$contentCreateStruct = $contentService->newContentCreateStruct($contentType, $mainLanguageCode);
$contentCreateStruct->setField('title', 'Test title');
$contentCreateStruct->setField('body', 'Nontranslatable body');

$contentDraft = $contentService->createContent(
$contentCreateStruct,
[
$locationService->newLocationCreateStruct(2),
]
);
$publishedContent = $contentService->publishVersion($contentDraft->getVersionInfo());

// Creating two drafts at the same time
$usDraft = $contentService->createContentDraft($publishedContent->contentInfo);
$gerDraft = $contentService->createContentDraft($publishedContent->contentInfo);

// Publishing the draft in eng-US language
$contentUpdateStruct = new ContentUpdateStruct([
'initialLanguageCode' => self::ENG_US,
'fields' => $usDraft->getFields(),
]);
$contentUpdateStruct->setField('title', 'Title v2', self::ENG_US);
$contentUpdateStruct->setField('body', 'Nontranslatable body v2', self::ENG_US);
$usContent = $contentService->updateContent($usDraft->getVersionInfo(), $contentUpdateStruct);
$contentService->publishVersion($usContent->getVersionInfo());

// Publishing the draft in ger-DE language
$contentUpdateStruct = new ContentUpdateStruct([
'initialLanguageCode' => self::GER_DE,
'fields' => $gerDraft->getFields(),
]);
$contentUpdateStruct->setField('title', 'Title ger', self::GER_DE);
$gerContent = $contentService->updateContent($gerDraft->getVersionInfo(), $contentUpdateStruct);
$contentService->publishVersion($gerContent->getVersionInfo());

// Loading main content
$mainPublishedContent = $contentService->loadContent($gerContent->id);
$bodyFieldValue = $mainPublishedContent->getField('body')->getValue();

self::assertSame('Nontranslatable body v2', $bodyFieldValue->text);
}

private function createNonTranslatableContentType(): void
{
$permissionResolver = self::getPermissionResolver();
Expand Down
Loading