🐛 Fixed upgrade button in trial banner not navigating to checkout #25674

rob-ghost · 2025-12-09T16:43:45Z

fixes https://linear.app/ghost/issue/BER-3122

The "Upgrade now" button in the React admin sidebar's trial upgrade banner had no click handler. Added navigation to #/pro?action=checkout to align with the Ember implementation.

coderabbitai · 2025-12-09T16:43:53Z

Walkthrough

This PR updates the upgrade banner component to navigate to a billing plans page by replacing a button action with an anchor link. It extends E2E test infrastructure with new page object locators for the upgrade banner, updates the test configuration fixture to support billing settings with optional fields, introduces a new E2E test validating the upgrade banner navigation flow, and adds a non-null assertion to an existing test for email inbox configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Button-to-anchor refactor in UpgradeBanner.tsx should be verified for proper link semantics and accessibility
New E2E test mocking strategy in upgrade-banner.test.ts (billing app route handler and HTML payload) should be reviewed for correctness
Configuration type changes in fixture.ts need verification that optional fields don't break existing test flows

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the bug fix of making the upgrade button navigate to checkout, matching the main change in the PR.
Description check	✅ Passed	The description accurately explains the issue, references the Linear ticket, and describes the solution of adding navigation to the billing route.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/upgrade-button-does-nothing

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-12-09T17:46:36Z

Ember E2E Tests Failed

To view the Playwright test report locally, run:

REPORT_DIR=$(mktemp -d) && gh run download 20071636694 -n playwright-report-ember -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR"

github-actions · 2025-12-09T17:46:38Z

React E2E Tests Failed

To view the Playwright test report locally, run:

REPORT_DIR=$(mktemp -d) && gh run download 20071636694 -n playwright-report-react -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR"

fixes https://linear.app/ghost/issue/BER-3122 The "Upgrade now" button in the React admin sidebar's trial upgrade banner had no click handler. This adds navigation to the pro checkout route, matching the behavior of the Ember implementation.

cursor · 2025-12-10T10:04:05Z

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 3.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

e2e/helpers/playwright/fixture.ts (1)

17-22: GhostConfig optional fields and billing keys look consistent; consider updating docs

Making the welcome-email fields optional and adding the two billing-related keys as optional strings fits the existing config pattern and aligns with how tests opt-in via test.use({config: {...}}). To keep the fixture self-documenting, you might extend the comment around lines 75–78 to also mention hostSettings__billing__enabled and hostSettings__billing__url as supported options.

e2e/tests/admin/sidebar/upgrade-banner.test.ts (1)

1-52: Well-structured e2e covers upgrade flow; consider aligning test name with route

The test setup looks solid: it enables billing via the shared config fixture, mocks the external billing app with page.route, uses SidebarPage for interactions, and follows the AAA pattern with a single, focused scenario asserting navigation to #/pro/billing/plans. That’s all in line with the e2e ADRs.

Minor nit: the test name mentions “pro checkout route” while the assertion checks the #/pro/billing/plans URL. For clarity and future grepping, you might rename it to something like upgrade now button - navigates to pro billing plans route.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 28f2384 and 4896f83.

📒 Files selected for processing (5)

apps/admin/src/layout/app-sidebar/UpgradeBanner.tsx (1 hunks)
e2e/helpers/pages/admin/sidebar/sidebar-page.ts (2 hunks)
e2e/helpers/playwright/fixture.ts (1 hunks)
e2e/tests/admin/sidebar/upgrade-banner.test.ts (1 hunks)
e2e/tests/public/member-signup.test.ts (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/fixture.ts
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
e2e/tests/public/member-signup.test.ts
e2e/tests/admin/sidebar/upgrade-banner.test.ts

e2e/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

E2E tests should use Playwright with Docker container isolation

Files:

e2e/helpers/playwright/fixture.ts
e2e/helpers/pages/admin/sidebar/sidebar-page.ts
e2e/tests/public/member-signup.test.ts
e2e/tests/admin/sidebar/upgrade-banner.test.ts

e2e/helpers/pages/**/*.ts

📄 CodeRabbit inference engine (e2e/AGENTS.md)

e2e/helpers/pages/**/*.ts: Page Objects should be located in helpers/pages/ directory
Expose locators as public readonly in Page Objects when used with assertions
Page Object methods should use semantic names (e.g., login() instead of clickLoginButton())
Use waitFor() for guards in Page Objects, never use expect() in page objects

Files:

e2e/helpers/pages/admin/sidebar/sidebar-page.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
Use getByTestId() only when semantic locators are unavailable, and suggest adding data-testid to 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 like waitForTimeout()
Never use networkidle in waits
Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Use test.only() for debugging single tests
Manual login should not be used - authentication is automatic via fixture

Files:

e2e/tests/public/member-signup.test.ts
e2e/tests/admin/sidebar/upgrade-banner.test.ts

🧠 Learnings (14)

📚 Learning: 2025-06-10T11:07:10.800Z

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.

Applied to files:

e2e/helpers/playwright/fixture.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

📚 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-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: Applies to ghost/i18n/locales/en/ghost.json : Add UI translations to `ghost/i18n/locales/en/ghost.json` for Admin UI features

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 : 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-09-11T20:38:41.537Z

Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24891
File: e2e/helpers/pages/portal/SignUpSuccessPage.ts:61-67
Timestamp: 2025-09-11T20:38:41.537Z
Learning: In Playwright, page.$() is deprecated and should be replaced with page.locator(). When checking element visibility with Locator.isVisible(), the method returns false both when the element doesn't exist and when it exists but isn't visible, eliminating the need for separate null checks that were required with ElementHandle.

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 : Manual login should not be used - authentication is automatic via fixture

Applied to files:

e2e/tests/public/member-signup.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 : Test suite names should follow the format 'Ghost Admin - Feature' or 'Ghost Public - Feature'

Applied to files:

e2e/tests/admin/sidebar/upgrade-banner.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 : Always follow ADRs in `../adr/` folder (ADR-0001: AAA pattern, ADR-0002: Page Objects)

Applied to files:

e2e/tests/admin/sidebar/upgrade-banner.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/upgrade-banner.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 : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability

Applied to files:

e2e/tests/admin/sidebar/upgrade-banner.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 : Use factory pattern for all test data creation instead of hard-coded data or direct database manipulation

Applied to files:

e2e/tests/admin/sidebar/upgrade-banner.test.ts

📚 Learning: 2025-11-06T05:35:41.162Z

Learnt from: danielraffel
Repo: TryGhost/Ghost PR: 25366
File: apps/admin/src/layout/app-sidebar/NavHeader.tsx:13-23
Timestamp: 2025-11-06T05:35:41.162Z
Learning: In apps/admin/src/layout/app-sidebar/NavHeader.tsx, the React component dispatches a synthetic KeyboardEvent to trigger the Ember keymaster.js search modal shortcut. This approach is known to have cross-browser reliability issues but was deferred for architectural refactoring in a separate PR. The recommended fix is to expose a global function or custom DOM event from the Ember app instead of relying on synthetic keyboard events with keymaster.js.

Applied to files:

apps/admin/src/layout/app-sidebar/UpgradeBanner.tsx

🧬 Code graph analysis (2)

e2e/tests/public/member-signup.test.ts (5)

ghost/core/core/server/services/member-welcome-emails/service.js (1)

config (5-5)

ghost/core/test/integration/jobs/process-outbox.test.js (1)

config (10-10)

ghost/core/test/integration/services/member-welcome-emails.test.js (1)

config (10-10)

ghost/core/test/unit/server/services/members/members-api/repositories/MemberRepository.test.js (1)

config (7-7)

ghost/core/core/server/services/members/members-api/repositories/MemberRepository.js (1)

config (11-11)

e2e/tests/admin/sidebar/upgrade-banner.test.ts (1)

e2e/helpers/pages/admin/sidebar/sidebar-page.ts (1)

SidebarPage (12-85)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

GitHub Check: E2E Tests (Ember 4/4)
GitHub Check: [Optional] E2E Tests (React 4/4)
GitHub Check: E2E Tests (Ember 1/4)
GitHub Check: [Optional] E2E Tests (React 2/4)
GitHub Check: E2E Tests (Ember 2/4)
GitHub Check: [Optional] E2E Tests (React 1/4)
GitHub Check: [Optional] E2E Tests (React 3/4)
GitHub Check: E2E Tests (Ember 3/4)
GitHub Check: Unit tests (Node 22.18.0)

🔇 Additional comments (3)

e2e/tests/public/member-signup.test.ts (1)

51-56: Non‑null assertion matches updated GhostConfig optionality

Using config!.memberWelcomeEmailTestInbox! is consistent with GhostConfig now marking this property as optional and with the describe-level test.use({config: {…}}) guaranteeing it for this suite. This keeps the type-checker happy while still making any missing config an explicit test setup error.

If you ever refactor this suite to remove or change the test.use({config: {...}}) block, double‑check this line so it fails with a clear message rather than a cryptic runtime error.

apps/admin/src/layout/app-sidebar/UpgradeBanner.tsx (1)

8-21: Anchor-based upgrade action fixes the missing navigation; confirm route parity

Switching to <Button asChild><a href="#/pro/billing/plans">…</a></Button> gives the upgrade control a concrete navigation target while preserving button styling, and it lines up with the new e2e assertion on #/pro/billing/plans. There is a slight mismatch with the PR description mentioning #/pro?action=checkout, so it’s worth double-checking that this hash route is exactly what the Ember sidebar uses and what billing expects long-term.

e2e/helpers/pages/admin/sidebar/sidebar-page.ts (1)

21-42: New sidebar locators are semantic and future-proof across Ember/React

Defining ghostProLink and upgradeNowLink as public readonly locators on the page object fits the helpers guidelines, and using getByRole with accessible names (plus the .or(...) composition for link/button) is a nice way to keep tests stable while both Ember and React variants exist.

ibalosh · 2025-12-10T10:15:43Z

e2e/tests/admin/sidebar/upgrade-banner.test.ts

+    });
+
+    test('upgrade now button navigates to pro checkout route', async ({page}) => {
+        // Arrange


I think we can kill these comments (arrange act..)

rob-ghost force-pushed the fix/upgrade-button-does-nothing branch 2 times, most recently from 8b433e7 to 61af6e7 Compare December 9, 2025 16:56

rob-ghost requested review from jonatansberg and kevinansfield December 9, 2025 17:14

rob-ghost force-pushed the fix/upgrade-button-does-nothing branch 3 times, most recently from 129c1ed to 080aac2 Compare December 10, 2025 09:55

rob-ghost force-pushed the fix/upgrade-button-does-nothing branch from 080aac2 to 4896f83 Compare December 10, 2025 10:00

rob-ghost marked this pull request as ready for review December 10, 2025 10:04

rob-ghost requested review from 9larsons, cmraible and ibalosh as code owners December 10, 2025 10:04

coderabbitai bot reviewed Dec 10, 2025

View reviewed changes

ibalosh approved these changes Dec 10, 2025

View reviewed changes

rob-ghost merged commit 5cc0f4c into main Dec 10, 2025
35 checks passed

rob-ghost deleted the fix/upgrade-button-does-nothing branch December 10, 2025 10:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

🐛 Fixed upgrade button in trial banner not navigating to checkout #25674

🐛 Fixed upgrade button in trial banner not navigating to checkout #25674

Uh oh!

rob-ghost commented Dec 9, 2025

Uh oh!

coderabbitai bot commented Dec 9, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 9, 2025

Uh oh!

github-actions bot commented Dec 9, 2025

Uh oh!

cursor bot commented Dec 10, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

ibalosh Dec 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

🐛 Fixed upgrade button in trial banner not navigating to checkout #25674

🐛 Fixed upgrade button in trial banner not navigating to checkout #25674

Uh oh!

Conversation

rob-ghost commented Dec 9, 2025

Uh oh!

coderabbitai bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

github-actions bot commented Dec 9, 2025

Ember E2E Tests Failed

Uh oh!

github-actions bot commented Dec 9, 2025

React E2E Tests Failed

Uh oh!

cursor bot commented Dec 10, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

ibalosh Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Dec 9, 2025 •

edited

Loading