From 5434b51f75d219f1ee218ef8d6b0a3aa25c27c03 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 24 Jul 2021 22:50:46 +0100 Subject: [PATCH 01/12] Switch filter to StripeSCA, this must have been an error, must be tested (manually or automatically) --- lib/open_food_network/available_payment_method_filter.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/open_food_network/available_payment_method_filter.rb b/lib/open_food_network/available_payment_method_filter.rb index f15714d1fb..4afbfa94d5 100644 --- a/lib/open_food_network/available_payment_method_filter.rb +++ b/lib/open_food_network/available_payment_method_filter.rb @@ -5,12 +5,12 @@ module OpenFoodNetwork def filter!(payment_methods) if stripe_enabled? payment_methods.to_a.reject! do |payment_method| - payment_method.type.ends_with?("StripeConnect") && + payment_method.type.ends_with?("StripeSCA") && stripe_configuration_incomplete?(payment_method) end else payment_methods.to_a.reject! do |payment_method| - payment_method.type.ends_with?("StripeConnect") + payment_method.type.ends_with?("StripeSCA") end end end From 60a8ae6675594928defee14e6a41199fdfdf8f06 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 24 Jul 2021 22:51:03 +0100 Subject: [PATCH 02/12] Remove Stripe Connect gateway and related code --- .../controllers/details_controller.js.coffee | 2 +- .../spree/admin/payment_methods_controller.rb | 5 +- app/models/spree/gateway/stripe_connect.rb | 109 ------ app/models/subscription.rb | 1 - .../api/admin/payment_method_serializer.rb | 3 +- .../_provider_settings.html.haml | 2 - config/application.rb | 1 - .../subscriptions/stripe_payment_setup.rb | 3 +- .../subscriptions/validator.rb | 3 +- .../subscriptions/validator_spec.rb | 4 +- .../spree/gateway/stripe_connect_spec.rb | 107 ------ spec/requests/checkout/stripe_connect_spec.rb | 310 ------------------ 12 files changed, 8 insertions(+), 542 deletions(-) delete mode 100644 app/models/spree/gateway/stripe_connect.rb delete mode 100644 spec/models/spree/gateway/stripe_connect_spec.rb delete mode 100644 spec/requests/checkout/stripe_connect_spec.rb diff --git a/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee b/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee index cb331a62f3..19cf5316a4 100644 --- a/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee +++ b/app/assets/javascripts/admin/subscriptions/controllers/details_controller.js.coffee @@ -16,7 +16,7 @@ angular.module("admin.subscriptions").controller "DetailsController", ($scope, $ return if !newValue? paymentMethod = ($scope.paymentMethods.filter (pm) -> pm.id == newValue)[0] return unless paymentMethod? - $scope.cardRequired = (paymentMethod.type == "Spree::Gateway::StripeConnect" || paymentMethod.type == "Spree::Gateway::StripeSCA") + $scope.cardRequired = (paymentMethod.type == "Spree::Gateway::StripeSCA") $scope.loadCustomer() if $scope.cardRequired && !$scope.customer $scope.loadCustomer = -> diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 9a882fc5c2..94d95b6a6c 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -145,12 +145,11 @@ module Spree end def stripe_payment_method? - ["Spree::Gateway::StripeConnect", - "Spree::Gateway::StripeSCA"].include? @payment_method.try(:type) + ["Spree::Gateway::StripeSCA"].include? @payment_method.try(:type) end def stripe_provider?(provider) - provider.name.ends_with?("StripeConnect", "StripeSCA") + provider.name.ends_with?("StripeSCA") end def base_params diff --git a/app/models/spree/gateway/stripe_connect.rb b/app/models/spree/gateway/stripe_connect.rb deleted file mode 100644 index 91090569ff..0000000000 --- a/app/models/spree/gateway/stripe_connect.rb +++ /dev/null @@ -1,109 +0,0 @@ -# frozen_string_literal: true - -require 'stripe/profile_storer' - -module Spree - class Gateway - class StripeConnect < Gateway - preference :enterprise_id, :integer - - validate :ensure_enterprise_selected - - def method_type - 'stripe' - end - - def provider_class - ActiveMerchant::Billing::StripeGateway - end - - def payment_profiles_supported? - true - end - - def stripe_account_id - StripeAccount.find_by(enterprise_id: preferred_enterprise_id)&.stripe_user_id - end - - # NOTE: the name of this method is determined by Spree::Payment::Processing - def purchase(money, creditcard, gateway_options) - provider.purchase(*options_for_purchase_or_auth(money, creditcard, gateway_options)) - rescue Stripe::StripeError => e - # This will be an error caused by generating a stripe token - failed_activemerchant_billing_response(e.message) - end - - def charge_offline(money, creditcard, gateway_options) - purchase(money, creditcard, gateway_options) - end - - # NOTE: the name of this method is determined by Spree::Payment::Processing - def void(response_code, _creditcard, gateway_options) - gateway_options[:stripe_account] = stripe_account_id - provider.void(response_code, gateway_options) - end - - # NOTE: the name of this method is determined by Spree::Payment::Processing - def credit(money, _creditcard, response_code, gateway_options) - gateway_options[:stripe_account] = stripe_account_id - provider.refund(money, response_code, gateway_options) - end - - def create_profile(payment) - return unless payment.source.gateway_customer_profile_id.nil? - - profile_storer = Stripe::ProfileStorer.new(payment, provider) - profile_storer.create_customer_from_token - end - - private - - # In this gateway, what we call 'secret_key' is the 'login' - def options - options = super - options.merge(login: Stripe.api_key) - end - - def options_for_purchase_or_auth(money, creditcard, gateway_options) - options = {} - options[:description] = "Spree Order ID: #{gateway_options[:order_id]}" - options[:currency] = gateway_options[:currency] - options[:stripe_account] = stripe_account_id - - creditcard = token_from_card_profile_ids(creditcard) - - [money, creditcard, options] - end - - def token_from_card_profile_ids(creditcard) - token_or_card_id = creditcard.gateway_payment_profile_id - customer = creditcard.gateway_customer_profile_id - - return nil if token_or_card_id.blank? - - # Assume the gateway_payment_profile_id is a token generated by StripeJS - return token_or_card_id if customer.blank? - - # Assume the gateway_payment_profile_id is a Stripe card_id - # So generate a new token, using the customer_id and card_id - tokenize_instance_customer_card(customer, token_or_card_id) - end - - def tokenize_instance_customer_card(customer, card) - token = Stripe::Token.create({ card: card, customer: customer }, - stripe_account: stripe_account_id) - token.id - end - - def failed_activemerchant_billing_response(error_message) - ActiveMerchant::Billing::Response.new(false, error_message) - end - - def ensure_enterprise_selected - return if preferred_enterprise_id&.positive? - - errors.add(:stripe_account_owner, I18n.t(:error_required)) - end - end - end -end diff --git a/app/models/subscription.rb b/app/models/subscription.rb index f533b617f5..6f07fb92ec 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -2,7 +2,6 @@ class Subscription < ApplicationRecord ALLOWED_PAYMENT_METHOD_TYPES = ["Spree::PaymentMethod::Check", - "Spree::Gateway::StripeConnect", "Spree::Gateway::StripeSCA"].freeze searchable_attributes :shop_id, :canceled_at, :paused_at diff --git a/app/serializers/api/admin/payment_method_serializer.rb b/app/serializers/api/admin/payment_method_serializer.rb index 1d80e2025b..116504cfde 100644 --- a/app/serializers/api/admin/payment_method_serializer.rb +++ b/app/serializers/api/admin/payment_method_serializer.rb @@ -6,8 +6,7 @@ module Api delegate :serializable_hash, to: :method_serializer def method_serializer - if object.type == 'Spree::Gateway::StripeConnect' || - object.type == 'Spree::Gateway::StripeSCA' + if object.type == 'Spree::Gateway::StripeSCA' Api::Admin::PaymentMethod::StripeSerializer.new(object) else Api::Admin::PaymentMethod::BaseSerializer.new(object) diff --git a/app/views/spree/admin/payment_methods/_provider_settings.html.haml b/app/views/spree/admin/payment_methods/_provider_settings.html.haml index 735b6052ab..a60a4ecd49 100644 --- a/app/views/spree/admin/payment_methods/_provider_settings.html.haml +++ b/app/views/spree/admin/payment_methods/_provider_settings.html.haml @@ -1,6 +1,4 @@ - case @payment_method -- when Spree::Gateway::StripeConnect - = render 'stripe_connect' - when Spree::Gateway::StripeSCA = render 'stripe_connect' - else diff --git a/config/application.rb b/config/application.rb index 8d7efdcdc1..800cff8ff7 100644 --- a/config/application.rb +++ b/config/application.rb @@ -136,7 +136,6 @@ module Openfoodnetwork # Register Spree payment methods initializer "spree.gateway.payment_methods", :after => "spree.register.payment_methods" do |app| - app.config.spree.payment_methods << Spree::Gateway::StripeConnect app.config.spree.payment_methods << Spree::Gateway::StripeSCA app.config.spree.payment_methods << Spree::Gateway::PayPalExpress end diff --git a/engines/order_management/app/services/order_management/subscriptions/stripe_payment_setup.rb b/engines/order_management/app/services/order_management/subscriptions/stripe_payment_setup.rb index 6a5cf33acb..7620b7a666 100644 --- a/engines/order_management/app/services/order_management/subscriptions/stripe_payment_setup.rb +++ b/engines/order_management/app/services/order_management/subscriptions/stripe_payment_setup.rb @@ -28,8 +28,7 @@ module OrderManagement end def stripe_payment_method? - [Spree::Gateway::StripeConnect, - Spree::Gateway::StripeSCA].include? @payment.payment_method.class + [Spree::Gateway::StripeSCA].include? @payment.payment_method.class end def card_set? diff --git a/engines/order_management/app/services/order_management/subscriptions/validator.rb b/engines/order_management/app/services/order_management/subscriptions/validator.rb index 6fdc0aa9ea..eaa73822e5 100644 --- a/engines/order_management/app/services/order_management/subscriptions/validator.rb +++ b/engines/order_management/app/services/order_management/subscriptions/validator.rb @@ -93,8 +93,7 @@ module OrderManagement end def stripe_payment_method?(payment_method) - payment_method.type == "Spree::Gateway::StripeConnect" || - payment_method.type == "Spree::Gateway::StripeSCA" + payment_method.type == "Spree::Gateway::StripeSCA" end def subscription_line_items_present? diff --git a/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb index 7c52fba12a..5219714424 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb @@ -360,9 +360,9 @@ module OrderManagement end end - context "when using the StripeConnect payment gateway" do + context "when using the StripeSCA payment gateway" do let(:payment_method) { - instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeConnect") + instance_double(Spree::PaymentMethod, type: "Spree::Gateway::StripeSCA") } before { expect(subscription).to receive(:customer).at_least(:once) { customer } } diff --git a/spec/models/spree/gateway/stripe_connect_spec.rb b/spec/models/spree/gateway/stripe_connect_spec.rb deleted file mode 100644 index 4b25c81439..0000000000 --- a/spec/models/spree/gateway/stripe_connect_spec.rb +++ /dev/null @@ -1,107 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Spree::Gateway::StripeConnect, type: :model do - let(:provider) do - instance_double(ActiveMerchant::Billing::StripeGateway).tap do |p| - allow(p).to receive(:purchase) - allow(p).to receive(:authorize) - allow(p).to receive(:capture) - allow(p).to receive(:refund) - end - end - - let(:stripe_account_id) { "acct_123" } - - before do - Stripe.api_key = "sk_test_123456" - allow(subject).to receive(:stripe_account_id) { stripe_account_id } - allow(subject).to receive(:options_for_purchase_or_auth).and_return(['money', 'cc', 'opts']) - allow(subject).to receive(:provider).and_return provider - end - - describe "#token_from_card_profile_ids" do - let(:creditcard) { double(:creditcard) } - context "when the credit card provided has a gateway_payment_profile_id" do - before do - allow(creditcard).to receive(:gateway_payment_profile_id) { "token_or_card_id123" } - allow(subject).to receive(:tokenize_instance_customer_card) { "tokenized" } - end - - context "when the credit card provided has a gateway_customer_profile_id" do - before { allow(creditcard).to receive(:gateway_customer_profile_id) { "customer_id123" } } - - it "requests a new token via tokenize_instance_customer_card" do - result = subject.send(:token_from_card_profile_ids, creditcard) - expect(result).to eq "tokenized" - end - end - - context "when the credit card provided does not have a gateway_customer_profile_id" do - before { allow(creditcard).to receive(:gateway_customer_profile_id) { nil } } - it "returns the gateway_payment_profile_id (assumed to be a token already)" do - result = subject.send(:token_from_card_profile_ids, creditcard) - expect(result).to eq "token_or_card_id123" - end - end - end - - context "when the credit card provided does not have a gateway_payment_profile_id" do - before { allow(creditcard).to receive(:gateway_payment_profile_id) { nil } } - before { allow(creditcard).to receive(:gateway_customer_profile_id) { "customer_id123" } } - - it "returns nil....?" do - result = subject.send(:token_from_card_profile_ids, creditcard) - expect(result).to be nil - end - end - end - - describe "#tokenize_instance_customer_card" do - let(:customer_id) { "customer123" } - let(:card_id) { "card123" } - let(:token_mock) { { id: "test_token123" } } - - before do - stub_request(:post, "https://api.stripe.com/v1/tokens") - .with(body: { "card" => "card123", "customer" => "customer123" }) - .to_return(body: JSON.generate(token_mock)) - end - - it "requests a new token for the customer and card from Stripe, and returns the id of the response" do - expect(subject.send(:tokenize_instance_customer_card, customer_id, - card_id)).to eq token_mock[:id] - end - end - - describe "#credit" do - let(:gateway_options) { { some: 'option' } } - let(:money) { double(:money) } - let(:response_code) { double(:response_code) } - - before do - subject.credit(money, double(:creditcard), response_code, gateway_options) - end - - it "delegates to ActiveMerchant::Billing::StripeGateway#refund" do - expect(provider).to have_received(:refund) - end - - it "adds the stripe_account to the gateway options hash" do - expect(provider).to have_received(:refund).with(money, response_code, - hash_including(stripe_account: stripe_account_id)) - end - end - - describe "#charging offline" do - let(:gateway_options) { { some: 'option' } } - let(:money) { double(:money) } - let(:card) { double(:creditcard) } - - it "uses #purchase to charge offline" do - subject.charge_offline(money, card, gateway_options) - expect(provider).to have_received(:purchase).with('money', 'cc', 'opts') - end - end -end diff --git a/spec/requests/checkout/stripe_connect_spec.rb b/spec/requests/checkout/stripe_connect_spec.rb deleted file mode 100644 index 4c7fe894ac..0000000000 --- a/spec/requests/checkout/stripe_connect_spec.rb +++ /dev/null @@ -1,310 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe "checking out an order with a Stripe Connect payment method", type: :request do - include ShopWorkflow - include AuthenticationHelper - include OpenFoodNetwork::ApiHelper - - let!(:order_cycle) { create(:simple_order_cycle) } - let!(:enterprise) { create(:distributor_enterprise) } - let!(:shipping_method) do - create( - :shipping_method, - calculator: Calculator::FlatRate.new(preferred_amount: 0), - distributors: [enterprise] - ) - end - let!(:payment_method) { create(:stripe_connect_payment_method, distributors: [enterprise]) } - let!(:stripe_account) { create(:stripe_account, enterprise: enterprise) } - let!(:line_item) { create(:line_item, price: 12.34) } - let!(:order) { line_item.order } - let(:address) { create(:address) } - let(:token) { "token123" } - let(:new_token) { "newtoken123" } - let(:card_id) { "card_XyZ456" } - let(:customer_id) { "cus_A123" } - let(:payments_attributes) do - { - payment_method_id: payment_method.id, - source_attributes: { - gateway_payment_profile_id: token, - cc_type: "visa", - last_digits: "4242", - month: 10, - year: 2025, - first_name: 'Jill', - last_name: 'Jeffreys' - } - } - end - let(:allowed_address_attributes) do - [ - "firstname", - "lastname", - "address1", - "address2", - "phone", - "city", - "zipcode", - "state_id", - "country_id" - ] - end - let(:params) do - { - format: :json, order: { - shipping_method_id: shipping_method.id, - payments_attributes: [payments_attributes], - bill_address_attributes: address.attributes.slice(*allowed_address_attributes), - ship_address_attributes: address.attributes.slice(*allowed_address_attributes) - } - } - end - - before do - order_cycle_distributed_variants = double(:order_cycle_distributed_variants) - allow(OrderCycleDistributedVariants).to receive(:new) { order_cycle_distributed_variants } - allow(order_cycle_distributed_variants).to receive(:distributes_order_variants?) { true } - - Stripe.api_key = "sk_test_12345" - order.update(distributor_id: enterprise.id, order_cycle_id: order_cycle.id) - order.reload.update_totals - set_order order - end - - context "when a new card is submitted" do - let(:store_response_mock) do - { - status: 200, - body: JSON.generate( - id: customer_id, - default_card: card_id, - sources: { data: [{ id: "1" }] } - ) - } - end - let(:token_response_mock) do - { status: 200, body: JSON.generate(id: new_token) } - end - let(:charge_response_mock) do - { status: 200, body: JSON.generate(id: "ch_1234", object: "charge", amount: 2000) } - end - - context "and the user doesn't request that the card is saved for later" do - before do - # Charges the card - stub_request(:post, "https://api.stripe.com/v1/charges") - .with(basic_auth: ["sk_test_12345", ""], body: /#{token}.*#{order.number}/) - .to_return(charge_response_mock) - end - - context "and the charge request is successful" do - it "should process the payment without storing card details" do - put update_checkout_path, params: params - - expect(json_response["path"]).to eq order_path(order) - expect(order.payments.completed.count).to be 1 - - card = order.payments.completed.first.source - - expect(card.gateway_customer_profile_id).to eq nil - expect(card.gateway_payment_profile_id).to eq token - expect(card.cc_type).to eq "visa" - expect(card.last_digits).to eq "4242" - expect(card.first_name).to eq "Jill" - expect(card.last_name).to eq "Jeffreys" - end - end - - context "when the charge request returns an error message" do - let(:charge_response_mock) do - { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } - end - - it "should not process the payment" do - put update_checkout_path, params: params - - expect(response.status).to be 400 - - expect(json_response["flash"]["error"]).to eq "charge-failure" - expect(order.payments.completed.count).to be 0 - end - end - end - - context "and the customer requests that the card is saved for later" do - before do - source_attributes = params[:order][:payments_attributes][0][:source_attributes] - source_attributes[:save_requested_by_customer] = '1' - - # Saves the card against the user - stub_request(:post, "https://api.stripe.com/v1/customers") - .with(basic_auth: ["sk_test_12345", ""], body: { card: token, email: order.email }) - .to_return(store_response_mock) - - # Requests a token from the newly saved card - stub_request(:post, "https://api.stripe.com/v1/tokens") - .with(body: { card: card_id, customer: customer_id }) - .to_return(token_response_mock) - - # Charges the card - stub_request(:post, "https://api.stripe.com/v1/charges") - .with( - basic_auth: ["sk_test_12345", ""], - body: /#{token}.*#{order.number}/ - ).to_return(charge_response_mock) - end - - context "and the store, token and charge requests are successful" do - it "should process the payment, and stores the card/customer details" do - put update_checkout_path, params: params - - expect(json_response["path"]).to eq order_path(order) - expect(order.payments.completed.count).to be 1 - - card = order.payments.completed.first.source - - expect(card.gateway_customer_profile_id).to eq customer_id - expect(card.gateway_payment_profile_id).to eq card_id - expect(card.cc_type).to eq "visa" - expect(card.last_digits).to eq "4242" - expect(card.first_name).to eq "Jill" - expect(card.last_name).to eq "Jeffreys" - end - end - - context "when the store request returns an error message" do - let(:store_response_mock) do - { status: 402, body: JSON.generate(error: { message: "store-failure" }) } - end - - it "should not process the payment" do - put update_checkout_path, params: params - - expect(response.status).to be 400 - - expect(json_response["flash"]["error"]) - .to eq(I18n.t(:spree_gateway_error_flash_for_checkout, error: 'store-failure')) - expect(order.payments.completed.count).to be 0 - end - end - - context "when the charge request returns an error message" do - let(:charge_response_mock) do - { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } - end - - it "should not process the payment" do - put update_checkout_path, params: params - - expect(response.status).to be 400 - - expect(json_response["flash"]["error"]).to eq "charge-failure" - expect(order.payments.completed.count).to be 0 - end - end - - context "when the token request returns an error message" do - let(:token_response_mock) do - { status: 402, body: JSON.generate(error: { message: "token-failure" }) } - end - - # Note, no requests have been stubbed - it "should not process the payment" do - put update_checkout_path, params: params - - expect(response.status).to be 400 - - expect(json_response["flash"]["error"]).to eq "token-failure" - expect(order.payments.completed.count).to be 0 - end - end - end - end - - context "when an existing card is submitted" do - let(:credit_card) do - create( - :credit_card, - user_id: order.user_id, - gateway_payment_profile_id: card_id, - gateway_customer_profile_id: customer_id, - last_digits: "4321", - cc_type: "master", - first_name: "Sammy", - last_name: "Signpost", - month: 11, year: 2026 - ) - end - - let(:token_response_mock) { { status: 200, body: JSON.generate(id: new_token) } } - let(:charge_response_mock) do - { status: 200, body: JSON.generate(id: "ch_1234", object: "charge", amount: 2000) } - end - - before do - params[:order][:existing_card_id] = credit_card.id - login_as(order.user) - - # Requests a token - stub_request(:post, "https://api.stripe.com/v1/tokens") - .with(body: { "card" => card_id, "customer" => customer_id }) - .to_return(token_response_mock) - - # Charges the card - stub_request(:post, "https://api.stripe.com/v1/charges") - .with(basic_auth: ["sk_test_12345", ""], body: /#{token}.*#{order.number}/) - .to_return(charge_response_mock) - end - - context "and the charge and token requests are accepted" do - it "should process the payment, and keep the profile ids and other card details" do - put update_checkout_path, params: params - - expect(json_response["path"]).to eq order_path(order) - expect(order.payments.completed.count).to be 1 - - card = order.payments.completed.first.source - - expect(card.gateway_customer_profile_id).to eq customer_id - expect(card.gateway_payment_profile_id).to eq card_id - expect(card.cc_type).to eq "master" - expect(card.last_digits).to eq "4321" - expect(card.first_name).to eq "Sammy" - expect(card.last_name).to eq "Signpost" - end - end - - context "when the charge request returns an error message" do - let(:charge_response_mock) do - { status: 402, body: JSON.generate(error: { message: "charge-failure" }) } - end - - it "should not process the payment" do - put update_checkout_path, params: params - - expect(response.status).to be 400 - - expect(json_response["flash"]["error"]).to eq "charge-failure" - expect(order.payments.completed.count).to be 0 - end - end - - context "when the token request returns an error message" do - let(:token_response_mock) do - { status: 402, body: JSON.generate(error: { message: "token-error" }) } - end - - it "should not process the payment" do - put update_checkout_path, params: params - - expect(response.status).to be 400 - - expect(json_response["flash"]["error"]).to eq "token-error" - expect(order.payments.completed.count).to be 0 - end - end - end -end From 9de9b4157553d4a8ac0f6cdc5e6e9bee980f8e76 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 24 Jul 2021 22:54:30 +0100 Subject: [PATCH 03/12] Remove stripe connect spec, there's a similar spec for stripesca below --- .../payments_controller_refunds_spec.rb | 120 ------------------ 1 file changed, 120 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 dccb39edc7..258292a562 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 @@ -16,126 +16,6 @@ describe Spree::Admin::PaymentsController, type: :controller 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 } } - - # 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_connect_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 - Stripe.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_f).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_f).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_f).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_f).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_connect_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 - Stripe.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_f).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_f).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_f).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_f).to eq(-5) - expect(flash[:error]).to eq "Bup-bow!" - end - end - end - end - end - context "StripeSCA" do context "voiding a payment" do let(:params) { { id: payment.id, order_id: order.number, e: :void } } From f90e34bb47bb2579cf1943f71d9acf66f5186228 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 24 Jul 2021 23:04:50 +0100 Subject: [PATCH 04/12] Adapt specs to use stripe_sca_payment_method factory and delete stripe_connect_payment_method --- .../stripe_payment_setup_spec.rb | 2 +- .../admin/subscriptions_controller_spec.rb | 2 +- .../payments/payments_controller_spec.rb | 2 +- .../admin/payment_methods_controller_spec.rb | 6 +- spec/factories/payment_method_factory.rb | 7 --- spec/features/admin/subscriptions_spec.rb | 6 +- .../consumer/shopping/checkout_stripe_spec.rb | 59 ------------------- spec/helpers/enterprises_helper_spec.rb | 4 +- spec/jobs/subscription_confirm_job_spec.rb | 6 +- spec/lib/stripe/profile_storer_spec.rb | 2 +- spec/models/spree/order_spec.rb | 2 +- spec/models/spree/payment_spec.rb | 4 +- 12 files changed, 18 insertions(+), 84 deletions(-) diff --git a/engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb index ee4d754b49..7936648f15 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb @@ -34,7 +34,7 @@ module OrderManagement end context "when the payment method is a stripe payment method" do - let(:payment_method) { create(:stripe_connect_payment_method) } + let(:payment_method) { create(:stripe_sca_payment_method) } context "and the card is already set (the payment source is a credit card)" do it "returns the pending payment with no change" do diff --git a/spec/controllers/admin/subscriptions_controller_spec.rb b/spec/controllers/admin/subscriptions_controller_spec.rb index 125271c483..8ca50fdacf 100644 --- a/spec/controllers/admin/subscriptions_controller_spec.rb +++ b/spec/controllers/admin/subscriptions_controller_spec.rb @@ -753,7 +753,7 @@ describe Admin::SubscriptionsController, type: :controller do end context "when other payment methods exist" do - let!(:stripe) { create(:stripe_connect_payment_method, distributors: [shop]) } + let!(:stripe) { create(:stripe_sca_payment_method, distributors: [shop]) } let!(:paypal) { Spree::Gateway::PayPalExpress.create!(name: "PayPalExpress", distributor_ids: [shop.id]) } 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 47d0376534..9b4610069d 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb @@ -45,7 +45,7 @@ describe Spree::Admin::PaymentsController, type: :controller do end context "with Stripe payment where payment.process! errors out" do - let!(:payment_method) { create(:stripe_connect_payment_method, distributors: [shop]) } + let!(:payment_method) { create(:stripe_sca_payment_method, distributors: [shop]) } before do allow_any_instance_of(Spree::Payment). to receive(:process_offline!). diff --git a/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/spec/controllers/spree/admin/payment_methods_controller_spec.rb index 418a7a2a56..cab1c80092 100644 --- a/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -108,12 +108,12 @@ module Spree end end - context "on a StripeConnect payment method" do + context "on a StripeSCA payment method" do let!(:user) { create(:user, enterprise_limit: 2) } let!(:enterprise1) { create(:distributor_enterprise, owner: user) } let!(:enterprise2) { create(:distributor_enterprise, owner: create(:user)) } let!(:payment_method) { - create(:stripe_connect_payment_method, distributor_ids: [enterprise1.id, enterprise2.id], + create(:stripe_sca_payment_method, distributor_ids: [enterprise1.id, enterprise2.id], preferred_enterprise_id: enterprise2.id) } @@ -124,7 +124,7 @@ module Spree { id: payment_method.id, payment_method: { - type: "Spree::Gateway::StripeConnect", + type: "Spree::Gateway::StripeSCA", preferred_enterprise_id: enterprise1.id } } diff --git a/spec/factories/payment_method_factory.rb b/spec/factories/payment_method_factory.rb index 0e796b050a..aadb01f9d6 100644 --- a/spec/factories/payment_method_factory.rb +++ b/spec/factories/payment_method_factory.rb @@ -23,13 +23,6 @@ FactoryBot.define do environment { 'test' } end - factory :stripe_connect_payment_method, class: Spree::Gateway::StripeConnect do - name { 'StripeConnect' } - environment { 'test' } - distributors { [FactoryBot.create(:enterprise)] } - preferred_enterprise_id { distributors.first.id } - end - factory :stripe_sca_payment_method, class: Spree::Gateway::StripeSCA do name { 'StripeSCA' } environment { 'test' } diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index b62f3b31a8..06a4cc4802 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -220,7 +220,7 @@ feature 'Subscriptions' do } let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } let!(:payment_method) { - create(:stripe_connect_payment_method, name: 'Credit Card', distributors: [shop]) + create(:stripe_sca_payment_method, name: 'Credit Card', distributors: [shop]) } let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } @@ -387,7 +387,7 @@ feature 'Subscriptions' do } let!(:payment_method) { create(:payment_method, distributors: [shop]) } let!(:stripe_payment_method) { - create(:stripe_connect_payment_method, name: 'Credit Card', distributors: [shop]) + create(:stripe_sca_payment_method, name: 'Credit Card', distributors: [shop]) } let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } let!(:subscription) { @@ -536,7 +536,7 @@ feature 'Subscriptions' do let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) } let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } - let!(:payment_method) { create(:stripe_connect_payment_method, distributors: [shop]) } + let!(:payment_method) { create(:stripe_sca_payment_method, distributors: [shop]) } let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } before do diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb index c81c44d577..e25eba85b2 100644 --- a/spec/features/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb @@ -37,65 +37,6 @@ feature "Check out with Stripe", js: true do distributor.shipping_methods << [shipping_with_fee, free_shipping] end - context 'login in as user' do - let(:user) { create(:user) } - - before do - login_as(user) - end - - context "with Stripe Connect" do - let!(:stripe_pm) do - create(:stripe_connect_payment_method, distributors: [distributor]) - end - - let!(:saved_card) do - create(:credit_card, - user_id: user.id, - month: "01", - year: "2025", - cc_type: "visa", - number: "1111111111111111", - payment_method_id: stripe_pm.id, - gateway_customer_profile_id: "i_am_saved") - end - - let!(:stripe_account) { - create(:stripe_account, enterprise_id: distributor.id, stripe_user_id: 'some_id') - } - - let(:response_mock) { { id: "ch_1234", object: "charge", amount: 2000 } } - - around do |example| - original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled] - example.run - Spree::Config.set(stripe_connect_enabled: original_stripe_connect_enabled) - end - - before do - stub_request(:post, "https://api.stripe.com/v1/charges") - .with(basic_auth: ["sk_test_12345", ""]) - .to_return(status: 200, body: JSON.generate(response_mock)) - - visit checkout_path - fill_out_form(shipping_with_fee.name, stripe_pm.name, save_default_addresses: false) - end - - it "allows use of a saved card" do - # shows the saved credit card dropdown - expect(page).to have_content I18n.t("spree.checkout.payment.stripe.used_saved_card") - - # default card is selected, form element is not shown - expect(page).to have_no_selector "#card-element.StripeElement" - expect(page).to have_select 'selected_card', selected: "Visa x-1111 Exp:01/2025" - - # allows checkout - place_order - expect(page).to have_content "Your order has been processed successfully" - end - end - end - describe "using Stripe SCA" do let!(:stripe_account) { create(:stripe_account, enterprise: distributor) } let!(:stripe_sca_payment_method) { diff --git a/spec/helpers/enterprises_helper_spec.rb b/spec/helpers/enterprises_helper_spec.rb index 5959392667..bfd331b4ac 100644 --- a/spec/helpers/enterprises_helper_spec.rb +++ b/spec/helpers/enterprises_helper_spec.rb @@ -264,11 +264,11 @@ describe EnterprisesHelper, type: :helper do context "when StripeConnect payment methods are present" do let!(:pm3) { - create(:stripe_connect_payment_method, distributors: [distributor], + create(:stripe_sca_payment_method, distributors: [distributor], preferred_enterprise_id: distributor.id) } let!(:pm4) { - create(:stripe_connect_payment_method, distributors: [distributor], + create(:stripe_sca_payment_method, distributors: [distributor], preferred_enterprise_id: some_other_distributor.id) } let(:available_payment_methods) { helper.available_payment_methods } diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index 8c2ac1a2cb..d5a6db2ca5 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -192,13 +192,13 @@ describe SubscriptionConfirmJob do end context "Stripe Connect" do - let(:stripe_connect_payment_method) { create(:stripe_connect_payment_method) } - let(:stripe_connect_payment) { + let(:stripe_sca_payment_method) { create(:stripe_sca_payment_method) } + let(:stripe_sca_payment) { create(:payment, amount: 10, payment_method: stripe_connect_payment_method) } before do - allow(order).to receive(:pending_payments) { [stripe_connect_payment] } + allow(order).to receive(:pending_payments) { [stripe_sca_payment] } allow(stripe_connect_payment_method).to receive(:purchase) { true } end diff --git a/spec/lib/stripe/profile_storer_spec.rb b/spec/lib/stripe/profile_storer_spec.rb index d23ed84e95..75c94a7ad6 100644 --- a/spec/lib/stripe/profile_storer_spec.rb +++ b/spec/lib/stripe/profile_storer_spec.rb @@ -6,7 +6,7 @@ module Stripe describe ProfileStorer do describe "create_customer_from_token" do let(:payment) { create(:payment) } - let(:stripe_payment_method) { create(:stripe_connect_payment_method) } + let(:stripe_payment_method) { create(:stripe_sca_payment_method) } let(:profile_storer) { Stripe::ProfileStorer.new(payment, stripe_payment_method.provider) } let(:customer_id) { "cus_A123" } diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index ecc4d5dae8..0d485041f9 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -1213,7 +1213,7 @@ describe Spree::Order do let!(:enterprise) { create(:enterprise) } let!(:order) { create(:order, distributor: enterprise) } let!(:payment_method) { - create(:stripe_connect_payment_method, distributor_ids: [enterprise.id]) + create(:stripe_sca_payment_method, distributor_ids: [enterprise.id]) } let!(:payment) { create(:payment, order: order, payment_method: payment_method) } diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 27c6700cf1..c094740f87 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -904,8 +904,8 @@ describe Spree::Payment do context "to Stripe payments" do let(:shop) { create(:enterprise) } let(:payment_method) { - create(:stripe_connect_payment_method, distributor_ids: [create(:distributor_enterprise).id], - preferred_enterprise_id: shop.id) + create(:stripe_sca_payment_method, distributor_ids: [create(:distributor_enterprise).id], + preferred_enterprise_id: shop.id) } let(:payment) { create(:payment, order: order, payment_method: payment_method, amount: order.total) From 0c240cee9a0b6cf3070b911103457f7cfdd9a73a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 24 Jul 2021 23:17:42 +0100 Subject: [PATCH 05/12] A few more removals of stripe connect related code --- .rubocop_manual_todo.yml | 1 - .rubocop_todo.yml | 2 -- knapsack_rspec_report.json | 2 -- spec/helpers/enterprises_helper_spec.rb | 6 +++--- spec/jobs/subscription_confirm_job_spec.rb | 6 +++--- spec/models/spree/gateway_tagging_spec.rb | 4 ++-- 6 files changed, 8 insertions(+), 13 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 6e2e25cc86..1494b0cc92 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -203,7 +203,6 @@ Layout/LineLength: - spec/models/spree/address_spec.rb - spec/models/spree/adjustment_spec.rb - spec/models/spree/classification_spec.rb - - spec/models/spree/gateway/stripe_connect_spec.rb - spec/models/spree/inventory_unit_spec.rb - spec/models/spree/line_item_spec.rb - spec/models/spree/order/checkout_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a643372650..581af643b0 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -622,7 +622,6 @@ Style/NumericPredicate: - 'app/helpers/shared_helper.rb' - 'app/models/product_import/product_importer.rb' - 'app/models/product_import/spreadsheet_entry.rb' - - 'app/models/spree/gateway/stripe_connect.rb' - 'app/models/spree/line_item.rb' - 'app/models/spree/order.rb' - 'app/models/spree/order_contents.rb' @@ -698,7 +697,6 @@ Style/Send: - 'spec/models/calculator/weight_spec.rb' - 'spec/models/enterprise_spec.rb' - 'spec/models/exchange_spec.rb' - - 'spec/models/spree/gateway/stripe_connect_spec.rb' - 'spec/models/spree/order_inventory_spec.rb' - 'spec/models/spree/order_spec.rb' - 'spec/models/spree/payment_spec.rb' diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index 4324cbcf86..2940d5baf4 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -17,7 +17,6 @@ "spec/controllers/line_items_controller_spec.rb": 9.073998928070068, "spec/lib/open_food_network/address_finder_spec.rb": 8.379472494125366, "spec/features/admin/enterprises/index_spec.rb": 4.946447134017944, - "spec/requests/checkout/stripe_connect_spec.rb": 7.5777587890625, "spec/models/spree/adjustment_spec.rb": 7.560959815979004, "spec/serializers/api/product_serializer_spec.rb": 8.135705709457397, "spec/lib/open_food_network/permissions_spec.rb": 3.799001455307007, @@ -233,7 +232,6 @@ "spec/lib/tasks/enterprises_rake_spec.rb": 0.0833287239074707, "spec/controllers/api/taxonomies_controller_spec.rb": 0.08802604675292969, "spec/helpers/spree/admin/orders_helper_spec.rb": 0.0646059513092041, - "spec/models/spree/gateway/stripe_connect_spec.rb": 0.05724668502807617, "spec/helpers/shop_helper_spec.rb": 0.3690376281738281, "spec/jobs/welcome_enterprise_job_spec.rb": 0.08383440971374512, "spec/jobs/confirm_order_job_spec.rb": 0.06512808799743652, diff --git a/spec/helpers/enterprises_helper_spec.rb b/spec/helpers/enterprises_helper_spec.rb index bfd331b4ac..7029ede7c3 100644 --- a/spec/helpers/enterprises_helper_spec.rb +++ b/spec/helpers/enterprises_helper_spec.rb @@ -262,14 +262,14 @@ describe EnterprisesHelper, type: :helper do end end - context "when StripeConnect payment methods are present" do + context "when Stripe payment methods are present" do let!(:pm3) { create(:stripe_sca_payment_method, distributors: [distributor], - preferred_enterprise_id: distributor.id) + preferred_enterprise_id: distributor.id) } let!(:pm4) { create(:stripe_sca_payment_method, distributors: [distributor], - preferred_enterprise_id: some_other_distributor.id) + preferred_enterprise_id: some_other_distributor.id) } let(:available_payment_methods) { helper.available_payment_methods } diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index d5a6db2ca5..ca93fc0f23 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -194,17 +194,17 @@ describe SubscriptionConfirmJob do context "Stripe Connect" do let(:stripe_sca_payment_method) { create(:stripe_sca_payment_method) } let(:stripe_sca_payment) { - create(:payment, amount: 10, payment_method: stripe_connect_payment_method) + create(:payment, amount: 10, payment_method: stripe_sca_payment_method) } before do allow(order).to receive(:pending_payments) { [stripe_sca_payment] } - allow(stripe_connect_payment_method).to receive(:purchase) { true } + allow(stripe_sca_payment_method).to receive(:purchase) { true } end it "runs the charges in offline mode" do job.send(:confirm_order!, order) - expect(stripe_connect_payment_method).to have_received(:purchase) + expect(stripe_sca_payment_method).to have_received(:purchase) end end end diff --git a/spec/models/spree/gateway_tagging_spec.rb b/spec/models/spree/gateway_tagging_spec.rb index 35b08ae9bb..410ef1ec35 100644 --- a/spec/models/spree/gateway_tagging_spec.rb +++ b/spec/models/spree/gateway_tagging_spec.rb @@ -45,9 +45,9 @@ module Spree it_behaves_like "taggable", "Spree::PaymentMethod" end - describe Gateway::StripeConnect do + describe Gateway::StripeSCA do subject do - # StripeConnect needs an owner to be valid. + # StripeSCA needs an owner to be valid. valid_subject.tap { |m| m.preferred_enterprise_id = shop.id } end From 48c1312ceada4aa007202f43c2f181a2cbe12be5 Mon Sep 17 00:00:00 2001 From: Nihal Mohammed Date: Thu, 5 Aug 2021 14:49:24 +0530 Subject: [PATCH 06/12] Remove expectation to generate clean name for StripeConnect from payment_method_spec --- spec/models/spree/payment_method_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index 324b1aebd5..d82b794b0a 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -60,7 +60,6 @@ module Spree it "generates a clean name for known Payment Method types" do expect(Spree::PaymentMethod::Check.clean_name).to eq(I18n.t("spree.admin.payment_methods.providers.check")) expect(Spree::Gateway::PayPalExpress.clean_name).to eq(I18n.t("spree.admin.payment_methods.providers.paypalexpress")) - expect(Spree::Gateway::StripeConnect.clean_name).to eq(I18n.t("spree.admin.payment_methods.providers.stripeconnect")) expect(Spree::Gateway::StripeSCA.clean_name).to eq(I18n.t("spree.admin.payment_methods.providers.stripesca")) expect(Spree::Gateway::BogusSimple.clean_name).to eq(I18n.t("spree.admin.payment_methods.providers.bogussimple")) expect(Spree::Gateway::Bogus.clean_name).to eq(I18n.t("spree.admin.payment_methods.providers.bogus")) From 696d43497918c31f3ec93936399023d01bf0fc99 Mon Sep 17 00:00:00 2001 From: Nihal Mohammed Date: Thu, 5 Aug 2021 16:53:03 +0530 Subject: [PATCH 07/12] Removed stripe connect example and fixed JSON generation in profile_storer spec --- spec/lib/stripe/profile_storer_spec.rb | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/spec/lib/stripe/profile_storer_spec.rb b/spec/lib/stripe/profile_storer_spec.rb index 75c94a7ad6..d39190e0d9 100644 --- a/spec/lib/stripe/profile_storer_spec.rb +++ b/spec/lib/stripe/profile_storer_spec.rb @@ -21,22 +21,9 @@ module Stripe .to_return(customer_response_mock) end - context "when called from Stripe Connect" do - let(:customer_response_body) { - JSON.generate(id: customer_id, default_card: card_id, sources: { data: [{ id: "1" }] }) - } - - it "fetches the customer id and the card id from the correct response fields" do - profile_storer.create_customer_from_token - - expect(payment.source.gateway_customer_profile_id).to eq customer_id - expect(payment.source.gateway_payment_profile_id).to eq card_id - end - end - context "when called from Stripe SCA" do let(:customer_response_body) { - JSON.generate(customer: customer_id, id: card_id, sources: { data: [{ id: "1" }] }) + JSON.generate(customer: customer_id, default_card: card_id, sources: { data: [{ id: "1" }] }) } it "fetches the customer id and the card id from the correct response fields" do From 6ac835de2e3092f1c04330d2e9ee8a54ca42ebf6 Mon Sep 17 00:00:00 2001 From: Nihal Mohammed Date: Thu, 5 Aug 2021 16:53:34 +0530 Subject: [PATCH 08/12] Fix response for payment_profile_id in profile_storer.rb --- lib/stripe/profile_storer.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/stripe/profile_storer.rb b/lib/stripe/profile_storer.rb index 688bb4f9e8..fb5b7d8ae9 100644 --- a/lib/stripe/profile_storer.rb +++ b/lib/stripe/profile_storer.rb @@ -67,11 +67,7 @@ module Stripe end def payment_profile_id(response) - if response.params['customer'] # Payment Intents API - response.params['id'] - else - response.params['default_source'] || response.params['default_card'] - end + response.params['default_source'] || response.params['default_card'] end end end From fa9f6432c3278d776cf3a5a3a7644908460585fa Mon Sep 17 00:00:00 2001 From: Nihal Mohammed Date: Fri, 6 Aug 2021 14:49:59 +0530 Subject: [PATCH 09/12] Fix profile_storer.rb and profile_storer_spec --- lib/stripe/profile_storer.rb | 8 ++++++-- spec/lib/stripe/profile_storer_spec.rb | 18 +++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/stripe/profile_storer.rb b/lib/stripe/profile_storer.rb index fb5b7d8ae9..c7fe1831f4 100644 --- a/lib/stripe/profile_storer.rb +++ b/lib/stripe/profile_storer.rb @@ -65,9 +65,13 @@ module Stripe def customer_profile_id(response) response.params['customer'] || response.params['id'] end - + def payment_profile_id(response) - response.params['default_source'] || response.params['default_card'] + if response.params['customer'] # Payment Intents API + response.params['id'] + else + response.params['default_source'] || response.params['default_card'] + end end end end diff --git a/spec/lib/stripe/profile_storer_spec.rb b/spec/lib/stripe/profile_storer_spec.rb index d39190e0d9..e7b69f8c8c 100644 --- a/spec/lib/stripe/profile_storer_spec.rb +++ b/spec/lib/stripe/profile_storer_spec.rb @@ -4,28 +4,28 @@ require 'spec_helper' module Stripe describe ProfileStorer do + include StripeStubs + describe "create_customer_from_token" do - let(:payment) { create(:payment) } let(:stripe_payment_method) { create(:stripe_sca_payment_method) } + let(:card) { create(:credit_card, gateway_payment_profile_id: card_id) } + let(:payment) { create(:payment, source: card, payment_method: stripe_payment_method) } let(:profile_storer) { Stripe::ProfileStorer.new(payment, stripe_payment_method.provider) } let(:customer_id) { "cus_A123" } let(:card_id) { "card_2342" } - let(:customer_response_mock) { { status: 200, body: customer_response_body } } + let(:customer_response_mock) { + { status: 200, body: JSON.generate(id: customer_id, sources: { data: [{ id: "1" }] }) } + } before do Stripe.api_key = "sk_test_12345" - stub_request(:post, "https://api.stripe.com/v1/customers") - .with(basic_auth: ["sk_test_12345", ""], body: { email: payment.order.email }) - .to_return(customer_response_mock) + stub_customers_post_request(email: payment.order.email, response: customer_response_mock) + stub_payment_method_attach_request(payment_method: card_id, customer: customer_id) end context "when called from Stripe SCA" do - let(:customer_response_body) { - JSON.generate(customer: customer_id, default_card: card_id, sources: { data: [{ id: "1" }] }) - } - it "fetches the customer id and the card id from the correct response fields" do profile_storer.create_customer_from_token From eb17c208f9622a2d6408587086dc4988d3afbf39 Mon Sep 17 00:00:00 2001 From: Nihal Mohammed Date: Fri, 6 Aug 2021 14:50:28 +0530 Subject: [PATCH 10/12] Update stripe_stubs to handle stripeSCA --- spec/support/request/stripe_stubs.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/support/request/stripe_stubs.rb b/spec/support/request/stripe_stubs.rb index 637c8b7ab7..53bf241c3b 100644 --- a/spec/support/request/stripe_stubs.rb +++ b/spec/support/request/stripe_stubs.rb @@ -28,11 +28,11 @@ module StripeStubs end # Attaches the payment method to the customer in the hub's stripe account - def stub_payment_method_attach_request + def stub_payment_method_attach_request(payment_method: "pm_123", customer: "cus_A123") stub_request(:post, - "https://api.stripe.com/v1/payment_methods/pm_123/attach") - .with(body: { customer: "cus_A123" }) - .to_return(hub_payment_method_response_mock({ pm_id: "pm_123" })) + "https://api.stripe.com/v1/payment_methods/#{payment_method}/attach") + .with(body: { customer: customer }) + .to_return(hub_payment_method_response_mock({ pm_id: payment_method })) end def stub_retrieve_payment_method_request(payment_method_id = "pm_1234") From 98ff3980f8b7cfea92f870a1c8fdcf57dfbb60b6 Mon Sep 17 00:00:00 2001 From: Nihal Mohammed Date: Tue, 17 Aug 2021 15:37:06 +0530 Subject: [PATCH 11/12] Migration of StripeConnect to StripeSCA --- ...57_rename_stripe_connect_payment_methods_to_stripe_sca.rb | 5 +++++ db/schema.rb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20210817100257_rename_stripe_connect_payment_methods_to_stripe_sca.rb diff --git a/db/migrate/20210817100257_rename_stripe_connect_payment_methods_to_stripe_sca.rb b/db/migrate/20210817100257_rename_stripe_connect_payment_methods_to_stripe_sca.rb new file mode 100644 index 0000000000..8c4447b6de --- /dev/null +++ b/db/migrate/20210817100257_rename_stripe_connect_payment_methods_to_stripe_sca.rb @@ -0,0 +1,5 @@ +class RenameStripeConnectPaymentMethodsToStripeSca < ActiveRecord::Migration[6.1] + def change + execute "UPDATE spree_payment_methods SET type = 'Spree::Gateway::StripeSCA' WHERE type = 'Spree::Gateway::StripeConnect'" + end +end diff --git a/db/schema.rb b/db/schema.rb index dc58145fb3..3f4ac65e4a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_06_17_203927) do +ActiveRecord::Schema.define(version: 2021_08_17_100257) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" From 98a879a0e9de7152067f47ca2aa94db76dcc3abd Mon Sep 17 00:00:00 2001 From: Nihal Date: Wed, 25 Aug 2021 14:01:58 +0530 Subject: [PATCH 12/12] Restore removed spec to profile_storer_spec --- .../orders/payments/payments_controller_spec.rb | 11 ++++++----- spec/jobs/subscription_confirm_job_spec.rb | 12 +++++++++--- spec/lib/stripe/profile_storer_spec.rb | 13 +++++++++++++ 3 files changed, 28 insertions(+), 8 deletions(-) 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 9b4610069d..546d67ec86 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb @@ -45,13 +45,14 @@ describe Spree::Admin::PaymentsController, type: :controller do end context "with Stripe payment where payment.process! errors out" do - let!(:payment_method) { create(:stripe_sca_payment_method, distributors: [shop]) } before do - allow_any_instance_of(Spree::Payment). - to receive(:process_offline!). - and_raise(Spree::Core::GatewayError.new("Payment Gateway Error")) + allow_any_instance_of(Spree::Payment).to receive(:authorize!) do |payment| + payment.update state: "pending" + end + allow_any_instance_of(Spree::Payment).to receive(:process_offline!). + and_raise(Spree::Core::GatewayError.new("Payment Gateway Error")) end - + it "redirects to new payment page with flash error" do spree_post :create, payment: params, order_id: order.number diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index ca93fc0f23..1bd129e603 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -191,20 +191,26 @@ describe SubscriptionConfirmJob do end end - context "Stripe Connect" do + context "Stripe SCA" do let(:stripe_sca_payment_method) { create(:stripe_sca_payment_method) } let(:stripe_sca_payment) { create(:payment, amount: 10, payment_method: stripe_sca_payment_method) } + let(:provider) { double } before do + allow_any_instance_of(Stripe::CreditCardCloner).to receive(:find_or_clone) { + ["cus_123", "pm_1234"] + } allow(order).to receive(:pending_payments) { [stripe_sca_payment] } - allow(stripe_sca_payment_method).to receive(:purchase) { true } + allow(stripe_sca_payment_method).to receive(:provider) { provider } + allow(stripe_sca_payment_method.provider).to receive(:purchase) { true } + allow(stripe_sca_payment_method.provider).to receive(:capture) { true } end it "runs the charges in offline mode" do job.send(:confirm_order!, order) - expect(stripe_sca_payment_method).to have_received(:purchase) + expect(stripe_sca_payment_method.provider).to have_received(:purchase) end end end diff --git a/spec/lib/stripe/profile_storer_spec.rb b/spec/lib/stripe/profile_storer_spec.rb index e7b69f8c8c..1836517bc1 100644 --- a/spec/lib/stripe/profile_storer_spec.rb +++ b/spec/lib/stripe/profile_storer_spec.rb @@ -25,6 +25,19 @@ module Stripe stub_payment_method_attach_request(payment_method: card_id, customer: customer_id) end + context "when called from Stripe SCA" do + let(:customer_response_body) { + JSON.generate(id: customer_id, default_card: card_id, sources: { data: [{ id: "1" }] }) + } + + it "fetches the customer id and the card id from the correct response fields" do + profile_storer.create_customer_from_token + + expect(payment.source.gateway_customer_profile_id).to eq customer_id + expect(payment.source.gateway_payment_profile_id).to eq card_id + end + end + context "when called from Stripe SCA" do it "fetches the customer id and the card id from the correct response fields" do profile_storer.create_customer_from_token