feat(website): add payment success and failed pages#11855
feat(website): add payment success and failed pages#11855christian-byrne wants to merge 6 commits intomainfrom
Conversation
The Stripe checkout flow in comfy-api redirects to https://www.comfy.org/payment/{success,failed} after a checkout session, but those pages were never migrated from Framer (they weren't listed in the sitemap). Users currently hit a 404 after completing or canceling a Stripe checkout. Recreate both pages on the Astro marketing site using the new design system: a centered hero-style status panel with brand iconography, SectionLabel, and BrandButton CTAs. The success page links to the platform dashboard and back to the marketing home; the failed page links to the platform dashboard (to retry) and to /contact. URL path note: matches the production Stripe config in comfy-api/run-service-{prod,staging}.yaml (/payment/failed, not /payment/failure).
- Point primary CTA at platform.comfy.org/profile/api-keys via
externalLinks.apiKeys (the platform root is just a redirect; this
is the actual authenticated landing page)
- Exclude /payment/{success,failed} from the generated sitemap, since
these are post-checkout redirect targets and already noindex
- Drop client:load on the static PaymentStatusSection \u2014 no client JS
needed
- Even out e2e coverage: assert noindex on every payment page and
verify the zh-CN failed primary CTA
📝 WalkthroughWalkthroughAdds localized payment success and failure pages (EN and zh-CN), a client-side PaymentStatusSection Vue component, translation keys, sitemap exclusions and robots rule for payment routes, and Playwright smoke tests asserting UI, CTAs, and noindex behavior. ChangesPayment status pages, component, translations, sitemap, and tests
sequenceDiagram
participant User
participant Browser
participant AstroPage as Astro Page
participant Component as PaymentStatusSection
participant Translations as i18n/translations
participant Sitemap as Astro sitemap filter
User->>Browser: Navigate to /payment/success
Browser->>AstroPage: Request page
AstroPage->>Component: Mount with props (status, locale)
Component->>Translations: request localized strings
Translations-->>Component: return strings
Component->>Browser: render icons, headings, CTAs
Browser->>User: display page
Note right of Sitemap: Sitemap filter excludes payment paths during build
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🌐 Website E2ETip All tests passed.
🔗 Website PreviewWebsite Preview: https://comfy-website-preview-pr-11855.vercel.app This commit: https://website-frontend-2fyoi2l4x-comfyui.vercel.app Last updated: 2026-05-04T07:17:15Z for |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/website/e2e/payment.spec.ts (1)
15-110: ⚡ Quick winKeep the smoke suite focused on rendering, not route targets.
These
@smoketests pin exacthrefdestinations for both CTAs across locales, which makes the suite brittle and couples it to routing details. Move the destination checks to a non-smoke integration/component test and keep smoke coverage limited to title,noindex, and visible copy.✂️ Suggested split
test('primary CTA links to platform dashboard', async ({ page }) => { const cta = page.getByRole('link', { name: /GO TO DASHBOARD/i }) await expect(cta).toBeVisible() - await expect(cta).toHaveAttribute('href', PLATFORM_DASHBOARD_URL) }) @@ test('secondary CTA links back to home', async ({ page }) => { const cta = page.getByRole('link', { name: /BACK TO HOME/i }) await expect(cta).toBeVisible() - await expect(cta).toHaveAttribute('href', '/') })[review_comment_end -->
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/e2e/payment.spec.ts` around lines 15 - 110, The smoke tests in payment.spec.ts are asserting exact CTA hrefs which couples rendering smoke tests to routing; remove href assertions from the `@smoke` suites and keep only title, noindex, and visible copy checks. Specifically, in the "Payment success page `@smoke`", "Payment failed page `@smoke`", and "Payment pages zh-CN `@smoke`" tests drop the await expect(...).toHaveAttribute('href', ...) lines for the CTAs (the tests that reference PLATFORM_DASHBOARD_URL, '/', '/contact', '/zh-CN/', and '/zh-CN/contact') but keep the getByRole(...).toBeVisible() checks; then add separate non-@smoke integration tests (e.g., new tests named like "Payment CTA destinations" or similar) that assert the exact href targets for those links so routing assertions live outside the smoke suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/website/e2e/payment.spec.ts`:
- Around line 15-110: The smoke tests in payment.spec.ts are asserting exact CTA
hrefs which couples rendering smoke tests to routing; remove href assertions
from the `@smoke` suites and keep only title, noindex, and visible copy checks.
Specifically, in the "Payment success page `@smoke`", "Payment failed page
`@smoke`", and "Payment pages zh-CN `@smoke`" tests drop the await
expect(...).toHaveAttribute('href', ...) lines for the CTAs (the tests that
reference PLATFORM_DASHBOARD_URL, '/', '/contact', '/zh-CN/', and
'/zh-CN/contact') but keep the getByRole(...).toBeVisible() checks; then add
separate non-@smoke integration tests (e.g., new tests named like "Payment CTA
destinations" or similar) that assert the exact href targets for those links so
routing assertions live outside the smoke suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 51fd7f66-558d-45a6-af00-246cf241c401
📒 Files selected for processing (8)
apps/website/astro.config.tsapps/website/e2e/payment.spec.tsapps/website/src/components/payment/PaymentStatusSection.vueapps/website/src/i18n/translations.tsapps/website/src/pages/payment/failed.astroapps/website/src/pages/payment/success.astroapps/website/src/pages/zh-CN/payment/failed.astroapps/website/src/pages/zh-CN/payment/success.astro
|
Pushing back on this one. The existing
For these payment status pages the CTA destination is the whole point — sending a user to |
Payment doesn't actually happen on platform.comfy.org \u2014 that's just the usage / API key dashboard. The real flow is: ComfyUI workspace (app.comfy.org) \u2192 Buy Credits dialog \u2192 Stripe (new tab) \u2192 www.comfy.org/payment/{success,failed} Update CTAs to match: - Success: "Continue in ComfyUI" (origin-aware via document.referrer with allowlist for app.comfy.org / stagingapp.comfy.org, fallback to app.comfy.org) + "View balance & usage" \u2192 platform.comfy.org. - Failed: "Contact support" \u2192 /contact, "Read billing docs" \u2192 docs.comfy.org/api-reference/cloud, "Close tab" \u2192 window.close(). Add `app` to externalLinks and a small `resolveAppOrigin` helper in config/routes.ts. Pages re-hydrate with client:load for the referrer detection. Tests + translations updated for both locales.
|
Reworked the CTAs after a clarification: payment doesn't actually happen on Success page
Failed page
Implementation notes
Screenshots |
dosubot
Bot
added
the
size:L
|
@Comfy-Org/comfy_frontend_devs @DrJKL ready for review — CI green, CodeRabbit approved. Recreates Framer-era |
DrJKL
left a comment
DrJKL
left a comment
There was a problem hiding this comment.
Solid, focused PR. A few inline notes — main concern is that the document.referrer allowlist in resolveAppOrigin likely never matches in the real Stripe Checkout redirect flow (referrer becomes checkout.stripe.com or empty under modern Referrer-Policy), making the logic effectively dead. Worth either dropping it or routing the source via a query param / Stripe session metadata. Also a few low-risk nits inline.
Open questions:
- ❓ Is Stripe
session_idverification handled elsewhere (webhooks)? If so, a one-line comment on the page explaining that this is a thank-you page only would help future readers. - ❓
noindexmeta is set and sitemap excludes these paths — should they also be inrobots.txt? Direct/accidental links remain crawlable until Googlebot hits them.
| const appOrigin = ref(externalLinks.app) | ||
| onMounted(() => { | ||
| if (status === 'success' && typeof document !== 'undefined') { | ||
| appOrigin.value = resolveAppOrigin(document.referrer) |
There was a problem hiding this comment.
🟡 risk: document.referrer after a Stripe Checkout redirect is https://checkout.stripe.com/ (or empty under Referrer-Policy: strict-origin-when-cross-origin), never app.comfy.org. With the allowlist in resolveAppOrigin, the success branch falls through to externalLinks.app in practice — making this whole ref/onMounted dance effectively dead code in the real flow.
Consider one of:
- Pass the source via query param:
?from=app/?from=stagingappand resolve from that. - Stash the origin in
sessionStoragebefore the redirect to Stripe and read it here. - Drop the logic and always link to
externalLinks.app.
There was a problem hiding this comment.
Good catch — you're right. After a Stripe Checkout redirect, document.referrer is https://checkout.stripe.com/ (or empty under Referrer-Policy: strict-origin-when-cross-origin), so the allowlist always misses and the whole onMounted dance is dead code in the real flow. I'd added it speculatively without testing the actual redirect.
Dropped it. Primary success CTA is now a static link to externalLinks.app, and the success pages are back to being fully static (no client:load). The query-param / sessionStorage approaches are real options if we ever need per-origin routing back, but for the current single-origin flow (app.comfy.org → Stripe → here) the static link is the right answer.
| }) | ||
|
|
||
| function handleCloseTab() { | ||
| if (typeof window !== 'undefined') window.close() |
There was a problem hiding this comment.
🟡 risk: window.close() silently no-ops unless the tab was opened by window.open() from script — which is not how users typically arrive after a Stripe redirect. Most users will click CLOSE TAB and see nothing happen.
Either remove the tertiary CTA or add a fallback navigation, e.g.:
function handleCloseTab() {
if (typeof window === 'undefined') return
window.close()
// window.close() is a no-op for tabs not script-opened; navigate home as a fallback.
setTimeout(() => { if (!window.closed) window.location.href = '/' }, 100)
}There was a problem hiding this comment.
Applied the suggested fallback exactly as written, with a brief comment explaining why. Users who land on this page after Stripe redirects them through the popup chain should be able to window.close() (the original window.open(checkout_url, '_blank') is in useAuthActions.ts:115), but the opener relationship through Stripe isn't reliable, so the setTimeout fallback to / covers the case where it no-ops.
|
|
||
| const routes = getRoutes(locale) | ||
|
|
||
| const appOrigin = ref(externalLinks.app) |
There was a problem hiding this comment.
🔵 nit: appOrigin is only meaningful on the success branch and is used as the href of the primary CTA. successCtaHref would describe intent better and avoid the implication that it's a generic origin string.
There was a problem hiding this comment.
Resolved by removing the ref entirely along with the referrer logic — the success primary CTA is now a static externalLinks.app link in the template.
| const SITEMAP_EXCLUDED_PATHNAMES = new Set([ | ||
| '/payment/success', | ||
| '/payment/failed', | ||
| '/zh-CN/payment/success', | ||
| '/zh-CN/payment/failed' | ||
| ]) |
There was a problem hiding this comment.
🔵 nit: hardcoding both /payment/... and /zh-CN/payment/... means future locales won't get auto-excluded. Derive instead:
| const SITEMAP_EXCLUDED_PATHNAMES = new Set([ | |
| '/payment/success', | |
| '/payment/failed', | |
| '/zh-CN/payment/success', | |
| '/zh-CN/payment/failed' | |
| ]) | |
| const PAYMENT_STATUSES = ['success', 'failed'] as const | |
| const LOCALE_PREFIXES = ['', '/zh-CN'] as const | |
| const SITEMAP_EXCLUDED_PATHNAMES = new Set( | |
| LOCALE_PREFIXES.flatMap((prefix) => | |
| PAYMENT_STATUSES.map((status) => `${prefix}/payment/${status}`) | |
| ) | |
| ) |
There was a problem hiding this comment.
Applied the suggestion verbatim. Future locales will now auto-exclude as long as they're added to LOCALE_PREFIXES.
| 'https://stagingapp.comfy.org' | ||
| ]) | ||
|
|
||
| export function resolveAppOrigin(referrer: string | undefined): string { |
There was a problem hiding this comment.
🔵 nit: document.referrer is always string ("" when absent), never undefined. The string | undefined type adds noise.
| export function resolveAppOrigin(referrer: string | undefined): string { | |
| export function resolveAppOrigin(referrer: string): string { |
There was a problem hiding this comment.
Resolved by deleting resolveAppOrigin and the allowlist entirely (dead code per the referrer-policy issue above).
| const APP_URL = 'https://app.comfy.org' | ||
| const PLATFORM_URL = 'https://platform.comfy.org' | ||
| const BILLING_DOCS_URL = 'https://docs.comfy.org/api-reference/cloud' |
There was a problem hiding this comment.
🔵 nit: re-declaring URLs that already live in externalLinks lets test and prod drift. Import the source of truth:
| const APP_URL = 'https://app.comfy.org' | |
| const PLATFORM_URL = 'https://platform.comfy.org' | |
| const BILLING_DOCS_URL = 'https://docs.comfy.org/api-reference/cloud' | |
| import { externalLinks } from '../src/config/routes' | |
| const APP_URL = externalLinks.app | |
| const PLATFORM_URL = externalLinks.platform | |
| const BILLING_DOCS_URL = externalLinks.docsApi |
There was a problem hiding this comment.
Done — tests now import externalLinks and assign APP_URL / PLATFORM_URL / BILLING_DOCS_URL from it. No more drift risk.
| }) | ||
| const cta = page.getByRole('link', { name: /CONTINUE IN COMFYUI/i }) | ||
| await expect(cta).toHaveAttribute('href', APP_URL) | ||
| }) |
There was a problem hiding this comment.
🔵 nit: the @interaction block only covers the negative (non-allowlisted) path. Add a positive case so allowlist behavior is locked in:
test('uses referrer origin for allowlisted referrer', async ({ page }) => {
await page.goto('/payment/success', { referer: 'https://app.comfy.org/checkout/' })
const cta = page.getByRole('link', { name: /CONTINUE IN COMFYUI/i })
await expect(cta).toHaveAttribute('href', 'https://app.comfy.org')
})There was a problem hiding this comment.
Resolved by removing the @interaction block entirely. Once the referrer logic was dropped, both branches of the test asserted the same static fallback — the test had no remaining behaviour to lock in.
- Drop document.referrer / resolveAppOrigin logic. After Stripe Checkout, document.referrer is checkout.stripe.com (or empty under modern Referrer-Policy), never app.comfy.org \u2014 the allowlist always missed, making the onMounted dance dead code. Primary success CTA now links statically to externalLinks.app. - Add fallback navigation to handleCloseTab so users aren't stuck if window.close() no-ops on tabs not script-opened. - Derive sitemap exclusions from PAYMENT_STATUSES x LOCALE_PREFIXES so future locales auto-exclude. - E2E tests import externalLinks instead of duplicating URL literals; drop the now-meaningless allowlist negative test. - Drop client:load on success pages (now fully static); failed pages keep it for the Close-tab handler. - Add Disallow: /payment/ to robots.txt as defense in depth alongside the per-page noindex meta and sitemap exclusion. - Document that these pages are display-only \u2014 payment state is verified server-side via Stripe webhooks.
glary-bot
Bot
dismissed stale reviews from DrJKL and coderabbitai[bot]
via
535a034
|
Thanks @DrJKL — addressed all 7 inline comments in Risks (both fixed):
Nits (all applied): sitemap exclusions derived from Open questions:
Also dropped |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/website/e2e/payment.spec.ts (1)
70-74: ⚡ Quick winUse
getRoutes()for the contact-link expectations too.These assertions still duplicate
/contactand/zh-CN/contacteven though the component reads them from route config. Pulling the expected values fromgetRoutes(locale)keeps the test focused on behavior instead of string drift.♻️ Proposed refactor
-import { externalLinks } from '../src/config/routes' +import { externalLinks, getRoutes } from '../src/config/routes' import { test } from './fixtures/blockExternalMedia' const APP_URL = externalLinks.app const PLATFORM_URL = externalLinks.platform const BILLING_DOCS_URL = externalLinks.docsApi +const EN_ROUTES = getRoutes('en') +const ZH_CN_ROUTES = getRoutes('zh-CN') @@ test('primary CTA links to contact page', async ({ page }) => { const cta = page.getByRole('link', { name: /CONTACT SUPPORT/i }) await expect(cta).toBeVisible() - await expect(cta).toHaveAttribute('href', '/contact') + await expect(cta).toHaveAttribute('href', EN_ROUTES.contact) }) @@ await expect(page.getByRole('link', { name: '联系支持' })).toHaveAttribute( 'href', - '/zh-CN/contact' + ZH_CN_ROUTES.contact )Also applies to: 111-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/e2e/payment.spec.ts` around lines 70 - 74, The test "primary CTA links to contact page" duplicates the contact URL string; update it to use the route config instead by calling getRoutes(locale) and asserting the CTA's href equals the contact route from that object. Locate the test function (test('primary CTA links to contact page', ...)) and replace the hard-coded '/contact' check with the value returned from getRoutes(locale). Do the same for the parallel test at lines 111-114 so both tests derive expected hrefs from getRoutes(locale) rather than literal strings.apps/website/astro.config.ts (1)
6-12: ⚡ Quick winDerive sitemap locale prefixes from the actual locale list.
LOCALE_PREFIXESnow duplicates thei18n.localessource later in this file. If another locale is added and this array is missed, its/payment/*pages will leak back into the sitemap.♻️ Proposed refactor
-const PAYMENT_STATUSES = ['success', 'failed'] as const -const LOCALE_PREFIXES = ['', '/zh-CN'] as const +const LOCALES = ['en', 'zh-CN'] as const +const PAYMENT_STATUSES = ['success', 'failed'] as const +const LOCALE_PREFIXES = LOCALES.map((locale) => + locale === 'en' ? '' : `/${locale}` +) const SITEMAP_EXCLUDED_PATHNAMES = new Set( LOCALE_PREFIXES.flatMap((prefix) => PAYMENT_STATUSES.map((status) => `${prefix}/payment/${status}`) ) ) @@ i18n: { - locales: ['en', 'zh-CN'], + locales: LOCALES, defaultLocale: 'en', routing: { prefixDefaultLocale: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/astro.config.ts` around lines 6 - 12, LOCALE_PREFIXES is being hard-coded and duplicates the source of truth (i18n.locales), causing new locales to be missed; change the code so LOCALE_PREFIXES is derived from the existing i18n.locales array instead of a manual array. Specifically, use the i18n.locales value (or move the i18n definition earlier) and produce prefixes by mapping locales to '' for the default and '/<locale>' for others, then use that derived LOCALE_PREFIXES when building SITEMAP_EXCLUDED_PATHNAMES alongside PAYMENT_STATUSES so any added locale is automatically included; update the declarations around LOCALE_PREFIXES, SITEMAP_EXCLUDED_PATHNAMES and i18n to ensure i18n is available when computing the prefixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/website/src/components/payment/PaymentStatusSection.vue`:
- Around line 23-31: The failed-state recovery text promises a "return-to-start"
flow but handleCloseTab only calls window.close() and falls back to routes.home,
losing billing context; either persist a return URL through checkout and use it
in handleCloseTab (read returnUrl from the query string or localStorage and
navigate to it instead of routes.home when window.close() fails) or update the
component copy and CTA in PaymentStatusSection.vue (subtitle and retry button)
to remove the return-to-start promise and present explicit options like "Contact
support" and "Start over" that navigate to docs/contact or a billing retry
route; adjust handleCloseTab and any CTA click handlers accordingly so behavior
matches the updated copy.
---
Nitpick comments:
In `@apps/website/astro.config.ts`:
- Around line 6-12: LOCALE_PREFIXES is being hard-coded and duplicates the
source of truth (i18n.locales), causing new locales to be missed; change the
code so LOCALE_PREFIXES is derived from the existing i18n.locales array instead
of a manual array. Specifically, use the i18n.locales value (or move the i18n
definition earlier) and produce prefixes by mapping locales to '' for the
default and '/<locale>' for others, then use that derived LOCALE_PREFIXES when
building SITEMAP_EXCLUDED_PATHNAMES alongside PAYMENT_STATUSES so any added
locale is automatically included; update the declarations around
LOCALE_PREFIXES, SITEMAP_EXCLUDED_PATHNAMES and i18n to ensure i18n is available
when computing the prefixes.
In `@apps/website/e2e/payment.spec.ts`:
- Around line 70-74: The test "primary CTA links to contact page" duplicates the
contact URL string; update it to use the route config instead by calling
getRoutes(locale) and asserting the CTA's href equals the contact route from
that object. Locate the test function (test('primary CTA links to contact page',
...)) and replace the hard-coded '/contact' check with the value returned from
getRoutes(locale). Do the same for the parallel test at lines 111-114 so both
tests derive expected hrefs from getRoutes(locale) rather than literal strings.
🪄 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
Run ID: 104f0442-0265-4f28-b180-4073bb865c7d
📒 Files selected for processing (7)
apps/website/astro.config.tsapps/website/e2e/payment.spec.tsapps/website/public/robots.txtapps/website/src/components/payment/PaymentStatusSection.vueapps/website/src/config/routes.tsapps/website/src/pages/payment/success.astroapps/website/src/pages/zh-CN/payment/success.astro
✅ Files skipped from review due to trivial changes (3)
- apps/website/public/robots.txt
- apps/website/src/pages/payment/success.astro
- apps/website/src/pages/zh-CN/payment/success.astro
| function handleCloseTab() { | ||
| if (typeof window === 'undefined') return | ||
| window.close() | ||
| // window.close() only succeeds on tabs that were script-opened. After a | ||
| // Stripe Checkout redirect the opener relationship is unreliable, so fall | ||
| // back to navigating home if the tab is still open shortly after. | ||
| setTimeout(() => { | ||
| if (!window.closed) window.location.href = routes.home | ||
| }, 100) |
There was a problem hiding this comment.
Failed-state recovery doesn't match what the page promises.
The failed subtitle says users can retry from where they started, but this component only offers contact/docs and a window.close() fallback to home. In the common case where close is ignored after the Stripe redirect, users lose their billing context instead of getting a retry path.
Please either carry a return URL through checkout and use it here, or change the copy/CTA so the page stops promising a return-to-start flow.
Also applies to: 110-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/components/payment/PaymentStatusSection.vue` around lines 23
- 31, The failed-state recovery text promises a "return-to-start" flow but
handleCloseTab only calls window.close() and falls back to routes.home, losing
billing context; either persist a return URL through checkout and use it in
handleCloseTab (read returnUrl from the query string or localStorage and
navigate to it instead of routes.home when window.close() fails) or update the
component copy and CTA in PaymentStatusSection.vue (subtitle and retry button)
to remove the return-to-start promise and present explicit options like "Contact
support" and "Start over" that navigate to docs/contact or a billing retry
route; adjust handleCloseTab and any CTA click handlers accordingly so behavior
matches the updated copy.
There was a problem hiding this comment.
Good catch — the copy was overpromising. Carrying a return URL through Stripe Checkout is the right long-term fix but it needs a coordinated comfy-api change (Stripe success_url / cancel_url augmentation + handling), which is out of scope for this PR.
Going with the copy fix here so the page stops promising what it can't deliver. Updated payment.failed.subtitle (en + zh-CN) to drop the "retry from where you started" phrasing and direct users to support / billing docs, which matches the actual CTAs:
Your payment didn't go through and you have not been charged. Reach out to support or check the billing docs if you need help.
The three CTAs (Contact support / Read billing docs / Close tab) now match what the copy promises. I'll file a follow-up issue for the proper return-URL plumbing once we want to do the backend change.
There was a problem hiding this comment.
Skipped: comment is from another GitHub bot.
- Failed-page subtitle no longer promises "retry from where you started" since this page can't reliably do that. Soften to direct users to support / billing docs, which matches what the CTAs actually do. - Derive sitemap LOCALE_PREFIXES from the same LOCALES constant used by Astro's i18n config so future locales auto-exclude. - E2E tests assert against getRoutes(locale).contact instead of duplicating literal /contact and /zh-CN/contact strings.
|
Addressed the new round of CodeRabbit feedback in
|
3 participants
PR Created by the Glary-Bot Agent
Summary
Recreate the two payment status pages that were never migrated from Framer (they weren't in the sitemap, so were missed). The Stripe checkout flow in
comfy-apiredirects tohttps://www.comfy.org/payment/{success,failed}after a checkout session, so users currently hit a 404 on completion or cancel.Changes
/payment/success,/payment/failedand their/zh-CN/variants, sharing aPaymentStatusSection.vuecomponent built from the existingBrandButton/SectionLabelprimitives. Translation keys live insrc/i18n/translations.tsfor both locales. Pages arenoindexand explicitly excluded from the generated sitemap. Adds a Playwright e2e spec covering both pages in both locales.Review Focus
STRIPE_CANCEL_URL=…/payment/failedincomfy-api/run-service-{prod,staging}.yaml), not/payment/failure.externalLinks.apiKeys(platform.comfy.org/profile/api-keys) — the platform root is just a redirect, so this avoids a hop.getRoutes(locale)so future i18n/route changes flow through the existing helper.comfy-api/gateways/stripe/stripe.go:159has an unrelated stale fallback pointing at/payments/(plural) that's overridden by the env config in production. Worth fixing in a follow-up.Screenshots
EN success / failed and zh-CN success / failed at desktop widths. Mobile viewport stacks the CTA buttons vertically (verified locally).
Screenshots
┆Issue is synchronized with this Notion page by Unito