From cfed6a7048f27764b0515e1f917903ee82bebb14 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 3 Oct 2020 16:32:45 +0100 Subject: [PATCH 01/18] Split payments_spec so that we can add more stripe specific specs --- spec/features/admin/payments_spec.rb | 27 --------------- spec/features/admin/payments_stripe_spec.rb | 37 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 27 deletions(-) create mode 100644 spec/features/admin/payments_stripe_spec.rb diff --git a/spec/features/admin/payments_spec.rb b/spec/features/admin/payments_spec.rb index 851843dc30..677b047045 100644 --- a/spec/features/admin/payments_spec.rb +++ b/spec/features/admin/payments_spec.rb @@ -30,31 +30,4 @@ feature ' expect(page).to have_content "New Payment" end end - - context "with a StripeSCA payment method" do - before do - stripe_payment_method = create(:stripe_sca_payment_method, distributors: [order.distributor]) - order.payments << create(:payment, payment_method: stripe_payment_method, order: order) - end - - it "renders the payment details" do - login_as_admin_and_visit spree.admin_order_payments_path order - - page.click_link("StripeSCA") - expect(page).to have_content order.payments.last.source.last_digits - end - - context "with a deleted credit card" do - before do - order.payments.last.update_attribute(:source, nil) - end - - it "renders the payment details" do - login_as_admin_and_visit spree.admin_order_payments_path order - - page.click_link("StripeSCA") - expect(page).to have_content order.payments.last.amount - end - end - end end diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb new file mode 100644 index 0000000000..43f179950a --- /dev/null +++ b/spec/features/admin/payments_stripe_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +feature ' + As an hub manager + I want to make Stripe payments +' do + include AuthenticationHelper + + let(:order) { create(:completed_order_with_fees) } + + context "with a payment using a StripeSCA payment method" do + before do + stripe_payment_method = create(:stripe_sca_payment_method, distributors: [order.distributor]) + order.payments << create(:payment, payment_method: stripe_payment_method, order: order) + end + + it "renders the payment details" do + login_as_admin_and_visit spree.admin_order_payments_path order + + page.click_link("StripeSCA") + expect(page).to have_content order.payments.last.source.last_digits + end + + context "with a deleted credit card" do + before do + order.payments.last.update_attribute(:source, nil) + end + + it "renders the payment details" do + login_as_admin_and_visit spree.admin_order_payments_path order + + page.click_link("StripeSCA") + expect(page).to have_content order.payments.last.amount + end + end + end +end From 0178d3f1e6b4aec9f2b4d34f14708324202663da Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 3 Oct 2020 18:10:06 +0100 Subject: [PATCH 02/18] Add spec that takes a stripe payment in the BO --- spec/features/admin/payments_stripe_spec.rb | 31 +++++++++++++++++++-- spec/support/request/stripe_helper.rb | 7 +++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 43f179950a..11b8e3cd34 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -5,12 +5,13 @@ feature ' I want to make Stripe payments ' do include AuthenticationHelper + include StripeHelper - let(:order) { create(:completed_order_with_fees) } + let!(:order) { create(:completed_order_with_fees) } + let!(:stripe_payment_method) { create(:stripe_sca_payment_method, distributors: [order.distributor]) } context "with a payment using a StripeSCA payment method" do before do - stripe_payment_method = create(:stripe_sca_payment_method, distributors: [order.distributor]) order.payments << create(:payment, payment_method: stripe_payment_method, order: order) end @@ -34,4 +35,30 @@ feature ' end end end + + context "making a new Stripe payment" do + let!(:stripe_account) { create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") } + + before do + stub_hub_payment_methods_request + stub_payment_intents_post_request order: order, stripe_account_header: true + stub_payment_intent_get_request + stub_successful_capture_request order: order + end + + it "adds a payment with state complete", js: true do + login_as_admin_and_visit spree.new_admin_order_payment_path order + + choose "StripeSCA" + fill_in "payment_amount", with: order.total.to_s + fill_in "cardholder_name", with: "David Gilmour" + fill_in "stripe-cardnumber", with: "4242424242424242" + fill_in "exp-date", with: "01-01-2050" + fill_in "cvc", with: "678" + click_button "Update" + + expect(page).to have_link "StripeSCA" + expect(order.payments.reload.first.state).to eq "completed" + end + end end diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index 608b1cadd0..da6d7cb36d 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -27,10 +27,11 @@ module StripeHelper Spree::Config.set(stripe_connect_enabled: true) end - def stub_payment_intents_post_request(order:, response: {}) - stub_request(:post, "https://api.stripe.com/v1/payment_intents") + def stub_payment_intents_post_request(order:, response: {}, stripe_account_header: true) + stub = 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_mock(response)) + stub = stub.with(headers: { 'Stripe-Account' => 'abc123' }) if stripe_account_header + stub.to_return(payment_intent_authorize_response_mock(response)) end def stub_payment_intent_get_request(response: {}, stripe_account_header: true) From 76a9271d9e13434063d19d51166ed9588515b2c6 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 5 Oct 2020 12:30:38 +0100 Subject: [PATCH 03/18] Add spec to cover payments in the backoffice for an order in the payment state --- spec/features/admin/payments_stripe_spec.rb | 37 +++++++++++++++------ spec/support/request/stripe_helper.rb | 8 +++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 11b8e3cd34..96ff9f2199 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -46,19 +46,34 @@ feature ' stub_successful_capture_request order: order end - it "adds a payment with state complete", js: true do - login_as_admin_and_visit spree.new_admin_order_payment_path order + context "for a complete order" do + it "adds a payment with state complete", js: true do + login_as_admin_and_visit spree.new_admin_order_payment_path order - choose "StripeSCA" - fill_in "payment_amount", with: order.total.to_s - fill_in "cardholder_name", with: "David Gilmour" - fill_in "stripe-cardnumber", with: "4242424242424242" - fill_in "exp-date", with: "01-01-2050" - fill_in "cvc", with: "678" - click_button "Update" + fill_in "payment_amount", with: order.total.to_s + fill_in_stripe_cards_details_in_backoffice + click_button "Update" - expect(page).to have_link "StripeSCA" - expect(order.payments.reload.first.state).to eq "completed" + expect(page).to have_link "StripeSCA" + expect(order.payments.reload.first.state).to eq "completed" + end + end + + context "for an order in payment state" do + let!(:order) { create(:order_with_line_items, distributor: create(:enterprise)) } + + before { while !order.payment? do break unless order.next! end } + + it "adds a payment with state complete", js: true do + login_as_admin_and_visit spree.new_admin_order_payment_path order + + fill_in "payment_amount", with: order.total.to_s + fill_in_stripe_cards_details_in_backoffice + click_button "Update" + + expect(page).to have_link "StripeSCA" + expect(order.payments.reload.first.state).to eq "completed" + end end end end diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index da6d7cb36d..fff0aa7f58 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -21,6 +21,14 @@ module StripeHelper fill_in 'CVC', with: '123' end + def fill_in_stripe_cards_details_in_backoffice + choose "StripeSCA" + fill_in "cardholder_name", with: "David Gilmour" + fill_in "stripe-cardnumber", with: "4242424242424242" + fill_in "exp-date", with: "01-01-2050" + fill_in "cvc", with: "678" + end + def setup_stripe allow(Stripe).to receive(:api_key) { "sk_test_12345" } allow(Stripe).to receive(:publishable_key) { "pk_test_12345" } From a5dbdaf22895ba33f7aa118ef38f48e675ce1643 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 5 Oct 2020 20:33:52 +0100 Subject: [PATCH 04/18] Add spec to cover stripe payment that fails on capture --- spec/features/admin/payments_stripe_spec.rb | 42 ++++++++++++++++----- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 96ff9f2199..cc966e99c6 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -43,26 +43,50 @@ feature ' stub_hub_payment_methods_request stub_payment_intents_post_request order: order, stripe_account_header: true stub_payment_intent_get_request - stub_successful_capture_request order: order end context "for a complete order" do - it "adds a payment with state complete", js: true do - login_as_admin_and_visit spree.new_admin_order_payment_path order + before { stub_successful_capture_request order: order } - fill_in "payment_amount", with: order.total.to_s - fill_in_stripe_cards_details_in_backoffice - click_button "Update" + context "with a valid credit card" do + it "adds a payment with state complete", js: true do + login_as_admin_and_visit spree.new_admin_order_payment_path order - expect(page).to have_link "StripeSCA" - expect(order.payments.reload.first.state).to eq "completed" + fill_in "payment_amount", with: order.total.to_s + fill_in_stripe_cards_details_in_backoffice + click_button "Update" + + expect(page).to have_link "StripeSCA" + expect(order.payments.reload.first.state).to eq "completed" + end + end + + context "with a card that fails the paymet capture step" do + let(:error_message) { "Card was declined: insufficient funds." } + + before { stub_failed_capture_request order: order, response: { message: error_message } } + + it "fails to add a payment due to card error", js: true do + login_as_admin_and_visit spree.new_admin_order_payment_path order + + fill_in "payment_amount", with: order.total.to_s + fill_in_stripe_cards_details_in_backoffice + click_button "Update" + + expect(page).to have_link "StripeSCA" + expect(page).to have_content "FAILED" + expect(order.payments.reload.second.state).to eq "failed" + end end end context "for an order in payment state" do let!(:order) { create(:order_with_line_items, distributor: create(:enterprise)) } - before { while !order.payment? do break unless order.next! end } + before do + stub_successful_capture_request order: order + while !order.payment? do break unless order.next! end + end it "adds a payment with state complete", js: true do login_as_admin_and_visit spree.new_admin_order_payment_path order From 4ab2a8ddd19873c03e24ec9fc9914183b4006f9e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 16:59:34 +0100 Subject: [PATCH 05/18] Add spec to cover stripe payment that fails on card registration with a request/redirect for extra SCA authorization --- spec/features/admin/payments_stripe_spec.rb | 74 +++++++++++++++------ 1 file changed, 55 insertions(+), 19 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index cc966e99c6..4e1c79afc6 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -36,37 +36,71 @@ feature ' end end - context "making a new Stripe payment" do + context "making a new Stripe payment", js: true do let!(:stripe_account) { create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") } before do stub_hub_payment_methods_request - stub_payment_intents_post_request order: order, stripe_account_header: true stub_payment_intent_get_request end context "for a complete order" do - before { stub_successful_capture_request order: order } + context "with a card that succceeds on card registration" do + before { stub_payment_intents_post_request order: order, stripe_account_header: true } - context "with a valid credit card" do - it "adds a payment with state complete", js: true do - login_as_admin_and_visit spree.new_admin_order_payment_path order + context "and succeeds on payment capture" do + before { stub_successful_capture_request order: order } - fill_in "payment_amount", with: order.total.to_s - fill_in_stripe_cards_details_in_backoffice - click_button "Update" + it "adds a payment with state complete" do + login_as_admin_and_visit spree.new_admin_order_payment_path order - expect(page).to have_link "StripeSCA" - expect(order.payments.reload.first.state).to eq "completed" + fill_in "payment_amount", with: order.total.to_s + fill_in_stripe_cards_details_in_backoffice + click_button "Update" + + expect(page).to have_link "StripeSCA" + expect(OrderPaymentFinder.new(order.reload).last_payment.state).to eq "completed" + end + end + + context "but fails on payment capture" do + let(:error_message) { "Card was declined: insufficient funds." } + + before { stub_failed_capture_request order: order, response: { message: error_message } } + + it "fails to add a payment due to card error" do + login_as_admin_and_visit spree.new_admin_order_payment_path order + + fill_in "payment_amount", with: order.total.to_s + fill_in_stripe_cards_details_in_backoffice + click_button "Update" + + expect(page).to have_link "StripeSCA" + expect(page).to have_content "FAILED" + expect(OrderPaymentFinder.new(order.reload).last_payment.state).to eq "failed" + end end end - context "with a card that fails the paymet capture step" do - let(:error_message) { "Card was declined: insufficient funds." } + context "with a card that fails on registration (requires extra SCA auth: redirects to stripe" 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 { stub_failed_capture_request order: order, response: { message: error_message } } + before 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) + end - it "fails to add a payment due to card error", js: true do + it "fails to add a payment due to card error" do login_as_admin_and_visit spree.new_admin_order_payment_path order fill_in "payment_amount", with: order.total.to_s @@ -74,8 +108,8 @@ feature ' click_button "Update" expect(page).to have_link "StripeSCA" - expect(page).to have_content "FAILED" - expect(order.payments.reload.second.state).to eq "failed" + expect(page).to have_content "PROCESSING" + expect(OrderPaymentFinder.new(order.reload).last_payment.state).to eq "processing" end end end @@ -84,11 +118,13 @@ feature ' let!(:order) { create(:order_with_line_items, distributor: create(:enterprise)) } before do + stub_payment_intents_post_request order: order, stripe_account_header: true stub_successful_capture_request order: order + while !order.payment? do break unless order.next! end end - it "adds a payment with state complete", js: true do + it "adds a payment with state complete" do login_as_admin_and_visit spree.new_admin_order_payment_path order fill_in "payment_amount", with: order.total.to_s @@ -96,7 +132,7 @@ feature ' click_button "Update" expect(page).to have_link "StripeSCA" - expect(order.payments.reload.first.state).to eq "completed" + expect(OrderPaymentFinder.new(order.reload).last_payment.state).to eq "completed" end end end From 59b4e425b6287a08585a387737f47d36b87d0608 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 17:18:27 +0100 Subject: [PATCH 06/18] Fix some rubocop issues --- spec/features/admin/payments_stripe_spec.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 4e1c79afc6..50dac43446 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' feature ' @@ -8,7 +10,9 @@ feature ' include StripeHelper let!(:order) { create(:completed_order_with_fees) } - let!(:stripe_payment_method) { create(:stripe_sca_payment_method, distributors: [order.distributor]) } + let!(:stripe_payment_method) do + create(:stripe_sca_payment_method, distributors: [order.distributor]) + end context "with a payment using a StripeSCA payment method" do before do @@ -24,7 +28,7 @@ feature ' context "with a deleted credit card" do before do - order.payments.last.update_attribute(:source, nil) + order.payments.last.update source: nil end it "renders the payment details" do @@ -37,7 +41,9 @@ feature ' end context "making a new Stripe payment", js: true do - let!(:stripe_account) { create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") } + let!(:stripe_account) do + create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") + end before do stub_hub_payment_methods_request @@ -82,7 +88,7 @@ feature ' end end - context "with a card that fails on registration (requires extra SCA auth: redirects to stripe" do + context "with a card that fails on registration because it requires(redirects) extra auth" 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", From b341f593e7486fd80a3ab5a796486062b9afe64d Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 17:21:22 +0100 Subject: [PATCH 07/18] Improve method names --- spec/features/admin/payments_stripe_spec.rb | 10 +++++----- .../features/consumer/shopping/checkout_stripe_spec.rb | 2 +- spec/support/request/stripe_helper.rb | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 50dac43446..41a3801c51 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -46,7 +46,7 @@ feature ' end before do - stub_hub_payment_methods_request + stub_payment_methods_post_request stub_payment_intent_get_request end @@ -61,7 +61,7 @@ feature ' login_as_admin_and_visit spree.new_admin_order_payment_path order fill_in "payment_amount", with: order.total.to_s - fill_in_stripe_cards_details_in_backoffice + fill_in_card_details_in_backoffice click_button "Update" expect(page).to have_link "StripeSCA" @@ -78,7 +78,7 @@ feature ' login_as_admin_and_visit spree.new_admin_order_payment_path order fill_in "payment_amount", with: order.total.to_s - fill_in_stripe_cards_details_in_backoffice + fill_in_card_details_in_backoffice click_button "Update" expect(page).to have_link "StripeSCA" @@ -110,7 +110,7 @@ feature ' login_as_admin_and_visit spree.new_admin_order_payment_path order fill_in "payment_amount", with: order.total.to_s - fill_in_stripe_cards_details_in_backoffice + fill_in_card_details_in_backoffice click_button "Update" expect(page).to have_link "StripeSCA" @@ -134,7 +134,7 @@ feature ' login_as_admin_and_visit spree.new_admin_order_payment_path order fill_in "payment_amount", with: order.total.to_s - fill_in_stripe_cards_details_in_backoffice + fill_in_card_details_in_backoffice click_button "Update" expect(page).to have_link "StripeSCA" diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index 349bb6838b..273519456d 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -93,7 +93,7 @@ feature "Check out with Stripe", js: true do context "with guest checkout" do before do stub_payment_intent_get_request - stub_hub_payment_methods_request + stub_payment_methods_post_request end context "when the card is accepted" do diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index fff0aa7f58..b9f586f9d6 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -21,7 +21,7 @@ module StripeHelper fill_in 'CVC', with: '123' end - def fill_in_stripe_cards_details_in_backoffice + def fill_in_card_details_in_backoffice choose "StripeSCA" fill_in "cardholder_name", with: "David Gilmour" fill_in "stripe-cardnumber", with: "4242424242424242" @@ -48,7 +48,7 @@ module StripeHelper stub.to_return(payment_intent_authorize_response_mock(response)) end - def stub_hub_payment_methods_request(response: {}) + def stub_payment_methods_post_request(response: {}) stub_request(:post, "https://api.stripe.com/v1/payment_methods") .with(body: { payment_method: "pm_123" }, headers: { 'Stripe-Account' => 'abc123' }) From a6ed003cb97478a3e65d113e6de845e1aef18da4 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 17:49:33 +0100 Subject: [PATCH 08/18] Extract redirect stub to stripe_helper --- spec/features/admin/payments_stripe_spec.rb | 16 ++-------------- .../consumer/shopping/checkout_stripe_spec.rb | 17 +++-------------- spec/support/request/stripe_helper.rb | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 41a3801c51..4287608c38 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -89,21 +89,9 @@ feature ' end context "with a card that fails on registration because it requires(redirects) extra auth" 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_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_payment_intents_post_request_with_redirect order: order, + redirect_url: "www.dummy.org" end it "fails to add a payment due to card error" do diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index 273519456d..6c99489013 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -127,21 +127,10 @@ feature "Check out with Stripe", js: true do 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_request(:post, "https://api.stripe.com/v1/payment_intents") - .with(basic_auth: ["sk_test_12345", ""], body: /.*#{order.number}/) - .to_return(payment_intent_authorize_response) + stripe_redirect_url = checkout_path(payment_intent: "pi_123") + stub_payment_intents_post_request_with_redirect order: order, + redirect_url: stripe_redirect_url end describe "and the authorization succeeds" do diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index b9f586f9d6..3f6eaa18e4 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -42,6 +42,12 @@ module StripeHelper stub.to_return(payment_intent_authorize_response_mock(response)) end + def stub_payment_intents_post_request_with_redirect(order:, redirect_url:) + stub_request(:post, "https://api.stripe.com/v1/payment_intents") + .with(basic_auth: ["sk_test_12345", ""], body: /.*#{order.number}/) + .to_return(payment_intent_redirect_response_mock(redirect_url)) + end + 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 @@ -83,6 +89,16 @@ module StripeHelper charges: { data: [{ id: "ch_1234", amount: 2000 }] }) } end + def payment_intent_redirect_response_mock(redirect_url) + { status: 200, body: JSON.generate(id: "pi_123", + object: "payment_intent", + next_source_action: { + type: "authorize_with_url", + authorize_with_url: { url: redirect_url } + }, + status: "requires_source_action") } + end + def payment_successful_capture_mock(options) { status: options[:code] || 200, body: JSON.generate(object: "payment_intent", From bce81d27ddb1b8746cb7f04e4e199313721e079b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 17:51:43 +0100 Subject: [PATCH 09/18] Move spec to end of file so we can extend this case with refunds and cancelations --- spec/features/admin/payments_stripe_spec.rb | 52 ++++++++++----------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 4287608c38..0d6f022453 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -14,32 +14,6 @@ feature ' create(:stripe_sca_payment_method, distributors: [order.distributor]) end - context "with a payment using a StripeSCA payment method" do - before do - order.payments << create(:payment, payment_method: stripe_payment_method, order: order) - end - - it "renders the payment details" do - login_as_admin_and_visit spree.admin_order_payments_path order - - page.click_link("StripeSCA") - expect(page).to have_content order.payments.last.source.last_digits - end - - context "with a deleted credit card" do - before do - order.payments.last.update source: nil - end - - it "renders the payment details" do - login_as_admin_and_visit spree.admin_order_payments_path order - - page.click_link("StripeSCA") - expect(page).to have_content order.payments.last.amount - end - end - end - context "making a new Stripe payment", js: true do let!(:stripe_account) do create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") @@ -130,4 +104,30 @@ feature ' end end end + + context "with a payment using a StripeSCA payment method" do + before do + order.payments << create(:payment, payment_method: stripe_payment_method, order: order) + end + + it "renders the payment details" do + login_as_admin_and_visit spree.admin_order_payments_path order + + page.click_link("StripeSCA") + expect(page).to have_content order.payments.last.source.last_digits + end + + context "with a deleted credit card" do + before do + order.payments.last.update source: nil + end + + it "renders the payment details" do + login_as_admin_and_visit spree.admin_order_payments_path order + + page.click_link("StripeSCA") + expect(page).to have_content order.payments.last.amount + end + end + end end From 62a54e5f172d938ed7c433b88c3da70ab87ee24a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 20:28:40 +0100 Subject: [PATCH 10/18] Add spec to cover stripe SCA refunds in the backoffice --- spec/features/admin/payments_stripe_spec.rb | 34 ++++++++++++++++++--- spec/support/request/stripe_helper.rb | 14 +++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index 0d6f022453..e4d227e773 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -13,12 +13,11 @@ feature ' let!(:stripe_payment_method) do create(:stripe_sca_payment_method, distributors: [order.distributor]) end + let!(:stripe_account) do + create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") + end context "making a new Stripe payment", js: true do - let!(:stripe_account) do - create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") - end - before do stub_payment_methods_post_request stub_payment_intent_get_request @@ -107,6 +106,7 @@ feature ' context "with a payment using a StripeSCA payment method" do before do + order.update payments: [] order.payments << create(:payment, payment_method: stripe_payment_method, order: order) end @@ -129,5 +129,31 @@ feature ' expect(page).to have_content order.payments.last.amount end end + + context "that is completed", js: true do + let(:payment) { OrderPaymentFinder.new(order.reload).last_payment } + + before do + stub_payment_intent_get_request stripe_account_header: false + stub_successful_capture_request order: order + + payment.update response_code: "pi_123", amount: order.total + payment.purchase! + + stub_refund_request + end + + it "allows to refund the payment" do + login_as_admin_and_visit spree.admin_order_payments_path order + + expect(page).to have_link "StripeSCA" + expect(page).to have_content "COMPLETED" + + page.find('a.icon-void').click + + expect(page).to have_content "VOID" + expect(payment.reload.state).to eq "void" + end + end end end diff --git a/spec/support/request/stripe_helper.rb b/spec/support/request/stripe_helper.rb index 3f6eaa18e4..ef71652b1a 100644 --- a/spec/support/request/stripe_helper.rb +++ b/spec/support/request/stripe_helper.rb @@ -76,6 +76,13 @@ module StripeHelper .to_return(response_mock) end + def stub_refund_request + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds") + .with(body: { amount: 2000, expand: ["charge"] }, + headers: { 'Stripe-Account' => 'abc123' }) + .to_return(payment_successful_refund_mock) + end + private def payment_intent_authorize_response_mock(options) @@ -116,4 +123,11 @@ module StripeHelper { status: options[:code] || 200, body: JSON.generate(id: "pm_456", customer: "cus_A123") } end + + def payment_successful_refund_mock + { status: 200, + body: JSON.generate(object: "refund", + amount: 2000, + charge: "ch_1234") } + end end From 6046b0f6e45fca14204773431a4b265600dffa40 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 20:32:38 +0100 Subject: [PATCH 11/18] Fix typo --- spec/features/admin/payments_stripe_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index e4d227e773..c229658e66 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -24,7 +24,7 @@ feature ' end context "for a complete order" do - context "with a card that succceeds on card registration" do + context "with a card that succeeds on card registration" do before { stub_payment_intents_post_request order: order, stripe_account_header: true } context "and succeeds on payment capture" do From 07b3c100e810293af935b94e59ccb4cbecf75a68 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 15 Oct 2020 15:58:09 +0100 Subject: [PATCH 12/18] Remove code and specs related to dynamic checkout workflow, we have a static workflow defines in the Order class --- app/models/spree/order.rb | 5 -- app/models/spree/order/checkout.rb | 43 ------------- spec/models/spree/order/checkout_spec.rb | 77 ------------------------ 3 files changed, 125 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index d44455d53b..00dfb64d4f 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -553,10 +553,6 @@ module Spree line_item_adjustments.destroy_all end - def has_step?(step) - checkout_steps.include?(step) - end - def state_changed(name) state = "#{name}_state" return unless persisted? @@ -803,7 +799,6 @@ module Spree end def has_available_shipment - return unless has_step?("delivery") return unless address? return unless ship_address&.valid? # errors.add(:base, :no_shipping_methods_available) if available_shipping_methods.empty? diff --git a/app/models/spree/order/checkout.rb b/app/models/spree/order/checkout.rb index ffecf63f23..82efef81e7 100644 --- a/app/models/spree/order/checkout.rb +++ b/app/models/spree/order/checkout.rb @@ -9,7 +9,6 @@ module Spree class_attribute :previous_states class_attribute :checkout_flow class_attribute :checkout_steps - class_attribute :removed_transitions def self.checkout_flow(&block) if block_given? @@ -24,7 +23,6 @@ module Spree self.checkout_steps = {} self.next_event_transitions = [] self.previous_states = [:cart] - self.removed_transitions = [] # Build the checkout flow using the checkout_flow defined either # within the Order class, or a decorator for that class. @@ -95,43 +93,6 @@ module Spree end end - def self.insert_checkout_step(name, options = {}) - before = options.delete(:before) - after = options.delete(:after) unless before - after = checkout_steps.keys.last unless before || after - - cloned_steps = checkout_steps.clone - cloned_removed_transitions = removed_transitions.clone - checkout_flow do - cloned_steps.each_pair do |key, value| - go_to_state(name, options) if key == before - go_to_state(key, value) - go_to_state(name, options) if key == after - end - cloned_removed_transitions.each do |transition| - remove_transition(transition) - end - end - end - - def self.remove_checkout_step(name) - cloned_steps = checkout_steps.clone - cloned_removed_transitions = removed_transitions.clone - checkout_flow do - cloned_steps.each_pair do |key, value| - go_to_state(key, value) unless key == name - end - cloned_removed_transitions.each do |transition| - remove_transition(transition) - end - end - end - - def self.remove_transition(options = {}) - removed_transitions << options - next_event_transitions.delete(find_transition(options)) - end - def self.find_transition(options = {}) return nil if options.nil? || !options.include?(:from) || !options.include?(:to) @@ -173,10 +134,6 @@ module Spree checkout_steps.index(step) end - def self.removed_transitions - @removed_transitions ||= [] - end - def can_go_to_state?(state) return false unless self.state.present? && checkout_step?(state) && diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index e5bcea4a2e..55cdb4c301 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -30,16 +30,6 @@ describe Spree::Order::Checkout do expect(Spree::Order.find_transition({ foo: :bar, baz: :dog })).to be_falsy end - it '.remove_transition' do - options = { from: transitions.first.keys.first, to: transitions.first.values.first } - allow(Spree::Order).to receive(:next_event_transition).and_return([options]) - expect(Spree::Order.remove_transition(options)).to be_truthy - end - - it '.remove_transition when contract was broken' do - expect(Spree::Order.remove_transition(nil)).to be_falsy - end - context "#checkout_steps" do context "when payment not required" do before { allow(order).to receive_messages payment_required?: false } @@ -232,73 +222,6 @@ describe Spree::Order::Checkout do end end - context "insert checkout step" do - before do - @old_checkout_flow = Spree::Order.checkout_flow - Spree::Order.class_eval do - insert_checkout_step :new_step, before: :address - end - end - - after do - Spree::Order.checkout_flow(&@old_checkout_flow) - end - - it "should maintain removed transitions" do - transition = Spree::Order.find_transition(from: :delivery, to: :confirm) - expect(transition).to be_nil - end - - context "before" do - before do - Spree::Order.class_eval do - insert_checkout_step :before_address, before: :address - end - end - - specify do - order = Spree::Order.new - expect(order.checkout_steps).to eq %w(new_step before_address address delivery complete) - end - end - - context "after" do - before do - Spree::Order.class_eval do - insert_checkout_step :after_address, after: :address - end - end - - specify do - order = Spree::Order.new - expect(order.checkout_steps).to eq %w(new_step address after_address delivery complete) - end - end - end - - context "remove checkout step" do - before do - @old_checkout_flow = Spree::Order.checkout_flow - Spree::Order.class_eval do - remove_checkout_step :address - end - end - - after do - Spree::Order.checkout_flow(&@old_checkout_flow) - end - - it "should maintain removed transitions" do - transition = Spree::Order.find_transition(from: :delivery, to: :confirm) - expect(transition).to be_nil - end - - specify do - order = Spree::Order.new - expect(order.checkout_steps).to eq %w(delivery complete) - end - end - describe 'event :restart_checkout' do let(:order) { build_stubbed(:order) } From 249b4d124f5233aed2aef3afb400050f0b0b8bb3 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 15 Oct 2020 15:59:38 +0100 Subject: [PATCH 13/18] Remove more specs related to dynamic order checkout workflow --- spec/models/spree/order/checkout_spec.rb | 52 ------------------------ 1 file changed, 52 deletions(-) diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index 55cdb4c301..bee9ef1caa 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -170,58 +170,6 @@ describe Spree::Order::Checkout do end end - context "re-define checkout flow" do - before do - @old_checkout_flow = Spree::Order.checkout_flow - Spree::Order.class_eval do - checkout_flow do - go_to_state :payment - go_to_state :complete - end - end - end - - after do - Spree::Order.checkout_flow(&@old_checkout_flow) - end - - it "should not keep old event transitions when checkout_flow is redefined" do - expect(Spree::Order.next_event_transitions).to eq [{ cart: :payment }, { payment: :complete }] - end - - it "should not keep old events when checkout_flow is redefined" do - state_machine = Spree::Order.state_machine - expect(state_machine.states.any? { |s| s.name == :address }).to be_falsy - known_states = state_machine.events[:next].branches.map(&:known_states).flatten - expect(known_states).to_not include(:address) - expect(known_states).to_not include(:delivery) - expect(known_states).to_not include(:confirm) - end - end - - # Regression test for Spree #3665 - context "with only a complete step" do - before do - @old_checkout_flow = Spree::Order.checkout_flow - Spree::Order.class_eval do - checkout_flow do - go_to_state :complete - end - end - end - - after do - Spree::Order.checkout_flow(&@old_checkout_flow) - end - - it "does not attempt to process payments" do - allow(order).to receive_message_chain(:line_items, :present?).and_return(true) - expect(order).to_not receive(:payment_required?) - expect(order).to_not receive(:process_payments!) - order.next! - end - end - describe 'event :restart_checkout' do let(:order) { build_stubbed(:order) } From fb3f35100f90be52f3ca71862555201e0ed076ff Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 15 Oct 2020 16:02:11 +0100 Subject: [PATCH 14/18] Remove specs related to subsclassing the order class This is not something we will do --- spec/models/spree/order/checkout_spec.rb | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index bee9ef1caa..6d24512509 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -150,26 +150,6 @@ describe Spree::Order::Checkout do end end - context "subclassed order" do - # This causes another test above to fail, but fixing this test should make - # the other test pass - class SubclassedOrder < Spree::Order - checkout_flow do - go_to_state :payment - go_to_state :complete - end - end - - it "should only call default transitions once when checkout_flow is redefined" do - order = SubclassedOrder.new - allow(order).to receive_messages payment_required?: true - expect(order).to receive(:process_payments!).once - order.state = "payment" - order.next! - expect(order.state).to eq "complete" - end - end - describe 'event :restart_checkout' do let(:order) { build_stubbed(:order) } From 5141723b21e76f840d5eda0ee1ed1bb8435ab110 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 15 Oct 2020 16:04:06 +0100 Subject: [PATCH 15/18] Re-add restart checkout specs These were breaking because of the class_evals on the checkout_spec --- spec/services/order_checkout_restart_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/services/order_checkout_restart_spec.rb b/spec/services/order_checkout_restart_spec.rb index c2dc9b43f1..f74a427c02 100644 --- a/spec/services/order_checkout_restart_spec.rb +++ b/spec/services/order_checkout_restart_spec.rb @@ -23,7 +23,7 @@ describe OrderCheckoutRestart do order.update_attribute(:state, "payment") end - xcontext "when order ship address is nil" do + context "when order ship address is nil" do before { order.ship_address = nil } it "resets the order state, and clears incomplete shipments and payments" do @@ -33,7 +33,7 @@ describe OrderCheckoutRestart do end end - xcontext "when order ship address is not empty" do + context "when order ship address is not empty" do before { order.ship_address = order.address_from_distributor } it "resets the order state, and clears incomplete shipments and payments" do From 7de98e74b8b448cc5cfb8211830b567c6730c223 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 14 Oct 2020 16:33:43 +0100 Subject: [PATCH 16/18] Use #setup_stripe helper in tests to ensure Stripe keys are present --- spec/features/admin/payments_stripe_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index c229658e66..b296d0fc32 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -17,6 +17,8 @@ feature ' create(:stripe_account, enterprise: order.distributor, stripe_user_id: "abc123") end + before { setup_stripe } + context "making a new Stripe payment", js: true do before do stub_payment_methods_post_request From 4c9e4ee926d06561f7ea5cd751164b21f7b676b7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 15 Oct 2020 17:19:39 +0100 Subject: [PATCH 17/18] Fix problem in calculator spec I am not sure why this started failing on this branch only and not in master but this fix is correct because line_items are created with order defined already, an order.reload is enough to load them into the order --- spec/models/spree/calculator_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/models/spree/calculator_spec.rb b/spec/models/spree/calculator_spec.rb index 2aa8c5d0a8..12d0853da8 100644 --- a/spec/models/spree/calculator_spec.rb +++ b/spec/models/spree/calculator_spec.rb @@ -10,9 +10,7 @@ module Spree let!(:line_item2) { create(:line_item, order: order) } before do - order.line_items << line_item - order.line_items << line_item2 - order.shipments = [shipment] + order.reload.shipments = [shipment] end describe "#line_items_for" do From 6bacc2f6270a296d0db5440628eb022218aa59d9 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 15 Oct 2020 18:33:56 +0100 Subject: [PATCH 18/18] Fix group_buy_report_spec by avoiding orders with duplicate line items --- spec/lib/open_food_network/group_buy_report_spec.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/spec/lib/open_food_network/group_buy_report_spec.rb b/spec/lib/open_food_network/group_buy_report_spec.rb index 4cadaf4688..8231b2da95 100644 --- a/spec/lib/open_food_network/group_buy_report_spec.rb +++ b/spec/lib/open_food_network/group_buy_report_spec.rb @@ -13,24 +13,23 @@ module OpenFoodNetwork @variant1 = create(:variant) @variant1.product.supplier = @supplier1 @variant1.product.save! + @variant1.reload + shipping_instructions = "pick up on thursday please!" order1 = create(:order, distributor: distributor, bill_address: bill_address, special_instructions: shipping_instructions) line_item11 = create(:line_item, variant: @variant1, order: order1) - order1.line_items << line_item11 - @orders << order1 + @orders << order1.reload order2 = create(:order, distributor: distributor, bill_address: bill_address, special_instructions: shipping_instructions) line_item21 = create(:line_item, variant: @variant1, order: order2) - order2.line_items << line_item21 @variant2 = create(:variant) @variant2.product.supplier = @supplier1 @variant2.product.save! line_item22 = create(:line_item, variant: @variant2, order: order2) - order2.line_items << line_item22 - @orders << order2 + @orders << order2.reload @supplier2 = create(:supplier_enterprise) @variant3 = create(:variant, weight: nil) @@ -39,8 +38,7 @@ module OpenFoodNetwork order3 = create(:order, distributor: distributor, bill_address: bill_address, special_instructions: shipping_instructions) line_item31 = create(:line_item, variant: @variant3, order: order3) - order3.line_items << line_item31 - @orders << order3 + @orders << order3.reload end it "should return a header row describing the report" do