From b517d2f0c769dfb40cabc2b0ffdd92bde656e517 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Tue, 27 Oct 2020 14:39:20 -0700
Subject: [PATCH 01/25] guard against a stripe customer already being deleted

---
 app/controllers/spree/credit_cards_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb
index bd623d4154..1c3f6b0aa6 100644
--- a/app/controllers/spree/credit_cards_controller.rb
+++ b/app/controllers/spree/credit_cards_controller.rb
@@ -65,7 +65,7 @@ module Spree
     def destroy_at_stripe
       stripe_customer = Stripe::Customer.retrieve(@credit_card.gateway_customer_profile_id, {})
 
-      stripe_customer&.delete
+      stripe_customer&.delete unless stripe_customer.deleted?
     end
 
     def stripe_account_id

From 215d1bbe2034a1fdc350a851c42f61df64fc82a7 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 29 Oct 2020 15:30:32 -0700
Subject: [PATCH 02/25] create js setup intent when authing shop

---
 .../darkswarm/services/credit_cards.js.coffee     |  3 +--
 .../darkswarm/services/customer.js.coffee         | 14 ++++++++++++--
 .../darkswarm/services/customers.js.coffee        |  4 ++++
 app/controllers/api/customers_controller.rb       |  3 +++
 app/models/customer.rb                            |  2 ++
 app/serializers/api/customer_serializer.rb        |  3 ++-
 app/services/recurring_payments.rb                | 15 +++++++++++++++
 7 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 app/services/recurring_payments.rb

diff --git a/app/assets/javascripts/darkswarm/services/credit_cards.js.coffee b/app/assets/javascripts/darkswarm/services/credit_cards.js.coffee
index 9ca8c37b3d..b7f1e34324 100644
--- a/app/assets/javascripts/darkswarm/services/credit_cards.js.coffee
+++ b/app/assets/javascripts/darkswarm/services/credit_cards.js.coffee
@@ -11,8 +11,7 @@ Darkswarm.factory 'CreditCards', ($http, $filter, savedCreditCards, Messages, Cu
         othercard.is_default = false
       $http.put("/credit_cards/#{card.id}", is_default: true).then (data) ->
         Messages.success(t('js.default_card_updated'))
-        for customer in Customers.index()
-          customer.allow_charges = false
+        Customers.clearAllAllowCharges()
       , (response) ->
         Messages.flash(response.data.flash)
 
diff --git a/app/assets/javascripts/darkswarm/services/customer.js.coffee b/app/assets/javascripts/darkswarm/services/customer.js.coffee
index d9e0804d8b..8fa25c8ff4 100644
--- a/app/assets/javascripts/darkswarm/services/customer.js.coffee
+++ b/app/assets/javascripts/darkswarm/services/customer.js.coffee
@@ -1,4 +1,4 @@
-angular.module("Darkswarm").factory 'Customer', ($resource, Messages) ->
+angular.module("Darkswarm").factory 'Customer', ($resource, $injector, Messages) ->
   Customer = $resource('/api/customers/:id/:action.json', {}, {
     'index':
       method: 'GET'
@@ -13,7 +13,17 @@ angular.module("Darkswarm").factory 'Customer', ($resource, Messages) ->
 
   Customer.prototype.update = ->
     @$update().then (response) =>
-      Messages.success(t('js.changes_saved'))
+      if response.gateway_recurring_payment_client_secret && $injector.has('stripeObject')
+        stripe = $injector.get('stripeObject')
+        stripe.confirmCardSetup(response.gateway_recurring_payment_client_secret).then (result) =>
+          if result.error
+            @allow_charges = false
+            @$update(allow_charges: false)
+            Messages.error(result.error.message)
+          else
+            Messages.success(t('js.changes_saved'))
+      else
+        Messages.success(t('js.changes_saved'))
     , (response) =>
       Messages.error(response.data.error)
 
diff --git a/app/assets/javascripts/darkswarm/services/customers.js.coffee b/app/assets/javascripts/darkswarm/services/customers.js.coffee
index fe2c862c37..bdb247ce12 100644
--- a/app/assets/javascripts/darkswarm/services/customers.js.coffee
+++ b/app/assets/javascripts/darkswarm/services/customers.js.coffee
@@ -12,3 +12,7 @@ angular.module("Darkswarm").factory 'Customers', (Customer) ->
       for customer in customers
         @all.push customer
         @byID[customer.id] = customer
+
+    clearAllAllowCharges: () ->
+      for customer in @index()
+        customer.allow_charges = false
diff --git a/app/controllers/api/customers_controller.rb b/app/controllers/api/customers_controller.rb
index 796032e763..3ea1c3cbd5 100644
--- a/app/controllers/api/customers_controller.rb
+++ b/app/controllers/api/customers_controller.rb
@@ -11,7 +11,10 @@ module Api
       @customer = Customer.find(params[:id])
       authorize! :update, @customer
 
+      client_secret = RecurringPayments.setup_for(@customer) if params[:customer][:allow_charges]
+
       if @customer.update(customer_params)
+        @customer.gateway_recurring_payment_client_secret = client_secret if client_secret
         render json: @customer, serializer: CustomerSerializer, status: :ok
       else
         invalid_resource!(@customer)
diff --git a/app/models/customer.rb b/app/models/customer.rb
index 8328aba831..2b657006e0 100644
--- a/app/models/customer.rb
+++ b/app/models/customer.rb
@@ -25,6 +25,8 @@ class Customer < ActiveRecord::Base
 
   before_create :associate_user
 
+  attr_accessor :gateway_recurring_payment_client_secret
+
   private
 
   def downcase_email
diff --git a/app/serializers/api/customer_serializer.rb b/app/serializers/api/customer_serializer.rb
index 44914e0a49..87fd58f140 100644
--- a/app/serializers/api/customer_serializer.rb
+++ b/app/serializers/api/customer_serializer.rb
@@ -1,5 +1,6 @@
 module Api
   class CustomerSerializer < ActiveModel::Serializer
-    attributes :id, :enterprise_id, :name, :code, :email, :allow_charges
+    attributes :id, :enterprise_id, :name, :code, :email, :allow_charges,
+               :gateway_recurring_payment_client_secret
   end
 end
diff --git a/app/services/recurring_payments.rb b/app/services/recurring_payments.rb
new file mode 100644
index 0000000000..4c352f57b5
--- /dev/null
+++ b/app/services/recurring_payments.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class RecurringPayments
+  def self.setup_for(customer)
+    return unless card = customer.user.default_card
+    return unless payment_method = card.gateway_payment_profile_id
+    return unless customer_profile_id = card.gateway_customer_profile_id
+
+    stripe_account = customer.enterprise.stripe_account&.stripe_user_id
+    setup_intent = Stripe::SetupIntent.create(
+      payment_method: payment_method, customer: customer_profile_id, on_behalf_of: stripe_account
+    )
+    setup_intent.client_secret
+  end
+end

From cad8a018f952d148c556b67861432d765fedc617 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Tue, 17 Nov 2020 16:55:46 -0800
Subject: [PATCH 03/25] put SetupIntent on the connected Stripe account

---
 .../javascripts/darkswarm/services/customer.js.coffee |  7 +++++--
 app/controllers/api/customers_controller.rb           |  5 +++--
 app/models/customer.rb                                |  1 +
 app/serializers/api/customer_serializer.rb            |  2 +-
 app/services/recurring_payments.rb                    | 11 +++++++----
 app/views/spree/users/show.html.haml                  |  1 +
 config/locales/en.yml                                 |  1 +
 7 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/darkswarm/services/customer.js.coffee b/app/assets/javascripts/darkswarm/services/customer.js.coffee
index 8fa25c8ff4..fa3b1b532f 100644
--- a/app/assets/javascripts/darkswarm/services/customer.js.coffee
+++ b/app/assets/javascripts/darkswarm/services/customer.js.coffee
@@ -12,9 +12,12 @@ angular.module("Darkswarm").factory 'Customer', ($resource, $injector, Messages)
   })
 
   Customer.prototype.update = ->
+    if @allow_charges
+      Messages.loading(t('js.authorising'))
     @$update().then (response) =>
-      if response.gateway_recurring_payment_client_secret && $injector.has('stripeObject')
-        stripe = $injector.get('stripeObject')
+      if response.gateway_recurring_payment_client_secret && $injector.has('stripePublishableKey')
+        Messages.clear()
+        stripe = Stripe($injector.get('stripePublishableKey'), { stripeAccount: response.gateway_shop_id })
         stripe.confirmCardSetup(response.gateway_recurring_payment_client_secret).then (result) =>
           if result.error
             @allow_charges = false
diff --git a/app/controllers/api/customers_controller.rb b/app/controllers/api/customers_controller.rb
index 3ea1c3cbd5..e0610e5e32 100644
--- a/app/controllers/api/customers_controller.rb
+++ b/app/controllers/api/customers_controller.rb
@@ -13,8 +13,9 @@ module Api
 
       client_secret = RecurringPayments.setup_for(@customer) if params[:customer][:allow_charges]
 
-      if @customer.update(customer_params)
-        @customer.gateway_recurring_payment_client_secret = client_secret if client_secret
+      if @customer.update(params[:customer])
+        @customer.gateway_recurring_payment_client_secret = client_secret
+        @customer.gateway_shop_id = @customer.enterprise.stripe_account&.stripe_user_id
         render json: @customer, serializer: CustomerSerializer, status: :ok
       else
         invalid_resource!(@customer)
diff --git a/app/models/customer.rb b/app/models/customer.rb
index 2b657006e0..256fc1b61c 100644
--- a/app/models/customer.rb
+++ b/app/models/customer.rb
@@ -26,6 +26,7 @@ class Customer < ActiveRecord::Base
   before_create :associate_user
 
   attr_accessor :gateway_recurring_payment_client_secret
+  attr_accessor :gateway_shop_id
 
   private
 
diff --git a/app/serializers/api/customer_serializer.rb b/app/serializers/api/customer_serializer.rb
index 87fd58f140..ee543e5031 100644
--- a/app/serializers/api/customer_serializer.rb
+++ b/app/serializers/api/customer_serializer.rb
@@ -1,6 +1,6 @@
 module Api
   class CustomerSerializer < ActiveModel::Serializer
     attributes :id, :enterprise_id, :name, :code, :email, :allow_charges,
-               :gateway_recurring_payment_client_secret
+               :gateway_recurring_payment_client_secret, :gateway_shop_id
   end
 end
diff --git a/app/services/recurring_payments.rb b/app/services/recurring_payments.rb
index 4c352f57b5..8058ee6674 100644
--- a/app/services/recurring_payments.rb
+++ b/app/services/recurring_payments.rb
@@ -3,12 +3,15 @@
 class RecurringPayments
   def self.setup_for(customer)
     return unless card = customer.user.default_card
-    return unless payment_method = card.gateway_payment_profile_id
-    return unless customer_profile_id = card.gateway_customer_profile_id
 
     stripe_account = customer.enterprise.stripe_account&.stripe_user_id
-    setup_intent = Stripe::SetupIntent.create(
-      payment_method: payment_method, customer: customer_profile_id, on_behalf_of: stripe_account
+
+    customer_id, payment_method_id = Stripe::CreditCardCloner.new.clone(card,
+                                                                        stripe_account)
+    setup_intent = Stripe::SetupIntent.create({
+      payment_method: payment_method_id, customer: customer_id }, {
+        stripe_account: stripe_account
+      }
     )
     setup_intent.client_secret
   end
diff --git a/app/views/spree/users/show.html.haml b/app/views/spree/users/show.html.haml
index 9b18322f86..bddce94ce9 100644
--- a/app/views/spree/users/show.html.haml
+++ b/app/views/spree/users/show.html.haml
@@ -5,6 +5,7 @@
   - if Stripe.publishable_key
     :javascript
       angular.module('Darkswarm').value("stripeObject", Stripe("#{Stripe.publishable_key}"))
+      angular.module('Darkswarm').value("stripePublishableKey", "#{Stripe.publishable_key}")
 
 .darkswarm
   .row.pad-top
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 8b5a184821..e4471bdefc 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2510,6 +2510,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
   js:
     saving: 'Saving...'
     changes_saved: 'Changes saved.'
+    authorising: "Authorising..."
     save_changes_first: Save changes first.
     all_changes_saved: All changes saved
     unsaved_changes: You have unsaved changes

From 13ab25ac45ca0f65eedeca30e5e40e62c78d81db Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Wed, 18 Nov 2020 13:38:14 -0800
Subject: [PATCH 04/25] separate method for charging offline

---
 app/jobs/subscription_confirm_job.rb   |  2 +-
 app/models/spree/gateway/stripe_sca.rb | 13 +++++++++++++
 app/models/spree/order.rb              |  4 ++--
 app/models/spree/payment/processing.rb | 11 +++++++++--
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index acf241e4e0..f3d16d2ea9 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -62,7 +62,7 @@ class SubscriptionConfirmJob
     return unless order.payment_required?
 
     prepare_for_payment!(order)
-    order.process_payments!
+    order.process_payments!(true)
     raise if order.errors.any?
   end
 
diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb
index 02681ae210..ade64f157b 100644
--- a/app/models/spree/gateway/stripe_sca.rb
+++ b/app/models/spree/gateway/stripe_sca.rb
@@ -45,6 +45,19 @@ module Spree
         failed_activemerchant_billing_response(e.message)
       end
 
+      # NOTE: the name of this method is determined by Spree::Payment::Processing
+      def charge_offline(money, creditcard, gateway_options)
+        options = basic_options(gateway_options)
+        customer_id, payment_method_id = Stripe::CreditCardCloner.new.clone(creditcard,
+                                                                            stripe_account_id)
+
+        options[:customer] = customer_id
+        options[:off_session] = true
+        provider.purchase(money, payment_method_id, options)
+      rescue Stripe::StripeError => e
+        failed_activemerchant_billing_response(e.message)
+      end
+
       # NOTE: the name of this method is determined by Spree::Payment::Processing
       def authorize(money, creditcard, gateway_options)
         authorize_response = provider.authorize(*options_for_authorize(money,
diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 029f5bb602..817b55d319 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -499,13 +499,13 @@ module Spree
     #   which gets rescued and converted to FALSE when
     #   :allow_checkout_on_gateway_error is set to false
     #
-    def process_payments!
+    def process_payments!(offline = false)
       raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
 
       pending_payments.each do |payment|
         break if payment_total >= total
 
-        payment.process!
+        payment.process!(offline)
 
         if payment.completed?
           self.payment_total += payment.amount
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index c5a9979ee3..de0c05d422 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -3,7 +3,7 @@
 module Spree
   class Payment < ActiveRecord::Base
     module Processing
-      def process!
+      def process!(offline = false)
         return unless payment_method&.source_required?
 
         raise Core::GatewayError, Spree.t(:payment_processing_failed) unless source
@@ -15,7 +15,9 @@ module Spree
           raise Core::GatewayError, Spree.t(:payment_method_not_supported)
         end
 
-        if payment_method.auto_capture?
+        if offline
+          charge_offline!
+        elsif payment_method.auto_capture?
           purchase!
         else
           authorize!
@@ -32,6 +34,11 @@ module Spree
         gateway_action(source, :purchase, :complete)
       end
 
+      def charge_offline!
+        started_processing!
+        gateway_action(source, :charge_offline, :complete)
+      end
+
       def capture!
         return true if completed?
 

From fea7576ac19201e3fed0e09746ef25598e512a77 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 19 Nov 2020 11:45:08 -0800
Subject: [PATCH 05/25] update CreditCardCloner to find existing clone

---
 app/models/spree/gateway/stripe_sca.rb |  8 +--
 app/services/recurring_payments.rb     | 14 ++--
 lib/stripe/credit_card_cloner.rb       | 91 ++++++++++++++++++++++----
 3 files changed, 90 insertions(+), 23 deletions(-)

diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb
index ade64f157b..a6a25aced4 100644
--- a/app/models/spree/gateway/stripe_sca.rb
+++ b/app/models/spree/gateway/stripe_sca.rb
@@ -48,8 +48,8 @@ module Spree
       # NOTE: the name of this method is determined by Spree::Payment::Processing
       def charge_offline(money, creditcard, gateway_options)
         options = basic_options(gateway_options)
-        customer_id, payment_method_id = Stripe::CreditCardCloner.new.clone(creditcard,
-                                                                            stripe_account_id)
+        customer_id, payment_method_id =
+          Stripe::CreditCardCloner.new.find_or_clone(creditcard, stripe_account_id)
 
         options[:customer] = customer_id
         options[:off_session] = true
@@ -110,8 +110,8 @@ module Spree
         options = basic_options(gateway_options)
         options[:return_url] = full_checkout_path
 
-        customer_id, payment_method_id = Stripe::CreditCardCloner.new.clone(creditcard,
-                                                                            stripe_account_id)
+        customer_id, payment_method_id =
+          Stripe::CreditCardCloner.new.find_or_clone(creditcard, stripe_account_id)
         options[:customer] = customer_id
         [money, payment_method_id, options]
       end
diff --git a/app/services/recurring_payments.rb b/app/services/recurring_payments.rb
index 8058ee6674..1c622fd6a8 100644
--- a/app/services/recurring_payments.rb
+++ b/app/services/recurring_payments.rb
@@ -3,15 +3,13 @@
 class RecurringPayments
   def self.setup_for(customer)
     return unless card = customer.user.default_card
+    return unless stripe_account = customer.enterprise.stripe_account&.stripe_user_id
 
-    stripe_account = customer.enterprise.stripe_account&.stripe_user_id
-
-    customer_id, payment_method_id = Stripe::CreditCardCloner.new.clone(card,
-                                                                        stripe_account)
-    setup_intent = Stripe::SetupIntent.create({
-      payment_method: payment_method_id, customer: customer_id }, {
-        stripe_account: stripe_account
-      }
+    customer_id, payment_method_id =
+      Stripe::CreditCardCloner.new.find_or_clone(card, stripe_account)
+    setup_intent = Stripe::SetupIntent.create(
+      { payment_method: payment_method_id, customer: customer_id },
+      { stripe_account: stripe_account }
     )
     setup_intent.client_secret
   end
diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index 64b67b5345..fb5e2d3d98 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -1,27 +1,37 @@
 # frozen_string_literal: true
 
-# Here we clone
-#   - a card (card_*) or payment_method (pm_*) stored (in a customer) in a platform account
-# into
+# Here we clone (or find a clone of)
+#   - a card (card_*) or payment_method (pm_*) stored (in a customer) in a platform account into
 #   - a payment method (pm_*) (in a new customer) in a connected account
 #
 # This is required when using the Stripe Payment Intents API:
 #   - the customer and payment methods are stored in the platform account
 #       so that they can be re-used across multiple sellers
-#   - when a card needs to be charged, we need to create it in the seller's stripe account
+#   - when a card needs to be charged, we need to clone (or find the clone)
+#       in the seller's stripe account
 #
-# We are doing this process every time the card is charged:
-#   - this means that, if the customer uses the same card on the same seller multiple times,
-#       the card will be created multiple times on the seller's account
-#   - to avoid this, we would have to store the IDs of every card on each seller's stripe account
-#       in our database (this way we only have to store the platform account ID)
+# To avoid creating a new clone of the card/customer each time the card is charged or
+# authorized (e.g. for SCA), we attach metadata { clone: true } to the card the first time we
+# clone it and look for a card with the same fingerprint (hash of the card number) and
+# that metadata key to avoid cloning it multiple times.
+
 module Stripe
   class CreditCardCloner
+    def find_or_clone(credit_card, connected_account_id)
+      if card = find_cloned_card(credit_card, connected_account_id)
+        card
+      else
+        clone(credit_card, connected_account_id)
+      end
+    end
+
+    private
+
     def clone(credit_card, connected_account_id)
       new_payment_method = clone_payment_method(credit_card, connected_account_id)
 
       # If no customer is given, it will clone the payment method only
-      return nil, new_payment_method.id if credit_card.gateway_customer_profile_id.blank?
+      return [nil, new_payment_method.id] if credit_card.gateway_customer_profile_id.blank?
 
       new_customer = Stripe::Customer.create({ email: credit_card.user.email },
                                              stripe_account: connected_account_id)
@@ -29,10 +39,63 @@ module Stripe
                                         new_customer.id,
                                         connected_account_id)
 
+      add_metadata_to_payment_method(new_payment_method.id, connected_account_id)
+
       [new_customer.id, new_payment_method.id]
     end
 
-    private
+    def find_cloned_card(card, connected_account_id)
+      matches = []
+      return matches unless fingerprint = fingerprint_for_card(card)
+
+      find_customers(card.user.email, connected_account_id).each do |customer|
+        find_payment_methods(customer.id, connected_account_id).each do |payment_method|
+          if payment_method_is_clone?(payment_method, fingerprint)
+            matches << [customer.id, payment_method.id]
+          end
+        end
+      end
+
+      matches.first
+    end
+
+    def payment_method_is_clone?(payment_method, fingerprint)
+      payment_method.card.fingerprint == fingerprint && payment_method.metadata["ofn-clone"]
+    end
+
+    def fingerprint_for_card(card)
+      Stripe::PaymentMethod.retrieve(card.gateway_payment_profile_id).card.fingerprint
+    end
+
+    def find_customers(email, connected_account_id)
+      starting_after = nil
+      customers = []
+
+      loop do
+        response = Stripe::Customer.list({ email: email, starting_after: starting_after },
+                                         { stripe_account: connected_account_id })
+        customers += response.data
+        break unless response.has_more
+
+        starting_after = response.data.last.id
+      end
+      customers
+    end
+
+    def find_payment_methods(customer_id, connected_account_id)
+      starting_after = nil
+      payment_methods = []
+
+      loop do
+        options = { customer: customer_id, type: 'card', starting_after: starting_after }
+        response = Stripe::PaymentMethod.list(options, { stripe_account: connected_account_id })
+        payment_methods += response.data
+        break unless response.has_more
+
+        starting_after = response.data.last.id
+      end
+      payment_methods
+    end
 
     def clone_payment_method(credit_card, connected_account_id)
       platform_acct_payment_method_id = credit_card.gateway_payment_profile_id
@@ -48,5 +111,11 @@ module Stripe
                                    { customer: customer_id },
                                    stripe_account: connected_account_id)
     end
+
+    def add_metadata_to_payment_method(payment_method_id, connected_account_id)
+      Stripe::PaymentMethod.update(payment_method_id,
+                                   { metadata: { "ofn-clone": true } },
+                                   stripe_account: connected_account_id)
+    end
   end
 end

From 3d47ad7e3337b67d741292db382411644b3bf6ff Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 19 Nov 2020 12:51:00 -0800
Subject: [PATCH 06/25] add stubs for stripe requests

---
 spec/features/admin/payments_stripe_spec.rb   |  3 +
 spec/features/consumer/account/cards_spec.rb  | 12 +++-
 .../consumer/shopping/checkout_stripe_spec.rb | 11 ++++
 spec/lib/stripe/credit_card_cloner_spec.rb    | 14 +++--
 .../stripe/payment_intent_validator_spec.rb   |  2 +-
 spec/requests/checkout/stripe_sca_spec.rb     |  7 +++
 spec/support/request/stripe_stubs.rb          | 61 ++++++++++++++++++-
 7 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb
index 1a3d6047bf..1f83d09cac 100644
--- a/spec/features/admin/payments_stripe_spec.rb
+++ b/spec/features/admin/payments_stripe_spec.rb
@@ -24,6 +24,9 @@ feature '
     before do
       stub_payment_methods_post_request
       stub_payment_intent_get_request
+      stub_retrieve_payment_method_request("pm_123")
+      stub_list_customers_request(email: order.user.email, response: {})
+      stub_get_customer_payment_methods_request(customer: "cus_A456", response: {})
     end
 
     context "for a complete order" do
diff --git a/spec/features/consumer/account/cards_spec.rb b/spec/features/consumer/account/cards_spec.rb
index 01d8dcea2d..fe5884e58b 100644
--- a/spec/features/consumer/account/cards_spec.rb
+++ b/spec/features/consumer/account/cards_spec.rb
@@ -4,11 +4,14 @@ require 'spec_helper'
 
 feature "Credit Cards", js: true do
   include AuthenticationHelper
+  include StripeHelper
+  include StripeStubs
+
   describe "as a logged in user" do
     let(:user) { create(:user) }
     let!(:customer) { create(:customer, user: user) }
-    let!(:default_card) { create(:credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_AZNMJ', is_default: true) }
-    let!(:non_default_card) { create(:credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_FDTG') }
+    let!(:default_card) { create(:stored_credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_AZNMJ', is_default: true) }
+    let!(:non_default_card) { create(:stored_credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_FDTG') }
 
     around do |example|
       original_stripe_connect_enabled = Spree::Config[:stripe_connect_enabled]
@@ -19,7 +22,7 @@ feature "Credit Cards", js: true do
     before do
       login_as user
 
-      allow(Stripe).to receive(:api_key) { "sk_test_xxxx" }
+      allow(Stripe).to receive(:api_key) { "sk_test_12345" }
       allow(Stripe).to receive(:publishable_key) { "some_token" }
       Spree::Config.set(stripe_connect_enabled: true)
 
@@ -28,6 +31,9 @@ feature "Credit Cards", js: true do
 
       stub_request(:delete, "https://api.stripe.com/v1/customers/cus_AZNMJ").
         to_return(status: 200, body: JSON.generate(deleted: true, id: "cus_AZNMJ"))
+      stub_retrieve_payment_method_request("card_1EY...")
+      stub_list_customers_request(email: user.email, response: {})
+      stub_get_customer_payment_methods_request(customer: "cus_AZNMJ", response: {})
     end
 
     it "passes the smoke test" do
diff --git a/spec/features/consumer/shopping/checkout_stripe_spec.rb b/spec/features/consumer/shopping/checkout_stripe_spec.rb
index e3a32718aa..06a38adeda 100644
--- a/spec/features/consumer/shopping/checkout_stripe_spec.rb
+++ b/spec/features/consumer/shopping/checkout_stripe_spec.rb
@@ -97,6 +97,12 @@ feature "Check out with Stripe", js: true do
     end
 
     context "with guest checkout" do
+      before do
+        stub_retrieve_payment_method_request("pm_123")
+        stub_list_customers_request(email: order.user.email, response: {})
+        stub_get_customer_payment_methods_request(customer: "cus_A456", response: {})
+      end
+
       context "when the card is accepted" do
         before do
           stub_payment_intents_post_request order: order
@@ -205,7 +211,12 @@ feature "Check out with Stripe", js: true do
 
       context "saving a card and re-using it" do
         before do
+          stub_retrieve_payment_method_request("pm_123")
+          stub_list_customers_request(email: order.user.email, response: {})
+          stub_get_customer_payment_methods_request(customer: "cus_A456", response: {})
+          stub_get_customer_payment_methods_request(customer: "cus_A123", response: {})
           stub_payment_methods_post_request request: { payment_method: "pm_123", customer: "cus_A123" }, response: { pm_id: "pm_123" }
+          stub_add_metadata_request(payment_method: "pm_123", response: {})
           stub_payment_intents_post_request order: order
           stub_successful_capture_request order: order
           stub_customers_post_request email: user.email
diff --git a/spec/lib/stripe/credit_card_cloner_spec.rb b/spec/lib/stripe/credit_card_cloner_spec.rb
index 5b05aa99d7..c7742d41f1 100644
--- a/spec/lib/stripe/credit_card_cloner_spec.rb
+++ b/spec/lib/stripe/credit_card_cloner_spec.rb
@@ -5,7 +5,7 @@ require 'stripe/credit_card_cloner'
 
 module Stripe
   describe CreditCardCloner do
-    describe "#clone" do
+    describe "#find_or_clone" do
       include StripeStubs
 
       let(:cloner) { Stripe::CreditCardCloner.new }
@@ -14,7 +14,7 @@ module Stripe
       let(:payment_method_id) { "pm_1234" }
       let(:new_customer_id) { "cus_A456" }
       let(:new_payment_method_id) { "pm_456" }
-      let(:stripe_account_id) { "acct_456" }
+      let(:stripe_account_id) { "abc123" }
       let(:payment_method_response_mock) { { status: 200, body: payment_method_response_body } }
 
       let(:credit_card) { create(:credit_card, user: create(:user)) }
@@ -29,6 +29,12 @@ module Stripe
         stub_customers_post_request email: credit_card.user.email,
                                     response: { customer_id: new_customer_id },
                                     stripe_account_header: true
+
+        stub_retrieve_payment_method_request(payment_method_id)
+        stub_list_customers_request(email: credit_card.user.email, response: {})
+        stub_get_customer_payment_methods_request(customer: "cus_A456", response: {})
+        stub_add_metadata_request(payment_method: "pm_456", response: {})
+
         stub_request(:post,
                      "https://api.stripe.com/v1/payment_methods/#{new_payment_method_id}/attach")
           .with(body: { customer: new_customer_id },
@@ -47,7 +53,7 @@ module Stripe
         end
 
         it "clones the payment method only" do
-          customer_id, payment_method_id = cloner.clone(credit_card, stripe_account_id)
+          customer_id, payment_method_id = cloner.find_or_clone(credit_card, stripe_account_id)
 
           expect(payment_method_id).to eq new_payment_method_id
           expect(customer_id).to eq nil
@@ -65,7 +71,7 @@ module Stripe
         end
 
         it "clones both the payment method and the customer" do
-          customer_id, payment_method_id = cloner.clone(credit_card, stripe_account_id)
+          customer_id, payment_method_id = cloner.find_or_clone(credit_card, stripe_account_id)
 
           expect(payment_method_id).to eq new_payment_method_id
           expect(customer_id).to eq new_customer_id
diff --git a/spec/lib/stripe/payment_intent_validator_spec.rb b/spec/lib/stripe/payment_intent_validator_spec.rb
index 0488c0b56f..a6b983b367 100644
--- a/spec/lib/stripe/payment_intent_validator_spec.rb
+++ b/spec/lib/stripe/payment_intent_validator_spec.rb
@@ -8,7 +8,7 @@ module Stripe
     describe "#call" do
       let(:validator) { Stripe::PaymentIntentValidator.new }
       let(:payment_intent_id) { "pi_123" }
-      let(:stripe_account_id) { "acct_456" }
+      let(:stripe_account_id) { "abc123" }
       let(:payment_intent_response_mock) { { status: 200, body: payment_intent_response_body } }
 
       before do
diff --git a/spec/requests/checkout/stripe_sca_spec.rb b/spec/requests/checkout/stripe_sca_spec.rb
index ad03211b1a..d1d959ff5f 100644
--- a/spec/requests/checkout/stripe_sca_spec.rb
+++ b/spec/requests/checkout/stripe_sca_spec.rb
@@ -6,6 +6,8 @@ describe "checking out an order with a Stripe SCA payment method", type: :reques
   include ShopWorkflow
   include AuthenticationHelper
   include OpenFoodNetwork::ApiHelper
+  include StripeHelper
+  include StripeStubs
 
   let!(:order_cycle) { create(:simple_order_cycle) }
   let!(:enterprise) { create(:distributor_enterprise) }
@@ -104,6 +106,11 @@ describe "checking out an order with a Stripe SCA payment method", type: :reques
     stub_request(:post, "https://api.stripe.com/v1/payment_intents/#{payment_intent_id}/capture")
       .with(basic_auth: ["sk_test_12345", ""], body: { amount_to_capture: "1234" })
       .to_return(payment_intent_response_mock)
+
+    stub_retrieve_payment_method_request("pm_123")
+    stub_list_customers_request(email: order.user.email, response: {})
+    stub_get_customer_payment_methods_request(customer: "cus_A456", response: {})
+    stub_add_metadata_request(payment_method: "pm_456", response: {})
   end
 
   context "when the user submits a new card and doesn't request that the card is saved for later" do
diff --git a/spec/support/request/stripe_stubs.rb b/spec/support/request/stripe_stubs.rb
index 909604b9ae..e90c756a81 100644
--- a/spec/support/request/stripe_stubs.rb
+++ b/spec/support/request/stripe_stubs.rb
@@ -35,14 +35,47 @@ module StripeStubs
       .to_return(hub_payment_method_response_mock({ pm_id: "pm_123" }))
   end
 
+  def stub_retrieve_payment_method_request(payment_method_id = "pm_1234")
+    stub_request(:get, "https://api.stripe.com/v1/payment_methods/#{payment_method_id}")
+      .with(headers: { 'Authorization' => 'Bearer sk_test_12345' })
+      .to_return(retrieve_payment_method_response_mock({}))
+  end
+
   # Stubs the customers call to both the main stripe account and the connected account
   def stub_customers_post_request(email:, response: {}, stripe_account_header: false)
     stub = stub_request(:post, "https://api.stripe.com/v1/customers")
       .with(body: { email: email })
-    stub = stub.with(headers: { 'Stripe-Account' => 'acct_456' }) if stripe_account_header
+    stub = stub.with(headers: { 'Stripe-Account' => 'abc123' }) if stripe_account_header
     stub.to_return(customers_response_mock(response))
   end
 
+  def stub_list_customers_request(email:, response: {})
+    stub = stub_request(:get, "https://api.stripe.com/v1/customers?email=#{email}&limit=100")
+    stub = stub.with(
+      headers: { 'Authorization' => 'Bearer sk_test_12345', 'Stripe-Account' => 'abc123' }
+    )
+    stub.to_return(list_customers_response_mock(response))
+  end
+
+  def stub_get_customer_payment_methods_request(customer: "cus_A456", response: {})
+    stub = stub_request(
+      :get, "https://api.stripe.com/v1/payment_methods?customer=#{customer}&limit=100&type=card"
+    )
+    stub = stub.with(
+      headers: { 'Authorization' => 'Bearer sk_test_12345', 'Stripe-Account' => 'abc123' }
+    )
+    stub.to_return(get_customer_payment_methods_response_mock(response))
+  end
+
+  def stub_add_metadata_request(payment_method: "pm_456", response: {})
+    stub = stub_request(:post, "https://api.stripe.com/v1/payment_methods/#{payment_method}")
+    stub = stub.with(body: { metadata: { "ofn-clone": true } })
+    stub = stub.with(
+      headers: { 'Authorization' => 'Bearer sk_test_12345', 'Stripe-Account' => 'abc123' }
+    )
+    stub.to_return(add_metadata_response_mock(response))
+  end
+
   def stub_successful_capture_request(order:, response: {})
     stub_capture_request(order, payment_successful_capture_mock(response))
   end
@@ -119,4 +152,30 @@ module StripeStubs
                           amount: 2000,
                           charge: "ch_1234") }
   end
+
+  def retrieve_payment_method_response_mock(options)
+    { status: options[:code] || 200,
+      body: JSON.generate(
+        id: options[:pm_id] || "pm_456", customer: "cus_A123", card: { fingerprint: "12345" }
+      ) }
+  end
+
+  def list_customers_response_mock(options)
+    { status: options[:code] || 200,
+      body: JSON.generate(has_more: false, data: [{ id: "cus_A456" }]) }
+  end
+
+  def get_customer_payment_methods_response_mock(options)
+    payment_method = options[:payment_method] || "pm_456"
+    fingerprint = options[:fingerprint] || "7890"
+    { status: options[:code] || 200,
+      body: JSON.generate(
+        has_more: false, data: [{ id: payment_method, card: { fingerprint: fingerprint } }]
+      ) }
+  end
+
+  def add_metadata_response_mock(options)
+    { status: options[:code] || 200,
+      body: JSON.generate({}) }
+  end
 end

From a466886a32ec930c6e98f6b23d01133baef05dd3 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 19 Nov 2020 13:44:41 -0800
Subject: [PATCH 07/25] fix rubocop warnings

---
 app/controllers/api/customers_controller.rb | 14 +++++++++++---
 app/models/spree/gateway/stripe_sca.rb      | 13 +++++--------
 app/models/spree/payment/processing.rb      | 15 ++++-----------
 app/services/recurring_payments.rb          |  2 +-
 lib/stripe/credit_card_cloner.rb            |  4 ++--
 5 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/app/controllers/api/customers_controller.rb b/app/controllers/api/customers_controller.rb
index e0610e5e32..33ac9d403d 100644
--- a/app/controllers/api/customers_controller.rb
+++ b/app/controllers/api/customers_controller.rb
@@ -13,9 +13,8 @@ module Api
 
       client_secret = RecurringPayments.setup_for(@customer) if params[:customer][:allow_charges]
 
-      if @customer.update(params[:customer])
-        @customer.gateway_recurring_payment_client_secret = client_secret
-        @customer.gateway_shop_id = @customer.enterprise.stripe_account&.stripe_user_id
+      if @customer.update(customer_params)
+        add_recurring_payment_info(@customer, client_secret)
         render json: @customer, serializer: CustomerSerializer, status: :ok
       else
         invalid_resource!(@customer)
@@ -25,5 +24,14 @@ module Api
     def customer_params
       params.require(:customer).permit(:code, :email, :enterprise_id, :allow_charges)
     end
+
+    private
+
+    def add_recurring_payment_info(customer, client_secret)
+      return unless client_secret
+
+      customer.gateway_recurring_payment_client_secret = client_secret
+      customer.gateway_shop_id = customer.enterprise.stripe_account&.stripe_user_id
+    end
   end
 end
diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb
index a6a25aced4..b12ff59200 100644
--- a/app/models/spree/gateway/stripe_sca.rb
+++ b/app/models/spree/gateway/stripe_sca.rb
@@ -47,22 +47,19 @@ module Spree
 
       # NOTE: the name of this method is determined by Spree::Payment::Processing
       def charge_offline(money, creditcard, gateway_options)
-        options = basic_options(gateway_options)
-        customer_id, payment_method_id =
+        customer, payment_method =
           Stripe::CreditCardCloner.new.find_or_clone(creditcard, stripe_account_id)
 
-        options[:customer] = customer_id
-        options[:off_session] = true
-        provider.purchase(money, payment_method_id, options)
+        options = basic_options(gateway_options).merge(customer: customer, off_session: true)
+        provider.purchase(money, payment_method, options)
       rescue Stripe::StripeError => e
         failed_activemerchant_billing_response(e.message)
       end
 
       # NOTE: the name of this method is determined by Spree::Payment::Processing
       def authorize(money, creditcard, gateway_options)
-        authorize_response = provider.authorize(*options_for_authorize(money,
-                                                                       creditcard,
-                                                                       gateway_options))
+        authorize_response =
+          provider.authorize(*options_for_authorize(money, creditcard, gateway_options))
         Stripe::AuthorizeResponsePatcher.new(authorize_response).call!
       rescue Stripe::StripeError => e
         failed_activemerchant_billing_response(e.message)
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index de0c05d422..eda657863e 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -15,10 +15,8 @@ module Spree
           raise Core::GatewayError, Spree.t(:payment_method_not_supported)
         end
 
-        if offline
-          charge_offline!
-        elsif payment_method.auto_capture?
-          purchase!
+        if payment_method.auto_capture?
+          purchase!(offline)
         else
           authorize!
         end
@@ -29,14 +27,9 @@ module Spree
         gateway_action(source, :authorize, :pend)
       end
 
-      def purchase!
+      def purchase!(offline = false)
         started_processing!
-        gateway_action(source, :purchase, :complete)
-      end
-
-      def charge_offline!
-        started_processing!
-        gateway_action(source, :charge_offline, :complete)
+        gateway_action(source, offline ? :charge_offline : :purchase, :complete)
       end
 
       def capture!
diff --git a/app/services/recurring_payments.rb b/app/services/recurring_payments.rb
index 1c622fd6a8..d6c9f1052f 100644
--- a/app/services/recurring_payments.rb
+++ b/app/services/recurring_payments.rb
@@ -9,7 +9,7 @@ class RecurringPayments
       Stripe::CreditCardCloner.new.find_or_clone(card, stripe_account)
     setup_intent = Stripe::SetupIntent.create(
       { payment_method: payment_method_id, customer: customer_id },
-      { stripe_account: stripe_account }
+      stripe_account: stripe_account
     )
     setup_intent.client_secret
   end
diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index fb5e2d3d98..9bbd009342 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -73,7 +73,7 @@ module Stripe
 
       loop do
         response = Stripe::Customer.list({ email: email, starting_after: starting_after },
-                                         { stripe_account: connected_account_id })
+                                         stripe_account: connected_account_id)
         customers += response.data
         break unless response.has_more
 
@@ -88,7 +88,7 @@ module Stripe
 
       loop do
         options = { customer: customer_id, type: 'card', starting_after: starting_after }
-        response = Stripe::PaymentMethod.list(options, { stripe_account: connected_account_id })
+        response = Stripe::PaymentMethod.list(options, stripe_account: connected_account_id)
         payment_methods += response.data
         break unless response.has_more
 

From 277d7f44b7ed2a32e1389ceee4301581f30d8ff0 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 19 Nov 2020 13:49:47 -0800
Subject: [PATCH 08/25] refactor api customers controller; resolve merge
 conflict

---
 app/controllers/api/customers_controller.rb | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/controllers/api/customers_controller.rb b/app/controllers/api/customers_controller.rb
index 33ac9d403d..7cb702a824 100644
--- a/app/controllers/api/customers_controller.rb
+++ b/app/controllers/api/customers_controller.rb
@@ -14,24 +14,24 @@ module Api
       client_secret = RecurringPayments.setup_for(@customer) if params[:customer][:allow_charges]
 
       if @customer.update(customer_params)
-        add_recurring_payment_info(@customer, client_secret)
+        add_recurring_payment_info(client_secret)
         render json: @customer, serializer: CustomerSerializer, status: :ok
       else
         invalid_resource!(@customer)
       end
     end
 
+    private
+
+    def add_recurring_payment_info(client_secret)
+      return unless client_secret
+
+      @customer.gateway_recurring_payment_client_secret = client_secret
+      @customer.gateway_shop_id = @customer.enterprise.stripe_account&.stripe_user_id
+    end
+
     def customer_params
       params.require(:customer).permit(:code, :email, :enterprise_id, :allow_charges)
     end
-
-    private
-
-    def add_recurring_payment_info(customer, client_secret)
-      return unless client_secret
-
-      customer.gateway_recurring_payment_client_secret = client_secret
-      customer.gateway_shop_id = customer.enterprise.stripe_account&.stripe_user_id
-    end
   end
 end

From bc3fd8c50cb43ec5fa4ca54177611fd6898f7d6d Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Fri, 20 Nov 2020 13:58:17 -0800
Subject: [PATCH 09/25] remove unused method

---
 app/controllers/spree/credit_cards_controller.rb | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb
index 1c3f6b0aa6..a92faf088b 100644
--- a/app/controllers/spree/credit_cards_controller.rb
+++ b/app/controllers/spree/credit_cards_controller.rb
@@ -68,13 +68,6 @@ module Spree
       stripe_customer&.delete unless stripe_customer.deleted?
     end
 
-    def stripe_account_id
-      StripeAccount.
-        find_by(enterprise_id: @credit_card.payment_method.preferred_enterprise_id).
-        andand.
-        stripe_user_id
-    end
-
     def create_customer(token)
       Stripe::Customer.create(email: spree_current_user.email, source: token)
     end

From 9c544ef2f4df69b2f54850d25a99e92cc0a307f9 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Fri, 20 Nov 2020 14:33:27 -0800
Subject: [PATCH 10/25] remove cloned cards after removing the platform card

---
 .../spree/credit_cards_controller.rb          |  3 ++-
 lib/stripe/credit_card_cloner.rb              | 20 +++++++++++++++----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb
index a92faf088b..330a59a500 100644
--- a/app/controllers/spree/credit_cards_controller.rb
+++ b/app/controllers/spree/credit_cards_controller.rb
@@ -63,8 +63,9 @@ module Spree
 
     # It destroys the whole customer object
     def destroy_at_stripe
-      stripe_customer = Stripe::Customer.retrieve(@credit_card.gateway_customer_profile_id, {})
+      Stripe::CreditCardCloner.new.destroy_clones(@credit_card)
 
+      stripe_customer = Stripe::Customer.retrieve(@credit_card.gateway_customer_profile_id, {})
       stripe_customer&.delete unless stripe_customer.deleted?
     end
 
diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index 9bbd009342..7d90960bf5 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -17,11 +17,23 @@
 
 module Stripe
   class CreditCardCloner
-    def find_or_clone(credit_card, connected_account_id)
-      if card = find_cloned_card(credit_card, connected_account_id)
-        card
+    def find_or_clone(card, connected_account_id)
+      if cloned_card = find_cloned_card(card, connected_account_id)
+        cloned_card
       else
-        clone(credit_card, connected_account_id)
+        clone(card, connected_account_id)
+      end
+    end
+
+    def destroy_clones(card)
+      card.user.customers.each do |customer|
+        next unless stripe_account = customer.enterprise.stripe_account&.stripe_user_id
+
+        customer_id, _payment_method_id = find_cloned_card(card, stripe_account)
+        next unless customer_id
+
+        customer = Stripe::Customer.retrieve(customer_id, { stripe_account: stripe_account })
+        customer&.delete unless customer.deleted?
       end
     end
 

From 103366ea97fd25f7bd768d6ead393e5a75dbf44c Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Fri, 20 Nov 2020 14:36:46 -0800
Subject: [PATCH 11/25] add request limits to credit card cloner

---
 lib/stripe/credit_card_cloner.rb | 36 +++++++++++++++++---------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index 7d90960bf5..d21bc2986c 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -18,7 +18,7 @@
 module Stripe
   class CreditCardCloner
     def find_or_clone(card, connected_account_id)
-      if cloned_card = find_cloned_card(card, connected_account_id)
+      if card.user && cloned_card = find_cloned_card(card, connected_account_id)
         cloned_card
       else
         clone(card, connected_account_id)
@@ -32,7 +32,7 @@ module Stripe
         customer_id, _payment_method_id = find_cloned_card(card, stripe_account)
         next unless customer_id
 
-        customer = Stripe::Customer.retrieve(customer_id, { stripe_account: stripe_account })
+        customer = Stripe::Customer.retrieve(customer_id, stripe_account: stripe_account)
         customer&.delete unless customer.deleted?
       end
     end
@@ -57,18 +57,16 @@ module Stripe
     end
 
     def find_cloned_card(card, connected_account_id)
-      matches = []
-      return matches unless fingerprint = fingerprint_for_card(card)
+      return nil unless fingerprint = fingerprint_for_card(card)
 
       find_customers(card.user.email, connected_account_id).each do |customer|
         find_payment_methods(customer.id, connected_account_id).each do |payment_method|
           if payment_method_is_clone?(payment_method, fingerprint)
-            matches << [customer.id, payment_method.id]
+            return [customer.id, payment_method.id]
           end
         end
       end
-
-      matches.first
+      nil
     end
 
     def payment_method_is_clone?(payment_method, fingerprint)
@@ -80,35 +78,39 @@ module Stripe
     end
 
     def find_customers(email, connected_account_id)
-      starting_after = nil
-      customers = []
+      start_after, customers = nil, []
 
-      loop do
-        response = Stripe::Customer.list({ email: email, starting_after: starting_after },
+      (1..request_limit = 100).each do |request_number|
+        response = Stripe::Customer.list({ email: email, starting_after: start_after, limit: 100 },
                                          stripe_account: connected_account_id)
         customers += response.data
         break unless response.has_more
 
-        starting_after = response.data.last.id
+        start_after = response.data.last.id
+        notify_limit(request_number, "customers") if request_limit == request_number
       end
       customers
     end
 
     def find_payment_methods(customer_id, connected_account_id)
-      starting_after = nil
-      payment_methods = []
+      start_after, payment_methods = nil, []
 
-      loop do
-        options = { customer: customer_id, type: 'card', starting_after: starting_after }
+      (1..request_limit = 10).each do |request_number|
+        options = { customer: customer_id, type: 'card', starting_after: start_after, limit: 100 }
         response = Stripe::PaymentMethod.list(options, stripe_account: connected_account_id)
         payment_methods += response.data
         break unless response.has_more
 
-        starting_after = response.data.last.id
+        start_after = response.data.last.id
+        notify_limit(request_number, "payment methods") if request_limit == request_number
       end
       payment_methods
     end
 
+    def notify_limit(request_number, retrieving)
+      Bugsnag.notify("Reached limit of #{request_number} requests retrieving #{retrieving}.")
+    end
+
     def clone_payment_method(credit_card, connected_account_id)
       platform_acct_payment_method_id = credit_card.gateway_payment_profile_id
       customer_id = credit_card.gateway_customer_profile_id

From f1d439870e55fb994237ba816b5d94d2f437f10b Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 3 Dec 2020 13:05:12 -0800
Subject: [PATCH 12/25] use a named argument for `offline` param

---
 app/jobs/subscription_confirm_job.rb | 2 +-
 app/models/spree/order.rb            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index f3d16d2ea9..d2b7991a03 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -62,7 +62,7 @@ class SubscriptionConfirmJob
     return unless order.payment_required?
 
     prepare_for_payment!(order)
-    order.process_payments!(true)
+    order.process_payments!(offline: true)
     raise if order.errors.any?
   end
 
diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 817b55d319..c85bf1ef05 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -499,7 +499,7 @@ module Spree
     #   which gets rescued and converted to FALSE when
     #   :allow_checkout_on_gateway_error is set to false
     #
-    def process_payments!(offline = false)
+    def process_payments!(offline: false)
       raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
 
       pending_payments.each do |payment|

From 3a8203094abcfd44f1fddb4d83d96bf636cee009 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Fri, 4 Dec 2020 15:38:25 -0800
Subject: [PATCH 13/25] refactor to remove boolean flag param

---
 app/jobs/subscription_confirm_job.rb   |  2 +-
 app/models/spree/order.rb              | 21 +++++++++--
 app/models/spree/payment/processing.rb | 48 ++++++++++++++++++--------
 3 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb
index d2b7991a03..06c205bad6 100644
--- a/app/jobs/subscription_confirm_job.rb
+++ b/app/jobs/subscription_confirm_job.rb
@@ -62,7 +62,7 @@ class SubscriptionConfirmJob
     return unless order.payment_required?
 
     prepare_for_payment!(order)
-    order.process_payments!(offline: true)
+    order.process_payments_offline!
     raise if order.errors.any?
   end
 
diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index c85bf1ef05..e99e26db67 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -499,13 +499,13 @@ module Spree
     #   which gets rescued and converted to FALSE when
     #   :allow_checkout_on_gateway_error is set to false
     #
-    def process_payments!(offline: false)
+    def process_payments!
       raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
 
       pending_payments.each do |payment|
         break if payment_total >= total
 
-        payment.process!(offline)
+        payment.process!
 
         if payment.completed?
           self.payment_total += payment.amount
@@ -516,6 +516,23 @@ module Spree
       errors.add(:base, e.message) && (return result)
     end
 
+    def process_payments_offline!
+      raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
+
+      pending_payments.each do |payment|
+        break if payment_total >= total
+
+        payment.process_offline!
+
+        if payment.completed?
+          self.payment_total += payment.amount
+        end
+      end
+    rescue Core::GatewayError => e
+      errors.add(:base, e.message)
+      false
+    end
+
     def billing_firstname
       bill_address.try(:firstname)
     end
diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index eda657863e..0b2daa4bc9 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -3,20 +3,21 @@
 module Spree
   class Payment < ActiveRecord::Base
     module Processing
-      def process!(offline = false)
-        return unless payment_method&.source_required?
-
-        raise Core::GatewayError, Spree.t(:payment_processing_failed) unless source
-
-        return if processing?
-
-        unless payment_method.supports?(source)
-          invalidate!
-          raise Core::GatewayError, Spree.t(:payment_method_not_supported)
-        end
+      def process!
+        return unless ready_to_process?
 
         if payment_method.auto_capture?
-          purchase!(offline)
+          purchase!
+        else
+          authorize!
+        end
+      end
+
+      def process_offline!
+        return unless ready_to_process?
+
+        if payment_method.auto_capture?
+          charge_offline!
         else
           authorize!
         end
@@ -27,9 +28,14 @@ module Spree
         gateway_action(source, :authorize, :pend)
       end
 
-      def purchase!(offline = false)
+      def purchase!
         started_processing!
-        gateway_action(source, offline ? :charge_offline : :purchase, :complete)
+        gateway_action(source, :purchase, :complete)
+      end
+
+      def charge_offline!
+        started_processing!
+        gateway_action(source, :charge_offline, :complete)
       end
 
       def capture!
@@ -193,6 +199,20 @@ module Spree
 
       private
 
+      def ready_to_process?
+        return false unless payment_method&.source_required?
+
+        raise Core::GatewayError, Spree.t(:payment_processing_failed) unless source
+
+        return false if processing?
+
+        unless payment_method.supports?(source)
+          invalidate!
+          raise Core::GatewayError, Spree.t(:payment_method_not_supported)
+        end
+        true
+      end
+
       def calculate_refund_amount(refund_amount = nil)
         refund_amount ||= if credit_allowed >= order.outstanding_balance.abs
                             order.outstanding_balance.abs

From eddf8da107700483dac3c86a3fef797222ca29ca Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Fri, 4 Dec 2020 17:35:20 -0800
Subject: [PATCH 14/25] update subscription spec with new method name

---
 spec/jobs/subscription_confirm_job_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb
index 4cc3f521b0..78c0e9cda8 100644
--- a/spec/jobs/subscription_confirm_job_spec.rb
+++ b/spec/jobs/subscription_confirm_job_spec.rb
@@ -160,7 +160,7 @@ describe SubscriptionConfirmJob do
 
         context "when an error occurs while processing the payment" do
           before do
-            expect(payment).to receive(:process!).and_raise Spree::Core::GatewayError, "payment failure error"
+            expect(payment).to receive(:process_offline!).and_raise Spree::Core::GatewayError, "payment failure error"
           end
 
           it "sends a failed payment email" do
@@ -176,7 +176,7 @@ describe SubscriptionConfirmJob do
           end
 
           before do
-            expect(payment).to receive(:process!) { true }
+            expect(payment).to receive(:process_offline!) { true }
             expect(payment).to receive(:completed?) { true }
           end
 

From d55343da1a6b00a9546ded88e9cdb9e7aa6da0e7 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Sat, 5 Dec 2020 07:20:26 -0800
Subject: [PATCH 15/25] only return gateway payment info if set on customer

---
 app/serializers/api/customer_serializer.rb | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/app/serializers/api/customer_serializer.rb b/app/serializers/api/customer_serializer.rb
index ee543e5031..1c437a987e 100644
--- a/app/serializers/api/customer_serializer.rb
+++ b/app/serializers/api/customer_serializer.rb
@@ -1,6 +1,14 @@
 module Api
   class CustomerSerializer < ActiveModel::Serializer
-    attributes :id, :enterprise_id, :name, :code, :email, :allow_charges,
-               :gateway_recurring_payment_client_secret, :gateway_shop_id
+    attributes :id, :enterprise_id, :name, :code, :email, :allow_charges
+
+    def attributes
+      hash = super
+      if secret = object.gateway_recurring_payment_client_secret
+        hash.merge!(gateway_recurring_payment_client_secret: secret)
+      end
+      hash.merge!(gateway_shop_id: object.gateway_shop_id) if object.gateway_shop_id
+      hash
+    end
   end
 end

From 50e87a023b640fdaaf1ce649aa209dba70065154 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 07:45:07 -0800
Subject: [PATCH 16/25] rename method to `validate!` since it can raise an
 error

---
 app/models/spree/payment/processing.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb
index 0b2daa4bc9..36704362a8 100644
--- a/app/models/spree/payment/processing.rb
+++ b/app/models/spree/payment/processing.rb
@@ -4,7 +4,7 @@ module Spree
   class Payment < ActiveRecord::Base
     module Processing
       def process!
-        return unless ready_to_process?
+        return unless validate!
 
         if payment_method.auto_capture?
           purchase!
@@ -14,7 +14,7 @@ module Spree
       end
 
       def process_offline!
-        return unless ready_to_process?
+        return unless validate!
 
         if payment_method.auto_capture?
           charge_offline!
@@ -199,7 +199,7 @@ module Spree
 
       private
 
-      def ready_to_process?
+      def validate!
         return false unless payment_method&.source_required?
 
         raise Core::GatewayError, Spree.t(:payment_processing_failed) unless source

From 13b95f41bbb1e6366decc8282829febe1e7b9446 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 08:19:41 -0800
Subject: [PATCH 17/25] use built-in `auto_paging_each` with stripe

---
 lib/stripe/credit_card_cloner.rb | 45 +++++++-------------------------
 1 file changed, 9 insertions(+), 36 deletions(-)

diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index d21bc2986c..c7ea7cddde 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -59,17 +59,20 @@ module Stripe
     def find_cloned_card(card, connected_account_id)
       return nil unless fingerprint = fingerprint_for_card(card)
 
-      find_customers(card.user.email, connected_account_id).each do |customer|
-        find_payment_methods(customer.id, connected_account_id).each do |payment_method|
-          if payment_method_is_clone?(payment_method, fingerprint)
-            return [customer.id, payment_method.id]
-          end
+      customers = Stripe::Customer.list({ email: card.user.email, limit: 100 },
+                                        stripe_account: connected_account_id)
+
+      customers.auto_paging_each do |customer|
+        options = { customer: customer.id, type: 'card', limit: 100 }
+        payment_methods = Stripe::PaymentMethod.list(options, stripe_account: connected_account_id)
+        payment_methods.auto_paging_each do |payment_method|
+          return [customer.id, payment_method.id] if clone?(payment_method, fingerprint)
         end
       end
       nil
     end
 
-    def payment_method_is_clone?(payment_method, fingerprint)
+    def clone?(payment_method, fingerprint)
       payment_method.card.fingerprint == fingerprint && payment_method.metadata["ofn-clone"]
     end
 
@@ -77,36 +80,6 @@ module Stripe
       Stripe::PaymentMethod.retrieve(card.gateway_payment_profile_id).card.fingerprint
     end
 
-    def find_customers(email, connected_account_id)
-      start_after, customers = nil, []
-
-      (1..request_limit = 100).each do |request_number|
-        response = Stripe::Customer.list({ email: email, starting_after: start_after, limit: 100 },
-                                         stripe_account: connected_account_id)
-        customers += response.data
-        break unless response.has_more
-
-        start_after = response.data.last.id
-        notify_limit(request_number, "customers") if request_limit == request_number
-      end
-      customers
-    end
-
-    def find_payment_methods(customer_id, connected_account_id)
-      start_after, payment_methods = nil, []
-
-      (1..request_limit = 10).each do |request_number|
-        options = { customer: customer_id, type: 'card', starting_after: start_after, limit: 100 }
-        response = Stripe::PaymentMethod.list(options, stripe_account: connected_account_id)
-        payment_methods += response.data
-        break unless response.has_more
-
-        start_after = response.data.last.id
-        notify_limit(request_number, "payment methods") if request_limit == request_number
-      end
-      payment_methods
-    end
-
     def notify_limit(request_number, retrieving)
       Bugsnag.notify("Reached limit of #{request_number} requests retrieving #{retrieving}.")
     end

From 0ac248f03ab2e4ffb13c312b86d6c719db2a693e Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 09:55:08 -0800
Subject: [PATCH 18/25] refactor offline payment methods

---
 app/models/spree/order.rb | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index e99e26db67..2b66cb9124 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -500,34 +500,14 @@ module Spree
     #   :allow_checkout_on_gateway_error is set to false
     #
     def process_payments!
-      raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
-
-      pending_payments.each do |payment|
-        break if payment_total >= total
-
-        payment.process!
-
-        if payment.completed?
-          self.payment_total += payment.amount
-        end
-      end
+      process_each_payment(&:process!)
     rescue Core::GatewayError => e
       result = !!Spree::Config[:allow_checkout_on_gateway_error]
       errors.add(:base, e.message) && (return result)
     end
 
     def process_payments_offline!
-      raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
-
-      pending_payments.each do |payment|
-        break if payment_total >= total
-
-        payment.process_offline!
-
-        if payment.completed?
-          self.payment_total += payment.amount
-        end
-      end
+      process_each_payment(&:process_offline!)
     rescue Core::GatewayError => e
       errors.add(:base, e.message)
       false
@@ -795,6 +775,20 @@ module Spree
 
     private
 
+    def process_each_payment
+      raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty?
+
+      pending_payments.each do |payment|
+        break if payment_total >= total
+
+        yield payment
+
+        if payment.completed?
+          self.payment_total += payment.amount
+        end
+      end
+    end
+
     def link_by_email
       self.email = user.email if user
     end

From 4c25edd91c395a22d592ad6ae7e0506e2879ca13 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 11:04:27 -0800
Subject: [PATCH 19/25] refactor find_cloned_card to separate class

---
 lib/stripe/credit_card_clone_finder.rb | 35 ++++++++++++++++++++++++++
 lib/stripe/credit_card_cloner.rb       | 34 +++----------------------
 2 files changed, 39 insertions(+), 30 deletions(-)
 create mode 100644 lib/stripe/credit_card_clone_finder.rb

diff --git a/lib/stripe/credit_card_clone_finder.rb b/lib/stripe/credit_card_clone_finder.rb
new file mode 100644
index 0000000000..a2c54aa07f
--- /dev/null
+++ b/lib/stripe/credit_card_clone_finder.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module Stripe
+  class CreditCardCloneFinder
+    def initialize(card, stripe_account)
+      @card = card
+      @stripe_account = stripe_account
+    end
+
+    def find_cloned_card
+      return nil unless fingerprint = fingerprint_for_card(@card) && email = @card.user&.email
+
+      customers = Stripe::Customer.list({ email: email, limit: 100 }, stripe_account: @stripe_account)
+
+      customers.auto_paging_each do |customer|
+        options = { customer: customer.id, type: 'card', limit: 100 }
+        payment_methods = Stripe::PaymentMethod.list(options, stripe_account: @stripe_account)
+        payment_methods.auto_paging_each do |payment_method|
+          return [customer.id, payment_method.id] if clone?(payment_method, fingerprint)
+        end
+      end
+      nil
+    end
+
+    private
+
+    def clone?(payment_method, fingerprint)
+      payment_method.card.fingerprint == fingerprint && payment_method.metadata["ofn-clone"]
+    end
+
+    def fingerprint_for_card(card)
+      Stripe::PaymentMethod.retrieve(card.gateway_payment_profile_id).card.fingerprint
+    end
+  end
+end
diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index c7ea7cddde..1518fc0af0 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -18,18 +18,16 @@
 module Stripe
   class CreditCardCloner
     def find_or_clone(card, connected_account_id)
-      if card.user && cloned_card = find_cloned_card(card, connected_account_id)
-        cloned_card
-      else
-        clone(card, connected_account_id)
-      end
+      cloned_card = CreditCardCloneFinder.new(card, connected_account_id).find_cloned_card
+      cloned_card || clone(card, connected_account_id)
     end
 
     def destroy_clones(card)
       card.user.customers.each do |customer|
         next unless stripe_account = customer.enterprise.stripe_account&.stripe_user_id
 
-        customer_id, _payment_method_id = find_cloned_card(card, stripe_account)
+        customer_id, _payment_method_id =
+          CreditCardCloneFinder.new(card, stripe_account).find_cloned_card
         next unless customer_id
 
         customer = Stripe::Customer.retrieve(customer_id, stripe_account: stripe_account)
@@ -56,30 +54,6 @@ module Stripe
       [new_customer.id, new_payment_method.id]
     end
 
-    def find_cloned_card(card, connected_account_id)
-      return nil unless fingerprint = fingerprint_for_card(card)
-
-      customers = Stripe::Customer.list({ email: card.user.email, limit: 100 },
-                                        stripe_account: connected_account_id)
-
-      customers.auto_paging_each do |customer|
-        options = { customer: customer.id, type: 'card', limit: 100 }
-        payment_methods = Stripe::PaymentMethod.list(options, stripe_account: connected_account_id)
-        payment_methods.auto_paging_each do |payment_method|
-          return [customer.id, payment_method.id] if clone?(payment_method, fingerprint)
-        end
-      end
-      nil
-    end
-
-    def clone?(payment_method, fingerprint)
-      payment_method.card.fingerprint == fingerprint && payment_method.metadata["ofn-clone"]
-    end
-
-    def fingerprint_for_card(card)
-      Stripe::PaymentMethod.retrieve(card.gateway_payment_profile_id).card.fingerprint
-    end
-
     def notify_limit(request_number, retrieving)
       Bugsnag.notify("Reached limit of #{request_number} requests retrieving #{retrieving}.")
     end

From 8c747e4812195c9a75e34c7fdc248122329af9f4 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 11:06:10 -0800
Subject: [PATCH 20/25] refactor destroy_clones to separate class

---
 .../spree/credit_cards_controller.rb          |  2 +-
 lib/stripe/credit_card_clone_destroyer.rb     | 24 +++++++++++++++++++
 lib/stripe/credit_card_cloner.rb              | 13 ----------
 3 files changed, 25 insertions(+), 14 deletions(-)
 create mode 100644 lib/stripe/credit_card_clone_destroyer.rb

diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb
index 330a59a500..02e23d750c 100644
--- a/app/controllers/spree/credit_cards_controller.rb
+++ b/app/controllers/spree/credit_cards_controller.rb
@@ -63,7 +63,7 @@ module Spree
 
     # It destroys the whole customer object
     def destroy_at_stripe
-      Stripe::CreditCardCloner.new.destroy_clones(@credit_card)
+      Stripe::CreditCardCloneDestroyer.new.destroy_clones(@credit_card)
 
       stripe_customer = Stripe::Customer.retrieve(@credit_card.gateway_customer_profile_id, {})
       stripe_customer&.delete unless stripe_customer.deleted?
diff --git a/lib/stripe/credit_card_clone_destroyer.rb b/lib/stripe/credit_card_clone_destroyer.rb
new file mode 100644
index 0000000000..9ef68892e0
--- /dev/null
+++ b/lib/stripe/credit_card_clone_destroyer.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+# Here we destroy (on Stripe) any clones that we have created for a platform card.
+# See CreditCardCloner for details.
+
+# This is useful when the platform card is deleted (and needs to happen before the
+# platform card is deleted on Stripe).
+
+module Stripe
+  class CreditCardCloneDestroyer
+    def destroy_clones(card)
+      card.user.customers.each do |customer|
+        next unless stripe_account = customer.enterprise.stripe_account&.stripe_user_id
+
+        customer_id, _payment_method_id =
+          CreditCardCloneFinder.new(card, stripe_account).find_cloned_card
+        next unless customer_id
+
+        customer = Stripe::Customer.retrieve(customer_id, stripe_account: stripe_account)
+        customer&.delete unless customer.deleted?
+      end
+    end
+  end
+end
diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index 1518fc0af0..8a1bedbaab 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -22,19 +22,6 @@ module Stripe
       cloned_card || clone(card, connected_account_id)
     end
 
-    def destroy_clones(card)
-      card.user.customers.each do |customer|
-        next unless stripe_account = customer.enterprise.stripe_account&.stripe_user_id
-
-        customer_id, _payment_method_id =
-          CreditCardCloneFinder.new(card, stripe_account).find_cloned_card
-        next unless customer_id
-
-        customer = Stripe::Customer.retrieve(customer_id, stripe_account: stripe_account)
-        customer&.delete unless customer.deleted?
-      end
-    end
-
     private
 
     def clone(credit_card, connected_account_id)

From a1f6fe552244618c49d4a57a858ff682b4aff645 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 11:07:54 -0800
Subject: [PATCH 21/25] remove unused method now that we use autopaging

---
 lib/stripe/credit_card_cloner.rb | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index 8a1bedbaab..2b86d83a1a 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -41,10 +41,6 @@ module Stripe
       [new_customer.id, new_payment_method.id]
     end
 
-    def notify_limit(request_number, retrieving)
-      Bugsnag.notify("Reached limit of #{request_number} requests retrieving #{retrieving}.")
-    end
-
     def clone_payment_method(credit_card, connected_account_id)
       platform_acct_payment_method_id = credit_card.gateway_payment_profile_id
       customer_id = credit_card.gateway_customer_profile_id

From f50577b4893632756d2fd38905d73dcc4e99b706 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 11:14:09 -0800
Subject: [PATCH 22/25] refactor cloner to use ivars

---
 app/models/spree/gateway/stripe_sca.rb |  4 +--
 app/services/recurring_payments.rb     |  2 +-
 lib/stripe/credit_card_cloner.rb       | 42 ++++++++++++++------------
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb
index b12ff59200..68116655fb 100644
--- a/app/models/spree/gateway/stripe_sca.rb
+++ b/app/models/spree/gateway/stripe_sca.rb
@@ -48,7 +48,7 @@ module Spree
       # NOTE: the name of this method is determined by Spree::Payment::Processing
       def charge_offline(money, creditcard, gateway_options)
         customer, payment_method =
-          Stripe::CreditCardCloner.new.find_or_clone(creditcard, stripe_account_id)
+          Stripe::CreditCardCloner.new(creditcard, stripe_account_id).find_or_clone
 
         options = basic_options(gateway_options).merge(customer: customer, off_session: true)
         provider.purchase(money, payment_method, options)
@@ -108,7 +108,7 @@ module Spree
         options[:return_url] = full_checkout_path
 
         customer_id, payment_method_id =
-          Stripe::CreditCardCloner.new.find_or_clone(creditcard, stripe_account_id)
+          Stripe::CreditCardCloner.new(creditcard, stripe_account_id).find_or_clone
         options[:customer] = customer_id
         [money, payment_method_id, options]
       end
diff --git a/app/services/recurring_payments.rb b/app/services/recurring_payments.rb
index d6c9f1052f..e7c9e682fe 100644
--- a/app/services/recurring_payments.rb
+++ b/app/services/recurring_payments.rb
@@ -6,7 +6,7 @@ class RecurringPayments
     return unless stripe_account = customer.enterprise.stripe_account&.stripe_user_id
 
     customer_id, payment_method_id =
-      Stripe::CreditCardCloner.new.find_or_clone(card, stripe_account)
+      Stripe::CreditCardCloner.new(card, stripe_account).find_or_clone
     setup_intent = Stripe::SetupIntent.create(
       { payment_method: payment_method_id, customer: customer_id },
       stripe_account: stripe_account
diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index 2b86d83a1a..64180b9ffc 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -17,49 +17,53 @@
 
 module Stripe
   class CreditCardCloner
-    def find_or_clone(card, connected_account_id)
-      cloned_card = CreditCardCloneFinder.new(card, connected_account_id).find_cloned_card
-      cloned_card || clone(card, connected_account_id)
+    def initialize(card, stripe_account)
+      @card = card
+      @stripe_account = stripe_account
+    end
+
+    def find_or_clone
+      cloned_card = CreditCardCloneFinder.new(@card, @stripe_account).find_cloned_card
+      cloned_card || clone
     end
 
     private
 
-    def clone(credit_card, connected_account_id)
-      new_payment_method = clone_payment_method(credit_card, connected_account_id)
+    def clone
+      new_payment_method = clone_payment_method
 
       # If no customer is given, it will clone the payment method only
-      return [nil, new_payment_method.id] if credit_card.gateway_customer_profile_id.blank?
+      return [nil, new_payment_method.id] if @card.gateway_customer_profile_id.blank?
 
-      new_customer = Stripe::Customer.create({ email: credit_card.user.email },
-                                             stripe_account: connected_account_id)
+      new_customer = Stripe::Customer.create({ email: @card.user.email },
+                                             stripe_account: @stripe_account)
       attach_payment_method_to_customer(new_payment_method.id,
-                                        new_customer.id,
-                                        connected_account_id)
+                                        new_customer.id)
 
-      add_metadata_to_payment_method(new_payment_method.id, connected_account_id)
+      add_metadata_to_payment_method(new_payment_method.id)
 
       [new_customer.id, new_payment_method.id]
     end
 
-    def clone_payment_method(credit_card, connected_account_id)
-      platform_acct_payment_method_id = credit_card.gateway_payment_profile_id
-      customer_id = credit_card.gateway_customer_profile_id
+    def clone_payment_method
+      platform_acct_payment_method_id = @card.gateway_payment_profile_id
+      customer_id = @card.gateway_customer_profile_id
 
       Stripe::PaymentMethod.create({ customer: customer_id,
                                      payment_method: platform_acct_payment_method_id },
-                                   stripe_account: connected_account_id)
+                                   stripe_account: @stripe_account)
     end
 
-    def attach_payment_method_to_customer(payment_method_id, customer_id, connected_account_id)
+    def attach_payment_method_to_customer(payment_method_id, customer_id)
       Stripe::PaymentMethod.attach(payment_method_id,
                                    { customer: customer_id },
-                                   stripe_account: connected_account_id)
+                                   stripe_account: @stripe_account)
     end
 
-    def add_metadata_to_payment_method(payment_method_id, connected_account_id)
+    def add_metadata_to_payment_method(payment_method_id)
       Stripe::PaymentMethod.update(payment_method_id,
                                    { metadata: { "ofn-clone": true } },
-                                   stripe_account: connected_account_id)
+                                   stripe_account: @stripe_account)
     end
   end
 end

From 655512adabb2d7d6f5af923bfdbaf9fff7c5fd6d Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 12:17:42 -0800
Subject: [PATCH 23/25] add missing `require` statements

---
 app/controllers/spree/credit_cards_controller.rb | 2 ++
 lib/stripe/credit_card_clone_destroyer.rb        | 2 +-
 lib/stripe/credit_card_cloner.rb                 | 4 +++-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb
index 02e23d750c..b084fc4707 100644
--- a/app/controllers/spree/credit_cards_controller.rb
+++ b/app/controllers/spree/credit_cards_controller.rb
@@ -1,3 +1,5 @@
+require 'stripe/credit_card_clone_destroyer'
+
 module Spree
   class CreditCardsController < BaseController
     def new_from_token
diff --git a/lib/stripe/credit_card_clone_destroyer.rb b/lib/stripe/credit_card_clone_destroyer.rb
index 9ef68892e0..6fa3db9a88 100644
--- a/lib/stripe/credit_card_clone_destroyer.rb
+++ b/lib/stripe/credit_card_clone_destroyer.rb
@@ -13,7 +13,7 @@ module Stripe
         next unless stripe_account = customer.enterprise.stripe_account&.stripe_user_id
 
         customer_id, _payment_method_id =
-          CreditCardCloneFinder.new(card, stripe_account).find_cloned_card
+          Stripe::CreditCardCloneFinder.new(card, stripe_account).find_cloned_card
         next unless customer_id
 
         customer = Stripe::Customer.retrieve(customer_id, stripe_account: stripe_account)
diff --git a/lib/stripe/credit_card_cloner.rb b/lib/stripe/credit_card_cloner.rb
index 64180b9ffc..441efbe52a 100644
--- a/lib/stripe/credit_card_cloner.rb
+++ b/lib/stripe/credit_card_cloner.rb
@@ -15,6 +15,8 @@
 # clone it and look for a card with the same fingerprint (hash of the card number) and
 # that metadata key to avoid cloning it multiple times.
 
+require 'stripe/credit_card_clone_finder'
+
 module Stripe
   class CreditCardCloner
     def initialize(card, stripe_account)
@@ -23,7 +25,7 @@ module Stripe
     end
 
     def find_or_clone
-      cloned_card = CreditCardCloneFinder.new(@card, @stripe_account).find_cloned_card
+      cloned_card = Stripe::CreditCardCloneFinder.new(@card, @stripe_account).find_cloned_card
       cloned_card || clone
     end
 

From 3daccb642013c73ba57bc95989750fbd72e49960 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 12:17:55 -0800
Subject: [PATCH 24/25] update specs with new method signature

---
 spec/lib/stripe/credit_card_cloner_spec.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/spec/lib/stripe/credit_card_cloner_spec.rb b/spec/lib/stripe/credit_card_cloner_spec.rb
index c7742d41f1..62584f8313 100644
--- a/spec/lib/stripe/credit_card_cloner_spec.rb
+++ b/spec/lib/stripe/credit_card_cloner_spec.rb
@@ -8,17 +8,17 @@ module Stripe
     describe "#find_or_clone" do
       include StripeStubs
 
-      let(:cloner) { Stripe::CreditCardCloner.new }
+      let(:credit_card) { create(:credit_card, user: create(:user)) }
+      let(:stripe_account_id) { "abc123" }
+
+      let(:cloner) { Stripe::CreditCardCloner.new(credit_card, stripe_account_id) }
 
       let(:customer_id) { "cus_A123" }
       let(:payment_method_id) { "pm_1234" }
       let(:new_customer_id) { "cus_A456" }
       let(:new_payment_method_id) { "pm_456" }
-      let(:stripe_account_id) { "abc123" }
       let(:payment_method_response_mock) { { status: 200, body: payment_method_response_body } }
 
-      let(:credit_card) { create(:credit_card, user: create(:user)) }
-
       let(:payment_method_response_body) {
         JSON.generate(id: new_payment_method_id)
       }
@@ -53,7 +53,7 @@ module Stripe
         end
 
         it "clones the payment method only" do
-          customer_id, payment_method_id = cloner.find_or_clone(credit_card, stripe_account_id)
+          customer_id, payment_method_id = cloner.find_or_clone
 
           expect(payment_method_id).to eq new_payment_method_id
           expect(customer_id).to eq nil
@@ -71,7 +71,7 @@ module Stripe
         end
 
         it "clones both the payment method and the customer" do
-          customer_id, payment_method_id = cloner.find_or_clone(credit_card, stripe_account_id)
+          customer_id, payment_method_id = cloner.find_or_clone
 
           expect(payment_method_id).to eq new_payment_method_id
           expect(customer_id).to eq new_customer_id

From 3b7313f7e3563c732dc5599882e95d57cd201df0 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 10 Dec 2020 12:31:58 -0800
Subject: [PATCH 25/25] add spec for deleting the default card

---
 .../spree/credit_cards_controller.rb          |  1 +
 .../spree/credit_cards_controller_spec.rb     | 20 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb
index b084fc4707..652e9878b0 100644
--- a/app/controllers/spree/credit_cards_controller.rb
+++ b/app/controllers/spree/credit_cards_controller.rb
@@ -47,6 +47,7 @@ module Spree
 
       # Using try because we may not have a card here
       if @credit_card.try(:destroy)
+        remove_shop_authorizations if @credit_card.is_default
         flash[:success] = I18n.t(:card_has_been_removed, number: "x-#{@credit_card.last_digits}")
       else
         flash[:error] = I18n.t(:card_could_not_be_removed)
diff --git a/spec/controllers/spree/credit_cards_controller_spec.rb b/spec/controllers/spree/credit_cards_controller_spec.rb
index 280119aa69..82da0f59d4 100644
--- a/spec/controllers/spree/credit_cards_controller_spec.rb
+++ b/spec/controllers/spree/credit_cards_controller_spec.rb
@@ -188,6 +188,26 @@ describe Spree::CreditCardsController, type: :controller do
             expect(flash[:success]).to eq I18n.t(:card_has_been_removed, number: "x-#{card.last_digits}")
             expect(response).to redirect_to spree.account_path(anchor: 'cards')
           end
+
+          context "the card is the default card and there are existing authorizations for the user" do
+            before do
+              card.update_attribute(:is_default, true)
+            end
+            let!(:customer1) { create(:customer, allow_charges: true) }
+            let!(:customer2) { create(:customer, allow_charges: true) }
+
+            it "removes the authorizations" do
+              customer1.user = card.user
+              customer2.user = card.user
+              customer1.save
+              customer2.save
+              expect(customer1.reload.allow_charges).to be true
+              expect(customer2.reload.allow_charges).to be true
+              spree_delete :destroy, params
+              expect(customer1.reload.allow_charges).to be false
+              expect(customer2.reload.allow_charges).to be false
+            end
+          end
         end
       end
     end