Repository navigation
fix(core): share taxonomy hierarchy and collections across locales - #3360
Conversation
🦋 Changeset detectedLatest commit: 5eae228 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Query-count snapshot changes6 routes changed, total Δ -7 queries. SQLite
Comparing snapshot files between base and head. Updated automatically on each push. |
Scope checkThis PR changes 1,661 lines across 47 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Each locale's taxonomy definition stored its own `hierarchical` and `collections`, and nothing kept the copies in agreement. A translation could apply the taxonomy to different collections or flatten its term tree, and a locale without its own definition row got no terms at all. The structure now lives once per taxonomy name in `_emdash_taxonomy_def_groups` (migration 084); `label` and `labelSingular` stay per locale. Writes through any locale change the structure for every locale. A definition created for a name that already exists in another locale takes the taxonomy's structure and rejects differing values, so a create cannot silently reset it. A seed translation takes the structure from the entry it translates, wherever it appears in the file, and a seed that replaces an unedited built-in `category` or `tag` definition replaces its structure too. The migration merges drifted definitions without hiding anything: hierarchical if any locale was, collections as the union. Definitions of different names that shared a translation group, which older seed files and API versions could create, get one group per name. The per-locale columns keep a copy for code that reads them directly, such as the plugin sandbox bridges. Public reads load every locale's definitions once per isolate and resolve labels in memory, so warm renders no longer walk the locale chain per taxonomy.
bcd1bbf to
a7503f0
Compare
There was a problem hiding this comment.
I reviewed the shared taxonomy structure / cross-locale collections PR (#3360) by examining the migration, the taxonomy-def repository, the taxonomy API handlers, seed apply/validate, WXR import, plugin context, the Astro loader, and the plugin-test fixture.
Overall the implementation is coherent and the migration is idempotent: _emdash_taxonomy_def_groups owns hierarchical / collections, per-locale definition rows keep mirrored copies, saveTaxonomyStructure links every row of a name to the same group, and reads fall back through the locale chain. The create/update handlers correctly reject attempts to give one locale a different structure than the rest of the taxonomy, and seed/WXR import mirror the new ownership model.
I found one non-blocking edge case in seed validation: the omission allowance for hierarchical/collections only looks one hop through translationOf, so a chain of translations can all omit the structure and validation still passes even though no entry provides it. The apply engine would then fall back to defaults, which is benign but inconsistent with the intended rule. Recommend resolving the chain to the root source or requiring one entry in the chain to carry the values.
…ntry A taxonomy entry that omits `hierarchical` and `collections` took them from the entry its `translationOf` names, one step only. When a translation of a translation came first in the file, it wrote the defaults, flat and without collections, for the whole taxonomy, and the declaring entry applied later in skip mode kept them. Apply and validation now follow `translationOf` through entries of the same name to the last one. Validation rejects an omitted structure when that chain loops, and checks term parents against the hierarchy the apply step will use.
The helper's comment named only the case where a translationOf chain leaves the taxonomy's name; most chains end at an entry without translationOf.
A translation listed before its source entry applied its terms with the taxonomy's stored structure. When the source entry then replaced that structure, as it does for an unedited built-in category or tag and under onConflict "update", the translation's term tree stayed flat, even when the translation declared the hierarchy itself. Applying a seed now takes the entries that declare their taxonomy's structure first. Validation warns when a translation declares values other than the ones it takes, or when two entries that declare one taxonomy's structure disagree, since only one of them applies.
Every structure write rewrote both hierarchical and collections from a read taken before the write. Two concurrent updates that each sent one field could lose one of them, and a create that joined an existing taxonomy could revert an update made in between. saveTaxonomyStructure takes the fields to overwrite: an update passes only the fields it received, and a create never overwrites an existing group. Definition rows copy the group's stored values instead of the caller's.
…hangeset A locale without its own definition now takes one by fallback, so the guide no longer says every locale has one. The changeset's upgrade note separates the required action from the explanation.
Main added 084_site_transfer. Two migrations with one number would sort the taxonomy group migration before one a site may already have run.
…group-owned # Conflicts: # packages/core/src/database/migrations/runner.ts
A site import writes taxonomy definition rows directly and clears the target's scaffold definitions, but not their structure groups. Reads prefer the group, so an imported category or tag showed the target scaffold's hierarchical and collections, and a taxonomy only the scaffold had left a group that a later create adopted. The import's rebuild stage gains a taxonomies step: each name's group is rebuilt from its imported definitions with the migration's rule (hierarchical if any is, every collection any lists), and groups without a definition are deleted. The definition rows keep the package's values, which the verify stage compares. The groups table is classified as derived for the transfer format.
…eturns Creating a definition in a new locale compared the request's collections with the stored list, while reads drop collections that no longer exist and the create rejects them. When the stored list named a missing collection, such as the built-in category's posts on a site without posts, every create that sent collections failed. The comparison now uses the collections a read returns.
… locale The built-in category and tag definitions carry locale en. A seed applied on a site whose default locale is another looked only at its own locale, found no definition there, and kept the built-in structure, so the seed's hierarchical and collections were dropped for every locale. A source entry now also rewrites the structure when every definition of its taxonomy is an untouched built-in. Which built-ins are untouched is read once before the entries apply, since a structure write changes the built-in rows of every locale.
Export linked a definition to the first one of its translation group. Rows of two taxonomy names can share a group, as a site import of an older package leaves them, and the second name's entry then pointed at the first name's and omitted its structure, which validation rejects. Every locale of a name is one taxonomy, so export links by name.
…cement Taxonomy entries of one name apply their structure's source first, so only terms need to follow the term they translate. A built-in category or tag replaced from another locale is described as well.
There was a problem hiding this comment.
This re-review confirms the PR's approach is sound and the prior finding is resolved. Moving taxonomy hierarchical and collections to _emdash_taxonomy_def_groups (one row per name), mirroring the values onto per-locale definition rows, and resolving them through selectTaxonomyDefs is a clean fit for EmDash's per-locale translation model and fixes the reported drift (#2962 / #2525).
What I checked:
- Prior finding: The seed validation/apply one-hop
translationOfissue is fixed.findTaxonomyStructureSourceinpackages/core/src/seed/validate.tsnow walks the full same-name chain to its root, rejects loops, and warns when entries disagree.apply.tsorders entries that declare their own structure first, so a translation's terms follow the structure the root entry writes regardless of file order. The new unit and integration tests cover the chain cases. - Migration 085: Idempotent, merges drifted rows defensively (hierarchical if any locale was, collections as the union), preserves old per-locale columns, and disentangles names that shared a translation group.
- Write paths: API create/update, MCP tools, seed apply, WXR import, plugin-test fixture, and transfer rebuild all route structure writes through
saveTaxonomyStructure, keeping every locale's row and the new group in sync. Cache invalidation (invalidateTaxonomyDefsCache) is called on every write path. - Read paths: Public
getTaxonomyDefs/getTaxonomyDef/getTaxonomyTermsnow resolve through the group table; the isolate + request cache keeps the logged-out hot path at one query. Query-count snapshots correctly drop on SQLite for category/post/tag pages and stay flat on D1, matching the PR claim. - Authorization and API envelope: Route wrappers still check
taxonomies:read/taxonomies:manage; handler errors remainApiResult-shaped. - Transfer format:
_emdash_taxonomy_def_groupsis correctly classified as derived/non-portable; the verify stage preserves the imported per-locale definition rows until the next structure write, as documented. - Changeset and docs: The changeset is detailed, migration guidance is present, and the updated taxonomies/seed/i18n/OpenAPI/MCP docs accurately describe the new behavior.
No blocking issues remain. The implementation is coherent and the regression tests are in place.
What does this PR do?
Stores a taxonomy's
hierarchicalandcollectionsonce per taxonomy instead of on each locale's definition. Each locale's definition held its own copy, so writes let translations drift apart and a locale without a definition of its own listed no terms.A new table keyed by taxonomy name (migration 085) holds both fields, so a definition created in a new locale joins the existing taxonomy, and one that sends
hierarchicalorcollectionsother than a read returns is rejected; labels stay per locale. Every write path changes the fields for all locales, including a seed that replaces an unedited built-incategoryortagdefinition (#3355) from any locale, and an update writes only the fields it sends. A seed translation may omit both fields and takes them from the entry its same-nametranslationOfchain ends at, which applies first wherever it sits in the file, so the translation's terms follow its structure; validation rejects a chain that loops and warns when entries disagree on the values. The migration merges drifted rows without hiding anything: hierarchical if any locale was, collections as the union. A site import rebuilds each taxonomy's fields from its imported definitions the same way, in a new step of its rebuild stage, and the transfer format classifies the new table as derived. The per-locale columns stay in sync for direct readers such as the sandbox bridges, except after importing a package whose locales disagreed: the import's verify stage compares definition rows with the package, so they keep the package's values until the next structure write. Public reads cache every locale's definitions in one query, so logged-out query counts drop on term and post pages on SQLite and stay level on D1.The changeset lists each behavior change, under
minorbecausemajorisn't accepted before 1.0. #1928 also adds migrations, and #3025 edits the seed-apply branch rewritten here.Part of #2525.
Closes #2962
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. (n/a: no admin UI change)collections/hierarchical— identity-level fields have no owner in the row-per-locale model #2525)AI-generated code disclosure
Screenshots / test output
Not applicable (no UI change). The new tests in
taxonomy-def-structure.test.tsfailed before the change, except the create that sends back a read's values, which keeps what main allowed; they and the migration tests pass on SQLite and PostgreSQL, andtests/workerd/migrations-d1.test.tspasses on D1. The site import cases intransfer/taxonomy-structure.test.tsfail without the rebuild step and pass on SQLite and PostgreSQL, and the transfer tests intests/workerdpass on D1. The built-in cases inseed-on-conflict.test.tsfail without the structure write, the one that lists a translation first also when entries apply in file order, and the chain cases in the validation tests fail when only onetranslationOfstep is followed; the ones that use a database run on SQLite only.