Skip to content

Commit

Permalink
EZP-29694: Draft conflict does not work as expected for a content ite…
Browse files Browse the repository at this point in the history
…m with multiple translations (#950)

Note: possible due to introducing of copying missing previously published translations to drafts via  ezsystems/ezpublish-kernel#2598
  • Loading branch information
ViniTou authored and alongosz committed Apr 18, 2019
1 parent 2b31ab8 commit 6c5b0f9
Show file tree
Hide file tree
Showing 12 changed files with 131 additions and 47 deletions.
15 changes: 6 additions & 9 deletions src/bundle/Controller/Content/VersionDraftConflictController.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,21 @@ public function __construct(

/**
* @param int $contentId
* @param int|null $locationId
* @param string $languageCode
*
* @return Response
*
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
* @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
* @throws \InvalidArgumentException
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
*/
public function draftHasNoConflictAction(int $contentId, ?int $locationId = null): Response
public function draftHasNoConflictAction(int $contentId, string $languageCode): Response
{
$content = $this->contentService->loadContent($contentId);
$location = $this->locationService->loadLocation(
$locationId ?? $content->contentInfo->mainLocationId
);
$location = $this->locationService->loadLocation($content->contentInfo->mainLocationId);
$contentInfo = $content->contentInfo;

try {
$contentDraftHasConflict = (new ContentDraftHasConflict($this->contentService))->isSatisfiedBy($contentInfo);
$contentDraftHasConflict = (new ContentDraftHasConflict($this->contentService, $languageCode))->isSatisfiedBy($contentInfo);
} catch (UnauthorizedException $e) {
$error = $this->translator->trans(
/** @Desc("Cannot check if the draft has no conflict with other drafts. %error%. ") */
Expand All @@ -91,7 +88,7 @@ public function draftHasNoConflictAction(int $contentId, ?int $locationId = null
if ($contentDraftHasConflict) {
$versionsDataset = $this->datasetFactory->versions();
$versionsDataset->load($contentInfo);
$conflictedDrafts = $versionsDataset->getConflictedDraftVersions($contentInfo->currentVersionNo);
$conflictedDrafts = $versionsDataset->getConflictedDraftVersions($contentInfo->currentVersionNo, $languageCode);

$modalContent = $this->renderView('@ezdesign/content/modal_draft_conflict.html.twig', [
'conflicted_drafts' => $conflictedDrafts,
Expand Down
28 changes: 28 additions & 0 deletions src/bundle/Controller/ContentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use eZ\Publish\API\Repository\LocationService;
use eZ\Publish\API\Repository\UserService;
use eZ\Publish\API\Repository\Values\Content\Content;
use eZ\Publish\API\Repository\Values\Content\Language;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\Core\Base\Exceptions\BadStateException;
use EzSystems\EzPlatformAdminUi\Exception\InvalidArgumentException as AdminInvalidArgumentException;
Expand Down Expand Up @@ -190,6 +191,7 @@ public function editAction(Request $request): Response

if (!$versionInfo->isDraft()) {
$contentDraft = $this->contentService->createContentDraft($contentInfo, $versionInfo);
$this->updateDraftInitialLanguage($contentDraft, $language);
$versionNo = $contentDraft->getVersionInfo()->versionNo;

$this->notificationHandler->success(
Expand Down Expand Up @@ -456,4 +458,30 @@ public function updateVisibilityAction(Request $request): Response

return $result instanceof Response ? $result : $this->redirectToRoute('ezplatform.dashboard');
}

/**
* @param \eZ\Publish\API\Repository\Values\Content\Content $contentDraft
* @param \eZ\Publish\API\Repository\Values\Content\Language $language
*
* @throws \eZ\Publish\API\Repository\Exceptions\BadStateException
* @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException
* @throws \eZ\Publish\API\Repository\Exceptions\ContentValidationException
* @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
*/
private function updateDraftInitialLanguage(
Content $contentDraft,
Language $language
): void {
if ($language->languageCode === $contentDraft->versionInfo->initialLanguageCode) {
return;
}

$contentUpdateStruct = $this->contentService->newContentUpdateStruct();

$contentUpdateStruct->initialLanguageCode = $language->languageCode;
$contentUpdateStruct->creatorId = $contentDraft->versionInfo->creatorId;

$this->contentService->updateContent($contentDraft->versionInfo, $contentUpdateStruct);
}
}
8 changes: 4 additions & 4 deletions src/bundle/Controller/Version/VersionConflictController.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,23 @@ public function __construct(ContentService $contentService)
*
* @param int $contentId
* @param int $versionNo
* @param string $languageCode
*
* @return Response
*
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
* @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
* @throws \InvalidArgumentException
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
* @throws \eZ\Publish\Core\Base\Exceptions\BadStateException
*/
public function versionHasNoConflictAction(int $contentId, int $versionNo): Response
public function versionHasNoConflictAction(int $contentId, int $versionNo, string $languageCode): Response
{
$versionInfo = $this->contentService->loadVersionInfoById($contentId, $versionNo);

if (!$versionInfo->isDraft()) {
throw new BadStateException('Version status', 'status is not draft');
}

if ((new VersionHasConflict($this->contentService))->isSatisfiedBy($versionInfo)) {
if ((new VersionHasConflict($this->contentService, $languageCode))->isSatisfiedBy($versionInfo)) {
return new Response('', Response::HTTP_CONFLICT);
}

Expand Down
2 changes: 1 addition & 1 deletion src/bundle/Resources/config/routing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ ezplatform.version.has_no_conflict:
languageCode: ~

ezplatform.version_draft.has_no_conflict:
path: /version-draft/has-no-conflict/{contentId}
path: /version-draft/has-no-conflict/{contentId}/{languageCode}
options:
expose: true
defaults:
Expand Down
5 changes: 3 additions & 2 deletions src/bundle/Resources/public/js/scripts/admin.location.view.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@
};
const handleEditItem = (content) => {
const contentId = content._id;
const checkVersionDraftLink = Routing.generate('ezplatform.version_draft.has_no_conflict', { contentId });
const languageCode = content.mainLanguageCode;
const checkVersionDraftLink = Routing.generate('ezplatform.version_draft.has_no_conflict', { contentId, languageCode });
const submitVersionEditForm = () => {
doc.querySelector('#form_subitems_content_edit_content_info').value = contentId;
doc.querySelector('#form_subitems_content_edit_version_info_content_info').value = contentId;
doc.querySelector('#form_subitems_content_edit_version_info_version_no').value =
content.CurrentVersion.Version.VersionInfo.versionNo;
doc.querySelector(`#form_subitems_content_edit_language_${content.mainLanguageCode}`).checked = true;
doc.querySelector(`#form_subitems_content_edit_language_${languageCode}`).checked = true;
doc.querySelector('#form_subitems_content_edit_create').click();
};
const addDraft = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
'input[name="' + versionEditFormName + '[version_info][version_no]"]'
);
const languageInput = versionEditForm.querySelector('#' + versionEditFormName + '_language_' + languageCode);
const checkVersionDraftLink = global.Routing.generate('ezplatform.version_draft.has_no_conflict', { contentId });
const checkVersionDraftLink = global.Routing.generate('ezplatform.version_draft.has_no_conflict', { contentId, languageCode });
const submitVersionEditForm = () => {
contentInfoInput.value = contentId;
versionInfoContentInfoInput.value = contentId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
const form = editActions.querySelector('form');
const contentIdInput = form.querySelector('#content_edit_content_info') || form.querySelector('#user_edit_content_info');
const contentId = contentIdInput.value;
const checkVersionDraftLink = global.Routing.generate('ezplatform.version_draft.has_no_conflict', { contentId });
const resetRadioButtons = () =>
btns.forEach((btn) => {
btn.checked = false;
Expand All @@ -13,12 +12,10 @@
form.submit();
$('#version-draft-conflict-modal').modal('hide');
};
const redirectToUserEdit = () => {
const redirectToUserEdit = (languageCode) => {
const versionNo = form.querySelector('#user_edit_version_info_version_no').value;
const checkedBtn = btns.find((btn) => btn.checked);
const language = checkedBtn.value;

window.location.href = global.Routing.generate('ez_user_update', { contentId, versionNo, language });
window.location.href = global.Routing.generate('ez_user_update', { contentId, versionNo, language: languageCode });
};
const onModalHidden = () => {
resetRadioButtons();
Expand Down Expand Up @@ -48,15 +45,19 @@
wrapper.innerHTML = modalHtml;
attachModalListeners(wrapper);
};
const changeHandler = () => {
const changeHandler = (event) => {
const checkedBtn = event.currentTarget;
const languageCode = checkedBtn.value;
const checkVersionDraftLink = global.Routing.generate('ezplatform.version_draft.has_no_conflict', { contentId, languageCode });

fetch(checkVersionDraftLink, {
credentials: 'same-origin',
}).then((response) => {
if (response.status === 409) {
response.text().then(showModal);
} else if (response.status === 200) {
if (form.querySelector('#user_edit_version_info')) {
redirectToUserEdit();
redirectToUserEdit(languageCode);

return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
path('ez_user_update', {
'contentId': version.contentInfo.id,
'versionNo': version.versionNo,
'language': version.translations[0].languageCode,
'language': version.initialLanguageCode,
}) : path('ez_content_draft_edit', {
'contentId': version.contentInfo.id,
'versionNo': version.versionNo,
'language': version.translations[0].languageCode,
'language': version.initialLanguageCode,
'locationId': location.id
})
%}
Expand Down Expand Up @@ -85,7 +85,7 @@
data-version-has-conflict-url="{{ path('ezplatform.version.has_no_conflict', {
'contentId': version.contentInfo.id,
'versionNo': version.versionNo,
'languageCode': version.translations[0].languageCode
'languageCode': version.initialLanguageCode
}) }}"
class="btn btn-icon mx-2 ez-btn--content-draft-edit"
title="{{ 'tab.versions.table.action.draft.edit'|trans|desc('Edit Draft') }}">
Expand Down
19 changes: 13 additions & 6 deletions src/lib/Specification/Content/ContentDraftHasConflict.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,30 @@

use eZ\Publish\API\Repository\Exceptions\UnauthorizedException;
use eZ\Publish\API\Repository\ContentService;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use EzSystems\EzPlatformAdminUi\Specification\AbstractSpecification;

class ContentDraftHasConflict extends AbstractSpecification
{
/** @var ContentService */
/** @var \eZ\Publish\API\Repository\ContentService */
private $contentService;

/** @var string */
private $languageCode;

/**
* @param ContentService $contentService
* @param \eZ\Publish\API\Repository\ContentService $contentService
* @param string $languageCode
*/
public function __construct(ContentService $contentService)
public function __construct(ContentService $contentService, string $languageCode)
{
$this->contentService = $contentService;
$this->languageCode = $languageCode;
}

/**
* Checks if Content has draft conflict.
*
* @param ContentInfo $contentInfo
* @param \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo
*
* @return bool
*
Expand All @@ -40,7 +44,10 @@ public function isSatisfiedBy($contentInfo): bool
$versions = $this->contentService->loadVersions($contentInfo);

foreach ($versions as $checkedVersionInfo) {
if ($checkedVersionInfo->isDraft() && $checkedVersionInfo->versionNo > $contentInfo->currentVersionNo) {
if ($checkedVersionInfo->isDraft()
&& $checkedVersionInfo->versionNo > $contentInfo->currentVersionNo
&& $checkedVersionInfo->initialLanguageCode === $this->languageCode
) {
return true;
}
}
Expand Down
16 changes: 12 additions & 4 deletions src/lib/Specification/Version/VersionHasConflict.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,20 @@

class VersionHasConflict extends AbstractSpecification
{
/** @var ContentService */
/** @var \eZ\Publish\API\Repository\ContentService */
private $contentService;

/** @var string */
private $languageCode;

/**
* @param ContentService $contentService
* @param \eZ\Publish\API\Repository\ContentService $contentService
* @param string $languageCode
*/
public function __construct(ContentService $contentService)
public function __construct(ContentService $contentService, string $languageCode)
{
$this->contentService = $contentService;
$this->languageCode = $languageCode;
}

/**
Expand All @@ -40,7 +45,10 @@ public function isSatisfiedBy($versionInfo): bool
$versions = $this->contentService->loadVersions($versionInfo->getContentInfo());

foreach ($versions as $checkedVersionInfo) {
if ($checkedVersionInfo->versionNo > $versionInfo->versionNo && $checkedVersionInfo->isPublished()) {
if ($checkedVersionInfo->versionNo > $versionInfo->versionNo
&& $checkedVersionInfo->isPublished()
&& $checkedVersionInfo->initialLanguageCode === $this->languageCode
) {
return true;
}
}
Expand Down
53 changes: 46 additions & 7 deletions src/lib/Tests/Specification/VersionHasConflictTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function testVersionWithStatusDraft()
$this->createVersionInfo(true, 4),
]);

$versionHasConflict = new VersionHasConflict($contentServiceMock);
$versionHasConflict = new VersionHasConflict($contentServiceMock, 'eng-GB');

self::assertFalse($versionHasConflict->isSatisfiedBy($this->createVersionInfo(false, 5)));
}
Expand All @@ -47,27 +47,66 @@ public function testVersionWithStatusDraftAndVersionConflict()
$this->createVersionInfo(true, 4),
]);

$versionHasConflict = new VersionHasConflict($contentServiceMock);
$versionHasConflict = new VersionHasConflict($contentServiceMock, 'eng-GB');

self::assertTrue($versionHasConflict->isSatisfiedBy($this->createVersionInfo(false, 2)));
}

public function testVersionWithStatusDraftAndVersionConflictWithAnotherLanguageCode()
{
$contentServiceMock = $this->createMock(ContentService::class);
$contentServiceMock
->method('loadVersions')
->willReturn([
$this->createVersionInfo(false, 1, 'pol-PL'),
$this->createVersionInfo(false, 3, 'pol-PL'),
$this->createVersionInfo(true, 4, 'pol-PL'),
]);

$versionHasConflict = new VersionHasConflict($contentServiceMock, 'eng-GB');

self::assertFalse($versionHasConflict->isSatisfiedBy($this->createVersionInfo(false, 2, 'eng-GB')));
}

/**
* Returns VersionInfo.
*
* @param bool $isPublished
* @param int $versionNo
* @param string $languageCode
*
* @return VersionInfo|MockObject
*/
private function createVersionInfo(bool $isPublished = false, int $versionNo = 1): VersionInfo
private function createVersionInfo(bool $isPublished = false, int $versionNo = 1, string $languageCode = 'eng-GB'): VersionInfo
{
$contentInfo = $this->createMock(ContentInfo::class);

$versionInfo = $this->getMockForAbstractClass(VersionInfo::class, [], '', true, true, true, ['isPublished', '__get', 'getContentInfo']);
$versionInfo->method('isPublished')->willReturn($isPublished);
$versionInfo->method('__get')->with($this->equalTo('versionNo'))->willReturn($versionNo);
$versionInfo->method('getContentInfo')->willReturn($contentInfo);
$versionInfo = $this->getMockForAbstractClass(
VersionInfo::class,
[],
'',
true,
true,
true,
['isPublished', '__get', 'getContentInfo']
);

$versionInfo
->method('isPublished')
->willReturn($isPublished);

$versionInfo
->method('__get')
->will($this->returnValueMap(
[
['initialLanguageCode', $languageCode],
['versionNo', $versionNo],
]
));

$versionInfo
->method('getContentInfo')
->willReturn($contentInfo);

return $versionInfo;
}
Expand Down
Loading

0 comments on commit 6c5b0f9

Please sign in to comment.