feat(billing): method-aware payment-method collection UX#13354
Closed
wei-hai wants to merge 4 commits into
Closed
Conversation
Add three surfaces for off-session payment collection: - Spend-limit dialog (SpendLimitDialogContent) with method-aware variants driven by payment_method_capability; add-method routes to the setup flow, a failed auto-charge routes to the billing portal. - Consent disclosure via a context prop on SubscriptionTermsNote. - Owed-balance notice in CreditsTile with a capability-aware CTA, a settle-endpoint feature flag (default off -> read-only fallback), and a dedicated pay_owed billing operation type. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
the
size:XL
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1691 passed, 0 failed · 2 flaky📊 Browser Reports
📦 Bundle: 7.77 MB gzip 🔴 +5.45 kBDetailsSummary
Category Glance App Entry Points — 47.4 kB (baseline 47.4 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 97.7 kB (baseline 97.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 12 added / 12 removed Panels & Settings — 546 kB (baseline 546 kB) • 🔴 +41 BConfiguration panels, inspectors, and settings screens
Status: 24 added / 24 removed / 3 unchanged User & Accounts — 26.9 kB (baseline 26.9 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 8 added / 8 removed / 2 unchanged Editors & Dialogs — 117 kB (baseline 117 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 5 added / 5 removed UI Components — 57.2 kB (baseline 57.2 kB) • ⚪ 0 BReusable component library chunks
Status: 10 added / 10 removed / 3 unchanged Data & Services — 270 kB (baseline 270 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 2 unchanged Utilities & Hooks — 3.37 MB (baseline 3.37 MB) • 🔴 +2.94 kBHelpers, composables, and utility bundles
Status: 23 added / 23 removed / 10 unchanged Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 1 added / 1 removed / 15 unchanged Other — 11.7 MB (baseline 11.7 MB) • 🔴 +16.2 kBBundles that do not match a named category
Status: 140 added / 138 removed / 27 unchanged ⚡ Performance
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds workspace billing support for pending charges and payment-method capabilities, including new workspace API endpoints, feature-flagged settlement flow, owed-balance UI, a ChangesOwed Balance Payment Settlement
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
wei-hai
added
the
cursor-review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@src/locales/en/main.json`:
- Around line 4497-4506: Add the missing billing.spendLimit.defaultMethod
translation entry in the main locale JSON so SpendLimitDialogContent.vue can
resolve t('billing.spendLimit.defaultMethod') when methodType is falsy. Update
the spendLimit object in src/locales/en/main.json to include a suitable default
method label that matches the existing spend-limit copy and keeps the
methodLabel computed in SpendLimitDialogContent.vue from falling back to a
missing-translation string.
In `@src/platform/cloud/subscription/components/CreditsTile.vue`:
- Around line 326-337: The popup flow in handleOwedAddPaymentMethod does not
handle a blocked window.open result, so add a check on the return value before
proceeding. If window.open returns a falsy value, show a warning toast through
toastStore similar to SpendLimitDialogContent’s handleMainCta, and keep the
existing error handling for initiateAddPaymentMethod failures unchanged. Use
handleOwedAddPaymentMethod and the toastStore pattern as the main touchpoints to
mirror the sibling component’s behavior consistently.
- Around line 356-368: The refresh-button focus logic in CreditsTile.vue is
using a fragile document.querySelector with a localized aria-label, which can
throw and incorrectly fall into the error path after a successful operation.
Replace that lookup with a template ref like the existing payNowButtonRef, and
bind a new refreshButtonRef on the refresh Button in the template. Then update
the success branch in the payment flow to focus refreshButtonRef directly after
nextTick, avoiding any selector built from t('subscription.refreshCredits').
- Around line 339-376: `handlePayNow` in `CreditsTile.vue` is vulnerable to
duplicate submissions because `isPayingOwed` only becomes true after async work
starts. Add a synchronous local in-flight guard (for example a
`isSubmittingPayNow` ref) at the top of `handlePayNow` before calling
`workspaceApi.settleOwedBalance()`, and clear it in a `finally` block. Also
update the pay-now button’s disabled state to include this new guard alongside
`isPayingOwed`, so rapid clicks or repeated keyboard submits cannot re-enter the
flow.
In `@src/platform/workspace/api/workspaceApi.ts`:
- Around line 831-844: The settleOwedBalance flow is currently optional on
idempotency, which allows duplicate settlement on retry. Update
workspaceApi.settleOwedBalance to require an idempotency key (or generate one
before the first call) and propagate it from the CreditsTile.vue caller so every
settlement request is uniquely keyed. Make sure the method signature and the
workspaceApiClient.post payload in settleOwedBalance use the required key
consistently.
- Around line 261-262: Require an idempotency key on settleOwedBalance and
update its call site(s) to always pass one. The current workspaceApi payment
flow allows the endpoint to be invoked without idempotency protection, so adjust
the settleOwedBalance API signature and the caller that uses it to supply a
stable key for retries/double submits. Use the settleOwedBalance method and its
related payment request types in workspaceApi as the main symbols to update.
In `@src/platform/workspace/components/SpendLimitDialogContent.vue`:
- Around line 128-144: `ctaAriaLabel` is out of sync with `ctaLabel` in
`SpendLimitDialogContent.vue`, causing the accessible name to differ from the
visible button text. Update the CTA so it uses the same computed label for both
the displayed text and accessibility, or remove the separate aria-label entirely
since the button already has visible text. Keep the fix localized to the
`ctaLabel` / `ctaAriaLabel` computed values used by the CTA rendering.
- Around line 82-95: Replace the local duplicated capability union in
SpendLimitDialogContent.vue with the shared PaymentMethodCapability type
exported from workspaceApi.ts, and update any matching usage in dialogService.ts
to reference the same type. Keep Scenario defined locally in
SpendLimitDialogContent.vue, but ensure the props and related dialogService
logic use the shared Capability symbol so the union is defined in one place
only.
- Around line 105-115: The labels in SpendLimitDialogContent’s methodLabel logic
are using hardcoded English text instead of vue-i18n. Update METHOD_LABELS and
the fallback in the computed methodLabel to read from translation keys under
billing.spendLimit, and add the matching entries in src/locales/en/main.json so
all user-facing payment method names are localized.
In `@src/platform/workspace/stores/billingOperationStore.ts`:
- Around line 87-94: The messageKey selection in billingOperationStore is using
an implicit catch-all else that assumes every non-subscription and non-topup
type is pay_owed, which can silently mislabel future OperationType values.
Update the branching in the relevant store method to use an explicit pay_owed
check alongside the existing subscription and topup cases, and make the
remaining path fail clearly or be otherwise exhaustive so any new OperationType
must be handled intentionally.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4506c47a-8a29-40d5-a94b-e0af05fe46cb
📒 Files selected for processing (14)
src/composables/billing/types.tssrc/composables/billing/useBillingContext.tssrc/composables/billing/useLegacyBilling.tssrc/composables/useFeatureFlags.tssrc/locales/en/main.jsonsrc/platform/cloud/subscription/components/CreditsTile.vuesrc/platform/workspace/api/workspaceApi.tssrc/platform/workspace/components/SpendLimitDialogContent.vuesrc/platform/workspace/components/SubscriptionTermsNote.vuesrc/platform/workspace/composables/useWorkspaceBilling.tssrc/platform/workspace/stores/billingOperationStore.test.tssrc/platform/workspace/stores/billingOperationStore.tssrc/services/dialogService.tssrc/storybook/mocks/useBillingContext.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @wei-hai.
Found 10 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 6 |
| 🟢 Low | 2 |
| ⚪ Nit | 1 |
Panel: 8/8 reviewers contributed findings.
…flow - Add SubscriptionTermsNote (context="payment_method") above the add-payment-method CTA in SpendLimitDialogContent (Variants A and B) and above the CTA in CreditsTile for none/one_time_only capability - Add Pay now button to CreditsTile owed-balance notice when paymentMethodCapability=reusable and the settle endpoint flag is on; passes a crypto.randomUUID() idempotency key to settleOwedBalance() - Guard handlePayNow with a local ref to prevent double-submission before the billing-operation store updates; focus management moves to the refresh button on success or pay-now button on failure/cancel - Validate Stripe redirect URL (hostname must end with .stripe.com) before window.open() in both CreditsTile and SpendLimitDialogContent; toast + abort on mismatch - Null-check the window.open() return value and toast a "popup blocked" warning when the browser suppresses the popup - Widen showSpendLimitDialog options with capabilityError?: boolean and fix ctaAriaLabel to be consistent with ctaLabel when capabilityError - Add billing.spendLimit.defaultMethod i18n key used as the fallback method label in SpendLimitDialogContent - Fix test suite: mock useBillingOperationStore with a plain getter for isPayingOwed (not a ComputedRef) to match Pinia's auto-unwrap behaviour; add mocks for useFeatureFlags, workspaceApi, and useToastStore; add 9 owed-balance notice test cases
Contributor
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)
src/platform/cloud/subscription/components/CreditsTile.vue (1)
147-223: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a fallback and permission gate for owed-balance actions
- Add a loading/disabled branch for
paymentMethodCapability === null; otherwise this alert can render with no CTA while status is still loading.- Apply
permissions.canTopUphere as well so the owed-balance actions match the main top-up button.🤖 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 `@src/platform/cloud/subscription/components/CreditsTile.vue` around lines 147 - 223, The owed-balance alert in CreditsTile.vue needs a loading fallback and the same permission gate as the main top-up flow. Update the v-if/v-else-if branches around the owed balance CTA so paymentMethodCapability === null renders a disabled or loading state instead of an empty alert, and ensure handleOwedAddPaymentMethod and handlePayNow are only shown when permissions.canTopUp is true. Use the existing owedBalanceAmount, paymentMethodCapability, settleEndpointEnabled, and permissions.canTopUp conditions to keep the logic consistent.
♻️ Duplicate comments (5)
src/platform/workspace/components/SpendLimitDialogContent.vue (3)
134-154: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ctaAriaLabelstill diverges fromctaLabelforreusable+limit_reached.For
capability === 'reusable'andscenario === 'limit_reached',ctaLabelresolves toupdatePaymentMethodCta, butctaAriaLabel's condition requiresscenario === 'payment_failed', so it falls back toaddPaymentMethodCta— the accessible name still contradicts the visible button text in this branch. Since the button already renders visible text, the cleanest fix remains removing the separate aria-label.🛠️ Proposed fix
-const ctaAriaLabel = computed(() => { - if ( - !capabilityError && - capability === 'reusable' && - scenario === 'payment_failed' - ) { - return t('billing.spendLimit.updatePaymentMethodCta') - } - return t('billing.spendLimit.addPaymentMethodCta') -})Remove the corresponding
:aria-label="ctaAriaLabel"binding on the CTA button in the template.🤖 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 `@src/platform/workspace/components/SpendLimitDialogContent.vue` around lines 134 - 154, The CTA accessible name still diverges from the visible label in SpendLimitDialogContent.vue because ctaAriaLabel only matches ctaLabel for one scenario. Remove the separate ctaAriaLabel computed logic and delete the corresponding :aria-label binding on the CTA button in the template so the button relies on its visible text for all capability/scenario branches.Source: Learnings
160-195: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
window.openmissingnoopener/noreferrer.Same gap as
CreditsTile.vue'shandleOwedAddPaymentMethod: the opened Stripe tab retainswindow.openeraccess.🔒 Proposed fix
- const paymentWindow = window.open(url, '_blank') + const paymentWindow = window.open(url, '_blank', 'noopener,noreferrer')🤖 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 `@src/platform/workspace/components/SpendLimitDialogContent.vue` around lines 160 - 195, The `handleMainCta` flow opens the Stripe payment URL with `window.open` but does not prevent the new tab from accessing `window.opener`. Update the `window.open` call in `SpendLimitDialogContent.vue` to include the appropriate `noopener`/`noreferrer` protection, matching the fix used in `CreditsTile.vue`’s `handleOwedAddPaymentMethod`, and keep the existing blocked-popup warning behavior unchanged.
111-121: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
METHOD_LABELSvalues are still hardcoded, unlocalized strings.The fallback in
methodLabelnow correctly usest('billing.spendLimit.defaultMethod'), but theMETHOD_LABELSmap itself ('Alipay', 'Your card', 'Your bank account', 'Link') still bypassesvue-i18n. As per coding guidelines,src/**/*.{js,ts,vue}must "Use vue-i18n for ALL user-facing strings, configured in src/locales/en/main.json."🌐 Proposed fix
-const METHOD_LABELS: Record<string, string> = { - alipay: 'Alipay', - card: 'Your card', - us_bank_account: 'Your bank account', - link: 'Link' -} - const methodLabel = computed(() => { if (!methodType) return t('billing.spendLimit.defaultMethod') - return METHOD_LABELS[methodType] ?? t('billing.spendLimit.defaultMethod') + return t(`billing.spendLimit.methodLabels.${methodType}`, { + defaultValue: t('billing.spendLimit.defaultMethod') + }) })🤖 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 `@src/platform/workspace/components/SpendLimitDialogContent.vue` around lines 111 - 121, The user-facing payment method labels in METHOD_LABELS are still hardcoded and need to be localized through vue-i18n. Update the methodLabel computed flow in SpendLimitDialogContent so each method key (alipay, card, us_bank_account, link) resolves via t(...) keys from the locale files instead of static English strings, while keeping the existing fallback to billing.spendLimit.defaultMethod for unknown or missing methods. Use the existing methodLabel and METHOD_LABELS symbols to locate the mapping and replace the literal labels with translation-based lookups.Source: Coding guidelines
src/platform/cloud/subscription/components/CreditsTile.vue (2)
318-326: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInvalid
currencycan still throwRangeErrorand crash the tile render.
currency ?? 'USD'only catches null/undefined; an empty string or non-ISO code from the API reachestoLocaleString({ style: 'currency', currency })uncaught, since this computed has no try/catch and runs inline in the template. This was raised previously and is still unresolved — the diff only added.toUpperCase().🛡️ Proposed fix
const owedBalanceAmount = computed(() => { const pendingMicros = balance.value?.pendingChargesMicros if (pendingMicros == null || pendingMicros <= 0) return null - const currency = balance.value?.currency ?? 'USD' - return (pendingMicros / 1_000_000).toLocaleString(locale.value, { - style: 'currency', - currency: currency.toUpperCase() - }) + const currency = balance.value?.currency?.toUpperCase() || 'USD' + try { + return (pendingMicros / 1_000_000).toLocaleString(locale.value, { + style: 'currency', + currency + }) + } catch { + return (pendingMicros / 1_000_000).toLocaleString(locale.value, { + style: 'currency', + currency: 'USD' + }) + } })🤖 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 `@src/platform/cloud/subscription/components/CreditsTile.vue` around lines 318 - 326, The owed balance formatter in the CreditsTile component can still throw a RangeError when balance.value.currency is empty or invalid, since owedBalanceAmount uses toLocaleString with currency directly. Update owedBalanceAmount to defensively validate or sanitize the currency value before formatting, and fall back to a safe default or null when the API returns a non-ISO code; keep the fix localized to the computed in CreditsTile.vue so the template render cannot crash.
336-363: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDouble-submit guard and popup-blocked handling are fixed;
window.openstill missingnoopener.The synchronous
isPayingNowguard and popup-blocked toast address prior critical/major findings. One remaining gap:window.open(url, '_blank')at line 348 doesn't passnoopener,noreferrer, so the opened tab retainswindow.openeraccess (reverse-tabnabbing), even though the URL is hostname-validated. Same pattern exists inSpendLimitDialogContent.vue.🔒 Proposed fix
- const win = window.open(url, '_blank') + const win = window.open(url, '_blank', 'noopener,noreferrer')Also applies to: 365-405
🤖 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 `@src/platform/cloud/subscription/components/CreditsTile.vue` around lines 336 - 363, The popup-open flow still uses a plain window.open call without security flags, leaving the new tab with access to window.opener. Update handleOwedAddPaymentMethod to open the Stripe URL with noopener,noreferrer in the target features, and apply the same fix to the matching window.open usage in SpendLimitDialogContent so both payment-method flows are protected against reverse-tabnabbing.
🤖 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 `@src/platform/cloud/subscription/components/CreditsTile.test.ts`:
- Around line 450-544: Add test coverage in the `owed balance notice` suite for
the unresolved `paymentMethodCapability === null` state. Use `renderTile()` with
a positive `pendingChargesMicros` and `state.paymentMethodCapability = null`,
then assert the notice renders without an actionable CTA (matching the expected
loading/unresolved behavior). Place the new case alongside the existing
capability-specific tests in `CreditsTile.test.ts` so the behavior is locked in
for `CreditsTile.vue`.
---
Outside diff comments:
In `@src/platform/cloud/subscription/components/CreditsTile.vue`:
- Around line 147-223: The owed-balance alert in CreditsTile.vue needs a loading
fallback and the same permission gate as the main top-up flow. Update the
v-if/v-else-if branches around the owed balance CTA so paymentMethodCapability
=== null renders a disabled or loading state instead of an empty alert, and
ensure handleOwedAddPaymentMethod and handlePayNow are only shown when
permissions.canTopUp is true. Use the existing owedBalanceAmount,
paymentMethodCapability, settleEndpointEnabled, and permissions.canTopUp
conditions to keep the logic consistent.
---
Duplicate comments:
In `@src/platform/cloud/subscription/components/CreditsTile.vue`:
- Around line 318-326: The owed balance formatter in the CreditsTile component
can still throw a RangeError when balance.value.currency is empty or invalid,
since owedBalanceAmount uses toLocaleString with currency directly. Update
owedBalanceAmount to defensively validate or sanitize the currency value before
formatting, and fall back to a safe default or null when the API returns a
non-ISO code; keep the fix localized to the computed in CreditsTile.vue so the
template render cannot crash.
- Around line 336-363: The popup-open flow still uses a plain window.open call
without security flags, leaving the new tab with access to window.opener. Update
handleOwedAddPaymentMethod to open the Stripe URL with noopener,noreferrer in
the target features, and apply the same fix to the matching window.open usage in
SpendLimitDialogContent so both payment-method flows are protected against
reverse-tabnabbing.
In `@src/platform/workspace/components/SpendLimitDialogContent.vue`:
- Around line 134-154: The CTA accessible name still diverges from the visible
label in SpendLimitDialogContent.vue because ctaAriaLabel only matches ctaLabel
for one scenario. Remove the separate ctaAriaLabel computed logic and delete the
corresponding :aria-label binding on the CTA button in the template so the
button relies on its visible text for all capability/scenario branches.
- Around line 160-195: The `handleMainCta` flow opens the Stripe payment URL
with `window.open` but does not prevent the new tab from accessing
`window.opener`. Update the `window.open` call in `SpendLimitDialogContent.vue`
to include the appropriate `noopener`/`noreferrer` protection, matching the fix
used in `CreditsTile.vue`’s `handleOwedAddPaymentMethod`, and keep the existing
blocked-popup warning behavior unchanged.
- Around line 111-121: The user-facing payment method labels in METHOD_LABELS
are still hardcoded and need to be localized through vue-i18n. Update the
methodLabel computed flow in SpendLimitDialogContent so each method key (alipay,
card, us_bank_account, link) resolves via t(...) keys from the locale files
instead of static English strings, while keeping the existing fallback to
billing.spendLimit.defaultMethod for unknown or missing methods. Use the
existing methodLabel and METHOD_LABELS symbols to locate the mapping and replace
the literal labels with translation-based lookups.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b980ab16-1e59-45b1-8dc2-0945bb2fc589
📒 Files selected for processing (5)
src/locales/en/main.jsonsrc/platform/cloud/subscription/components/CreditsTile.test.tssrc/platform/cloud/subscription/components/CreditsTile.vuesrc/platform/workspace/components/SpendLimitDialogContent.vuesrc/services/dialogService.ts
- Validate currency code with /^[A-Z]{3}$/ before passing to
toLocaleString, falling back to USD for empty/invalid values
- Gate owed-balance notice CTAs (terms note + action buttons) behind
permissions.canTopUp so non-owner members see the balance text but
not owner-only billing actions
- Add neutral message branch for null paymentMethodCapability (status
still loading or unavailable) so users are never stuck with no
feedback
- Add noopener,noreferrer to window.open calls in CreditsTile and
SpendLimitDialogContent to prevent reverse-tabnabbing
- Fix SpendLimitDialogContent title/ctaLabel to branch on scenario as
well as capability, making visible label, aria-label, and action
self-consistent for reusable+limit_reached
- Replace hardcoded English METHOD_LABELS with vue-i18n keys under
billing.spendLimit.methodLabels
- Import PaymentMethodCapability from workspaceApi instead of
duplicating the union in SpendLimitDialogContent and dialogService
- Make the pay_owed processing-toast branch explicit in
billingOperationStore to avoid a silent implicit else
- Add tests: currency fallback, canTopUp gate, null-capability branch
Contributor
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)
src/services/dialogService.ts (1)
630-634: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMisplaced JSDoc: describes
showDowngradeToPersonalDialog, notshowSpendLimitDialog.This docstring ("Downgrade a team plan to a personal plan (FE-977)...") clearly documents
showDowngradeToPersonalDialog(defined at line 654), but sits above the newly-insertedshowSpendLimitDialog.showDowngradeToPersonalDialogis now left without its own docstring, and readers ofshowSpendLimitDialogget an unrelated/misleading description.📝 Proposed fix
- /** - * Downgrade a team plan to a personal plan (FE-977). Skips the type-"I - * understand" confirm dialog when the workspace has no other members; - * failures on that path surface as an error toast. - */ async function showSpendLimitDialog(options: { scenario: 'limit_reached' | 'payment_failed' capability: PaymentMethodCapability methodType?: string capabilityError?: boolean }) {+ /** + * Downgrade a team plan to a personal plan (FE-977). Skips the type-"I + * understand" confirm dialog when the workspace has no other members; + * failures on that path surface as an error toast. + */ async function showDowngradeToPersonalDialog(options: { planName: string planSlug: string }) {🤖 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 `@src/services/dialogService.ts` around lines 630 - 634, The JSDoc currently attached to showSpendLimitDialog is actually documenting showDowngradeToPersonalDialog, so move that comment down to the showDowngradeToPersonalDialog declaration and add or keep a spend-limit-specific docstring above showSpendLimitDialog. Use the function names showSpendLimitDialog and showDowngradeToPersonalDialog to place the comments correctly so each method has the right description.
🤖 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 `@src/platform/workspace/components/SpendLimitDialogContent.vue`:
- Around line 111-120: The methodLabel computed in SpendLimitDialogContent.vue
eagerly calls t() for all payment methods on every run; refactor it to resolve
only the active methodType key directly and fall back to
billing.spendLimit.defaultMethod when needed. Use the methodLabel computed and
the existing translation keys for alipay, card, us_bank_account, and link to
keep the logic simple and avoid building the labels map.
---
Outside diff comments:
In `@src/services/dialogService.ts`:
- Around line 630-634: The JSDoc currently attached to showSpendLimitDialog is
actually documenting showDowngradeToPersonalDialog, so move that comment down to
the showDowngradeToPersonalDialog declaration and add or keep a
spend-limit-specific docstring above showSpendLimitDialog. Use the function
names showSpendLimitDialog and showDowngradeToPersonalDialog to place the
comments correctly so each method has the right description.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: dd20115f-8d27-43b9-9bc6-2b61d767887a
📒 Files selected for processing (6)
src/locales/en/main.jsonsrc/platform/cloud/subscription/components/CreditsTile.test.tssrc/platform/cloud/subscription/components/CreditsTile.vuesrc/platform/workspace/components/SpendLimitDialogContent.vuesrc/platform/workspace/stores/billingOperationStore.tssrc/services/dialogService.ts
…rmsNote refactor SubscriptionTermsNote gained a <script setup> that imports useSettingsDialog, pulling in dialogService -> @/i18n -> createI18n from vue-i18n. Two existing tests mocked vue-i18n with only useI18n, so createI18n was undefined at module load time, collapsing both test suites. Use importOriginal in both test mocks so all real vue-i18n exports (including createI18n) are preserved while only useI18n is overridden. Also stub SubscriptionTermsNote in SubscriptionAddPaymentPreviewWorkspace.test.ts to prevent the component's setup() from calling useSettingsDialog (which requires an active Pinia instance not set up in that test). Also simplify methodLabel computed in SpendLimitDialogContent.vue to look up the single needed i18n key instead of eagerly translating all four method labels on every recompute.
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #13354 +/- ##
==========================================
- Coverage 76.89% 76.83% -0.07%
==========================================
Files 1636 1640 +4
Lines 98625 98836 +211
Branches 33194 33262 +68
==========================================
+ Hits 75842 75943 +101
- Misses 22067 22177 +110
Partials 716 716
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 15 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Contributor
Author
|
Closing for now: payment-method capture is handled server-side (hosted checkout + webhook); the method-aware prompt UX is deferred. Branch retained. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds three UI surfaces for off-session payment collection, driven by a new
payment_method_capabilitybilling-status field (none/one_time_only/reusable):SpendLimitDialogContent.vue, dispatched viadialogService.showSpendLimitDialog) — method-aware variants: no method → add a method; a non-reusable method (e.g. Alipay) → prompt for a card/bank/Link (never implies the user has none); a failed automatic charge → update the method via the billing portal. Workspace-scoped; no-op for legacy accounts.contextprop onSubscriptionTermsNoterenders an authorization disclosure for automatic off-session charges on the card-save screen.CreditsTile— capability-aware CTA; apay_owedbilling operation with its own success handler (no dialog-close / no navigation); a settle-endpoint feature flag (default off → read-only fallback).Notes
Button,CreditsTilenotice pattern,billingOperationStore, toasts); one new component.role="alert"on the owed notice, focus management after settle, method-aware labels.Tests
New unit coverage for the
pay_owedoperation type (success does not close a dialog or navigate; failure/timeout messaging; lifecycle); billing-context + feature-flag + dialog tests pass; typecheck clean.🤖 Generated with Claude Code