From ab7fe4106da8900148ac966eb254f31d27ed833a Mon Sep 17 00:00:00 2001 From: Tomas Vondracek Date: Thu, 18 Jun 2026 09:11:53 +0200 Subject: [PATCH 1/2] fix(canonical): slugless canonical for singletons; drop volatile listing query params Singletons are served at their slugless listing URL (ListingController forwards a singleton listing to the record), but getCanonicalRouteAndParams returned the record-detail route, exposing two URLs for identical content (/{singularSlug} and /{singularSlug}/{slug}). Canonicalize singletons to the slugless /{singularSlug} URL instead. Also stop merging listing query params (order/status/filters) into the listing canonical, where they appeared as ?order=-createdAt&status=published. --- src/Controller/Frontend/ListingController.php | 8 +++++--- src/Utils/ContentHelper.php | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Controller/Frontend/ListingController.php b/src/Controller/Frontend/ListingController.php index 905775011..c7ac7036b 100644 --- a/src/Controller/Frontend/ListingController.php +++ b/src/Controller/Frontend/ListingController.php @@ -69,13 +69,15 @@ public function listing(Request $request, ContentRepository $contentRepository, $records = $this->setRecords($content, $amountPerPage, $page); - // Set canonical URL + // Set canonical URL. Note: query params (order/status/filters from + // parseQueryParams) are intentionally NOT merged in — they are volatile and + // would pollute the canonical (e.g. ?order=-createdAt&status=published). $this->canonical->setPath( 'listing_locale', - array_merge([ + [ 'contentTypeSlug' => $contentType->get('slug'), '_locale' => $request->getLocale(), - ], $params) + ] ); // Render diff --git a/src/Utils/ContentHelper.php b/src/Utils/ContentHelper.php index 43110a4f4..50123b024 100644 --- a/src/Utils/ContentHelper.php +++ b/src/Utils/ContentHelper.php @@ -68,6 +68,22 @@ private function getCanonicalRouteAndParams(Content $record, ?string $locale = n ]; } + // Singletons are served at their slugless listing URL: ListingController + // forwards a singleton "listing" to the record. Canonicalize to that URL + // (`/{singularSlug}`) instead of the record-detail route + // (`/{singularSlug}/{slug}`), so a singleton has a single, slugless URL + // rather than two URLs serving identical content. + $definition = $record->getDefinition(); + if ($definition !== null && $definition->get('singleton')) { + return [ + 'route' => 'listing_locale', + 'params' => [ + 'contentTypeSlug' => $record->getContentTypeSingularSlug(), + '_locale' => $locale, + ], + ]; + } + return [ 'route' => $record->getDefinition()->get('record_route'), 'params' => [ From f966cb95052614350952b87fd38745baa29c8f3e Mon Sep 17 00:00:00 2001 From: Tomas Vondracek Date: Thu, 18 Jun 2026 10:44:23 +0200 Subject: [PATCH 2/2] test(canonical): cover slugless singleton canonical and clean listing canonical Unit tests for ContentHelper canonical route resolution: - singleton -> slugless listing route (no slugOrId) - non-singleton -> record route with slug (regression guard) - homepage singleton -> homepage route, not listing (precedence) - non-Content input is ignored Functional tests: - listing canonical omits volatile query params (order/status/filters) - a singleton's slugged record URL canonicalizes to its slugless URL, and the slugless URL is self-referential (end-to-end) Support changes: - add an 'about' non-homepage singleton to the test-scaffolding content types so the end-to-end singleton test has a record to exercise - ContentTypesParserTest: demo content-type count 9 -> 10, assert 'about' key - phpunit.xml.dist: set BOLT_CANONICAL so the Canonical service bootstraps in the test environment (otherwise it fatals on an empty value) --- config/bolt/contenttypes.yaml | 21 +++ phpunit.xml.dist | 2 + .../Parser/ContentTypesParserTest.php | 3 +- .../Frontend/ListingCanonicalTest.php | 28 ++++ .../Frontend/SingletonCanonicalTest.php | 45 ++++++ tests/php/Utils/ContentHelperTest.php | 142 ++++++++++++++++++ 6 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 tests/php/Controller/Frontend/ListingCanonicalTest.php create mode 100644 tests/php/Controller/Frontend/SingletonCanonicalTest.php create mode 100644 tests/php/Utils/ContentHelperTest.php diff --git a/config/bolt/contenttypes.yaml b/config/bolt/contenttypes.yaml index acfae9abf..59d0e30b0 100644 --- a/config/bolt/contenttypes.yaml +++ b/config/bolt/contenttypes.yaml @@ -566,6 +566,27 @@ validationdemos: min: 1 max: 2 +# This contenttype is here to use for (automated) tests. +# A non-homepage singleton: Bolt serves a singleton at its slugless listing URL +# (the ListingController forwards a singleton "listing" to the record), so its +# canonical should be `/about`, not the record-detail route `/about/{slug}`. +about: + name: About + singular_name: About + slug: about + fields: + title: + type: text + label: Title + slug: + type: slug + uses: title + group: meta + viewless: false + singleton: true + icon_many: "fa:info-circle" + icon_one: "fa:info-circle" + # Possible field types: # # text - varchar(256) - input type text. diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 37b5d0cdd..776345493 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,6 +18,8 @@ + + diff --git a/tests/php/Configuration/Parser/ContentTypesParserTest.php b/tests/php/Configuration/Parser/ContentTypesParserTest.php index 1333bdd91..6912e0923 100644 --- a/tests/php/Configuration/Parser/ContentTypesParserTest.php +++ b/tests/php/Configuration/Parser/ContentTypesParserTest.php @@ -70,9 +70,10 @@ public function testHasConfig(): void $contentTypesParser = new ContentTypesParser($this->getProjectDir(), $generalParser->parse(), self::DEFAULT_LOCALE, self::ALLOWED_LOCALES); $config = $contentTypesParser->parse(); - $this->assertCount(9, $config); + $this->assertCount(10, $config); $this->assertArrayHasKey('homepage', $config); + $this->assertArrayHasKey('about', $config); $this->assertCount(self::AMOUNT_OF_ATTRIBUTES_IN_CONTENT_TYPE, $config['homepage']); $this->assertSame('Homepage', $config['homepage']['name']); diff --git a/tests/php/Controller/Frontend/ListingCanonicalTest.php b/tests/php/Controller/Frontend/ListingCanonicalTest.php new file mode 100644 index 000000000..747de7006 --- /dev/null +++ b/tests/php/Controller/Frontend/ListingCanonicalTest.php @@ -0,0 +1,28 @@ +. + */ +class ListingCanonicalTest extends DbAwareTestCase +{ + public function testListingCanonicalOmitsQueryParams(): void + { + $crawler = $this->client->request('GET', '/showcases?order=title'); + + $this->assertResponseIsSuccessful(); + + $canonical = $crawler->filter('link[rel="canonical"]')->attr('href'); + + $this->assertStringEndsWith('/showcases', $canonical); + $this->assertStringNotContainsString('?', $canonical); + $this->assertStringNotContainsString('order', $canonical); + } +} diff --git a/tests/php/Controller/Frontend/SingletonCanonicalTest.php b/tests/php/Controller/Frontend/SingletonCanonicalTest.php new file mode 100644 index 000000000..3b6917a7a --- /dev/null +++ b/tests/php/Controller/Frontend/SingletonCanonicalTest.php @@ -0,0 +1,45 @@ +getEm()->getRepository(Content::class) + ->findOneBy(['contentType' => 'about']); + + $this->assertNotNull($record, 'Expected the "about" singleton fixture to be seeded.'); + + $singularSlug = $record->getContentTypeSingularSlug(); + $slug = $record->getSlug(); + $expectedCanonical = 'http://localhost/' . $singularSlug; + + // The slugged record URL must canonicalize to the slugless singleton URL. + $crawler = $this->client->request('GET', sprintf('/%s/%s', $singularSlug, $slug)); + $this->assertResponseIsSuccessful(); + $this->assertSame( + $expectedCanonical, + $crawler->filter('link[rel="canonical"]')->attr('href') + ); + + // The slugless URL itself is self-referential (same canonical). + $crawler = $this->client->request('GET', '/' . $singularSlug); + $this->assertResponseIsSuccessful(); + $this->assertSame( + $expectedCanonical, + $crawler->filter('link[rel="canonical"]')->attr('href') + ); + } +} diff --git a/tests/php/Utils/ContentHelperTest.php b/tests/php/Utils/ContentHelperTest.php new file mode 100644 index 000000000..2bbea8224 --- /dev/null +++ b/tests/php/Utils/ContentHelperTest.php @@ -0,0 +1,142 @@ +createContent( + singleton: true, + singularSlug: 'contact', + slug: 'contact-info', + recordSlug: 'contact-2' + ); + + $canonical = $this->createMock(Canonical::class); + $canonical->expects($this->once()) + ->method('setPath') + ->with('listing_locale', [ + 'contentTypeSlug' => 'contact', + '_locale' => 'en', + ]); + + $this->createContentHelper($canonical)->setCanonicalPath($content, 'en'); + } + + public function testNonSingletonCanonicalUsesRecordRouteWithSlug(): void + { + $content = $this->createContent( + singleton: false, + singularSlug: 'entry', + slug: 'entries', + recordSlug: 'my-post', + recordRoute: 'record' + ); + + $canonical = $this->createMock(Canonical::class); + $canonical->expects($this->once()) + ->method('setPath') + ->with('record', [ + 'contentTypeSlug' => 'entry', + 'slugOrId' => 'my-post', + '_locale' => 'en', + ]); + + $this->createContentHelper($canonical)->setCanonicalPath($content, 'en'); + } + + public function testHomepageCanonicalIsNotAffectedBySingletonHandling(): void + { + // The homepage is itself a singleton, but must keep resolving to the + // homepage route, not the listing route. + $content = $this->createContent( + singleton: true, + singularSlug: 'homepage', + slug: 'homepage', + recordSlug: 'homepage' + ); + + $canonical = $this->createMock(Canonical::class); + $canonical->expects($this->once()) + ->method('setPath') + ->with('homepage_locale', [ + '_locale' => 'en', + ]); + + $this->createContentHelper($canonical)->setCanonicalPath($content, 'en'); + } + + public function testNonContentIsIgnored(): void + { + $canonical = $this->createMock(Canonical::class); + $canonical->expects($this->never()) + ->method('setPath'); + + $this->createContentHelper($canonical)->setCanonicalPath(null, 'en'); + } + + private function createContent( + bool $singleton, + string $singularSlug, + string $slug, + string $recordSlug, + ?string $recordRoute = null + ): Content { + $definition = $this->createMock(ContentType::class); + $definition->method('get')->willReturnCallback( + static fn (string $key) => match ($key) { + 'singleton' => $singleton, + 'record_route' => $recordRoute, + default => null, + } + ); + + $content = $this->createMock(Content::class); + $content->method('getDefinition')->willReturn($definition); + $content->method('getContentTypeSingularSlug')->willReturn($singularSlug); + $content->method('getContentTypeSlug')->willReturn($slug); + $content->method('getSlug')->willReturn($recordSlug); + $content->method('getId')->willReturn(1); + + return $content; + } + + private function createContentHelper(Canonical $canonical): ContentHelper + { + $config = $this->createMock(Config::class); + // Only `general/homepage` is consulted (via isHomepage()); a content type + // is the homepage when its slug matches this value. + $config->method('get')->willReturnCallback( + static fn (string $path) => $path === 'general/homepage' ? 'homepage' : null + ); + + $requestStack = $this->createMock(RequestStack::class); + $requestStack->method('getCurrentRequest')->willReturn(new Request()); + + return new ContentHelper( + $canonical, + $requestStack, + $config, + $this->createMock(LocaleExtension::class) + ); + } +}