Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve payment service providers switching errors #3837

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions core/app/models/spree/payment_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module Spree
#
class PaymentMethod < Spree::Base
include Spree::Preferences::Persistable
class UnsupportedPaymentMethod < StandardError; end
luca-landa marked this conversation as resolved.
Show resolved Hide resolved

preference :server, :string, default: 'test'
preference :test_mode, :boolean, default: true
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions core/lib/tasks/payment_method.rake
Original file line number Diff line number Diff line change
@@ -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
64 changes: 64 additions & 0 deletions core/spec/lib/tasks/payment_method_spec.rb
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions core/spec/models/spree/payment_method_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions guides/source/developers/payments/payment-methods.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
luca-landa marked this conversation as resolved.
Show resolved Hide resolved

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.

luca-landa marked this conversation as resolved.
Show resolved Hide resolved
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.