🎨 Changed the upgrade destination on host limits to be configurable - #29843
🎨 Changed the upgrade destination on host limits to be configurable#29843betschki wants to merge 4 commits into
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (11)
WalkthroughThe PR adds billing upgrade URL configuration and shared route helpers. Admin analytics and settings limit flows now use configured upgrade destinations instead of fixed Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/admin/src/settings/app/components/settings/membership/analytics.tsx (1)
72-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a keyboard-operable control for “Upgrade now”.
The upgrade action at
apps/admin/src/settings/app/components/settings/membership/analytics.tsx:77is a clickablespanwith onlyonClick, so keyboard and assistive technology users cannot activate it. Use a<button>or<a>, still callingupdateRoute({route: upgradeUrl, isExternal: true})as needed.🤖 Prompt for 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. In `@apps/admin/src/settings/app/components/settings/membership/analytics.tsx` around lines 72 - 78, The “Upgrade now” action inside the isWebAnalyticsLimited membership notice is a clickable span that is not keyboard-operable. Replace that span with an accessible button or link while preserving the existing updateRoute call using upgradeUrl and isExternal: true.apps/admin/src/settings/app/components/settings/advanced/integrations/add-integration-modal.tsx (1)
27-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep limit-modal callbacks synchronized with the resolved upgrade URL.
upgradeUrlcomes fromuseGlobalData().config, and these effects miss it from their dependency arrays. IncludeupgradeUrlin each callback effect so a config refresh does not leave the onOk callback pointing at the old upgrade destination.🤖 Prompt for 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. In `@apps/admin/src/settings/app/components/settings/advanced/integrations/add-integration-modal.tsx` around lines 27 - 33, Keep the limit-modal callback effects synchronized with the resolved upgradeUrl by adding upgradeUrl to the dependency arrays that capture it. Update the affected effects in apps/admin/src/settings/app/components/settings/advanced/integrations/add-integration-modal.tsx lines 27-33, apps/admin/src/settings/app/components/settings/email/newsletters/add-newsletter-modal.tsx lines 80-86, and apps/admin/src/settings/app/components/settings/membership/stripe/stripe-connect-modal.tsx lines 280-291; no other callback behavior should change.
🤖 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/admin/src/settings/app/components/settings/site/design-and-theme-modal.tsx`:
- Around line 29-31: Update the useCallback dependency list for
showThemeLimitModal to include upgradeUrl alongside updateRoute, so the modal
callback refreshes whenever the configured billing upgrade route changes.
---
Outside diff comments:
In
`@apps/admin/src/settings/app/components/settings/advanced/integrations/add-integration-modal.tsx`:
- Around line 27-33: Keep the limit-modal callback effects synchronized with the
resolved upgradeUrl by adding upgradeUrl to the dependency arrays that capture
it. Update the affected effects in
apps/admin/src/settings/app/components/settings/advanced/integrations/add-integration-modal.tsx
lines 27-33,
apps/admin/src/settings/app/components/settings/email/newsletters/add-newsletter-modal.tsx
lines 80-86, and
apps/admin/src/settings/app/components/settings/membership/stripe/stripe-connect-modal.tsx
lines 280-291; no other callback behavior should change.
In `@apps/admin/src/settings/app/components/settings/membership/analytics.tsx`:
- Around line 72-78: The “Upgrade now” action inside the isWebAnalyticsLimited
membership notice is a clickable span that is not keyboard-operable. Replace
that span with an accessible button or link while preserving the existing
updateRoute call using upgradeUrl and isExternal: true.
🪄 Autofix
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: 4981e2b5-e230-487b-b141-cccedc7a92ed
📒 Files selected for processing (20)
apps/admin-x-framework/src/api/config.tsapps/admin/src/analytics/views/stats/overview/components/overview-kpis.tsxapps/admin/src/settings/app/components/settings/advanced/integrations/add-integration-modal.tsxapps/admin/src/settings/app/components/settings/email/newsletters/add-newsletter-modal.tsxapps/admin/src/settings/app/components/settings/email/newsletters/newsletter-detail-modal.tsxapps/admin/src/settings/app/components/settings/general/user-detail-modal.tsxapps/admin/src/settings/app/components/settings/membership/analytics.tsxapps/admin/src/settings/app/components/settings/membership/stripe/stripe-connect-modal.tsxapps/admin/src/settings/app/components/settings/membership/tiers.tsxapps/admin/src/settings/app/components/settings/site/change-theme.tsxapps/admin/src/settings/app/components/settings/site/design-and-theme-modal.tsxapps/admin/src/settings/app/components/settings/site/theme-modal.tsxapps/admin/src/settings/app/components/settings/site/theme/advanced-theme-settings.tsxapps/admin/src/settings/app/hooks/use-upgrade-url.test.tsxapps/admin/src/settings/app/hooks/use-upgrade-url.tsapps/ember-admin/app/components/editor/modals/publish-flow/confirm.jsapps/ember-admin/app/components/modals/limits/publish-limit.hbsapps/ember-admin/app/components/modals/limits/publish-limit.jsapps/ember-admin/app/utils/publish-options.jsapps/ember-admin/app/utils/upgrade-url.js
no ref Every upgrade prompt on a host limit sent people to Ghost(Pro)'s billing app at a hardcoded '/pro', so a managed host running its own billing had no way to redirect them. Adds hostSettings.billing.upgradeUrl, resolved through upgradeRoute() in admin-x-framework so both admin apps read it the same way whichever config provider they have, and through a matching util in Ember, which can't import it. The setting is stored as an href to match the existing upgradeUrl settings for the pre-launch and trial banners. Navigation takes a route, so React drops a leading '#'; Ember uses the value as-is. The publish-limit modal's Upgrade button became an anchor to carry an arbitrary destination, and its default is now the billing root rather than opening checkout directly.
326033b to
b24283e
Compare
9larsons
left a comment
9larsons
left a comment
There was a problem hiding this comment.
Thanks for this — the approach is right and the React side is in good shape. The config plumbing works with no server changes (hostSettings is passed through to the config endpoint wholesale), and routing overrides through navigateTo() means absolute URLs get scheme-validated for free. Nice tests on the hook.
Main blocker before merge: the publish-limit modal loses its direct-to-checkout behavior for Ghost(Pro) — details inline. Also a small escaping concern on the htmlSafe interpolations, plus a couple of nits.
no ref - keeps the publish-limit modal going straight to checkout, via a per-surface fallback on the Ember util, with a test for it - anchors a configured value that is neither a hash nor a URL, and rejects a scheme that isn't http(s), matching navigateTo() on the React side - escapes the URL before interpolating it into the limit message markup - renames the React hook to useUpgradeRoute, since it returns a route
|
@9larsons thank you, all addressed :) |
|
One host-limit upgrade surface is still bypassing the new setting:
updateRoute({route: 'pro', isExternal: true});Those Zapier and Transistor cards are disabled by |
no ref The Zapier and Transistor cards are disabled by the customIntegrations limit and their CTA still routed to 'pro' directly. The route is resolved in BuiltInIntegrations and passed down: reading it per item subscribed every card to global data, which re-rendered the custom integration modals mid-edit. Updates the acceptance test to the resolved default and adds one for a configured destination.
…imit-upgrade-url # Conflicts: # apps/admin/src/settings/app/components/settings/advanced/integrations/add-integration-modal.tsx # apps/admin/src/settings/app/components/settings/email/newsletters/add-newsletter-modal.tsx # apps/admin/src/settings/app/components/settings/membership/stripe/stripe-connect-modal.tsx # apps/admin/src/settings/app/components/settings/site/change-theme.tsx # apps/admin/src/settings/app/components/settings/site/design-and-theme-modal.tsx
2 participants
Every limit dialogue sent people to Ghost(Pro)'s billing app at a hardcoded '/pro', so a managed host running its own billing had no way to redirect them. Adds hostSettings.billing.upgradeUrl, read through a shared helper alongside the existing upgradeUrl settings for the pre-launch and trial banners.
Each surface keeps its own fallback so Ghost(Pro) lands where it always has: the publish-limit modal still opens checkout, and the inline publish messages still point at the billing root.
hostSettings holds an href, so the React hook drops a leading '#' before handing it to updateRoute(), which reads that as an internal route.
Got some code for us? Awesome 🎊!
Please take a minute to explain the change you're making:
Please check your PR against these items:
We appreciate your contribution! 🙏