Merge pull request #3837 from luca-landa/improve-psp-switching-errors

Improve payment service providers switching errors

core/app/models/spree/payment_method.rb

@@ -13,6 +13,7 @@ module Spree
   #
   class PaymentMethod < Spree::Base
     include Spree::Preferences::Persistable
+    class UnsupportedPaymentMethod < StandardError; end
 
     preference :server, :string, default: 'test'
     preference :test_mode, :boolean, default: true
@@ -60,6 +61,16 @@ class << self
       def model_name
         ModelName.new(self, Spree)
       end
+
+      def find_sti_class(type_name)
+        super(type_name)
+      rescue ActiveRecord::SubclassNotFound
+        raise UnsupportedPaymentMethod, "Found invalid payment type '#{type_name}'.\n"\
+          "This may happen after switching payment service provider, when payment methods "\
+          "reference old types that are not supported any more.\n"\
+          "If that is the case, consider running 'rake payment_method:deprecate_unsupported_payment_methods' "\
+          "to fix the issue."
+      end
     end
 
     # Represents the gateway of this payment method

core/db/migrate/20201127212108_add_type_before_removal_to_spree_payment_methods.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddTypeBeforeRemovalToSpreePaymentMethods < ActiveRecord::Migration[5.2]
+  def change
+    add_column :spree_payment_methods, :type_before_removal, :string
+  end
+end

core/lib/tasks/payment_method.rake

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+namespace :payment_method do
+  desc "Deactivates old payment methods and fixes ActiveRecord::SubclassNotFound error, "\
+  "which happens after switching Payment Service Provider."
+  task deactivate_unsupported_payment_methods: :environment do
+    Spree::PaymentMethod.pluck(:id, :type).select do |id, type|
+      ActiveSupport::Dependencies.constantize(type)
+    rescue NameError
+      fix_payment_method_record(id, type)
+    end
+  end
+
+  def fix_payment_method_record(id, previous_type)
+    connection = ActiveRecord::Base.connection
+    false_value = connection.quoted_false
+    connection.exec_update(<<-SQL
+      UPDATE spree_payment_methods
+      SET
+        type='#{Spree::PaymentMethod.name}',
+        type_before_removal='#{previous_type}',
+        active=#{false_value},
+        available_to_users=#{false_value},
+        available_to_admin=#{false_value}
+      WHERE id=#{id};
+    SQL
+    )
+  end
+end

core/spec/lib/tasks/payment_method_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'payment_method:deactivate_unsupported_payment_methods' do
+  include_context(
+    'rake',
+    task_name: 'payment_method:deactivate_unsupported_payment_methods',
+    task_path: Spree::Core::Engine.root.join('lib/tasks/payment_method.rake'),
+  )
+
+  let!(:unsupported_payment_method_id) { 0 }
+  let!(:unsupported_payment_method) { create(:payment_method, id: unsupported_payment_method_id) }
+  let!(:supported_payment_method) { create(:payment_method, id: 1) }
+
+  def unsupported_payment_method_reloaded
+    Spree::PaymentMethod.find_by(id: unsupported_payment_method_id)
+  end
+
+  before do
+    unsupported_payment_method.update type: 'UnsupportedPaymentMethod'
+  end
+
+  context "with an unsupported payment method" do
+    it "allows payment method records retrieval" do
+      task.invoke
+
+      expect {
+        Spree::PaymentMethod.find_by(id: unsupported_payment_method_id)
+      }.not_to raise_error
+    end
+  end
+
+  context "on an unsupported payment method" do
+    before { task.invoke }
+    subject { unsupported_payment_method_reloaded }
+
+    it "sets payment method type to 'Spree::PaymentMethod'" do
+      expect(subject.type).to eq 'Spree::PaymentMethod'
+    end
+
+    it "sets payment method type_before_removal correctly" do
+      expect(subject.type_before_removal).to eq 'UnsupportedPaymentMethod'
+    end
+
+    it "resets payment method active flag" do
+      expect(subject.active).to be false
+    end
+
+    it "resets payment method available_to_users flag" do
+      expect(subject.available_to_users).to be false
+    end
+
+    it "resets payment method available_to_admin flag" do
+      expect(subject.available_to_admin).to be false
+    end
+  end
+
+  context "on a supported payment method" do
+    it "does not change payment method attributes" do
+      expect { task.invoke }.not_to change { supported_payment_method.reload }
+    end
+  end
+end

core/spec/models/spree/payment_method_spec.rb

@@ -233,4 +233,20 @@ def gateway_class
       end
     end
   end
+
+  describe "#find_sti_class" do
+    context "with an unexisting type" do
+      context "while retrieving records" do
+        let!(:unsupported_payment_method) { create(:payment_method, type: 'UnsupportedPaymentMethod') }
+
+        it "raises an UnsupportedPaymentMethod error" do
+          expect { Spree::PaymentMethod.all.to_json }
+            .to raise_error(
+              Spree::PaymentMethod::UnsupportedPaymentMethod,
+              /Found invalid payment type 'UnsupportedPaymentMethod'/
+            )
+        end
+      end
+    end
+  end
 end

guides/source/developers/payments/payment-methods.html.md

@@ -30,6 +30,10 @@ service providers][payment-service-providers] article.
 - `available_to_users`: Determines if the payment method is visible to users.
 - `available_to_admin`: Determines if the payment method is visible to
   administrators.
+- `type_before_removal`: Contains the previous real payment type, in case `type` has 
+  been removed after switching Payment Service Provider. Defaults to `nil`. For more 
+  information, see the [Payment service providers][payment-service-providers] 
+  article.
 
 [payment-method-base]: https://github.com/solidusio/solidus/blob/master/core/app/models/spree/payment_method.rb
 [payment-service-providers]: payment-service-providers.html

guides/source/developers/payments/payment-service-providers.html.md

@@ -66,3 +66,16 @@ You would need to extend or rewrite this class with your preferred PSP
 integration.
 
 [credit-card-base]: https://github.com/solidusio/solidus/blob/master/core/app/models/spree/payment_method/credit_card.rb
+
+### Switching payment service provider
+
+After switching payment service provider, there may be `Spree::PaymentMethod` 
+records referencing a `type` class that does not exist anymore. Trying to 
+retrieve these records through an `ActiveRecord` query raises a 
+`Spree::PaymentMethod::UnsupportedPaymentMethod` error.
+
+If you cannot delete these records, you can deactivate them running 
+`rake payment_method:deactivate_unsupported_payment_methods`. 
+This way, their `type` will be set to `Spree::PaymentMethod`, allowing for 
+records retrieval without errors. Also, their real `type` value will be stored
+in the `type_before_removal` attribute.