fix(core): resolve collection url_pattern for content preview links - #1745
fix(core): resolve collection url_pattern for content preview links#1745swissky wants to merge 2 commits into
Conversation
The preview-url endpoint defaulted to a hard-coded /{collection}/{id}, so the
admin Preview button 404'd on any site with a custom permalink (e.g.
/blog/{slug}). The sitemap and "View published" links already resolve the
collection's url_pattern; preview links now do too, via the shared
interpolateUrlPattern + localizePath helpers. Falls back to /{collection}/{id}
when no pattern is configured; an explicit pathPattern or
EMDASH_PREVIEW_PATH_PATTERN still wins.
🦋 Changeset detectedLatest commit: e766a63 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 |
github-actions
Bot
added
review/needs-review
@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: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a focused, backwards-compatible bug fix: the preview-URL endpoint now resolves a collection’s configured url_pattern (e.g. /blog/{slug}) before falling back to the generic /{collection}/{id} path, matching what the sitemap and published links already do. The precedence logic is implemented correctly, authorization uses the documented content:read_drafts permission, and the new route-level regression tests cover the primary slug-pattern, fallback, and body-override branches. The changeset is user-facing and accurate.
I did find one concrete code-level concern: a comment in the new code path claims date tokens (e.g. {year}/{month}/{day}) are supported, but the shared interpolateUrlPattern helper only substitutes {slug}, {id}, and {collection} — there is no date-token implementation in the codebase. The PR description makes the same unsupported claim, but the changeset does not, so it won’t leak into release notes. I also noted two test gaps: the EMDASH_PREVIEW_PATH_PATTERN env override branch and the new localizePath i18n prefix path are not exercised by the new tests.
None of these block the primary fix, so I’m leaving comment-level feedback rather than requesting changes.
| // No explicit override: reuse the collection's `url_pattern` so the | ||
| // preview link points at the same route the sitemap and "View | ||
| // published" links already use (custom permalinks like `/blog/{slug}`, | ||
| // date tokens, etc.). Without this the generic `/{collection}/{id}` |
There was a problem hiding this comment.
[suggestion] The comment says the resolved URL pattern supports “custom permalinks like /blog/{slug}, date tokens, etc.”, but the shared interpolateUrlPattern helper used here only substitutes {slug}, {id}, and {collection} — there is no support for {year}, {month}, {day} or any other date tokens anywhere in the codebase. This will mislead future maintainers (and already misstates the PR’s scope in the description). Suggestion:
| // date tokens, etc.). Without this the generic `/{collection}/{id}` | |
| // preview link points at the same route the sitemap and "View | |
| // published" links already use (custom permalinks like `/blog/{slug}`). | |
| // Without this the generic `/{collection}/{id}` fallback 404s on any |
There was a problem hiding this comment.
Fixed in e766a63 — the comment now says only "custom permalinks like /blog/{slug}". You're right that date tokens don't exist yet: they're PR #1526, which is still open. The PR description's claim was forward-looking (once #1526 lands in interpolateUrlPattern, preview links pick it up for free since they route through the same helper), but the code comment shouldn't describe features that aren't merged.
| expect(url.startsWith(`/post/${id}?_preview=`)).toBe(true); | ||
| }); | ||
|
|
||
| it("lets an explicit pathPattern override the url_pattern", async () => { |
There was a problem hiding this comment.
[suggestion] The new tests cover the body-level pathPattern override and the collection url_pattern branch, but the EMDASH_PREVIEW_PATH_PATTERN env-override path in the route is not exercised. Because that branch is supposed to stay at the same precedence as a body override, consider adding a test that confirms it still wins over a configured url_pattern — even if it has to rely on build-time stubs for import.meta.env.
There was a problem hiding this comment.
Added in e766a63: vi.stubEnv("EMDASH_PREVIEW_PATH_PATTERN", "/env/{id}") (stubs import.meta.env under vitest) with a collection that also has url_pattern: /blog/{slug} configured — the test asserts the env pattern wins and /blog/ doesn't leak through.
| await teardownTestDatabase(db); | ||
| }); | ||
|
|
||
| it("resolves the configured url_pattern into the preview link", async () => { |
There was a problem hiding this comment.
[suggestion] The new url_pattern code path calls localizePath, which handles locale prefixes and custom path/codes mappings, but the current tests run with no i18n config so that logic is never exercised. Consider adding a test that sets an i18n config (via setI18nConfig) and verifies the preview link receives the expected locale prefix for a non-default-locale entry.
There was a problem hiding this comment.
Added in e766a63: the test sets setI18nConfig({ defaultLocale: "en", locales: ["en", "de"], prefixDefaultLocale: false }) (plus _resetAstroI18nCacheForTests(), same pattern as the sitemap route tests), creates a de entry, and asserts the preview link comes back as /de/blog/hallo-welt?_preview=….
github-actions
Bot
added
review/awaiting-author
… and i18n prefix
The code comment claimed date tokens resolve through url_pattern, but
interpolateUrlPattern only substitutes {slug}/{id} (date tokens are
PR emdash-cms#1526, not yet merged) — comment now sticks to what exists. Two new
route tests close the review's coverage gaps: EMDASH_PREVIEW_PATH_PATTERN
still wins over a configured url_pattern, and a non-default-locale entry
gets its locale prefix via localizePath.
github-actions
Bot
added
review/needs-rereview
swissky
added
bot:review
There was a problem hiding this comment.
This is the right bug fix: the preview-URL endpoint now resolves a collection's configured url_pattern before falling back to the generic /{collection}/{id} path, keeping preview links consistent with the sitemap and published permalinks. The precedence order (body → env → collection url_pattern → fallback) is implemented correctly, authorization uses the documented content:read_drafts permission, and the tests cover the new behavior plus the two branches that were missing in the first pass.
The prior review's concerns are addressed: the route comments no longer imply unsupported date-token resolution, and the new tests exercise both EMDASH_PREVIEW_PATH_PATTERN and the localizePath i18n prefix.
I have one non-blocking suggestion: the try/catch around path resolution now also covers the new getCollectionInfoWithDb lookup, but any failure there is still surfaced as TOKEN_ERROR, which is misleading for schema/env failures. Making the error code and construction path more specific would help diagnostics, but it does not block the fix.
Findings
-
[suggestion]
packages/core/src/astro/routes/api/content/[collection]/[id]/preview-url.ts:139With the new
getCollectionInfoWithDblookup moved inside the sametryblock, a schema/DB failure here is now reported to the client asTOKEN_ERRORwith the generic message "Failed to generate preview URL". That code is accurate for token-signing failures but misleading for collection metadata or path-resolution failures.Consider either moving the schema lookup out of the token-generation
try/catchor giving it its own catch so callers get a more useful code (e.g.PREVIEW_URL_ERRORor the sameUNEXPECTED_ERROR-style code used elsewhere). Not a blocker for the primary fix.
github-actions
Bot
added
review/awaiting-author
|
This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity. If you're still working on this, please push an update or leave a comment. |
1 participant
What does this PR do?
The content Preview endpoint (
POST /_emdash/api/content/{collection}/{id}/preview-url) defaulted to a hard-coded/{collection}/{id}path. On any site whose content is served at a custom permalink — e.g. apostscollection withurl_pattern: "/blog/{slug}"— the admin "Preview" button produced a link that 404s, because the real route is/blog/{slug}, not/posts/{id}.The sitemap (
sitemap-[collection].xml.ts) and the "View published" links already resolve the collection'surl_patternvia the sharedinterpolateUrlPattern+localizePathhelpers. This PR makes the preview link use the same resolution, so preview and published URLs stay consistent.Path-resolution precedence is now (highest first):
pathPatternin the request body (per-call override) — unchangedEMDASH_PREVIEW_PATH_PATTERNenv (project-wide override) — unchangedurl_pattern— new/{collection}/{id}fallback — unchanged (used only when nourl_patternis set)Because it routes through
interpolateUrlPattern, any future additions to that helper apply to preview links automatically — e.g. if the WordPress-style date tokens proposed in #1526 ({year}/{month}/{day}/..., still open) land there, preview links will resolve them too. Today the helper substitutes{slug}and{id}.Fully backward-compatible: sites without a
url_patternsee no change, and both explicit overrides still win.Related: discussion #1525, PR #1526.
Type of change
Checklist
pnpm typecheckpasses —emdash(core) cleanpnpm lintpasses —oxlint --type-aware --deny-warningsclean on changed filespnpm testpasses (targeted): newpreview-url-route.test.ts(3/3) plus adjacentpreview,url-pattern,public-url,content-route-permissionssuites (79/79)pnpm formathas been runmessages.pochanges)emdash, patch)AI-generated code disclosure
Screenshots / test output
Example: a
postscollection withurl_pattern: "/blog/{slug}"and an entryhello-worldnow yields/blog/hello-world?_preview=…instead of/posts/{id}?_preview=….