From 9abacf9047208ffe7f0b0518b189cc37db45f040 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 20 Jan 2022 17:04:26 +1100 Subject: [PATCH 1/5] Upgrade old StripeConnect payment methods to SCA StripeConnect has been replaced by StripeSCA. But we still have some StripeConnect payment methods in our production databases. We need to convert them to StripeSCA methods before we can remove the related code and clean up. Otherwise our code would raise errors when encountering an old StripeConnect method, not knowing what that is. --- ...07_convert_stripe_connect_to_stripe_sca.rb | 57 +++++++++++ db/schema.rb | 2 +- ...nvert_stripe_connect_to_stripe_sca_spec.rb | 95 +++++++++++++++++++ 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220118053107_convert_stripe_connect_to_stripe_sca.rb create mode 100644 spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb diff --git a/db/migrate/20220118053107_convert_stripe_connect_to_stripe_sca.rb b/db/migrate/20220118053107_convert_stripe_connect_to_stripe_sca.rb new file mode 100644 index 0000000000..5d57da7e9f --- /dev/null +++ b/db/migrate/20220118053107_convert_stripe_connect_to_stripe_sca.rb @@ -0,0 +1,57 @@ +class ConvertStripeConnectToStripeSca < ActiveRecord::Migration[6.1] + class SpreePreference < ActiveRecord::Base + scope :leftover_from_payment_type, ->(class_name) { + joins <<~SQL + JOIN spree_payment_methods + ON spree_preferences.key = + CONCAT('/#{class_name.underscore}/enterprise_id/', spree_payment_methods.id) + AND spree_payment_methods.type != '#{class_name}' + SQL + } + end + + def up + delete_outdated_spree_preferences + upgrade_stripe_payment_methods + update_payment_method_preferences + end + + private + + # When changing the type of a payment method, we leave orphaned records in + # the spree_preferences table. The key of the preference contains the type + # of the payment method and therefore changing the type disconnects the + # preference. + # + # Here we delete orphaned preferences first so that we don't have any + # conflicts later when updating preferences alongside the connected + # payment methods. + def delete_outdated_spree_preferences + outdated_keys = + SpreePreference.leftover_from_payment_type("Spree::Gateway::StripeConnect").pluck(:key) + + SpreePreference.leftover_from_payment_type("Spree::Gateway::StripeSCA").pluck(:key) + + SpreePreference.where(key: outdated_keys).delete_all + + # Spree preferences are cached and we want to avoid reading old values. + # Danger: The cache may alter the given array in place. Make sure to + # not use the `outdated_keys` variable after this call. + Rails.cache.delete_multi(outdated_keys) + end + + def upgrade_stripe_payment_methods + execute <<~SQL + UPDATE spree_payment_methods + SET type = 'Spree::Gateway::StripeSCA' + WHERE type = 'Spree::Gateway::StripeConnect' + SQL + end + + def update_payment_method_preferences + execute <<~SQL + UPDATE spree_preferences + SET key = replace(key, 'stripe_connect', 'stripe_sca') + WHERE key LIKE '/spree/gateway/stripe_connect/%' + SQL + end +end diff --git a/db/schema.rb b/db/schema.rb index d3ec898e90..b1c9499ddb 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: 2022_01_14_110920) do +ActiveRecord::Schema.define(version: 2022_01_18_053107) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb b/spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb new file mode 100644 index 0000000000..6a37b1b4a5 --- /dev/null +++ b/spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../db/migrate/20220118053107_convert_stripe_connect_to_stripe_sca' + +describe ConvertStripeConnectToStripeSca do + let(:owner) { create(:distributor_enterprise) } + let(:new_owner) { create(:distributor_enterprise) } + let(:old_stripe_connect) { + Spree::Gateway::StripeConnect.create!( + name: "Stripe", + environment: "test", + preferred_enterprise_id: owner.id, + distributor_ids: [owner.id] + ) + } + let(:result) { Spree::PaymentMethod.find(old_stripe_connect.id) } + + before do + # Activate the cache because it's deactivated in test environment. + allow(Spree::Preferences::Store.instance).to receive(:should_persist?).and_return(true) + + # Create the payment method after cache activation to store the owner. + old_stripe_connect + end + + it "converts payment methods" do + subject.up + + expect(result.class).to eq Spree::Gateway::StripeSCA + end + + it "keeps attributes" do + subject.up + + expect(result.name).to eq "Stripe" + expect(result.environment).to eq "test" + expect(result.distributor_ids).to eq [owner.id] + end + + it "keeps Spree preferences" do + subject.up + + expect(result.preferred_enterprise_id).to eq owner.id + end + + it "doesn't move outdated StripeConnect preferences to StripeSCA methods" do + # When you change the type of a payment method in the admin screen + # it leaves old entries in the spree_preferences table. + # Here is a simulation of such a change: + old_stripe_connect.update_columns(type: "Spree::Gateway::StripeSCA") + changed_method = Spree::PaymentMethod.find(old_stripe_connect.id) + changed_method.preferred_enterprise_id = new_owner.id + + subject.up + + expect(result.preferred_enterprise_id).to eq new_owner.id + end + + it "keeps Spree preferences despite conflicting preference keys" do + # We change the payment method to StripeSCA and then back to StripeConnect + # to generate a conflicting preference. We want to keep the preference + # of the current payment method, not the intermediately changed one. + old_stripe_connect.update_columns(type: "Spree::Gateway::StripeSCA") + changed_method = Spree::PaymentMethod.find(old_stripe_connect.id) + changed_method.preferred_enterprise_id = owner.id + old_stripe_connect.update_columns(type: "Spree::Gateway::StripeConnect") + old_stripe_connect.preferred_enterprise_id = new_owner.id + + subject.up + + expect(result.preferred_enterprise_id).to eq new_owner.id + end + + it "doesn't mess with new Stripe payment methods" do + stripe = Spree::Gateway::StripeSCA.create!( + name: "Modern Stripe", + environment: "test", + preferred_enterprise_id: owner.id, + distributor_ids: [owner.id] + ) + + expect { subject.up }.to_not change { stripe.reload.attributes } + end + + it "doesn't mess with other payment methods" do + cash = Spree::PaymentMethod::Check.create!( + name: "Cash on delivery", + environment: "test", + distributor_ids: [owner.id] + ) + + expect { subject.up }.to_not change { cash.reload.attributes } + end +end From 191646b611f2e84b8e725f1705661ec0957cac9e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 24 Jan 2022 10:56:27 +1100 Subject: [PATCH 2/5] Update Stripe config description --- .env | 4 ++-- app/models/spree/app_configuration.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.env b/.env index 2d0c0571c2..5b1523fcba 100644 --- a/.env +++ b/.env @@ -53,8 +53,8 @@ SMTP_PASSWORD="f00d" # see="https://developers.google.com/maps/documentation/javascript/get-api-key # GOOGLE_MAPS_API_KEY="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -# Stripe Connect details for instance account -# Find these under 'API keys' and 'Connect' in your Stripe account dashboard -> Account Settings +# Stripe details for instance account +# Find these under 'Developers' -> 'API keys' in your Stripe account dashboard. # Under 'Connect', the Redirect URI should be set to https://YOUR_SERVER_URL/stripe/callbacks (e.g. https://openfoodnetwork.org.uk/stripe/callbacks) # Under 'Webhooks', you should set up a Connect endpoint pointing to https://YOUR_SERVER_URL/stripe/webhooks e.g. (https://openfoodnetwork.org.uk/stripe/webhooks) # STRIPE_INSTANCE_SECRET_KEY="sk_test_xxxxxx" # This can be a test key or a live key diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index ae15100696..dccce80ce3 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -130,7 +130,7 @@ module Spree preference :invoice_style2?, :boolean, default: false preference :enable_receipt_printing?, :boolean, default: false - # Stripe Connect + # Stripe payments preference :stripe_connect_enabled, :boolean, default: false # Number localization From f22c2b0e73adb3e608a6579c0188f4462216e116 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 24 Jan 2022 11:17:23 +1100 Subject: [PATCH 3/5] Remove option to create StripeConnect methods --- .../spree/admin/payment_methods_controller.rb | 13 +++---------- config/application.rb | 1 - .../spree/admin/payment_methods_controller_spec.rb | 12 +++++------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 1a682d58f5..e467e8356c 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -132,12 +132,6 @@ module Spree providers.reject! { |provider| stripe_provider?(provider) } end - # This method is deprecated and will be removed soon: - unless @payment_method&.type == "Spree::Gateway::StripeConnect" || - OpenFoodNetwork::FeatureToggle.enabled?("StripeConnect") - providers.reject! { |provider| provider.name.ends_with?("StripeConnect") } - end - providers end @@ -160,12 +154,11 @@ module Spree end def stripe_payment_method? - ["Spree::Gateway::StripeConnect", - "Spree::Gateway::StripeSCA"].include? @payment_method.try(:type) + @payment_method.try(:type) == "Spree::Gateway::StripeSCA" end def stripe_provider?(provider) - provider.name.ends_with?("StripeConnect", "StripeSCA") + provider.name.ends_with?("StripeSCA") end def base_params @@ -177,7 +170,7 @@ module Spree raw_params[ActiveModel::Naming.param_key(@payment_method)] || {} end - # Merge payment method params with gateway params like :gateway_stripe_connect + # Merge payment method params with gateway params like :gateway_stripe_sca # Also, remove password if present and blank def update_params @update_params ||= begin diff --git a/config/application.rb b/config/application.rb index c90188c8ed..bc6697b9dd 100644 --- a/config/application.rb +++ b/config/application.rb @@ -146,7 +146,6 @@ module Openfoodnetwork # Register Spree payment methods initializer "spree.gateway.payment_methods", :after => "spree.register.payment_methods" do |app| Rails.application.reloader.to_prepare do - 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/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/spec/controllers/spree/admin/payment_methods_controller_spec.rb index 837f175469..d402e0d981 100644 --- a/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -34,14 +34,13 @@ module Spree providers = assigns(:providers).map(&:to_s) expect(providers).to include "Spree::Gateway::StripeSCA" - expect(providers).to_not include "Spree::Gateway::StripeConnect" end end describe "#edit" do - let(:deprecated_stripe) { + let(:stripe) { create( - :stripe_connect_payment_method, + :stripe_sca_payment_method, distributor_ids: [enterprise_id], preferred_enterprise_id: enterprise_id ) @@ -50,14 +49,13 @@ module Spree before { allow(controller).to receive(:spree_current_user) { user } } - it "shows the current gateway type even if deprecated" do - allow(Spree::Config).to receive(:stripe_connect_enabled).and_return(true) + it "shows the current gateway type even if not enabled" do + allow(Spree::Config).to receive(:stripe_connect_enabled).and_return(false) - spree_get :edit, id: deprecated_stripe.id + spree_get :edit, id: stripe.id providers = assigns(:providers).map(&:to_s) expect(providers).to include "Spree::Gateway::StripeSCA" - expect(providers).to include "Spree::Gateway::StripeConnect" end end From ee85b9654d9d10f6971a362b75c59276350f33a3 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 24 Jan 2022 11:43:35 +1100 Subject: [PATCH 4/5] Remove references to StripeConnect class --- .../controllers/details_controller.js.coffee | 2 +- .../api/admin/payment_method_serializer.rb | 3 +- .../_provider_settings.html.haml | 2 - .../subscriptions/stripe_payment_setup.rb | 3 +- .../subscriptions/validator.rb | 3 +- .../payments_controller_refunds_spec.rb | 120 ------- spec/factories/payment_method_factory.rb | 7 - spec/models/spree/payment_method_spec.rb | 1 - spec/requests/checkout/stripe_connect_spec.rb | 310 ------------------ 9 files changed, 4 insertions(+), 447 deletions(-) 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..dea96a800c 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/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/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..df79b45ddd 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 + @payment.payment_method.type == "Spree::Gateway::StripeSCA" 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/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb index 62400b8392..8c995de106 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, :completed, order: order, 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, :completed, order: order, 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 } } 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/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")) diff --git a/spec/requests/checkout/stripe_connect_spec.rb b/spec/requests/checkout/stripe_connect_spec.rb deleted file mode 100644 index 6b47efdb59..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, order_token: order.token) - 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, order_token: order.token) - 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, order_token: order.token) - 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 19909deaf6384658c59c19f5e5f05e8ea390eee6 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 24 Jan 2022 11:53:22 +1100 Subject: [PATCH 5/5] Remove obsolete StripeConnect payment gateway The newer StripeSCA is a complete replacement. --- app/models/spree/gateway/stripe_connect.rb | 109 ------------------ ...nvert_stripe_connect_to_stripe_sca_spec.rb | 9 ++ .../spree/gateway/stripe_connect_spec.rb | 107 ----------------- 3 files changed, 9 insertions(+), 216 deletions(-) delete mode 100644 app/models/spree/gateway/stripe_connect.rb delete mode 100644 spec/models/spree/gateway/stripe_connect_spec.rb 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/spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb b/spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb index 6a37b1b4a5..a6cd7c244e 100644 --- a/spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb +++ b/spec/migrations/convert_stripe_connect_to_stripe_sca_spec.rb @@ -3,6 +3,15 @@ require 'spec_helper' require_relative '../../db/migrate/20220118053107_convert_stripe_connect_to_stripe_sca' +module Spree + class Gateway + class StripeConnect < Gateway::StripeSCA + # This class got deleted from the code base but this minimum definition + # is enough for this test. + end + end +end + describe ConvertStripeConnectToStripeSca do let(:owner) { create(:distributor_enterprise) } let(:new_owner) { create(:distributor_enterprise) } 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