Skip to content

IBX-10854 not possible to hide content draft #667

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

Open
wants to merge 3 commits into
base: 4.6
Choose a base branch
from
Open

IBX-10854 not possible to hide content draft #667

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed818bb
IBX-10854: Not possible to hide content draft
vidarl Oct 24, 2025
3dfb2c7
Added tests for IBX-10854: Not possible to hide content draft
vidarl Oct 24, 2025
acbb2d8
fixup! IBX-10854: Not possible to hide content draft
vidarl Oct 27, 2025
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
38 changes: 22 additions & 16 deletions src/lib/Repository/ContentService.php
View file
Open in desktop
Copy link
Contributor

@Steveb-p Steveb-p Nov 18, 2025

Choose a reason for hiding this comment

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

Doesn't this mean, with this change, that for drafts every user has ability to hide & reveal?

Because we won't be checking permission resolver for drafts.

Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<?php

/**
Expand Down Expand Up @@ -2528,14 +2528,17 @@
*/
public function hideContent(ContentInfo $contentInfo): void
{
$locationTarget = (new DestinationLocationTarget($contentInfo->mainLocationId, $contentInfo));
if (!$this->permissionResolver->canUser(
'content',
'hide',
$contentInfo,
[$locationTarget]
)) {
throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]);
// If content is in draft state, mainLocationId is yet not set
if ($contentInfo->mainLocationId !== null) {
Copy link
Contributor

@Steveb-p Steveb-p Oct 24, 2025

Choose a reason for hiding this comment

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

If this is true, then a dedicated ContentInfo method should probably be introduced.

Copy link
Contributor Author

@vidarl vidarl Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@Steveb-p
Okay, so I first thought about adding function like this:

+    public function isInitialDraft(): bool
+    {
+        return $this->status === self::STATUS_DRAFT && $this->currentVersionNo === 1;
+    }

However, then I realized that this is not needed for ContentInfo as existing ContentInfo::isDraft() will do just fine. This method will only return true if Content in question has never been published. Once Content is published, ContentInfo::isDraft() will still return false when you create new drafts for that content.

Fixed in acbb2d8

$locationTarget = (new DestinationLocationTarget($contentInfo->mainLocationId, $contentInfo));
if (!$this->permissionResolver->canUser(
'content',
'hide',
$contentInfo,
[$locationTarget]
)) {
throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]);
}
}

$this->repository->beginTransaction();
Expand Down Expand Up @@ -2568,14 +2571,17 @@
*/
public function revealContent(ContentInfo $contentInfo): void
{
$locationTarget = (new DestinationLocationTarget($contentInfo->mainLocationId, $contentInfo));
if (!$this->permissionResolver->canUser(
'content',
'hide',
$contentInfo,
[$locationTarget]
)) {
throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]);
// If content is in draft state, mainLocationId is yet not set
if ($contentInfo->mainLocationId !== null) {
$locationTarget = (new DestinationLocationTarget($contentInfo->mainLocationId, $contentInfo));
if (!$this->permissionResolver->canUser(
'content',
'hide',
$contentInfo,
[$locationTarget]
)) {
throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]);
}
}

$this->repository->beginTransaction();
Expand Down
70 changes: 70 additions & 0 deletions tests/integration/Core/Repository/ContentServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3697,7 +3697,7 @@
{
$draft = $this->createContentDraftVersion1();

self::assertSame(0, $this->contentService->countRelations($draft->getVersionInfo()));

Check failure on line 3700 in tests/integration/Core/Repository/ContentServiceTest.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3) 

Ignored error pattern #^Parameter \#1 \$locationId of method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:loadLocation\(\) expects int, int\|null given\.$# (argument.type) in path /home/runner/work/core/core/tests/integration/Core/Repository/ContentServiceTest.php is expected to occur 12 times, but occurred 14 times.
}

public function testCountRelationsForUnauthorizedUser(): void
Expand Down Expand Up @@ -6229,6 +6229,76 @@
$this->assertEquals($hiddenLocations, $hiddenLocationsAfterReveal);
}

/**
* @covers \Ibexa\Contracts\Core\Repository\ContentService::hideContent

*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\APIInvalidArgumentException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\ContentFieldValidationException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\ContentValidationException
*/
public function testHideContentDraft(): void
{
$publishedContent = $this->createContentForHideRevealDraftTests(false);
$location = $this->locationService->loadLocation($publishedContent->contentInfo->mainLocationId);

$content = $this->contentService->loadContent($publishedContent->contentInfo->id);
self::assertTrue($content->contentInfo->isHidden, 'Content is not hidden');
self::assertTrue($location->isHidden(), 'Location is visible');
}

/**
* @covers \Ibexa\Contracts\Core\Repository\ContentService::revealContent

*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException
*/
public function testHideAndRevealContentDraft(): void
{
$publishedContent = $this->createContentForHideRevealDraftTests(true);
$location = $this->locationService->loadLocation($publishedContent->contentInfo->mainLocationId);

$content = $this->contentService->loadContent($publishedContent->contentInfo->id);
self::assertFalse($content->contentInfo->isHidden, 'Content is hidden');
self::assertFalse($location->isHidden(), 'Location is hidden');
}

/**
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\APIInvalidArgumentException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\ContentFieldValidationException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\ContentValidationException
*/
private function createContentForHideRevealDraftTests(bool $hideAndRevel): Content
{
$contentTypeService = $this->getRepository()->getContentTypeService();

$locationCreateStructs = $this->locationService->newLocationCreateStruct(2);

$contentType = $contentTypeService->loadContentTypeByIdentifier('folder');

$contentCreate = $this->contentService->newContentCreateStruct($contentType, self::ENG_US);
$contentCreate->setField('name', 'Folder to hide');

$draft = $this->contentService->createContent(
$contentCreate,
[$locationCreateStructs]
);

$this->contentService->hideContent($draft->contentInfo);
if ($hideAndRevel) {
$this->contentService->revealContent($draft->contentInfo);
}

return $this->contentService->publishVersion($draft->versionInfo);
}

/**
* @depends testRevealContent
*/
Expand Down Expand Up @@ -6357,7 +6427,7 @@

public function testHideContentWithParentLocation()
{
$contentTypeService = $this->getRepository()->getContentTypeService();

Check failure on line 6430 in tests/integration/Core/Repository/ContentServiceTest.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3) 

PHPDoc tag `@throws` with type Ibexa\Contracts\Core\Repository\Exceptions\APIInvalidArgumentException|Ibexa\Contracts\Core\Repository\Exceptions\BadStateException|Ibexa\Contracts\Core\Repository\Exceptions\ContentFieldValidationException|Ibexa\Contracts\Core\Repository\Exceptions\ContentValidationException|Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException|Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException is not subtype of Throwable

$contentType = $contentTypeService->loadContentTypeByIdentifier('folder');

Expand Down Expand Up @@ -6392,7 +6462,7 @@
);

$publishedChildContent = $this->contentService->publishVersion($childContent->versionInfo);

Check failure on line 6465 in tests/integration/Core/Repository/ContentServiceTest.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3) 

PHPDoc tag `@throws` with type Ibexa\Contracts\Core\Repository\Exceptions\APIInvalidArgumentException|Ibexa\Contracts\Core\Repository\Exceptions\BadStateException|Ibexa\Contracts\Core\Repository\Exceptions\ContentFieldValidationException|Ibexa\Contracts\Core\Repository\Exceptions\ContentValidationException|Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException|Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException is not subtype of Throwable
$childLocations = $this->locationService->loadLocations($publishedChildContent->contentInfo);

$this->assertTrue($locations[0]->hidden);
Expand Down Expand Up @@ -6515,10 +6585,10 @@
]
);
$publishedContent = $this->contentService->publishVersion($contentDraft->versionInfo);

Check failure on line 6588 in tests/integration/Core/Repository/ContentServiceTest.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3) 

Parameter #1 $locationId of method Ibexa\Contracts\Core\Repository\LocationService::loadLocation() expects int, int|null given.
// Create translation draft that would act as an OLD version
$deDraft = $this->contentService->createContentDraft($publishedContent->contentInfo);
$contentUpdateStruct = new ContentUpdateStruct([

Check failure on line 6591 in tests/integration/Core/Repository/ContentServiceTest.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3) 

Parameter #1 $locationId of method Ibexa\Contracts\Core\Repository\LocationService::loadLocation() expects int, int|null given.
'initialLanguageCode' => self::GER_DE,
'fields' => $contentDraft->getFields(),
]);
Expand Down
Loading