diff --git a/enterprise/app/services/enterprise/billing/currencies.rb b/enterprise/app/services/enterprise/billing/currencies.rb index 90a96e6ab..ec137af68 100644 --- a/enterprise/app/services/enterprise/billing/currencies.rb +++ b/enterprise/app/services/enterprise/billing/currencies.rb @@ -19,8 +19,7 @@ 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). + # Payment method types locked to one currency; types not listed (e.g. card) bill in any currency. CURRENCY_LOCKED_PAYMENT_METHOD_TYPES = { 'pix' => 'brl', 'boleto' => 'brl' @@ -45,8 +44,7 @@ module Enterprise::Billing::Currencies LOCALE_DEFAULTS.fetch(locale.to_s, DEFAULT) end - # Master switch for multi-currency billing (currency switch UI + non-default onboarding currency). - # Read raw from InstallationConfig so a super-admin toggle takes effect without cache staleness. + # Master switch for multi-currency billing; read raw so a super-admin toggle isn't cache-stale. def multi_currency_supported? ActiveModel::Type::Boolean.new.cast(InstallationConfig.find_by(name: 'MULTIPLE_CURRENCY_SUPPORTED')&.value) end diff --git a/enterprise/app/services/enterprise/billing/currency_switch_eligibility.rb b/enterprise/app/services/enterprise/billing/currency_switch_eligibility.rb index 4a05e3738..de6e27d02 100644 --- a/enterprise/app/services/enterprise/billing/currency_switch_eligibility.rb +++ b/enterprise/app/services/enterprise/billing/currency_switch_eligibility.rb @@ -26,8 +26,7 @@ class Enterprise::Billing::CurrencySwitchEligibility raise Error, I18n.t('errors.billing.stripe_customer_not_configured') if stripe_customer_id.blank? end - # Exactly one live subscription in a switchable state. Anything else (pending or extra) is rejected - # up front so we never mutate Stripe for an edge case. + # Exactly one live subscription in a switchable state; anything else is rejected before mutating Stripe. def eligible_subscription! subscription = live_subscriptions.first eligible = live_subscriptions.one? && SWITCHABLE_STATUSES.include?(subscription&.status) diff --git a/enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb b/enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb index a1c5358d6..2ea3dd92c 100644 --- a/enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb +++ b/enterprise/app/services/enterprise/billing/default_payment_method_reconciler.rb @@ -1,8 +1,5 @@ -# 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. -# Keyed off invoice_settings.default_payment_method (what Stripe Billing charges), inspecting it before -# falling back to a legacy default_source card so an incompatible invoice default can't be masked. +# Makes the customer's default payment method one that can bill the given currency (PIX/boleto are +# BRL-only). Drops an incompatible default, picks a compatible card, else falls back to default_source. class Enterprise::Billing::DefaultPaymentMethodReconciler pattr_initialize [:account!, :currency!] @@ -15,8 +12,7 @@ class Enterprise::Billing::DefaultPaymentMethodReconciler compatible = payment_methods.find { |method| compatible?(method) } return make_default(compatible.id) if compatible - # No compatible attached PaymentMethod. Drop an incompatible invoice default so Stripe won't charge - # it, then fall back to a legacy default_source card (currency-agnostic) the invoice can still use. + # Drop the incompatible invoice default, then fall back to a legacy default_source card if present. unset_default if current_default.present? customer.default_source.presence end diff --git a/enterprise/app/services/enterprise/billing/plan_price_resolver.rb b/enterprise/app/services/enterprise/billing/plan_price_resolver.rb index 03db18cf7..c280884f7 100644 --- a/enterprise/app/services/enterprise/billing/plan_price_resolver.rb +++ b/enterprise/app/services/enterprise/billing/plan_price_resolver.rb @@ -13,8 +13,7 @@ class Enterprise::Billing::PlanPriceResolver target_prices = by_currency[target_currency] raise Error, I18n.t('errors.billing.currency_not_available_for_plan') if target_prices.blank? - # Map by the current price's position within its own currency, so monthly->monthly / annual->annual - # instead of always landing on the first configured price (which could change the customer's cadence). + # Map by the current price's index within its currency so cadence (monthly/annual) is preserved. source_prices = by_currency.values.find { |ids| ids.include?(current_price_id) } || [] index = source_prices.index(current_price_id) || 0 target_prices[index] || target_prices.first 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 16f16c0e3..f391949ce 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,5 @@ -# 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. +# Stripe-side currency switch. Stripe forbids two currencies on one customer, so the old subscription +# is cancelled before the new one is created; on a create failure the original is re-created. class Enterprise::Billing::StripeCurrencySwitchExecutor class Error < StandardError; end @@ -18,7 +15,7 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor begin replace_subscription(subscription, change) rescue StandardError - # The subscription swap reverted to the old currency — undo the customer location change too. + # Swap reverted to the old currency — undo the location change too. sync_customer_location(previous_currency) raise end @@ -30,15 +27,13 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor 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 create_or_revert(change) create_currency_subscription(change[:new_price_id], change, idempotency_key) rescue Stripe::StripeError - # Old sub is already cancelled; re-create the original so the customer keeps a subscription, then - # surface the original failure. + # Old sub already cancelled; re-create the original to keep the customer subscribed, then re-raise. create_currency_subscription(change[:original_price_id], change, revert_idempotency_key) raise end @@ -54,13 +49,12 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor 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. + # trial_end preserves 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 - # 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. + # Fresh per attempt so a retry never replays a cancelled sub, and revert never collides with the create. def attempt_token @attempt_token ||= SecureRandom.uuid end @@ -73,15 +67,12 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor "switch-revert-#{account.id}-#{attempt_token}" end - # 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 - # one (usd) we clear any prior override so the customer matches how a usd customer is first created - # — otherwise switching away from BRL would leave a stale BR/pt-BR address on the customer. + # Push the country override for currencies that need one (BRL/PIX); clear it otherwise so switching + # to usd doesn't leave a stale BR address. def sync_customer_location(currency_code) country = Enterprise::Billing::Currencies.country_for(currency_code) locale = Enterprise::Billing::Currencies.preferred_locale_for(currency_code) diff --git a/enterprise/app/services/enterprise/billing/switch_currency_service.rb b/enterprise/app/services/enterprise/billing/switch_currency_service.rb index f60ef3c5b..03435e027 100644 --- a/enterprise/app/services/enterprise/billing/switch_currency_service.rb +++ b/enterprise/app/services/enterprise/billing/switch_currency_service.rb @@ -1,12 +1,4 @@ -# Orchestrates a billing currency switch: -# acquire per-account lock -> eligibility (no mutation) -> resolve target price -> Stripe swap -# (self-reverting) -> persist local state (last). Each concern lives in its own collaborator so this -# stays a thin coordinator. The pending marker is set under a row lock first, so a second concurrent -# switch is rejected before it can create a duplicate Stripe subscription. Any failure before the Stripe -# swap aborts cleanly with the marker cleared. The swap is the second-to-last step and local persist is -# last, so the only split-state window is a DB error on that final write (rare — moments after a healthy -# locked write); the marker's stale window then frees switching and a later subscription.updated webhook -# reconciles the attributes from Stripe. +# Orchestrates a billing currency switch: lock -> eligibility -> resolve price -> Stripe swap -> persist. class Enterprise::Billing::SwitchCurrencyService include BillingHelper @@ -15,12 +7,10 @@ class Enterprise::Billing::SwitchCurrencyService # Tags a cancelled sub so the deleted-webhook skips re-subscribing the default plan. SWITCH_METADATA_KEY = 'chatwoot_currency_switch'.freeze - # Records the in-flight switch so a second concurrent request is rejected and a crash mid-switch is - # visible; cleared on success or by the subscription webhook once it reconciles the state from Stripe. + # Marks an in-flight switch to reject concurrent requests; cleared on completion or by the webhook. PENDING_CURRENCY_KEY = 'billing_currency_switch_pending'.freeze - # A pending marker older than this is treated as abandoned (e.g. a crashed prior attempt), so a stuck - # marker can't block switches forever. Switches complete in seconds, so this is comfortably generous. + # A pending marker older than this is treated as abandoned so a crashed switch can't block forever. STALE_SWITCH_SECONDS = 10.minutes.to_i pattr_initialize [:account!, :currency!] @@ -41,13 +31,11 @@ class Enterprise::Billing::SwitchCurrencyService rescue Enterprise::Billing::CurrencySwitchEligibility::Error, Enterprise::Billing::PlanPriceResolver::Error, Enterprise::Billing::StripeCurrencySwitchExecutor::Error => e - # The Stripe swap self-reverted, so drop the pending marker and surface a single error type. + # Swap self-reverted; drop the marker and surface a single error type. clear_pending raise Error, e.message rescue Stripe::StripeError - # A raw Stripe failure in the preflight (payment-method/location sync) happens before any - # subscription change, so drop the marker too — otherwise a transient blip locks out switching - # until the stale timeout. A post-success persist failure isn't a Stripe error and is left for the webhook. + # Preflight Stripe failure (before any subscription change); clear the marker so a blip can't lock switching. clear_pending raise end @@ -55,8 +43,7 @@ class Enterprise::Billing::SwitchCurrencyService private - # Reject a second switch for this account while one is in flight. The check-and-set runs under a row - # lock so two concurrent requests can't both pass it and create duplicate Stripe subscriptions. + # Check-and-set the marker under a row lock so concurrent switches can't both create a subscription. def acquire_switch_lock! account.with_lock do raise Error, I18n.t('errors.billing.switch_in_progress') if switch_in_progress? @@ -90,11 +77,10 @@ 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. + # 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. + # Paid plans trial until paid-through; the free default plan switches immediately. paid_through: default_plan ? nil : subscription_period_end(subscription), default_plan: default_plan } diff --git a/spec/enterprise/controllers/enterprise/api/v1/accounts_controller_spec.rb b/spec/enterprise/controllers/enterprise/api/v1/accounts_controller_spec.rb index f840ddc11..795ad8334 100644 --- a/spec/enterprise/controllers/enterprise/api/v1/accounts_controller_spec.rb +++ b/spec/enterprise/controllers/enterprise/api/v1/accounts_controller_spec.rb @@ -286,6 +286,7 @@ RSpec.describe 'Enterprise Billing APIs', type: :request do limits: { 'captain_responses' => 1000 } ) allow(Stripe::Customer).to receive(:retrieve).with(stripe_customer_id).and_return(stripe_customer) + allow(Stripe::PaymentMethod).to receive(:list).and_return(Struct.new(:data).new([Struct.new(:id, :type).new('pm_test123', 'card')])) allow(Stripe::Invoice).to receive(:create).and_return(stripe_invoice) allow(Stripe::InvoiceItem).to receive(:create) allow(Stripe::Invoice).to receive(:finalize_invoice)