From 9d5ac6dfcfe8ca1bbfeee6a184daa4b3ba4c4d62 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Fri, 24 Oct 2025 13:40:57 +0200 Subject: [PATCH 1/2] IBX-10854: Not possible to hide content draft --- src/lib/Repository/ContentService.php | 38 ++++++++++++++++----------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/lib/Repository/ContentService.php b/src/lib/Repository/ContentService.php index 9c4fe46152..20678b6d79 100644 --- a/src/lib/Repository/ContentService.php +++ b/src/lib/Repository/ContentService.php @@ -2531,14 +2531,17 @@ public function deleteTranslationFromDraft(APIVersionInfo $versionInfo, string $ */ 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) { + $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(); @@ -2571,14 +2574,17 @@ public function hideContent(ContentInfo $contentInfo): void */ 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(); From 80a7dc1aeebf045b3d246866b42164f9484ba9e5 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Fri, 24 Oct 2025 14:13:16 +0200 Subject: [PATCH 2/2] Added tests for IBX-10854: Not possible to hide content draft --- phpstan-baseline.neon | 5 - src/lib/Repository/ContentService.php | 45 ++-- .../Core/Repository/ContentServiceTest.php | 236 ++++++++++++++++-- 3 files changed, 237 insertions(+), 49 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4adced4037..16941fd93c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -33768,11 +33768,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\, iterable\ given\.$#' diff --git a/src/lib/Repository/ContentService.php b/src/lib/Repository/ContentService.php index 20678b6d79..e4792e39aa 100644 --- a/src/lib/Repository/ContentService.php +++ b/src/lib/Repository/ContentService.php @@ -2531,17 +2531,17 @@ public function deleteTranslationFromDraft(APIVersionInfo $versionInfo, string $ */ public function hideContent(ContentInfo $contentInfo): void { - // 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]); - } + // 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, + $targets + )) { + throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]); } $this->repository->beginTransaction(); @@ -2574,17 +2574,18 @@ public function hideContent(ContentInfo $contentInfo): void */ public function revealContent(ContentInfo $contentInfo): void { - // 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]); - } + // 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, + $targets + )) { + throw new UnauthorizedException('content', 'hide', ['contentId' => $contentInfo->id]); } $this->repository->beginTransaction(); diff --git a/tests/integration/Core/Repository/ContentServiceTest.php b/tests/integration/Core/Repository/ContentServiceTest.php index b609acec3c..8c7f1ff590 100644 --- a/tests/integration/Core/Repository/ContentServiceTest.php +++ b/tests/integration/Core/Repository/ContentServiceTest.php @@ -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); // Trashing Content's last Location will change its status to archived, // in this case relation towards it will not be loaded. @@ -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. @@ -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); @@ -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); @@ -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. @@ -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); @@ -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 ); @@ -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); @@ -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); @@ -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 + */ + 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 */ @@ -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, @@ -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( + $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,