Skip to content

Commit 8f2a24a

Browse files
committed
fix(billing): address review findings on payment-method collection UX
- 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
1 parent a222ad0 commit 8f2a24a

6 files changed

Lines changed: 99 additions & 44 deletions

File tree

src/locales/en/main.json

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4503,7 +4503,13 @@
45034503
"updatePaymentMethodCta": "Update payment method",
45044504
"orBuyManually": "Or buy credits manually",
45054505
"capabilityError": "Unable to load payment method status. You can add a payment method below.",
4506-
"defaultMethod": "Your current payment method"
4506+
"defaultMethod": "Your current payment method",
4507+
"methodLabels": {
4508+
"alipay": "Alipay",
4509+
"card": "Your card",
4510+
"us_bank_account": "Your bank account",
4511+
"link": "Link"
4512+
}
45074513
},
45084514
"owedBalance": {
45094515
"title": "Outstanding balance: {amount}",
@@ -4512,7 +4518,8 @@
45124518
"oneTimeOnlyHint": "Your current method can't be used to settle a balance — add a card, bank account, or Link",
45134519
"payNow": "Pay now",
45144520
"processing": "Processing payment…",
4515-
"chargeAutomatic": "A charge will process automatically."
4521+
"chargeAutomatic": "A charge will process automatically.",
4522+
"unknownCapability": "Payment method status unavailable. Refresh to try again."
45164523
},
45174524
"consent": {
45184525
"paymentMethodBody": "By adding a payment method, you authorize Comfy Org to automatically charge it — without further action from you at the time — for unpaid balances and usage overages, in the amount you owe at billing time. You can remove it at any time in {settingsLink} (an active subscription or unpaid balance may need to be resolved first).",

src/platform/cloud/subscription/components/CreditsTile.test.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ type Balance = Pick<
1414
| 'cloudCreditBalanceMicros'
1515
| 'prepaidBalanceMicros'
1616
| 'pendingChargesMicros'
17-
>
17+
> & { currency?: string }
1818
type Subscription = Pick<SubscriptionInfo, 'duration' | 'renewalDate'> & {
1919
tier: SubscriptionInfo['tier'] | 'TEAM'
2020
}
@@ -183,7 +183,9 @@ const i18n = createI18n({
183183
"Your current method can't be used to settle a balance — add a card, bank account, or Link",
184184
chargeAutomatic: 'A charge will process automatically.',
185185
payNow: 'Pay now',
186-
processing: 'Processing payment…'
186+
processing: 'Processing payment…',
187+
unknownCapability:
188+
'Payment method status unavailable. Refresh to try again.'
187189
}
188190
}
189191
}
@@ -541,5 +543,44 @@ describe('CreditsTile', () => {
541543
const btn = screen.getByRole('button', { name: /Processing payment/i })
542544
expect(btn.getAttribute('disabled')).not.toBeNull()
543545
})
546+
547+
it('falls back to USD when balance.currency is an empty string', () => {
548+
activeProSubscription()
549+
state.balance = {
550+
amountMicros: 500,
551+
pendingChargesMicros: 5_000_000,
552+
currency: ''
553+
}
554+
state.paymentMethodCapability = 'reusable'
555+
renderTile()
556+
const alert = screen.getByRole('alert')
557+
expect(alert.textContent).toContain('$5.00')
558+
})
559+
560+
it('hides CTAs when canTopUp is false even when balance is owed', () => {
561+
activeProSubscription()
562+
state.balance = { amountMicros: 500, pendingChargesMicros: 3_000_000 }
563+
state.paymentMethodCapability = 'none'
564+
state.canTopUp = false
565+
renderTile()
566+
const alert = screen.getByRole('alert')
567+
expect(alert.textContent).toContain('Outstanding balance')
568+
expect(screen.queryByText('Add a payment method')).toBeNull()
569+
expect(screen.queryByTestId('terms-note')).toBeNull()
570+
})
571+
572+
it('shows a neutral message when paymentMethodCapability is null', () => {
573+
activeProSubscription()
574+
state.balance = { amountMicros: 500, pendingChargesMicros: 3_000_000 }
575+
state.paymentMethodCapability = null
576+
state.canTopUp = true
577+
renderTile()
578+
const alert = screen.getByRole('alert')
579+
expect(alert.textContent).toContain(
580+
'Payment method status unavailable. Refresh to try again.'
581+
)
582+
expect(screen.queryByText('Pay now')).toBeNull()
583+
expect(screen.queryByText('Add a payment method')).toBeNull()
584+
})
544585
})
545586
})

src/platform/cloud/subscription/components/CreditsTile.vue

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,17 +177,19 @@
177177
<!-- consent note before add-payment-method CTA -->
178178
<SubscriptionTermsNote
179179
v-if="
180-
paymentMethodCapability === 'none' ||
181-
paymentMethodCapability === 'one_time_only'
180+
permissions.canTopUp &&
181+
(paymentMethodCapability === 'none' ||
182+
paymentMethodCapability === 'one_time_only')
182183
"
183184
context="payment_method"
184185
/>
185186

186187
<!-- CTA: none / one_time_only → add payment method -->
187188
<Button
188189
v-if="
189-
paymentMethodCapability === 'none' ||
190-
paymentMethodCapability === 'one_time_only'
190+
permissions.canTopUp &&
191+
(paymentMethodCapability === 'none' ||
192+
paymentMethodCapability === 'one_time_only')
191193
"
192194
variant="primary"
193195
size="sm"
@@ -204,7 +206,9 @@
204206
<!-- CTA: reusable + flag on → Pay now -->
205207
<Button
206208
v-else-if="
207-
paymentMethodCapability === 'reusable' && settleEndpointEnabled
209+
permissions.canTopUp &&
210+
paymentMethodCapability === 'reusable' &&
211+
settleEndpointEnabled
208212
"
209213
ref="payNowButtonRef"
210214
variant="primary"
@@ -218,6 +222,13 @@
218222
}}</span>
219223
<template v-else>{{ $t('billing.owedBalance.payNow') }}</template>
220224
</Button>
225+
226+
<!-- capability loading/unknown: show a neutral message -->
227+
<span
228+
v-if="permissions.canTopUp && paymentMethodCapability === null"
229+
class="text-muted"
230+
>{{ $t('billing.owedBalance.unknownCapability') }}</span
231+
>
221232
</div>
222233
</div>
223234

@@ -318,10 +329,11 @@ const settleEndpointEnabled = computed(() => flags.settleEndpointEnabled)
318329
const owedBalanceAmount = computed(() => {
319330
const pendingMicros = balance.value?.pendingChargesMicros
320331
if (pendingMicros == null || pendingMicros <= 0) return null
321-
const currency = balance.value?.currency ?? 'USD'
332+
const rawCurrency = (balance.value?.currency ?? '').toUpperCase()
333+
const currency = /^[A-Z]{3}$/.test(rawCurrency) ? rawCurrency : 'USD'
322334
return (pendingMicros / 1_000_000).toLocaleString(locale.value, {
323335
style: 'currency',
324-
currency: currency.toUpperCase()
336+
currency
325337
})
326338
})
327339
@@ -345,7 +357,7 @@ async function handleOwedAddPaymentMethod() {
345357
})
346358
return
347359
}
348-
const win = window.open(url, '_blank')
360+
const win = window.open(url, '_blank', 'noopener,noreferrer')
349361
if (!win) {
350362
toastStore.add({
351363
severity: 'warn',

src/platform/workspace/components/SpendLimitDialogContent.vue

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
variant="primary"
5454
size="lg"
5555
class="h-10 justify-center"
56-
:aria-label="ctaAriaLabel"
56+
:aria-label="ctaLabel"
5757
@click="handleMainCta"
5858
>
5959
<Skeleton v-if="ctaLoading" class="h-4 w-32" />
@@ -80,13 +80,13 @@ import Button from '@/components/ui/button/Button.vue'
8080
import Skeleton from '@/components/ui/skeleton/Skeleton.vue'
8181
import { useBillingContext } from '@/composables/billing/useBillingContext'
8282
import { useToastStore } from '@/platform/updates/common/toastStore'
83+
import type { PaymentMethodCapability } from '@/platform/workspace/api/workspaceApi'
8384
import { workspaceApi } from '@/platform/workspace/api/workspaceApi'
8485
import SubscriptionTermsNote from '@/platform/workspace/components/SubscriptionTermsNote.vue'
8586
import { useDialogService } from '@/services/dialogService'
8687
import { useDialogStore } from '@/stores/dialogStore'
8788
8889
type Scenario = 'limit_reached' | 'payment_failed'
89-
type Capability = 'none' | 'one_time_only' | 'reusable'
9090
9191
const {
9292
scenario,
@@ -95,7 +95,7 @@ const {
9595
capabilityError = false
9696
} = defineProps<{
9797
scenario: Scenario
98-
capability: Capability
98+
capability: PaymentMethodCapability
9999
methodType?: string
100100
capabilityError?: boolean
101101
}>()
@@ -108,16 +108,15 @@ const { manageSubscription } = useBillingContext()
108108
109109
const ctaLoading = ref(false)
110110
111-
const METHOD_LABELS: Record<string, string> = {
112-
alipay: 'Alipay',
113-
card: 'Your card',
114-
us_bank_account: 'Your bank account',
115-
link: 'Link'
116-
}
117-
118111
const methodLabel = computed(() => {
119112
if (!methodType) return t('billing.spendLimit.defaultMethod')
120-
return METHOD_LABELS[methodType] ?? t('billing.spendLimit.defaultMethod')
113+
const labels: Record<string, string | undefined> = {
114+
alipay: t('billing.spendLimit.methodLabels.alipay'),
115+
card: t('billing.spendLimit.methodLabels.card'),
116+
us_bank_account: t('billing.spendLimit.methodLabels.us_bank_account'),
117+
link: t('billing.spendLimit.methodLabels.link')
118+
}
119+
return labels[methodType] ?? t('billing.spendLimit.defaultMethod')
121120
})
122121
123122
const title = computed(() => {
@@ -128,7 +127,10 @@ const title = computed(() => {
128127
) {
129128
return t('billing.spendLimit.addPaymentMethodTitle')
130129
}
131-
return t('billing.spendLimit.paymentFailedTitle')
130+
if (scenario === 'payment_failed') {
131+
return t('billing.spendLimit.paymentFailedTitle')
132+
}
133+
return t('billing.spendLimit.addPaymentMethodTitle')
132134
})
133135
134136
const ctaLabel = computed(() => {
@@ -139,15 +141,7 @@ const ctaLabel = computed(() => {
139141
) {
140142
return t('billing.spendLimit.addPaymentMethodCta')
141143
}
142-
return t('billing.spendLimit.updatePaymentMethodCta')
143-
})
144-
145-
const ctaAriaLabel = computed(() => {
146-
if (
147-
!capabilityError &&
148-
capability === 'reusable' &&
149-
scenario === 'payment_failed'
150-
) {
144+
if (scenario === 'payment_failed') {
151145
return t('billing.spendLimit.updatePaymentMethodCta')
152146
}
153147
return t('billing.spendLimit.addPaymentMethodCta')
@@ -174,7 +168,7 @@ async function handleMainCta() {
174168
})
175169
return
176170
}
177-
const paymentWindow = window.open(url, '_blank')
171+
const paymentWindow = window.open(url, '_blank', 'noopener,noreferrer')
178172
if (!paymentWindow) {
179173
toastStore.add({
180174
severity: 'warn',

src/platform/workspace/stores/billingOperationStore.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,12 @@ export const useBillingOperationStore = defineStore('billingOperation', () => {
8484
intervals.set(opId, INITIAL_INTERVAL_MS)
8585

8686
if (type !== 'cancel') {
87-
let messageKey: string
88-
if (type === 'subscription') {
89-
messageKey = 'billingOperation.subscriptionProcessing'
90-
} else if (type === 'topup') {
91-
messageKey = 'billingOperation.topupProcessing'
92-
} else {
93-
messageKey = 'billingOperation.payOwedProcessing'
94-
}
87+
const messageKey =
88+
type === 'subscription'
89+
? 'billingOperation.subscriptionProcessing'
90+
: type === 'topup'
91+
? 'billingOperation.topupProcessing'
92+
: 'billingOperation.payOwedProcessing'
9593

9694
const toastMessage: ToastMessageOptions = {
9795
severity: 'info',

src/services/dialogService.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ import type {
1919

2020
import type { ComponentAttrs } from 'vue-component-type-helpers'
2121
import type { SubscriptionDialogReason } from '@/platform/cloud/subscription/composables/useSubscriptionDialog'
22-
import type { WorkspaceRole } from '@/platform/workspace/api/workspaceApi'
22+
import type {
23+
PaymentMethodCapability,
24+
WorkspaceRole
25+
} from '@/platform/workspace/api/workspaceApi'
2326

2427
// Lazy loaders for dialogs - components are loaded on first use
2528
const lazyApiNodesSignInContent = () =>
@@ -631,7 +634,7 @@ export const useDialogService = () => {
631634
*/
632635
async function showSpendLimitDialog(options: {
633636
scenario: 'limit_reached' | 'payment_failed'
634-
capability: 'none' | 'one_time_only' | 'reusable'
637+
capability: PaymentMethodCapability
635638
methodType?: string
636639
capabilityError?: boolean
637640
}) {

0 commit comments

Comments
 (0)