Add SEO checks: detect SEO issues on user-defined pages (#2) by loevgaard · Pull Request #6 · Setono/sylius-seo-plugin

loevgaard · 2026-06-22T11:51:36Z

Summary

Adds an admin SEO checks feature (resolves #2): define the pages you want to monitor, fetch them over HTTP, and run them through a battery of generic checks that produce an issues grid with severities and an ignore action.

The core is a generic detection engine so virtually anything can be tested — IssueDetectorInterface::detect(Inspection) hands every check the HTTP status, all response headers, the raw body and the parsed DOM.

What's included

15 built-in checks: HTTP status, X-Robots-Tag / meta noindex, title present/length, meta description present/length, single H1, canonical, image alt, html lang, viewport, Open Graph, JSON-LD validity, mixed content.
4 backend-configurable checks (no code): element_content (CSS or XPath selector + optional attribute + optional JSON path + contains/equals/matches/exists/absent), element_exists, header, status_code.
Admin: a Page resource (route + representative sample resource + per-page check selection via a Symfony UX live form) and an Issue grid (severity badges, filters, ignore/restore). Issues upsert by a stable fingerprint, so ignores survive re-runs and resolved issues are tracked.
Console: setono:sylius-seo:detect-issues (--channel, --page).
Doctrine migrations (MySQL + PostgreSQL), translations, and a config node (scheme, base_url).

Extending

Implement IssueDetectorInterface (autoconfigured) to add a check — it becomes selectable on every page. Implement ConfigurableIssueDetectorInterface for admin-editable configuration.

Tests & verification

297 unit tests (detectors incl. CSS/XPath/JSON-path, persister lifecycle, runner, dynamic form, URL resolvers) + a functional command test.
PHPStan (max) and ECS clean.
Verified end-to-end in the browser (Playwright): live check form, run checks, issues grid, ignore + re-run persistence.

Setup for consumers

Import @SetonoSyliusSEOPlugin/config/routes/admin.yaml under the admin prefix and run the migrations. See the README's "SEO checks" section.

Implements the issues grid requested in #2 via a generic, backend-extensible detection engine. IssueDetectorInterface::detect(Inspection) hands every check the HTTP status, all response headers, the raw body and the parsed DOM, so virtually anything can be tested on a page. - 15 built-in checks (HTTP status, X-Robots-Tag / meta noindex, title, meta description, H1, canonical, image alt, html lang, viewport, Open Graph, JSON-LD, mixed content) plus 4 backend-configurable checks with no code: element_content (CSS/XPath selector + optional attribute + JSON path + contains/equals/matches/exists/absent), element_exists, header, status_code - Admin Page resource (route + sample resource + per-page checks via a Symfony UX live form) and Issue grid with severity badges, filters and ignore/restore - HTTP fetcher, runner and fingerprint-based upsert persistence (ignores survive re-runs; missing issues are marked resolved) - setono:sylius-seo:detect-issues command (--channel, --page) - Doctrine migrations (MySQL + PostgreSQL), translations, config (scheme, base_url) - Unit + functional tests; PHPStan (max) and ECS clean

codecov · 2026-06-22T11:52:35Z

Codecov Report

❌ Patch coverage is 41.58416% with 708 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.69%. Comparing base (064fe29) to head (448e76e).

Files with missing lines	Patch %	Lines
src/Form/Type/PageType.php	0.00%	56 Missing ⚠️
src/Command/DetectIssuesCommand.php	0.00%	33 Missing ⚠️
src/Form/Type/Check/ElementExistsConfigType.php	0.00%	32 Missing ⚠️
src/Model/Issue.php	51.61%	30 Missing ⚠️
src/Checker/UrlResolver/ProductPageUrlResolver.php	0.00%	28 Missing ⚠️
src/Form/Type/Check/HeaderConfigType.php	0.00%	26 Missing ⚠️
src/Model/Page.php	35.00%	26 Missing ⚠️
...Checker/Detector/Builtin/ElementExistsDetector.php	0.00%	24 Missing ⚠️
src/Checker/Detector/Builtin/HeaderDetector.php	0.00%	23 Missing ⚠️
...c/Checker/Detector/Builtin/JsonLdValidDetector.php	0.00%	21 Missing ⚠️
... and 37 more

Additional details and impacted files

@@              Coverage Diff              @@
##                2.x       #6       +/-   ##
=============================================
- Coverage     96.81%   63.69%   -33.12%     
- Complexity      264      606      +342     
=============================================
  Files            52      105       +53     
  Lines           816     2019     +1203     
=============================================
+ Hits            790     1286      +496     
- Misses           26      733      +707

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

loevgaard · 2026-06-22T11:58:54Z

+# Admin routes for the SEO checks feature.
+#
+# Import this file from your application UNDER the admin prefix and only when the feature is
+# enabled (`setono_sylius_seo.checks.enabled: true`), e.g. in config/routes/setono_sylius_seo_admin.yaml:
+#
+#     setono_sylius_seo_admin:
+#         resource: "@SetonoSyliusSEOPlugin/config/routes/admin.yaml"
+#         prefix: '/%sylius_admin.path_name%'


Remove this

Removed the comment block. Admin routes now follow the skeleton pattern — config/routes.yaml imports config/routes/admin.yaml under /admin, so consumers get them from the main routing import. (b84e085)

loevgaard · 2026-06-22T12:03:01Z

    <php>
        <env name="APP_ENV" value="test"/>
        <env name="SHELL_VERBOSITY" value="-1"/>
+        <env name="KERNEL_CLASS" value="Setono\SyliusSEOPlugin\Tests\Application\Kernel"/>


Usually this is defined in .env.test

Removed it from phpunit.xml.dist. KERNEL_CLASS was already defined in tests/Application/.env.test, which now takes effect. (b84e085)

loevgaard · 2026-06-22T12:07:08Z

@@ -0,0 +1,5 @@
+{% if data %}


I am pretty sure Sylius has a built-in grid field like this

You are right — switched to the built-in @SyliusUi/grid/field/yes_no.html.twig and deleted the custom template. (b84e085)

loevgaard · 2026-06-22T12:09:18Z

+        # In local development the shop runs on a non-standard port, so point the checker at it
+        # explicitly (the channel hostname alone has no port). Using the channel's "localhost" host
+        # means Sylius still resolves the right channel for the fetched page.
+        base_url: 'http://localhost:8099'


We should not have a base_url configuration parameter. We should use the built-in in the framework or create a compiler pass or something else in the test application that will discover the URL by running symfony status or something

Removed the base_url/scheme config entirely. ChannelUrlGenerator now builds absolute URLs from the framework router request context (framework.router.default_uri), overriding the host with the channel hostname. The test app discovers the running URL from the SYMFONY_DEFAULT_ROUTE_URL env var that symfony serve injects (see tests/Application/config/packages/routing.yaml); functional tests point it at a refused port for determinism. (b84e085)

loevgaard · 2026-06-22T12:10:24Z

@@ -0,0 +1,3 @@
+setono_sylius_seo_admin:


Use the pattern for routes that the plugin skeleton uses: https://github.com/Setono/SyliusPluginSkeleton/tree/2.2.x/config

Adopted the skeleton pattern: the plugin config/routes.yaml imports config/routes/admin.yaml with prefix: /admin, and the separate setono_sylius_seo_admin.yaml import in the test app is gone. (b84e085)

loevgaard · 2026-06-22T12:11:08Z

+
+interface PageRepositoryInterface
+{
+    public function findOneById(int $id): ?PageInterface;


Repositories should use the built-in Sylius interfaces an classes

Done — PageRepositoryInterface/IssueRepositoryInterface now extend Sylius\Resource\Doctrine\Persistence\RepositoryInterface (generic, like the core repos), the concretes extend EntityRepository, and I dropped the custom findOneById() for the inherited find(). (b84e085)

loevgaard · 2026-06-22T12:15:57Z

@@ -0,0 +1,55 @@
+<?php


Plugins should not carry migrations. The end user will run doctrine:diff instead

Done — removed src/Migrations. The README now tells consumers to generate one with doctrine:migrations:diff. (b84e085)

loevgaard · 2026-06-22T12:16:16Z

+
+use Sylius\Bundle\UiBundle\Menu\Event\MenuBuilderEvent;
+
+final class AdminMenuListener


Make this an event subscriber instead and put it in src/EventSubscriber

Done — moved to src/EventSubscriber/AdminMenuSubscriber.php implementing EventSubscriberInterface. (b84e085)

loevgaard · 2026-06-22T12:17:00Z

+            ->add('channel', ChannelChoiceType::class, [
+                'label' => 'sylius.ui.channel',
+            ])
+            ->add('localeCode', TextType::class, [


Bad UX that we need to put the locale code as free text

Agreed. The locale is now a dropdown of the store enabled locales (via Sylius LocaleProviderInterface) with a "Use the channel default" placeholder, instead of free text. (b84e085)

loevgaard · 2026-06-22T12:29:16Z

+ * Delegates URL resolution to the first registered page-type resolver that supports the page.
+ * Also exposes the list of available page types for the admin form.
+ */
+final class CompositeUrlResolver implements UrlResolverInterface


Use this library: https://github.com/Setono/composite-compiler-pass for composite services

Done — CompositeUrlResolver now extends CompositeService and the resolvers are collected by a CompositeCompilerPass registered in the bundle, matching the data-mapper composites. (448e76e)

loevgaard · 2026-06-22T12:30:13Z

+ * request to derive the host/scheme from. The path comes from the router; the authority comes
+ * from the configured base URL, or the channel hostname plus the configured scheme.
+ */
+final class ChannelUrlGenerator


Services should usually implement an interface, so plugin users can override/decorate the service easily

Done — extracted ChannelUrlGeneratorInterface; ChannelUrlGenerator implements it and the resolvers depend on the interface, so it can be decorated/overridden. The other checker services already expose interfaces (CheckRunnerInterface, PageFetcherInterface, IssuePersisterInterface, UrlResolverInterface, DetectorRegistryInterface). (448e76e)

loevgaard · 2026-06-22T12:42:05Z

+     * A stable, unique machine identifier for this check (e.g. "title_length"). Used to assign the
+     * check to a page and to group/filter the resulting issues.
+     */
+    public function getCode(): string;


Should we have an PHP attribute where you can define the code instead?

I kept getCode() as a method for now — it mirrors the Sylius convention (grid field/filter types expose getName()/getType() the same way) and keeps the registry reflection-free (the tagged iterator plus getCode() is enough to index by code). An attribute such as #[AsIssueDetector("title_length")] would be a nice declarative alternative but needs a compiler pass to read it. Happy to switch if you prefer the attribute approach.

loevgaard · 2026-06-22T12:42:40Z

+
+namespace Setono\SyliusSEOPlugin\Checker\Detector;
+
+interface DetectorRegistryInterface


Should registries be traversable?

The registry exposes all() returning a code => detector map (what the admin form needs to render the catalog) plus get(code)/has(code) for the runner. I left it non-Traversable since callers rely on the code keys and there is no iterate-all use case beyond all(). I can make it IteratorAggregate yielding code => detector if you would rather drop all() in favour of iteration — let me know.

- Register Page/Issue as Sylius resources via AbstractResourceExtension + AbstractResourceBundle (registerResources) instead of hand-rolled sylius_resource config - Repositories extend Sylius's generic RepositoryInterface and use the inherited find() - Drop the shipped migration; consumers generate one with doctrine:migrations:diff - Convert the admin menu listener to an EventSubscriber - Remove the base_url config option; build absolute URLs from the framework router request context (framework.router.default_uri), discovered from `symfony serve` in the test app - Follow the plugin skeleton routing pattern: config/routes.yaml imports the admin routes under /admin - Restrict the page locale field to the store's enabled locales instead of free-text - Use the built-in @SyliusUi/grid/field/yes_no.html.twig grid field - Move KERNEL_CLASS from phpunit.xml.dist to .env.test

…face) - Collect the page URL resolvers with setono/composite-compiler-pass (CompositeUrlResolver now extends CompositeService), matching the data-mapper composites - Extract ChannelUrlGeneratorInterface so the URL generator can be decorated/overridden; the resolvers depend on the interface

loevgaard · 2026-06-22T13:21:29Z

Thanks for the thorough review! Pushed two commits (b84e085, 448e76e) and replied to every inline comment.

Entities as Sylius resources

Switched from the hand-rolled prependExtensionConfig('sylius_resource', …) to the resource-bundle base classes:

SetonoSyliusSEOPlugin now extends AbstractResourceBundle.
SetonoSyliusSEOExtension extends AbstractResourceExtension and registers the page/issue resources via registerResources(), driven by a resources section in Configuration (model / interface / repository / controller / factory / form) — same shape the Sylius core bundles use.

One judgement call worth flagging: AbstractResourceBundle's auto Doctrine mapping only supports XML/YAML/annotation, and ORM 3 removed the annotation driver. Since the entities are attribute-mapped, the bundle registers the attribute mapping driver directly in build() instead of shipping XML. Happy to convert the mapping to XML if you'd prefer AbstractResourceBundle to own it end-to-end.

Status of the comments

Fixed (11): route comment, KERNEL_CLASS → .env.test, built-in yes_no grid field, skeleton routing pattern, base_url removal (framework router.default_uri), Sylius repository interfaces, no shipped migrations, menu → event subscriber, locale dropdown, composite-compiler-pass for the URL resolvers, ChannelUrlGeneratorInterface.
Open questions (2) — replied inline, awaiting your preference: a #[AsIssueDetector] attribute vs the current getCode() method, and making DetectorRegistry Traversable vs the current all().

Verified green: PHPStan max, ECS, 297 tests (incl. the functional command test), and in the browser — the Pages/Issues grids, the live check form with the restricted locale dropdown, running checks, and the ignore lifecycle all work.

loevgaard commented Jun 22, 2026

View reviewed changes

loevgaard added 2 commits June 22, 2026 15:12


		use Sylius\Bundle\UiBundle\Menu\Event\MenuBuilderEvent;

		final class AdminMenuListener


		namespace Setono\SyliusSEOPlugin\Checker\Detector;

		interface DetectorRegistryInterface

Conversation

loevgaard commented Jun 22, 2026

Summary

What's included

Extending

Tests & verification

Setup for consumers

Uh oh!

codecov Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loevgaard commented Jun 22, 2026

Entities as Sylius resources

Status of the comments

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Jun 22, 2026 •

edited

Loading