🐛 Fixed force upgrade mode not blocking React shell routes #25687
Conversation
WalkthroughThis PR implements a site-wide force upgrade feature for Ghost Admin. Changes include: adding a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
rob-ghost
force-pushed
the
force-upgrade-react-shell
branch
3 times, most recently
from
eff6f0c to
78cb936
Compare
ref https://linear.app/ghost/issue/BER-3127 When Ghost is in forceUpgrade mode (requires billing action), routes handled by React (Analytics, Network, Tags) were bypassing the protection and rendering instead of redirecting to the billing page. This adds a declarative route protection system using React Router's handle property. A ForceUpgradeGuard component wraps all routes and redirects to /pro when forceUpgrade is active, unless the route has handle.allowInForceUpgrade set to true.
rob-ghost
force-pushed
the
force-upgrade-react-shell
branch
from
78cb936 to
735ef58
Compare
rob-ghost
requested review from
9larsons,
cmraible and
ibalosh
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
e2e/tests/admin/sidebar/force-upgrade.test.ts (2)
77-82: Consider using a semantic locator for the billing iframe.Per coding guidelines, prefer semantic locators over CSS selectors. The iframe has a title attribute, so you could use
getByTitle:-const billingIframe = page.locator('iframe[title="Billing"]'); +const billingIframe = page.getByTitle('Billing');This aligns with the guideline to prefer
getByRole,getByLabel,getByTextover test IDs and CSS selectors.
115-151: Good coverage of allowed routes with clear test boundaries.Each test covers a single scenario as required by the guidelines. The sign-out test properly validates the end state (redirect to signin page).
Minor observation: The Settings tests (lines 116-135) don't explicitly verify that the billing iframe is not visible after navigation. Consider adding a negative assertion for completeness:
// Settings should be allowed - verify billing iframe is NOT visible await expect(page.getByTitle('Billing')).not.toBeVisible();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/admin-x-framework/src/api/config.ts(1 hunks)apps/admin/src/ember-bridge/ember-bridge.tsx(3 hunks)apps/admin/src/ember-bridge/force-upgrade-guard.tsx(1 hunks)apps/admin/src/ember-bridge/index.ts(1 hunks)apps/admin/src/routes.tsx(1 hunks)e2e/helpers/pages/admin/sidebar/sidebar-page.ts(1 hunks)e2e/helpers/playwright/fixture.ts(1 hunks)e2e/helpers/playwright/flows/login.ts(1 hunks)e2e/tests/admin/sidebar/force-upgrade.test.ts(1 hunks)e2e/tests/admin/sidebar/navigation.test.ts(1 hunks)ghost/admin/app/routes/application.js(2 hunks)ghost/admin/app/services/state-bridge.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (e2e/AGENTS.md)
Prefer less comments and give things clear names
Files:
e2e/helpers/playwright/flows/login.tse2e/tests/admin/sidebar/force-upgrade.test.tse2e/tests/admin/sidebar/navigation.test.tse2e/helpers/pages/admin/sidebar/sidebar-page.tse2e/helpers/playwright/fixture.ts
e2e/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
E2E tests should use Playwright with Docker container isolation
Files:
e2e/helpers/playwright/flows/login.tse2e/tests/admin/sidebar/force-upgrade.test.tse2e/tests/admin/sidebar/navigation.test.tse2e/helpers/pages/admin/sidebar/sidebar-page.tse2e/helpers/playwright/fixture.ts
e2e/**/*.test.ts
📄 CodeRabbit inference engine (e2e/AGENTS.md)
e2e/**/*.test.ts: Always follow ADRs in../adr/folder (ADR-0001: AAA pattern, ADR-0002: Page Objects)
Never use CSS/XPath selectors - only use semantic locators or data-testid
Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'
Test names should be lowercase and follow the format 'what is tested - expected outcome'
Each test should represent one scenario only - never mix multiple scenarios in a single test
Follow the AAA (Arrange, Act, Assert) pattern in test structure
Prefer semantic locators (getByRole,getByLabel,getByText) over test IDs and never use CSS selectors, XPath, nth-child, or class names
UsegetByTestId()only when semantic locators are unavailable, and suggest addingdata-testidto Ghost codebase when needed
Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation
Never use hard-coded waits likewaitForTimeout()
Never usenetworkidlein waits
Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Usetest.only()for debugging single tests
Manual login should not be used - authentication is automatic via fixture
Files:
e2e/tests/admin/sidebar/force-upgrade.test.tse2e/tests/admin/sidebar/navigation.test.ts
e2e/helpers/pages/**/*.ts
📄 CodeRabbit inference engine (e2e/AGENTS.md)
e2e/helpers/pages/**/*.ts: Page Objects should be located inhelpers/pages/directory
Expose locators aspublic readonlyin Page Objects when used with assertions
Page Object methods should use semantic names (e.g.,login()instead ofclickLoginButton())
UsewaitFor()for guards in Page Objects, never useexpect()in page objects
Files:
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24673
File: ghost/i18n/lib/i18n.js:34-35
Timestamp: 2025-11-24T11:12:15.712Z
Learning: In the Ghost i18n package (ghost/i18n/lib/i18n.js), changing existing locale codes requires backwards compatibility handling for users who have already configured those locales. Such changes should be done in a separate PR with migration logic rather than included in feature PRs.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Applies to ghost/i18n/locales/en/ghost.json : Add UI translations to `ghost/i18n/locales/en/ghost.json` for Admin UI features
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 23770
File: apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModalLabs.tsx:435-450
Timestamp: 2025-06-10T11:07:10.800Z
Learning: In Ghost's newsletter customization features, when promoting a feature from alpha to beta status, the feature flag guards are updated to make the feature available under both the `emailCustomization` (beta) and `emailCustomizationAlpha` (alpha) flags. This is done by either removing conditional guards entirely when the component is already behind both flags, or by updating conditionals to check for either flag.
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
e2e/helpers/playwright/flows/login.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/helpers/pages/**/*.ts : Use `waitFor()` for guards in Page Objects, never use `expect()` in page objects
Applied to files:
e2e/helpers/playwright/flows/login.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Never use hard-coded waits like `waitForTimeout()`
Applied to files:
e2e/helpers/playwright/flows/login.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/helpers/pages/**/*.ts : Page Object methods should use semantic names (e.g., `login()` instead of `clickLoginButton()`)
Applied to files:
e2e/helpers/playwright/flows/login.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Manual login should not be used - authentication is automatic via fixture
Applied to files:
e2e/helpers/playwright/flows/login.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ember admin uses `AdminXComponent` to dynamically import React apps with Suspense and error boundaries
Applied to files:
apps/admin/src/ember-bridge/force-upgrade-guard.tsxapps/admin/src/ember-bridge/ember-bridge.tsxapps/admin/src/ember-bridge/index.ts
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/admin/src/ember-bridge/force-upgrade-guard.tsx
📚 Learning: 2025-10-30T17:13:26.190Z
Learnt from: sam-lord
Repo: TryGhost/Ghost PR: 25303
File: ghost/core/core/server/services/email-service/BatchSendingService.js:19-19
Timestamp: 2025-10-30T17:13:26.190Z
Learning: In ghost/core/core/server/services/email-service/BatchSendingService.js and similar files in the Ghost codebase, prefer using `{...options}` spread syntax without explicit guards like `...(options || {})` when spreading potentially undefined objects, as the maintainer prefers cleaner syntax over defensive patterns when the behavior is safe.
Applied to files:
ghost/admin/app/services/state-bridge.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'
Applied to files:
e2e/tests/admin/sidebar/force-upgrade.test.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
e2e/tests/admin/sidebar/force-upgrade.test.tse2e/tests/admin/sidebar/navigation.test.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn test:integration` for integration tests in ghost/core
Applied to files:
e2e/tests/admin/sidebar/force-upgrade.test.ts
📚 Learning: 2025-11-10T17:07:54.169Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25377
File: apps/admin/src/ember-bridge/EmberBridge.test.tsx:146-174
Timestamp: 2025-11-10T17:07:54.169Z
Learning: In apps/admin/src/ember-bridge/EmberBridge.tsx, the useEmberAuthSync hook invalidates all React Query caches whenever an emberAuthChange event is emitted, regardless of the isAuthenticated field value in the event payload. The isAuthenticated field exists in the event type but is not currently used by the handler.
Applied to files:
apps/admin/src/ember-bridge/ember-bridge.tsxapps/admin/src/ember-bridge/index.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation
Applied to files:
e2e/tests/admin/sidebar/navigation.test.ts
📚 Learning: 2025-11-05T16:42:12.989Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25356
File: apps/admin/test-utils/fixtures/query-client.tsx:17-35
Timestamp: 2025-11-05T16:42:12.989Z
Learning: In apps/admin/test-utils/fixtures/query-client.tsx, the createTestQueryClient function is intentionally duplicated from admin-x-framework to reduce external dependencies in the admin app's test utilities.
Applied to files:
e2e/tests/admin/sidebar/navigation.test.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
e2e/tests/admin/sidebar/navigation.test.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Prefer semantic locators (`getByRole`, `getByLabel`, `getByText`) over test IDs and never use CSS selectors, XPath, nth-child, or class names
Applied to files:
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/helpers/pages/**/*.ts : Expose locators as `public readonly` in Page Objects when used with assertions
Applied to files:
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use `getByTestId()` only when semantic locators are unavailable, and suggest adding `data-testid` to Ghost codebase when needed
Applied to files:
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
📚 Learning: 2025-11-03T12:33:31.093Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25320
File: apps/admin/src/layout/app-sidebar/NavHeader.tsx:10-10
Timestamp: 2025-11-03T12:33:31.093Z
Learning: The Ghost admin apps (apps/admin/**) do not use SSR, so accessing browser APIs like `navigator`, `window`, or `document` at module load time is safe and does not require typeof guards.
Applied to files:
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
📚 Learning: 2025-12-09T12:37:30.664Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25501
File: apps/shade/src/hooks/use-mobile.tsx:5-10
Timestamp: 2025-12-09T12:37:30.664Z
Learning: The Ghost admin apps/shade/** components do not use SSR, so accessing browser APIs like `navigator`, `window`, or `document` at module load time is safe and does not require typeof guards.
Applied to files:
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
📚 Learning: 2025-12-01T08:42:35.320Z
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25552
File: e2e/helpers/environment/service-managers/dev-ghost-manager.ts:210-247
Timestamp: 2025-12-01T08:42:35.320Z
Learning: In e2e/helpers/environment/service-managers/dev-ghost-manager.ts, the hardcoded volume name 'ghost-dev_shared-config' at line 231 is intentional. E2E tests run under the 'ghost-dev-e2e' project namespace but deliberately mount the shared-config volume from the main 'ghost-dev' project to access Tinybird credentials created by yarn dev:forward. This is cross-project volume sharing by design.
Applied to files:
e2e/helpers/playwright/fixture.ts
🧬 Code graph analysis (2)
e2e/helpers/playwright/flows/login.ts (1)
e2e/helpers/pages/admin/login-page.ts (1)
LoginPage(4-63)
e2e/tests/admin/sidebar/force-upgrade.test.ts (1)
e2e/helpers/pages/admin/sidebar/sidebar-page.ts (2)
NAV_ITEMS(17-25)SidebarPage(35-108)
🔇 Additional comments (16)
e2e/helpers/playwright/flows/login.ts (2)
1-1: LGTM: Simplified import.Removing the
AnalyticsOverviewPageimport is correct since the function no longer waits for the analytics page header.
8-11: Consider using a more specific selector for the sidebar navigation.The
getByRole('navigation')selector will match the first visible<nav>element on the page. Since the sidebar component has multiple navigation elements (desktop, mobile, and collapsed states), this could match the wrong one depending on render order and viewport size. Usepage.locator('[data-sidebar="sidebar"]')to specifically target the sidebar, or combine the role selector with additional filtering to ensure you're waiting for the correct navigation element.Likely an incorrect or invalid review comment.
apps/admin/src/ember-bridge/force-upgrade-guard.tsx (1)
29-44: LGTM! Clean declarative guard implementation.The guard correctly leverages React Router's
useMatches()to inspect route metadata and redirects appropriately. Thereplaceprop onNavigateis a good choice to prevent the blocked route from appearing in browser history.e2e/helpers/playwright/fixture.ts (1)
22-22: LGTM! Config property follows established conventions.The
hostSettings__forceUpgradeproperty correctly uses the double-underscore convention for nested configuration, consistent with existing properties likehostSettings__billing__enabled.apps/admin-x-framework/src/api/config.ts (1)
35-35: LGTM! Type definition correctly extends the config surface.The optional
forceUpgradeboolean is well-placed withinhostSettingsand maintains backward compatibility.ghost/admin/app/services/state-bridge.js (1)
204-206: LGTM! Simplified data passing.Removing the unnecessary object spread aligns with the codebase preference for cleaner syntax. The data structure is now created by the caller, which is appropriate.
apps/admin/src/ember-bridge/index.ts (1)
5-8: LGTM! Clean barrel exports.The new exports follow the established pattern of the file, properly separating component/hook exports from type exports.
e2e/tests/admin/sidebar/navigation.test.ts (1)
1-1: LGTM! Good consolidation of navigation data.Importing
NAV_ITEMSfrom the centralized location eliminates duplication and establishes a single source of truth for navigation test data.e2e/helpers/pages/admin/sidebar/sidebar-page.ts (2)
4-25: LGTM! Well-structured navigation data model.The
NavIteminterface andNAV_ITEMSconstant provide a clean, typed data structure for navigation testing. The RegExp patterns appropriately handle URL variations, and the role definitions enable role-based test scenarios.
44-64: LGTM! Locators follow Page Object best practices.The new locators correctly use semantic selectors (
getByRole) as per coding guidelines. The.or()pattern forupgradeNowLinkis a pragmatic solution for handling both React and Ember implementations during the migration period.ghost/admin/app/routes/application.js (1)
237-241: LGTM! Proper initialization of React subscription state.The subscription change notification is well-placed after the billing window logic, ensuring React has the subscription data needed to derive the
forceUpgradestate. The empty object fallback is appropriate—theSubscriptionStateinterface defines all subscription properties as optional (status?,isActiveTrial?,trial_end?), so the fallback safely handles cases wherethis.billing.subscriptionis undefined, and the React code uses optional chaining to accesssubscription?.status.apps/admin/src/ember-bridge/ember-bridge.tsx (2)
45-51: LGTM! Making fields optional accommodates varying subscription states.The optional fields allow the interface to handle partial subscription data from different sources (Ember bridge events, initial state).
298-316: Consider the initial loading state behavior.When
subscriptionStatusisnull(before Ember emits the firstsubscriptionChangeevent), the hook returnstrueifconfigForceUpgradeis truthy. This is likely the desired behavior—default to blocking access until subscription status is confirmed—but verify this doesn't cause a brief flash of the billing iframe on normal loads.The hook logic correctly mirrors the Ember behavior described in the PR objectives.
e2e/tests/admin/sidebar/force-upgrade.test.ts (1)
84-102: Well-structured parameterized tests following AAA pattern.The tests properly iterate over
NAV_ITEMSusing the page object pattern, follow the AAA structure (Arrange: goto, Act: click nav, Assert: expect iframe visible), and test names are lowercase. Good use of the shared fixture configuration.apps/admin/src/routes.tsx (2)
22-80: Clean implementation of the force upgrade guard pattern.The route structure correctly:
- Wraps all routes under
ForceUpgradeGuardas a layout route- Opts-in specific routes (
settings/*, catch-all for Ember) to bypass protection- Leaves React-migrated routes (Analytics, Network/ActivityPub, Tags list) blocked by default
The use of
handlemetadata withallowInForceUpgradeis an idiomatic React Router pattern for route-level configuration.
34-37: Verify the tag detail route should be allowed during force upgrade.The
/tags/:tagSlugroute hasallowInForceUpgrade: true, but the comment suggests this is for Ember delegation purposes rather than intentional force-upgrade access. Should users be able to access tag detail pages during force upgrade mode, or should this route be blocked?Based on PR objectives, only Settings and /pro should be accessible. If tag details should be blocked, remove the
handleproperty.
jonatansberg
left a comment
jonatansberg
left a comment
There was a problem hiding this comment.
Overall this looks good. I added a few comments on some minor things that we might want to look at before merging.
| isActiveTrial?: boolean; | ||
| trial_end?: string | null; | ||
| status?: string; |
There was a problem hiding this comment.
Nit: Are these all individually optional, or is it actually the full object that should be optional?
| const configForceUpgrade = config?.config?.hostSettings?.forceUpgrade; | ||
|
|
||
| // If config doesn't have forceUpgrade, we're not in force upgrade mode | ||
| if (!configForceUpgrade) { |
There was a problem hiding this comment.
Thought: Maybe redundant, but would it make sense to also check if billing is enabled?
| { | ||
| path: `settings/*`, | ||
| element: <Settings />, | ||
| handle: { allowInForceUpgrade: true } as RouteHandle, |
There was a problem hiding this comment.
Can't we fix the RouteObject rather than assert here?
| @@ -0,0 +1,152 @@ | |||
| import {NAV_ITEMS, SidebarPage} from '@/helpers/pages'; | |||
There was a problem hiding this comment.
Nit: shouldn't this be a billing test rather than a sidebar test?
| return billingIframe; | ||
| } | ||
|
|
||
| test.describe('sidebar navigation shows billing iframe', () => { |
There was a problem hiding this comment.
Is this meant to test the redirect? It's not the sidebar that shows the iframe, no?
| this.stateBridge.triggerSubscriptionChange({ | ||
| subscription: this.billing.subscription || {} | ||
| }); |
There was a problem hiding this comment.
Non blocking question: Is this needed? I assumed that iframe would load way after the bridge has been initialized, but maybe that's a naive assumption.
| // Check if any matched route allows access in force upgrade mode | ||
| const isAllowed = matches.some((match) => { | ||
| const handle = match.handle as RouteHandle | undefined; | ||
| return handle?.allowInForceUpgrade === true; |
2 participants
ref https://linear.app/ghost/issue/BER-3127
Summary
What is Force Upgrade mode?
Force Upgrade is a Ghost(Pro) billing feature that locks users out of most of the admin when a subscription action is required (e.g., payment failed, plan expired). In this mode, users are redirected to the
/probilling page, with only settings and sign-out remaining accessible.The Bug
The force upgrade protection was implemented in Ember's routing layer, which intercepts route transitions and redirects to
/pro. However, as we've migrated routes to React (Analytics, Network/ActivityPub, Tags), these routes bypassed the Ember protection entirely - React was unaware of the force upgrade state, so these pages would render normally when they should have been blocked.The Fix
We chose a declarative approach using React Router's
handlemetadata:hostSettings.forceUpgradeflag is set by the server and available via the config endpointforceUpgrade: truebut subscription status isactive, billing was just completed (server hasn't restarted yet), so we clear the lock locally. This mirrors Ember's logic.handle: { allowInForceUpgrade: true }to bypass the guard (used for settings and the Ember fallback which handles/pro)This approach mirrors how Ember handles it (checking route transitions) but uses React Router's native patterns.