From d89c9230366ff7e78bd9f07e38d94b4de21c301a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Fri, 17 Jul 2026 14:43:10 +0200 Subject: [PATCH 1/3] IBX-11780: Integrated ContentPublicationStrategyInterface into ContentFormProcessor publish flow, replaced content payload with content_type and covered processPublish with unit tests --- src/bundle/Resources/config/services.yaml | 1 + .../Form/Processor/ContentFormProcessor.php | 30 +- .../Processor/ContentFormProcessorTest.php | 330 ++++++++++++++++++ 3 files changed, 354 insertions(+), 7 deletions(-) create mode 100644 tests/lib/Form/Processor/ContentFormProcessorTest.php diff --git a/src/bundle/Resources/config/services.yaml b/src/bundle/Resources/config/services.yaml index f2b92354..42d61687 100644 --- a/src/bundle/Resources/config/services.yaml +++ b/src/bundle/Resources/config/services.yaml @@ -58,6 +58,7 @@ services: - '@ibexa.api.service.content' - '@ibexa.api.service.location' - '@router' + - '@Ibexa\Contracts\Core\Repository\Strategy\ContentPublication\ContentPublicationStrategyInterface' tags: - { name: kernel.event_subscriber } diff --git a/src/lib/Form/Processor/ContentFormProcessor.php b/src/lib/Form/Processor/ContentFormProcessor.php index 0222971f..fae2827e 100644 --- a/src/lib/Form/Processor/ContentFormProcessor.php +++ b/src/lib/Form/Processor/ContentFormProcessor.php @@ -15,6 +15,7 @@ use Ibexa\ContentForms\Event\FormActionEvent; use Ibexa\Contracts\Core\Repository\ContentService; use Ibexa\Contracts\Core\Repository\LocationService; +use Ibexa\Contracts\Core\Repository\Strategy\ContentPublication\ContentPublicationStrategyInterface; use Ibexa\Contracts\Core\Repository\Values\Content\Content; use Ibexa\Contracts\Core\Repository\Values\Content\ContentStruct; use Ibexa\Contracts\Core\Repository\Values\Content\Location; @@ -33,7 +34,8 @@ public function __construct( private ContentService $contentService, private LocationService $locationService, - private RouterInterface $router + private RouterInterface $router, + private ContentPublicationStrategyInterface $contentPublicationStrategy ) { } @@ -150,19 +152,33 @@ public function processPublish(FormActionEvent $event): void $draft = $this->saveDraft($data, $form->getConfig()->getOption('languageCode')); $versionInfo = $draft->getVersionInfo(); - $content = $this->contentService->publishVersion( + $publicationResult = $this->contentPublicationStrategy->publishVersion( $versionInfo, [$versionInfo->getInitialLanguage()->getLanguageCode()] ); - $event->setPayload('content', $content); + $event->setPayload('content_type', $draft->getContentType()); $event->setPayload('is_new', $draft->getContentInfo()->isDraft()); - $locationId = $referrerLocation !== null && $data instanceof ContentUpdateData - ? $referrerLocation->id - : $content->getContentInfo()->getMainLocationId(); + $publishedContent = $publicationResult->publishedContent; + if ($publishedContent !== null) { + $locationId = $referrerLocation !== null && $data instanceof ContentUpdateData + ? $referrerLocation->id + : $publishedContent->getContentInfo()->getMainLocationId(); + + $contentId = $publishedContent->getId(); + } else { + // The publication is deferred to background processing; the published version and, + // for never-published content, its main location do not exist yet. + + // TODO: handle null location for async/content creation path, where we should redirect user to? + $locationId = $referrerLocation !== null && $data instanceof ContentUpdateData + ? $referrerLocation->id + : $draft->getContentInfo()->getMainLocationId(); + + $contentId = $draft->getContentInfo()->getId(); + } - $contentId = $content->getId(); $redirectUrl = $form['redirectUrlAfterPublish']->getData() ?: $this->router->generate( 'ibexa.content.view', [ diff --git a/tests/lib/Form/Processor/ContentFormProcessorTest.php b/tests/lib/Form/Processor/ContentFormProcessorTest.php new file mode 100644 index 00000000..b279c5ce --- /dev/null +++ b/tests/lib/Form/Processor/ContentFormProcessorTest.php @@ -0,0 +1,330 @@ +createDraft(self::DRAFT_MAIN_LOCATION_ID, false); + $versionInfo = $draft->getVersionInfo(); + + $contentService = $this->createMock(ContentService::class); + $contentService->method('updateContent')->willReturn($draft); + $contentService + ->expects(self::never()) + ->method('publishVersion'); + + $contentPublicationStrategy = $this->createMock(ContentPublicationStrategyInterface::class); + $contentPublicationStrategy + ->expects(self::once()) + ->method('publishVersion') + ->with(self::identicalTo($versionInfo), [self::LANGUAGE_CODE]) + ->willReturn(new ContentPublicationResult($this->createPublishedContent())); + + $processor = new ContentFormProcessor( + $contentService, + $this->createStub(LocationService::class), + $this->createRouterStub(), + $contentPublicationStrategy + ); + + $processor->processPublish( + $this->createEvent($this->createUpdateData($draft), $this->createForm()) + ); + } + + /** + * @testWith [true, true] + * [false, false] + */ + public function testProcessPublishSetsPayloads(bool $isNewContent, bool $expectedIsNewPayload): void + { + $draft = $this->createDraft($isNewContent ? null : self::DRAFT_MAIN_LOCATION_ID, $isNewContent); + $data = $isNewContent ? $this->createCreateData() : $this->createUpdateData($draft); + $event = $this->createEvent($data, $this->createForm()); + + $processor = $this->createProcessor($draft, new ContentPublicationResult(null)); + $processor->processPublish($event); + + self::assertFalse($event->hasPayload('content')); + self::assertSame($draft->getContentType(), $event->getPayload('content_type')); + self::assertSame($expectedIsNewPayload, $event->getPayload('is_new')); + } + + /** + * @dataProvider providerForTestProcessPublishRedirect + * + * @param array $expectedRouteParameters + */ + public function testProcessPublishRedirect( + bool $publishedSynchronously, + bool $isNewContent, + bool $withReferrerLocation, + array $expectedRouteParameters + ): void { + $draft = $this->createDraft($isNewContent ? null : self::DRAFT_MAIN_LOCATION_ID, $isNewContent); + $data = $isNewContent ? $this->createCreateData() : $this->createUpdateData($draft); + $publicationResult = new ContentPublicationResult( + $publishedSynchronously ? $this->createPublishedContent() : null + ); + + $router = $this->createMock(RouterInterface::class); + $router + ->expects(self::once()) + ->method('generate') + ->with('ibexa.content.view', $expectedRouteParameters) + ->willReturn(self::GENERATED_URL); + + $options = $withReferrerLocation + ? ['referrerLocation' => new Location(['id' => self::REFERRER_LOCATION_ID])] + : []; + $event = $this->createEvent($data, $this->createForm(), $options); + + $processor = $this->createProcessor($draft, $publicationResult, $router); + $processor->processPublish($event); + + $response = $event->getResponse(); + self::assertInstanceOf(RedirectResponse::class, $response); + self::assertSame(self::GENERATED_URL, $response->getTargetUrl()); + } + + /** + * @return iterable}> + */ + public static function providerForTestProcessPublishRedirect(): iterable + { + yield 'sync: update with referrer location' => [ + true, + false, + true, + [ + 'contentId' => self::CONTENT_ID, + 'locationId' => self::REFERRER_LOCATION_ID, + 'publishedContentId' => self::CONTENT_ID, + ], + ]; + + yield 'sync: update without referrer location' => [ + true, + false, + false, + [ + 'contentId' => self::CONTENT_ID, + 'locationId' => self::PUBLISHED_MAIN_LOCATION_ID, + 'publishedContentId' => self::CONTENT_ID, + ], + ]; + + yield 'sync: new content' => [ + true, + true, + false, + [ + 'contentId' => self::CONTENT_ID, + 'locationId' => self::PUBLISHED_MAIN_LOCATION_ID, + 'publishedContentId' => self::CONTENT_ID, + ], + ]; + + yield 'async: update with referrer location' => [ + false, + false, + true, + [ + 'contentId' => self::CONTENT_ID, + 'locationId' => self::REFERRER_LOCATION_ID, + 'publishedContentId' => self::CONTENT_ID, + ], + ]; + + yield 'async: update without referrer location' => [ + false, + false, + false, + [ + 'contentId' => self::CONTENT_ID, + 'locationId' => self::DRAFT_MAIN_LOCATION_ID, + 'publishedContentId' => self::CONTENT_ID, + ], + ]; + + yield 'async: new content without location yet' => [ + false, + true, + false, + [ + 'contentId' => self::CONTENT_ID, + 'locationId' => null, + 'publishedContentId' => self::CONTENT_ID, + ], + ]; + } + + public function testProcessPublishUsesRedirectUrlAfterPublishFormData(): void + { + $draft = $this->createDraft(self::DRAFT_MAIN_LOCATION_ID, false); + + $router = $this->createMock(RouterInterface::class); + $router + ->expects(self::never()) + ->method('generate'); + + $event = $this->createEvent( + $this->createUpdateData($draft), + $this->createForm('custom-redirect-url') + ); + + $processor = $this->createProcessor($draft, new ContentPublicationResult(null), $router); + $processor->processPublish($event); + + $response = $event->getResponse(); + self::assertInstanceOf(RedirectResponse::class, $response); + self::assertSame('custom-redirect-url', $response->getTargetUrl()); + } + + private function createProcessor( + Content $draft, + ContentPublicationResult $publicationResult, + ?RouterInterface $router = null + ): ContentFormProcessor { + $contentService = $this->createStub(ContentService::class); + $contentService->method('createContent')->willReturn($draft); + $contentService->method('updateContent')->willReturn($draft); + + $contentPublicationStrategy = $this->createStub(ContentPublicationStrategyInterface::class); + $contentPublicationStrategy->method('publishVersion')->willReturn($publicationResult); + + return new ContentFormProcessor( + $contentService, + $this->createStub(LocationService::class), + $router ?? $this->createRouterStub(), + $contentPublicationStrategy + ); + } + + private function createDraft(?int $mainLocationId, bool $neverPublished): Content + { + $contentInfo = new ContentInfo([ + 'id' => self::CONTENT_ID, + 'mainLocationId' => $mainLocationId, + 'mainLanguageCode' => self::LANGUAGE_CODE, + 'status' => $neverPublished ? ContentInfo::STATUS_DRAFT : ContentInfo::STATUS_PUBLISHED, + ]); + + $versionInfo = $this->createStub(VersionInfo::class); + $versionInfo->method('getInitialLanguage')->willReturn( + new Language(['languageCode' => self::LANGUAGE_CODE]) + ); + $versionInfo->method('getContentInfo')->willReturn($contentInfo); + + $draft = $this->createStub(Content::class); + $draft->method('getVersionInfo')->willReturn($versionInfo); + $draft->method('getContentInfo')->willReturn($contentInfo); + $draft->method('getContentType')->willReturn($this->createStub(ContentType::class)); + + return $draft; + } + + private function createPublishedContent(): Content + { + $contentInfo = new ContentInfo([ + 'id' => self::CONTENT_ID, + 'mainLocationId' => self::PUBLISHED_MAIN_LOCATION_ID, + 'mainLanguageCode' => self::LANGUAGE_CODE, + 'status' => ContentInfo::STATUS_PUBLISHED, + ]); + + $publishedContent = $this->createStub(Content::class); + $publishedContent->method('getContentInfo')->willReturn($contentInfo); + $publishedContent->method('getId')->willReturn(self::CONTENT_ID); + + return $publishedContent; + } + + private function createUpdateData(Content $contentDraft): ContentUpdateData + { + return new ContentUpdateData([ + 'contentDraft' => $contentDraft, + 'fieldsData' => [], + ]); + } + + private function createCreateData(): ContentCreateData + { + return new ContentCreateData([ + 'mainLanguageCode' => self::LANGUAGE_CODE, + 'fieldsData' => [], + ]); + } + + private function createForm(?string $redirectUrlAfterPublish = null): FormInterface + { + $formConfig = $this->createStub(FormConfigInterface::class); + $formConfig->method('getOption')->willReturn(self::LANGUAGE_CODE); + + $redirectUrlField = $this->createStub(FormInterface::class); + $redirectUrlField->method('getData')->willReturn($redirectUrlAfterPublish); + + $form = $this->createStub(FormInterface::class); + $form->method('getConfig')->willReturn($formConfig); + $form->method('offsetGet')->willReturn($redirectUrlField); + + return $form; + } + + /** + * @param array $options + */ + private function createEvent( + ContentCreateData|ContentUpdateData $data, + FormInterface $form, + array $options = [] + ): FormActionEvent { + return new FormActionEvent($form, $data, 'publish', $options); + } + + private function createRouterStub(): RouterInterface + { + $router = $this->createStub(RouterInterface::class); + $router->method('generate')->willReturn(self::GENERATED_URL); + + return $router; + } +} From 88eb9a30bb1e46d76452314e505755e4327b6037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Mon, 20 Jul 2026 11:36:00 +0200 Subject: [PATCH 2/3] IBX-11780: Adjusted redirection and improved tests --- src/bundle/Resources/config/services.yaml | 1 + .../Form/Processor/ContentFormProcessor.php | 10 +- .../Processor/ContentFormProcessorTest.php | 262 ++++++++---------- 3 files changed, 128 insertions(+), 145 deletions(-) diff --git a/src/bundle/Resources/config/services.yaml b/src/bundle/Resources/config/services.yaml index 42d61687..ec128802 100644 --- a/src/bundle/Resources/config/services.yaml +++ b/src/bundle/Resources/config/services.yaml @@ -58,6 +58,7 @@ services: - '@ibexa.api.service.content' - '@ibexa.api.service.location' - '@router' + - '@ibexa.config.resolver' - '@Ibexa\Contracts\Core\Repository\Strategy\ContentPublication\ContentPublicationStrategyInterface' tags: - { name: kernel.event_subscriber } diff --git a/src/lib/Form/Processor/ContentFormProcessor.php b/src/lib/Form/Processor/ContentFormProcessor.php index fae2827e..2f51e142 100644 --- a/src/lib/Form/Processor/ContentFormProcessor.php +++ b/src/lib/Form/Processor/ContentFormProcessor.php @@ -20,6 +20,7 @@ use Ibexa\Contracts\Core\Repository\Values\Content\ContentStruct; use Ibexa\Contracts\Core\Repository\Values\Content\Location; use Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo; +use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface; use Ibexa\Core\Base\Exceptions\InvalidArgumentException; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -35,6 +36,7 @@ public function __construct( private ContentService $contentService, private LocationService $locationService, private RouterInterface $router, + private ConfigResolverInterface $configResolver, private ContentPublicationStrategyInterface $contentPublicationStrategy ) { } @@ -165,18 +167,14 @@ public function processPublish(FormActionEvent $event): void $locationId = $referrerLocation !== null && $data instanceof ContentUpdateData ? $referrerLocation->id : $publishedContent->getContentInfo()->getMainLocationId(); - $contentId = $publishedContent->getId(); } else { // The publication is deferred to background processing; the published version and, // for never-published content, its main location do not exist yet. - - // TODO: handle null location for async/content creation path, where we should redirect user to? $locationId = $referrerLocation !== null && $data instanceof ContentUpdateData ? $referrerLocation->id - : $draft->getContentInfo()->getMainLocationId(); - - $contentId = $draft->getContentInfo()->getId(); + : (int) $this->configResolver->getParameter('content.tree_root.location_id'); + $contentId = $this->locationService->loadLocation($locationId)->getContentId(); } $redirectUrl = $form['redirectUrlAfterPublish']->getData() ?: $this->router->generate( diff --git a/tests/lib/Form/Processor/ContentFormProcessorTest.php b/tests/lib/Form/Processor/ContentFormProcessorTest.php index b279c5ce..4abd9f01 100644 --- a/tests/lib/Form/Processor/ContentFormProcessorTest.php +++ b/tests/lib/Form/Processor/ContentFormProcessorTest.php @@ -21,6 +21,7 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Language; use Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo; use Ibexa\Contracts\Core\Repository\Values\ContentType\ContentType; +use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface; use Ibexa\Core\Repository\Values\Content\Location; use PHPUnit\Framework\TestCase; use Symfony\Component\Form\FormConfigInterface; @@ -37,104 +38,120 @@ final class ContentFormProcessorTest extends TestCase private const DRAFT_MAIN_LOCATION_ID = 42; private const PUBLISHED_MAIN_LOCATION_ID = 77; private const REFERRER_LOCATION_ID = 55; + private const TREE_ROOT_LOCATION_ID = 2; private const LANGUAGE_CODE = 'eng-GB'; private const GENERATED_URL = 'generated-url'; - public function testProcessPublishPublishesVersionViaStrategy(): void - { - $draft = $this->createDraft(self::DRAFT_MAIN_LOCATION_ID, false); + /** + * @dataProvider provideProcessPublishCases + * + * @param array|null $expectedRouteParameters route parameters the redirect + * is expected to be generated with, or null when a custom redirect URL is provided and + * the router must not be invoked at all + */ + public function testProcessPublish( + bool $isNewContent, + bool $publishedSynchronously, + bool $withReferrerLocation, + ?string $customRedirectUrl, + ?array $expectedRouteParameters + ): void { + $draft = $this->createDraft( + $isNewContent ? null : self::DRAFT_MAIN_LOCATION_ID, + $isNewContent ? ContentInfo::STATUS_DRAFT : ContentInfo::STATUS_PUBLISHED, + ); $versionInfo = $draft->getVersionInfo(); + $data = $isNewContent ? $this->createCreateData() : $this->createUpdateData($draft); + $options = $withReferrerLocation + ? ['referrerLocation' => new Location(['id' => self::REFERRER_LOCATION_ID])] + : []; + $event = $this->createEvent($data, $this->createForm($customRedirectUrl), $options); + + // The draft is persisted via create/update; publication must never go through the service. $contentService = $this->createMock(ContentService::class); + $contentService->method('createContent')->willReturn($draft); $contentService->method('updateContent')->willReturn($draft); $contentService ->expects(self::never()) ->method('publishVersion'); + // Publication is delegated to the strategy, called once with the draft version and language. + $publishedContent = $publishedSynchronously ? $this->createPublishedContent() : null; $contentPublicationStrategy = $this->createMock(ContentPublicationStrategyInterface::class); $contentPublicationStrategy ->expects(self::once()) ->method('publishVersion') ->with(self::identicalTo($versionInfo), [self::LANGUAGE_CODE]) - ->willReturn(new ContentPublicationResult($this->createPublishedContent())); + ->willReturn(new ContentPublicationResult($publishedContent)); + + // The router is called once with the expected route parameters, unless a custom redirect + // URL coming from the form data short-circuits URL generation entirely. + $router = $this->createMock(RouterInterface::class); + if ($expectedRouteParameters === null) { + $router + ->expects(self::never()) + ->method('generate'); + } else { + $router + ->expects(self::once()) + ->method('generate') + ->with('ibexa.content.view', $expectedRouteParameters) + ->willReturn(self::GENERATED_URL); + } + + // The deferred (async) redirect resolves its location from the content tree root config + // and the location lookup. + $configResolver = $this->createStub(ConfigResolverInterface::class); + $configResolver->method('getParameter')->willReturn(self::TREE_ROOT_LOCATION_ID); + + $locationService = $this->createStub(LocationService::class); + $locationService->method('loadLocation')->willReturn( + new Location([ + 'id' => self::TREE_ROOT_LOCATION_ID, + 'contentInfo' => new ContentInfo(['id' => self::CONTENT_ID]), + ]) + ); $processor = new ContentFormProcessor( $contentService, - $this->createStub(LocationService::class), - $this->createRouterStub(), + $locationService, + $router, + $configResolver, $contentPublicationStrategy ); - $processor->processPublish( - $this->createEvent($this->createUpdateData($draft), $this->createForm()) - ); - } - - /** - * @testWith [true, true] - * [false, false] - */ - public function testProcessPublishSetsPayloads(bool $isNewContent, bool $expectedIsNewPayload): void - { - $draft = $this->createDraft($isNewContent ? null : self::DRAFT_MAIN_LOCATION_ID, $isNewContent); - $data = $isNewContent ? $this->createCreateData() : $this->createUpdateData($draft); - $event = $this->createEvent($data, $this->createForm()); - - $processor = $this->createProcessor($draft, new ContentPublicationResult(null)); $processor->processPublish($event); - self::assertFalse($event->hasPayload('content')); self::assertSame($draft->getContentType(), $event->getPayload('content_type')); - self::assertSame($expectedIsNewPayload, $event->getPayload('is_new')); - } - - /** - * @dataProvider providerForTestProcessPublishRedirect - * - * @param array $expectedRouteParameters - */ - public function testProcessPublishRedirect( - bool $publishedSynchronously, - bool $isNewContent, - bool $withReferrerLocation, - array $expectedRouteParameters - ): void { - $draft = $this->createDraft($isNewContent ? null : self::DRAFT_MAIN_LOCATION_ID, $isNewContent); - $data = $isNewContent ? $this->createCreateData() : $this->createUpdateData($draft); - $publicationResult = new ContentPublicationResult( - $publishedSynchronously ? $this->createPublishedContent() : null - ); - - $router = $this->createMock(RouterInterface::class); - $router - ->expects(self::once()) - ->method('generate') - ->with('ibexa.content.view', $expectedRouteParameters) - ->willReturn(self::GENERATED_URL); - - $options = $withReferrerLocation - ? ['referrerLocation' => new Location(['id' => self::REFERRER_LOCATION_ID])] - : []; - $event = $this->createEvent($data, $this->createForm(), $options); - - $processor = $this->createProcessor($draft, $publicationResult, $router); - $processor->processPublish($event); + self::assertSame($isNewContent, $event->getPayload('is_new')); $response = $event->getResponse(); self::assertInstanceOf(RedirectResponse::class, $response); - self::assertSame(self::GENERATED_URL, $response->getTargetUrl()); + self::assertSame($customRedirectUrl ?? self::GENERATED_URL, $response->getTargetUrl()); } /** - * @return iterable}> + * @return iterable */ - public static function providerForTestProcessPublishRedirect(): iterable + public static function provideProcessPublishCases(): iterable { yield 'sync: update with referrer location' => [ - true, - false, - true, - [ + 'isNewContent' => false, + 'publishedSynchronously' => true, + 'withReferrerLocation' => true, + 'customRedirectUrl' => null, + 'expectedRouteParameters' => [ 'contentId' => self::CONTENT_ID, 'locationId' => self::REFERRER_LOCATION_ID, 'publishedContentId' => self::CONTENT_ID, @@ -142,10 +159,11 @@ public static function providerForTestProcessPublishRedirect(): iterable ]; yield 'sync: update without referrer location' => [ - true, - false, - false, - [ + 'isNewContent' => false, + 'publishedSynchronously' => true, + 'withReferrerLocation' => false, + 'customRedirectUrl' => null, + 'expectedRouteParameters' => [ 'contentId' => self::CONTENT_ID, 'locationId' => self::PUBLISHED_MAIN_LOCATION_ID, 'publishedContentId' => self::CONTENT_ID, @@ -153,10 +171,11 @@ public static function providerForTestProcessPublishRedirect(): iterable ]; yield 'sync: new content' => [ - true, - true, - false, - [ + 'isNewContent' => true, + 'publishedSynchronously' => true, + 'withReferrerLocation' => false, + 'customRedirectUrl' => null, + 'expectedRouteParameters' => [ 'contentId' => self::CONTENT_ID, 'locationId' => self::PUBLISHED_MAIN_LOCATION_ID, 'publishedContentId' => self::CONTENT_ID, @@ -164,88 +183,57 @@ public static function providerForTestProcessPublishRedirect(): iterable ]; yield 'async: update with referrer location' => [ - false, - false, - true, - [ + 'isNewContent' => false, + 'publishedSynchronously' => false, + 'withReferrerLocation' => true, + 'customRedirectUrl' => null, + 'expectedRouteParameters' => [ 'contentId' => self::CONTENT_ID, 'locationId' => self::REFERRER_LOCATION_ID, 'publishedContentId' => self::CONTENT_ID, ], ]; - yield 'async: update without referrer location' => [ - false, - false, - false, - [ + yield 'async: update without referrer location redirects to tree root' => [ + 'isNewContent' => false, + 'publishedSynchronously' => false, + 'withReferrerLocation' => false, + 'customRedirectUrl' => null, + 'expectedRouteParameters' => [ 'contentId' => self::CONTENT_ID, - 'locationId' => self::DRAFT_MAIN_LOCATION_ID, + 'locationId' => self::TREE_ROOT_LOCATION_ID, 'publishedContentId' => self::CONTENT_ID, ], ]; - yield 'async: new content without location yet' => [ - false, - true, - false, - [ + yield 'async: new content without location yet redirects to tree root' => [ + 'isNewContent' => true, + 'publishedSynchronously' => false, + 'withReferrerLocation' => false, + 'customRedirectUrl' => null, + 'expectedRouteParameters' => [ 'contentId' => self::CONTENT_ID, - 'locationId' => null, + 'locationId' => self::TREE_ROOT_LOCATION_ID, 'publishedContentId' => self::CONTENT_ID, ], ]; - } - - public function testProcessPublishUsesRedirectUrlAfterPublishFormData(): void - { - $draft = $this->createDraft(self::DRAFT_MAIN_LOCATION_ID, false); - - $router = $this->createMock(RouterInterface::class); - $router - ->expects(self::never()) - ->method('generate'); - - $event = $this->createEvent( - $this->createUpdateData($draft), - $this->createForm('custom-redirect-url') - ); - - $processor = $this->createProcessor($draft, new ContentPublicationResult(null), $router); - $processor->processPublish($event); - $response = $event->getResponse(); - self::assertInstanceOf(RedirectResponse::class, $response); - self::assertSame('custom-redirect-url', $response->getTargetUrl()); - } - - private function createProcessor( - Content $draft, - ContentPublicationResult $publicationResult, - ?RouterInterface $router = null - ): ContentFormProcessor { - $contentService = $this->createStub(ContentService::class); - $contentService->method('createContent')->willReturn($draft); - $contentService->method('updateContent')->willReturn($draft); - - $contentPublicationStrategy = $this->createStub(ContentPublicationStrategyInterface::class); - $contentPublicationStrategy->method('publishVersion')->willReturn($publicationResult); - - return new ContentFormProcessor( - $contentService, - $this->createStub(LocationService::class), - $router ?? $this->createRouterStub(), - $contentPublicationStrategy - ); + yield 'custom redirect URL after publish bypasses router' => [ + 'isNewContent' => false, + 'publishedSynchronously' => false, + 'withReferrerLocation' => false, + 'customRedirectUrl' => 'custom-redirect-url', + 'expectedRouteParameters' => null, + ]; } - private function createDraft(?int $mainLocationId, bool $neverPublished): Content + private function createDraft(?int $mainLocationId, int $status): Content { $contentInfo = new ContentInfo([ 'id' => self::CONTENT_ID, 'mainLocationId' => $mainLocationId, 'mainLanguageCode' => self::LANGUAGE_CODE, - 'status' => $neverPublished ? ContentInfo::STATUS_DRAFT : ContentInfo::STATUS_PUBLISHED, + 'status' => $status, ]); $versionInfo = $this->createStub(VersionInfo::class); @@ -294,6 +282,9 @@ private function createCreateData(): ContentCreateData ]); } + /** + * @return \Symfony\Component\Form\FormInterface + */ private function createForm(?string $redirectUrlAfterPublish = null): FormInterface { $formConfig = $this->createStub(FormConfigInterface::class); @@ -310,6 +301,7 @@ private function createForm(?string $redirectUrlAfterPublish = null): FormInterf } /** + * @param \Symfony\Component\Form\FormInterface $form * @param array $options */ private function createEvent( @@ -319,12 +311,4 @@ private function createEvent( ): FormActionEvent { return new FormActionEvent($form, $data, 'publish', $options); } - - private function createRouterStub(): RouterInterface - { - $router = $this->createStub(RouterInterface::class); - $router->method('generate')->willReturn(self::GENERATED_URL); - - return $router; - } } From a82943504dbdd761d2f8ed54bb071aae58904c33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Nowak?= Date: Mon, 20 Jul 2026 12:16:31 +0200 Subject: [PATCH 3/3] IBX-11780: Added constant type declarations and ContentService invocation expectations in ContentFormProcessor test --- .../Processor/ContentFormProcessorTest.php | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/lib/Form/Processor/ContentFormProcessorTest.php b/tests/lib/Form/Processor/ContentFormProcessorTest.php index 4abd9f01..eadd4523 100644 --- a/tests/lib/Form/Processor/ContentFormProcessorTest.php +++ b/tests/lib/Form/Processor/ContentFormProcessorTest.php @@ -34,13 +34,13 @@ */ final class ContentFormProcessorTest extends TestCase { - private const CONTENT_ID = 123; - private const DRAFT_MAIN_LOCATION_ID = 42; - private const PUBLISHED_MAIN_LOCATION_ID = 77; - private const REFERRER_LOCATION_ID = 55; - private const TREE_ROOT_LOCATION_ID = 2; - private const LANGUAGE_CODE = 'eng-GB'; - private const GENERATED_URL = 'generated-url'; + private const int CONTENT_ID = 123; + private const int DRAFT_MAIN_LOCATION_ID = 42; + private const int PUBLISHED_MAIN_LOCATION_ID = 77; + private const int REFERRER_LOCATION_ID = 55; + private const int TREE_ROOT_LOCATION_ID = 2; + private const string LANGUAGE_CODE = 'eng-GB'; + private const string GENERATED_URL = 'generated-url'; /** * @dataProvider provideProcessPublishCases @@ -68,10 +68,17 @@ public function testProcessPublish( : []; $event = $this->createEvent($data, $this->createForm($customRedirectUrl), $options); - // The draft is persisted via create/update; publication must never go through the service. + // The draft is persisted via create for new content or update for existing content; + // publication must never go through the service. $contentService = $this->createMock(ContentService::class); - $contentService->method('createContent')->willReturn($draft); - $contentService->method('updateContent')->willReturn($draft); + $contentService + ->expects($isNewContent ? self::once() : self::never()) + ->method('createContent') + ->willReturn($draft); + $contentService + ->expects($isNewContent ? self::never() : self::once()) + ->method('updateContent') + ->willReturn($draft); $contentService ->expects(self::never()) ->method('publishVersion');