From 83456f94e3e6dadff2f3cd73f8772b205a5a6512 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 2 Oct 2020 19:12:56 +0100 Subject: [PATCH 1/6] Simplify test by re-using helper stripe mock --- .../payments/payments_controller_refunds_spec.rb | 14 +++++--------- spec/support/request/stripe_helper.rb | 1 + 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb index 6216228619..23febd49da 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Spree::Admin::PaymentsController, type: :controller do + include StripeHelper + let!(:shop) { create(:enterprise) } let!(:user) { shop.owner } let!(:order) { create(:order, distributor: shop, state: 'complete') } @@ -152,19 +154,13 @@ describe Spree::Admin::PaymentsController, type: :controller do allow(Stripe).to receive(:api_key) { "sk_test_12345" } allow(StripeAccount).to receive(:find_by) { stripe_account } - # Retrieves payment intent info - stub_request(:get, "https://api.stripe.com/v1/payment_intents/pi_123") - .with(headers: { 'Stripe-Account' => 'abc123' }) - .to_return({ status: 200, body: JSON.generate( - amount_received: 2000, - charges: { data: [{ id: "ch_1a2b3c" }] } - ) }) + stub_payment_intent_get_request end context "where the request succeeds" do before do # Issues the refund - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). with(basic_auth: ["sk_test_12345", ""]). to_return(status: 200, body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) @@ -184,7 +180,7 @@ describe Spree::Admin::PaymentsController, type: :controller do context "where the request fails" do before do - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). with(basic_auth: ["sk_test_12345", ""]). to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) end diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index ce981a06dc..122c35d5a8 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -55,6 +55,7 @@ module StripeHelper body: JSON.generate(id: "pi_123", object: "payment_intent", amount: 2000, + amount_received: 2000, status: options[:intent_status] || "requires_capture", last_payment_error: nil, charges: { data: [{ id: "ch_1234", amount: 2000 }] }) } From ef70c1fc5c4fe851e275e1dcfd17a74aed0db91b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 2 Oct 2020 19:20:32 +0100 Subject: [PATCH 2/6] Make helper more flexible and use it in a spec --- .../payments/payments_controller_refunds_spec.rb | 11 +++-------- spec/support/request/stripe_helper.rb | 8 ++++---- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb index 23febd49da..594b9e2dbb 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb @@ -216,17 +216,12 @@ describe Spree::Admin::PaymentsController, type: :controller do before do allow(Stripe).to receive(:api_key) { "sk_test_12345" } - # Retrieves payment intent info - stub_request(:get, "https://api.stripe.com/v1/payment_intents/pi_123") - .to_return({ status: 200, body: JSON.generate( - amount_received: 2000, - charges: { data: [{ id: "ch_1a2b3c" }] } - ) }) + stub_payment_intent_get_request stripe_account_header: false end context "where the request succeeds" do before do - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). with(basic_auth: ["sk_test_12345", ""]). to_return(status: 200, body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) @@ -246,7 +241,7 @@ describe Spree::Admin::PaymentsController, type: :controller do context "where the request fails" do before do - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). with(basic_auth: ["sk_test_12345", ""]). to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) end diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index 122c35d5a8..0af9ac97ca 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -20,10 +20,10 @@ module StripeHelper .to_return(payment_intent_authorize_response_mock(response)) end - def stub_payment_intent_get_request(response: {}) - stub_request(:get, "https://api.stripe.com/v1/payment_intents/pi_123") - .with(headers: { 'Stripe-Account' => 'abc123' }) - .to_return(payment_intent_authorize_response_mock(response)) + def stub_payment_intent_get_request(response: {}, stripe_account_header: true) + stub = stub_request(:get, "https://api.stripe.com/v1/payment_intents/pi_123") + stub = stub.with(headers: { 'Stripe-Account' => 'abc123' }) if stripe_account_header + stub.to_return(payment_intent_authorize_response_mock(response)) end def stub_hub_payment_methods_request(response: {}) From df4ec67974c3ec8641cb76861d0dd5dbe24d0aa9 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 3 Oct 2020 11:01:29 +0100 Subject: [PATCH 3/6] Add feature spec that covers stripe_sca redirect case --- .../consumer/shopping/checkout_stripe_spec.rb | 51 ++++++++++++++++--- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index e9b5e65230..bfcd521961 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -100,7 +100,6 @@ feature "Check out with Stripe", js: true do it "completes checkout successfully" do visit checkout_path - checkout_as_guest fill_out_form( @@ -108,13 +107,10 @@ feature "Check out with Stripe", js: true do stripe_sca_payment_method.name, save_default_addresses: false ) - fill_out_card_details - place_order expect(page).to have_content "Confirmed" - expect(order.reload.completed?).to eq true expect(order.payments.first.state).to eq "completed" end @@ -132,7 +128,6 @@ feature "Check out with Stripe", js: true do it "shows an error message from the Stripe response" do visit checkout_path - checkout_as_guest fill_out_form( @@ -140,17 +135,57 @@ feature "Check out with Stripe", js: true do stripe_sca_payment_method.name, save_default_addresses: false ) - fill_out_card_details - place_order expect(page).to have_content error_message - expect(order.reload.state).to eq "cart" expect(order.payments.first.state).to eq "failed" end end + + context "when the card needs extra SCA authorization", js: true do + let(:stripe_redirect_url) { checkout_path(payment_intent: "pi_123") } + let(:payment_intent_authorize_response) do + { status: 200, body: JSON.generate(id: "pi_123", + object: "payment_intent", + next_source_action: { + type: "authorize_with_url", + authorize_with_url: { url: stripe_redirect_url } + }, + status: "requires_source_action") } + end + + before do + stub_payment_intent_get_request + stub_hub_payment_methods_request + stub_request(:post, "https://api.stripe.com/v1/payment_intents") + .with(basic_auth: ["sk_test_12345", ""], body: /.*#{order.number}/) + .to_return(payment_intent_authorize_response) + + stub_successful_capture_request order: order + end + + it "completes checkout successfully" do + visit checkout_path + checkout_as_guest + + fill_out_form( + free_shipping.name, + stripe_sca_payment_method.name, + save_default_addresses: false + ) + fill_out_card_details + place_order + + # We make stripe return stripe_redirect_url (which is already sending the user back to the checkout) as if the authorization was done + # We can then control the actual authorization or failure of the authorization through the mock stub_payment_intent_get_request + + expect(page).to have_content "Confirmed" + expect(order.reload.completed?).to eq true + expect(order.payments.first.state).to eq "completed" + end + end end end end From 14f5ecfe0bbd122ad5c8b64dce54b3b59bb0d8be Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 3 Oct 2020 11:18:54 +0100 Subject: [PATCH 4/6] Add spec to cover StripeSCA extra auth with redirect and failed auth --- .../consumer/shopping/checkout_stripe_spec.rb | 66 ++++++++++++++----- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index bfcd521961..1a41186c04 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -162,28 +162,60 @@ feature "Check out with Stripe", js: true do stub_request(:post, "https://api.stripe.com/v1/payment_intents") .with(basic_auth: ["sk_test_12345", ""], body: /.*#{order.number}/) .to_return(payment_intent_authorize_response) - - stub_successful_capture_request order: order end - it "completes checkout successfully" do - visit checkout_path - checkout_as_guest + describe "and the authorization succeeds" do + before do + stub_successful_capture_request order: order + end - fill_out_form( - free_shipping.name, - stripe_sca_payment_method.name, - save_default_addresses: false - ) - fill_out_card_details - place_order + it "completes checkout successfully" do + visit checkout_path + checkout_as_guest - # We make stripe return stripe_redirect_url (which is already sending the user back to the checkout) as if the authorization was done - # We can then control the actual authorization or failure of the authorization through the mock stub_payment_intent_get_request + fill_out_form( + free_shipping.name, + stripe_sca_payment_method.name, + save_default_addresses: false + ) + fill_out_card_details + place_order - expect(page).to have_content "Confirmed" - expect(order.reload.completed?).to eq true - expect(order.payments.first.state).to eq "completed" + # We make stripe return stripe_redirect_url (which is already sending the user back to the checkout) as if the authorization was done + # We can then control the actual authorization or failure of the payment through the mock stub_successful_capture_request + + expect(page).to have_content "Confirmed" + expect(order.reload.completed?).to eq true + expect(order.payments.first.state).to eq "completed" + end + end + + describe "and the authorization fails" do + let(:error_message) { "Card was declined: insufficient funds." } + + before do + stub_failed_capture_request order: order, response: { message: error_message } + end + + it "completes checkout successfully" do + visit checkout_path + checkout_as_guest + + fill_out_form( + free_shipping.name, + stripe_sca_payment_method.name, + save_default_addresses: false + ) + fill_out_card_details + place_order + + # We make stripe return stripe_redirect_url (which is already sending the user back to the checkout) as if the authorization was done + # We can then control the actual authorization or failure of the payment through the mock stub_failed_capture_request + + expect(page).to have_content error_message + expect(order.reload.state).to eq "cart" + expect(order.payments.first.state).to eq "failed" + end end end end From 80ea80f26eaf2651ac67f4057395c5729407de97 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 3 Oct 2020 11:50:04 +0100 Subject: [PATCH 5/6] DRY stripe spec --- .../consumer/shopping/checkout_stripe_spec.rb | 55 +++---------------- spec/support/request/stripe_helper.rb | 13 +++++ 2 files changed, 22 insertions(+), 46 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index 1a41186c04..d992465a0c 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -90,25 +90,19 @@ feature "Check out with Stripe", js: true do let!(:shipping_method) { create(:shipping_method) } context "with guest checkout" do + before do + stub_payment_intent_get_request + stub_hub_payment_methods_request + end + context "when the card is accepted" do before do stub_payment_intents_post_request order: order - stub_payment_intent_get_request - stub_hub_payment_methods_request stub_successful_capture_request order: order end it "completes checkout successfully" do - visit checkout_path - checkout_as_guest - - fill_out_form( - free_shipping.name, - stripe_sca_payment_method.name, - save_default_addresses: false - ) - fill_out_card_details - place_order + checkout_with_stripe expect(page).to have_content "Confirmed" expect(order.reload.completed?).to eq true @@ -121,22 +115,11 @@ feature "Check out with Stripe", js: true do before do stub_payment_intents_post_request order: order - stub_payment_intent_get_request - stub_hub_payment_methods_request stub_failed_capture_request order: order, response: { message: error_message } end it "shows an error message from the Stripe response" do - visit checkout_path - checkout_as_guest - - fill_out_form( - free_shipping.name, - stripe_sca_payment_method.name, - save_default_addresses: false - ) - fill_out_card_details - place_order + checkout_with_stripe expect(page).to have_content error_message expect(order.reload.state).to eq "cart" @@ -157,8 +140,6 @@ feature "Check out with Stripe", js: true do end before do - stub_payment_intent_get_request - stub_hub_payment_methods_request stub_request(:post, "https://api.stripe.com/v1/payment_intents") .with(basic_auth: ["sk_test_12345", ""], body: /.*#{order.number}/) .to_return(payment_intent_authorize_response) @@ -170,16 +151,7 @@ feature "Check out with Stripe", js: true do end it "completes checkout successfully" do - visit checkout_path - checkout_as_guest - - fill_out_form( - free_shipping.name, - stripe_sca_payment_method.name, - save_default_addresses: false - ) - fill_out_card_details - place_order + checkout_with_stripe # We make stripe return stripe_redirect_url (which is already sending the user back to the checkout) as if the authorization was done # We can then control the actual authorization or failure of the payment through the mock stub_successful_capture_request @@ -198,16 +170,7 @@ feature "Check out with Stripe", js: true do end it "completes checkout successfully" do - visit checkout_path - checkout_as_guest - - fill_out_form( - free_shipping.name, - stripe_sca_payment_method.name, - save_default_addresses: false - ) - fill_out_card_details - place_order + checkout_with_stripe # We make stripe return stripe_redirect_url (which is already sending the user back to the checkout) as if the authorization was done # We can then control the actual authorization or failure of the payment through the mock stub_failed_capture_request diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index 0af9ac97ca..120b5c8bfc 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -1,6 +1,19 @@ # frozen_string_literal: true module StripeHelper + def checkout_with_stripe + visit checkout_path + checkout_as_guest + + fill_out_form( + free_shipping.name, + stripe_sca_payment_method.name, + save_default_addresses: false + ) + fill_out_card_details + place_order + end + def fill_out_card_details expect(page).to have_css("input[name='cardnumber']") fill_in 'Card number', with: '4242424242424242' From a5309627b740421ecfa4e6f35c5464bec3755833 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 3 Oct 2020 15:46:29 +0100 Subject: [PATCH 6/6] Fix a typo --- spec/features/consumer/shopping/checkout_stripe_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index d992465a0c..40543e164f 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -169,7 +169,7 @@ feature "Check out with Stripe", js: true do stub_failed_capture_request order: order, response: { message: error_message } end - it "completes checkout successfully" do + it "shows an error message from the Stripe response" do checkout_with_stripe # We make stripe return stripe_redirect_url (which is already sending the user back to the checkout) as if the authorization was done