🎨 Added configurable search entries for billing - #29645
Conversation
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin:test:acceptance |
✅ Succeeded | 6m 12s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-31 14:56:49 UTC
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds gated static Ghost(Pro) billing search results with keyword matching and support in both search providers. Selected results navigate to Billing sub-routes through iframe messaging and pending-route handling. Ghost(Pro) results are excluded from Koenig editor link searches, and force-upgrade routing preserves Pro child paths. Tests cover search behavior and Billing navigation. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5395a8b33
ℹ️ 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".
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #29645 +/- ##
==========================================
+ Coverage 75.38% 75.40% +0.02%
==========================================
Files 1607 1609 +2
Lines 141658 141742 +84
Branches 17495 17517 +22
==========================================
+ Hits 106784 106879 +95
+ Misses 33806 33797 -9
+ Partials 1068 1066 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ember-admin/app/services/billing.js`:
- Around line 531-535: Update the routeUpdate postMessage call in the billing
service to use getBillingAppOrigin() as targetOrigin instead of '*'. If the
origin cannot be resolved, retain destinationRoute as pending rather than
sending the message; update the billing-service test to assert the resolved
origin is passed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f0f0b430-f492-445e-9da4-0c34db649491
📒 Files selected for processing (11)
apps/ember-admin/app/components/gh-search-input.jsapps/ember-admin/app/components/koenig-lexical-editor-input.jsapps/ember-admin/app/components/koenig-lexical-editor.jsapps/ember-admin/app/routes/pro/pro-sub.jsapps/ember-admin/app/services/billing.jsapps/ember-admin/app/services/search-provider-basic.jsapps/ember-admin/app/services/search-provider-flex.jsapps/ember-admin/app/utils/search.jsapps/ember-admin/tests/acceptance/search-test.jsapps/ember-admin/tests/integration/services/search-test.jsapps/ember-admin/tests/unit/services/billing-test.js
e5395a8 to
ccf3f49
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ember-admin/app/routes/application.js`:
- Around line 221-224: Update the billingRoute selection around requestedRoute
to accept only `/pro`, `/pro/...`, or optionally `/pro?...` paths, rather than
using an unbounded startsWith('/pro') match; preserve the fallback to `/pro` and
pass the validated route to billing.openBillingWindow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 036399c0-b487-4ccb-8b77-5def1d2da426
📒 Files selected for processing (11)
apps/ember-admin/app/components/koenig-lexical-editor-input.jsapps/ember-admin/app/components/koenig-lexical-editor.jsapps/ember-admin/app/routes/application.jsapps/ember-admin/app/services/billing.jsapps/ember-admin/app/services/search-provider-basic.jsapps/ember-admin/app/services/search-provider-flex.jsapps/ember-admin/app/utils/route.jsapps/ember-admin/app/utils/search.jsapps/ember-admin/tests/acceptance/search-test.jsapps/ember-admin/tests/integration/services/search-test.jsapps/ember-admin/tests/unit/services/billing-test.js
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/ember-admin/app/components/koenig-lexical-editor.js
- apps/ember-admin/tests/acceptance/search-test.js
- apps/ember-admin/app/utils/search.js
- apps/ember-admin/app/services/search-provider-flex.js
- apps/ember-admin/tests/integration/services/search-test.js
- apps/ember-admin/app/components/koenig-lexical-editor-input.js
- apps/ember-admin/app/services/billing.js
6615584 to
00e410a
Compare
|
Just stumbled upon this and would like to raise a point: the way I read this, this PR would inject a Ghost(Pro) search group into any managed hosting provider that uses the billing portal. Even if we change the name of the group, they would still assume the same Ghost(Pro)-specific routes. I'd love to see some way to configure this. After all, that's why the Raising this, since I've been trying to reduce friction like this recently; I know, I am no authority for this (and do have other interests in mind than Ghost(Pro)), but would be cool if we could implement a pattern like that here. |
|
Thank you @sagzy ❤️ |
710ea11 to
d637e98
Compare
f97b183 to
4bdd781
Compare
no ref Ghost (Pro) publishers had no way to find billing actions — changing plan, setting up a custom domain, requesting a backup, contacting support — from the cmd+K search; they had to know these lived behind the Ghost (Pro) tab. Search now surfaces them as a "Ghost (Pro)" group, shown above content results so they aren't buried under posts. - Entries are a static client-side list indexed by both search providers (title + keywords, eg. "dns", "invoice", "renewal"), so no new API endpoint is needed. The group only appears when the site has billing enabled and the user can access the billing app (owner, or anyone in a force-upgrade state), mirroring the /pro route rules. - Selecting a result deep-links into the billing app. That exposed a bug in sub-route forwarding: the billing iframe is preloaded, so in-app navigation to /pro/* left it on its boot route, and the app's own trial redirect to /plans raced (and beat) any post-load route message. The billing service now boots the iframe directly on the destination route when opening, and sends a routeUpdate postMessage when the app is already loaded. - 'export' is deliberately not a backup keyword to avoid colliding with content export in settings, and Ghost (Pro) results are excluded from the editor's link search since they aren't linkable site content.
no ref - The routeUpdate postMessage now targets the billing app origin instead of '*', so the destination route can't be delivered to an unrelated document if the iframe has navigated away; when the origin can't be resolved the route stays pending instead. - Deep links to /pro/* now survive reloads and history navigation in a force-upgrade state: the boot-time redirect preserves the requested /pro child route and the global route guard allows all pro.* routes instead of only pro.index.
no ref
Search results now show the plain action ('Change plan') instead of
repeating the group label in every row, and the group header uses
Ghost(Pro) — the spelling used everywhere else in Admin. So that
searching the product name still lists the whole group, every entry
shares ghost/pro/ghost(pro) keywords instead of relying on the title
prefix.
no ref Removed comments that narrate what the adjacent code already shows and trimmed the remaining ones down to the constraints they exist to record (preload timing, redirect race, deliberate keyword omissions).
no ref
The /pro prefix check accepted any hash starting with '/pro', so an
unrelated route could be forwarded to the billing window verbatim; the
match now requires a route boundary ('/pro', '/pro/...', '/pro?...').
no ref The generic isSearchableAvailable() utility dressed a single concrete policy — show Ghost(Pro) results only with billing access — as a per-searchable framework, and made both providers inject config and session to repeat the same guard. The search service now filters the one gated group in one place; providers stay policy-free matchers.
ref https://linear.app/ghost/issue/GVA-861 The Ghost(Pro) deep-link mechanism kept its pending sub route alive past its useful life, which let a stale destination hijack a later plain /pro open: nothing cleared it when the overlay closed, a search selection armed it even when the transition was subsequently aborted (eg. unsaved changes), and a selection made while the overlay was already open fell back to a post-ready postMessage that can lose a race with the billing app's own boot redirects. - Closing the billing window now abandons any queued deep link - Search selection transitions first and lets the pro-sub route forward the destination, so an aborted transition leaves the billing app untouched and the double routeUpdate send is gone; the direct send survives only for the no-op transition case - A route queued while the overlay is already visible and the app is still loading now reloads the iframe with the destination baked into the URL instead of racing the app's initial redirects - The checkout action delegates to navigateToSubRoute, gaining origin pinning and ready-queueing instead of a '*'-targeted message that was dropped (with the intent cleared) when the app wasn't ready - Hash-to-billing-route parsing is anchored and owned by a single billing service helper, so unrelated hashes like #/products can't leak into the iframe URL and force-upgrade no longer re-implements the parsing - The billing access rule and the editor link-search filter each live in one shared place instead of duplicated copies
ref https://linear.app/ghost/issue/GVA-861 The search palette injected a hardcoded Ghost(Pro) group into any managed hosting provider that configures hostSettings.billing — the group name, action titles, keywords and /pro sub-routes are all Ghost(Pro)-specific, so other hosts' publishers would see actions that don't exist on their platform. Following the pattern established for other host-specific admin copy (the trial banner, pre-launch banner and email verification messages), hosts can now set hostSettings.billing.search.groupName to rename the group and hostSettings.billing.search.items to replace the actions with their own billing portal's pages (or remove the group entirely with an empty list). Ghost(Pro)'s defaults apply when nothing is configured, so behaviour is unchanged unless a host overrides it. Because the group name is no longer a stable value, consumers that dispatched on it (result selection, the owner-only gating filter and the editor's link-search exclusion) now dispatch on a stable groupKey carried by search groups and results — renaming the group can't silently break navigation or leak billing links into editor content. Custom entries are validated to deep-link into the billing app (/pro paths only) so host config can't route search results at arbitrary admin routes.
ref https://linear.app/ghost/issue/GVA-861 Alternative to the default-on approach in the previous commit, kept separate for comparison: drop this commit to keep default-on, keep it to ship opt-in. With default-on, every host that has billing enabled inherits Ghost(Pro)'s actions unless they override them. This variant only renders the group when hostSettings.billing.search is configured, so hosts never show another platform's actions by default — Ghost(Pro) opts in with an empty object (`search: {}`) to keep its defaults, and other hosts opt in with their own group name and items.
ref https://linear.app/ghost/issue/GVA-861 Third comparison variant, on top of the opt-in one: drop this commit to keep code-side defaults, keep it to define the group entirely in host config. Ghost core no longer ships any billing search content — no default group name, no default action list. The group only exists when hostSettings.billing.search provides a groupName and at least one valid item, making Ghost(Pro) just another host that supplies its catalog at the infra level. What stays in code is the host-agnostic mechanism: static-item indexing, the groupKey dispatch, owner/force-upgrade gating and the billing iframe plumbing. Configured item paths are routes within the host's own billing app (eg. '/plans'), not Admin URLs — Admin's /pro mounting point for the billing iframe is not part of the config contract. The billing service owns the mapping from a billing sub-route to the Admin route hosting it, so hosts only ever describe pages of the app they control. The trade-off is that the shipped catalog loses CI coverage: the tests now exercise the mechanism against a fixture config rather than asserting Ghost(Pro)'s actual entries, so typos or drift in the infra config can't be caught in this repo.
The route resolves no model — its only job is relaying the requested /pro/* segment to the billing iframe — so doing that in model() read as if a model was expected. beforeModel is the conventional hook for side effects that don't produce data, and it re-runs on wildcard segment changes just like model does, so behaviour is unchanged.
Review findings on the configurable billing search entries: - Entering the billing root never synced the iframe — browser Back from /pro/domain, or selecting an item with path '/', left the iframe on the old sub-page under the /pro URL. A new pro/index route forwards '/' the same way pro-sub forwards child routes, which also stops a deep-link destination baked into a hidden in-flight load from hijacking a later plain /pro open. - Opening with a pending sub route always reloaded the iframe, discarding a healthy in-flight load whose URL already targeted that route; repeat selections while the spinner showed restarted the load (and its failure monitor) every click. The load's destination is now tracked so a covered pending route rides the existing load instead of restarting it. - The trailing-hash location generates trailing-slash URLs and star segments keep the slash, so canonical /pro/domain/ deep links forwarded '/domain/' — off the slash-free contract the billing app expects. - Host config could name the billing group 'Posts' (double-navigating selections and cross-matching groups) or use paths with query strings (forwarded inconsistently between the transition and postMessage paths). configure() now rejects reserved names and non-plain paths. - The /pro route guard kept its own access predicate without the billing.enabled check, so a disabled site could still activate the route with no modal to show. It now shares billing.canAccessBilling, which treats force-upgrade as a full bypass so that flow is unchanged. - Messages to the billing iframe went through three divergent senders, one still posting with a wildcard target origin. They now share one origin-targeted helper. - Searchables are resolved once per session instead of on every keystroke, flexsearch indexes are only allocated for configured groups, keyword matching honours the searchable's index declaration, and the Koenig link filter drops non-linkable results by missing url instead of a billing-specific group check.
4bdd781 to
ef71970
Compare

ref GVA-861
Ghost(Pro) publishers had no way togo find billing actions — changing plan, setting up a custom domain, requesting a backup, contacting support — from the cmd+K search; they had to know these lived behind the Ghost(Pro) tab. Search can now surface them as a group shown above content results so they aren't buried under posts.
Following review feedback from @betschki, the group is defined entirely by host config rather than hardcoded: Ghost core ships no group name and no action list, so no hosting provider inherits another platform's actions. Any managed host that uses the billing portal — Ghost(Pro) included — opts in via
hostSettings.billing.search:groupNameis the heading shown above the group in search resultspathis a route within the host's own billing app (eg./plans) — Admin's/promounting point is not part of the config contract; the billing service owns that mappingkeywordsgroupNameand at least one valid item the group doesn't existThe group only appears when the site has billing enabled (
hostSettings.billing.enabled) and the current user can access the billing app — the owner, or anyone while the site is in a force-upgrade state — mirroring the access rules of the/proroute (the rule lives on the billing service and is shared). Because the group name is host-defined, all behaviour dispatches on a stablegroupKeycarried by search groups and results — selection handling, access gating and the editor link-search exclusion never hang off display copy.Note for rollout: until Ghost(Pro)'s infra config adds
hostSettings.billing.search, publishers won't see the group — the Admin change ships dark and is harmless on its own.Also fixed: deep links in the billing iframe
Selecting a result navigates to
/pro/<page>, and that surfaced a pre-existing problem: the billing app iframe is preloaded when Admin boots, so in-app navigation to/pro/*left it sitting on its boot route — and the app's own trial redirect to/plansraced (and beat) any post-load route message, which is why deep links always landed on the Billing tab. The billing service now:routeUpdatepostMessage when the app is already loaded, queuing it until the app reports ready otherwise.The queued route's lifecycle is kept tight so a stale destination can never hijack a later plain
/proopen: closing the billing window abandons any queued deep link, a route queued while the window is already visible (but the app still loading) reloads the iframe with the destination baked into the URL rather than racing the app's boot redirects, and search selections transition first — thepro/pro-subroute forwards the destination once the transition succeeds, so an aborted transition (eg. unsaved changes) leaves the billing app untouched and nothing is sent twice. The pre-existing?action=checkoutdeep link now goes through the same channel, gaining origin pinning and ready-queueing instead of a'*'-targeted message that was dropped when the app wasn't ready. Hash→billing-route parsing is anchored in a single billing service helper so unrelated hashes (eg.#/products) can't leak into the iframe URL.A new
pro/pro-subroute forwards the wildcard segment, so any future in-app deep link to/pro/*works, not just search. Billing results are also excluded from the editor's internal link search (via a shared filter helper), since they aren't linkable site content.