fix(billing): harden currency switch idempotency, usd location reset, and default-plan resolution
This commit is contained in:
@@ -36,8 +36,8 @@ module Enterprise::Billing::PlanConfiguration
|
||||
price_ids_by_currency(plan).values.flatten.compact.include?(price_id)
|
||||
end
|
||||
|
||||
def default_price?(price_id)
|
||||
plan_contains_price_id?(default_plan, price_id)
|
||||
def default_plan?(plan)
|
||||
plan.present? && plan['name'] == default_plan&.dig('name')
|
||||
end
|
||||
|
||||
# [plan, currency] for a price id, else [nil, nil].
|
||||
|
||||
@@ -10,7 +10,7 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor
|
||||
|
||||
# Returns the newly-created Stripe subscription.
|
||||
def execute(subscription:, change:)
|
||||
validate_payment_method! unless Enterprise::Billing::PlanConfiguration.default_price?(subscription['plan']['id'])
|
||||
validate_payment_method! unless change[:default_plan]
|
||||
|
||||
previous_currency = account.billing_currency
|
||||
sync_customer_location(target_currency)
|
||||
@@ -55,7 +55,13 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor
|
||||
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: "switch-#{account.id}-#{change[:key]}" })
|
||||
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}"
|
||||
end
|
||||
|
||||
def validate_payment_method!
|
||||
@@ -68,16 +74,17 @@ class Enterprise::Billing::StripeCurrencySwitchExecutor
|
||||
Stripe::Customer.update(stripe_customer_id, invoice_settings: { default_payment_method: payment_methods.data.first.id })
|
||||
end
|
||||
|
||||
# Only currencies that need a country override (e.g. BRL/PIX) push an address/locale to Stripe;
|
||||
# usd keeps Stripe's defaults, matching how the customer is first created.
|
||||
# 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.
|
||||
def sync_customer_location(currency_code)
|
||||
country = Enterprise::Billing::Currencies.country_for(currency_code)
|
||||
return if country.blank?
|
||||
locale = Enterprise::Billing::Currencies.preferred_locale_for(currency_code)
|
||||
|
||||
Stripe::Customer.update(
|
||||
stripe_customer_id,
|
||||
address: { country: country },
|
||||
preferred_locales: [Enterprise::Billing::Currencies.preferred_locale_for(currency_code)]
|
||||
address: { country: country.presence || '' },
|
||||
preferred_locales: locale.present? ? [locale] : []
|
||||
)
|
||||
end
|
||||
|
||||
|
||||
@@ -21,12 +21,13 @@ class Enterprise::Billing::SwitchCurrencyService
|
||||
def perform
|
||||
subscription = eligibility.subscription!
|
||||
resolver = Enterprise::Billing::PlanPriceResolver.new(subscription: subscription, target_currency: target_currency)
|
||||
change = change_for(subscription, resolver.target_price_id)
|
||||
plan = resolver.plan
|
||||
change = change_for(subscription, resolver.target_price_id, default_plan: Enterprise::Billing::PlanConfiguration.default_plan?(plan))
|
||||
|
||||
mark_pending
|
||||
new_subscription = executor.execute(subscription: subscription, change: change)
|
||||
|
||||
persist_currency(build_custom_attributes(new_subscription, resolver.plan))
|
||||
persist_currency(build_custom_attributes(new_subscription, plan))
|
||||
Enterprise::Billing::ReconcilePlanFeaturesService.new(account: account).perform
|
||||
rescue Enterprise::Billing::CurrencySwitchEligibility::Error,
|
||||
Enterprise::Billing::PlanPriceResolver::Error,
|
||||
@@ -50,14 +51,14 @@ class Enterprise::Billing::SwitchCurrencyService
|
||||
@target_currency ||= Enterprise::Billing::Currencies.normalize(currency)
|
||||
end
|
||||
|
||||
def change_for(subscription, new_price_id)
|
||||
def change_for(subscription, new_price_id, default_plan:)
|
||||
{
|
||||
new_price_id: new_price_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_through: Enterprise::Billing::PlanConfiguration.default_price?(subscription['plan']['id']) ? nil : subscription_period_end(subscription),
|
||||
key: subscription.id
|
||||
paid_through: default_plan ? nil : subscription_period_end(subscription),
|
||||
default_plan: default_plan
|
||||
}
|
||||
end
|
||||
|
||||
|
||||
@@ -57,6 +57,14 @@ describe Enterprise::Billing::SwitchCurrencyService do
|
||||
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
|
||||
service.perform
|
||||
|
||||
expect(Stripe::Subscription).to have_received(:create).with(
|
||||
anything, hash_including(idempotency_key: a_string_matching(/\Aswitch-#{account.id}-[0-9a-f-]{36}\z/))
|
||||
)
|
||||
end
|
||||
|
||||
it 'persists the new currency and clears the pending marker' do
|
||||
service.perform
|
||||
|
||||
@@ -146,5 +154,67 @@ describe Enterprise::Billing::SwitchCurrencyService do
|
||||
expect(attributes).not_to have_key('billing_currency_switch_pending')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a free default-plan subscription has a stale price id' do
|
||||
# Price id no longer in CHATWOOT_CLOUD_PLANS, but product_id still maps to the default (Hacker) plan.
|
||||
let(:active_subscription) do
|
||||
Stripe::Subscription.construct_from(
|
||||
id: 'sub_hacker', status: 'active', quantity: 1, current_period_end: period_end,
|
||||
plan: { id: 'price_hacker_legacy_usd', product: 'prod_hacker' }, metadata: {}
|
||||
)
|
||||
end
|
||||
let(:new_subscription) do
|
||||
Stripe::Subscription.construct_from(
|
||||
id: 'sub_hacker_brl', status: 'active', quantity: 1, current_period_end: period_end,
|
||||
plan: { id: 'price_hacker_brl', product: 'prod_hacker' }, metadata: {}
|
||||
)
|
||||
end
|
||||
|
||||
before do
|
||||
account.update!(custom_attributes: { plan_name: 'Hacker', stripe_customer_id: stripe_customer_id, billing_currency: 'usd' })
|
||||
# No payment method available — a default-plan switch must not require one.
|
||||
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([]))
|
||||
end
|
||||
|
||||
it 'switches without requiring a payment method or a paid-through trial' do
|
||||
service.perform
|
||||
|
||||
expect(Stripe::Customer).not_to have_received(:retrieve)
|
||||
expect(Stripe::Subscription).to have_received(:create).with(hash_not_including(:trial_end), anything)
|
||||
expect(account.reload.custom_attributes['billing_currency']).to eq('brl')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when switching from brl to usd' do
|
||||
let(:target_currency) { 'usd' }
|
||||
let(:active_subscription) do
|
||||
Stripe::Subscription.construct_from(
|
||||
id: 'sub_brl', status: 'active', quantity: 2, current_period_end: period_end,
|
||||
plan: { id: 'price_business_brl', product: 'prod_business' }, metadata: {}
|
||||
)
|
||||
end
|
||||
let(:new_subscription) do
|
||||
Stripe::Subscription.construct_from(
|
||||
id: 'sub_usd', status: 'trialing', quantity: 2, current_period_end: period_end,
|
||||
plan: { id: 'price_business_usd', product: 'prod_business' }, metadata: {}
|
||||
)
|
||||
end
|
||||
|
||||
before do
|
||||
account.update!(custom_attributes: { plan_name: 'Business', stripe_customer_id: stripe_customer_id, billing_currency: 'brl' })
|
||||
end
|
||||
|
||||
it 'clears the stripe billing country and locale override' do
|
||||
service.perform
|
||||
|
||||
expect(Stripe::Customer).to have_received(:update).with(
|
||||
stripe_customer_id, hash_including(address: { country: '' }, preferred_locales: [])
|
||||
)
|
||||
expect(account.reload.custom_attributes['billing_currency']).to eq('usd')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user