From 49aec5517b7a3d12ef174766fe0fd5170ea86e70 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Mon, 15 Jun 2026 16:20:48 +0530 Subject: [PATCH] fix(billing): cancel-first currency switch and currency-aware default payment method --- config/locales/en.yml | 1 - .../services/enterprise/billing/currencies.rb | 13 +++++ .../default_payment_method_reconciler.rb | 45 +++++++++++++++ .../stripe_currency_switch_executor.rb | 53 +++++++++-------- .../billing/switch_currency_service.rb | 2 + .../billing/topup_checkout_service.rb | 13 +---- .../billing/switch_currency_service_spec.rb | 57 +++++++++++++++---- .../billing/topup_checkout_service_spec.rb | 1 + 8 files changed, 140 insertions(+), 45 deletions(-) create mode 100644 enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb diff --git a/config/locales/en.yml b/config/locales/en.yml index 266a82807..4d6343d9c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -179,7 +179,6 @@ en: switch_requires_active_subscription: Currency can only be changed when you have a single active subscription. Please resolve any pending billing first. unknown_plan: Could not determine the current plan currency_not_available_for_plan: The selected currency is not available for your current plan - no_payment_method: No payment methods found. Please add a payment method before switching currency. reports: date_range_too_long: Date range cannot exceed 6 months profile: diff --git a/enterprise/app/services/enterprise/billing/currencies.rb b/enterprise/app/services/enterprise/billing/currencies.rb index 50d60122e..e2c059421 100644 --- a/enterprise/app/services/enterprise/billing/currencies.rb +++ b/enterprise/app/services/enterprise/billing/currencies.rb @@ -19,6 +19,13 @@ module Enterprise::Billing::Currencies 'brl' => 'pt-BR' }.freeze + # Stripe payment method types locked to a single currency; any type not listed (e.g. card) bills + # in any currency. Used to drop a method that can't pay the customer's currency (PIX/boleto are BRL-only). + CURRENCY_LOCKED_PAYMENT_METHOD_TYPES = { + 'pix' => 'brl', + 'boleto' => 'brl' + }.freeze + module_function def normalize(code) @@ -45,4 +52,10 @@ module Enterprise::Billing::Currencies def preferred_locale_for(code) PREFERRED_LOCALE_BY_CURRENCY[to_supported(code)] end + + # Can a payment method of this Stripe type bill the given currency? + def payment_method_supports?(payment_method_type, code) + locked_currency = CURRENCY_LOCKED_PAYMENT_METHOD_TYPES[payment_method_type.to_s] + locked_currency.nil? || locked_currency == to_supported(code) + end end diff --git a/enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb b/enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb new file mode 100644 index 000000000..cbadc2f4e --- /dev/null +++ b/enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb @@ -0,0 +1,45 @@ +# Ensures the customer's default Stripe payment method can actually bill the given currency. PIX/boleto +# are BRL-only, so when an account moves to a currency they can't pay we drop them as the default and +# pick a compatible method (a card) if one is attached. Incompatible methods stay attached for later. +class Enterprise::Billing::DefaultPaymentMethodReconciler + pattr_initialize [:account!, :currency!] + + # Returns the id of a currency-compatible default payment method, or nil if none is available. + def reconcile + customer = Stripe::Customer.retrieve(stripe_customer_id) + # Legacy card sources are currency-agnostic. + return customer.default_source if customer.default_source.present? + + current_default = customer.invoice_settings.default_payment_method + return current_default if compatible?(payment_methods.find { |method| method.id == current_default }) + + compatible = payment_methods.find { |method| compatible?(method) } + return make_default(compatible.id) if compatible + + unset_default if current_default.present? + nil + end + + private + + def payment_methods + @payment_methods ||= Stripe::PaymentMethod.list(customer: stripe_customer_id, limit: 100).data + end + + def compatible?(payment_method) + payment_method.present? && Enterprise::Billing::Currencies.payment_method_supports?(payment_method.type, currency) + end + + def make_default(payment_method_id) + Stripe::Customer.update(stripe_customer_id, invoice_settings: { default_payment_method: payment_method_id }) + payment_method_id + end + + def unset_default + Stripe::Customer.update(stripe_customer_id, invoice_settings: { default_payment_method: '' }) + end + + def stripe_customer_id + account.custom_attributes['stripe_customer_id'] + end +end diff --git a/enterprise/app/services/enterprise/billing/stripe_currency_switch_executor.rb b/enterprise/app/services/enterprise/billing/stripe_currency_switch_executor.rb index fd09730f0..16f16c0e3 100644 --- a/enterprise/app/services/enterprise/billing/stripe_currency_switch_executor.rb +++ b/enterprise/app/services/enterprise/billing/stripe_currency_switch_executor.rb @@ -1,8 +1,8 @@ -# Performs the Stripe-side currency switch: sync the customer location, create the new-currency -# subscription, then cancel the old one. Stripe can't change a subscription's currency in place and -# can't prorate across currencies, so the switch is a cancel + recreate. Creating the new sub *before* -# cancelling the old one means any failure leaves the customer on their original subscription rather -# than with none — the whole operation self-reverts. +# Performs the Stripe-side currency switch: sync the customer location, cancel the old-currency +# subscription, then create the new-currency one. Stripe can't change a subscription's currency in +# place, can't prorate across currencies, and forbids two currencies on a single customer — so the old +# subscription must be cancelled *before* the new one can be created. If the create fails afterwards we +# re-create the original (its currency is free again) so the customer isn't left without a subscription. class Enterprise::Billing::StripeCurrencySwitchExecutor class Error < StandardError; end @@ -10,7 +10,7 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor # Returns the newly-created Stripe subscription. def execute(subscription:, change:) - validate_payment_method! unless change[:default_plan] + reconcile_default_payment_method unless change[:default_plan] previous_currency = account.billing_currency sync_customer_location(target_currency) @@ -27,18 +27,19 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor private def replace_subscription(subscription, change) - new_subscription = create_currency_subscription(change[:new_price_id], change) - cancel_old_subscription(subscription, new_subscription) - new_subscription + cancel_subscription(subscription) + create_or_revert(change) rescue Stripe::StripeError => e + # Reaches here only if cancel itself failed (old sub still active) or the revert create failed. raise Error, e.message end - def cancel_old_subscription(old_subscription, new_subscription) - cancel_subscription(old_subscription) + def create_or_revert(change) + create_currency_subscription(change[:new_price_id], change, idempotency_key) rescue Stripe::StripeError - # Couldn't retire the old sub: cancel the just-created one so the customer keeps a single subscription. - Stripe::Subscription.cancel(new_subscription.id, { prorate: false }) + # Old sub is already cancelled; re-create the original so the customer keeps a subscription, then + # surface the original failure. + create_currency_subscription(change[:original_price_id], change, revert_idempotency_key) raise end @@ -51,27 +52,31 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor raise end - def create_currency_subscription(price_id, change) + def create_currency_subscription(price_id, change, idempotency_key) params = { customer: stripe_customer_id, items: [{ price: price_id, quantity: change[:quantity] }] } # trial_end preserves the already-paid time so switching mid-cycle doesn't double-charge. params[:trial_end] = change[:paid_through] if change[:paid_through].present? && change[:paid_through] > Time.current.to_i Stripe::Subscription.create(params, { idempotency_key: idempotency_key }) end - # Fresh per switch attempt: a retry after a rolled-back (cancelled) create must create a new - # subscription, not replay Stripe's stored response for the now-cancelled one. - def idempotency_key - @idempotency_key ||= "switch-#{account.id}-#{SecureRandom.uuid}" + # Distinct keys per switch attempt: a retry must never replay a cancelled subscription, and the + # revert create must never be conflated with the forward create. + def attempt_token + @attempt_token ||= SecureRandom.uuid end - def validate_payment_method! - customer = Stripe::Customer.retrieve(stripe_customer_id) - return if customer.invoice_settings.default_payment_method.present? || customer.default_source.present? + def idempotency_key + "switch-#{account.id}-#{attempt_token}" + end - payment_methods = Stripe::PaymentMethod.list(customer: stripe_customer_id, limit: 1) - raise Error, I18n.t('errors.billing.no_payment_method') if payment_methods.data.empty? + def revert_idempotency_key + "switch-revert-#{account.id}-#{attempt_token}" + end - Stripe::Customer.update(stripe_customer_id, invoice_settings: { default_payment_method: payment_methods.data.first.id }) + # Drop a default that can't bill the new currency (e.g. PIX on a USD switch) and pick a compatible one + # if attached; leaving none is fine — the user is prompted to add a method before the next charge. + def reconcile_default_payment_method + Enterprise::Billing::DefaultPaymentMethodReconciler.new(account: account, currency: target_currency).reconcile end # Currencies that need a country override (e.g. BRL/PIX) push it to Stripe; for currencies without diff --git a/enterprise/app/services/enterprise/billing/switch_currency_service.rb b/enterprise/app/services/enterprise/billing/switch_currency_service.rb index 17e78ccbf..155aec1d6 100644 --- a/enterprise/app/services/enterprise/billing/switch_currency_service.rb +++ b/enterprise/app/services/enterprise/billing/switch_currency_service.rb @@ -54,6 +54,8 @@ class Enterprise::Billing::SwitchCurrencyService def change_for(subscription, new_price_id, default_plan:) { new_price_id: new_price_id, + # Original price is needed to re-create the subscription if the new-currency create fails. + original_price_id: subscription['plan']['id'], quantity: subscription['quantity'], # Paid plans preserve paid-through (new sub trials until then); the free default plan switches # immediately to an active sub, so a default-plan account can switch again any time. diff --git a/enterprise/app/services/enterprise/billing/topup_checkout_service.rb b/enterprise/app/services/enterprise/billing/topup_checkout_service.rb index 5588c0c71..0d1cc0be6 100644 --- a/enterprise/app/services/enterprise/billing/topup_checkout_service.rb +++ b/enterprise/app/services/enterprise/billing/topup_checkout_service.rb @@ -34,22 +34,15 @@ class Enterprise::Billing::TopupCheckoutService topup_option = find_topup_option(credits) raise Error, I18n.t('errors.topup.invalid_option') unless topup_option - # Validate payment method exists + # Ensure a default payment method that can bill the account's currency (PIX can't pay a USD invoice). validate_payment_method! topup_option end def validate_payment_method! - customer = Stripe::Customer.retrieve(stripe_customer_id) - - return if customer.invoice_settings.default_payment_method.present? || customer.default_source.present? - - # Auto-set first payment method as default if available - payment_methods = Stripe::PaymentMethod.list(customer: stripe_customer_id, limit: 1) - raise Error, I18n.t('errors.topup.no_payment_method') if payment_methods.data.empty? - - Stripe::Customer.update(stripe_customer_id, invoice_settings: { default_payment_method: payment_methods.data.first.id }) + reconciler = Enterprise::Billing::DefaultPaymentMethodReconciler.new(account: account, currency: account.billing_currency) + raise Error, I18n.t('errors.topup.no_payment_method') if reconciler.reconcile.blank? end def charge_customer(topup_option, credits) diff --git a/spec/enterprise/services/enterprise/billing/switch_currency_service_spec.rb b/spec/enterprise/services/enterprise/billing/switch_currency_service_spec.rb index 21d64e99c..de2f754ba 100644 --- a/spec/enterprise/services/enterprise/billing/switch_currency_service_spec.rb +++ b/spec/enterprise/services/enterprise/billing/switch_currency_service_spec.rb @@ -24,6 +24,7 @@ describe Enterprise::Billing::SwitchCurrencyService do let(:invoice_settings) { Struct.new(:default_payment_method).new('pm_test') } let(:stripe_customer) { Struct.new(:invoice_settings, :default_source).new(invoice_settings, nil) } + let(:default_payment_methods) { [Struct.new(:id, :type).new('pm_test', 'card')] } before do create(:installation_config, name: 'CHATWOOT_CLOUD_PLANS', value: [ @@ -41,20 +42,21 @@ describe Enterprise::Billing::SwitchCurrencyService do allow(Stripe::Subscription).to receive(:cancel) allow(Stripe::Customer).to receive(:retrieve).and_return(stripe_customer) allow(Stripe::Customer).to receive(:update) + allow(Stripe::PaymentMethod).to receive(:list).and_return(Struct.new(:data).new(default_payment_methods)) reconcile = instance_double(Enterprise::Billing::ReconcilePlanFeaturesService, perform: true) allow(Enterprise::Billing::ReconcilePlanFeaturesService).to receive(:new).and_return(reconcile) end describe '#perform' do - it 'creates the new-currency subscription before cancelling the old one' do + it 'cancels the old subscription before creating the new-currency one' do service.perform + expect(Stripe::Subscription).to have_received(:cancel).with('sub_usd', { prorate: false }).ordered expect(Stripe::Subscription).to have_received(:create).with( hash_including(customer: stripe_customer_id, items: [{ price: 'price_business_brl', quantity: 2 }]), hash_including(:idempotency_key) ).ordered - expect(Stripe::Subscription).to have_received(:cancel).with('sub_usd', { prorate: false }).ordered end it 'uses a per-attempt idempotency key not derived from the subscription id' do @@ -124,26 +126,33 @@ describe Enterprise::Billing::SwitchCurrencyService do expect { service.perform }.to raise_error(described_class::Error, I18n.t('errors.billing.currency_not_available_for_plan')) end - it 'raises when the customer has no payment method' do + it 'completes the switch without a payment method (the user is prompted later)' do allow(Stripe::Customer).to receive(:retrieve).and_return(Struct.new(:invoice_settings, :default_source).new( Struct.new(:default_payment_method).new(nil), nil )) allow(Stripe::PaymentMethod).to receive(:list).and_return(Struct.new(:data).new([])) - expect { service.perform }.to raise_error(described_class::Error, I18n.t('errors.billing.no_payment_method')) + expect { service.perform }.not_to raise_error + expect(account.reload.custom_attributes['billing_currency']).to eq('brl') end - context 'when creating the new subscription fails' do + context 'when creating the new-currency subscription fails' do before do - allow(Stripe::Subscription).to receive(:create).and_raise(Stripe::StripeError.new('card declined')) + # Only the new (brl) create fails; re-creating the original (usd) succeeds. + allow(Stripe::Subscription).to receive(:create) do |params, _opts| + raise Stripe::StripeError, 'cannot combine currencies' if params[:items].first[:price] == 'price_business_brl' + + active_subscription + end end - it 'leaves the old subscription untouched after the failed switch' do + it 'cancels the old subscription then re-creates the original to restore service' do expect { service.perform }.to raise_error(described_class::Error) - expect(Stripe::Subscription).not_to have_received(:cancel) - # The target (brl) location override was pushed before the create attempt failed. - expect(Stripe::Customer).to have_received(:update).with(stripe_customer_id, hash_including(address: { country: 'BR' })) + expect(Stripe::Subscription).to have_received(:cancel).with('sub_usd', { prorate: false }) + expect(Stripe::Subscription).to have_received(:create).with( + hash_including(items: [{ price: 'price_business_usd', quantity: 2 }]), anything + ) end it 'keeps the account on the original currency and clears the pending marker' do @@ -215,6 +224,34 @@ describe Enterprise::Billing::SwitchCurrencyService do ) expect(account.reload.custom_attributes['billing_currency']).to eq('usd') end + + context 'when the default payment method cannot bill the new currency' do + let(:pix) { Struct.new(:id, :type).new('pm_pix', 'pix') } + let(:card) { Struct.new(:id, :type).new('pm_card', 'card') } + + before do + allow(Stripe::Customer).to receive(:retrieve).and_return( + Struct.new(:invoice_settings, :default_source).new(Struct.new(:default_payment_method).new('pm_pix'), nil) + ) + end + + it 'switches the default to an attached compatible card' do + allow(Stripe::PaymentMethod).to receive(:list).and_return(Struct.new(:data).new([pix, card])) + + service.perform + + expect(Stripe::Customer).to have_received(:update).with(stripe_customer_id, invoice_settings: { default_payment_method: 'pm_card' }) + end + + it 'unsets the default when no compatible method is attached' do + allow(Stripe::PaymentMethod).to receive(:list).and_return(Struct.new(:data).new([pix])) + + service.perform + + expect(Stripe::Customer).to have_received(:update).with(stripe_customer_id, invoice_settings: { default_payment_method: '' }) + expect(account.reload.custom_attributes['billing_currency']).to eq('usd') + end + end end end end diff --git a/spec/enterprise/services/enterprise/billing/topup_checkout_service_spec.rb b/spec/enterprise/services/enterprise/billing/topup_checkout_service_spec.rb index 2629bdbd9..fa42ea5ad 100644 --- a/spec/enterprise/services/enterprise/billing/topup_checkout_service_spec.rb +++ b/spec/enterprise/services/enterprise/billing/topup_checkout_service_spec.rb @@ -30,6 +30,7 @@ describe Enterprise::Billing::TopupCheckoutService do ) allow(Stripe::Customer).to receive(:retrieve).and_return(stripe_customer) + allow(Stripe::PaymentMethod).to receive(:list).and_return(Struct.new(:data).new([Struct.new(:id, :type).new('pm_test', 'card')])) allow(Stripe::Invoice).to receive(:create).and_return(stripe_invoice) allow(Stripe::InvoiceItem).to receive(:create) allow(Stripe::Invoice).to receive(:finalize_invoice)