Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b3e7707
IBX-10854: Not possible to hide content draft
vidarl Oct 24, 2025
37f717f
Added tests for IBX-10854: Not possible to hide content draft
vidarl Oct 24, 2025
70d5de8
fixup! IBX-10854: Not possible to hide content draft
vidarl Oct 27, 2025
2f5ff0e
fixup! IBX-10854: Not possible to hide content draft
vidarl Feb 10, 2026
79d1d5d
fixup! fixup! IBX-10854: Not possible to hide content draft
vidarl Feb 10, 2026
7ec84c3
fixup! Added tests for IBX-10854: Not possible to hide content draft
vidarl Feb 10, 2026
082cdc4
Fixed PHPstan
vidarl Feb 10, 2026
bdcfc49
fixup! fixup! Added tests for IBX-10854: Not possible to hide content…
vidarl Feb 10, 2026
510f0fc
fixup! fixup! fixup! Added tests for IBX-10854: Not possible to hide …
vidarl Feb 24, 2026
17caab8
Revert "fixup! fixup! fixup! Added tests for IBX-10854: Not possible …
vidarl Feb 24, 2026
f4ed055
Apply suggestions from code review for Tests
vidarl Mar 6, 2026
ecb639f
Apply suggestions from code review
vidarl Mar 6, 2026
bc5306c
fixup! Apply suggestions from code review for Tests
vidarl Mar 9, 2026
d82173c
fixup! Added tests for IBX-10854: Not possible to hide content draft
vidarl Mar 11, 2026
a2b9550
fixup! fixup! Added tests for IBX-10854: Not possible to hide content…
vidarl Mar 11, 2026
b2ffb94
fixup! fixup! Added tests for IBX-10854: Not possible to hide content…
vidarl Mar 11, 2026
8a4ab35
fixup! IBX-10854: Not possible to hide content draft
vidarl Mar 11, 2026
c6617ff
Apply suggestions from code review
vidarl Apr 20, 2026
17173e9
fixup! Apply suggestions from code review
vidarl Apr 20, 2026
57300ea
Apply suggestions from code review
vidarl May 20, 2026
14e3fdd
Apply suggestions from code review on tests
vidarl May 20, 2026
c2ea69b
fixup! Apply suggestions from code review on tests
vidarl May 20, 2026
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: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -33782,11 +33782,6 @@ parameters:
count: 4
path: tests/integration/Core/Repository/ContentServiceTest.php

-
message: '#^Parameter \#1 \$locationId of method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:loadLocation\(\) expects int, int\|null given\.$#'
identifier: argument.type
count: 12
path: tests/integration/Core/Repository/ContentServiceTest.php

-
message: '#^Parameter \#1 \$locations of method Ibexa\\Tests\\Integration\\Core\\Repository\\ContentServiceTest\:\:filterHiddenLocations\(\) expects array\<Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location\>, iterable\<Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location\> given\.$#'
Expand Down
15 changes: 11 additions & 4 deletions src/lib/Repository/ContentService.php
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right. Fixed in ca2b306.
Unsure if it 100% safe to have $targets=[] in parameter to canUser() though, but seems to work fine for me when I test with limitation on content/hide policy

@Steveb-p

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Explanation of what happens when $targets=[] is addressed in #667 (comment)

@Steveb-p

Original file line number Diff line number Diff line change
Expand Up @@ -2531,12 +2531,15 @@ public function deleteTranslationFromDraft(APIVersionInfo $versionInfo, string $
*/
public function hideContent(ContentInfo $contentInfo): void
{
$locationTarget = (new DestinationLocationTarget($contentInfo->mainLocationId, $contentInfo));
// If ContentInfo is in draft state, mainLocationId is yet not set
$targets = $contentInfo->isDraft()
? []
: [new DestinationLocationTarget($contentInfo->getMainLocationId(), $contentInfo)];
if (!$this->permissionResolver->canUser(
'content',
'hide',
$contentInfo,
[$locationTarget]
$targets
)) {
throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]);
}
Expand Down Expand Up @@ -2571,12 +2574,16 @@ public function hideContent(ContentInfo $contentInfo): void
*/
public function revealContent(ContentInfo $contentInfo): void
{
$locationTarget = (new DestinationLocationTarget($contentInfo->mainLocationId, $contentInfo));
// If ContentInfo is in draft state, mainLocationId is yet not set
$targets = $contentInfo->isDraft()
? []
: [new DestinationLocationTarget($contentInfo->getMainLocationId(), $contentInfo)];

if (!$this->permissionResolver->canUser(
'content',
'hide',
$contentInfo,
[$locationTarget]
$targets
)) {
throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]);
}
Expand Down
236 changes: 214 additions & 22 deletions tests/integration/Core/Repository/ContentServiceTest.php
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note

These are legacy integration tests, we shouldn't expand it but rather use RepositoryTestCase. But given you've put some work into it already, I'll allow it. Just saying for the future :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sorry, this confuses me. So new integration tests should extend Ibexa\Tests\Integration\Core\RepositoryTestCase, not Ibexa\Tests\Integration\Core\Repository\BaseTest, but still be placed in the the Ibexa\Tests\Integration\Core\Repository\ namespace. correct ?

Original file line number Diff line number Diff line change
Expand Up @@ -3610,7 +3610,9 @@ public function testLoadRelationsSkipsArchivedContent()
$demoDesign
);

$demoDesignLocation = $this->locationService->loadLocation($demoDesign->mainLocationId);
$demoDesignMainLocationId = $demoDesign->getMainLocationId();
self::assertNotNull($demoDesignMainLocationId, 'Expected mainLocationId to be set for this test case.');
$demoDesignLocation = $this->locationService->loadLocation($demoDesignMainLocationId);
Comment thread
alongosz marked this conversation as resolved.

// Trashing Content's last Location will change its status to archived,
// in this case relation towards it will not be loaded.
Expand Down Expand Up @@ -3936,7 +3938,9 @@ public function testLoadReverseRelationsSkipsArchivedContent()
$this->contentService->publishVersion($mediaDraft->getVersionInfo());
$this->contentService->publishVersion($demoDesignDraft->getVersionInfo());

$demoDesignLocation = $this->locationService->loadLocation($demoDesignDraft->contentInfo->mainLocationId);
$demoDesignLocationId = $demoDesignDraft->getContentInfo()->getMainLocationId();
self::assertNotNull($demoDesignLocationId, 'Expected mainLocationId to be set for this test case.');
$demoDesignLocation = $this->locationService->loadLocation($demoDesignLocationId);

// Trashing Content's last Location will change its status to archived,
// in this case relation from it will not be loaded.
Expand Down Expand Up @@ -4138,7 +4142,9 @@ public function testLoadReverseRelationListSkipsArchivedContent(): void
$draft3,
]);

$locationToTrash = $this->locationService->loadLocation($draft3->contentInfo->mainLocationId);
$draft3MainLocationId = $draft3->getContentInfo()->getMainLocationId();
self::assertNotNull($draft3MainLocationId, 'Expected mainLocationId to be set for this test case.');
$locationToTrash = $this->locationService->loadLocation($draft3MainLocationId);

// Trashing Content's last Location will change its status to archived, in this case relation from it will not be loaded.
$trashService->trash($locationToTrash);
Expand Down Expand Up @@ -5100,9 +5106,9 @@ public function testURLAliasesCreatedForNewContent()
// Automatically creates a new URLAlias for the content
$liveContent = $this->contentService->publishVersion($draft->getVersionInfo());

$location = $this->locationService->loadLocation(
$liveContent->getVersionInfo()->getContentInfo()->mainLocationId
);
$liveContentInfoMainLocationId = $liveContent->getVersionInfo()->getContentInfo()->getMainLocationId();
self::assertNotNull($liveContentInfoMainLocationId, 'Expected mainLocationId to be set for this test case.');
$location = $this->locationService->loadLocation($liveContentInfoMainLocationId);

$aliases = $urlAliasService->listLocationAliases($location, false);

Expand All @@ -5128,9 +5134,9 @@ public function testURLAliasesCreatedForUpdatedContent()

$draft = $this->createUpdatedDraftVersion2();

$location = $this->locationService->loadLocation(
$draft->getVersionInfo()->getContentInfo()->mainLocationId
);
$draftMainLocationId = $draft->getVersionInfo()->getContentInfo()->getMainLocationId();
self::assertNotNull($draftMainLocationId, 'Expected mainLocationId to be set for this test case.');
$location = $this->locationService->loadLocation($draftMainLocationId);

// Load and assert URL aliases before publishing updated Content, so that
// SPI cache is warmed up and cache invalidation is also tested.
Expand All @@ -5156,9 +5162,9 @@ public function testURLAliasesCreatedForUpdatedContent()
// and creates new aliases, based on the changes
$liveContent = $this->contentService->publishVersion($draft->getVersionInfo());

$location = $this->locationService->loadLocation(
$liveContent->getVersionInfo()->getContentInfo()->mainLocationId
);
$liveContentInfoMainLocationId = $liveContent->getVersionInfo()->getContentInfo()->getMainLocationId();
self::assertNotNull($liveContentInfoMainLocationId, 'Expected mainLocationId to be set for this test case.');
$location = $this->locationService->loadLocation($liveContentInfoMainLocationId);

$aliases = $urlAliasService->listLocationAliases($location, false);

Expand Down Expand Up @@ -5195,11 +5201,11 @@ public function testCustomURLAliasesNotHistorizedOnUpdatedContent()

$content = $this->createContentVersion1();

$contentMainLocationId = $content->getVersionInfo()->getContentInfo()->getMainLocationId();
self::assertNotNull($contentMainLocationId, 'Expected mainLocationId to be set for this test case.');
// Create a custom URL alias
$urlAliasService->createUrlAlias(
$this->locationService->loadLocation(
$content->getVersionInfo()->getContentInfo()->mainLocationId
),
$this->locationService->loadLocation($contentMainLocationId),
'/my/fancy/story-about-ibexa-dxp',
self::ENG_US
);
Expand All @@ -5219,9 +5225,9 @@ public function testCustomURLAliasesNotHistorizedOnUpdatedContent()
// the custom one is left untouched
$liveContent = $this->contentService->publishVersion($draftVersion2->getVersionInfo());

$location = $this->locationService->loadLocation(
$liveContent->getVersionInfo()->getContentInfo()->mainLocationId
);
$liveContentMainLocationId = $liveContent->getVersionInfo()->getContentInfo()->getMainLocationId();
self::assertNotNull($liveContentMainLocationId, 'Expected mainLocationId to be set for this test case.');
$location = $this->locationService->loadLocation($liveContentMainLocationId);

$aliases = $urlAliasService->listLocationAliases($location);

Expand Down Expand Up @@ -5391,7 +5397,12 @@ public function testDeleteTranslationUpdatesUrlAlias()
$urlAliasService = $this->getRepository()->getURLAliasService();

$content = $this->createContentVersion2();
$mainLocation = $this->locationService->loadLocation($content->contentInfo->mainLocationId);
$contentMainLocationId = $content->getContentInfo()->getMainLocationId();
self::assertNotNull(
$contentMainLocationId,
'Expected mainLocationId to be set for this test case.'
);
$mainLocation = $this->locationService->loadLocation($contentMainLocationId);

// create custom URL alias for Content main Location
$urlAliasService->createUrlAlias($mainLocation, '/my-custom-url', self::ENG_GB);
Expand Down Expand Up @@ -6229,6 +6240,171 @@ function (Location $parentLocation) {
$this->assertEquals($hiddenLocations, $hiddenLocationsAfterReveal);
}

/**
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @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 testPublishHiddenDraft(): void
{
$draft = $this->createFolderDraft();
$draftContentInfo = $draft->getContentInfo();
$this->contentService->hideContent($draftContentInfo);

$publishedContent = $this->contentService->publishVersion($draft->getVersionInfo());
$contentInfo = $publishedContent->getContentInfo();

self::assertTrue($contentInfo->isHidden(), 'Content is not hidden');

$mainLocationId = $contentInfo->getMainLocationId();

self::assertNotNull(
$mainLocationId,
'Expected mainLocationId to be set for this test case.'
);

$location = $this->locationService->loadLocation($mainLocationId);
self::assertTrue($location->isHidden(), 'Location is visible');
}

/**
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @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 testPublishRevealedDraft(): void
{
$draft = $this->createFolderDraft();
$draftContentInfo = $draft->getContentInfo();

$this->contentService->hideContent($draftContentInfo);
self::assertTrue(
$this->contentService
->loadContent($draftContentInfo->getId())
->getContentInfo()
->isHidden()
);

$this->contentService->revealContent($draftContentInfo);
self::assertFalse(
$this->contentService
->loadContent($draftContentInfo->getId())
->getContentInfo()
->isHidden()
);

$publishedContent = $this->contentService->publishVersion(
$draft->getVersionInfo()
);

$contentInfo = $publishedContent->getContentInfo();
$mainLocationId = $contentInfo->getMainLocationId();

self::assertFalse($contentInfo->isHidden(), 'Content is hidden');
self::assertNotNull(
$mainLocationId,
'Expected mainLocationId to be set for this test case.'
);

$location = $this->locationService->loadLocation($mainLocationId);

self::assertFalse($location->isHidden(), 'Location is hidden');
}

/**
* @dataProvider draftVisibilityTransitionsProvider
*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @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 testDraftVisibilityTransitions(
bool $initiallyHidden,
bool $secondDraftHidden
): void {
$draft = $this->createFolderDraft();

if ($initiallyHidden) {
$this->contentService->hideContent($draft->getContentInfo());
}

$publishedContent = $this->contentService->publishVersion($draft->getVersionInfo());
$draft2 = $this->contentService->createContentDraft($publishedContent->getContentInfo());

if ($secondDraftHidden) {
$this->contentService->hideContent($draft2->getContentInfo());
} else {
$this->contentService->revealContent($draft2->getContentInfo());
}

$publishedContent2 = $this->contentService->publishVersion($draft2->getVersionInfo());
$contentInfo = $publishedContent2->getContentInfo();

self::assertSame(
$secondDraftHidden,
$contentInfo->isHidden(),
'Unexpected final hidden state for content.'
);

$mainLocationId = $contentInfo->getMainLocationId();

self::assertNotNull(
$mainLocationId,
'Expected mainLocationId to be set.'
);

$location = $this->locationService->loadLocation($mainLocationId);

self::assertSame(
$secondDraftHidden,
$location->isHidden(),
'Unexpected final hidden state for location.'
);
}

/**
* @return iterable<string, array{bool, bool}>
*/
public static function draftVisibilityTransitionsProvider(): iterable
{
yield 'hidden -> hidden' => [true, true];
yield 'hidden -> visible' => [true, false];
yield 'visible -> hidden' => [false, true];
yield 'visible -> visible' => [false, false];
}

/**
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @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 createFolderDraft(): 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');

return $this->contentService->createContent(
$contentCreate,
[$locationCreateStructs]
);
}

/**
* @depends testRevealContent
*/
Expand Down Expand Up @@ -6270,7 +6446,13 @@ public function testRevealContentWithHiddenParent()
$this->contentService->revealContent($contents[2]->contentInfo);

$parentContent = $this->contentService->loadContent($contents[0]->id);
$parentLocation = $this->locationService->loadLocation($parentContent->contentInfo->mainLocationId);
$parentContentMainLocationId = $parentContent->getContentInfo()->getMainLocationId();

self::assertNotNull(
$parentContentMainLocationId,
'Expected mainLocationId to be set for this test case.'
);
$parentLocation = $this->locationService->loadLocation($parentContentMainLocationId);
$parentSublocations = $this->locationService->loadLocationList([
$contents[1]->contentInfo->mainLocationId,
$contents[2]->contentInfo->mainLocationId,
Expand Down Expand Up @@ -6328,10 +6510,20 @@ public function testRevealContentWithHiddenChildren()
$this->contentService->revealContent($contents[0]->contentInfo);

$directChildContent = $this->contentService->loadContent($contents[1]->id);
$directChildLocation = $this->locationService->loadLocation($directChildContent->contentInfo->mainLocationId);
$directChildContentMainLocationId = $directChildContent->getContentInfo()->getMainLocationId();
self::assertNotNull(
Comment thread
vidarl marked this conversation as resolved.
$directChildContentMainLocationId,
'Expected mainLocationId to be set for this test case.'
);
$directChildLocation = $this->locationService->loadLocation($directChildContentMainLocationId);

$childContent = $this->contentService->loadContent($contents[2]->id);
$childLocation = $this->locationService->loadLocation($childContent->contentInfo->mainLocationId);
$childContentMainLocationId = $childContent->getContentInfo()->getMainLocationId();
self::assertNotNull(
$childContentMainLocationId,
'Expected mainLocationId to be set for this test case.'
);
$childLocation = $this->locationService->loadLocation($childContentMainLocationId);
$childSublocations = $this->locationService->loadLocationList([
$contents[3]->contentInfo->mainLocationId,
$contents[4]->contentInfo->mainLocationId,
Expand Down
Loading