From b5010caca31c2efa1f23685a640cd187f14d642c Mon Sep 17 00:00:00 2001 From: Wojciech Zakrzewski Date: Thu, 20 Aug 2026 16:56:59 +0200 Subject: [PATCH 1/5] Fix ACH unstore payment method selection --- .../gateways/stripe_ach_setup_intents.rb | 42 ++++++++----- .../gateways/stripe_ach_setup_intents_test.rb | 59 +++++++++++++++++++ 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb index 6de9df0f2b7..bc34fcf3165 100644 --- a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb +++ b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb @@ -80,19 +80,32 @@ def void(identification, options = {}) commit(:post, "refunds", { payment_intent: identification }, options) end - # Detaches the customer's us_bank_account PaymentMethod(s). The legacy StripeGateway#unstore - # deletes a card source (customers/{id}/cards/{id}), which does not apply to PaymentMethod-based - # us_bank_account profiles. + # Detaches the one us_bank_account PaymentMethod this profile owns. The legacy + # StripeGateway#unstore deletes a card source (customers/{id}/cards/{id}), which does not apply + # to PaymentMethod-based us_bank_account profiles. One Stripe customer can back several payment + # profiles, so detaching every listed method would remove instruments belonging to profiles the + # caller never asked about. Raises on ambiguity rather than guessing, matching + # #us_bank_account_payment_method_for_customer. def unstore(identification, options = {}, _deprecated_options = {}) - customer_id = identification.to_s.split("|").first - payment_methods = list_us_bank_account_payment_methods(customer_id) - return Response.new(true, "No us_bank_account payment method to detach") if payment_methods.empty? - - MultiResponse.run(:first) do |r| - payment_methods.each do |payment_method| - r.process { commit(:post, "payment_methods/#{CGI.escape(payment_method["id"])}/detach", {}, options) } + customer_id, instrument_id = identification.to_s.split("|") + listed = list_us_bank_account_payment_methods(customer_id).map { |pm| pm["id"] } + return Response.new(true, "No us_bank_account payment method to detach") if listed.empty? + + payment_method_id = + if listed.include?(instrument_id) + instrument_id + elsif listed.size == 1 + listed.first + else + customer_default_payment_method(customer_id) end + + unless listed.include?(payment_method_id) + raise StripeCustomerManyPaymentMethodWithoutDefault, + "Customer has more than one us_bank_account payment method and none identifies this profile." end + + commit(:post, "payment_methods/#{CGI.escape(payment_method_id)}/detach", {}, options) end private @@ -192,11 +205,10 @@ def customer_acceptance(options) # back to the single listed us_bank_account PM. Raises on ambiguity or absence rather than # silently charging the wrong method. def us_bank_account_payment_method_for_customer(customer) - # Only modern PaymentMethods (pm_*) are valid as a PaymentIntent payment_method. Stripe can - # list legacy bank-account sources here with their original ba_* id; those must not be used. - payment_method_ids = list_us_bank_account_payment_methods(customer) - .map { |pm| pm["id"] } - .select { |id| id.to_s.start_with?("pm_") } + # Deliberately not filtered by id prefix. A legacy bank account keeps its ba_* id after Stripe + # mandates it, and PaymentIntent accepts that id; filtering to pm_* excluded the whole legacy + # book, which is the book this gateway exists to charge. + payment_method_ids = list_us_bank_account_payment_methods(customer).map { |pm| pm["id"] } return payment_method_ids.first if payment_method_ids.size == 1 diff --git a/test/unit/gateways/stripe_ach_setup_intents_test.rb b/test/unit/gateways/stripe_ach_setup_intents_test.rb index 9916754d2d6..4345871cf09 100644 --- a/test/unit/gateways/stripe_ach_setup_intents_test.rb +++ b/test/unit/gateways/stripe_ach_setup_intents_test.rb @@ -166,6 +166,18 @@ def test_purchase_raises_when_customer_has_no_us_bank_account_pm end end + def test_purchase_uses_a_legacy_bank_account_id_as_the_payment_method + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") + end.returns(payment_methods_list_with_single_legacy_bank_account) + @gateway.expects(:ssl_request).with do |_m, endpoint, post, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_intents") && + post.include?("payment_method=ba_legacy123") + end.returns(successful_payment_intent_response("processing")) + + assert_success @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) + end + # --- refund -------------------------------------------------------------------------------- def test_refund_refunds_by_payment_intent @@ -201,6 +213,41 @@ def test_unstore_is_a_noop_when_no_bank_account_pm_exists assert_success @gateway.unstore("cus_ACH") end + def test_unstore_detaches_only_the_payment_method_named_in_the_identification + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") + end.returns(payment_methods_list_with_two_bank_accounts) + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint == "https://api.stripe.com/v1/payment_methods/pm_bank999/detach" + end.returns(successful_detach_response) + + assert_success @gateway.unstore("cus_ACH|pm_bank999") + end + + def test_unstore_detaches_a_legacy_bank_account_id + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") + end.returns(payment_methods_list_with_single_legacy_bank_account) + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint == "https://api.stripe.com/v1/payment_methods/ba_legacy123/detach" + end.returns(successful_detach_response) + + assert_success @gateway.unstore("cus_ACH|ba_legacy123") + end + + def test_unstore_raises_rather_than_detaching_every_method_when_the_profile_is_ambiguous + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") + end.returns(payment_methods_list_with_two_bank_accounts) + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint == "https://api.stripe.com/v1/customers/cus_ACH" + end.returns(customer_without_default_payment_method) + + assert_raises(StripeCustomerManyPaymentMethodWithoutDefault) do + @gateway.unstore("cus_ACH") + end + end + private def stub_request_for(_endpoint, response) @@ -243,6 +290,18 @@ def payment_methods_list_with_single_bank_account %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}], "livemode": false}) end + def payment_methods_list_with_single_legacy_bank_account + %({"object": "list", "data": [{"id": "ba_legacy123", "type": "us_bank_account"}], "livemode": false}) + end + + def payment_methods_list_with_two_bank_accounts + %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}, {"id": "pm_bank999", "type": "us_bank_account"}], "livemode": false}) + end + + def customer_without_default_payment_method + %({"id": "cus_ACH", "object": "customer", "invoice_settings": {"default_payment_method": null}, "livemode": false}) + end + def empty_payment_methods_list %({"object": "list", "data": [], "livemode": false}) end From 9a179fc623cdd313ab4dd258fd57251a8e111725 Mon Sep 17 00:00:00 2001 From: Wojciech Zakrzewski Date: Fri, 21 Aug 2026 12:40:39 +0200 Subject: [PATCH 2/5] Pin that several instruments without a default cannot be charged --- .../gateways/stripe_ach_setup_intents_test.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/unit/gateways/stripe_ach_setup_intents_test.rb b/test/unit/gateways/stripe_ach_setup_intents_test.rb index 4345871cf09..4291a57b82e 100644 --- a/test/unit/gateways/stripe_ach_setup_intents_test.rb +++ b/test/unit/gateways/stripe_ach_setup_intents_test.rb @@ -178,6 +178,21 @@ def test_purchase_uses_a_legacy_bank_account_id_as_the_payment_method assert_success @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) end + # Documents why Stripe::AchRoutingState keeps a customer with several legacy instruments on the + # legacy path: here there is nothing to disambiguate them with, so the charge cannot be made at all. + def test_purchase_raises_when_several_instruments_have_no_usable_default + @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") + end.returns(payment_methods_list_with_two_legacy_bank_accounts) + @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/customers/") + end.returns(customer_without_default_payment_method) + + assert_raises(ActiveMerchant::Billing::StripeCustomerManyPaymentMethodWithoutDefault) do + @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) + end + end + # --- refund -------------------------------------------------------------------------------- def test_refund_refunds_by_payment_intent @@ -298,6 +313,10 @@ def payment_methods_list_with_two_bank_accounts %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}, {"id": "pm_bank999", "type": "us_bank_account"}], "livemode": false}) end + def payment_methods_list_with_two_legacy_bank_accounts + %({"object": "list", "data": [{"id": "ba_legacy123", "type": "us_bank_account"}, {"id": "ba_legacy999", "type": "us_bank_account"}], "livemode": false}) + end + def customer_without_default_payment_method %({"id": "cus_ACH", "object": "customer", "invoice_settings": {"default_payment_method": null}, "livemode": false}) end From e9e076f0307489faea7a336a24af5f0fa8a55a6c Mon Sep 17 00:00:00 2001 From: Wojciech Zakrzewski Date: Fri, 21 Aug 2026 15:35:42 +0200 Subject: [PATCH 3/5] Prefer the single modern payment method over a legacy sibling --- .../gateways/stripe_ach_setup_intents.rb | 7 ++++++ .../gateways/stripe_ach_setup_intents_test.rb | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb index bc34fcf3165..ceedb2942b0 100644 --- a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb +++ b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb @@ -218,6 +218,13 @@ def us_bank_account_payment_method_for_customer(customer) default = customer_default_payment_method(customer) return default if default && payment_method_ids.include?(default) + # Before giving up, prefer the single modern PaymentMethod if there is exactly one. Removing + # the pm_* filter above widened this list, and a customer holding one pm_* beside a legacy + # ba_* used to reduce to that one pm_* and charge fine. Without this the widening turns a + # working profile into a raise, which is the one regression the change could cause. + modern = payment_method_ids.select { |id| id.to_s.start_with?("pm_") } + return modern.first if modern.size == 1 + raise StripeCustomerManyPaymentMethodWithoutDefault, "Customer has more than one us_bank_account payment method but no default one." end diff --git a/test/unit/gateways/stripe_ach_setup_intents_test.rb b/test/unit/gateways/stripe_ach_setup_intents_test.rb index 4291a57b82e..5a816376d93 100644 --- a/test/unit/gateways/stripe_ach_setup_intents_test.rb +++ b/test/unit/gateways/stripe_ach_setup_intents_test.rb @@ -193,6 +193,24 @@ def test_purchase_raises_when_several_instruments_have_no_usable_default end end + # A customer holding one modern PaymentMethod beside a legacy bank account charged fine before the + # pm_* filter was removed, because the filter reduced the list to that one id. Widening the list must + # not turn that into a raise. + def test_purchase_prefers_the_single_modern_payment_method_over_a_legacy_sibling + @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") + end.returns(payment_methods_list_with_a_modern_and_a_legacy_bank_account) + @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/customers/") + end.returns(customer_without_default_payment_method) + @gateway.expects(:ssl_request).with do |_m, endpoint, post, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_intents") && + post.include?("payment_method=pm_bank123") + end.returns(successful_payment_intent_response("processing")) + + assert_success @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) + end + # --- refund -------------------------------------------------------------------------------- def test_refund_refunds_by_payment_intent @@ -313,6 +331,10 @@ def payment_methods_list_with_two_bank_accounts %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}, {"id": "pm_bank999", "type": "us_bank_account"}], "livemode": false}) end + def payment_methods_list_with_a_modern_and_a_legacy_bank_account + %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}, {"id": "ba_legacy123", "type": "us_bank_account"}], "livemode": false}) + end + def payment_methods_list_with_two_legacy_bank_accounts %({"object": "list", "data": [{"id": "ba_legacy123", "type": "us_bank_account"}, {"id": "ba_legacy999", "type": "us_bank_account"}], "livemode": false}) end From 8cbff29142b2736bc29b3a0e00df599146c87fe7 Mon Sep 17 00:00:00 2001 From: Wojciech Zakrzewski Date: Mon, 24 Aug 2026 11:55:07 +0200 Subject: [PATCH 4/5] Treat a named but detached instrument as already unstored --- .../billing/gateways/stripe_ach_setup_intents.rb | 7 +++++++ test/unit/gateways/stripe_ach_setup_intents_test.rb | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb index ceedb2942b0..33e120486ad 100644 --- a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb +++ b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb @@ -91,6 +91,13 @@ def unstore(identification, options = {}, _deprecated_options = {}) listed = list_us_bank_account_payment_methods(customer_id).map { |pm| pm["id"] } return Response.new(true, "No us_bank_account payment method to detach") if listed.empty? + # A named instrument that is no longer attached means the detach already happened. Falling + # through to the singleton or default guesses would detach an instrument the caller never + # named — possibly a sibling profile's — so this succeeds as a no-op instead. + if instrument_id && !listed.include?(instrument_id) + return Response.new(true, "Payment method already detached") + end + payment_method_id = if listed.include?(instrument_id) instrument_id diff --git a/test/unit/gateways/stripe_ach_setup_intents_test.rb b/test/unit/gateways/stripe_ach_setup_intents_test.rb index 5a816376d93..133afd956bf 100644 --- a/test/unit/gateways/stripe_ach_setup_intents_test.rb +++ b/test/unit/gateways/stripe_ach_setup_intents_test.rb @@ -268,6 +268,17 @@ def test_unstore_detaches_a_legacy_bank_account_id assert_success @gateway.unstore("cus_ACH|ba_legacy123") end + def test_unstore_is_a_noop_when_the_named_instrument_is_already_detached + @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| + endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") + end.returns(payment_methods_list_with_single_bank_account) + + response = @gateway.unstore("cus_ACH|ba_gone123") + + assert_success response + assert_equal "Payment method already detached", response.message + end + def test_unstore_raises_rather_than_detaching_every_method_when_the_profile_is_ambiguous @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") From 38424e39ae64f93c48696ea13cacb8e77c7923b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Ba=C4=87kowski?= Date: Fri, 28 Aug 2026 11:10:50 +0200 Subject: [PATCH 5/5] Refactor (#258) --- .../gateways/stripe_ach_setup_intents.rb | 64 +++---- .../gateways/stripe_ach_setup_intents_test.rb | 169 +++++------------- 2 files changed, 64 insertions(+), 169 deletions(-) diff --git a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb index 33e120486ad..f22ec2aac9c 100644 --- a/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb +++ b/lib/active_merchant/billing/gateways/stripe_ach_setup_intents.rb @@ -84,35 +84,22 @@ def void(identification, options = {}) # StripeGateway#unstore deletes a card source (customers/{id}/cards/{id}), which does not apply # to PaymentMethod-based us_bank_account profiles. One Stripe customer can back several payment # profiles, so detaching every listed method would remove instruments belonging to profiles the - # caller never asked about. Raises on ambiguity rather than guessing, matching - # #us_bank_account_payment_method_for_customer. + # caller never asked about. def unstore(identification, options = {}, _deprecated_options = {}) customer_id, instrument_id = identification.to_s.split("|") listed = list_us_bank_account_payment_methods(customer_id).map { |pm| pm["id"] } - return Response.new(true, "No us_bank_account payment method to detach") if listed.empty? - - # A named instrument that is no longer attached means the detach already happened. Falling - # through to the singleton or default guesses would detach an instrument the caller never - # named — possibly a sibling profile's — so this succeeds as a no-op instead. - if instrument_id && !listed.include?(instrument_id) - return Response.new(true, "Payment method already detached") - end - - payment_method_id = - if listed.include?(instrument_id) - instrument_id - elsif listed.size == 1 - listed.first - else - customer_default_payment_method(customer_id) - end + # Nothing listed, or a named instrument no longer listed: the detach already happened. + already_detached = listed.empty? || (instrument_id && !listed.include?(instrument_id)) + return Response.new(true, "No us_bank_account payment method to detach") if already_detached + payment_method_id = instrument_id || + (listed.size == 1 ? listed.first : customer_default_payment_method(customer_id)) unless listed.include?(payment_method_id) raise StripeCustomerManyPaymentMethodWithoutDefault, "Customer has more than one us_bank_account payment method and none identifies this profile." end - commit(:post, "payment_methods/#{CGI.escape(payment_method_id)}/detach", {}, options) + commit(:post, "payment_methods/#{payment_method_id}/detach", {}, options) end private @@ -212,31 +199,22 @@ def customer_acceptance(options) # back to the single listed us_bank_account PM. Raises on ambiguity or absence rather than # silently charging the wrong method. def us_bank_account_payment_method_for_customer(customer) - # Deliberately not filtered by id prefix. A legacy bank account keeps its ba_* id after Stripe - # mandates it, and PaymentIntent accepts that id; filtering to pm_* excluded the whole legacy - # book, which is the book this gateway exists to charge. - payment_method_ids = list_us_bank_account_payment_methods(customer).map { |pm| pm["id"] } - - return payment_method_ids.first if payment_method_ids.size == 1 - - if payment_method_ids.size > 1 - # Disambiguate by the customer's default payment method, but only when the default is - # itself one of the us_bank_account methods (it could be a card). - default = customer_default_payment_method(customer) - return default if default && payment_method_ids.include?(default) - - # Before giving up, prefer the single modern PaymentMethod if there is exactly one. Removing - # the pm_* filter above widened this list, and a customer holding one pm_* beside a legacy - # ba_* used to reduce to that one pm_* and charge fine. Without this the widening turns a - # working profile into a raise, which is the one regression the change could cause. - modern = payment_method_ids.select { |id| id.to_s.start_with?("pm_") } - return modern.first if modern.size == 1 + # Not filtered by id prefix: a legacy bank account keeps its ba_* id, and PaymentIntent takes it. + ids = list_us_bank_account_payment_methods(customer).map { |pm| pm["id"] } - raise StripeCustomerManyPaymentMethodWithoutDefault, - "Customer has more than one us_bank_account payment method but no default one." - end + raise RuntimeError, "Customer has no us_bank_account payment method." if ids.empty? + return ids.first if ids.size == 1 + + # The default is only usable when it is itself one of the us_bank_account methods (it could be a card). + default = customer_default_payment_method(customer) + return default if ids.include?(default) + + # A single modern PM beside a legacy ba_* charged fine before the pm_* filter went away; keep it working. + modern = ids.grep(/\Apm_/) + return modern.first if modern.size == 1 - raise RuntimeError, "Customer has no us_bank_account payment method." + raise StripeCustomerManyPaymentMethodWithoutDefault, + "Customer has more than one us_bank_account payment method but no default one." end def list_us_bank_account_payment_methods(customer) diff --git a/test/unit/gateways/stripe_ach_setup_intents_test.rb b/test/unit/gateways/stripe_ach_setup_intents_test.rb index 133afd956bf..239ac050c90 100644 --- a/test/unit/gateways/stripe_ach_setup_intents_test.rb +++ b/test/unit/gateways/stripe_ach_setup_intents_test.rb @@ -59,13 +59,9 @@ def test_store_rejects_non_bank_account_payment end def test_store_creates_us_bank_account_pm_and_setup_intent_on_new_customer - stub_pm = stub_request_for("payment_methods", successful_payment_method_response) - stub_customer = stub_request_for("customers", successful_new_customer_response) - stub_setup_intent = stub_request_for("setup_intents", successful_setup_intent_response) - - expect_ssl_for("/v1/payment_methods", stub_pm) - expect_ssl_for("/v1/customers", stub_customer) - expect_ssl_for("/v1/setup_intents", stub_setup_intent) + expect_ssl_for("/v1/payment_methods", successful_payment_method_response) + expect_ssl_for("/v1/customers", successful_new_customer_response) + expect_ssl_for("/v1/setup_intents", successful_setup_intent_response) response = @gateway.store(@check, @options) @@ -83,23 +79,15 @@ def test_store_creates_payment_method_with_us_bank_account_fields_and_mapped_hol post.include?("us_bank_account[account_holder_type]=individual") && # personal -> individual post.include?("us_bank_account[account_type]=checking") end.returns(successful_payment_method_response) - @gateway.stubs(:ssl_request).with do |_method, endpoint, _post, _headers| - endpoint.start_with?("https://api.stripe.com/v1/customers") - end.returns(successful_new_customer_response) - @gateway.stubs(:ssl_request).with do |_method, endpoint, _post, _headers| - endpoint.start_with?("https://api.stripe.com/v1/setup_intents") - end.returns(successful_setup_intent_response) + stub_ssl_for("/v1/customers", successful_new_customer_response) + stub_ssl_for("/v1/setup_intents", successful_setup_intent_response) assert_success @gateway.store(@check, @options) end def test_store_uses_online_mandate_for_browser_channel - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods") - end.returns(successful_payment_method_response) - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/customers") - end.returns(successful_new_customer_response) + stub_ssl_for("/v1/payment_methods", successful_payment_method_response) + stub_ssl_for("/v1/customers", successful_new_customer_response) @gateway.expects(:ssl_request).with do |_m, endpoint, post, _h| endpoint.start_with?("https://api.stripe.com/v1/setup_intents") && post.include?("mandate_data[customer_acceptance][type]=online") && @@ -111,16 +99,10 @@ def test_store_uses_online_mandate_for_browser_channel end def test_store_uses_offline_mandate_for_api_channel - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods") - end.returns(successful_payment_method_response) - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/customers") - end.returns(successful_new_customer_response) - @gateway.expects(:ssl_request).with do |_m, endpoint, post, _h| - endpoint.start_with?("https://api.stripe.com/v1/setup_intents") && - post.include?("mandate_data[customer_acceptance][type]=offline") - end.returns(successful_setup_intent_response) + stub_ssl_for("/v1/payment_methods", successful_payment_method_response) + stub_ssl_for("/v1/customers", successful_new_customer_response) + expect_ssl_for("/v1/setup_intents", successful_setup_intent_response, + "mandate_data[customer_acceptance][type]=offline") assert_success @gateway.store(@check, @options.merge(channel: "api")) end @@ -128,9 +110,7 @@ def test_store_uses_offline_mandate_for_api_channel # --- purchase ------------------------------------------------------------------------------ def test_purchase_creates_off_session_payment_intent_with_resolved_bank_pm - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_single_bank_account) + expect_ssl_for("payment_methods?customer", payment_methods_list("pm_bank123")) @gateway.expects(:ssl_request).with do |_m, endpoint, post, _h| endpoint.start_with?("https://api.stripe.com/v1/payment_intents") && post.include?("off_session=true") && @@ -146,20 +126,14 @@ def test_purchase_creates_off_session_payment_intent_with_resolved_bank_pm end def test_purchase_treats_processing_payment_intent_as_success - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_single_bank_account) - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_intents") - end.returns(successful_payment_intent_response("processing")) + stub_ssl_for("payment_methods?customer", payment_methods_list("pm_bank123")) + stub_ssl_for("/v1/payment_intents", successful_payment_intent_response("processing")) assert_success @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) end def test_purchase_raises_when_customer_has_no_us_bank_account_pm - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(empty_payment_methods_list) + stub_ssl_for("payment_methods?customer", payment_methods_list) assert_raises(RuntimeError) do @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) @@ -167,13 +141,9 @@ def test_purchase_raises_when_customer_has_no_us_bank_account_pm end def test_purchase_uses_a_legacy_bank_account_id_as_the_payment_method - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_single_legacy_bank_account) - @gateway.expects(:ssl_request).with do |_m, endpoint, post, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_intents") && - post.include?("payment_method=ba_legacy123") - end.returns(successful_payment_intent_response("processing")) + expect_ssl_for("payment_methods?customer", payment_methods_list("ba_legacy123")) + expect_ssl_for("/v1/payment_intents", successful_payment_intent_response("processing"), + "payment_method=ba_legacy123") assert_success @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) end @@ -181,32 +151,18 @@ def test_purchase_uses_a_legacy_bank_account_id_as_the_payment_method # Documents why Stripe::AchRoutingState keeps a customer with several legacy instruments on the # legacy path: here there is nothing to disambiguate them with, so the charge cannot be made at all. def test_purchase_raises_when_several_instruments_have_no_usable_default - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_two_legacy_bank_accounts) - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/customers/") - end.returns(customer_without_default_payment_method) + stub_ssl_for("payment_methods?customer", payment_methods_list("ba_legacy123", "ba_legacy999")) + stub_ssl_for("/v1/customers/", customer_without_default_payment_method) assert_raises(ActiveMerchant::Billing::StripeCustomerManyPaymentMethodWithoutDefault) do @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) end end - # A customer holding one modern PaymentMethod beside a legacy bank account charged fine before the - # pm_* filter was removed, because the filter reduced the list to that one id. Widening the list must - # not turn that into a raise. def test_purchase_prefers_the_single_modern_payment_method_over_a_legacy_sibling - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_a_modern_and_a_legacy_bank_account) - @gateway.stubs(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/customers/") - end.returns(customer_without_default_payment_method) - @gateway.expects(:ssl_request).with do |_m, endpoint, post, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_intents") && - post.include?("payment_method=pm_bank123") - end.returns(successful_payment_intent_response("processing")) + stub_ssl_for("payment_methods?customer", payment_methods_list("pm_bank123", "ba_legacy123")) + stub_ssl_for("/v1/customers/", customer_without_default_payment_method) + expect_ssl_for("/v1/payment_intents", successful_payment_intent_response("processing"), "payment_method=pm_bank123") assert_success @gateway.purchase(@amount, nil, @options.merge(customer: "cus_ACH")) end @@ -228,64 +184,44 @@ def test_refund_refunds_by_payment_intent # --- unstore (detach) ---------------------------------------------------------------------- def test_unstore_detaches_us_bank_account_payment_methods - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_single_bank_account) - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint == "https://api.stripe.com/v1/payment_methods/pm_bank123/detach" - end.returns(successful_detach_response) + expect_ssl_for("payment_methods?customer", payment_methods_list("pm_bank123")) + expect_ssl_for("payment_methods/pm_bank123/detach", successful_detach_response) assert_success @gateway.unstore("cus_ACH") end def test_unstore_is_a_noop_when_no_bank_account_pm_exists - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(empty_payment_methods_list) + expect_ssl_for("payment_methods?customer", payment_methods_list) assert_success @gateway.unstore("cus_ACH") end def test_unstore_detaches_only_the_payment_method_named_in_the_identification - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_two_bank_accounts) - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint == "https://api.stripe.com/v1/payment_methods/pm_bank999/detach" - end.returns(successful_detach_response) + expect_ssl_for("payment_methods?customer", payment_methods_list("pm_bank123", "pm_bank999")) + expect_ssl_for("payment_methods/pm_bank999/detach", successful_detach_response) assert_success @gateway.unstore("cus_ACH|pm_bank999") end def test_unstore_detaches_a_legacy_bank_account_id - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_single_legacy_bank_account) - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint == "https://api.stripe.com/v1/payment_methods/ba_legacy123/detach" - end.returns(successful_detach_response) + expect_ssl_for("payment_methods?customer", payment_methods_list("ba_legacy123")) + expect_ssl_for("payment_methods/ba_legacy123/detach", successful_detach_response) assert_success @gateway.unstore("cus_ACH|ba_legacy123") end def test_unstore_is_a_noop_when_the_named_instrument_is_already_detached - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_single_bank_account) + expect_ssl_for("payment_methods?customer", payment_methods_list("pm_bank123")) response = @gateway.unstore("cus_ACH|ba_gone123") assert_success response - assert_equal "Payment method already detached", response.message + assert_equal "No us_bank_account payment method to detach", response.message end def test_unstore_raises_rather_than_detaching_every_method_when_the_profile_is_ambiguous - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint.start_with?("https://api.stripe.com/v1/payment_methods?customer") - end.returns(payment_methods_list_with_two_bank_accounts) - @gateway.expects(:ssl_request).with do |_m, endpoint, _p, _h| - endpoint == "https://api.stripe.com/v1/customers/cus_ACH" - end.returns(customer_without_default_payment_method) + expect_ssl_for("payment_methods?customer", payment_methods_list("pm_bank123", "pm_bank999")) + expect_ssl_for("/v1/customers/cus_ACH", customer_without_default_payment_method) assert_raises(StripeCustomerManyPaymentMethodWithoutDefault) do @gateway.unstore("cus_ACH") @@ -294,14 +230,14 @@ def test_unstore_raises_rather_than_detaching_every_method_when_the_profile_is_a private - def stub_request_for(_endpoint, response) - response + def expect_ssl_for(path_fragment, response, post_fragment = nil, mode: :expects) + @gateway.send(mode, :ssl_request).with do |_method, endpoint, post, _headers| + endpoint.include?(path_fragment) && (post_fragment.nil? || post.to_s.include?(post_fragment)) + end.returns(response) end - def expect_ssl_for(path_fragment, response) - @gateway.expects(:ssl_request).with do |_method, endpoint, _post, _headers| - endpoint.include?(path_fragment) - end.returns(response) + def stub_ssl_for(path_fragment, response, post_fragment = nil) + expect_ssl_for(path_fragment, response, post_fragment, mode: :stubs) end # --- fixtures ------------------------------------------------------------------------------ @@ -330,31 +266,12 @@ def successful_detach_response %({"id": "pm_bank123", "object": "payment_method", "customer": null, "livemode": false}) end - def payment_methods_list_with_single_bank_account - %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}], "livemode": false}) - end - - def payment_methods_list_with_single_legacy_bank_account - %({"object": "list", "data": [{"id": "ba_legacy123", "type": "us_bank_account"}], "livemode": false}) - end - - def payment_methods_list_with_two_bank_accounts - %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}, {"id": "pm_bank999", "type": "us_bank_account"}], "livemode": false}) - end - - def payment_methods_list_with_a_modern_and_a_legacy_bank_account - %({"object": "list", "data": [{"id": "pm_bank123", "type": "us_bank_account"}, {"id": "ba_legacy123", "type": "us_bank_account"}], "livemode": false}) - end - - def payment_methods_list_with_two_legacy_bank_accounts - %({"object": "list", "data": [{"id": "ba_legacy123", "type": "us_bank_account"}, {"id": "ba_legacy999", "type": "us_bank_account"}], "livemode": false}) + def payment_methods_list(*ids) + data = ids.map { |id| %({"id": "#{id}", "type": "us_bank_account"}) }.join(", ") + %({"object": "list", "data": [#{data}], "livemode": false}) end def customer_without_default_payment_method %({"id": "cus_ACH", "object": "customer", "invoice_settings": {"default_payment_method": null}, "livemode": false}) end - - def empty_payment_methods_list - %({"object": "list", "data": [], "livemode": false}) - end end