fix(menus): resolve content-picker collection items to the entry URL, not the archive#1810
Conversation
… not the archive The admin content picker stores entries from custom collections as type "collection" with referenceCollection + referenceId, but the resolver ignored the reference_id for this type and returned the archive URL, so every such menu item linked to /projects/ instead of the picked entry. Entry references now resolve like page/post items (url_pattern and locale aware); items whose entry is gone are hidden, matching the page/post contract, as are malformed items with no collection slug (previously rendered href="/null/").
🦋 Changeset detectedLatest commit: 8bbeb84 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
There was a problem hiding this comment.
This is the right fix for a clear bug. The admin content picker stores custom-collection picks as type: "collection" with referenceCollection + referenceId (a translation_group), but resolveMenuItem() was ignoring reference_id for that type and always emitting the archive URL. The PR reuses the existing resolveContentUrl helper so collection entry references now get the same urlPattern interpolation and per-locale resolution as page/post items, while preserving archive-only behavior and adding the missing /null/ guard.
I checked the diff against the runtime semantics and AGENTS.md conventions:
- Locale filtering:
resolveContentUrlalready filters by locale first, then falls back deterministically; this change inherits that behavior. - SQL safety: no raw interpolation is introduced; the dynamic table name in
resolveContentUrlusessql.ref()aftervalidateIdentifier(). - Changeset: present, user-facing, and correctly scoped as a patch for
emdash. - Tests: cover entry resolution,
urlPattern, archive fallback, missing content rows, and missingreference_collection. - No admin UI strings changed, so Lingui/i18n is not applicable.
No findings. LGTM.
@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: |
What does this PR do?
Menu items added through the admin content picker from a custom collection (anything that isn't
pages/posts) link to the collection archive instead of the picked entry.The admin's
MenuEditormaps those picks totype: "collection"withreferenceCollection+referenceId, and the API schema documents that non-custom types "resolve a URL fromreferenceCollection+referenceId" — butresolveMenuItem()'scase "collection"ignoredreference_identirely and always returned/{collection}/. Picking "Widget Co" from aprojectscollection produced a nav link to/projects/.This PR makes entry references resolve the same way
page/postitems do (sharedresolveContentUrl, sourlPatterninterpolation and per-locale resolution apply), while archive links keep their current behavior:type: "collection"+reference_id→ entry URL (/projects/widget-co, or the collection'surlPattern); the item is hidden if the referenced entry no longer exists, matching the page/post contract.type: "collection"withoutreference_id→ archive URL/{collection}/, unchanged.type: "collection"with noreference_collection→ item hidden (previously renderedhref="/null/").Found while building a theme on EmDash and auditing how each menu item type resolves. Tests cover all five shapes above; the entry-reference and url_pattern tests fail on
main(expected '/projects/' to be '/projects/widget-co').Issue: none filed.
Two small related observations from the same audit — happy to open a Discussion for either:
getMenu(name)is an exact-match lookup, sogetMenu("primary")misses a menu saved asPrimary. Would you take a documented lowercase-name convention in the docs, or a case-insensitive lookup?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 strings changed.AI-generated code disclosure
Screenshots / test output
mainbefore the fix:AssertionError: expected '/projects/' to be '/projects/widget-co'pnpm --filter emdash exec vitest run tests/unit/menus/→ 52 passedpnpm lint:json | jq '.diagnostics | length'→0(clean before and after)pnpm typecheck→ pass (workspace)pnpm format→ ranemdash, patch)