From faa0f42e67b5d107947be10535d061d22297ea26 Mon Sep 17 00:00:00 2001 From: Julien Dehais Date: Wed, 14 Oct 2026 14:45:54 +0200 Subject: [PATCH 1/4] docs: record the architecture decisions in docs/adr One file per decision, 35 records: the invariants of CLAUDE.md, the choices the code does not explain (materialized conditional taxons owning their links, counted filter subqueries, media zones, the search form takeover, the condition preview) and what was kept as it is on purpose (random display modes, translation key namespaces, option conditions on translated values). The records hold the decisions and their reasons only; CLAUDE.md keeps every rule in full and links its record. The README, the docs index, the overview, CONTRIBUTING and the pull request template point to them, and the public contract says that a record promises nothing. --- .github/PULL_REQUEST_TEMPLATE.md | 1 + CLAUDE.md | 36 +++++++++---- CONTRIBUTING.md | 9 +++- README.md | 1 + docs/README.md | 3 +- docs/adr/0001-never-replace-a-sylius-model.md | 31 +++++++++++ ...kept-out-of-main-taxons-by-a-constraint.md | 27 ++++++++++ ...zones-replace-the-native-images-section.md | 30 +++++++++++ .../0004-twig-hooks-imported-not-prepended.md | 25 +++++++++ ...taken-over-to-keep-the-advanced-filters.md | 27 ++++++++++ .../0006-taxon-listing-is-the-sylius-grid.md | 28 ++++++++++ ...-facet-counts-start-from-the-grid-query.md | 26 ++++++++++ ...ilters-are-counted-subqueries-with-caps.md | 29 +++++++++++ docs/adr/0009-no-like-on-json-columns.md | 25 +++++++++ ...-featured-items-filtered-at-render-time.md | 24 +++++++++ ...ons-materialized-as-product-taxon-links.md | 29 +++++++++++ ...onditional-taxon-owns-its-product-links.md | 26 ++++++++++ ...ducts-detached-through-the-unit-of-work.md | 24 +++++++++ ...tched-through-messenger-after-the-flush.md | 32 ++++++++++++ ...on-applicable-condition-matches-nothing.md | 27 ++++++++++ ...6-conditions-compare-case-insensitively.md | 23 ++++++++ ...n-conditions-match-the-translated-value.md | 25 +++++++++ ...ion-preview-mirrors-the-synchronization.md | 27 ++++++++++ docs/adr/0019-pictograms-are-sylius-images.md | 27 ++++++++++ docs/adr/0020-values-sanitized-on-write.md | 27 ++++++++++ docs/adr/0021-template-icons-are-local.md | 28 ++++++++++ ...-icon-the-shop-cannot-render-is-refused.md | 24 +++++++++ .../0023-random-media-display-modes-kept.md | 23 ++++++++ docs/adr/0024-no-script-in-templates.md | 24 +++++++++ ...-twig-functions-served-by-lazy-runtimes.md | 24 +++++++++ ...ts-only-where-the-effect-is-not-obvious.md | 24 +++++++++ ...-translation-keys-keep-their-namespaces.md | 23 ++++++++ .../0028-explicit-services-no-autowiring.md | 28 ++++++++++ ...9-per-request-memory-services-are-reset.md | 27 ++++++++++ ...0-bundle-class-with-sylius-plugin-trait.md | 24 +++++++++ .../0031-schema-change-is-a-new-migration.md | 27 ++++++++++ docs/adr/0032-no-changelog.md | 23 ++++++++ ...tream-workarounds-never-constrain-shops.md | 26 ++++++++++ ...4-static-analysis-on-the-primary-ci-job.md | 24 +++++++++ .../0035-contributor-stack-ships-chrome.md | 24 +++++++++ docs/adr/README.md | 52 +++++++++++++++++++ docs/architecture/overview.md | 26 ++++++---- docs/architecture/public-contract.md | 3 ++ 43 files changed, 1020 insertions(+), 23 deletions(-) create mode 100644 docs/adr/0001-never-replace-a-sylius-model.md create mode 100644 docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md create mode 100644 docs/adr/0003-media-zones-replace-the-native-images-section.md create mode 100644 docs/adr/0004-twig-hooks-imported-not-prepended.md create mode 100644 docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md create mode 100644 docs/adr/0006-taxon-listing-is-the-sylius-grid.md create mode 100644 docs/adr/0007-facet-counts-start-from-the-grid-query.md create mode 100644 docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md create mode 100644 docs/adr/0009-no-like-on-json-columns.md create mode 100644 docs/adr/0010-featured-items-filtered-at-render-time.md create mode 100644 docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md create mode 100644 docs/adr/0012-conditional-taxon-owns-its-product-links.md create mode 100644 docs/adr/0013-products-detached-through-the-unit-of-work.md create mode 100644 docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md create mode 100644 docs/adr/0015-non-applicable-condition-matches-nothing.md create mode 100644 docs/adr/0016-conditions-compare-case-insensitively.md create mode 100644 docs/adr/0017-option-conditions-match-the-translated-value.md create mode 100644 docs/adr/0018-condition-preview-mirrors-the-synchronization.md create mode 100644 docs/adr/0019-pictograms-are-sylius-images.md create mode 100644 docs/adr/0020-values-sanitized-on-write.md create mode 100644 docs/adr/0021-template-icons-are-local.md create mode 100644 docs/adr/0022-icon-the-shop-cannot-render-is-refused.md create mode 100644 docs/adr/0023-random-media-display-modes-kept.md create mode 100644 docs/adr/0024-no-script-in-templates.md create mode 100644 docs/adr/0025-twig-functions-served-by-lazy-runtimes.md create mode 100644 docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md create mode 100644 docs/adr/0027-translation-keys-keep-their-namespaces.md create mode 100644 docs/adr/0028-explicit-services-no-autowiring.md create mode 100644 docs/adr/0029-per-request-memory-services-are-reset.md create mode 100644 docs/adr/0030-bundle-class-with-sylius-plugin-trait.md create mode 100644 docs/adr/0031-schema-change-is-a-new-migration.md create mode 100644 docs/adr/0032-no-changelog.md create mode 100644 docs/adr/0033-upstream-workarounds-never-constrain-shops.md create mode 100644 docs/adr/0034-static-analysis-on-the-primary-ci-job.md create mode 100644 docs/adr/0035-contributor-stack-ships-chrome.md create mode 100644 docs/adr/README.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e942b3c..61395b8 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -8,4 +8,5 @@ - [ ] PHPStan and ECS are green - [ ] If the schema changed: the migration is updated, and `sh tests/check-schema-drift.sh` prints nothing (MySQL or PostgreSQL) - [ ] `UPGRADE.md` is up to date if the public contract changes (traits and interfaces, Twig hooks and functions, grid query) +- [ ] A decision taken or reversed has its record in `docs/adr/` - [ ] No customer data, shop secrets or pre-production URLs appear in the code, the tests or this description diff --git a/CLAUDE.md b/CLAUDE.md index f87c294..b200f36 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,39 +30,51 @@ The plugin targets Sylius 2.1, 2.2 and 2.3, Symfony 7.4 and 8 (8 with Sylius 2.3 ## Invariants +Each rule is here in full, with the names the code uses; its reasons are in the decision record it +links ([`docs/adr/`](docs/adr/README.md)). Read the record before changing a rule, and write a new +record rather than editing an accepted one - the records hold no path nor figure, this file does. + - **Never replace a Sylius model.** The application owns `Taxon`, `Channel` and `TaxonImage`; the plugin only ships traits (mapping as attributes) and interfaces. Relations target the Sylius interfaces (`TaxonInterface`, `ProductInterface`, `TaxonImageInterface`), resolved by - `resolve_target_entities`. Code tests `instanceof AdvancedTaxonInterface`, never a class. + `resolve_target_entities`. Code tests `instanceof AdvancedTaxonInterface`, never a class + ([0001](docs/adr/0001-never-replace-a-sylius-model.md)). - **The taxon listing is the Sylius grid.** `ShopProductListQueryBuilder` calls `createShopListQueryBuilder()` and adds the children products and the advanced filters: no parallel product query in templates. Facet counts come from `createBaseProductsQb()`, which must - select exactly what the grid lists. + select exactly what the grid lists ([0006](docs/adr/0006-taxon-listing-is-the-sylius-grid.md), + [0007](docs/adr/0007-facet-counts-start-from-the-grid-query.md)). - **Conditional taxons fail closed.** A stored condition that is not applicable (wrong operator, - missing reference or value) makes the taxon match nothing, never the whole catalog. + missing reference or value) makes the taxon match nothing, never the whole catalog + ([0015](docs/adr/0015-non-applicable-condition-matches-nothing.md)). - **No LIKE on JSON columns** (not portable to PostgreSQL): select attribute values are resolved in - PHP, then filtered by identifier. + PHP, then filtered by identifier ([0009](docs/adr/0009-no-like-on-json-columns.md)). - **Services are explicit**: one file per area in `config/services/`, ids prefixed with `cyllene_digital_sylius_advanced_taxon.` (the controller keeps its class as id), no autowiring nor autoconfiguration (Symfony bundle best practice). A class implementing `ResetInterface` needs the - `kernel.reset` tag by hand. + `kernel.reset` tag by hand ([0028](docs/adr/0028-explicit-services-no-autowiring.md), + [0029](docs/adr/0029-per-request-memory-services-are-reset.md)). - **Template icons are local**: `tabler:` when Sylius ships the icon, otherwise a Tabler SVG added to - `assets/icons/` and used as `at:`. A unit test fails on any other prefix (Iconify). + `assets/icons/` and used as `at:`. A unit test fails on any other prefix (Iconify) + ([0021](docs/adr/0021-template-icons-are-local.md)). - **Twig hooks are imported, not prepended**: `config/config.yaml` must be loaded after the Sylius - configuration, since a prepended hook loses against the Sylius core hooks it overrides. + configuration, since a prepended hook loses against the Sylius core hooks it overrides + ([0004](docs/adr/0004-twig-hooks-imported-not-prepended.md)). - **Pictograms are Sylius images** (`image:` in the icon field, Sylius image uploader and storage, Liip filter `cyllene_advanced_taxon_icon`). Values rendered in `style` or `href` - attributes are sanitized on write (`AdvancedTaxonValueSanitizer`). + attributes are sanitized on write (`AdvancedTaxonValueSanitizer`) + ([0019](docs/adr/0019-pictograms-are-sylius-images.md), [0020](docs/adr/0020-values-sanitized-on-write.md)). - **A schema change is a new migration** in `src/Migrations/` (DBAL `Schema` API): `v1.0.0` shipped `Version20260828173000`, which is never edited again. Free of drift on MySQL, MariaDB and PostgreSQL (CI runs `down()` / `up()` then `tests/check-schema-drift.sh`, which lists the columns - added to the Sylius tables: keep it in step with the migrations). + added to the Sylius tables: keep it in step with the migrations) + ([0031](docs/adr/0031-schema-change-is-a-new-migration.md)). - The traits, interfaces, Twig hooks and functions, bundle configuration, message, command, admin route and schema are the public contract ([`docs/architecture/public-contract.md`](docs/architecture/public-contract.md)): they only break in a major version, with an entry in `UPGRADE.md`. - **No `CHANGELOG.md`**: the GitHub release notes are the changelog, `UPGRADE.md` says what a shop - has to do. + has to do ([0032](docs/adr/0032-no-changelog.md)). ## Flex recipe @@ -119,6 +131,8 @@ The QA commands are in [`CONTRIBUTING.md`](CONTRIBUTING.md). What it does not sa override a `{% props %}` default, and the Sylius grid delete action passes a null label and icon to `sylius_admin:delete_modal` (empty delete buttons in every admin grid). Drop the constraint once Sylius passes defaults itself ([Sylius#19278](https://github.com/Sylius/Sylius/issues/19278)). + An upstream workaround never goes into the plugin's own constraints + ([0033](docs/adr/0033-upstream-workarounds-never-constrain-shops.md)). - `sylius_test_html_attribute()` attributes (`data-test-*`) are what the Behat pages target: keep them when reworking a shop template. @@ -134,6 +148,8 @@ The same facts live in several places; change one, change them all: - the Twig hooks: `config/twig_hooks/*.yaml` and `docs/integration/theming.md`; - the Twig functions: `src/Twig/*.php`, `docs/shop/twig-functions.md` and `docs/architecture/public-contract.md`; - the Behat features: `features/`, `tests/Behat/Resources/suites.php` and `docs/testing/tests.md`; +- a decision taken or reversed: a new record in `docs/adr/`, its line in `docs/adr/README.md`, and + the link from the rule here; - what a screen looks like: the screenshots in `docs/assets/` after a visible change. ## What does not belong here diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index afee250..dc2aae1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -235,9 +235,10 @@ Sylius plugins, do not add personal rules to it. integration tests on a real database; prefer a Behat scenario when the bug is visible in the shop or the back office. - The plugin never replaces a Sylius model: new fields on `Taxon`, `Channel` or `TaxonImage` go into - the matching trait and interface, and the migration. + the matching trait and interface, and the migration + ([why](docs/adr/0001-never-replace-a-sylius-model.md)). - Services are defined explicitly in `config/services/`, without autowiring - ([code map](docs/development/code-map.md#wiring)). + ([code map](docs/development/code-map.md#wiring), [why](docs/adr/0028-explicit-services-no-autowiring.md)). - Update **`UPGRADE.md`** when the public contract changes; the change itself is described in the pull request, which feeds the release notes. Public contract items are listed in [docs/architecture/public-contract.md](docs/architecture/public-contract.md); they only break in @@ -246,6 +247,10 @@ Sylius plugins, do not add personal rules to it. - The codebase is written in **English** (code, comments, commits). - A map of the code is in [docs/development/code-map.md](docs/development/code-map.md); the test strategy in [docs/testing/tests.md](docs/testing/tests.md). +- Why the plugin is built the way it is, and what was left as it is on purpose, is in the + [decision records](docs/adr/README.md): read the one a change touches first. A pull request + that takes a decision, or reverses one, adds a record - an accepted record is superseded, not + edited. ## What does not belong here diff --git a/README.md b/README.md index be3eb85..050f8bd 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,7 @@ full list and the support policy: - [Back-office guide](https://github.com/CylleneDigital/SyliusAdvancedTaxonPlugin/blob/main/docs/admin/user-guide.md): for merchants - [Theming and Twig hooks](https://github.com/CylleneDigital/SyliusAdvancedTaxonPlugin/blob/main/docs/integration/theming.md): for integrators - [Technical documentation](https://github.com/CylleneDigital/SyliusAdvancedTaxonPlugin/blob/main/docs/README.md): every page, by audience +- [Decision records](https://github.com/CylleneDigital/SyliusAdvancedTaxonPlugin/blob/main/docs/adr/README.md): why the plugin is built the way it is, one decision per file ## Status diff --git a/docs/README.md b/docs/README.md index 9abf1fe..e10fec9 100644 --- a/docs/README.md +++ b/docs/README.md @@ -11,7 +11,7 @@ list: [Features](features.md). Installation in short and production notes: the r | Merchant (back office) | [Back-office guide](admin/user-guide.md), [Storefront](shop/storefront.md), [Conditional taxons](domain/conditional-taxons.md) | | Sylius integrator | [Installation](integration/installation.md), [Configuration](integration/configuration.md), [Theming and Twig hooks](integration/theming.md), [Assets](integration/assets.md), [Twig functions](shop/twig-functions.md), [Public contract](architecture/public-contract.md), [Troubleshooting](integration/troubleshooting.md) | | Ops (deployment) | [Migrations](persistence/migrations.md), [Database schema](persistence/schema.md), [Configuration: Messenger](integration/configuration.md), [Uninstall](integration/uninstall.md), [UPGRADE](../UPGRADE.md) | -| Plugin contributor | [Architecture overview](architecture/overview.md), [Code map](development/code-map.md), [Tests](testing/tests.md), [CONTRIBUTING](../CONTRIBUTING.md) | +| Plugin contributor | [Architecture overview](architecture/overview.md), [Decision records](adr/README.md), [Code map](development/code-map.md), [Tests](testing/tests.md), [CONTRIBUTING](../CONTRIBUTING.md) | ## Table of contents @@ -45,6 +45,7 @@ list: [Features](features.md). Installation in short and production notes: the r - [Architecture overview](architecture/overview.md): layers, main flows, principles - [Public contract](architecture/public-contract.md): what is stable (semver major), support policy +- [Decision records](adr/README.md): one file per architecture decision - its context, the decision, what follows from it - [Database schema](persistence/schema.md): added columns and tables - [Migrations](persistence/migrations.md): the single migration, portability, drift check diff --git a/docs/adr/0001-never-replace-a-sylius-model.md b/docs/adr/0001-never-replace-a-sylius-model.md new file mode 100644 index 0000000..73c9140 --- /dev/null +++ b/docs/adr/0001-never-replace-a-sylius-model.md @@ -0,0 +1,31 @@ +# 0001. The plugin never replaces a Sylius model + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A plugin that adds fields to taxons, channels and taxon images could ship its own entities and +ask the application to use them. The application then loses its own models, and two plugins doing +the same cannot be installed together. A relation that names a concrete class also breaks as soon +as the application overrides that class. + +## Decision + +The application keeps its own `Taxon`, `Channel` and `TaxonImage`. The plugin ships a trait and +an interface for each, with the mapping as attributes in the trait, the usual Sylius 2 pattern: an +XML mapping to copy into the application would be heavier to install. Relations target the Sylius +interfaces, resolved by Doctrine to the application's classes. The code tests the plugin interfaces, +never a class. What the plugin owns alone lives in tables of its own. + +## Consequences + +- Installing the plugin means editing three application entities, which the installation guide + and the Flex recipe describe. +- A taxon that does not implement the plugin interface is ignored by the shop helpers instead of + failing. +- The traits and interfaces are public contract: changing them is a major version. + +## Related + +- [0031. A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) diff --git a/docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md b/docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md new file mode 100644 index 0000000..0d95ee8 --- /dev/null +++ b/docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md @@ -0,0 +1,27 @@ +# 0002. A universe is kept out of main taxons by a validation constraint, not a form override + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A universe page lists child taxons, not products. A product whose main taxon is a universe would +get a breadcrumb and a canonical URL pointing to a page where it does not appear. Filtering the +choices of the main taxon field of the product form would only protect the back office, and +replacing a core field leaves the loading order of the form extensions decide which definition +wins, losing the native options of the field. + +## Decision + +A validation constraint on the product, in the `sylius` group, rejects a universe as main taxon. +The product form is not touched. + +## Consequences + +- The rule holds wherever the product is validated: back office, API, imports, fixtures. +- A universe still appears in the choices of the field; picking it shows a validation error. +- Code that saves a product without validating it is not protected. + +## Related + +- [0003. Plugin media zones replace the native taxon Images section](0003-media-zones-replace-the-native-images-section.md) diff --git a/docs/adr/0003-media-zones-replace-the-native-images-section.md b/docs/adr/0003-media-zones-replace-the-native-images-section.md new file mode 100644 index 0000000..1dcf58d --- /dev/null +++ b/docs/adr/0003-media-zones-replace-the-native-images-section.md @@ -0,0 +1,30 @@ +# 0003. Plugin media zones replace the native taxon Images section + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The plugin places taxon images in zones (main image, top, bottom, left, inside the product list, +featured, universe slider), each with its own settings, stored as Sylius taxon images with a type. +Keeping the native Images collection next to the zones would have two forms managing the same +images, and the native collection would delete the zone media when the taxon is saved. Showing +the native collection only for the types the plugin does not use would still give two image +interfaces in one form. + +## Decision + +The native Images section is removed from the taxon form and replaced by the zones. A main image +zone takes over the images Sylius stores without type; they get the type of that zone when the +taxon is saved, and the shop keeps showing the first one as the taxon image. + +## Consequences + +- Images of other types, set by a theme or another plugin, are kept in the database but can no + longer be edited from the taxon form. +- One image interface in the taxon form, with one uploader and one storage: the Sylius ones. + +## Related + +- [0019. Pictograms are Sylius images](0019-pictograms-are-sylius-images.md) +- [0023. The random media display modes are kept](0023-random-media-display-modes-kept.md) diff --git a/docs/adr/0004-twig-hooks-imported-not-prepended.md b/docs/adr/0004-twig-hooks-imported-not-prepended.md new file mode 100644 index 0000000..ae95c92 --- /dev/null +++ b/docs/adr/0004-twig-hooks-imported-not-prepended.md @@ -0,0 +1,25 @@ +# 0004. Twig hooks are imported, not prepended + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Several plugin hooks override hookables of the Sylius core. A configuration prepended by the +bundle extension is loaded before the application's and the Sylius one, so the Sylius core hooks +would win over the plugin hooks that replace them. + +## Decision + +The hooks live in YAML files that the application imports through the plugin configuration file, +after the Sylius configuration. + +## Consequences + +- Installing the plugin means importing that file, which the Flex recipe does. +- An application can still override any plugin hook after the import. +- The hooks and their hookables are public contract. + +## Related + +- [0005. The plugin takes over the Sylius search form so it keeps the advanced filters](0005-search-form-taken-over-to-keep-the-advanced-filters.md) diff --git a/docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md b/docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md new file mode 100644 index 0000000..12d8ffd --- /dev/null +++ b/docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md @@ -0,0 +1,27 @@ +# 0005. The plugin takes over the Sylius search form so it keeps the advanced filters + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The search form of the Sylius product listing only sends the search text: submitting it dropped +the advanced filters, the sort, the limit and the view. Hidden fields added through the nested +search hook would land inside the input group and break its rounded corners. A Stimulus controller +copying the parameters would not work without JavaScript and adds code. + +## Decision + +The plugin replaces the search hookable of the listing filters with a form that sends the +current advanced filters, sort, limit and view back, and starts again from the first page. Its +clear button only removes the search. + +## Consequences + +- Searching inside a filtered listing keeps the filters, with or without JavaScript. +- A theme that customizes this form has to keep the hidden fields. + +## Related + +- [0004. Twig hooks are imported, not prepended](0004-twig-hooks-imported-not-prepended.md) +- [0006. The taxon listing is the Sylius grid](0006-taxon-listing-is-the-sylius-grid.md) diff --git a/docs/adr/0006-taxon-listing-is-the-sylius-grid.md b/docs/adr/0006-taxon-listing-is-the-sylius-grid.md new file mode 100644 index 0000000..368ab1d --- /dev/null +++ b/docs/adr/0006-taxon-listing-is-the-sylius-grid.md @@ -0,0 +1,28 @@ +# 0006. The taxon listing is the Sylius grid + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The plugin adds products of child taxons and advanced filters to the taxon page. A product query +of its own, in a Twig function, would lose what the Sylius grid does: pagination, sorting, search, +channel restriction, and what other plugins add to it. Decorating the product repository is more +intrusive and clashes with the other plugins decorating it too. + +## Decision + +The plugin listens to the definition of the shop product grid and points its repository method +to a query builder that calls the native one, then adds the children products and the advanced +filters to the same query. An application that already replaced that method keeps its own query. + +## Consequences + +- No parallel product query in the templates: the grid stays the only listing. +- The grid query builder is public contract. +- An application with its own listing query does not get the plugin filters on it. + +## Related + +- [0007. Facet counts start from the query the grid lists](0007-facet-counts-start-from-the-grid-query.md) +- [0008. Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) diff --git a/docs/adr/0007-facet-counts-start-from-the-grid-query.md b/docs/adr/0007-facet-counts-start-from-the-grid-query.md new file mode 100644 index 0000000..590383d --- /dev/null +++ b/docs/adr/0007-facet-counts-start-from-the-grid-query.md @@ -0,0 +1,26 @@ +# 0007. Facet counts start from the query the grid lists + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The sidebar shows, for each attribute, option and sub-taxon, how many products match. Counts +computed on another product set than the listing would announce products the page does not show. + +## Decision + +The facet queries start from a base query that selects exactly what the grid lists: enabled +products of the current channel, in the taxon or its descendants, with the grid's translation +join, its search and its descendants setting. Each facet family ignores its own selection when it +counts the alternatives. + +## Consequences + +- A change to what the grid lists has to be made in the base query too. +- Choosing a value never shows a count the listing contradicts. + +## Related + +- [0006. The taxon listing is the Sylius grid](0006-taxon-listing-is-the-sylius-grid.md) +- [0008. Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) diff --git a/docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md b/docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md new file mode 100644 index 0000000..dc48833 --- /dev/null +++ b/docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md @@ -0,0 +1,29 @@ +# 0008. Advanced filters are counted subqueries, with caps on codes and values + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The advanced filters come from the query string, which anyone can write. Each filtered attribute +or option added joins: a forged URL could join more tables than MySQL accepts and answer an error, +or send heavy queries for every page view without being logged in. With about ten filters as +joins, the join order search of MySQL does not finish; an `EXISTS` subquery does not help, MySQL +turns it back into joins. + +## Decision + +Each attribute or option filter is a correlated count subquery compared to zero. The number of +filtered codes and the number of values per code are capped; what goes beyond is ignored. + +## Consequences + +- The query stays the same shape whatever the database, and a forged URL cannot make it grow + without limit. +- A shop that needs more filters at once than the caps allow cannot get them from the URL. +- The caps are constants of the plugin, not configuration. + +## Related + +- [0006. The taxon listing is the Sylius grid](0006-taxon-listing-is-the-sylius-grid.md) +- [0009. No `LIKE` on JSON columns](0009-no-like-on-json-columns.md) diff --git a/docs/adr/0009-no-like-on-json-columns.md b/docs/adr/0009-no-like-on-json-columns.md new file mode 100644 index 0000000..9a0eb16 --- /dev/null +++ b/docs/adr/0009-no-like-on-json-columns.md @@ -0,0 +1,25 @@ +# 0009. No `LIKE` on JSON columns + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The values of a select attribute are stored in a JSON column. Matching them with `LIKE` works on +MySQL and MariaDB, not on PostgreSQL, and depends on how the JSON is serialized. + +## Decision + +The choices of a select attribute are read once per attribute code and matched in PHP; the query +then filters on the identifiers of the matching values. + +## Consequences + +- Every value of a filtered select attribute is read, once per request and code: an accepted + cost on a large catalog. +- The filters behave the same on MySQL, MariaDB and PostgreSQL. + +## Related + +- [0008. Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) +- [0016. Conditions compare case-insensitively on every database](0016-conditions-compare-case-insensitively.md) diff --git a/docs/adr/0010-featured-items-filtered-at-render-time.md b/docs/adr/0010-featured-items-filtered-at-render-time.md new file mode 100644 index 0000000..d70858a --- /dev/null +++ b/docs/adr/0010-featured-items-filtered-at-render-time.md @@ -0,0 +1,24 @@ +# 0010. Featured items are filtered for visibility at render time + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A taxon features products and child taxons picked in the back office. Later on, a product can be +disabled or leave the channel, a taxon can be disabled or moved under another parent. Validating +the selection on save would not catch these later changes. + +## Decision + +The selection is stored as is and filtered when the shop renders it: disabled products, products +outside the current channel, disabled taxons and children moved elsewhere are left out. + +## Consequences + +- The back office keeps a selection the shop may partly hide. +- The filtering costs a query for the channels not loaded yet, not one per product. + +## Related + +- [0025. Twig functions are served by lazy runtimes](0025-twig-functions-served-by-lazy-runtimes.md) diff --git a/docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md b/docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md new file mode 100644 index 0000000..6a9d059 --- /dev/null +++ b/docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md @@ -0,0 +1,29 @@ +# 0011. Conditional taxons are materialized as product-taxon links + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A conditional taxon gets its products from rules (attribute, option, name, description, stock, +taxon membership). Resolving the +rules when the taxon is listed would only serve the plugin listing: the API, the search, the +product forms and the other plugins read the product-taxon links Sylius stores. + +## Decision + +The rules are materialized: the matching products are attached to the taxon as regular Sylius +product-taxon links, and kept in step when the conditions change or on demand with a command. +Nothing is resolved when the shop reads the taxon. + +## Consequences + +- Every part of Sylius sees a conditional taxon as a normal one. +- A product that starts or stops matching (a new attribute value, a new product) is only attached + or detached by the next synchronization: the command is meant to be scheduled. + +## Related + +- [0012. A conditional taxon owns its product links](0012-conditional-taxon-owns-its-product-links.md) +- [0014. The synchronization is dispatched through Messenger once the flush is over, synchronous unless routed](0014-synchronization-dispatched-through-messenger-after-the-flush.md) +- [0015. A condition that cannot apply matches nothing](0015-non-applicable-condition-matches-nothing.md) diff --git a/docs/adr/0012-conditional-taxon-owns-its-product-links.md b/docs/adr/0012-conditional-taxon-owns-its-product-links.md new file mode 100644 index 0000000..ef7f7f7 --- /dev/null +++ b/docs/adr/0012-conditional-taxon-owns-its-product-links.md @@ -0,0 +1,26 @@ +# 0012. A conditional taxon owns its product links + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Products can be attached to a taxon by hand before it becomes conditional, or after. Keeping +them as manual assignments next to the materialized ones would make the content of a conditional +taxon depend on its history, not on its rules. + +## Decision + +The synchronization alone manages the links of a conditional taxon: products attached by hand +that do not match are detached. Removing the last condition, or turning the taxon back into a +regular one, detaches all its products. + +## Consequences + +- What a conditional taxon lists is what its conditions select, nothing else. +- Turning a taxon conditional, then back, loses its manual assignments. + +## Related + +- [0011. Conditional taxons are materialized as product-taxon links](0011-conditional-taxons-materialized-as-product-taxon-links.md) +- [0013. Products are detached through the unit of work, not a DQL `DELETE`](0013-products-detached-through-the-unit-of-work.md) diff --git a/docs/adr/0013-products-detached-through-the-unit-of-work.md b/docs/adr/0013-products-detached-through-the-unit-of-work.md new file mode 100644 index 0000000..69d2091 --- /dev/null +++ b/docs/adr/0013-products-detached-through-the-unit-of-work.md @@ -0,0 +1,24 @@ +# 0013. Products are detached through the unit of work, not a DQL `DELETE` + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Detaching products from a conditional taxon with a bulk `DELETE` would skip the Doctrine events +(search indexers listen to them) and leave the product collections already in memory with links +that no longer exist. + +## Decision + +The links to remove are loaded and removed through the entity manager, by chunks. A product +already in memory has its collection updated; the others are not loaded. + +## Consequences + +- Listeners see each removed link, like a removal from the product form. +- Detaching costs more than a bulk delete, without loading the products themselves. + +## Related + +- [0012. A conditional taxon owns its product links](0012-conditional-taxon-owns-its-product-links.md) diff --git a/docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md b/docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md new file mode 100644 index 0000000..e8ab486 --- /dev/null +++ b/docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md @@ -0,0 +1,32 @@ +# 0014. The synchronization is dispatched through Messenger once the flush is over, synchronous unless routed + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Saving a conditional taxon has to recompute its products. Doing it in a Doctrine `postFlush` +listener means flushing inside a flush, which Doctrine does not support: the collection deletions +of the outer flush, not cleaned up yet, run again and lose data. Doing it in a worker only would +force every shop to run one. + +## Decision + +The taxons to synchronize are collected during the flush; a message per taxon is dispatched on +the Sylius command bus once the flush is over, at the end of the request, of the console command +or of the message a worker handled. The handler is registered on that bus only, whose transaction +middleware flushes. With no routing, the message is handled synchronously. + +## Consequences + +- No dependency on a transport: a shop routes the message to an asynchronous one when saving a + taxon must not wait for the assignments. +- Synchronously, a failing synchronization answers an error although the taxon is already saved. +- Code that flushes outside a request, a command or a worker (a plain script) leaves the + synchronization to the command. +- The message is public contract. + +## Related + +- [0011. Conditional taxons are materialized as product-taxon links](0011-conditional-taxons-materialized-as-product-taxon-links.md) +- [0029. Services holding per-request memory are reset](0029-per-request-memory-services-are-reset.md) diff --git a/docs/adr/0015-non-applicable-condition-matches-nothing.md b/docs/adr/0015-non-applicable-condition-matches-nothing.md new file mode 100644 index 0000000..d0e5ea7 --- /dev/null +++ b/docs/adr/0015-non-applicable-condition-matches-nothing.md @@ -0,0 +1,27 @@ +# 0015. A condition that cannot apply matches nothing + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A stored condition can stop being applicable: an operator its type does not accept, a missing +value, an attribute, option or taxon that was deleted. Ignoring it would widen the taxon, up to +the whole catalog when it was the only condition. + +## Decision + +Conditional taxons fail closed: a condition that cannot apply makes the taxon match nothing, +negative operators included. A branch the validation makes unreachable throws instead of skipping +the condition. + +## Consequences + +- A broken condition empties the taxon instead of flooding it: visible, and harmless. +- The validation rejects what it can on save (wrong operator, missing reference or value, unknown + or self reference); what breaks later is caught by the query. + +## Related + +- [0011. Conditional taxons are materialized as product-taxon links](0011-conditional-taxons-materialized-as-product-taxon-links.md) +- [0018. The condition preview mirrors the synchronization](0018-condition-preview-mirrors-the-synchronization.md) diff --git a/docs/adr/0016-conditions-compare-case-insensitively.md b/docs/adr/0016-conditions-compare-case-insensitively.md new file mode 100644 index 0000000..9ab6565 --- /dev/null +++ b/docs/adr/0016-conditions-compare-case-insensitively.md @@ -0,0 +1,23 @@ +# 0016. Conditions compare case-insensitively on every database + +- Status: accepted +- Date: 2026-10-14 + +## Context + +MySQL and MariaDB compare text case-insensitively with the usual collations, PostgreSQL does not: +the same condition matched different products depending on the database. + +## Decision + +Text comparisons of the conditions lower both sides. + +## Consequences + +- A conditional taxon lists the same products on MySQL, MariaDB and PostgreSQL. +- A condition cannot tell values apart by their case. + +## Related + +- [0009. No `LIKE` on JSON columns](0009-no-like-on-json-columns.md) +- [0017. Option conditions match the translated value, and `in` takes one value](0017-option-conditions-match-the-translated-value.md) diff --git a/docs/adr/0017-option-conditions-match-the-translated-value.md b/docs/adr/0017-option-conditions-match-the-translated-value.md new file mode 100644 index 0000000..ef93794 --- /dev/null +++ b/docs/adr/0017-option-conditions-match-the-translated-value.md @@ -0,0 +1,25 @@ +# 0017. Option conditions match the translated value, and `in` takes one value + +- Status: accepted +- Date: 2026-10-14 + +## Context + +An option condition compares the translated value of an option value, in any locale, not its +code, and its `in` operator takes a single value. Changing either changes what stored conditions +mean. + +## Decision + +Both are kept as accepted limitations, documented with the conditions. + +## Consequences + +- Renaming an option value in every locale breaks the conditions that use it: positive ones + stop matching, negative ones stop excluding. +- Matching on codes, or several values for `in`, would be a new operator or a major version, not + a change of the existing ones. + +## Related + +- [0016. Conditions compare case-insensitively on every database](0016-conditions-compare-case-insensitively.md) diff --git a/docs/adr/0018-condition-preview-mirrors-the-synchronization.md b/docs/adr/0018-condition-preview-mirrors-the-synchronization.md new file mode 100644 index 0000000..62f0378 --- /dev/null +++ b/docs/adr/0018-condition-preview-mirrors-the-synchronization.md @@ -0,0 +1,27 @@ +# 0018. The condition preview mirrors the synchronization + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The back office previews the products matching the conditions being edited. A preview that +reads another locale, or other products, than the synchronization would announce a result the +taxon will not get. + +## Decision + +The preview runs the synchronization query on the same products, disabled ones included, in the +default locale, whatever the locale of the back office. The request body is decoded leniently: an +invalid body previews no product instead of failing. The number of conditions read and of products +returned is capped. + +## Consequences + +- What the preview shows is what the next synchronization attaches. +- Rows being typed, incomplete or with a reference that does not exist, are skipped instead of + failing the preview. + +## Related + +- [0015. A condition that cannot apply matches nothing](0015-non-applicable-condition-matches-nothing.md) diff --git a/docs/adr/0019-pictograms-are-sylius-images.md b/docs/adr/0019-pictograms-are-sylius-images.md new file mode 100644 index 0000000..4f78007 --- /dev/null +++ b/docs/adr/0019-pictograms-are-sylius-images.md @@ -0,0 +1,27 @@ +# 0019. Pictograms are Sylius images + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A taxon can show an uploaded pictogram instead of a glyph. Storing the file in the public +directory breaks on several servers, on a read-only container, and on a shop whose images are in +object storage. + +## Decision + +A pictogram is stored through the Sylius image uploader and storage, and resized by a Liip +Imagine filter the plugin prepends (an application can override it). The icon field holds an +`image:` reference. The file is removed once the change that drops it is flushed; the previous +value is read from the database, never from the submitted form. + +## Consequences + +- Pictograms follow the shop's image storage, wherever it is. +- A failed flush leaves the file in place; a forged form cannot get another file deleted. + +## Related + +- [0003. Plugin media zones replace the native taxon Images section](0003-media-zones-replace-the-native-images-section.md) +- [0020. Values rendered in `style` or `href` are sanitized on write](0020-values-sanitized-on-write.md) diff --git a/docs/adr/0020-values-sanitized-on-write.md b/docs/adr/0020-values-sanitized-on-write.md new file mode 100644 index 0000000..1b25f76 --- /dev/null +++ b/docs/adr/0020-values-sanitized-on-write.md @@ -0,0 +1,27 @@ +# 0020. Values rendered in `style` or `href` are sanitized on write + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The color of a taxon is rendered in a `style` attribute, the icon in markup, a media link in an +`href`. Escaping alone does not stop a `javascript:` link or a value that closes the CSS +declaration. + +## Decision + +The setters sanitize these values: a color that is not a hexadecimal color, an icon that is not +an icon name or an image reference, a link that is neither an HTTP(S) URL nor a same-origin path +are dropped. Media links are sanitized +again when read. The media display mode setters fall back to their default on an unknown value. + +## Consequences + +- Whatever writes the entity (back office, API, import, fixtures) cannot store a dangerous value. +- An invalid value is lost silently instead of raising an error. + +## Related + +- [0019. Pictograms are Sylius images](0019-pictograms-are-sylius-images.md) +- [0022. An icon the shop cannot render is refused on save](0022-icon-the-shop-cannot-render-is-refused.md) diff --git a/docs/adr/0021-template-icons-are-local.md b/docs/adr/0021-template-icons-are-local.md new file mode 100644 index 0000000..e0acca9 --- /dev/null +++ b/docs/adr/0021-template-icons-are-local.md @@ -0,0 +1,28 @@ +# 0021. Template icons are local + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Symfony UX Icons fetches from Iconify, at render time, any icon its local sets do not have. A +template icon outside the few Tabler icons Sylius ships would then depend on the network, and on +`ux:icons:lock` to work offline. Mixing icon libraries also mixes licences and styles. + +## Decision + +Templates only use Tabler icons: the ones Sylius ships, or a Tabler SVG added to the plugin's own +set under a prefix of its own. A unit test fails on any other prefix. The plugin set does not reuse +a library prefix: a local set named after a library would silently hide that library for the whole +shop. + +## Consequences + +- No network access to render a plugin template, one licence (MIT), a style consistent with the + Sylius admin. +- A new template icon may need a new SVG in the plugin set. + +## Related + +- [0022. An icon the shop cannot render is refused on save](0022-icon-the-shop-cannot-render-is-refused.md) +- [0024. No script in the templates, behaviour in Stimulus controllers](0024-no-script-in-templates.md) diff --git a/docs/adr/0022-icon-the-shop-cannot-render-is-refused.md b/docs/adr/0022-icon-the-shop-cannot-render-is-refused.md new file mode 100644 index 0000000..20aa091 --- /dev/null +++ b/docs/adr/0022-icon-the-shop-cannot-render-is-refused.md @@ -0,0 +1,24 @@ +# 0022. An icon the shop cannot render is refused on save + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Sylius configures UX Icons to ignore an icon it cannot find: a taxon icon with a typo or from a +library the shop does not have disappears without any error. + +## Decision + +On save, the taxon form asks the icon renderer to render the icon and refuses it with a +validation error when it cannot. + +## Consequences + +- An administrator sees at once that an icon does not exist. +- An icon that becomes unavailable later (a library removed) still disappears silently. + +## Related + +- [0020. Values rendered in `style` or `href` are sanitized on write](0020-values-sanitized-on-write.md) +- [0021. Template icons are local](0021-template-icons-are-local.md) diff --git a/docs/adr/0023-random-media-display-modes-kept.md b/docs/adr/0023-random-media-display-modes-kept.md new file mode 100644 index 0000000..a4b1217 --- /dev/null +++ b/docs/adr/0023-random-media-display-modes-kept.md @@ -0,0 +1,23 @@ +# 0023. The random media display modes are kept + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Some media zones offer a random display mode (shuffled media, a random spotlight, media inserted +at random places in the listing). A page that changes on every view cannot be cached as such by an +HTTP cache. + +## Decision + +The random modes are kept: they are a merchandising feature, chosen per taxon. Behind an HTTP +cache, the draw is frozen for the lifetime of each cache entry. + +## Consequences + +- A shop behind a cache gets a draw per cache entry, not per view. + +## Related + +- [0003. Plugin media zones replace the native taxon Images section](0003-media-zones-replace-the-native-images-section.md) diff --git a/docs/adr/0024-no-script-in-templates.md b/docs/adr/0024-no-script-in-templates.md new file mode 100644 index 0000000..fc3de48 --- /dev/null +++ b/docs/adr/0024-no-script-in-templates.md @@ -0,0 +1,24 @@ +# 0024. No script in the templates, behaviour in Stimulus controllers + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Inline scripts are hard to test, cannot be overridden by a theme without copying the template, +and conflict with a strict Content Security Policy. + +## Decision + +The templates hold no script. A behaviour is a Stimulus controller of the plugin entrypoints, +under a prefix of its own; the texts it shows come from the translations, passed as Stimulus +values. + +## Consequences + +- The shop and the back office have to build the plugin entrypoints. +- A theme changes a behaviour by replacing or extending its controller. + +## Related + +- [0021. Template icons are local](0021-template-icons-are-local.md) diff --git a/docs/adr/0025-twig-functions-served-by-lazy-runtimes.md b/docs/adr/0025-twig-functions-served-by-lazy-runtimes.md new file mode 100644 index 0000000..ef065c9 --- /dev/null +++ b/docs/adr/0025-twig-functions-served-by-lazy-runtimes.md @@ -0,0 +1,24 @@ +# 0025. Twig functions are served by lazy runtimes + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The plugin declares many Twig functions, most of them used by a single kind of page. Declaring +them on extensions that hold their dependencies builds every dependency on every page. + +## Decision + +One Twig extension declares the functions; each is served by a runtime, built on the first call +of one of its functions. + +## Consequences + +- A page only builds what it uses. +- The function names are public contract, the runtimes are internal. + +## Related + +- [0010. Featured items are filtered for visibility at render time](0010-featured-items-filtered-at-render-time.md) +- [0028. Services are declared explicitly, without autowiring](0028-explicit-services-no-autowiring.md) diff --git a/docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md b/docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md new file mode 100644 index 0000000..a1b3683 --- /dev/null +++ b/docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md @@ -0,0 +1,24 @@ +# 0026. Help texts only where a field's effect is not obvious + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The taxon form has many fields. A help text on each would load the form, multiply the texts to +translate and repeat the user guide until both diverge. Tooltips hide the help, work poorly on +touch screens and need JavaScript. + +## Decision + +A field gets a one-line help text only when its effect is not obvious: a side effect, a scope +narrower than expected, an expected format. The user guide stays the reference. + +## Consequences + +- A new field gets a help text only if it meets that bar. +- The form stays readable; the details are in the guide. + +## Related + +- [0027. Translation keys keep their namespaces](0027-translation-keys-keep-their-namespaces.md) diff --git a/docs/adr/0027-translation-keys-keep-their-namespaces.md b/docs/adr/0027-translation-keys-keep-their-namespaces.md new file mode 100644 index 0000000..53ae81f --- /dev/null +++ b/docs/adr/0027-translation-keys-keep-their-namespaces.md @@ -0,0 +1,23 @@ +# 0027. Translation keys keep their namespaces + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The translation keys of the plugin sit under a few namespaces (shop, form, ui, facet), while +other plugins put their labels under a single `ui` namespace. Aligning them renames keys an +application may have overridden. + +## Decision + +The keys keep their namespaces. + +## Consequences + +- An application's overrides keep working. +- Aligning the keys would be a major version. + +## Related + +- [0026. Help texts only where a field's effect is not obvious](0026-help-texts-only-where-the-effect-is-not-obvious.md) diff --git a/docs/adr/0028-explicit-services-no-autowiring.md b/docs/adr/0028-explicit-services-no-autowiring.md new file mode 100644 index 0000000..a02ef34 --- /dev/null +++ b/docs/adr/0028-explicit-services-no-autowiring.md @@ -0,0 +1,28 @@ +# 0028. Services are declared explicitly, without autowiring + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Symfony recommends that a reusable bundle does not rely on autowiring nor autoconfiguration: its +services must not depend on what the host application happens to alias or tag, and autowiring +would turn every class of the plugin into an injectable service, public by accident. + +## Decision + +Every service is declared by hand, one file per area, with a prefixed id and every argument and +tag written out. The controller is the exception to the prefix: it keeps its class as id, since its +route names it that way. The interfaces an application may decorate are aliased to their service. + +## Consequences + +- A new constructor argument is added to the service definition. +- A tag autoconfiguration would have set is added by hand. +- Only the attributes read at runtime are used in the classes; the ones that only + autoconfiguration reads would do nothing. + +## Related + +- [0029. Services holding per-request memory are reset](0029-per-request-memory-services-are-reset.md) +- [0030. A `Bundle` class with `SyliusPluginTrait`, not `AbstractBundle`](0030-bundle-class-with-sylius-plugin-trait.md) diff --git a/docs/adr/0029-per-request-memory-services-are-reset.md b/docs/adr/0029-per-request-memory-services-are-reset.md new file mode 100644 index 0000000..161ecab --- /dev/null +++ b/docs/adr/0029-per-request-memory-services-are-reset.md @@ -0,0 +1,27 @@ +# 0029. Services holding per-request memory are reset + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Some services keep memory for the duration of a request: resolved attribute values, checked +references, visibility lookups, taxons waiting for a synchronization. In a long-running process +(FrankenPHP worker mode, Messenger workers) that memory would leak from one request or message to +the next. + +## Decision + +Services are `final` and stateless where they can be. Those that hold per-request memory +implement `ResetInterface`, and get the `kernel.reset` tag in their definition. + +## Consequences + +- A worker never serves stale data from a previous request. +- A new service that keeps memory needs both the interface and the tag: without autoconfiguration, + the interface alone does nothing. + +## Related + +- [0014. The synchronization is dispatched through Messenger once the flush is over, synchronous unless routed](0014-synchronization-dispatched-through-messenger-after-the-flush.md) +- [0028. Services are declared explicitly, without autowiring](0028-explicit-services-no-autowiring.md) diff --git a/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md b/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md new file mode 100644 index 0000000..7512fa6 --- /dev/null +++ b/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md @@ -0,0 +1,24 @@ +# 0030. A `Bundle` class with `SyliusPluginTrait`, not `AbstractBundle` + +- Status: accepted +- Date: 2026-10-14 + +## Context + +Symfony's `AbstractBundle` puts the configuration and the extension in the bundle class. A Sylius +plugin uses `SyliusPluginTrait`, which redefines `getContainerExtension()`. + +## Decision + +The bundle is a plain `Bundle` with `SyliusPluginTrait` and a classic `Extension` class, like the +other Sylius plugins. + +## Consequences + +- The extension loads the services and prepends what the plugin configures in other bundles + (Doctrine mapping, migrations, Liip Imagine, UX Icons). +- The bundle class and its namespace are part of the public surface. + +## Related + +- [0028. Services are declared explicitly, without autowiring](0028-explicit-services-no-autowiring.md) diff --git a/docs/adr/0031-schema-change-is-a-new-migration.md b/docs/adr/0031-schema-change-is-a-new-migration.md new file mode 100644 index 0000000..3a39722 --- /dev/null +++ b/docs/adr/0031-schema-change-is-a-new-migration.md @@ -0,0 +1,27 @@ +# 0031. A schema change is a new migration, written against the DBAL `Schema` API + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The plugin adds columns to Sylius tables and owns tables of its own; shops run it on MySQL, +MariaDB or PostgreSQL. A migration written in SQL would target one of them. A published migration +that is edited never runs again on the shops that already executed it. + +## Decision + +The schema changes through migrations of the plugin, written against the DBAL `Schema` API and +registered through Sylius, which lets an application turn their prepending off. A released +migration is never edited: a change is a new migration. CI replays the migrations down and up and +checks that the schema has no drift from the mapping on every database. + +## Consequences + +- One migration source for the three databases. +- A change to the columns added to Sylius tables also updates the drift check. +- The schema is public contract. + +## Related + +- [0001. The plugin never replaces a Sylius model](0001-never-replace-a-sylius-model.md) diff --git a/docs/adr/0032-no-changelog.md b/docs/adr/0032-no-changelog.md new file mode 100644 index 0000000..7168b53 --- /dev/null +++ b/docs/adr/0032-no-changelog.md @@ -0,0 +1,23 @@ +# 0032. No `CHANGELOG.md` + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A changelog file repeats what the pull requests already describe, and drifts from the releases +when it is forgotten. + +## Decision + +The GitHub release notes, built from the pull requests, are the changelog. `UPGRADE.md` only says +what a shop has to do when it upgrades. + +## Consequences + +- A pull request describes its change for the release notes. +- A pull request that breaks the public contract adds its entry to `UPGRADE.md`. + +## Related + +- [0031. A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) diff --git a/docs/adr/0033-upstream-workarounds-never-constrain-shops.md b/docs/adr/0033-upstream-workarounds-never-constrain-shops.md new file mode 100644 index 0000000..4aba577 --- /dev/null +++ b/docs/adr/0033-upstream-workarounds-never-constrain-shops.md @@ -0,0 +1,26 @@ +# 0033. A workaround for an upstream bug never constrains the shops + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A bug in Sylius or in a dependency can break the plugin's tests or its test application. Capping +the dependency in the plugin's requirements would impose the cap on every shop, including the ones +the bug does not affect or that fixed it their own way. + +## Decision + +A workaround for an upstream bug stays in the development requirements or in the CI jobs. The +plugin's own constraints stay open. Each workaround is noted with the upstream issue that will lift +it. + +## Consequences + +- The plugin never blocks a shop from upgrading a dependency. +- A shop affected by the upstream bug applies the workaround itself. +- A workaround is removed once the upstream fix is released. + +## Related + +- [0034. Static analysis and coverage run on the primary CI job only](0034-static-analysis-on-the-primary-ci-job.md) diff --git a/docs/adr/0034-static-analysis-on-the-primary-ci-job.md b/docs/adr/0034-static-analysis-on-the-primary-ci-job.md new file mode 100644 index 0000000..7835d12 --- /dev/null +++ b/docs/adr/0034-static-analysis-on-the-primary-ci-job.md @@ -0,0 +1,24 @@ +# 0034. Static analysis and coverage run on the primary CI job only + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The CI matrix runs every supported combination of Sylius, Symfony, PHP and database. Static +analysis, the coding standard and the template lint depend neither on the database nor on the PHP +version under test; coverage is only meaningful once. + +## Decision + +They run on the primary job only. The other jobs run the tests. + +## Consequences + +- A faster matrix, with one place to read the analysis and the coverage. +- A problem that only static analysis on another version would show is not caught. + +## Related + +- [0033. A workaround for an upstream bug never constrains the shops](0033-upstream-workarounds-never-constrain-shops.md) +- [0035. The contributor stack ships a Chrome service](0035-contributor-stack-ships-chrome.md) diff --git a/docs/adr/0035-contributor-stack-ships-chrome.md b/docs/adr/0035-contributor-stack-ships-chrome.md new file mode 100644 index 0000000..d597bcc --- /dev/null +++ b/docs/adr/0035-contributor-stack-ships-chrome.md @@ -0,0 +1,24 @@ +# 0035. The contributor stack ships a Chrome service + +- Status: accepted +- Date: 2026-10-14 + +## Context + +The Behat Chrome driver asks Chrome for its version as soon as it is built, so Behat does not +start without a Chrome, even for the scenarios that do not use a browser. A contributor stack that +cannot run Behat does not help. + +## Decision + +The versioned Docker stack contributors use has a headless Chrome service, sharing the network of +the PHP service, so Behat reaches it the way its configuration expects. + +## Consequences + +- The whole Behat suite runs in the contributor stack. +- The stack differs from the other plugins' on this point. + +## Related + +- [0034. Static analysis and coverage run on the primary CI job only](0034-static-analysis-on-the-primary-ci-job.md) diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..c23f432 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,52 @@ +# Architecture decision records + +One file per decision: its context, what was decided, what follows from it. A decision is not +edited once accepted: a new record supersedes it, and the old one says so in its status. + +A record holds the decision and its reasons, which do not move with the code. What does - file +paths, class lists, figures - is in the [architecture overview](../architecture/overview.md), the +[code map](../development/code-map.md), the [public contract](../architecture/public-contract.md) +and the guides, which follow the code: a record that quoted them would be wrong at the next +refactoring, and could not be corrected. A record explains a choice, it promises nothing: only the +public contract says what is stable. + +A new record takes the next number, the template of the others (Status, Date, Context, Decision, +Consequences, Related) and a line in the table below. + +| # | Decision | +|---|---| +| 0001 | [The plugin never replaces a Sylius model](0001-never-replace-a-sylius-model.md) | +| 0002 | [A universe is kept out of main taxons by a validation constraint, not a form override](0002-universe-kept-out-of-main-taxons-by-a-constraint.md) | +| 0003 | [Plugin media zones replace the native taxon Images section](0003-media-zones-replace-the-native-images-section.md) | +| 0004 | [Twig hooks are imported, not prepended](0004-twig-hooks-imported-not-prepended.md) | +| 0005 | [The plugin takes over the Sylius search form so it keeps the advanced filters](0005-search-form-taken-over-to-keep-the-advanced-filters.md) | +| 0006 | [The taxon listing is the Sylius grid](0006-taxon-listing-is-the-sylius-grid.md) | +| 0007 | [Facet counts start from the query the grid lists](0007-facet-counts-start-from-the-grid-query.md) | +| 0008 | [Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) | +| 0009 | [No `LIKE` on JSON columns](0009-no-like-on-json-columns.md) | +| 0010 | [Featured items are filtered for visibility at render time](0010-featured-items-filtered-at-render-time.md) | +| 0011 | [Conditional taxons are materialized as product-taxon links](0011-conditional-taxons-materialized-as-product-taxon-links.md) | +| 0012 | [A conditional taxon owns its product links](0012-conditional-taxon-owns-its-product-links.md) | +| 0013 | [Products are detached through the unit of work, not a DQL `DELETE`](0013-products-detached-through-the-unit-of-work.md) | +| 0014 | [The synchronization is dispatched through Messenger once the flush is over, synchronous unless routed](0014-synchronization-dispatched-through-messenger-after-the-flush.md) | +| 0015 | [A condition that cannot apply matches nothing](0015-non-applicable-condition-matches-nothing.md) | +| 0016 | [Conditions compare case-insensitively on every database](0016-conditions-compare-case-insensitively.md) | +| 0017 | [Option conditions match the translated value, and `in` takes one value](0017-option-conditions-match-the-translated-value.md) | +| 0018 | [The condition preview mirrors the synchronization](0018-condition-preview-mirrors-the-synchronization.md) | +| 0019 | [Pictograms are Sylius images](0019-pictograms-are-sylius-images.md) | +| 0020 | [Values rendered in `style` or `href` are sanitized on write](0020-values-sanitized-on-write.md) | +| 0021 | [Template icons are local](0021-template-icons-are-local.md) | +| 0022 | [An icon the shop cannot render is refused on save](0022-icon-the-shop-cannot-render-is-refused.md) | +| 0023 | [The random media display modes are kept](0023-random-media-display-modes-kept.md) | +| 0024 | [No script in the templates, behaviour in Stimulus controllers](0024-no-script-in-templates.md) | +| 0025 | [Twig functions are served by lazy runtimes](0025-twig-functions-served-by-lazy-runtimes.md) | +| 0026 | [Help texts only where a field's effect is not obvious](0026-help-texts-only-where-the-effect-is-not-obvious.md) | +| 0027 | [Translation keys keep their namespaces](0027-translation-keys-keep-their-namespaces.md) | +| 0028 | [Services are declared explicitly, without autowiring](0028-explicit-services-no-autowiring.md) | +| 0029 | [Services holding per-request memory are reset](0029-per-request-memory-services-are-reset.md) | +| 0030 | [A `Bundle` class with `SyliusPluginTrait`, not `AbstractBundle`](0030-bundle-class-with-sylius-plugin-trait.md) | +| 0031 | [A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) | +| 0032 | [No `CHANGELOG.md`](0032-no-changelog.md) | +| 0033 | [A workaround for an upstream bug never constrains the shops](0033-upstream-workarounds-never-constrain-shops.md) | +| 0034 | [Static analysis and coverage run on the primary CI job only](0034-static-analysis-on-the-primary-ci-job.md) | +| 0035 | [The contributor stack ships a Chrome service](0035-contributor-stack-ships-chrome.md) | diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 7d0e26c..265a0d2 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -71,7 +71,7 @@ src/ | DependencyInjection | `Configuration`, `CylleneDigitalSyliusAdvancedTaxonExtension` | `icon_libraries` config, Doctrine mapping, migrations, Liip filter sets, the `at` UX Icons set | Services are defined explicitly in `config/services/*.php`, without autowiring nor autoconfiguration, -as Symfony recommends for reusable bundles. The wiring rules are in +as Symfony recommends for reusable bundles ([why](../adr/0028-explicit-services-no-autowiring.md)). The wiring rules are in [Code map](../development/code-map.md#wiring). A file-by-file map is in [Code map](../development/code-map.md). @@ -122,7 +122,10 @@ A file-by-file map is in [Code map](../development/code-map.md). 5. `ConditionalTaxonProductAssigner` computes the matching product ids with `ConditionalTaxonQueryBuilder::buildQuery()` and attaches / detaches `ProductTaxon` links. -Details: [Conditional taxons](../domain/conditional-taxons.md). +Details: [Conditional taxons](../domain/conditional-taxons.md). Why the products are materialized, +and why the synchronization waits for the end of the flush: +[0011](../adr/0011-conditional-taxons-materialized-as-product-taxon-links.md), +[0014](../adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md). ### Admin preview of conditions @@ -144,14 +147,17 @@ featured children and images in grouped queries. | Principle | Where it shows | |-----------|----------------| -| Never replace a Sylius model | Traits + interfaces only; relations target `TaxonInterface`, `ProductInterface`, `TaxonImageInterface` (Doctrine `resolve_target_entities`); code tests `instanceof AdvancedTaxonInterface` | -| The taxon listing is the Sylius grid | No parallel product query in templates; facets count what the grid lists | -| Fail closed | A stored condition that is not applicable (incomplete, or whose attribute, option or taxon does not exist) makes the taxon match nothing (`buildQuery()` adds `1 = 0`), negative operators included | -| No `LIKE` on JSON columns | Select attribute values are read once per attribute code and matched in PHP, then filtered by id (`IN` with inlined integers) | -| Twig hooks are imported, not prepended | `config/config.yaml` is imported by the application after the Sylius configuration, so the plugin hooks win over the Sylius core hooks they override | -| Sanitize on write | `setColor()`, `setIcon()`, `setUrl()` go through `AdvancedTaxonValueSanitizer`; the media display mode setters fall back to their default on an unknown value; `advanced_taxon_media_href()` sanitizes media links again on read | -| Show only what is visible | Featured products and children are filtered at render time (`advanced_taxon_visible_products()`, `advanced_taxon_visible_taxons()`): disabled products, products outside the current channel and disabled taxons are left out | -| Stateless services | Services are `final` and mostly `readonly`; the six holding per-request memory (`SelectAttributeValueResolver`, `FacetReferenceChecker`, `FacetRuntime`, `VisibilityRuntime`, `ConditionalTaxonSyncListener`, `TaxonIconRemovalListener`) implement `ResetInterface`, so they are reset between requests (FrankenPHP worker mode, Messenger workers) | +| [Never replace a Sylius model](../adr/0001-never-replace-a-sylius-model.md) | Traits + interfaces only; relations target `TaxonInterface`, `ProductInterface`, `TaxonImageInterface` (Doctrine `resolve_target_entities`); code tests `instanceof AdvancedTaxonInterface` | +| [The taxon listing is the Sylius grid](../adr/0006-taxon-listing-is-the-sylius-grid.md) | No parallel product query in templates; facets count what the grid lists | +| [Fail closed](../adr/0015-non-applicable-condition-matches-nothing.md) | A stored condition that is not applicable (incomplete, or whose attribute, option or taxon does not exist) makes the taxon match nothing (`buildQuery()` adds `1 = 0`), negative operators included | +| [No `LIKE` on JSON columns](../adr/0009-no-like-on-json-columns.md) | Select attribute values are read once per attribute code and matched in PHP, then filtered by id (`IN` with inlined integers) | +| [Twig hooks are imported, not prepended](../adr/0004-twig-hooks-imported-not-prepended.md) | `config/config.yaml` is imported by the application after the Sylius configuration, so the plugin hooks win over the Sylius core hooks they override | +| [Sanitize on write](../adr/0020-values-sanitized-on-write.md) | `setColor()`, `setIcon()`, `setUrl()` go through `AdvancedTaxonValueSanitizer`; the media display mode setters fall back to their default on an unknown value; `advanced_taxon_media_href()` sanitizes media links again on read | +| [Show only what is visible](../adr/0010-featured-items-filtered-at-render-time.md) | Featured products and children are filtered at render time (`advanced_taxon_visible_products()`, `advanced_taxon_visible_taxons()`): disabled products, products outside the current channel and disabled taxons are left out | +| [Stateless services](../adr/0029-per-request-memory-services-are-reset.md) | Services are `final` and mostly `readonly`; the six holding per-request memory (`SelectAttributeValueResolver`, `FacetReferenceChecker`, `FacetRuntime`, `VisibilityRuntime`, `ConditionalTaxonSyncListener`, `TaxonIconRemovalListener`) implement `ResetInterface`, so they are reset between requests (FrankenPHP worker mode, Messenger workers) | + +Why each of them holds, and what was left as it is on purpose: the +[decision records](../adr/README.md). ## Entry files diff --git a/docs/architecture/public-contract.md b/docs/architecture/public-contract.md index f867a49..0bd502d 100644 --- a/docs/architecture/public-contract.md +++ b/docs/architecture/public-contract.md @@ -264,6 +264,9 @@ Everything not listed above, in particular: Decorating or replacing an internal service works, but is not covered by semver. +The [decision records](../adr/README.md) explain choices, some of them about internals: a record +promises nothing, only this page says what is stable. + ## Support policy | Line | What is covered | From 85652aaba6b52f9df3ca8722fcb3edd1f2645c07 Mon Sep 17 00:00:00 2001 From: Julien Dehais Date: Wed, 14 Oct 2026 14:56:51 +0200 Subject: [PATCH 2/4] docs: check the decision records and the docs against the code Drop the three records about the project's own tooling (changelog, CI organisation, contributor stack): they are not decisions about the plugin a shop installs. Correct the records where the code says otherwise (fixtures skip validation, only attribute and option filters are subqueries, the preview skips rows the synchronization fails closed on, only a new icon is checked, ...). Bring the pages in line: the released migration is never edited, syncTaxon() takes no locale, the preview orders by id, the visibility runtime caches channel membership. The conditional taxons page and the code map keep the facts and link the records for the reasons. Fix stale code comments and drop the ones that only restate the code. --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- CLAUDE.md | 15 +++++---- CONTRIBUTING.md | 4 +-- assets/shop/js/mobile-menu-controller.js | 9 ----- assets/shop/scss/mega-menu.scss | 6 ---- config/services.php | 3 +- docs/README.md | 2 +- docs/adr/0001-never-replace-a-sylius-model.md | 18 +++++----- ...kept-out-of-main-taxons-by-a-constraint.md | 23 ++++++------- ...zones-replace-the-native-images-section.md | 10 +++--- .../0004-twig-hooks-imported-not-prepended.md | 6 ++-- ...taken-over-to-keep-the-advanced-filters.md | 8 ++--- .../0006-taxon-listing-is-the-sylius-grid.md | 18 +++++----- ...-facet-counts-start-from-the-grid-query.md | 10 +++--- ...ilters-are-counted-subqueries-with-caps.md | 18 +++++----- docs/adr/0009-no-like-on-json-columns.md | 11 ++++--- ...-featured-items-filtered-at-render-time.md | 4 +-- ...ons-materialized-as-product-taxon-links.md | 11 +++---- ...onditional-taxon-owns-its-product-links.md | 12 +++---- ...ducts-detached-through-the-unit-of-work.md | 9 ++--- ...tched-through-messenger-after-the-flush.md | 16 ++++----- ...on-applicable-condition-matches-nothing.md | 14 ++++---- ...6-conditions-compare-case-insensitively.md | 5 +-- ...n-conditions-match-the-translated-value.md | 13 ++++---- ...ion-preview-mirrors-the-synchronization.md | 19 ++++++----- docs/adr/0019-pictograms-are-sylius-images.md | 15 +++++---- docs/adr/0020-values-sanitized-on-write.md | 8 ++--- docs/adr/0021-template-icons-are-local.md | 7 ++-- ...-icon-the-shop-cannot-render-is-refused.md | 7 ++-- .../0023-random-media-display-modes-kept.md | 11 ++++--- docs/adr/0024-no-script-in-templates.md | 11 +++---- ...-twig-functions-served-by-lazy-runtimes.md | 8 ++--- ...ts-only-where-the-effect-is-not-obvious.md | 14 ++++---- ...-translation-keys-keep-their-namespaces.md | 5 ++- .../0028-explicit-services-no-autowiring.md | 12 +++---- ...9-per-request-memory-services-are-reset.md | 6 ++-- ...0-bundle-class-with-sylius-plugin-trait.md | 4 +++ .../0031-schema-change-is-a-new-migration.md | 13 ++++---- docs/adr/0032-no-changelog.md | 23 ------------- ...tream-workarounds-never-constrain-shops.md | 21 ++++++++++++ ...tream-workarounds-never-constrain-shops.md | 26 --------------- ...4-static-analysis-on-the-primary-ci-job.md | 24 -------------- .../0035-contributor-stack-ships-chrome.md | 24 -------------- docs/adr/README.md | 7 ++-- docs/architecture/overview.md | 2 +- docs/development/code-map.md | 13 ++++---- docs/domain/conditional-taxons.md | 33 ++++++++++--------- docs/persistence/migrations.md | 4 +-- docs/shop/storefront.md | 3 +- docs/shop/twig-functions.md | 4 ++- src/Entity/AdvancedTaxonValueSanitizer.php | 2 +- .../TaxonMediaZonesTypeExtension.php | 5 ++- src/Form/Extension/TaxonTypeExtension.php | 5 ++- src/Service/ConditionalTaxonQueryBuilder.php | 4 +-- src/Service/FacetProductQueryBuilder.php | 6 ++-- src/Service/SelectAttributeValueResolver.php | 2 +- src/Twig/Runtime/FacetRuntime.php | 2 +- .../FacetProductQueryBuilderTest.php | 3 +- 58 files changed, 260 insertions(+), 340 deletions(-) delete mode 100644 docs/adr/0032-no-changelog.md create mode 100644 docs/adr/0032-upstream-workarounds-never-constrain-shops.md delete mode 100644 docs/adr/0033-upstream-workarounds-never-constrain-shops.md delete mode 100644 docs/adr/0034-static-analysis-on-the-primary-ci-job.md delete mode 100644 docs/adr/0035-contributor-stack-ships-chrome.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 61395b8..0db16db 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -6,7 +6,7 @@ - [ ] Tests pass (`vendor/bin/phpunit`, `vendor/bin/behat --strict`) - [ ] PHPStan and ECS are green -- [ ] If the schema changed: the migration is updated, and `sh tests/check-schema-drift.sh` prints nothing (MySQL or PostgreSQL) +- [ ] If the schema changed: a new migration is added, and `sh tests/check-schema-drift.sh` prints nothing (MySQL or PostgreSQL) - [ ] `UPGRADE.md` is up to date if the public contract changes (traits and interfaces, Twig hooks and functions, grid query) - [ ] A decision taken or reversed has its record in `docs/adr/` - [ ] No customer data, shop secrets or pre-production URLs appear in the code, the tests or this description diff --git a/CLAUDE.md b/CLAUDE.md index b200f36..2d26178 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,11 +51,13 @@ record rather than editing an accepted one - the records hold no path nor figure PHP, then filtered by identifier ([0009](docs/adr/0009-no-like-on-json-columns.md)). - **Services are explicit**: one file per area in `config/services/`, ids prefixed with `cyllene_digital_sylius_advanced_taxon.` (the controller keeps its class as id), no autowiring nor - autoconfiguration (Symfony bundle best practice). A class implementing `ResetInterface` needs the + autoconfiguration. A class implementing `ResetInterface` needs the `kernel.reset` tag by hand ([0028](docs/adr/0028-explicit-services-no-autowiring.md), [0029](docs/adr/0029-per-request-memory-services-are-reset.md)). - **Template icons are local**: `tabler:` when Sylius ships the icon, otherwise a Tabler SVG added to - `assets/icons/` and used as `at:`. A unit test fails on any other prefix (Iconify) + `assets/icons/` and used as `at:` (Sylius ships 82 Tabler icons only, any other is fetched + from Iconify at render time). A unit test fails on an icon written in a template that no local set + ships ([0021](docs/adr/0021-template-icons-are-local.md)). - **Twig hooks are imported, not prepended**: `config/config.yaml` must be loaded after the Sylius configuration, since a prepended hook loses against the Sylius core hooks it overrides @@ -74,7 +76,7 @@ record rather than editing an accepted one - the records hold no path nor figure route and schema are the public contract ([`docs/architecture/public-contract.md`](docs/architecture/public-contract.md)): they only break in a major version, with an entry in `UPGRADE.md`. - **No `CHANGELOG.md`**: the GitHub release notes are the changelog, `UPGRADE.md` says what a shop - has to do ([0032](docs/adr/0032-no-changelog.md)). + has to do. ## Flex recipe @@ -110,7 +112,8 @@ The QA commands are in [`CONTRIBUTING.md`](CONTRIBUTING.md). What it does not sa (`database_setup: schema`), and the `down()` / `up()` replay is their only run of the plugin migration. - **Sylius 2.1 mappings fail `doctrine/orm` 3.7 validation** (`ShipmentUnit#shipment`): CI caps the - ORM below 3.7 before Sylius 2.3, the plugin's own constraint stays open. + ORM below 3.7 before Sylius 2.3, the plugin's own constraint stays open + ([0032](docs/adr/0032-upstream-workarounds-never-constrain-shops.md)). - **Behat**: the Sylius suites are not imported (2.1 / 2.2 ship them in YAML, 2.3 in PHP); the plugin suites only use the Sylius contexts, which are services. The plugin contexts use PHP attributes (`#[Given]`, `#[AfterScenario]`): Behat 4, the one Symfony 8 needs, no longer reads annotations; the CI keeps Behat 3 before Sylius 2.3, @@ -125,14 +128,12 @@ The QA commands are in [`CONTRIBUTING.md`](CONTRIBUTING.md). What it does not sa - **The taxon form template renders `form._token` itself** (`form_with_tabs.html.twig`): with `render_rest` off, nothing else does on Sylius 2.1. - `sylius:debug:twig-hooks` exists from `sylius/twig-hooks` 0.12, Sylius 2.3 only. -- **Template icons must be local**: Sylius ships 82 Tabler icons only; another one goes to - `assets/icons/` as `at:`, or it is fetched from Iconify at render time. - **`symfony/ux-twig-component` is kept below 3.5 in `require-dev`**: 3.5 lets an explicit `null` override a `{% props %}` default, and the Sylius grid delete action passes a null label and icon to `sylius_admin:delete_modal` (empty delete buttons in every admin grid). Drop the constraint once Sylius passes defaults itself ([Sylius#19278](https://github.com/Sylius/Sylius/issues/19278)). An upstream workaround never goes into the plugin's own constraints - ([0033](docs/adr/0033-upstream-workarounds-never-constrain-shops.md)). + ([0032](docs/adr/0032-upstream-workarounds-never-constrain-shops.md)). - `sylius_test_html_attribute()` attributes (`data-test-*`) are what the Behat pages target: keep them when reworking a shop template. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dc2aae1..35d78ef 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -235,8 +235,8 @@ Sylius plugins, do not add personal rules to it. integration tests on a real database; prefer a Behat scenario when the bug is visible in the shop or the back office. - The plugin never replaces a Sylius model: new fields on `Taxon`, `Channel` or `TaxonImage` go into - the matching trait and interface, and the migration - ([why](docs/adr/0001-never-replace-a-sylius-model.md)). + the matching trait and interface, and a new migration + ([why](docs/adr/0001-never-replace-a-sylius-model.md), [migrations](docs/adr/0031-schema-change-is-a-new-migration.md)). - Services are defined explicitly in `config/services/`, without autowiring ([code map](docs/development/code-map.md#wiring), [why](docs/adr/0028-explicit-services-no-autowiring.md)). - Update **`UPGRADE.md`** when the public contract changes; the change itself is described in the diff --git a/assets/shop/js/mobile-menu-controller.js b/assets/shop/js/mobile-menu-controller.js index a90578d..bd8999c 100644 --- a/assets/shop/js/mobile-menu-controller.js +++ b/assets/shop/js/mobile-menu-controller.js @@ -39,10 +39,6 @@ export default class extends Controller { document.removeEventListener('keydown', this._keyHandler); } - // ------------------------------------------------------------------ // - // Public actions (callable via data-action) - // ------------------------------------------------------------------ // - open() { this.element.classList.add('at-mn-drawer--open'); this.element.setAttribute('aria-hidden', 'false'); @@ -97,7 +93,6 @@ export default class extends Controller { this._focusActiveScreen(); } - /** Navigate back to the previous screen. */ goBack() { if (this._stack.length <= 1) { this.close(); return; } const currentId = this._stack.pop(); @@ -110,10 +105,6 @@ export default class extends Controller { this._focusActiveScreen(); } - // ------------------------------------------------------------------ // - // Private helpers - // ------------------------------------------------------------------ // - _handleBurgerClick(event) { // Prevent Bootstrap (or any other handler) from processing this click. event.stopImmediatePropagation(); diff --git a/assets/shop/scss/mega-menu.scss b/assets/shop/scss/mega-menu.scss index fbd23f2..14e95c4 100644 --- a/assets/shop/scss/mega-menu.scss +++ b/assets/shop/scss/mega-menu.scss @@ -14,7 +14,6 @@ border-bottom: 1px solid #dee2e6; } -// Horizontal navigation bar. .at-mn-list { display: flex; list-style: none; @@ -73,7 +72,6 @@ display: block; } -// Centred inner container. .at-mn-panel-inner { display: flex; align-items: flex-start; @@ -116,7 +114,6 @@ &:hover { opacity: 0.8; } } -// Regular columns. .at-mn-cols { display: flex; flex-wrap: wrap; @@ -229,7 +226,6 @@ } } -// Semi-transparent backdrop. .at-mn-drawer-backdrop { position: absolute; inset: 0; @@ -272,7 +268,6 @@ &[data-mn-state="right"] { transform: translateX(100%); pointer-events: none; } } -// Screen header bar. .at-mn-screen-head { display: flex; align-items: center; @@ -331,7 +326,6 @@ &:hover { text-decoration: underline; } } -// List of navigation items. .at-mn-screen-list { list-style: none; padding: 0; diff --git a/config/services.php b/config/services.php index 36f53b2..26f0221 100644 --- a/config/services.php +++ b/config/services.php @@ -6,7 +6,8 @@ /* * Services are defined explicitly, without autowiring nor autoconfiguration, as Symfony recommends - * for reusable bundles: ids are prefixed with the bundle alias and private unless stated otherwise. + * for reusable bundles: ids are prefixed with the bundle alias and private unless stated otherwise (the controller keeps its + * class as id, its route names it). */ return static function (ContainerConfigurator $container): void { $container->import('services/*.php'); diff --git a/docs/README.md b/docs/README.md index e10fec9..7856174 100644 --- a/docs/README.md +++ b/docs/README.md @@ -10,7 +10,7 @@ list: [Features](features.md). Installation in short and production notes: the r |---------|-------------------| | Merchant (back office) | [Back-office guide](admin/user-guide.md), [Storefront](shop/storefront.md), [Conditional taxons](domain/conditional-taxons.md) | | Sylius integrator | [Installation](integration/installation.md), [Configuration](integration/configuration.md), [Theming and Twig hooks](integration/theming.md), [Assets](integration/assets.md), [Twig functions](shop/twig-functions.md), [Public contract](architecture/public-contract.md), [Troubleshooting](integration/troubleshooting.md) | -| Ops (deployment) | [Migrations](persistence/migrations.md), [Database schema](persistence/schema.md), [Configuration: Messenger](integration/configuration.md), [Uninstall](integration/uninstall.md), [UPGRADE](../UPGRADE.md) | +| Ops (deployment) | [Migrations](persistence/migrations.md), [Database schema](persistence/schema.md), [Configuration: Messenger](integration/configuration.md#conditional-taxon-synchronization-messenger), [Uninstall](integration/uninstall.md), [UPGRADE](../UPGRADE.md) | | Plugin contributor | [Architecture overview](architecture/overview.md), [Decision records](adr/README.md), [Code map](development/code-map.md), [Tests](testing/tests.md), [CONTRIBUTING](../CONTRIBUTING.md) | ## Table of contents diff --git a/docs/adr/0001-never-replace-a-sylius-model.md b/docs/adr/0001-never-replace-a-sylius-model.md index 73c9140..7ed70f8 100644 --- a/docs/adr/0001-never-replace-a-sylius-model.md +++ b/docs/adr/0001-never-replace-a-sylius-model.md @@ -5,23 +5,23 @@ ## Context -A plugin that adds fields to taxons, channels and taxon images could ship its own entities and -ask the application to use them. The application then loses its own models, and two plugins doing -the same cannot be installed together. A relation that names a concrete class also breaks as soon -as the application overrides that class. +A plugin that adds fields to taxons, channels and taxon images could ship its own entities and ask +the application to use them. The application then loses its own models, and two plugins doing the +same cannot be installed together. A relation that names a concrete class also breaks as soon as the +application overrides that class. ## Decision -The application keeps its own `Taxon`, `Channel` and `TaxonImage`. The plugin ships a trait and +The application keeps its own taxon, channel and taxon image entities. The plugin ships a trait and an interface for each, with the mapping as attributes in the trait, the usual Sylius 2 pattern: an XML mapping to copy into the application would be heavier to install. Relations target the Sylius -interfaces, resolved by Doctrine to the application's classes. The code tests the plugin interfaces, -never a class. What the plugin owns alone lives in tables of its own. +interfaces, resolved by Doctrine to the application's classes. The code tests interfaces, never a +Sylius or application model class. What the plugin owns alone lives in tables of its own. ## Consequences -- Installing the plugin means editing three application entities, which the installation guide - and the Flex recipe describe. +- Installing the plugin means editing three application entities, which the installation guide and + the Flex recipe describe. - A taxon that does not implement the plugin interface is ignored by the shop helpers instead of failing. - The traits and interfaces are public contract: changing them is a major version. diff --git a/docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md b/docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md index 0d95ee8..c05ea4d 100644 --- a/docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md +++ b/docs/adr/0002-universe-kept-out-of-main-taxons-by-a-constraint.md @@ -5,23 +5,20 @@ ## Context -A universe page lists child taxons, not products. A product whose main taxon is a universe would -get a breadcrumb and a canonical URL pointing to a page where it does not appear. Filtering the -choices of the main taxon field of the product form would only protect the back office, and -replacing a core field leaves the loading order of the form extensions decide which definition -wins, losing the native options of the field. +A universe page lists child taxons, not products. A product whose main taxon is a universe would get +a breadcrumb and a canonical URL pointing to a page where it does not appear. Filtering the choices +of the main taxon field of the product form would only protect the back office, and replacing a core +field leaves the loading order of the form extensions decide which definition wins, losing the +native options of the field. ## Decision -A validation constraint on the product, in the `sylius` group, rejects a universe as main taxon. -The product form is not touched. +A validation constraint on the product, in the `sylius` group, rejects a universe as main taxon. The +product form is not touched. ## Consequences -- The rule holds wherever the product is validated: back office, API, imports, fixtures. +- The rule holds wherever the product is validated in the `sylius` group: the back office and the + API. - A universe still appears in the choices of the field; picking it shows a validation error. -- Code that saves a product without validating it is not protected. - -## Related - -- [0003. Plugin media zones replace the native taxon Images section](0003-media-zones-replace-the-native-images-section.md) +- Code that saves a product without validating it, such as the Sylius fixtures, is not protected. diff --git a/docs/adr/0003-media-zones-replace-the-native-images-section.md b/docs/adr/0003-media-zones-replace-the-native-images-section.md index 1dcf58d..3802c51 100644 --- a/docs/adr/0003-media-zones-replace-the-native-images-section.md +++ b/docs/adr/0003-media-zones-replace-the-native-images-section.md @@ -6,17 +6,17 @@ ## Context The plugin places taxon images in zones (main image, top, bottom, left, inside the product list, -featured, universe slider), each with its own settings, stored as Sylius taxon images with a type. -Keeping the native Images collection next to the zones would have two forms managing the same -images, and the native collection would delete the zone media when the taxon is saved. Showing +featured, universe slider), most with their own display settings, stored as Sylius taxon images with +a type. Keeping the native Images collection next to the zones would have two forms managing the +same images, and the native collection would delete the zone media when the taxon is saved. Showing the native collection only for the types the plugin does not use would still give two image interfaces in one form. ## Decision The native Images section is removed from the taxon form and replaced by the zones. A main image -zone takes over the images Sylius stores without type; they get the type of that zone when the -taxon is saved, and the shop keeps showing the first one as the taxon image. +zone takes over the images Sylius stores without type; they get the type of that zone when the taxon +is saved, and the shop shows the first one, by position, as the taxon image. ## Consequences diff --git a/docs/adr/0004-twig-hooks-imported-not-prepended.md b/docs/adr/0004-twig-hooks-imported-not-prepended.md index ae95c92..e138a55 100644 --- a/docs/adr/0004-twig-hooks-imported-not-prepended.md +++ b/docs/adr/0004-twig-hooks-imported-not-prepended.md @@ -5,9 +5,9 @@ ## Context -Several plugin hooks override hookables of the Sylius core. A configuration prepended by the -bundle extension is loaded before the application's and the Sylius one, so the Sylius core hooks -would win over the plugin hooks that replace them. +Several plugin hooks override hookables of the Sylius core. A configuration prepended by the bundle +extension is loaded before the application's and the Sylius one, so the Sylius core hooks would win +over the plugin hooks that replace them. ## Decision diff --git a/docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md b/docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md index 12d8ffd..174d910 100644 --- a/docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md +++ b/docs/adr/0005-search-form-taken-over-to-keep-the-advanced-filters.md @@ -5,16 +5,16 @@ ## Context -The search form of the Sylius product listing only sends the search text: submitting it dropped +The search form of the Sylius product listing only sends the search text: submitting it would drop the advanced filters, the sort, the limit and the view. Hidden fields added through the nested search hook would land inside the input group and break its rounded corners. A Stimulus controller copying the parameters would not work without JavaScript and adds code. ## Decision -The plugin replaces the search hookable of the listing filters with a form that sends the -current advanced filters, sort, limit and view back, and starts again from the first page. Its -clear button only removes the search. +The plugin replaces the search hookable of the listing filters with a form that sends the current +advanced filters, sort, limit and view back, and starts again from the first page. Its clear button +only removes the search. ## Consequences diff --git a/docs/adr/0006-taxon-listing-is-the-sylius-grid.md b/docs/adr/0006-taxon-listing-is-the-sylius-grid.md index 368ab1d..fe6dfae 100644 --- a/docs/adr/0006-taxon-listing-is-the-sylius-grid.md +++ b/docs/adr/0006-taxon-listing-is-the-sylius-grid.md @@ -5,24 +5,26 @@ ## Context -The plugin adds products of child taxons and advanced filters to the taxon page. A product query -of its own, in a Twig function, would lose what the Sylius grid does: pagination, sorting, search, +The plugin adds products of child taxons and advanced filters to the taxon page. A product query of +its own, in a Twig function, would lose what the Sylius grid does: pagination, sorting, search, channel restriction, and what other plugins add to it. Decorating the product repository is more -intrusive and clashes with the other plugins decorating it too. +intrusive and has to compose with any other plugin decorating it. ## Decision -The plugin listens to the definition of the shop product grid and points its repository method -to a query builder that calls the native one, then adds the children products and the advanced -filters to the same query. An application that already replaced that method keeps its own query. +The plugin listens to the definition of the shop product grid and points its repository method to a +query builder that calls the native one, asking it for the descendants' products when the taxon +includes them, then adds the advanced filters to the same query. An application that already +replaced that method keeps its own query. ## Consequences - No parallel product query in the templates: the grid stays the only listing. - The grid query builder is public contract. -- An application with its own listing query does not get the plugin filters on it. +- An application with its own listing query does not get the plugin filters nor the children + products option on it. ## Related - [0007. Facet counts start from the query the grid lists](0007-facet-counts-start-from-the-grid-query.md) -- [0008. Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) +- [0008. Attribute and option filters are counted subqueries, with caps](0008-advanced-filters-are-counted-subqueries-with-caps.md) diff --git a/docs/adr/0007-facet-counts-start-from-the-grid-query.md b/docs/adr/0007-facet-counts-start-from-the-grid-query.md index 590383d..2773342 100644 --- a/docs/adr/0007-facet-counts-start-from-the-grid-query.md +++ b/docs/adr/0007-facet-counts-start-from-the-grid-query.md @@ -10,10 +10,10 @@ computed on another product set than the listing would announce products the pag ## Decision -The facet queries start from a base query that selects exactly what the grid lists: enabled -products of the current channel, in the taxon or its descendants, with the grid's translation -join, its search and its descendants setting. Each facet family ignores its own selection when it -counts the alternatives. +The facet queries start from a base query that selects exactly what the grid lists: enabled products +of the current channel, in the taxon or its descendants, with the grid's translation join, its +search and its descendants setting. Each facet family ignores its own selection when it counts the +alternatives. ## Consequences @@ -23,4 +23,4 @@ counts the alternatives. ## Related - [0006. The taxon listing is the Sylius grid](0006-taxon-listing-is-the-sylius-grid.md) -- [0008. Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) +- [0008. Attribute and option filters are counted subqueries, with caps](0008-advanced-filters-are-counted-subqueries-with-caps.md) diff --git a/docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md b/docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md index dc48833..c7cbc44 100644 --- a/docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md +++ b/docs/adr/0008-advanced-filters-are-counted-subqueries-with-caps.md @@ -1,25 +1,25 @@ -# 0008. Advanced filters are counted subqueries, with caps on codes and values +# 0008. Attribute and option filters are counted subqueries, with caps - Status: accepted - Date: 2026-10-14 ## Context -The advanced filters come from the query string, which anyone can write. Each filtered attribute -or option added joins: a forged URL could join more tables than MySQL accepts and answer an error, -or send heavy queries for every page view without being logged in. With about ten filters as -joins, the join order search of MySQL does not finish; an `EXISTS` subquery does not help, MySQL -turns it back into joins. +The advanced filters come from the query string, which anyone can write. Each filtered attribute or +option added joins: a forged URL could join more tables than MySQL accepts and answer an error, or +send heavy queries for every page view without being logged in. With a handful of filters as joins, +the join order search of MySQL does not finish; an `EXISTS` subquery does not help, MySQL turns it +back into joins. ## Decision Each attribute or option filter is a correlated count subquery compared to zero. The number of -filtered codes and the number of values per code are capped; what goes beyond is ignored. +filtered codes, of values per code and of sub-taxons is capped; what goes beyond is ignored. ## Consequences -- The query stays the same shape whatever the database, and a forged URL cannot make it grow - without limit. +- The number of joined tables no longer grows with the filters, and a forged URL cannot make the + query grow without limit. - A shop that needs more filters at once than the caps allow cannot get them from the URL. - The caps are constants of the plugin, not configuration. diff --git a/docs/adr/0009-no-like-on-json-columns.md b/docs/adr/0009-no-like-on-json-columns.md index 9a0eb16..e01810a 100644 --- a/docs/adr/0009-no-like-on-json-columns.md +++ b/docs/adr/0009-no-like-on-json-columns.md @@ -10,16 +10,17 @@ MySQL and MariaDB, not on PostgreSQL, and depends on how the JSON is serialized. ## Decision -The choices of a select attribute are read once per attribute code and matched in PHP; the query -then filters on the identifiers of the matching values. +The select values of an attribute, and its choice labels, are read once per attribute code and +request, and matched in PHP; the query then filters on the identifiers of the matching values. ## Consequences -- Every value of a filtered select attribute is read, once per request and code: an accepted - cost on a large catalog. +- Every value of a filtered select attribute is read, once per request and code: an accepted cost on + a large catalog. - The filters behave the same on MySQL, MariaDB and PostgreSQL. ## Related -- [0008. Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) +- [0007. Facet counts start from the query the grid lists](0007-facet-counts-start-from-the-grid-query.md) +- [0008. Attribute and option filters are counted subqueries, with caps](0008-advanced-filters-are-counted-subqueries-with-caps.md) - [0016. Conditions compare case-insensitively on every database](0016-conditions-compare-case-insensitively.md) diff --git a/docs/adr/0010-featured-items-filtered-at-render-time.md b/docs/adr/0010-featured-items-filtered-at-render-time.md index d70858a..019f0aa 100644 --- a/docs/adr/0010-featured-items-filtered-at-render-time.md +++ b/docs/adr/0010-featured-items-filtered-at-render-time.md @@ -6,8 +6,8 @@ ## Context A taxon features products and child taxons picked in the back office. Later on, a product can be -disabled or leave the channel, a taxon can be disabled or moved under another parent. Validating -the selection on save would not catch these later changes. +disabled or leave the channel, a taxon can be disabled or moved under another parent. Validating the +selection on save would not catch these later changes. ## Decision diff --git a/docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md b/docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md index 6a9d059..5e6a2f1 100644 --- a/docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md +++ b/docs/adr/0011-conditional-taxons-materialized-as-product-taxon-links.md @@ -5,10 +5,9 @@ ## Context -A conditional taxon gets its products from rules (attribute, option, name, description, stock, -taxon membership). Resolving the -rules when the taxon is listed would only serve the plugin listing: the API, the search, the -product forms and the other plugins read the product-taxon links Sylius stores. +A conditional taxon gets its products from rules (attribute, option, name, description, stock, taxon +membership). Resolving the rules when the taxon is listed would only serve the plugin listing: the +API, the search, the product forms and the other plugins read the product-taxon links Sylius stores. ## Decision @@ -19,8 +18,8 @@ Nothing is resolved when the shop reads the taxon. ## Consequences - Every part of Sylius sees a conditional taxon as a normal one. -- A product that starts or stops matching (a new attribute value, a new product) is only attached - or detached by the next synchronization: the command is meant to be scheduled. +- A product that starts or stops matching (a new attribute value, a new product) is only attached or + detached by the next synchronization: the command is meant to be scheduled. ## Related diff --git a/docs/adr/0012-conditional-taxon-owns-its-product-links.md b/docs/adr/0012-conditional-taxon-owns-its-product-links.md index ef7f7f7..011a147 100644 --- a/docs/adr/0012-conditional-taxon-owns-its-product-links.md +++ b/docs/adr/0012-conditional-taxon-owns-its-product-links.md @@ -5,15 +5,15 @@ ## Context -Products can be attached to a taxon by hand before it becomes conditional, or after. Keeping -them as manual assignments next to the materialized ones would make the content of a conditional -taxon depend on its history, not on its rules. +Products can be attached to a taxon by hand before it becomes conditional, or after. Keeping them as +manual assignments next to the materialized ones would make the content of a conditional taxon +depend on its history, not on its rules. ## Decision -The synchronization alone manages the links of a conditional taxon: products attached by hand -that do not match are detached. Removing the last condition, or turning the taxon back into a -regular one, detaches all its products. +The synchronization alone manages the links of a conditional taxon: products attached by hand that +do not match are detached. Removing the last condition, or turning the taxon back into a regular +one, detaches all its products. ## Consequences diff --git a/docs/adr/0013-products-detached-through-the-unit-of-work.md b/docs/adr/0013-products-detached-through-the-unit-of-work.md index 69d2091..5bea350 100644 --- a/docs/adr/0013-products-detached-through-the-unit-of-work.md +++ b/docs/adr/0013-products-detached-through-the-unit-of-work.md @@ -6,13 +6,13 @@ ## Context Detaching products from a conditional taxon with a bulk `DELETE` would skip the Doctrine events -(search indexers listen to them) and leave the product collections already in memory with links -that no longer exist. +(search indexers listen to them) and leave the product collections already in memory with links that +no longer exist. ## Decision -The links to remove are loaded and removed through the entity manager, by chunks. A product -already in memory has its collection updated; the others are not loaded. +The links to remove are loaded and removed through the entity manager, by chunks. A product already +in memory has its collection updated; the others are not loaded. ## Consequences @@ -22,3 +22,4 @@ already in memory has its collection updated; the others are not loaded. ## Related - [0012. A conditional taxon owns its product links](0012-conditional-taxon-owns-its-product-links.md) +- [0014. The synchronization is dispatched through Messenger once the flush is over, synchronous unless routed](0014-synchronization-dispatched-through-messenger-after-the-flush.md) diff --git a/docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md b/docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md index e8ab486..43024d4 100644 --- a/docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md +++ b/docs/adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md @@ -6,21 +6,21 @@ ## Context Saving a conditional taxon has to recompute its products. Doing it in a Doctrine `postFlush` -listener means flushing inside a flush, which Doctrine does not support: the collection deletions -of the outer flush, not cleaned up yet, run again and lose data. Doing it in a worker only would -force every shop to run one. +listener means flushing inside a flush, which Doctrine does not support: the collection deletions of +the outer flush, not cleaned up yet, run again and lose data. Doing it in a worker only would force +every shop to run one. ## Decision -The taxons to synchronize are collected during the flush; a message per taxon is dispatched on -the Sylius command bus once the flush is over, at the end of the request, of the console command -or of the message a worker handled. The handler is registered on that bus only, whose transaction +The taxons to synchronize are collected during the flush; a message per taxon is dispatched on the +Sylius command bus once the flush is over, at the end of the request, of the console command or of +the message a worker handled. The handler is registered on that bus only, whose transaction middleware flushes. With no routing, the message is handled synchronously. ## Consequences -- No dependency on a transport: a shop routes the message to an asynchronous one when saving a - taxon must not wait for the assignments. +- No dependency on a transport: a shop routes the message to an asynchronous one when saving a taxon + must not wait for the assignments. - Synchronously, a failing synchronization answers an error although the taxon is already saved. - Code that flushes outside a request, a command or a worker (a plain script) leaves the synchronization to the command. diff --git a/docs/adr/0015-non-applicable-condition-matches-nothing.md b/docs/adr/0015-non-applicable-condition-matches-nothing.md index d0e5ea7..d171017 100644 --- a/docs/adr/0015-non-applicable-condition-matches-nothing.md +++ b/docs/adr/0015-non-applicable-condition-matches-nothing.md @@ -5,21 +5,21 @@ ## Context -A stored condition can stop being applicable: an operator its type does not accept, a missing -value, an attribute, option or taxon that was deleted. Ignoring it would widen the taxon, up to -the whole catalog when it was the only condition. +A stored condition can be or become not applicable: an operator its type does not accept (written by +the API or an import), a missing value, an attribute, option or taxon that was deleted. Ignoring it +would widen the taxon, up to the whole catalog when it was the only condition. ## Decision Conditional taxons fail closed: a condition that cannot apply makes the taxon match nothing, -negative operators included. A branch the validation makes unreachable throws instead of skipping -the condition. +negative operators included. A branch the applicability check makes unreachable throws instead of +skipping the condition. ## Consequences - A broken condition empties the taxon instead of flooding it: visible, and harmless. -- The validation rejects what it can on save (wrong operator, missing reference or value, unknown - or self reference); what breaks later is caught by the query. +- The validation rejects what it can on save (wrong operator, missing reference or value, unknown or + self reference); what breaks later is caught by the query. ## Related diff --git a/docs/adr/0016-conditions-compare-case-insensitively.md b/docs/adr/0016-conditions-compare-case-insensitively.md index 9ab6565..9b2e0d6 100644 --- a/docs/adr/0016-conditions-compare-case-insensitively.md +++ b/docs/adr/0016-conditions-compare-case-insensitively.md @@ -6,7 +6,7 @@ ## Context MySQL and MariaDB compare text case-insensitively with the usual collations, PostgreSQL does not: -the same condition matched different products depending on the database. +the same condition would match different products depending on the database. ## Decision @@ -14,7 +14,8 @@ Text comparisons of the conditions lower both sides. ## Consequences -- A conditional taxon lists the same products on MySQL, MariaDB and PostgreSQL. +- Case no longer makes a conditional taxon list different products on MySQL, MariaDB and PostgreSQL; + accent sensitivity still follows the database collation. - A condition cannot tell values apart by their case. ## Related diff --git a/docs/adr/0017-option-conditions-match-the-translated-value.md b/docs/adr/0017-option-conditions-match-the-translated-value.md index ef93794..472d514 100644 --- a/docs/adr/0017-option-conditions-match-the-translated-value.md +++ b/docs/adr/0017-option-conditions-match-the-translated-value.md @@ -5,9 +5,8 @@ ## Context -An option condition compares the translated value of an option value, in any locale, not its -code, and its `in` operator takes a single value. Changing either changes what stored conditions -mean. +An option condition compares the translated value of an option value, in any locale, not its code, +and its `in` operator takes a single value. Changing either changes what stored conditions mean. ## Decision @@ -15,10 +14,10 @@ Both are kept as accepted limitations, documented with the conditions. ## Consequences -- Renaming an option value in every locale breaks the conditions that use it: positive ones - stop matching, negative ones stop excluding. -- Matching on codes, or several values for `in`, would be a new operator or a major version, not - a change of the existing ones. +- Renaming an option value in every locale breaks the conditions that use it: positive ones stop + matching, negative ones stop excluding. +- Matching on codes, or several values for `in`, would be a new operator or a major version, not a + change of the existing ones. ## Related diff --git a/docs/adr/0018-condition-preview-mirrors-the-synchronization.md b/docs/adr/0018-condition-preview-mirrors-the-synchronization.md index 62f0378..742e082 100644 --- a/docs/adr/0018-condition-preview-mirrors-the-synchronization.md +++ b/docs/adr/0018-condition-preview-mirrors-the-synchronization.md @@ -5,23 +5,26 @@ ## Context -The back office previews the products matching the conditions being edited. A preview that -reads another locale, or other products, than the synchronization would announce a result the -taxon will not get. +The back office previews the products matching the conditions being edited. A preview that reads +another locale, or other products, than the synchronization would announce a result the taxon will +not get. ## Decision -The preview runs the synchronization query on the same products, disabled ones included, in the -default locale, whatever the locale of the back office. The request body is decoded leniently: an -invalid body previews no product instead of failing. The number of conditions read and of products -returned is capped. +The preview applies the conditions the way the synchronization does, on the same products, disabled +ones included, in the default locale, whatever the locale of the back office. The request body is +decoded leniently: an invalid body previews no product instead of failing. The number of conditions +read and of products returned is capped. ## Consequences -- What the preview shows is what the next synchronization attaches. +- Once every row is complete and valid, what the preview shows is what the next synchronization + attaches. - Rows being typed, incomplete or with a reference that does not exist, are skipped instead of failing the preview. ## Related +- [0011. Conditional taxons are materialized as product-taxon links](0011-conditional-taxons-materialized-as-product-taxon-links.md) - [0015. A condition that cannot apply matches nothing](0015-non-applicable-condition-matches-nothing.md) +- [0016. Conditions compare case-insensitively on every database](0016-conditions-compare-case-insensitively.md) diff --git a/docs/adr/0019-pictograms-are-sylius-images.md b/docs/adr/0019-pictograms-are-sylius-images.md index 4f78007..4d706ae 100644 --- a/docs/adr/0019-pictograms-are-sylius-images.md +++ b/docs/adr/0019-pictograms-are-sylius-images.md @@ -5,16 +5,16 @@ ## Context -A taxon can show an uploaded pictogram instead of a glyph. Storing the file in the public -directory breaks on several servers, on a read-only container, and on a shop whose images are in -object storage. +A taxon can show an uploaded pictogram instead of a glyph. Storing the file in the public directory +breaks on several servers, on a read-only container, and on a shop whose images are in object +storage. ## Decision -A pictogram is stored through the Sylius image uploader and storage, and resized by a Liip -Imagine filter the plugin prepends (an application can override it). The icon field holds an -`image:` reference. The file is removed once the change that drops it is flushed; the previous -value is read from the database, never from the submitted form. +A pictogram is stored through the Sylius image uploader and storage, and resized by a Liip Imagine +filter the plugin prepends (an application can override it). The icon field holds an `image:` +reference. The file is removed once the change that drops it is flushed; the previous value is the +one loaded with the taxon, never the submitted one. ## Consequences @@ -25,3 +25,4 @@ value is read from the database, never from the submitted form. - [0003. Plugin media zones replace the native taxon Images section](0003-media-zones-replace-the-native-images-section.md) - [0020. Values rendered in `style` or `href` are sanitized on write](0020-values-sanitized-on-write.md) +- [0022. An icon the shop cannot render is refused on save](0022-icon-the-shop-cannot-render-is-refused.md) diff --git a/docs/adr/0020-values-sanitized-on-write.md b/docs/adr/0020-values-sanitized-on-write.md index 1b25f76..acc6aff 100644 --- a/docs/adr/0020-values-sanitized-on-write.md +++ b/docs/adr/0020-values-sanitized-on-write.md @@ -11,10 +11,10 @@ declaration. ## Decision -The setters sanitize these values: a color that is not a hexadecimal color, an icon that is not -an icon name or an image reference, a link that is neither an HTTP(S) URL nor a same-origin path -are dropped. Media links are sanitized -again when read. The media display mode setters fall back to their default on an unknown value. +The setters sanitize these values: a color that is not a hexadecimal color, an icon that is not an +icon name or an image reference, a link that is neither an HTTP(S) URL nor a same-origin path are +dropped. Media links are sanitized again when rendered. The setters of the display choices fall back +to their default on an unknown value. ## Consequences diff --git a/docs/adr/0021-template-icons-are-local.md b/docs/adr/0021-template-icons-are-local.md index e0acca9..d78e9dd 100644 --- a/docs/adr/0021-template-icons-are-local.md +++ b/docs/adr/0021-template-icons-are-local.md @@ -12,9 +12,9 @@ template icon outside the few Tabler icons Sylius ships would then depend on the ## Decision Templates only use Tabler icons: the ones Sylius ships, or a Tabler SVG added to the plugin's own -set under a prefix of its own. A unit test fails on any other prefix. The plugin set does not reuse -a library prefix: a local set named after a library would silently hide that library for the whole -shop. +set under a prefix of its own. A unit test fails on an icon written in a template that no local set +ships. The plugin set does not reuse a library prefix: a local set named after a library would +silently hide that library for the whole shop. ## Consequences @@ -25,4 +25,3 @@ shop. ## Related - [0022. An icon the shop cannot render is refused on save](0022-icon-the-shop-cannot-render-is-refused.md) -- [0024. No script in the templates, behaviour in Stimulus controllers](0024-no-script-in-templates.md) diff --git a/docs/adr/0022-icon-the-shop-cannot-render-is-refused.md b/docs/adr/0022-icon-the-shop-cannot-render-is-refused.md index 20aa091..cdf726e 100644 --- a/docs/adr/0022-icon-the-shop-cannot-render-is-refused.md +++ b/docs/adr/0022-icon-the-shop-cannot-render-is-refused.md @@ -10,13 +10,14 @@ library the shop does not have disappears without any error. ## Decision -On save, the taxon form asks the icon renderer to render the icon and refuses it with a -validation error when it cannot. +On save, the taxon form asks the icon renderer to render a new icon and adds a form error when it +cannot. ## Consequences - An administrator sees at once that an icon does not exist. -- An icon that becomes unavailable later (a library removed) still disappears silently. +- An icon already stored is not checked again: one that becomes unavailable later (a library + removed) still disappears silently. ## Related diff --git a/docs/adr/0023-random-media-display-modes-kept.md b/docs/adr/0023-random-media-display-modes-kept.md index a4b1217..dce6453 100644 --- a/docs/adr/0023-random-media-display-modes-kept.md +++ b/docs/adr/0023-random-media-display-modes-kept.md @@ -5,14 +5,14 @@ ## Context -Some media zones offer a random display mode (shuffled media, a random spotlight, media inserted -at random places in the listing). A page that changes on every view cannot be cached as such by an -HTTP cache. +Some media zones offer a random display mode (shuffled media, a random spotlight, media inserted at +random places in the listing). A page that changes on every view cannot be cached as such by an HTTP +cache. ## Decision -The random modes are kept: they are a merchandising feature, chosen per taxon. Behind an HTTP -cache, the draw is frozen for the lifetime of each cache entry. +The random modes are kept: they are a merchandising feature, chosen per taxon. Behind an HTTP cache, +the draw is frozen for the lifetime of each cache entry. ## Consequences @@ -21,3 +21,4 @@ cache, the draw is frozen for the lifetime of each cache entry. ## Related - [0003. Plugin media zones replace the native taxon Images section](0003-media-zones-replace-the-native-images-section.md) +- [0020. Values rendered in `style` or `href` are sanitized on write](0020-values-sanitized-on-write.md) diff --git a/docs/adr/0024-no-script-in-templates.md b/docs/adr/0024-no-script-in-templates.md index fc3de48..75434bd 100644 --- a/docs/adr/0024-no-script-in-templates.md +++ b/docs/adr/0024-no-script-in-templates.md @@ -5,14 +5,13 @@ ## Context -Inline scripts are hard to test, cannot be overridden by a theme without copying the template, -and conflict with a strict Content Security Policy. +Inline scripts are hard to test, cannot be overridden by a theme without copying the template, and +conflict with a strict script Content Security Policy. ## Decision -The templates hold no script. A behaviour is a Stimulus controller of the plugin entrypoints, -under a prefix of its own; the texts it shows come from the translations, passed as Stimulus -values. +The templates hold no script. A behaviour lives in the plugin entrypoints, as a Stimulus controller +under a prefix of its own; the texts it shows come from the translations, passed as Stimulus values. ## Consequences @@ -21,4 +20,4 @@ values. ## Related -- [0021. Template icons are local](0021-template-icons-are-local.md) +- [0027. Translation keys keep their namespaces](0027-translation-keys-keep-their-namespaces.md) diff --git a/docs/adr/0025-twig-functions-served-by-lazy-runtimes.md b/docs/adr/0025-twig-functions-served-by-lazy-runtimes.md index ef065c9..79372ef 100644 --- a/docs/adr/0025-twig-functions-served-by-lazy-runtimes.md +++ b/docs/adr/0025-twig-functions-served-by-lazy-runtimes.md @@ -5,13 +5,13 @@ ## Context -The plugin declares many Twig functions, most of them used by a single kind of page. Declaring -them on extensions that hold their dependencies builds every dependency on every page. +The plugin declares many Twig functions, most of them used by a single kind of page. Declaring them +on extensions that hold their dependencies builds every dependency on every page. ## Decision -One Twig extension declares the functions; each is served by a runtime, built on the first call -of one of its functions. +One Twig extension declares the functions; each is served by a runtime, built on the first call of +one of its functions. ## Consequences diff --git a/docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md b/docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md index a1b3683..85cf533 100644 --- a/docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md +++ b/docs/adr/0026-help-texts-only-where-the-effect-is-not-obvious.md @@ -5,19 +5,19 @@ ## Context -The taxon form has many fields. A help text on each would load the form, multiply the texts to -translate and repeat the user guide until both diverge. Tooltips hide the help, work poorly on -touch screens and need JavaScript. +The taxon form has many fields. Explanations repeated in the form would multiply the texts to +translate and repeat the user guide until both diverge. Tooltips hide the help, work poorly on touch +screens and need JavaScript. ## Decision -A field gets a one-line help text only when its effect is not obvious: a side effect, a scope -narrower than expected, an expected format. The user guide stays the reference. +Help is a short text under the field, never a tooltip. It says what the field does when the label +does not, or what it expects; the explanations stay in the user guide, which is the reference. ## Consequences -- A new field gets a help text only if it meets that bar. -- The form stays readable; the details are in the guide. +- The help is visible without interaction, on any screen. +- A help text that grows into an explanation moves to the guide. ## Related diff --git a/docs/adr/0027-translation-keys-keep-their-namespaces.md b/docs/adr/0027-translation-keys-keep-their-namespaces.md index 53ae81f..689749b 100644 --- a/docs/adr/0027-translation-keys-keep-their-namespaces.md +++ b/docs/adr/0027-translation-keys-keep-their-namespaces.md @@ -5,9 +5,8 @@ ## Context -The translation keys of the plugin sit under a few namespaces (shop, form, ui, facet), while -other plugins put their labels under a single `ui` namespace. Aligning them renames keys an -application may have overridden. +The translation keys of the plugin sit under a few namespaces (shop, form, ui, facet). Moving them +under a single namespace renames keys an application may have overridden. ## Decision diff --git a/docs/adr/0028-explicit-services-no-autowiring.md b/docs/adr/0028-explicit-services-no-autowiring.md index a02ef34..9841630 100644 --- a/docs/adr/0028-explicit-services-no-autowiring.md +++ b/docs/adr/0028-explicit-services-no-autowiring.md @@ -6,21 +6,21 @@ ## Context Symfony recommends that a reusable bundle does not rely on autowiring nor autoconfiguration: its -services must not depend on what the host application happens to alias or tag, and autowiring -would turn every class of the plugin into an injectable service, public by accident. +services must not depend on what the host application happens to alias or tag, and autowiring would +turn every class of the plugin into an injectable service, public by accident. ## Decision -Every service is declared by hand, one file per area, with a prefixed id and every argument and -tag written out. The controller is the exception to the prefix: it keeps its class as id, since its +Every service is declared by hand, one file per area, with a prefixed id and every argument and tag +written out. The controller is the exception to the prefix: it keeps its class as id, since its route names it that way. The interfaces an application may decorate are aliased to their service. ## Consequences - A new constructor argument is added to the service definition. - A tag autoconfiguration would have set is added by hand. -- Only the attributes read at runtime are used in the classes; the ones that only - autoconfiguration reads would do nothing. +- Only the attributes read at runtime are used in the classes; the ones that only autoconfiguration + reads would do nothing. ## Related diff --git a/docs/adr/0029-per-request-memory-services-are-reset.md b/docs/adr/0029-per-request-memory-services-are-reset.md index 161ecab..b2a7536 100644 --- a/docs/adr/0029-per-request-memory-services-are-reset.md +++ b/docs/adr/0029-per-request-memory-services-are-reset.md @@ -12,8 +12,9 @@ the next. ## Decision -Services are `final` and stateless where they can be. Those that hold per-request memory -implement `ResetInterface`, and get the `kernel.reset` tag in their definition. +Services are `final` and stateless where they can be. Those that hold per-request memory implement +`ResetInterface`, and get the `kernel.reset` tag in their definition. A weak map keyed by an entity +needs no reset: it frees itself with the entity. ## Consequences @@ -24,4 +25,5 @@ implement `ResetInterface`, and get the `kernel.reset` tag in their definition. ## Related - [0014. The synchronization is dispatched through Messenger once the flush is over, synchronous unless routed](0014-synchronization-dispatched-through-messenger-after-the-flush.md) +- [0025. Twig functions are served by lazy runtimes](0025-twig-functions-served-by-lazy-runtimes.md) - [0028. Services are declared explicitly, without autowiring](0028-explicit-services-no-autowiring.md) diff --git a/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md b/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md index 7512fa6..2aebc35 100644 --- a/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md +++ b/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md @@ -21,4 +21,8 @@ other Sylius plugins. ## Related +- [0004. Twig hooks are imported, not prepended](0004-twig-hooks-imported-not-prepended.md) +- [0019. Pictograms are Sylius images](0019-pictograms-are-sylius-images.md) +- [0021. Template icons are local](0021-template-icons-are-local.md) - [0028. Services are declared explicitly, without autowiring](0028-explicit-services-no-autowiring.md) +- [0031. A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) diff --git a/docs/adr/0031-schema-change-is-a-new-migration.md b/docs/adr/0031-schema-change-is-a-new-migration.md index 3a39722..bba1f46 100644 --- a/docs/adr/0031-schema-change-is-a-new-migration.md +++ b/docs/adr/0031-schema-change-is-a-new-migration.md @@ -5,21 +5,20 @@ ## Context -The plugin adds columns to Sylius tables and owns tables of its own; shops run it on MySQL, -MariaDB or PostgreSQL. A migration written in SQL would target one of them. A published migration -that is edited never runs again on the shops that already executed it. +The plugin adds columns to Sylius tables and owns tables of its own; shops run it on MySQL, MariaDB +or PostgreSQL. A migration written in SQL would target one of them. A published migration that is +edited never runs again on the shops that already executed it. ## Decision The schema changes through migrations of the plugin, written against the DBAL `Schema` API and -registered through Sylius, which lets an application turn their prepending off. A released -migration is never edited: a change is a new migration. CI replays the migrations down and up and -checks that the schema has no drift from the mapping on every database. +registered through Sylius, which lets an application turn their prepending off. A released migration +is never edited: a change is a new migration. ## Consequences - One migration source for the three databases. -- A change to the columns added to Sylius tables also updates the drift check. +- A shop that already ran a migration gets a schema change from the next one. - The schema is public contract. ## Related diff --git a/docs/adr/0032-no-changelog.md b/docs/adr/0032-no-changelog.md deleted file mode 100644 index 7168b53..0000000 --- a/docs/adr/0032-no-changelog.md +++ /dev/null @@ -1,23 +0,0 @@ -# 0032. No `CHANGELOG.md` - -- Status: accepted -- Date: 2026-10-14 - -## Context - -A changelog file repeats what the pull requests already describe, and drifts from the releases -when it is forgotten. - -## Decision - -The GitHub release notes, built from the pull requests, are the changelog. `UPGRADE.md` only says -what a shop has to do when it upgrades. - -## Consequences - -- A pull request describes its change for the release notes. -- A pull request that breaks the public contract adds its entry to `UPGRADE.md`. - -## Related - -- [0031. A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) diff --git a/docs/adr/0032-upstream-workarounds-never-constrain-shops.md b/docs/adr/0032-upstream-workarounds-never-constrain-shops.md new file mode 100644 index 0000000..e44ca8e --- /dev/null +++ b/docs/adr/0032-upstream-workarounds-never-constrain-shops.md @@ -0,0 +1,21 @@ +# 0032. A workaround for an upstream bug never constrains the shops + +- Status: accepted +- Date: 2026-10-14 + +## Context + +A bug in Sylius or in a dependency can break the plugin's tests or its test application. Capping the +dependency in the plugin's requirements would impose the cap on every shop, including the ones the +bug does not affect or that fixed it their own way. + +## Decision + +A workaround for an upstream bug stays in the development requirements or in the CI jobs. The +plugin's own constraints stay open. + +## Consequences + +- The plugin never blocks a shop from upgrading a dependency. +- A shop affected by the upstream bug applies the workaround itself. +- A workaround is removed once the upstream fix is released. diff --git a/docs/adr/0033-upstream-workarounds-never-constrain-shops.md b/docs/adr/0033-upstream-workarounds-never-constrain-shops.md deleted file mode 100644 index 4aba577..0000000 --- a/docs/adr/0033-upstream-workarounds-never-constrain-shops.md +++ /dev/null @@ -1,26 +0,0 @@ -# 0033. A workaround for an upstream bug never constrains the shops - -- Status: accepted -- Date: 2026-10-14 - -## Context - -A bug in Sylius or in a dependency can break the plugin's tests or its test application. Capping -the dependency in the plugin's requirements would impose the cap on every shop, including the ones -the bug does not affect or that fixed it their own way. - -## Decision - -A workaround for an upstream bug stays in the development requirements or in the CI jobs. The -plugin's own constraints stay open. Each workaround is noted with the upstream issue that will lift -it. - -## Consequences - -- The plugin never blocks a shop from upgrading a dependency. -- A shop affected by the upstream bug applies the workaround itself. -- A workaround is removed once the upstream fix is released. - -## Related - -- [0034. Static analysis and coverage run on the primary CI job only](0034-static-analysis-on-the-primary-ci-job.md) diff --git a/docs/adr/0034-static-analysis-on-the-primary-ci-job.md b/docs/adr/0034-static-analysis-on-the-primary-ci-job.md deleted file mode 100644 index 7835d12..0000000 --- a/docs/adr/0034-static-analysis-on-the-primary-ci-job.md +++ /dev/null @@ -1,24 +0,0 @@ -# 0034. Static analysis and coverage run on the primary CI job only - -- Status: accepted -- Date: 2026-10-14 - -## Context - -The CI matrix runs every supported combination of Sylius, Symfony, PHP and database. Static -analysis, the coding standard and the template lint depend neither on the database nor on the PHP -version under test; coverage is only meaningful once. - -## Decision - -They run on the primary job only. The other jobs run the tests. - -## Consequences - -- A faster matrix, with one place to read the analysis and the coverage. -- A problem that only static analysis on another version would show is not caught. - -## Related - -- [0033. A workaround for an upstream bug never constrains the shops](0033-upstream-workarounds-never-constrain-shops.md) -- [0035. The contributor stack ships a Chrome service](0035-contributor-stack-ships-chrome.md) diff --git a/docs/adr/0035-contributor-stack-ships-chrome.md b/docs/adr/0035-contributor-stack-ships-chrome.md deleted file mode 100644 index d597bcc..0000000 --- a/docs/adr/0035-contributor-stack-ships-chrome.md +++ /dev/null @@ -1,24 +0,0 @@ -# 0035. The contributor stack ships a Chrome service - -- Status: accepted -- Date: 2026-10-14 - -## Context - -The Behat Chrome driver asks Chrome for its version as soon as it is built, so Behat does not -start without a Chrome, even for the scenarios that do not use a browser. A contributor stack that -cannot run Behat does not help. - -## Decision - -The versioned Docker stack contributors use has a headless Chrome service, sharing the network of -the PHP service, so Behat reaches it the way its configuration expects. - -## Consequences - -- The whole Behat suite runs in the contributor stack. -- The stack differs from the other plugins' on this point. - -## Related - -- [0034. Static analysis and coverage run on the primary CI job only](0034-static-analysis-on-the-primary-ci-job.md) diff --git a/docs/adr/README.md b/docs/adr/README.md index c23f432..9e8a63f 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -22,7 +22,7 @@ Consequences, Related) and a line in the table below. | 0005 | [The plugin takes over the Sylius search form so it keeps the advanced filters](0005-search-form-taken-over-to-keep-the-advanced-filters.md) | | 0006 | [The taxon listing is the Sylius grid](0006-taxon-listing-is-the-sylius-grid.md) | | 0007 | [Facet counts start from the query the grid lists](0007-facet-counts-start-from-the-grid-query.md) | -| 0008 | [Advanced filters are counted subqueries, with caps on codes and values](0008-advanced-filters-are-counted-subqueries-with-caps.md) | +| 0008 | [Attribute and option filters are counted subqueries, with caps](0008-advanced-filters-are-counted-subqueries-with-caps.md) | | 0009 | [No `LIKE` on JSON columns](0009-no-like-on-json-columns.md) | | 0010 | [Featured items are filtered for visibility at render time](0010-featured-items-filtered-at-render-time.md) | | 0011 | [Conditional taxons are materialized as product-taxon links](0011-conditional-taxons-materialized-as-product-taxon-links.md) | @@ -46,7 +46,4 @@ Consequences, Related) and a line in the table below. | 0029 | [Services holding per-request memory are reset](0029-per-request-memory-services-are-reset.md) | | 0030 | [A `Bundle` class with `SyliusPluginTrait`, not `AbstractBundle`](0030-bundle-class-with-sylius-plugin-trait.md) | | 0031 | [A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) | -| 0032 | [No `CHANGELOG.md`](0032-no-changelog.md) | -| 0033 | [A workaround for an upstream bug never constrains the shops](0033-upstream-workarounds-never-constrain-shops.md) | -| 0034 | [Static analysis and coverage run on the primary CI job only](0034-static-analysis-on-the-primary-ci-job.md) | -| 0035 | [The contributor stack ships a Chrome service](0035-contributor-stack-ships-chrome.md) | +| 0032 | [A workaround for an upstream bug never constrains the shops](0032-upstream-workarounds-never-constrain-shops.md) | diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 265a0d2..9f187ff 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -153,7 +153,7 @@ featured children and images in grouped queries. | [No `LIKE` on JSON columns](../adr/0009-no-like-on-json-columns.md) | Select attribute values are read once per attribute code and matched in PHP, then filtered by id (`IN` with inlined integers) | | [Twig hooks are imported, not prepended](../adr/0004-twig-hooks-imported-not-prepended.md) | `config/config.yaml` is imported by the application after the Sylius configuration, so the plugin hooks win over the Sylius core hooks they override | | [Sanitize on write](../adr/0020-values-sanitized-on-write.md) | `setColor()`, `setIcon()`, `setUrl()` go through `AdvancedTaxonValueSanitizer`; the media display mode setters fall back to their default on an unknown value; `advanced_taxon_media_href()` sanitizes media links again on read | -| [Show only what is visible](../adr/0010-featured-items-filtered-at-render-time.md) | Featured products and children are filtered at render time (`advanced_taxon_visible_products()`, `advanced_taxon_visible_taxons()`): disabled products, products outside the current channel and disabled taxons are left out | +| [Show only what is visible](../adr/0010-featured-items-filtered-at-render-time.md) | Featured products and children are filtered at render time (`advanced_taxon_visible_products()`, `advanced_taxon_visible_taxons()`): disabled products, products outside the current channel, disabled taxons and featured children moved under another parent (template filter) are left out | | [Stateless services](../adr/0029-per-request-memory-services-are-reset.md) | Services are `final` and mostly `readonly`; the six holding per-request memory (`SelectAttributeValueResolver`, `FacetReferenceChecker`, `FacetRuntime`, `VisibilityRuntime`, `ConditionalTaxonSyncListener`, `TaxonIconRemovalListener`) implement `ResetInterface`, so they are reset between requests (FrankenPHP worker mode, Messenger workers) | Why each of them holds, and what was left as it is on purpose: the diff --git a/docs/development/code-map.md b/docs/development/code-map.md index eec5aa6..e40bc84 100644 --- a/docs/development/code-map.md +++ b/docs/development/code-map.md @@ -70,8 +70,8 @@ middleware of `sylius.command_bus` does. A full synchronization flushes and clea ## Wiring -- Services are defined explicitly, without autowiring nor autoconfiguration, as Symfony recommends - for reusable bundles: `config/services.php` imports one file per area from `config/services/` +- Services are defined explicitly, without autowiring nor autoconfiguration + ([why](../adr/0028-explicit-services-no-autowiring.md)): `config/services.php` imports one file per area from `config/services/` (`facet`, `grid`, `conditional_taxon`, `media`, `form`, `twig`, `controller`). Ids are prefixed with `cyllene_digital_sylius_advanced_taxon.` and private, except the controller, whose id is its class (the route refers to it); tags (`twig.extension`, `form.type_extension`, @@ -84,7 +84,7 @@ middleware of `sylius.command_bus` does. A full synchronization flushes and clea metadata: `#[AsCommand]`, `#[AsEntityAutocompleteField]` (alias and route), `#[IsGranted]`. - `src/DependencyInjection/CylleneDigitalSyliusAdvancedTaxonExtension.php` prepends the Doctrine mapping, the migrations, the Liip Imagine filter sets and the `at` UX Icons set (`assets/icons/`). The Twig hooks are imported through - `config/config.yaml`, not prepended: they override Sylius core hooks. + `config/config.yaml`, not prepended ([why](../adr/0004-twig-hooks-imported-not-prepended.md)). ## Front-end assets @@ -95,10 +95,11 @@ middleware of `sylius.command_bus` does. A full synchronization flushes and clea Conventions: -- No inline script in templates: a behaviour is a Stimulus controller, registered in its entrypoint - with the `at-` prefix. +- No inline script in templates ([why](../adr/0024-no-script-in-templates.md)): a behaviour lives in + the entrypoints, as a Stimulus controller registered with the `at-` prefix. - Texts shown by a controller come from the translations, passed as a Stimulus value. - Static layout rules live in the SCSS files under `assets/*/scss/`. - A template icon is `tabler:` when Sylius ships it, otherwise a Tabler SVG added to `assets/icons/` - and used as `at:`; a test fails on any other prefix. + and used as `at:` ([why](../adr/0021-template-icons-are-local.md)); a test fails on an icon + written in a template that no local set ships. - A row added to a form collection takes its index from `assets/admin/js/collection-index.js`. diff --git a/docs/domain/conditional-taxons.md b/docs/domain/conditional-taxons.md index db2991b..e9fc664 100644 --- a/docs/domain/conditional-taxons.md +++ b/docs/domain/conditional-taxons.md @@ -3,7 +3,7 @@ A conditional taxon gets its products from rules instead of manual assignment. The plugin **materializes** the rules: matching products are attached to the taxon as regular Sylius `ProductTaxon` links, so the storefront, the API, the search and the product forms see a normal -taxon. Nothing is resolved at query time. Admin usage: [User guide](../admin/user-guide.md). +taxon. Nothing is resolved at query time ([why](../adr/0011-conditional-taxons-materialized-as-product-taxon-links.md)). Admin usage: [User guide](../admin/user-guide.md). ## Model @@ -53,7 +53,7 @@ the only values a condition reads. `contains` patterns are escaped: `%`, `_` and the escape character typed by the merchant are literal. Every comparison ignores the case, on every database: the SQL ones (name, description, attribute text, option value) compare `LOWER()` of both sides, the select attribute choices are compared in -PHP. +PHP ([why](../adr/0016-conditions-compare-case-insensitively.md)). ### Negative operators @@ -100,12 +100,11 @@ the taxon itself for `taxon_membership`, value present when required. `setRefere code trimmed, so that a stray space never makes the reference miss. If any stored condition is not applicable, or if there is none, the query gets `1 = 0` and the taxon matches **no** product. Incomplete data written without validation (API, fixtures, import, SQL), or a reference deleted or mistyped, never widens the -selection to the whole catalog: without this check, a negative operator on a missing reference -(`NOT EXISTS` always true) would match every product. +selection ([why](../adr/0015-non-applicable-condition-matches-nothing.md)). ## Synchronization -`ConditionalTaxonProductAssigner::syncTaxon(AdvancedTaxonInterface $taxon, ?string $localeCode = null)` +`ConditionalTaxonProductAssigner::syncTaxon(AdvancedTaxonInterface $taxon)` returns `{detached, attached, matched}` and does not flush. | Case | Result | @@ -121,13 +120,14 @@ Rules: - **Disabled products are kept.** `buildQuery()` does not filter on `enabled`: a product disabled for a while keeps its assignment and position. The storefront lists enabled products only. - **The synchronization owns the links.** A product assigned by hand to a conditional taxon is - detached at the next synchronization if it does not match. + detached at the next synchronization if it does not match + ([why](../adr/0012-conditional-taxon-owns-its-product-links.md)). - **Leaving conditional mode detaches everything**, including products that were assigned by hand before the taxon became conditional. -- Links are removed through the unit of work (`removeProductTaxon()` + `remove()`), so Doctrine - events fire (search indexers…); they are loaded by chunks of 1000. -- **Locale.** `name` and `description` are read in `$localeCode`, by default the Sylius default - locale (`sylius.translation_locale_provider`, `getDefaultLocaleCode()`). +- Links are removed through the unit of work (`removeProductTaxon()` + `remove()`), loaded by + chunks of 1000 ([why](../adr/0013-products-detached-through-the-unit-of-work.md)). +- **Locale.** `name` and `description` are read in the Sylius default locale + (`sylius.translation_locale_provider`, `getDefaultLocaleCode()`). - **Channel.** The match is not restricted to a channel. ### Triggers @@ -149,8 +149,8 @@ skipped. A deleted taxon is skipped (its links go with it, `ON DELETE CASCADE`). The `SynchronizeConditionalTaxon(taxonId)` messages go out once the flush is over: at the end of the request (`kernel.response`, before the response is sent), of the console command -(`console.terminate`) or of the message a worker handled (`WorkerMessageHandledEvent`). Doctrine -does not support a flush inside `postFlush`, and a synchronous handler flushes. A flag prevents +(`console.terminate`) or of the message a worker handled (`WorkerMessageHandledEvent`), never in +`postFlush` ([why](../adr/0014-synchronization-dispatched-through-messenger-after-the-flush.md)). A flag prevents re-entrance while dispatching. Code that flushes outside these three (a plain script) leaves the synchronization to the [command](#console-command). @@ -193,24 +193,25 @@ stock. The conditions tab posts the rows being edited to `AdminFacetPreviewController` ([route contract](../architecture/public-contract.md#admin-route)), which calls -`ConditionalTaxonQueryBuilder::previewConditions()`: +`ConditionalTaxonQueryBuilder::previewConditions()` +([why it mirrors the synchronization](../adr/0018-condition-preview-mirrors-the-synchronization.md)): - rows that are not applicable (incomplete, still being filled, or whose reference does not exist) are **skipped**; the others are previewed. With no applicable row, the answer is `count: 0`; - the products are the ones the synchronization assigns: disabled products included, `name` and `description` read in the Sylius default locale (`sylius.translation_locale_provider`), the translation being optional; -- the answer holds the total count and up to 20 products (the query has no `ORDER BY`); at most 32 +- the answer holds the total count and up to 20 products, by id; at most 32 conditions are read. ## Known limitations | Limitation | Effect | |------------|--------| -| Option conditions compare the **translated** option value, not the option value code | Renaming an option value in every locale breaks the condition: positive conditions stop matching, `not_in` stops excluding | +| Option conditions compare the **translated** option value, not the option value code ([why](../adr/0017-option-conditions-match-the-translated-value.md)) | Renaming an option value in every locale breaks the condition: positive conditions stop matching, `not_in` stops excluding | | Deleting a referenced attribute, option or taxon | The condition is no longer applicable: the taxon matches no product, and the next synchronization (command or condition change) detaches all its products | | `name` / `description` use the Sylius default locale | A product without a translation in that locale never matches `equals` / `contains` on these types | | `taxon_membership` on another conditional taxon | Reads that taxon's current links: the command processes taxons by id, so a dependency synchronized later is only seen at the next run | | `syncAllConditionalTaxons()` clears the entity manager | Meant for the console; calling it in a request detaches every loaded entity | | No trigger on catalog changes | Products created or changed after the last save are only picked up by the command | -| A condition on a select attribute reads every value of that attribute | The choices are matched in PHP (no `LIKE` on a JSON column), then the matching products are passed to the query by id: on a very large catalog, a frequent value makes a long `IN` list | +| A condition on a select attribute reads every value of that attribute | The choices are matched in PHP ([no `LIKE` on a JSON column](../adr/0009-no-like-on-json-columns.md)), then the matching products are passed to the query by id: on a very large catalog, a frequent value makes a long `IN` list | diff --git a/docs/persistence/migrations.md b/docs/persistence/migrations.md index 6a5c1b0..c4923f7 100644 --- a/docs/persistence/migrations.md +++ b/docs/persistence/migrations.md @@ -116,8 +116,8 @@ DBAL reports every nullable column again and cannot introspect the default Maria ## Changing the schema 1. Change the mapping in the trait or entity. -2. Change `addedColumns()` or `createPluginTables()` in the migration (before `v1.0.0`), or add a - migration using the same DBAL `Schema` approach (after). +2. Add a new migration in `src/Migrations/`, written against the DBAL `Schema` API: the released one + is never edited ([why](../adr/0031-schema-change-is-a-new-migration.md)). 3. Run the drift check above on MySQL and PostgreSQL. 4. Write the change in [UPGRADE.md](../../UPGRADE.md) when it affects existing data or the [public contract](../architecture/public-contract.md#database). diff --git a/docs/shop/storefront.md b/docs/shop/storefront.md index 67af332..4e57f0c 100644 --- a/docs/shop/storefront.md +++ b/docs/shop/storefront.md @@ -119,7 +119,8 @@ With a single product on the page, random media go after it. No card is added to | **Spotlight zone** | mega menu hover image, child taxon cards | one media | | **Universe slider** | [hero slider](#hero-slider) of a universe page | position | -**Stacked** lists the media by position; **Random** shuffles them on every view. Top, left and bottom +**Stacked** lists the media by position; **Random** shuffles them on every view (behind an HTTP cache, +once per cache entry: [why](../adr/0023-random-media-display-modes-kept.md)). Top, left and bottom media are full-width images, lazy loaded, with the media title as `title` and the description as `alt` (the taxon name when empty). They are not links. **Slider** (top and bottom zones, from two media) shows one media at a time in position order, with the markup of the [hero slider](#hero-slider) diff --git a/docs/shop/twig-functions.md b/docs/shop/twig-functions.md index 6026eaf..402d421 100644 --- a/docs/shop/twig-functions.md +++ b/docs/shop/twig-functions.md @@ -313,7 +313,9 @@ mode **every call picks again**: call it once per render and keep the result. `n ## Visibility (`VisibilityRuntime`) Featured products and featured children are stored as picked in the back office. These functions -leave out what must not reach the storefront any more. No memoization. +leave out what must not reach the storefront any more +([why at render time](../adr/0010-featured-items-filtered-at-render-time.md)). Channel membership is +cached for the request (reset between requests). ### `advanced_taxon_visible_products` diff --git a/src/Entity/AdvancedTaxonValueSanitizer.php b/src/Entity/AdvancedTaxonValueSanitizer.php index bfb51f0..09dd41d 100644 --- a/src/Entity/AdvancedTaxonValueSanitizer.php +++ b/src/Entity/AdvancedTaxonValueSanitizer.php @@ -5,7 +5,7 @@ namespace CylleneDigital\SyliusAdvancedTaxonPlugin\Entity; /** - * Sanitizes the free-form values rendered on the storefront (icon references and media links). + * Sanitizes the free-form values rendered on the storefront (colors, icon references and media links). */ final class AdvancedTaxonValueSanitizer { diff --git a/src/Form/Extension/TaxonMediaZonesTypeExtension.php b/src/Form/Extension/TaxonMediaZonesTypeExtension.php index 412f8d4..9affe59 100644 --- a/src/Form/Extension/TaxonMediaZonesTypeExtension.php +++ b/src/Form/Extension/TaxonMediaZonesTypeExtension.php @@ -55,7 +55,8 @@ final class TaxonMediaZonesTypeExtension extends AbstractTypeExtension * links: the other zones ignore a destination URL. * * The optional `display_mode_*` keys declare the display mode field of the zone: - * `display_mode_choices` holds its options and `display_mode_label` its label. A zone without + * `display_mode_choices` holds its options, `display_mode_label` its label and `display_mode_help` + * its help text. A zone without * them (the main image, the universe slider) only exposes its media collection. * * @var array '__media_' . $zone['type'] . '__', 'button_add_label' => 'cyllene_digital_sylius_advanced_taxon.form.media.add', 'button_delete_label' => 'cyllene_digital_sylius_advanced_taxon.form.media.remove', - // New media are appended to the zone, so their position is pre-filled with the - // highest position already used by the zone media, incremented by one. 'prototype_data' => $this->createPrefilledMedia($taxon, $zone['type']), 'entry_options' => [ 'label' => false, diff --git a/src/Form/Extension/TaxonTypeExtension.php b/src/Form/Extension/TaxonTypeExtension.php index de928ac..8d00926 100644 --- a/src/Form/Extension/TaxonTypeExtension.php +++ b/src/Form/Extension/TaxonTypeExtension.php @@ -165,9 +165,8 @@ public function buildForm(FormBuilderInterface $builder, array $options): void } } - // A taxon is treated as conditional when at least one facet condition is configured. - // Its matching products are materialized by ConditionalTaxonSyncListener, once the - // taxon has effectively been flushed. + // Its products are synchronized by a message dispatched once the flush is over + // (ConditionalTaxonSyncListener). $taxon->setConditional(!$taxon->getFacetConditions()->isEmpty()); }); } diff --git a/src/Service/ConditionalTaxonQueryBuilder.php b/src/Service/ConditionalTaxonQueryBuilder.php index 819c6b9..32182d5 100644 --- a/src/Service/ConditionalTaxonQueryBuilder.php +++ b/src/Service/ConditionalTaxonQueryBuilder.php @@ -19,8 +19,8 @@ * compiled to `NOT EXISTS` subqueries: a product must own no value matching the excluded one. A plain * `!=` on a joined alias would be wrong as soon as a product owns several values. * - * A condition that cannot be applied (operator of another type, missing reference or value) makes - * the whole query match nothing. + * In the query of a taxon, a condition that cannot be applied (operator of another type, missing + * reference or value) makes it match nothing; the preview skips it, as a row still being filled in. */ final readonly class ConditionalTaxonQueryBuilder { diff --git a/src/Service/FacetProductQueryBuilder.php b/src/Service/FacetProductQueryBuilder.php index 92da994..7c3f93f 100644 --- a/src/Service/FacetProductQueryBuilder.php +++ b/src/Service/FacetProductQueryBuilder.php @@ -107,8 +107,8 @@ public function applyStorefrontFilters(QueryBuilder $qb, array $filters, string */ private function normalizeAdvancedFilters(array $filters): array { - // Every selected code adds joins and facet queries: an unbounded query string would let anyone - // build a query MySQL refuses (61 tables at most) or the database chokes on. + // Every selected code adds a subquery to the listing and to every facet query, and a facet + // query of its own: an unbounded query string would let anyone send heavy queries. $codesLeft = self::MAX_FILTER_CODES; $normalizedAttributes = self::normalizeValuesByCode($filters['attributes'] ?? [], $codesLeft); $normalizedOptions = self::normalizeValuesByCode($filters['options'] ?? [], $codesLeft); @@ -595,7 +595,7 @@ private function getPriceFacet(AdvancedTaxonInterface $taxon, string $localeCode } /** - * Unavailable in CLI contexts (for example while synchronizing conditional taxons). + * Unavailable outside a shop request (CLI). */ private function getCurrentChannel(): ?ChannelInterface { diff --git a/src/Service/SelectAttributeValueResolver.php b/src/Service/SelectAttributeValueResolver.php index 9396c18..ef1aed2 100644 --- a/src/Service/SelectAttributeValueResolver.php +++ b/src/Service/SelectAttributeValueResolver.php @@ -40,7 +40,7 @@ public function __construct( /** * Returns the identifiers of the select attribute values of an attribute whose choices match. * - * @param callable(list): bool $matches receives the keys and the labels of one choice + * @param callable(list): bool $matches receives the key and the labels of one choice * * @return list */ diff --git a/src/Twig/Runtime/FacetRuntime.php b/src/Twig/Runtime/FacetRuntime.php index bbfbf86..08bb61b 100644 --- a/src/Twig/Runtime/FacetRuntime.php +++ b/src/Twig/Runtime/FacetRuntime.php @@ -92,7 +92,7 @@ public function getAvailableFilters( } /** - * Returns the references a facet condition can target (code => label), by condition type, for + * Returns the references a facet condition can target (label => code), by condition type, for * the facet condition JS. Labels are read in the current locale with scalar queries: the admin * form never hydrates the whole catalog structure. * diff --git a/tests/Integration/FacetProductQueryBuilderTest.php b/tests/Integration/FacetProductQueryBuilderTest.php index 48f15a5..d64e2c7 100644 --- a/tests/Integration/FacetProductQueryBuilderTest.php +++ b/tests/Integration/FacetProductQueryBuilderTest.php @@ -423,7 +423,8 @@ public function test_a_condition_on_an_attribute_that_is_neither_text_nor_select $light = $this->catalog->product('Light', [$channel]); $this->catalog->attributeValue($light, $weight, 1); - // Conditions only read text values: on an integer attribute, a negation would match all. + // Conditions only read text and select values: on an integer attribute, a negation would + // otherwise match every product. self::assertSame([], $this->matchingNames([$this->condition(FacetCondition::TYPE_ATTRIBUTE, 'not_equals', (string) $weight->getCode(), '5')], [$heavy, $light])); } From 39eeb65c485a6a87282b0bcca31de4d1c4683527 Mon Sep 17 00:00:00 2001 From: Julien Dehais Date: Wed, 14 Oct 2026 15:04:46 +0200 Subject: [PATCH 3/4] docs: keep the decision records on the plugin itself The upstream workaround record now states the plugin's choice (its requirements stay open) without the project's tests and CI; the icon record no longer describes the unit test that enforces it, and the bundle record no longer compares with other plugins. --- docs/adr/0021-template-icons-are-local.md | 5 ++--- .../0030-bundle-class-with-sylius-plugin-trait.md | 4 ++-- ...-upstream-workarounds-never-constrain-shops.md | 15 +++++++-------- docs/adr/README.md | 2 +- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/docs/adr/0021-template-icons-are-local.md b/docs/adr/0021-template-icons-are-local.md index d78e9dd..e0b51ba 100644 --- a/docs/adr/0021-template-icons-are-local.md +++ b/docs/adr/0021-template-icons-are-local.md @@ -12,9 +12,8 @@ template icon outside the few Tabler icons Sylius ships would then depend on the ## Decision Templates only use Tabler icons: the ones Sylius ships, or a Tabler SVG added to the plugin's own -set under a prefix of its own. A unit test fails on an icon written in a template that no local set -ships. The plugin set does not reuse a library prefix: a local set named after a library would -silently hide that library for the whole shop. +set under a prefix of its own. The plugin set does not reuse a library prefix: a local set named +after a library would silently hide that library for the whole shop. ## Consequences diff --git a/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md b/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md index 2aebc35..8d288d2 100644 --- a/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md +++ b/docs/adr/0030-bundle-class-with-sylius-plugin-trait.md @@ -10,8 +10,8 @@ plugin uses `SyliusPluginTrait`, which redefines `getContainerExtension()`. ## Decision -The bundle is a plain `Bundle` with `SyliusPluginTrait` and a classic `Extension` class, like the -other Sylius plugins. +The bundle is a plain `Bundle` with `SyliusPluginTrait` and a classic `Extension` class, the +structure the trait expects. ## Consequences diff --git a/docs/adr/0032-upstream-workarounds-never-constrain-shops.md b/docs/adr/0032-upstream-workarounds-never-constrain-shops.md index e44ca8e..326789a 100644 --- a/docs/adr/0032-upstream-workarounds-never-constrain-shops.md +++ b/docs/adr/0032-upstream-workarounds-never-constrain-shops.md @@ -1,21 +1,20 @@ -# 0032. A workaround for an upstream bug never constrains the shops +# 0032. The plugin does not cap a dependency for an upstream bug - Status: accepted - Date: 2026-10-14 ## Context -A bug in Sylius or in a dependency can break the plugin's tests or its test application. Capping the -dependency in the plugin's requirements would impose the cap on every shop, including the ones the -bug does not affect or that fixed it their own way. +A bug in Sylius or in a dependency can affect some of their versions. Capping the dependency in the +plugin's requirements would impose the cap on every shop, including the ones the bug does not affect +or that work around it their own way. ## Decision -A workaround for an upstream bug stays in the development requirements or in the CI jobs. The -plugin's own constraints stay open. +The plugin's requirements stay open to every version it supports: it does not cap a dependency for +a bug that is not its own. ## Consequences - The plugin never blocks a shop from upgrading a dependency. -- A shop affected by the upstream bug applies the workaround itself. -- A workaround is removed once the upstream fix is released. +- A shop affected by the upstream bug applies the workaround itself, in its own requirements. diff --git a/docs/adr/README.md b/docs/adr/README.md index 9e8a63f..ed69b5f 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -46,4 +46,4 @@ Consequences, Related) and a line in the table below. | 0029 | [Services holding per-request memory are reset](0029-per-request-memory-services-are-reset.md) | | 0030 | [A `Bundle` class with `SyliusPluginTrait`, not `AbstractBundle`](0030-bundle-class-with-sylius-plugin-trait.md) | | 0031 | [A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) | -| 0032 | [A workaround for an upstream bug never constrains the shops](0032-upstream-workarounds-never-constrain-shops.md) | +| 0032 | [The plugin does not cap a dependency for an upstream bug](0032-upstream-workarounds-never-constrain-shops.md) | From c298c7b17249e720f87644266390f0e70da307e5 Mon Sep 17 00:00:00 2001 From: Julien Dehais Date: Wed, 14 Oct 2026 15:04:51 +0200 Subject: [PATCH 4/4] docs: name the dependency cap record after its decision --- CLAUDE.md | 4 ++-- ...shops.md => 0032-no-dependency-cap-for-an-upstream-bug.md} | 0 docs/adr/README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename docs/adr/{0032-upstream-workarounds-never-constrain-shops.md => 0032-no-dependency-cap-for-an-upstream-bug.md} (100%) diff --git a/CLAUDE.md b/CLAUDE.md index 2d26178..58279d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -113,7 +113,7 @@ The QA commands are in [`CONTRIBUTING.md`](CONTRIBUTING.md). What it does not sa migration. - **Sylius 2.1 mappings fail `doctrine/orm` 3.7 validation** (`ShipmentUnit#shipment`): CI caps the ORM below 3.7 before Sylius 2.3, the plugin's own constraint stays open - ([0032](docs/adr/0032-upstream-workarounds-never-constrain-shops.md)). + ([0032](docs/adr/0032-no-dependency-cap-for-an-upstream-bug.md)). - **Behat**: the Sylius suites are not imported (2.1 / 2.2 ship them in YAML, 2.3 in PHP); the plugin suites only use the Sylius contexts, which are services. The plugin contexts use PHP attributes (`#[Given]`, `#[AfterScenario]`): Behat 4, the one Symfony 8 needs, no longer reads annotations; the CI keeps Behat 3 before Sylius 2.3, @@ -133,7 +133,7 @@ The QA commands are in [`CONTRIBUTING.md`](CONTRIBUTING.md). What it does not sa to `sylius_admin:delete_modal` (empty delete buttons in every admin grid). Drop the constraint once Sylius passes defaults itself ([Sylius#19278](https://github.com/Sylius/Sylius/issues/19278)). An upstream workaround never goes into the plugin's own constraints - ([0032](docs/adr/0032-upstream-workarounds-never-constrain-shops.md)). + ([0032](docs/adr/0032-no-dependency-cap-for-an-upstream-bug.md)). - `sylius_test_html_attribute()` attributes (`data-test-*`) are what the Behat pages target: keep them when reworking a shop template. diff --git a/docs/adr/0032-upstream-workarounds-never-constrain-shops.md b/docs/adr/0032-no-dependency-cap-for-an-upstream-bug.md similarity index 100% rename from docs/adr/0032-upstream-workarounds-never-constrain-shops.md rename to docs/adr/0032-no-dependency-cap-for-an-upstream-bug.md diff --git a/docs/adr/README.md b/docs/adr/README.md index ed69b5f..9229f32 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -46,4 +46,4 @@ Consequences, Related) and a line in the table below. | 0029 | [Services holding per-request memory are reset](0029-per-request-memory-services-are-reset.md) | | 0030 | [A `Bundle` class with `SyliusPluginTrait`, not `AbstractBundle`](0030-bundle-class-with-sylius-plugin-trait.md) | | 0031 | [A schema change is a new migration, written against the DBAL `Schema` API](0031-schema-change-is-a-new-migration.md) | -| 0032 | [The plugin does not cap a dependency for an upstream bug](0032-upstream-workarounds-never-constrain-shops.md) | +| 0032 | [The plugin does not cap a dependency for an upstream bug](0032-no-dependency-cap-for-an-upstream-bug.md) |