feat(nextly): resolve an entry preview url in one place - #855
Conversation
A collection may declare a preview two ways and they are disjoint: a code-first collection writes a function of the entry, a UI-created one writes a template string because it has nowhere to put a function. Both answered the same question in different places, so both are answered here and each caller asks rather than deciding. Resolution runs on the server because the function only exists in the server module graph. That also removes a permission the browser could not satisfy: settings is a system resource the editor and author presets do not grant, and those are the roles that share preview links. Resolving here means a caller needs collection access and never settings. The result is a four-case union rather than a nullable string. Three of the cases render as no button, but noSiteUrl is the one where a host is guessable and the guess is the admin origin, so it carries its own value and the path it could not base.
The admin needs one thing from a preview declaration before it can draw anything: whether a button belongs on the page. That is a boolean, and a boolean is storable even though the function it comes from is not, so the boolean is what the registry now holds. It is derived by the same predicate the resolver consults, so a stored true and a resolution reporting notConfigured cannot disagree. The URL is still never stored: it depends on the entry. preview therefore leaves the not-persisted list. Verified that the completeness assertion catches the alternative: dropping the projection while the key is absent from that list fails the build naming preview.
The declaration a code-first collection writes is a function, so it lives only in the server module graph and the admin cannot read it back. This is the route it asks instead, returning a finished absolute URL. Gated on read rather than the update that guards minting a preview LINK. A link is a bearer credential and is gated at the level of someone who may edit the draft it opens; this returns no credential, so requiring update would hide the button from a reviewer who may read but not change. The authored config is consulted before the registry. A code-first collection is synced into the registry too, but the function cannot survive the trip, so reading the registry first would find an empty declaration for exactly the collections whose preview works.
The route takes no path parameters: what it is asked about is form state, including values not yet saved, so no id identifies it. A deeper path is therefore a mistake rather than a variant and is refused. Left to fall through it would be answered by the bare route, which is the shape that once served webhook signing secrets from an invalid URL. ServiceType gains previewUrl, which is what forced the registration to be complete rather than merely compiling.
The panel decides WHETHER to offer the button from a boolean the registry stores, and asks the server WHERE only on click. It no longer reads the declaration itself: the code-first form is a function, and the template form is the resolver input rather than anything to interpolate here. The tab is claimed synchronously inside the click and navigated once the URL arrives. A window opened after an await has lost the user-gesture context and Safari and Firefox block it. noopener cannot be passed for that, since it makes window.open return null and leaves nothing to navigate, so the opener is severed by hand while the tab is still blank. A click that cannot open anything says which of the three reasons it hit, except notConfigured: the button should not have been drawn, so reporting it would describe a state the editor cannot act on.
|
@codex please review this PR |
|
Warning Review limit reached
Next review available in: 10 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0fea784bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
github-actions
Bot
added
scope: core
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
The resolved string is assigned to location.href by the admin, so its scheme decides whether the browser navigates or RUNS it. z.string().url() accepts javascript: and data: — measured, both parse — so a settings write could put script into the admin origin for whoever next clicked Preview. Navigable schemes are now an allowlist, applied to the configured site and to anything an authored function returns, so the two cannot disagree. A site url that fails it reports noSiteUrl rather than resolving: the remedy is the same as an absent one, an administrator setting a real value. The write schema rejects it too, so only a row stored before this existed can still carry one. Also: previewUrl joins the direct-dispatch set, without which the first authenticated request in a process reached an uninitialised container. The admin now writes unsaved data BEFORE opening the tab, since a new context copies session storage at creation and never sees a later write. And a blocked popup is reported instead of navigating this window, which would have taken the editor off the form and discarded their unsaved changes.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbbea33cfa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… key The preview-url parser never received the http method, so a GET matched and reached the json-body handler instead of being answered method-not-allowed. The adjacent preview-link parser rejects non-POST on its first line; this one now does the same. Session storage is partitioned by origin, and a resolved preview url is routinely on another one now that relative paths rebase onto the site url. The unsaved-data key would then name a payload the preview page cannot reach, so it is omitted and the caller is told the preview shows saved content. Appending it anyway looked like it worked while rendering stale data, which is what that path exists to prevent. That report is its own callback rather than a new unavailable reason: one says the click produced nothing, the other that it produced less than was asked for, and the editor acts differently on each. Availability now counts a stored template as declaring a preview. Only the code-first sync writes the boolean, so requiring it hid a preview that a ui-created row already declares.
|
@codex please review this PR |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What
admin.preview.urlnever reached the admin panel. A code-first collection could configure a preview button innextly.config.ts, type-check, and the button would simply never appear.This resolves an entry's preview URL in one place, on the server, and wires the panel to ask for it.
Why it was invisible: the feature was dead at four independent layers
Each layer returns "no preview button", so every one of them looks like the layer below it merely isn't configured.
mainbefore this PRpreview.urlADMIN_KEYS_NOT_PERSISTED— a function, no column can hold it, so it never reached the panelpreview.urlTemplateuseEntryPreviewPreviewActionsEntryFormActions, which the standalone editor never uses (found by the lane on #842)Layer 2 contradicts the task file, which claimed
urlTemplatewas "implemented end to end". Positive control for that measurement: sibling keyuseAsTitlescores 35 by the same search form.Design
One resolver.
resolvePreviewUrlanswers "where does this entry preview" for both authoring paths. The admin no longer interpolates templates — that would be a second implementation of the same question.Server-side, which removes a permission problem rather than routing around it.
settingsis a system resource;editorgrants(!isSystem && !isPlugin) || resource === "media"andauthorreturns false for every system resource but media. So neither role can read the configured site URL — and they are exactly who previews content. A browser-side answer would fall back towindow.location.origin, i.e. the admin's host, confidently wrong. Resolving on the server means the client reads no gated resource at all.Four states, not a nullable string.
Three render as "no button".
noSiteUrlearns its own case because it is the one where an origin is guessable and the guess is wrong. Collapsing them is what produced the defect above.hasPreviewis persisted, derived by the same predicate the resolver consults, so a storedtruecannot outlive the declaration it came from. The button's presence therefore needs no round trip; only the URL does.Popup blocking. Once the URL comes from a round trip,
window.open()after anawaitloses the user-gesture context and Safari and Firefox block it. The tab is claimed synchronously inside the click and navigated when the URL arrives.noopenercannot be passed — it makeswindow.openreturnnull, leaving nothing to navigate — soopeneris severed by hand while the tab is still blank and same-origin.Verification
Every guard here was broken on purpose and required to fail for the intended reason:
noSiteUrl→unavailableexpected { status: 'unavailable' } to deeply equal { status: 'noSiteUrl' }expected { status: 'resolved' } to deeply equal { status: 'unavailable' }preview/preview-url/123answered by the bare routeexpected { service: 'previewUrl' } to deeply equal {}awaitexpected [ 'resolve', 'open' ] to deeply equal [ 'open', 'resolve' ]Admin baseline unchanged: 19 failed / 5 files (
StatsCard · RoleBasicInfo · BasicsTab · SlugInput · DefaultValueField), identical tomain. Compared by file list, not count.Known gaps, deliberately not in this PR
createPreviewRouteappears intemplates/apps/create-nextly-apponly in a CHANGELOG line. Every scaffolded app will still 404 on a preview link after this merges. Filed; I am taking it as my next PR. Positive controls recorded: the same command finds the definition, and 21 realroute.tsfiles exist in that search path.urlTemplatestill has no UI, so Schema-Builder collections cannot configure a preview yet. The resolver handles the shape; the settings field is the PR after next. Founder-decided sequencing.admin.previewconfig at all, and the mint endpoint is collection-only by construction. A feature on both halves, not a wiring gap — out of scope by decision, not by oversight.Coordination
#842(preview link affordance) is held pending this. Seam agreed: I answer "what URL", that lane answers "what grant". ItsPreviewActionsprop surface already hasisPreviewAvailable/onPreviewunset and waiting.routeHandler.tscleared by the lane that held it (their feat(nextly): serve only branding to an anonymous caller #845 merged first).collection-sync-service.tscleared by the schema lane, measured.