From b7ee1fc3dd4cf21789911565907889573ed2ae5c Mon Sep 17 00:00:00 2001 From: luca Date: Sun, 8 Nov 2020 17:36:10 +0100 Subject: [PATCH 1/3] Allow payment methods retrieval after PSP switch After switching payment service provider, previous payment method records still exist in spree_payment_methods table, referencing an unexisting class for STI. This generates ActiveRecord::SubclassNotFound errors while retrieving the records from the database. This commit adds a task that resets the type of unsupported payment methods to Spree::PaymentMethod, and records the old actual type in a new "type_before_removal" field. This allows to keep previous orders which still reference these unsupported payment methods. --- ...before_removal_to_spree_payment_methods.rb | 7 ++ core/lib/tasks/payment_method.rake | 29 +++++++++ core/spec/lib/tasks/payment_method_spec.rb | 64 +++++++++++++++++++ 3 files changed, 100 insertions(+) create mode 100644 core/db/migrate/20201127212108_add_type_before_removal_to_spree_payment_methods.rb create mode 100644 core/lib/tasks/payment_method.rake create mode 100644 core/spec/lib/tasks/payment_method_spec.rb diff --git a/core/db/migrate/20201127212108_add_type_before_removal_to_spree_payment_methods.rb b/core/db/migrate/20201127212108_add_type_before_removal_to_spree_payment_methods.rb new file mode 100644 index 00000000000..5b038517003 --- /dev/null +++ b/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 diff --git a/core/lib/tasks/payment_method.rake b/core/lib/tasks/payment_method.rake new file mode 100644 index 00000000000..b02650ed46d --- /dev/null +++ b/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 diff --git a/core/spec/lib/tasks/payment_method_spec.rb b/core/spec/lib/tasks/payment_method_spec.rb new file mode 100644 index 00000000000..9ea9b7310b6 --- /dev/null +++ b/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 From 719dbf5125ad3c49a38d4dd2c7b0a6fc12c7393c Mon Sep 17 00:00:00 2001 From: luca Date: Mon, 9 Nov 2020 21:24:10 +0100 Subject: [PATCH 2/3] Improve error on invalid payment methods retrieval Show a simple error after retrieving payment methods no more supported, suggesting to run a specific rake task to fix the issue and without having to delete those payment methods. --- core/app/models/spree/payment_method.rb | 11 +++++++++++ core/spec/models/spree/payment_method_spec.rb | 16 ++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb index 43c5b9540b2..c8b6e523c78 100644 --- a/core/app/models/spree/payment_method.rb +++ b/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 diff --git a/core/spec/models/spree/payment_method_spec.rb b/core/spec/models/spree/payment_method_spec.rb index 84fd9a62d15..9c377fe2fce 100644 --- a/core/spec/models/spree/payment_method_spec.rb +++ b/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 From a6a2d4457330aba68bece64eefd635e13aa7c046 Mon Sep 17 00:00:00 2001 From: luca Date: Mon, 9 Nov 2020 22:07:36 +0100 Subject: [PATCH 3/3] Update docs on payment methods deprecation --- .../developers/payments/payment-methods.html.md | 4 ++++ .../payments/payment-service-providers.html.md | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/guides/source/developers/payments/payment-methods.html.md b/guides/source/developers/payments/payment-methods.html.md index e55cc5ce77b..63004d77a3e 100644 --- a/guides/source/developers/payments/payment-methods.html.md +++ b/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 diff --git a/guides/source/developers/payments/payment-service-providers.html.md b/guides/source/developers/payments/payment-service-providers.html.md index 7310e8f8c5f..e1846b80ef1 100644 --- a/guides/source/developers/payments/payment-service-providers.html.md +++ b/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.