feat(core): configurable displayField and dateField per collection#1973
Conversation
🦋 Changeset detectedLatest commit: 55ccedc The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
PR template validation failedPlease fix the following issues by editing your PR description:
See CONTRIBUTING.md for the full contribution policy. |
github-actions
Bot
added
review/needs-review
Scope checkThis PR changes 645 lines across 23 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-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@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: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR implements configurable displayField and dateField collection options in a sound, additive way: new nullable columns on _emdash_collections, server-side validation of the configured slugs, a shared getEntryTitle helper used across the admin list/editor/picker, and a closed set of allowed orderBy values resolved server-side. The approach fits EmDash's architecture and the migration/seed paths look correct.
I checked the diff against AGENTS.md conventions and runtime semantics. The implementation is generally clean and well tested, but I found a few issues worth fixing before merge:
- Referential integrity on field deletion —
SchemaRegistry.deleteFielddrops theec_*column but never clears the collection'sdisplay_field/date_fieldwhen that slug is referenced. A deleted field used asdateFieldwill later crash the content list when the admin sorts by it ("no such column"). This is the most important blocker. - React
useMemodependency arrays — both the content list and the content picker filter bygetEntryTitle(item, displayField)but omitdisplayFieldfrom their memo dependencies, which can leave client-side search results stale. - API schema surface is incomplete —
updateCollectionBodyandcollectionSchemadon't includedisplayField/dateField, so the REST collection API cannot read or write them even though the registry supports it. Combined withContentTypeEditornot exposing the fields, the feature is currently seed-file-only. This matches the PR description, but wiring them through the schemas would make the backend/frontend work consistent. displayFieldtype is not validated — any existing field slug is accepted, including JSON/portableText/image/repeater fields, whose raw stored values would render poorly as entry titles.
The tests cover the happy paths and the server-side sort validation well. Fixing #1 and #2 would unblock merge from my perspective; #3 and #4 are follow-ups that can be addressed here or in a later pass.
Findings
-
[needs fixing]
packages/core/src/schema/registry.ts:777deleteFielddrops the column from theec_*table but never checks whether the slug is still referenced bydisplay_fieldordate_fieldon_emdash_collections. After deletion, sorting the content list by the configured column fails with "no such column". Clear the collection metadata when the referenced field is removed.async deleteField(collectionSlug: string, fieldSlug: string): Promise<void> { const field = await this.getField(collectionSlug, fieldSlug); if (!field) { throw new SchemaError( `Field "${fieldSlug}" not found in collection "${collectionSlug}"`, "FIELD_NOT_FOUND", ); } const collection = await this.getCollection(collectionSlug); if (collection && (collection.displayField === fieldSlug || collection.dateField === fieldSlug)) { await this.updateCollection(collectionSlug, { displayField: collection.displayField === fieldSlug ? null : undefined, dateField: collection.dateField === fieldSlug ? null : undefined, }); } let schemaMutated = false; try { await withTransaction(this.db, async (trx) => {
github-actions
Bot
added
query-count changed
github-actions
Bot
added
review/needs-rereview
…e-diplay-and-date-field-in-collections # Conflicts: # packages/core/src/database/migrations/runner.ts # packages/core/tests/integration/database/migrations.test.ts # scripts/query-counts.snapshot.d1.json # scripts/query-counts.snapshot.sqlite.json
github-actions
Bot
removed
needs-rebase
query-count changed
Note for maintainer: This feature has already been placed in the 1.0 milestone by a maintainer. |
There was a problem hiding this comment.
This is a re-review of PR #1973. Since the last pass, the author has fixed the three blocking/regression concerns from the prior review:
SchemaRegistry.deleteFieldnow clearsdisplay_field/date_fieldin the same transaction when the referenced field is removed, so the content list won't later crash sorting by a dropped column.- The React
useMemodependency arrays inContentListandContentPickerModalnow includedisplayField, so client-side title filtering stays reactive. SchemaRegistry.updateCollectionvalidates thatdisplayFieldis a text-like field (string,text,slug) anddateFieldis adatetimefield.
The tests added for custom-field sorting, search result titles, and collection metadata behavior cover the happy paths well.
One new issue remains: SchemaRegistry.updateField allows in-place field-type changes as long as the SQLite column type doesn't change (e.g., datetime → string, or text → url). Because all of those types map to TEXT, the change is accepted even when the field is configured as a displayField or dateField, breaking the type invariant this PR establishes. After the change, the admin may sort the "Date" column by arbitrary strings or use a URL as a title. This should be guarded the same way updateCollection guards the initial assignment.
Prior finding #3 — the REST/OpenAPI collection schemas and the admin Content Type editor don't expose displayField/dateField — is still present. It aligns with the seed-file-only scope stated in the PR description and was previously flagged as a follow-up, so I'm not blocking on it here.
Overall the approach is solid and additive; fixing the updateField invariant would make the validation complete.
Findings
-
[needs fixing]
packages/core/src/schema/registry.ts:657After this block accepts an in-place type change because the SQLite column type stayed the same, the collection's
displayField/dateFieldmetadata can become invalid. For example, adatetimefield used asdateFieldcan be changed tostring(bothTEXT), or atextfield used asdisplayFieldcan be changed tourl, leaving the metadata claiming a type thatvalidateDisplayDateFieldswould reject onupdateCollection.Re-validate against the collection's configured
displayField/dateFieldwheninput.typechanges, or block the change and require the user to clear the metadata first. Add a test that changes adateFieldfield to a non-datetime type and expects aSchemaError.let nextType = field.type; let nextColumnType = field.columnType; if (input.type !== undefined && input.type !== field.type) { const newColumnType = FIELD_TYPE_TO_COLUMN[input.type]; if (newColumnType !== field.columnType) { throw new SchemaError( `Cannot change field "${fieldSlug}" in collection "${collectionSlug}" from type ` + `"${field.type}" to "${input.type}": the underlying column type would change from ` + `${field.columnType} to ${newColumnType}, which requires a manual content migration. ` + `Drop and re-create the field, or migrate the column data, before changing its type.`, "FIELD_TYPE_COLUMN_CHANGE", ); } // A same-column-type change can still violate the displayField/dateField // invariants (#1133). Read the collection only when a type change is // actually requested. const collection = await this.getCollection(collectionSlug); if (collection?.displayField === fieldSlug && !DISPLAY_FIELD_TYPES.has(input.type)) { throw new SchemaError( `Cannot change field "${fieldSlug}" to type "${input.type}" because it is the collection's displayField. ` + `Clear displayField or change it to another field first.`, "INVALID_DISPLAY_FIELD", ); } if (collection?.dateField === fieldSlug && input.type !== "datetime") { throw new SchemaError( `Cannot change field "${fieldSlug}" to type "${input.type}" because it is the collection's dateField. ` + `Clear dateField or change it to another field first.`, "INVALID_DATE_FIELD", ); } nextType = input.type; nextColumnType = newColumnType; }
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
There was a problem hiding this comment.
The approach is solid: titleField/dateField are additive, nullable columns on _emdash_collections, validated in the registry, and wired through the admin list, editor header, reference picker, and search-result titles. The migration and changeset are present.
Since the last review, the three blockers are fixed:
SchemaRegistry.deleteFieldclearstitle_field/date_fieldin the same transaction.- The
ContentListandContentPickerModaluseMemodependency arrays includetitleField, so client-side title filtering stays reactive. SchemaRegistry.updateCollectionrejects non-texttitleFieldand non-datetimedateFieldvalues.SchemaRegistry.updateFieldnow also guards in-place, same-column-type changes that would violate thetitleField/dateFieldtype invariants, and tests cover both the dateField→string and titleField→url cases.
One real gap remains: the server-side content-list q filter still resolves its search columns against the hard-coded title/name fallback and ignores a configured titleField. Because the list page runs in server-search mode, searching a collection whose titleField is not literally title or name will not match the display title produced by getEntryTitle. canUseFtsForListFilter will also incorrectly fall back to a LIKE on slug only. A related follow-up is that getSuggestions still skips collections without a literal title slug and selects c.title, so custom-titleField collections are either excluded or show the wrong title in autocomplete.
I also noticed the two new integration test files (display-date-sort.test.ts and title-date-sort.test.ts) are identical; one of them should probably exercise sorting by a configured titleField instead of duplicating the dateField case.
No security, data-loss, or build-breaking issues found.
Findings
-
[needs fixing]
packages/core/src/api/handlers/content.ts:328resolveSearchColumnsdecides which columns the content-listqfilter matches, but it only checks for fields literally namedtitleorname. A configuredtitleField(e.g.full_name) is ignored, so the server-side search on the list page won't match the same display title thatgetEntryTitlerenders.canUseFtsForListFilterwill also see an uncovered search column and fall back to a slug-only LIKE.Include the collection's
title_fieldin the candidate set (and de-duplicate):const row = await db .selectFrom("_emdash_collections") .select(["id", "title_field"]) .where("slug", "=", collection) .executeTakeFirst(); if (!row) return columns; const fields = await db .selectFrom("_emdash_fields") .select("slug") .where("collection_id", "=", row.id) .execute(); const fieldSlugs = new Set(fields.map((f) => f.slug)); const candidates = new Set<string>(); if (row.title_field) candidates.add(row.title_field); for (const candidate of ["title", "name"]) { if (fieldSlugs.has(candidate)) candidates.add(candidate); } return ["slug", ...candidates];Add a test that sets a custom
titleFieldand searches the content list by that field's value. -
[suggestion]
packages/core/src/search/query.ts:439getSuggestionsskips collections that don't have a field literally namedtitle, and then selectsc.title. With a configuredtitleField, autocomplete either excludes the collection or displays the wrong column. Consider readingconfig.titleFieldand selecting that column (falling back to thetitlecolumn only when present), so suggestions stay consistent with search-result titles.
github-actions
Bot
added
review/awaiting-author
…e-diplay-and-date-field-in-collections # Conflicts: # packages/admin/src/components/ContentEditor.tsx # packages/core/src/database/migrations/runner.ts # packages/core/src/seed/apply.ts # packages/core/tests/integration/database/migrations.test.ts
github-actions
Bot
added
review/needs-rereview
CacheMeOwside
added
bot:review
There was a problem hiding this comment.
The approach is sound and the prior blockers are resolved. titleField/dateField are additive, nullable collection metadata, validated server-side, and wired through the manifest into the admin list, editor header, reference picker, and search/autocomplete titles. The server-side content-list q search now includes the configured titleField; getSuggestions now draws its title from titleField; and the sort path validates custom fields against the collection's configured set rather than reopening arbitrary-column sorting. Tests now cover custom-field search, custom-field sort (both titleField and dateField), and suggestion/search title behavior, and the duplicate integration-test file is gone.
The remaining issue is comment hygiene. This PR introduces issue/PR references (#1133) in many code comments across the admin and core packages. AGENTS.md explicitly forbids referencing issues, PRs, or review threads in comments — that context belongs in commit messages and PR descriptions, not in source files. These need to be removed everywhere before merge. I have not run tests or lint; this review is static.
github-actions
Bot
added
the
review/needs-rereview
CacheMeOwside
added
bot:review
There was a problem hiding this comment.
The approach is sound and the prior blockers are resolved. Adding nullable titleField/dateField metadata on _emdash_collections, validating them server-side, and wiring them through the manifest into the admin list, editor header, reference picker, and search/autocomplete titles is the right shape for #1133. Tests now cover custom-field search, custom-field sort, schema validation, field-deletion cleanup, and search/suggestion title behavior. Query-count snapshots only changed the shape of an existing query, not the count.
The remaining issue is comment hygiene. This PR reintroduces an issue reference (#1178) in a newly-added comment in packages/core/src/search/query.ts, which violates the same AGENTS.md comment-discipline rule that the previous round asked to clean up. Remove it before merge.
A non-blocking note: the PR title/description still refer to displayField, but the code, types, changeset, and seed examples all use titleField. Align the PR description so the seed-file example matches the actual option name.
I did not run tests, lint, or typecheck; this review is static.
| // neither can't produce a suggestion and are skipped (selecting a missing | ||
| // column would error -- see #1178). |
There was a problem hiding this comment.
[needs fixing] This comment block ends with an issue reference (see #1178). AGENTS.md forbids issue/PR references in comments — that context belongs in commit messages and PR descriptions, not in source files. The previous round already asked for issue/PR references to be removed; this newly-added reference should be dropped too.
| // neither can't produce a suggestion and are skipped (selecting a missing | |
| // column would error -- see #1178). | |
| // neither can't produce a suggestion and are skipped (selecting a missing | |
| // column would error). |
github-actions
Bot
added
the
review/awaiting-author
3 participants
What does this PR do?
Closes #1133
Adds two optional collection options, displayField and dateField, so the admin shows a meaningful title and date for collections whose data doesn't follow the title/updated-date convention (e.g. an employees collection where name is the title and a custom column is the real date to be displayed).
Follow-ups: filtering the date-range picker by the custom dateField. The date-range filter still uses system dates (Created/Updated/Published), not a custom dateField. Only display and sort use it.
Note
The newly added displayField/dateField not exposed via REST/OpenAPI or the Content Type editor. This is intentional, config-as-code scope, matching #1133's proposed API, which defines these as collection-definition properties, not admin-UI or REST-managed settings. For this PR they're seed-file / config-as-code only.
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.AI-generated code disclosure
Screenshots / test output
Add this to the seed file (e.g.
seed/seed.json), then apply the seed and start the dev server:Then open Content → Employees in the Admin: the Title column shows names (not job titles), the Date column shows the start date, and sorting/search use those fields.