feat(core): content references database schema#1367
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Guard every CREATE TABLE/INDEX in 043 with .ifNotExists() so a crash mid-migration (or the runner's race-recovery re-run) re-applies cleanly, and add 043 to the trailing-migration re-run test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the .ifNotExists() hardening on up() so dropTable is safe to re-run after a partial rollback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: dbbf5dc The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
@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-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@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: |
There was a problem hiding this comment.
Approach assessment
This is well-scoped, schema-only groundwork for content references. The design sensibly mirrors the existing taxonomy/byline row-per-locale + translation-group model (_emdash_taxonomy_defs / content_taxonomies). Locale-agnostic edges linked by translation_group are the right fit for EmDash's content-localization architecture, and keeping foreign keys out of the edge table is consistent with how content_taxonomies already works.
The migration is additive, forward-only, and idempotent (.ifNotExists() / .ifExists()), which is exactly what the runner's partial-run recovery path needs. The down() ordering drops the edge table before the definition table, which is correct.
What I checked
- Migration correctness (idempotency, SQLite/Postgres compatibility, column types, defaults, unique constraints, index shapes)
- Type definitions in
types.tsagainst the migration schema - Runner registration and
MIGRATION_COUNT - Test coverage (table existence, defaults, unique constraints, cross-locale duplicates, forward/backlink traversal, self-references, index existence, down/up round-trip)
- Cross-cutting concerns: no orphaned-table impact, no handler/repository side effects (intentionally out of scope), no SQL interpolation issues
Headline conclusion
The implementation is clean and the tests are thorough. There are no logic bugs or regressions. I have two non-blocking suggestions:
- Index naming: align with the literal
idx_{table}_{column}convention used in recent migrations (double-underscore for_emdash_*tables). - Changeset level: consider
minorinstead ofpatchfor new feature groundwork. - Optional: consider a partial unique index on
_emdash_relations(translation_group, locale)to enforce the row-per-locale invariant at the DB level, matching what migration 040 does for bylines.
Good to merge once the maintainer weighs the naming suggestion.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "emdash": patch | |||
| --- | |||
There was a problem hiding this comment.
[suggestion] This is net-new feature groundwork. While the package is pre-1.0, a minor bump is more idiomatic in Changesets for additive capabilities than patch.
|
Nice work @MA2153 I pulled this down locally and tested it against nearby open work. Overall, this looks like solid schema groundwork. The main thing I’d recommend before merge is the relation group invariant. UNIQUE (translation_group, locale) WHERE translation_group IS NOT NULL |
…names Address PR emdash-cms#1367 review: - Make _emdash_relations.translation_group NOT NULL. Edges reference relations by translation_group (relation_group is NOT NULL), so a null group is an unreferenceable dead row. New table, no backfill window. - Add a unique index on (translation_group, locale) — one relation variant per locale. Plain (not partial like bylines' 040) since the column is now NOT NULL. - Align index names with the idx_{full_table}_{column} convention used by 036/040/042 (idx__emdash_*). - Bump changeset to minor (additive feature groundwork). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for pulling it down @masonjames. On the relation-group invariant: I took the stronger of your two options — |
What does this PR do?
Adds the content-references database schema — the narrow first step toward reference fields. Schema only: no field type, API, handlers, repositories, or admin UI yet (deliberately out of scope; those come in follow-ups).
Two new system tables via forward-only migration
043_content_references.ts:_emdash_relations— relationship-type definitions. Row-per-locale, mirroring_emdash_taxonomy_defs: declares which collection is the parent and which is the child (the multipliable side), plus localizedparent_label/child_labelfor each role.UNIQUE(name, locale)._emdash_content_references— directedparent → childedges between content entries. Both endpoints and the relation are referenced bytranslation_group, so edges are locale-agnostic. Mirrorscontent_taxonomies: group-linking means no foreign keys (atranslation_groupis unique nowhere), and dangling-edge cleanup is an application-layer obligation for the future write path.UNIQUE(relation_group, parent_group, child_group).Design notes: relations are directed; the child is the side that multiplies; many-to-many within a single relation is intentionally not modeled (swap parent/child to express the inverse). Same-collection and self-references are permitted. The migration is idempotent (
.ifNotExists()/.ifExists()) so it survives the runner's partial-run recovery path.Related Discussion: #386
Closes #
Type of change
Checklist
pnpm typecheckpasses —packages/coretypechecks clean. (A pre-existing, unrelated failure inpackages/plugin-clire:registry-lexiconsexports is present onmainand not touched here.)pnpm lintpasses — no diagnostics in changed files. (8 pre-existing diagnostics in unrelatedtemplates//demos/astro configs predate this PR.)pnpm testpasses (targeted) — 23/23 acrosscontent-references.test.ts(8) andmigrations.test.ts(15), SQLite. Postgres parity runs viadescribeEachDialectwhenPG_CONNECTION_STRINGis set.pnpm formathas been runAI-generated code disclosure
Screenshots / test output
New schema tests cover: table existence; insert + default backfill (
locale='en',sort_order=0); both unique constraints (edge triple + name/locale, incl. cross-locale allow); forward + backlink traversal with ordering; same-collection / self-references; index existence (SQLite); anddown()→up()rollback. Dialect-agnostic (SQLite + Postgres).🤖 Generated with Claude Code