Changed from Stripe checkout to Stripe billing portal #25887
Conversation
WalkthroughRenames frontend billing action from Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/portal/src/utils/api.js (1)
565-577: Fix Billing Portal redirect to use session URL instead of Checkout method.The endpoint creates a Stripe Billing Portal session, which returns a URL to redirect to, not Checkout session credentials. The current code attempts to extract
result.publicKeyandresult.sessionId(which don't exist in a Portal response) and callsStripe.redirectToCheckout(), which is incorrect for this flow. Additionally, the error message incorrectly references "checkout session" instead of "billing portal session."Redirect via
result.urlto the portal session, or add fallback logic for legacy Checkout responses:Suggested fix
}).then(function (res) { if (!res.ok) { - throw new Error('Unable to create stripe checkout session'); + throw new Error('Unable to create Stripe billing portal session'); } return res.json(); }).then(function (result) { - const stripe = window.Stripe(result.publicKey); - return stripe.redirectToCheckout({ - sessionId: result.sessionId - }); - }).then(function (result) { - if (result.error) { - throw new Error(result.error.message); - } + if (result.url) { + window.location.assign(result.url); + return; + } + // Fallback for legacy checkout-session responses + if (result.publicKey && result.sessionId && window.Stripe) { + const stripe = window.Stripe(result.publicKey); + return stripe.redirectToCheckout({sessionId: result.sessionId}); + } + throw new Error('Billing portal session URL missing'); + }).then(function (redirectResult) { + if (redirectResult?.error) { + throw new Error(redirectResult.error.message); + } }).catch(function (err) { throw err; });
🤖 Fix all issues with AI agents
In
`@apps/portal/src/components/pages/AccountHomePage/components/paid-account-actions.js`:
- Around line 116-118: The loading check uses the old action string
'editBilling:running' so the spinner never appears; update the condition that
sets label (where action is compared) to check for the renamed action
'manageBilling:running' (i.e., replace the 'editBilling:running' literal with
'manageBilling:running') so the LoaderIcon in paid-account-actions.js renders
when the manageBilling action is running.
In
`@ghost/core/core/server/services/members/members-api/controllers/router-controller.js`:
- Around line 229-238: The code in router-controller.js accesses
subscription.get('customer_id') without checking if subscription is found, which
can throw when req.body.subscription_id is provided but no match exists; update
the handler (same method that contains the shown diff) to check if subscription
is undefined/null after const subscription = subscriptions.models.find(...) and,
if missing, return the same 404-style response used by
createCheckoutSetupSession (so do not call this._stripeAPIService.getCustomer
when subscription is falsy), otherwise proceed to call
this._stripeAPIService.getCustomer(subscription.get('customer_id')).
🧹 Nitpick comments (3)
ghost/core/core/server/web/members/app.js (1)
105-107: Missing semicolon at the end of the statement.Line 107 is missing a trailing semicolon, unlike the other route definitions in this file (e.g., line 104).
Suggested fix
membersApp.post('/api/create-stripe-billing-portal-session', function lazyCreateBillingPortalSessionMw(req, res, next) { return membersService.api.middleware.createBillingPortalSession(req, res, next); - }) + });ghost/core/core/server/services/stripe/stripe-api.js (1)
703-719: Incorrect JSDoc description.The JSDoc comment says "Create a new Stripe Checkout Setup Session" but this method creates a Billing Portal Session.
Proposed fix
/** - * Create a new Stripe Checkout Setup Session. + * Create a new Stripe Billing Portal Session. * * `@param` {ICustomer} customer * `@param` {object} options * `@param` {string} options.returnUrl * `@returns` {Promise<IBillingSession>} */ghost/core/core/server/services/members/members-api/controllers/router-controller.js (1)
207-213: Redundant identity check.The condition
if (!identity)at line 208 is dead code since the early return at lines 201-204 already handles this case. The same pattern exists increateCheckoutSetupSession.Simplified version
let email; try { - if (!identity) { - email = null; - } else { - const claims = await this._tokenService.decodeToken(identity); - email = claims && claims.sub; - } + const claims = await this._tokenService.decodeToken(identity); + email = claims && claims.sub; } catch (err) { logging.error(err); res.writeHead(401); return res.end('Unauthorized'); }
apps/portal/src/components/pages/AccountHomePage/components/paid-account-actions.js
Outdated
Show resolved
Hide resolved
ghost/core/core/server/services/members/members-api/controllers/router-controller.js
Show resolved
Hide resolved
sam-lord
force-pushed
the
billing-portal
branch
from
41559d1 to
af52f69
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/portal/src/utils/api.js (1)
544-577: Redirect to billing portal URL; current Stripe Checkout flow will break.
createBillingPortalSessionreturns a portal URL, not a Checkout session. The currentredirectToCheckoutpath expectspublicKey/sessionId, so this will fail at runtime.🐛 Proposed fix
}).then(function (res) { if (!res.ok) { - throw new Error('Unable to create stripe checkout session'); + throw new Error('Unable to create billing portal session'); } return res.json(); }).then(function (result) { - const stripe = window.Stripe(result.publicKey); - return stripe.redirectToCheckout({ - sessionId: result.sessionId - }); - }).then(function (result) { - if (result.error) { - throw new Error(result.error.message); - } - }).catch(function (err) { - throw err; - }); + if (!result?.url) { + throw new Error('Billing portal URL missing from response'); + } + window.location.assign(result.url); + });
♻️ Duplicate comments (1)
ghost/core/core/server/services/members/members-api/controllers/router-controller.js (1)
233-238: Add missing subscription-not-found handling before accessingcustomer_id.If the provided
subscription_iddoesn’t match a record,subscriptionisundefinedandsubscription.get(...)throws, producing a 500 instead of a clean 404. Mirror the existing 404 branch used increateCheckoutSetupSession.🐛 Proposed fix
const subscription = subscriptions.models.find((sub) => { return sub.get('subscription_id') === req.body.subscription_id; }); + if (!subscription) { + res.writeHead(404, { + 'Content-Type': 'text/plain;charset=UTF-8' + }); + return res.end(`Could not find subscription ${req.body.subscription_id}`); + } + customer = await this._stripeAPIService.getCustomer(subscription.get('customer_id'));
sam-lord
force-pushed
the
billing-portal
branch
from
af52f69 to
8c78bf5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@ghost/i18n/locales/hu/portal.json`:
- Line 77: The translation for the portal error key "Failed to open billing
portal, please try again" is missing; add a Hungarian translation using
payment-focused wording (prefer "fizetési" for payment-related errors) to the
value for that key in portal.json so the error text is displayed (update the
entry for "Failed to open billing portal, please try again"). Ensure the
translated string reads naturally in Hungarian and fits the UX context of a
billing/payment portal error.
In `@ghost/i18n/locales/uz/portal.json`:
- Line 77: The key "Failed to open billing portal, please try again" in
ghost/i18n/locales/uz/portal.json is missing an Uzbek translation; update that
JSON entry by replacing the empty string value with the Uzbek translation (for
example: "Billing portalni ochishda xatolik yuz berdi, iltimos qayta urinib
ko‘ring") so the localization returns Uzbek text instead of falling back to the
default.
| const member = email ? await this._memberRepository.get({email}) : null; | ||
|
|
||
| if (!member) { | ||
| res.writeHead(403); |
There was a problem hiding this comment.
I assume we intentionally return a 403 instead of a 404 here?
| "Back": "Terug", | ||
| "Back to Log in": "Terug na aanmelding", | ||
| "Billing info": "Rekening inligting", | ||
| "Billing info & receipts": "", |
There was a problem hiding this comment.
Having this string empty will now fall back to English, if I understand correctly. Does this mean that members on a translated site will see this as mixed content? (question applies to all updated translation strings) that now have no content in their respective language.
There was a problem hiding this comment.
Yeah that's my understanding.
There was a problem hiding this comment.
I also think that's the intention with these string changes. When we make changes to user facing strings, it needs to be translated again.
aileen
left a comment
aileen
left a comment
There was a problem hiding this comment.
I left a comment regarding the translation string, which will appear in Portal. I'm just a bit concerned that we remove already translated strings and will replace them with fallback English. But I also don't see another way of solving this 🤔
sam-lord
force-pushed
the
billing-portal
branch
from
b95afe3 to
aa0ff6b
Compare
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 (1)
apps/portal/src/utils/api.js (1)
544-571: Pass the current page location asreturnUrlin themanageBillingcall, or update the fallback to use the referring page instead of site root.The call at
apps/portal/src/components/pages/AccountHomePage/components/paid-account-actions.js:14only passessubscriptionIdand omitsreturnUrl, which causes users to be redirected to the site root (withstripe=billing-portal-closed) after closing the billing portal instead of returning to their original page. Either pass the current location asreturnUrlor update the fallback to detect and use the referring page for same-origin navigation.
🤖 Fix all issues with AI agents
In `@ghost/i18n/locales/zh-Hant/portal.json`:
- Line 77: The translation for the message key "Failed to open billing portal,
please try again" is empty; update the zh-Hant locale entry by replacing the
empty string with a Traditional Chinese translation (e.g. "無法開啟帳單入口，請再試一次") so
zh-Hant users see the localized error message.
| "Failed to cancel subscription, please try again": "取消訂閱失敗，請再試一次", | ||
| "Failed to log in, please try again": "登入失敗，請再試一次", | ||
| "Failed to log out, please try again": "登出失敗，請再試一次", | ||
| "Failed to open billing portal, please try again": "", |
There was a problem hiding this comment.
Missing Traditional Chinese translation for error message.
The new error message key has an empty translation value. This will display English text to zh-Hant users when the billing portal fails to open.
Suggested translation (following the pattern of similar error messages in this file):
- "Failed to open billing portal, please try again": "",
+ "Failed to open billing portal, please try again": "無法開啟帳單管理頁面，請再試一次",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Failed to open billing portal, please try again": "", | |
| "Failed to open billing portal, please try again": "無法開啟帳單管理頁面，請再試一次", |
🤖 Prompt for AI Agents
In `@ghost/i18n/locales/zh-Hant/portal.json` at line 77, The translation for the
message key "Failed to open billing portal, please try again" is empty; update
the zh-Hant locale entry by replacing the empty string with a Traditional
Chinese translation (e.g. "無法開啟帳單入口，請再試一次") so zh-Hant users see the localized
error message.
sam-lord
force-pushed
the
billing-portal
branch
from
aa0ff6b to
632a926
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/portal/src/utils/api.js`:
- Around line 569-571: The code currently does
window.location.assign(result.url) which will navigate to "undefined" if
result.url is missing; update the promise handler (the function that receives
result) to first validate that result && result.url is a non-empty string and
only call window.location.assign(result.url) when valid, otherwise reject/throw
a clear error (e.g., throw new Error("Missing billing portal URL in response")
or return Promise.reject(...)) so callers can surface a readable message instead
of redirecting to "undefined".
🧹 Nitpick comments (2)
ghost/i18n/locales/de-CH/portal.json (1)
26-26: New translation keys added with empty values.Both new keys (
"Billing info & receipts"and"Failed to open billing portal, please try again") have empty string values. This is expected in the i18n workflow, but the UI will display English text for Swiss German users until translations are provided.Suggested translations for de-CH:
"Billing info & receipts":"Rechnungsinformationen & Belege""Failed to open billing portal, please try again":"Das Abrechnungsportal konnte nicht geöffnet werden. Bitte versuchen Sie es erneut."Also applies to: 77-77
ghost/i18n/locales/context.json (1)
29-29: Consider adding context descriptions for translator clarity.Both new translation keys have empty context descriptions, while similar entries in this file include brief descriptions (e.g.,
"error message"for other failure messages, or"A label in Portal..."for UI elements). Adding context helps translators understand where and how these strings appear.📝 Suggested context descriptions
- "Billing info & receipts": "", + "Billing info & receipts": "A label in Portal for the billing management section",- "Failed to open billing portal, please try again": "", + "Failed to open billing portal, please try again": "error message",Also applies to: 100-100
| }).then(function (result) { | ||
| const stripe = window.Stripe(result.publicKey); | ||
| return stripe.redirectToCheckout({ | ||
| sessionId: result.sessionId | ||
| }); | ||
| }).then(function (result) { | ||
| if (result.error) { | ||
| throw new Error(result.error.message); | ||
| } | ||
| return window.location.assign(result.url); | ||
| }).catch(function (err) { |
There was a problem hiding this comment.
Guard against missing billing portal URL.
If the API returns a success payload without url, this will redirect to "undefined". Add a defensive check and surface a clear error instead.
💡 Suggested fix
- }).then(function (result) {
- return window.location.assign(result.url);
- }).catch(function (err) {
+ }).then(function (result) {
+ if (!result?.url) {
+ throw new Error('Stripe billing portal URL was not returned');
+ }
+ return window.location.assign(result.url);
+ }).catch(function (err) {
throw err;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }).then(function (result) { | |
| const stripe = window.Stripe(result.publicKey); | |
| return stripe.redirectToCheckout({ | |
| sessionId: result.sessionId | |
| }); | |
| }).then(function (result) { | |
| if (result.error) { | |
| throw new Error(result.error.message); | |
| } | |
| return window.location.assign(result.url); | |
| }).catch(function (err) { | |
| }).then(function (result) { | |
| if (!result?.url) { | |
| throw new Error('Stripe billing portal URL was not returned'); | |
| } | |
| return window.location.assign(result.url); | |
| }).catch(function (err) { |
🤖 Prompt for AI Agents
In `@apps/portal/src/utils/api.js` around lines 569 - 571, The code currently does
window.location.assign(result.url) which will navigate to "undefined" if
result.url is missing; update the promise handler (the function that receives
result) to first validate that result && result.url is a non-empty string and
only call window.location.assign(result.url) when valid, otherwise reject/throw
a clear error (e.g., throw new Error("Missing billing portal URL in response")
or return Promise.reject(...)) so callers can surface a readable message instead
of redirecting to "undefined".
React E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 21254990929 -n playwright-report-react -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
1 similar comment
React E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 21254990929 -n playwright-report-react -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
|
The error seems like something unrelated in the canary build :) |
ref FEA-502
Added members API for creating the Stripe billing portal session & switched the front-end to use that API.