From 2179cc7fafd686d02a55a63b995878ee66bd6a04 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sun, 17 May 2020 11:35:22 +0100 Subject: [PATCH 1/6] Make StripeSCA void action make a refund instead StripeSCA does not support voiding confirmed payment intents so we need to make a refund instead --- app/models/spree/gateway/stripe_sca.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index 21fa698c3b..02681ae210 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -57,8 +57,11 @@ module Spree # NOTE: the name of this method is determined by Spree::Payment::Processing def void(response_code, _creditcard, gateway_options) + payment_intent_id = response_code + payment_intent_response = Stripe::PaymentIntent.retrieve(payment_intent_id, + stripe_account: stripe_account_id) gateway_options[:stripe_account] = stripe_account_id - provider.void(response_code, gateway_options) + provider.refund(payment_intent_response.amount_received, response_code, gateway_options) end # NOTE: the name of this method is determined by Spree::Payment::Processing From 9e4a793b24cc4c003b6603efc703ae61c12d77c6 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 10 Jun 2020 15:36:02 +0100 Subject: [PATCH 2/6] Fix rubocop issues --- spec/requests/checkout/stripe_sca_spec.rb | 33 ++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/spec/requests/checkout/stripe_sca_spec.rb b/spec/requests/checkout/stripe_sca_spec.rb index e590b3f601..5018ad321c 100644 --- a/spec/requests/checkout/stripe_sca_spec.rb +++ b/spec/requests/checkout/stripe_sca_spec.rb @@ -64,12 +64,20 @@ describe "checking out an order with a Stripe SCA payment method", type: :reques } end let(:payment_intent_response_mock) do - { status: 200, body: JSON.generate(object: "payment_intent", amount: 2000, charges: { data: [{ id: "ch_1234", amount: 2000 }] }) } + { + status: 200, body: JSON.generate(object: "payment_intent", + amount: 2000, + charges: { data: [{ id: "ch_1234", amount: 2000 }] }) + } end let(:payment_intent_authorize_response_mock) do - { status: 200, body: JSON.generate(id: payment_intent_id, object: "payment_intent", amount: 2000, + { + status: 200, body: JSON.generate(id: payment_intent_id, + object: "payment_intent", + amount: 2000, status: "requires_capture", last_payment_error: nil, - charges: { data: [{ id: "ch_1234", amount: 2000 }] }) } + charges: { data: [{ id: "ch_1234", amount: 2000 }] }) + } end before do @@ -166,13 +174,15 @@ describe "checking out an order with a Stripe SCA payment method", type: :reques headers: { 'Stripe-Account' => 'abc123' }) .to_return(hubs_payment_method_response_mock) - # Creates a customer (this stubs the customers call to the main stripe account and also the call to the connected account) + # Creates a customer + # This stubs the customers call to both the main stripe account and the connected account stub_request(:post, "https://api.stripe.com/v1/customers") .with(body: { email: order.email }) .to_return(customer_response_mock) # Attaches the payment method to the customer in the hub's stripe account - stub_request(:post, "https://api.stripe.com/v1/payment_methods/#{hubs_stripe_payment_method}/attach") + stub_request(:post, + "https://api.stripe.com/v1/payment_methods/#{hubs_stripe_payment_method}/attach") .with(body: { customer: customer_id }, headers: { 'Stripe-Account' => 'abc123' }) .to_return(hubs_payment_method_response_mock) @@ -191,7 +201,8 @@ describe "checking out an order with a Stripe SCA payment method", type: :reques source_attributes[:save_requested_by_customer] = '1' # Attaches the payment method to the customer - stub_request(:post, "https://api.stripe.com/v1/payment_methods/#{stripe_payment_method}/attach") + stub_request(:post, + "https://api.stripe.com/v1/payment_methods/#{stripe_payment_method}/attach") .with(body: { customer: customer_id }) .to_return(payment_method_attach_response_mock) end @@ -316,9 +327,13 @@ describe "checking out an order with a Stripe SCA payment method", type: :reques context "when the stripe API sends a url for the authorization of the transaction" do let(:payment_intent_authorize_response_mock) do - { status: 200, body: JSON.generate(id: payment_intent_id, object: "payment_intent", - next_source_action: { type: "authorize_with_url", authorize_with_url: { url: stripe_redirect_url } }, - status: "requires_source_action" ) } + { status: 200, body: JSON.generate(id: payment_intent_id, + object: "payment_intent", + next_source_action: { + type: "authorize_with_url", + authorize_with_url: { url: stripe_redirect_url } + }, + status: "requires_source_action") } end it "redirects the user to the authorization stripe url" do From ecb1920fa97db1e93e90e4b923cfc0d9a863c072 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 10 Jun 2020 15:44:03 +0100 Subject: [PATCH 3/6] Move payment_controller_spec to specific folder so we can break it in more specific parts --- .../spree/admin/{ => orders/payments}/payments_controller_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/controllers/spree/admin/{ => orders/payments}/payments_controller_spec.rb (100%) diff --git a/spec/controllers/spree/admin/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb similarity index 100% rename from spec/controllers/spree/admin/payments_controller_spec.rb rename to spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb From ce493866f95e6eae4735bc9f972991bf3d6884f7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 10 Jun 2020 15:47:05 +0100 Subject: [PATCH 4/6] Extract refunds specs from payments controller spec --- .../payments_controller_refunds_spec.rb | 136 ++++++++++++++++++ .../payments/payments_controller_spec.rb | 124 ---------------- 2 files changed, 136 insertions(+), 124 deletions(-) create mode 100644 spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb 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 new file mode 100644 index 0000000000..cb43a2e4f6 --- /dev/null +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb @@ -0,0 +1,136 @@ +require 'spec_helper' + +describe Spree::Admin::PaymentsController, type: :controller do + let!(:shop) { create(:enterprise) } + let!(:user) { shop.owner } + let!(:order) { create(:order, distributor: shop, state: 'complete') } + let!(:line_item) { create(:line_item, order: order, price: 5.0) } + + before do + allow(controller).to receive(:spree_current_user) { user } + end + + context "as an enterprise user" do + before do + order.reload.update_totals + end + + context "requesting a refund on a payment" do + let(:params) { { id: payment.id, order_id: order.number, e: :void } } + + # Required for the respond override in the controller decorator to work + before { @request.env['HTTP_REFERER'] = spree.admin_order_payments_url(payment) } + + context "that was processed by stripe" do + let!(:payment_method) { create(:stripe_payment_method, distributors: [shop]) } + let!(:payment) do + create(:payment, order: order, state: 'completed', payment_method: payment_method, + response_code: 'ch_1a2b3c', amount: order.total) + end + + before do + allow(Stripe).to receive(:api_key) { "sk_test_12345" } + end + + context "where the request succeeds" do + before do + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, + body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + end + + it "voids the payment" do + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance).to eq 0 + spree_put :fire, params + expect(payment.reload.state).to eq 'void' + order.reload + expect(order.payment_total).to eq 0 + expect(order.outstanding_balance).to_not eq 0 + end + end + + context "where the request fails" do + before do + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) + end + + it "does not void the payment" do + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance).to eq 0 + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance).to eq 0 + expect(flash[:error]).to eq "Bup-bow!" + end + end + end + end + + context "requesting a partial credit on a payment" do + let(:params) { { id: payment.id, order_id: order.number, e: :credit } } + + # Required for the respond override in the controller decorator to work + before { @request.env['HTTP_REFERER'] = spree.admin_order_payments_url(payment) } + + context "that was processed by stripe" do + let!(:payment_method) { create(:stripe_payment_method, distributors: [shop]) } + let!(:payment) do + create(:payment, order: order, state: 'completed', payment_method: payment_method, + response_code: 'ch_1a2b3c', amount: order.total + 5) + end + + before do + allow(Stripe).to receive(:api_key) { "sk_test_12345" } + end + + context "where the request succeeds" do + before do + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, + body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + end + + it "partially refunds the payment" do + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to eq order.total + expect(order.outstanding_balance).to eq 0 + end + end + + context "where the request fails" do + before do + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) + end + + it "does not void the payment" do + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + expect(flash[:error]).to eq "Bup-bow!" + end + end + end + end + end +end diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb index 6c7c2eeb50..6932efa236 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb @@ -138,128 +138,4 @@ describe Spree::Admin::PaymentsController, type: :controller do end end end - - context "as an enterprise user" do - before do - order.reload.update_totals - end - - context "requesting a refund on a payment" do - let(:params) { { id: payment.id, order_id: order.number, e: :void } } - - # Required for the respond override in the controller decorator to work - before { @request.env['HTTP_REFERER'] = spree.admin_order_payments_url(payment) } - - context "that was processed by stripe" do - let!(:payment_method) { create(:stripe_payment_method, distributors: [shop]) } - let!(:payment) do - create(:payment, order: order, state: 'completed', payment_method: payment_method, - response_code: 'ch_1a2b3c', amount: order.total) - end - - before do - allow(Stripe).to receive(:api_key) { "sk_test_12345" } - end - - context "where the request succeeds" do - before do - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). - with(basic_auth: ["sk_test_12345", ""]). - to_return(status: 200, - body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) - end - - it "voids the payment" do - order.reload - expect(order.payment_total).to_not eq 0 - expect(order.outstanding_balance).to eq 0 - spree_put :fire, params - expect(payment.reload.state).to eq 'void' - order.reload - expect(order.payment_total).to eq 0 - expect(order.outstanding_balance).to_not eq 0 - end - end - - context "where the request fails" do - before do - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). - with(basic_auth: ["sk_test_12345", ""]). - to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) - end - - it "does not void the payment" do - order.reload - expect(order.payment_total).to_not eq 0 - expect(order.outstanding_balance).to eq 0 - spree_put :fire, params - expect(payment.reload.state).to eq 'completed' - order.reload - expect(order.payment_total).to_not eq 0 - expect(order.outstanding_balance).to eq 0 - expect(flash[:error]).to eq "Bup-bow!" - end - end - end - end - - context "requesting a partial credit on a payment" do - let(:params) { { id: payment.id, order_id: order.number, e: :credit } } - - # Required for the respond override in the controller decorator to work - before { @request.env['HTTP_REFERER'] = spree.admin_order_payments_url(payment) } - - context "that was processed by stripe" do - let!(:payment_method) { create(:stripe_payment_method, distributors: [shop]) } - let!(:payment) do - create(:payment, order: order, state: 'completed', payment_method: payment_method, - response_code: 'ch_1a2b3c', amount: order.total + 5) - end - - before do - allow(Stripe).to receive(:api_key) { "sk_test_12345" } - end - - context "where the request succeeds" do - before do - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). - with(basic_auth: ["sk_test_12345", ""]). - to_return(status: 200, - body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) - end - - it "partially refunds the payment" do - order.reload - expect(order.payment_total).to eq order.total + 5 - expect(order.outstanding_balance).to eq(-5) - spree_put :fire, params - expect(payment.reload.state).to eq 'completed' - order.reload - expect(order.payment_total).to eq order.total - expect(order.outstanding_balance).to eq 0 - end - end - - context "where the request fails" do - before do - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). - with(basic_auth: ["sk_test_12345", ""]). - to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) - end - - it "does not void the payment" do - order.reload - expect(order.payment_total).to eq order.total + 5 - expect(order.outstanding_balance).to eq(-5) - spree_put :fire, params - expect(payment.reload.state).to eq 'completed' - order.reload - expect(order.payment_total).to eq order.total + 5 - expect(order.outstanding_balance).to eq(-5) - expect(flash[:error]).to eq "Bup-bow!" - end - end - end - end - end end From 6555f8bfba32c027136e0b39a5b3911498544e38 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 10 Jun 2020 16:42:08 +0100 Subject: [PATCH 5/6] Add specs to cover stripeSCA refunds Duplication between stripe connect and stripeSCA is done on purpose so we can easily delete stripeConnect code when the migration is done --- .../payments_controller_refunds_spec.rb | 147 +++++++++++++++++- 1 file changed, 142 insertions(+), 5 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 cb43a2e4f6..08e923b5c3 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 @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Admin::PaymentsController, type: :controller do @@ -8,13 +10,10 @@ describe Spree::Admin::PaymentsController, type: :controller do before do allow(controller).to receive(:spree_current_user) { user } + order.reload.update_totals end - context "as an enterprise user" do - before do - order.reload.update_totals - end - + context "Stripe Connect" do context "requesting a refund on a payment" do let(:params) { { id: payment.id, order_id: order.number, e: :void } } @@ -133,4 +132,142 @@ describe Spree::Admin::PaymentsController, type: :controller do end end end + + context "StripeSCA" do + context "requesting a refund on a payment" do + let(:params) { { id: payment.id, order_id: order.number, e: :void } } + + # Required for the respond override in the controller decorator to work + before { @request.env['HTTP_REFERER'] = spree.admin_order_payments_url(payment) } + + context "that was processed by stripe" do + let!(:payment_method) { create(:stripe_sca_payment_method, distributors: [shop]) } + let!(:payment) do + create(:payment, order: order, state: 'completed', payment_method: payment_method, + response_code: 'pi_123', amount: order.total) + end + let(:stripe_account) { create(:stripe_account, enterprise: shop) } + + before 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" }] } + ) }) + end + + context "where the request succeeds" do + before do + # Issues the refund + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, + body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + end + + it "voids the payment" do + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance).to eq 0 + spree_put :fire, params + expect(payment.reload.state).to eq 'void' + order.reload + expect(order.payment_total).to eq 0 + expect(order.outstanding_balance).to_not eq 0 + end + end + + context "where the request fails" do + before do + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) + end + + it "does not void the payment" do + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance).to eq 0 + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance).to eq 0 + expect(flash[:error]).to eq "Bup-bow!" + end + end + end + end + + context "requesting a partial credit on a payment" do + let(:params) { { id: payment.id, order_id: order.number, e: :credit } } + + # Required for the respond override in the controller decorator to work + before { @request.env['HTTP_REFERER'] = spree.admin_order_payments_url(payment) } + + context "that was processed by stripe" do + let!(:payment_method) { create(:stripe_sca_payment_method, distributors: [shop]) } + let!(:payment) do + create(:payment, order: order, state: 'completed', payment_method: payment_method, + response_code: 'pi_123', amount: order.total + 5) + end + + 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" }] } + ) }) + end + + context "where the request succeeds" do + before do + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, + body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + end + + it "partially refunds the payment" do + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to eq order.total + expect(order.outstanding_balance).to eq 0 + end + end + + context "where the request fails" do + before do + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1a2b3c/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) + end + + it "does not void the payment" do + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to eq order.total + 5 + expect(order.outstanding_balance).to eq(-5) + expect(flash[:error]).to eq "Bup-bow!" + end + end + end + end + end end From e8417b8be606ff048fbb423eaa1d901603c24e53 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 12 Jun 2020 10:56:51 +0100 Subject: [PATCH 6/6] Remove specs testing filtering of master variants Master variants are not used in the report --- .../products_and_inventory_report_base.rb | 1 - .../products_and_inventory_report_spec.rb | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/lib/open_food_network/products_and_inventory_report_base.rb b/lib/open_food_network/products_and_inventory_report_base.rb index d0f4a50de8..c669dc4d6a 100644 --- a/lib/open_food_network/products_and_inventory_report_base.rb +++ b/lib/open_food_network/products_and_inventory_report_base.rb @@ -36,7 +36,6 @@ module OpenFoodNetwork end # Using the `in_stock?` method allows overrides by distributors. - # It also allows the upgrade to Spree 2.0. def filter_on_hand(variants) if params[:report_type] == 'inventory' variants.select(&:in_stock?) diff --git a/spec/lib/open_food_network/products_and_inventory_report_spec.rb b/spec/lib/open_food_network/products_and_inventory_report_spec.rb index e4d4d27142..f0844dfffb 100644 --- a/spec/lib/open_food_network/products_and_inventory_report_spec.rb +++ b/spec/lib/open_food_network/products_and_inventory_report_spec.rb @@ -97,18 +97,7 @@ module OpenFoodNetwork describe "Filtering variants" do let(:variants) { Spree::Variant.where(nil).joins(:product).where(is_master: false) } - it "should return unfiltered variants sans-params" do - product1 = create(:simple_product, supplier: supplier) - product2 = create(:simple_product, supplier: supplier) - expect(subject.filter(Spree::Variant.where(nil))).to match_array [product1.master, product1.variants.first, product2.master, product2.variants.first] - end - it "should filter deleted products" do - product1 = create(:simple_product, supplier: supplier) - product2 = create(:simple_product, supplier: supplier) - product2.destroy - expect(subject.filter(Spree::Variant.where(nil))).to match_array [product1.master, product1.variants.first] - end describe "based on report type" do it "returns only variants on hand" do product1 = create(:simple_product, supplier: supplier, on_hand: 99)