fix(billing): stub payment method list in topup spec and trim comments

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

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)

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

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

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)

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
     }

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)