From d1082aff1b98d7aff4825f8a55cb4f4b567b45c0 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Mon, 29 May 2017 11:12:38 +0200 Subject: [PATCH 01/11] Rename bogus gateways into payment methods To remove confusion around payment methods, gateways and providers we rename the `Spree::Gateway::Bogus` and `Spree::Gateway::BogusSimple` payment methods into `Spree::PaymentMethod::BogusCreditCard` and `Spree::PaymentMethod::SimpleBogusCreditCard`. --- CHANGELOG.md | 4 ++++ .../spree/api/checkouts_controller_spec.rb | 2 +- .../spree/api/payments_controller_spec.rb | 10 +++++----- .../spree/admin/payment_methods_controller_spec.rb | 4 ++-- .../admin/configuration/payment_methods_spec.rb | 8 ++++---- .../bogus_credit_card.rb} | 2 +- .../simple_bogus_credit_card.rb} | 2 +- core/lib/spree/core/engine.rb | 4 ++-- .../factories/payment_method_factory.rb | 4 ++-- core/spec/models/spree/gateway_spec.rb | 2 +- core/spec/models/spree/order_capturing_spec.rb | 14 +++++++------- .../bogus_credit_card_spec.rb} | 2 +- .../simple_bogus_credit_card_spec.rb} | 4 ++-- core/spec/models/spree/payment_spec.rb | 2 +- .../reimbursement_type/original_payment_spec.rb | 2 +- sample/db/samples/payment_methods.rb | 2 +- 16 files changed, 36 insertions(+), 32 deletions(-) rename core/app/models/spree/{gateway/bogus.rb => payment_method/bogus_credit_card.rb} (98%) rename core/app/models/spree/{gateway/bogus_simple.rb => payment_method/simple_bogus_credit_card.rb} (92%) rename core/spec/models/spree/{gateway/bogus_spec.rb => payment_method/bogus_credit_card_spec.rb} (77%) rename core/spec/models/spree/{gateway/bogus_simple.rb => payment_method/simple_bogus_credit_card_spec.rb} (77%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e131df3475..5f0a8e9053b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Solidus 2.3.0 (master, unreleased) +- Renamed bogus payment methods [\#2000](https://github.com/solidusio/solidus/pull/2000) ([tvdeyen](https://github.com/tvdeyen)) + `Spree::Gateway::BogusSimple` and `Spree::Gateway::Bogus` were renamed into `Spree::PaymentMethod::SimpleBogusCreditCard` and `Spree::PaymentMethod::BogusCreditCard` + Run `rake solidus:migrations:rename_gateways:up` to migrate your data. + - Deprecate `Spree::Core::CurrentStore` in favor of `Spree::CurrentStoreSelector`. [\#1993](https://github.com/solidusio/solidus/pull/1993) - Deprecate `Spree::Order#assign_default_addresses!` in favor of `Order.new.assign_default_user_addresses`. [\#1954](https://github.com/solidusio/solidus/pull/1954) ([kennyadsl](https://github.com/kennyadsl)) - Change how line item options are allowed in line items controller. [\#1943](https://github.com/solidusio/solidus/pull/1943) diff --git a/api/spec/controllers/spree/api/checkouts_controller_spec.rb b/api/spec/controllers/spree/api/checkouts_controller_spec.rb index 0755fc809bd..764bd68dc8f 100644 --- a/api/spec/controllers/spree/api/checkouts_controller_spec.rb +++ b/api/spec/controllers/spree/api/checkouts_controller_spec.rb @@ -154,7 +154,7 @@ module Spree it "can update payment method and transition from payment to confirm" do order.update_column(:state, "payment") - allow_any_instance_of(Spree::Gateway::Bogus).to receive(:source_required?).and_return(false) + allow_any_instance_of(Spree::PaymentMethod::BogusCreditCard).to receive(:source_required?).and_return(false) api_put :update, id: order.to_param, order_token: order.guest_token, order: { payments_attributes: [{ payment_method_id: @payment_method.id }] } expect(json_response['state']).to eq('confirm') diff --git a/api/spec/controllers/spree/api/payments_controller_spec.rb b/api/spec/controllers/spree/api/payments_controller_spec.rb index 372af60cf47..6b63e5bf2f4 100644 --- a/api/spec/controllers/spree/api/payments_controller_spec.rb +++ b/api/spec/controllers/spree/api/payments_controller_spec.rb @@ -37,7 +37,7 @@ module Spree context "payment source is not required" do before do - allow_any_instance_of(Spree::Gateway::Bogus).to receive(:source_required?).and_return(false) + allow_any_instance_of(Spree::PaymentMethod::BogusCreditCard).to receive(:source_required?).and_return(false) end it "can create a new payment" do @@ -160,7 +160,7 @@ module Spree context "authorization fails" do before do fake_response = double(success?: false, to_s: "Could not authorize card") - expect_any_instance_of(Spree::Gateway::Bogus).to receive(:authorize).and_return(fake_response) + expect_any_instance_of(Spree::PaymentMethod::BogusCreditCard).to receive(:authorize).and_return(fake_response) api_put :authorize, id: payment.to_param end @@ -187,7 +187,7 @@ module Spree context "capturing fails" do before do fake_response = double(success?: false, to_s: "Insufficient funds") - expect_any_instance_of(Spree::Gateway::Bogus).to receive(:capture).and_return(fake_response) + expect_any_instance_of(Spree::PaymentMethod::BogusCreditCard).to receive(:capture).and_return(fake_response) end it "returns a 422 status" do @@ -209,7 +209,7 @@ module Spree context "purchasing fails" do before do fake_response = double(success?: false, to_s: "Insufficient funds") - expect_any_instance_of(Spree::Gateway::Bogus).to receive(:purchase).and_return(fake_response) + expect_any_instance_of(Spree::PaymentMethod::BogusCreditCard).to receive(:purchase).and_return(fake_response) end it "returns a 422 status" do @@ -231,7 +231,7 @@ module Spree context "voiding fails" do before do fake_response = double(success?: false, to_s: "NO REFUNDS") - expect_any_instance_of(Spree::Gateway::Bogus).to receive(:void).and_return(fake_response) + expect_any_instance_of(Spree::PaymentMethod::BogusCreditCard).to receive(:void).and_return(fake_response) end it "returns a 422 status" do diff --git a/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb index 0bbe044bb97..d09d931bad9 100644 --- a/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -28,13 +28,13 @@ class GatewayWithPassword < PaymentMethod context "tries to save invalid payment" do it "doesn't break, responds nicely" do - post :create, params: { payment_method: { name: "", type: "Spree::Gateway::Bogus" } } + post :create, params: { payment_method: { name: "", type: "Spree::PaymentMethod::BogusCreditCard" } } end end it "can create a payment method of a valid type" do expect { - post :create, params: { payment_method: { name: "Test Method", type: "Spree::Gateway::Bogus" } } + post :create, params: { payment_method: { name: "Test Method", type: "Spree::PaymentMethod::BogusCreditCard" } } }.to change(Spree::PaymentMethod, :count).by(1) expect(response).to be_redirect diff --git a/backend/spec/features/admin/configuration/payment_methods_spec.rb b/backend/spec/features/admin/configuration/payment_methods_spec.rb index b4fbce13544..a9d2c3179a5 100644 --- a/backend/spec/features/admin/configuration/payment_methods_spec.rb +++ b/backend/spec/features/admin/configuration/payment_methods_spec.rb @@ -67,7 +67,7 @@ context "changing type and payment_source", js: true do after do # cleanup - Spree::Config.static_model_preferences.for_class(Spree::Gateway::Bogus).clear + Spree::Config.static_model_preferences.for_class(Spree::PaymentMethod::BogusCreditCard).clear end it "displays message when changing type" do @@ -81,13 +81,13 @@ expect(page).to have_no_content('Test Mode') # change back - select2_search 'Spree::Gateway::Bogus', from: 'Provider' + select2_search 'Spree::PaymentMethod::BogusCreditCard', from: 'Provider' expect(page).to have_no_content('you must save first') expect(page).to have_content('Test Mode') end it "displays message when changing preference source" do - Spree::Config.static_model_preferences.add(Spree::Gateway::Bogus, 'my_prefs', {}) + Spree::Config.static_model_preferences.add(Spree::PaymentMethod::BogusCreditCard, 'my_prefs', {}) create(:credit_card_payment_method) click_link "Payment Methods" @@ -105,7 +105,7 @@ end it "updates successfully and keeps secrets" do - Spree::Config.static_model_preferences.add(Spree::Gateway::Bogus, 'my_prefs', { server: 'secret' }) + Spree::Config.static_model_preferences.add(Spree::PaymentMethod::BogusCreditCard, 'my_prefs', { server: 'secret' }) create(:credit_card_payment_method) click_link "Payment Methods" diff --git a/core/app/models/spree/gateway/bogus.rb b/core/app/models/spree/payment_method/bogus_credit_card.rb similarity index 98% rename from core/app/models/spree/gateway/bogus.rb rename to core/app/models/spree/payment_method/bogus_credit_card.rb index 7d1946801a8..127b7e61a0c 100644 --- a/core/app/models/spree/gateway/bogus.rb +++ b/core/app/models/spree/payment_method/bogus_credit_card.rb @@ -1,5 +1,5 @@ module Spree - class Gateway::Bogus < Gateway + class PaymentMethod::BogusCreditCard < Gateway TEST_VISA = ['4111111111111111', '4012888888881881', '4222222222222'] TEST_MC = ['5500000000000004', '5555555555554444', '5105105105105100'] TEST_AMEX = ['378282246310005', '371449635398431', '378734493671000', '340000000000009'] diff --git a/core/app/models/spree/gateway/bogus_simple.rb b/core/app/models/spree/payment_method/simple_bogus_credit_card.rb similarity index 92% rename from core/app/models/spree/gateway/bogus_simple.rb rename to core/app/models/spree/payment_method/simple_bogus_credit_card.rb index 568353967ae..10e3803f892 100644 --- a/core/app/models/spree/gateway/bogus_simple.rb +++ b/core/app/models/spree/payment_method/simple_bogus_credit_card.rb @@ -1,6 +1,6 @@ module Spree # Bogus Gateway that doesn't support payment profiles. - class Gateway::BogusSimple < Gateway::Bogus + class PaymentMethod::SimpleBogusCreditCard < PaymentMethod::BogusCreditCard def payment_profiles_supported? false end diff --git a/core/lib/spree/core/engine.rb b/core/lib/spree/core/engine.rb index f098b013fc7..d7e4bd81fd0 100644 --- a/core/lib/spree/core/engine.rb +++ b/core/lib/spree/core/engine.rb @@ -43,8 +43,8 @@ class Engine < ::Rails::Engine initializer "spree.register.payment_methods", before: :load_config_initializers do |app| app.config.spree.payment_methods = %w[ - Spree::Gateway::Bogus - Spree::Gateway::BogusSimple + Spree::PaymentMethod::BogusCreditCard + Spree::PaymentMethod::SimpleBogusCreditCard Spree::PaymentMethod::StoreCredit Spree::PaymentMethod::Check ] diff --git a/core/lib/spree/testing_support/factories/payment_method_factory.rb b/core/lib/spree/testing_support/factories/payment_method_factory.rb index ff84ef4a061..5f4ccd85d97 100644 --- a/core/lib/spree/testing_support/factories/payment_method_factory.rb +++ b/core/lib/spree/testing_support/factories/payment_method_factory.rb @@ -1,5 +1,5 @@ FactoryGirl.define do - factory :payment_method, aliases: [:credit_card_payment_method], class: Spree::Gateway::Bogus do + factory :payment_method, aliases: [:credit_card_payment_method], class: Spree::PaymentMethod::BogusCreditCard do name 'Credit Card' available_to_admin true available_to_users true @@ -13,7 +13,7 @@ # authorize.net was moved to spree_gateway. # Leaving this factory in place with bogus in case anyone is using it. - factory :simple_credit_card_payment_method, class: Spree::Gateway::BogusSimple do + factory :simple_credit_card_payment_method, class: Spree::PaymentMethod::SimpleBogusCreditCard do name 'Credit Card' available_to_admin true available_to_users true diff --git a/core/spec/models/spree/gateway_spec.rb b/core/spec/models/spree/gateway_spec.rb index 814aba26ca9..8b008a1e343 100644 --- a/core/spec/models/spree/gateway_spec.rb +++ b/core/spec/models/spree/gateway_spec.rb @@ -95,7 +95,7 @@ def provider_class end context 'using preference_source' do - let(:klass){ Spree::Gateway::Bogus } + let(:klass){ Spree::PaymentMethod::BogusCreditCard } before do Spree::Config.static_model_preferences.add(klass, 'test_preference_source', server: 'bar') end diff --git a/core/spec/models/spree/order_capturing_spec.rb b/core/spec/models/spree/order_capturing_spec.rb index efae69eda34..9d358bde196 100644 --- a/core/spec/models/spree/order_capturing_spec.rb +++ b/core/spec/models/spree/order_capturing_spec.rb @@ -58,10 +58,10 @@ context "payment method ordering" do let(:secondary_payment_method) { SecondaryBogusPaymentMethod } - class SecondaryBogusPaymentMethod < Spree::Gateway::Bogus; end + class SecondaryBogusPaymentMethod < Spree::PaymentMethod::BogusCreditCard; end context "SecondaryBogusPaymentMethod payments are prioritized" do - let(:payment_methods) { [SecondaryBogusPaymentMethod, Spree::Gateway::Bogus] } + let(:payment_methods) { [SecondaryBogusPaymentMethod, Spree::PaymentMethod::BogusCreditCard] } it "captures SecondaryBogusPaymentMethod payments first" do @bogus_payment.update!(amount: bogus_total + 100) @@ -72,7 +72,7 @@ class SecondaryBogusPaymentMethod < Spree::Gateway::Bogus; end end context "Bogus payments are prioritized" do - let(:payment_methods) { [Spree::Gateway::Bogus, SecondaryBogusPaymentMethod] } + let(:payment_methods) { [Spree::PaymentMethod::BogusCreditCard, SecondaryBogusPaymentMethod] } it "captures Bogus payments first" do @secondary_bogus_payment.update!(amount: secondary_total + 100) @@ -89,7 +89,7 @@ class SecondaryBogusPaymentMethod < Spree::Gateway::Bogus; end before do allow(Spree::OrderCapturing).to receive(:sorted_payment_method_classes).and_return( - [SecondaryBogusPaymentMethod, Spree::Gateway::Bogus] + [SecondaryBogusPaymentMethod, Spree::PaymentMethod::BogusCreditCard] ) end @@ -103,7 +103,7 @@ class SecondaryBogusPaymentMethod < Spree::Gateway::Bogus; end context "when a payment is not needed to capture the entire order" do let(:secondary_payment_method) { SecondaryBogusPaymentMethod } - let(:payment_methods) { [Spree::Gateway::Bogus, SecondaryBogusPaymentMethod] } + let(:payment_methods) { [Spree::PaymentMethod::BogusCreditCard, SecondaryBogusPaymentMethod] } before do @bogus_payment.update!(amount: order.total) @@ -132,9 +132,9 @@ class SecondaryBogusPaymentMethod < Spree::Gateway::Bogus; end let(:secondary_payment_method) { ExceptionallyBogusPaymentMethod } let(:bogus_total) { order.total - 1 } let(:secondary_total) { 1 } - let(:payment_methods) { [Spree::Gateway::Bogus, ExceptionallyBogusPaymentMethod] } + let(:payment_methods) { [Spree::PaymentMethod::BogusCreditCard, ExceptionallyBogusPaymentMethod] } - class ExceptionallyBogusPaymentMethod < Spree::Gateway::Bogus + class ExceptionallyBogusPaymentMethod < Spree::PaymentMethod::BogusCreditCard def capture(*_args) raise ActiveMerchant::ConnectionError.new("foo", nil) end diff --git a/core/spec/models/spree/gateway/bogus_spec.rb b/core/spec/models/spree/payment_method/bogus_credit_card_spec.rb similarity index 77% rename from core/spec/models/spree/gateway/bogus_spec.rb rename to core/spec/models/spree/payment_method/bogus_credit_card_spec.rb index e56887435c3..700c357e685 100644 --- a/core/spec/models/spree/gateway/bogus_spec.rb +++ b/core/spec/models/spree/payment_method/bogus_credit_card_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' module Spree - describe Gateway::Bogus, type: :model do + describe PaymentMethod::BogusCreditCard, type: :model do let(:bogus) { create(:credit_card_payment_method) } let!(:cc) { create(:credit_card, payment_method: bogus, gateway_customer_profile_id: "BGS-RERTERT") } end diff --git a/core/spec/models/spree/gateway/bogus_simple.rb b/core/spec/models/spree/payment_method/simple_bogus_credit_card_spec.rb similarity index 77% rename from core/spec/models/spree/gateway/bogus_simple.rb rename to core/spec/models/spree/payment_method/simple_bogus_credit_card_spec.rb index fb1423d9e9d..39bfab5d7ef 100644 --- a/core/spec/models/spree/gateway/bogus_simple.rb +++ b/core/spec/models/spree/payment_method/simple_bogus_credit_card_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe Spree::Gateway::BogusSimple, type: :model do - subject { Spree::Gateway::BogusSimple.new } +describe Spree::PaymentMethod::SimpleBogusCreditCard, type: :model do + subject { Spree::PaymentMethod::SimpleBogusCreditCard.new } # regression test for https://github.com/spree/spree/issues/3824 describe "#capture" do diff --git a/core/spec/models/spree/payment_spec.rb b/core/spec/models/spree/payment_spec.rb index 8d1bdbb3492..2f530c3fe39 100644 --- a/core/spec/models/spree/payment_spec.rb +++ b/core/spec/models/spree/payment_spec.rb @@ -6,7 +6,7 @@ let(:refund_reason) { create(:refund_reason) } let(:gateway) do - gateway = Spree::Gateway::Bogus.new(active: true, name: 'Bogus gateway') + gateway = Spree::PaymentMethod::BogusCreditCard.new(active: true, name: 'Bogus gateway') allow(gateway).to receive_messages source_required: true gateway end diff --git a/core/spec/models/spree/reimbursement_type/original_payment_spec.rb b/core/spec/models/spree/reimbursement_type/original_payment_spec.rb index e0a3d901667..f644363b4a7 100644 --- a/core/spec/models/spree/reimbursement_type/original_payment_spec.rb +++ b/core/spec/models/spree/reimbursement_type/original_payment_spec.rb @@ -54,7 +54,7 @@ module Spree context "multiple payment methods" do let(:simulate) { true } let!(:check_payment) { create(:check_payment, order: reimbursement.order, amount: 5.0, state: "completed") } - let(:payment) { reimbursement.order.payments.detect { |p| p.payment_method.is_a? Spree::Gateway::Bogus } } + let(:payment) { reimbursement.order.payments.detect { |p| p.payment_method.is_a? Spree::PaymentMethod::BogusCreditCard } } let(:refund_amount) { 10.0 } let(:refund_payment_methods) { subject.map { |refund| refund.payment.payment_method } } diff --git a/sample/db/samples/payment_methods.rb b/sample/db/samples/payment_methods.rb index 41434aedbeb..8c5ba88cd69 100644 --- a/sample/db/samples/payment_methods.rb +++ b/sample/db/samples/payment_methods.rb @@ -1,4 +1,4 @@ -Spree::Gateway::Bogus.create!( +Spree::PaymentMethod::BogusCreditCard.create!( { name: "Credit Card", description: "Bogus payment gateway", From 24521c40330a0d296ab594ab8d347033cd711af0 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 6 Jun 2017 09:57:12 +0200 Subject: [PATCH 02/11] Deprecate Gateway::Bogus classes Use `Spree:: PaymentMethod::BogusCreditCard` and `Spree::PaymentMethod::SimpleBogusCreditCard ` instead. --- core/app/models/spree/gateway/bogus.rb | 11 +++++++++++ core/app/models/spree/gateway/bogus_simple.rb | 11 +++++++++++ core/spec/models/spree/gateway/bogus_simple.rb | 12 ++++++++++++ core/spec/models/spree/gateway/bogus_spec.rb | 12 ++++++++++++ 4 files changed, 46 insertions(+) create mode 100644 core/app/models/spree/gateway/bogus.rb create mode 100644 core/app/models/spree/gateway/bogus_simple.rb create mode 100644 core/spec/models/spree/gateway/bogus_simple.rb create mode 100644 core/spec/models/spree/gateway/bogus_spec.rb diff --git a/core/app/models/spree/gateway/bogus.rb b/core/app/models/spree/gateway/bogus.rb new file mode 100644 index 00000000000..269704565e7 --- /dev/null +++ b/core/app/models/spree/gateway/bogus.rb @@ -0,0 +1,11 @@ +module Spree + # @deprecated Use Spree::PaymentMethod::BogusCreditCard instead + class Gateway::Bogus < PaymentMethod::BogusCreditCard + def initialize(*args) + Spree::Deprecation.warn \ + 'Spree::Gateway::Bogus is deprecated. ' \ + 'Please use Spree::PaymentMethod::BogusCreditCard instead' + super + end + end +end diff --git a/core/app/models/spree/gateway/bogus_simple.rb b/core/app/models/spree/gateway/bogus_simple.rb new file mode 100644 index 00000000000..3328cf0e7e4 --- /dev/null +++ b/core/app/models/spree/gateway/bogus_simple.rb @@ -0,0 +1,11 @@ +module Spree + # @deprecated Use Spree::PaymentMethod::SimpleBogusCreditCard instead + class Gateway::BogusSimple < Spree::PaymentMethod::SimpleBogusCreditCard + def initialize(*args) + Spree::Deprecation.warn \ + 'Spree::Gateway::BogusSimple is deprecated. ' \ + 'Please use Spree::PaymentMethod::SimpleBogusCreditCard instead' + super + end + end +end diff --git a/core/spec/models/spree/gateway/bogus_simple.rb b/core/spec/models/spree/gateway/bogus_simple.rb new file mode 100644 index 00000000000..d6105b15431 --- /dev/null +++ b/core/spec/models/spree/gateway/bogus_simple.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe Spree::Gateway::BogusSimple do + it 'is deprecated' do + expect(Spree::Deprecation).to receive(:warn) + described_class.new + end + + it 'has Spree::PaymentMethod::SimpleBogusCreditCard as superclass' do + expect(described_class.ancestors).to include(Spree::PaymentMethod::SimpleBogusCreditCard) + end +end diff --git a/core/spec/models/spree/gateway/bogus_spec.rb b/core/spec/models/spree/gateway/bogus_spec.rb new file mode 100644 index 00000000000..82cca16eba1 --- /dev/null +++ b/core/spec/models/spree/gateway/bogus_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe Spree::Gateway::Bogus do + it 'is deprecated' do + expect(Spree::Deprecation).to receive(:warn) + described_class.new + end + + it 'has Spree::PaymentMethod::BogusCreditCard as superclass' do + expect(described_class.ancestors).to include(Spree::PaymentMethod::BogusCreditCard) + end +end From 04dffbf0468e9f998f74c8a0fe7c7a190f764dbf Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 7 Jun 2017 09:48:29 +0200 Subject: [PATCH 03/11] Add Rake task to rename Bogus gateways Running rake solidus:migrations:rename_gateways:up helps migrating your data to new bogus payment method class names. rake solidus:migrations:rename_gateways:down reverts this. Also includes a migration that invokes that task, so you don't need to care when deploying this change. --- .../20170608074534_rename_bogus_gateways.rb | 13 +++++++ .../lib/solidus/migrations/rename_gateways.rb | 38 +++++++++++++++++++ .../lib/tasks/migrations/rename_gateways.rake | 19 ++++++++++ 3 files changed, 70 insertions(+) create mode 100644 core/db/migrate/20170608074534_rename_bogus_gateways.rb create mode 100644 core/lib/solidus/migrations/rename_gateways.rb create mode 100644 core/lib/tasks/migrations/rename_gateways.rake diff --git a/core/db/migrate/20170608074534_rename_bogus_gateways.rb b/core/db/migrate/20170608074534_rename_bogus_gateways.rb new file mode 100644 index 00000000000..7a3da21eb61 --- /dev/null +++ b/core/db/migrate/20170608074534_rename_bogus_gateways.rb @@ -0,0 +1,13 @@ +class RenameBogusGateways < ActiveRecord::Migration[5.0] + def up + say_with_time 'Renaming bogus gateways into payment methods' do + Rake::Task['solidus:migrations:rename_gateways:up'].invoke + end + end + + def down + say_with_time 'Renaming bogus payment methods into gateways' do + Rake::Task['solidus:migrations:rename_gateways:down'].invoke + end + end +end diff --git a/core/lib/solidus/migrations/rename_gateways.rb b/core/lib/solidus/migrations/rename_gateways.rb new file mode 100644 index 00000000000..fef103cb9e9 --- /dev/null +++ b/core/lib/solidus/migrations/rename_gateways.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Solidus + module Migrations + class RenameGateways + DEFAULT_MAPPING = { + 'Spree::Gateway::Bogus' => 'Spree::PaymentMethod::BogusCreditCard', + 'Spree::Gateway::BogusSimple' => 'Spree::PaymentMethod::SimpleBogusCreditCard' + } + + attr_reader :gateway_mapping + + def initialize(gateway_mapping = DEFAULT_MAPPING) + @gateway_mapping = gateway_mapping + end + + def up + gateway_mapping.inject(0) do |count, mapping| + count + update(from: mapping[0], to: mapping[1]) + end + end + + def down + gateway_mapping.inject(0) do |count, mapping| + count + update(from: mapping[1], to: mapping[0]) + end + end + + private + + def update(from:, to:) + ActiveRecord::Base.connection.update <<-SQL.strip_heredoc + UPDATE spree_payment_methods SET type = '#{to}' WHERE type = '#{from}'; + SQL + end + end + end +end diff --git a/core/lib/tasks/migrations/rename_gateways.rake b/core/lib/tasks/migrations/rename_gateways.rake new file mode 100644 index 00000000000..a62bed55a5e --- /dev/null +++ b/core/lib/tasks/migrations/rename_gateways.rake @@ -0,0 +1,19 @@ +require 'solidus/migrations/rename_gateways' + +namespace 'solidus:migrations:rename_gateways' do + task up: :environment do + count = Solidus::Migrations::RenameGateways.new.up + + unless ENV['VERBOSE'] == 'false' || !verbose + puts "Renamed #{count} gateways into payment methods." + end + end + + task down: :environment do + count = Solidus::Migrations::RenameGateways.new.down + + unless ENV['VERBOSE'] == 'false' || !verbose + puts "Renamed #{count} payment methods into gateways." + end + end +end From c974bcaee20bc5b0e4a9e4200ba448a1a94b9b36 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Mon, 29 May 2017 17:11:59 +0200 Subject: [PATCH 04/11] Move common methods from gateway to payment method `Spree::Gateway` holds many methods that actually belongs into its parent class - `Spree::PaymentMethod`. It rather should only hold commonalities that are relevant for credit card payment methods. All payment methods that are not a credit card should inherit directly from `Spree::PaymentMethod` - like `Spree::PaymentMethod::StoreCredit` and friends already does. --- core/app/models/spree/gateway.rb | 24 ---------- core/app/models/spree/payment_method.rb | 20 ++++++++- core/spec/models/spree/gateway_spec.rb | 45 ------------------- core/spec/models/spree/order_spec.rb | 8 ++-- core/spec/models/spree/payment_method_spec.rb | 45 +++++++++++++++++++ 5 files changed, 68 insertions(+), 74 deletions(-) diff --git a/core/app/models/spree/gateway.rb b/core/app/models/spree/gateway.rb index 35976afa7df..443ccdd0df7 100644 --- a/core/app/models/spree/gateway.rb +++ b/core/app/models/spree/gateway.rb @@ -4,34 +4,10 @@ module Spree # offically supported payment gateway implementations. # class Gateway < PaymentMethod - delegate :authorize, :purchase, :capture, :void, :credit, to: :provider - - validates :name, :type, presence: true - - preference :server, :string, default: 'test' - preference :test_mode, :boolean, default: true - def payment_source_class CreditCard end - def provider - gateway_options = options - gateway_options.delete :login if gateway_options.key?(:login) && gateway_options[:login].nil? - if gateway_options[:server] - ActiveMerchant::Billing::Base.mode = gateway_options[:server].to_sym - end - @provider ||= provider_class.new(gateway_options) - end - - def options - preferences.to_hash - end - - def payment_profiles_supported? - false - end - def method_type 'gateway' end diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb index d6d0d7d7901..d52ed513abe 100644 --- a/core/app/models/spree/payment_method.rb +++ b/core/app/models/spree/payment_method.rb @@ -2,11 +2,14 @@ module Spree # An abstract class which is implemented most commonly as a `Spree::Gateway`. # class PaymentMethod < Spree::Base + preference :server, :string, default: 'test' + preference :test_mode, :boolean, default: true + acts_as_paranoid acts_as_list DISPLAY = [:both, :front_end, :back_end] - validates :name, presence: true + validates :name, :type, presence: true has_many :payments, class_name: "Spree::Payment", inverse_of: :payment_method has_many :credit_cards, class_name: "Spree::CreditCard" @@ -22,6 +25,8 @@ class PaymentMethod < Spree::Base store.payment_methods.empty? ? all : where(id: store.payment_method_ids) end + delegate :authorize, :purchase, :capture, :void, :credit, to: :provider + include Spree::Preferences::StaticallyConfigurable def self.providers @@ -32,6 +37,19 @@ def provider_class raise ::NotImplementedError, "You must implement provider_class method for #{self.class}." end + def provider + gateway_options = options + gateway_options.delete :login if gateway_options.key?(:login) && gateway_options[:login].nil? + if gateway_options[:server] + ActiveMerchant::Billing::Base.mode = gateway_options[:server].to_sym + end + @gateway ||= provider_class.new(gateway_options) + end + + def options + preferences.to_hash + end + # The class that will process payments for this payment type, used for @payment.source # e.g. CreditCard in the case of a the Gateway payment type # nil means the payment method doesn't require a source e.g. check diff --git a/core/spec/models/spree/gateway_spec.rb b/core/spec/models/spree/gateway_spec.rb index 8b008a1e343..e7183d6c57b 100644 --- a/core/spec/models/spree/gateway_spec.rb +++ b/core/spec/models/spree/gateway_spec.rb @@ -1,51 +1,6 @@ require 'spec_helper' describe Spree::Gateway, type: :model do - class Provider - def initialize(options) - end - - def authorize; 'authorize'; end - - def purchase; 'purchase'; end - - def capture; 'capture'; end - - def void; 'void'; end - - def credit; 'credit'; end - end - - class TestGateway < Spree::Gateway - def provider_class - Provider - end - end - - describe 'ActiveMerchant methods' do - let(:gateway) { TestGateway.new } - - it "passes through authorize" do - expect(gateway.authorize).to eq 'authorize' - end - - it "passes through purchase" do - expect(gateway.purchase).to eq 'purchase' - end - - it "passes through capture" do - expect(gateway.capture).to eq 'capture' - end - - it "passes through void" do - expect(gateway.void).to eq 'void' - end - - it "passes through credit" do - expect(gateway.credit).to eq 'credit' - end - end - context "fetching payment sources" do let(:store) { create :store } let(:user) { create :user } diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index a688a247756..75c0c40bb81 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -523,7 +523,7 @@ def merge!(other_order, user = nil) # Regression test for https://github.com/spree/spree/issues/4199 context "#available_payment_methods" do it "includes frontend payment methods" do - payment_method = Spree::PaymentMethod.create!({ + payment_method = Spree::PaymentMethod::Check.create!({ name: "Fake", active: true, available_to_users: true, @@ -533,7 +533,7 @@ def merge!(other_order, user = nil) end it "includes 'both' payment methods" do - payment_method = Spree::PaymentMethod.create!({ + payment_method = Spree::PaymentMethod::Check.create!({ name: "Fake", active: true, available_to_users: true, @@ -543,7 +543,7 @@ def merge!(other_order, user = nil) end it "does not include a payment method twice" do - payment_method = Spree::PaymentMethod.create!({ + payment_method = Spree::PaymentMethod::Check.create!({ name: "Fake", active: true, available_to_users: true, @@ -554,7 +554,7 @@ def merge!(other_order, user = nil) end it "does not include inactive payment methods" do - Spree::PaymentMethod.create!({ + Spree::PaymentMethod::Check.create!({ name: "Fake", active: false, available_to_users: true, diff --git a/core/spec/models/spree/payment_method_spec.rb b/core/spec/models/spree/payment_method_spec.rb index 5072380513e..7091bf50ceb 100644 --- a/core/spec/models/spree/payment_method_spec.rb +++ b/core/spec/models/spree/payment_method_spec.rb @@ -309,4 +309,49 @@ def provider_class end end end + + describe 'ActiveMerchant methods' do + class Provider + def initialize(options) + end + + def authorize; 'authorize'; end + + def purchase; 'purchase'; end + + def capture; 'capture'; end + + def void; 'void'; end + + def credit; 'credit'; end + end + + class TestPaymentMethod < Spree::PaymentMethod + def provider_class + Provider + end + end + + let(:payment_method) { TestPaymentMethod.new } + + it "passes through authorize" do + expect(payment_method.authorize).to eq 'authorize' + end + + it "passes through purchase" do + expect(payment_method.purchase).to eq 'purchase' + end + + it "passes through capture" do + expect(payment_method.capture).to eq 'capture' + end + + it "passes through void" do + expect(payment_method.void).to eq 'void' + end + + it "passes through credit" do + expect(payment_method.credit).to eq 'credit' + end + end end From 1e9ce908712e6c7df17e883629052e5f8fd68f0d Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Mon, 29 May 2017 17:15:16 +0200 Subject: [PATCH 05/11] Rename PaymentMethod#provider to gateway In order to remove confusing parts of our current payment methods implementation we rename the `provider` methods from `Spree::PaymentMethod` into `gateway`. A gateway is a class that implements the API communication to the provider. See the `active_merchant` gem for a set of gateway classes we use in `solidus_gateway`. A provider on the other hand is a service vendor that offers a payment gateway and multiple payment methods (like Braintree). Therefore we plan to introduce a provider class later on. This then holds commonalities for mulitple payment methods, like credentials. Also it will be an ActiveRecord model so that payment methods can be grouped under one provider, which is very useful for BI and reporting. --- core/app/models/spree/billing_integration.rb | 4 +- core/app/models/spree/gateway.rb | 4 +- core/app/models/spree/payment_method.rb | 40 +++++++++++++++---- .../spree/payment_method/bogus_credit_card.rb | 2 +- core/spec/models/spree/payment_method_spec.rb | 8 ++-- core/spec/models/spree/payment_spec.rb | 4 +- 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/core/app/models/spree/billing_integration.rb b/core/app/models/spree/billing_integration.rb index 53287742af2..0c1018d5b03 100644 --- a/core/app/models/spree/billing_integration.rb +++ b/core/app/models/spree/billing_integration.rb @@ -5,11 +5,11 @@ class BillingIntegration < PaymentMethod preference :server, :string, default: 'test' preference :test_mode, :boolean, default: true - def provider + def gateway integration_options = options ActiveMerchant::Billing::Base.integration_mode = integration_options[:server].to_sym integration_options[:test] = true if integration_options[:test_mode] - @provider ||= provider_class.new(integration_options) + @gateway ||= gateway_class.new(integration_options) end def options diff --git a/core/app/models/spree/gateway.rb b/core/app/models/spree/gateway.rb index 443ccdd0df7..3783640617c 100644 --- a/core/app/models/spree/gateway.rb +++ b/core/app/models/spree/gateway.rb @@ -13,8 +13,8 @@ def method_type end def supports?(source) - return true unless provider_class.respond_to? :supports? - return true if source.brand && provider_class.supports?(source.brand) + return true unless gateway_class.respond_to? :supports? + return true if source.brand && gateway_class.supports?(source.brand) source.has_payment_profile? end diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb index d52ed513abe..2923cb46492 100644 --- a/core/app/models/spree/payment_method.rb +++ b/core/app/models/spree/payment_method.rb @@ -25,7 +25,7 @@ class PaymentMethod < Spree::Base store.payment_methods.empty? ? all : where(id: store.payment_method_ids) end - delegate :authorize, :purchase, :capture, :void, :credit, to: :provider + delegate :authorize, :purchase, :capture, :void, :credit, to: :gateway include Spree::Preferences::StaticallyConfigurable @@ -33,18 +33,28 @@ def self.providers Rails.application.config.spree.payment_methods end - def provider_class - raise ::NotImplementedError, "You must implement provider_class method for #{self.class}." - end - - def provider + # Represents the gateway of this payment method + # + # The gateway is responsible for communicating with the providers API. + # + # It implements methods for: + # + # - authorize + # - purchase + # - capture + # - void + # - credit + # + def gateway gateway_options = options gateway_options.delete :login if gateway_options.key?(:login) && gateway_options[:login].nil? if gateway_options[:server] ActiveMerchant::Billing::Base.mode = gateway_options[:server].to_sym end - @gateway ||= provider_class.new(gateway_options) + @gateway ||= gateway_class.new(gateway_options) end + alias_method :provider, :gateway + deprecate provider: :gateway, deprecator: Spree::Deprecation def options preferences.to_hash @@ -143,5 +153,21 @@ def cancel(_response) def store_credit? is_a? Spree::PaymentMethod::StoreCredit end + + protected + + # Represents the gateway class of this payment method + # + def gateway_class + if respond_to? :provider_class + Spree::Deprecation.warn \ + "provider_class is deprecated and will be removed from Solidus 3.0 " \ + "(use gateway_class instead)" + public_send :provider_class + else + raise ::NotImplementedError, "You must implement gateway_class method for #{self.class}." + end + end + deprecate provider_class: :gateway_class, deprecator: Spree::Deprecation end end diff --git a/core/app/models/spree/payment_method/bogus_credit_card.rb b/core/app/models/spree/payment_method/bogus_credit_card.rb index 127b7e61a0c..d9bcdc33bd4 100644 --- a/core/app/models/spree/payment_method/bogus_credit_card.rb +++ b/core/app/models/spree/payment_method/bogus_credit_card.rb @@ -9,7 +9,7 @@ class PaymentMethod::BogusCreditCard < Gateway attr_accessor :test - def provider_class + def gateway_class self.class end diff --git a/core/spec/models/spree/payment_method_spec.rb b/core/spec/models/spree/payment_method_spec.rb index 7091bf50ceb..eca67b6b2e5 100644 --- a/core/spec/models/spree/payment_method_spec.rb +++ b/core/spec/models/spree/payment_method_spec.rb @@ -181,7 +181,7 @@ describe '#auto_capture?' do class TestGateway < Spree::Gateway - def provider_class + def gateway_class Provider end end @@ -311,7 +311,7 @@ def provider_class end describe 'ActiveMerchant methods' do - class Provider + class PaymentGateway def initialize(options) end @@ -327,8 +327,8 @@ def credit; 'credit'; end end class TestPaymentMethod < Spree::PaymentMethod - def provider_class - Provider + def gateway_class + PaymentGateway end end diff --git a/core/spec/models/spree/payment_spec.rb b/core/spec/models/spree/payment_spec.rb index 2f530c3fe39..04dfb6307b7 100644 --- a/core/spec/models/spree/payment_spec.rb +++ b/core/spec/models/spree/payment_spec.rb @@ -209,7 +209,7 @@ # Regression test for https://github.com/spree/spree/issues/4598 it "should allow payments with a gateway_customer_profile_id" do payment.source.update!(gateway_customer_profile_id: "customer_1", brand: 'visa') - expect(payment.payment_method.provider_class).to receive(:supports?).with('visa').and_return(false) + expect(payment.payment_method.gateway_class).to receive(:supports?).with('visa').and_return(false) expect(payment).to receive(:started_processing!) payment.process! end @@ -217,7 +217,7 @@ # Another regression test for https://github.com/spree/spree/issues/4598 it "should allow payments with a gateway_payment_profile_id" do payment.source.update!(gateway_payment_profile_id: "customer_1", brand: 'visa') - expect(payment.payment_method.provider_class).to receive(:supports?).with('visa').and_return(false) + expect(payment.payment_method.gateway_class).to receive(:supports?).with('visa').and_return(false) expect(payment).to receive(:started_processing!) payment.process! end From 12db4a9cbe0ec35d82b60350c5b75cad7cd467b5 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Mon, 29 May 2017 17:25:51 +0200 Subject: [PATCH 06/11] Add documentation to PaymentMethod class This corrects the documentation on the Spree::PaymentMethod class. --- core/app/models/spree/payment_method.rb | 44 ++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb index 2923cb46492..6799bfe6b0a 100644 --- a/core/app/models/spree/payment_method.rb +++ b/core/app/models/spree/payment_method.rb @@ -1,5 +1,13 @@ module Spree - # An abstract class which is implemented most commonly as a `Spree::Gateway`. + # A base class which is used for implementing payment methods. + # + # See https://github.com/solidusio/solidus_gateway/ for + # offically supported payment method implementations. + # + # Uses STI (single table inheritance) to store all implemented payment methods + # in one table (+spree_payment_methods+). + # + # This class is not ment to be instantiated. Please create instances of concrete payment methods. # class PaymentMethod < Spree::Base preference :server, :string, default: 'test' @@ -56,13 +64,20 @@ def gateway alias_method :provider, :gateway deprecate provider: :gateway, deprecator: Spree::Deprecation + # Represents all preferences as a Hash + # + # Each preference is a key holding the value(s) and gets passed to the gateway via +gateway_options+ + # + # @return Hash def options preferences.to_hash end - # The class that will process payments for this payment type, used for @payment.source - # e.g. CreditCard in the case of a the Gateway payment type - # nil means the payment method doesn't require a source e.g. check + # The class that will store payment sources (re)usable with this payment method + # + # Used by Spree::Payment as source (e.g. Spree::CreditCard in the case of a credit card payment method). + # + # Returning nil means the payment method doesn't support storing sources (e.g. Spree::PaymentMethod::Check) def payment_source_class raise ::NotImplementedError, "You must implement payment_source_class method for #{self.class}." end @@ -116,6 +131,22 @@ def self.active? where(type: to_s, active: true).count > 0 end + # Used as partial name for your payment method + # + # Currently your payment method needs to provide these partials: + # + # 1. app/views/spree/checkout/payment/_{method_type}.html.erb + # The form your customer enters the payment information in during checkout + # + # 2. app/views/spree/checkout/existing_payment/_{method_type}.html.erb + # The payment information of your customers resuable sources during checkout + # + # 3. app/views/spree/admin/payments/source_forms/_{method_type}.html.erb + # The form an admin enters payment information in when creating orders in the backend + # + # 4. app/views/spree/admin/payments/source_views/_{method_type}.html.erb + # The view that represents your payment method on orders in the backend + # def method_type type.demodulize.downcase end @@ -142,6 +173,11 @@ def auto_capture? auto_capture.nil? ? Spree::Config[:auto_capture] : auto_capture end + # Check if given source is supported by this payment method + # + # Please implement validation logic in your payment method implementation + # + # @see Spree::Gateway#supports? def supports?(_source) true end From 5e15d82b3e1a059fde4be97bc569896281094378 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 31 May 2017 11:14:54 +0200 Subject: [PATCH 07/11] Move PaymentMethod class_methods to top of class Moves all `Spree::PaymentMethod` class methods to top of class instead of having them distributed all over the file. --- core/app/models/spree/payment_method.rb | 66 +++++++++++++------------ 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb index 6799bfe6b0a..fa4e5835ecd 100644 --- a/core/app/models/spree/payment_method.rb +++ b/core/app/models/spree/payment_method.rb @@ -37,8 +37,40 @@ class PaymentMethod < Spree::Base include Spree::Preferences::StaticallyConfigurable - def self.providers - Rails.application.config.spree.payment_methods + class << self + def providers + Rails.application.config.spree.payment_methods + end + + def available(display_on = nil, store: nil) + Spree::Deprecation.warn "Spree::PaymentMethod.available is deprecated."\ + "Please use .active, .available_to_users, and .available_to_admin scopes instead."\ + "For payment methods associated with a specific store, use Spree::PaymentMethod.available_to_store(your_store)"\ + " as the base applying any further filtering" + + display_on = display_on.to_s + + available_payment_methods = + case display_on + when 'front_end' + active.available_to_users + when 'back_end' + active.available_to_admin + else + active.available_to_users.available_to_admin + end + available_payment_methods.select do |p| + store.nil? || store.payment_methods.empty? || store.payment_methods.include?(p) + end + end + + def active? + where(type: to_s, active: true).count > 0 + end + + def find_with_destroyed(*args) + unscoped { find(*args) } + end end # Represents the gateway of this payment method @@ -105,32 +137,6 @@ def display_on end end - def self.available(display_on = nil, store: nil) - Spree::Deprecation.warn "Spree::PaymentMethod.available is deprecated."\ - "Please use .active, .available_to_users, and .available_to_admin scopes instead."\ - "For payment methods associated with a specific store, use Spree::PaymentMethod.available_to_store(your_store)"\ - " as the base applying any further filtering" - - display_on = display_on.to_s - - available_payment_methods = - case display_on - when 'front_end' - active.available_to_users - when 'back_end' - active.available_to_admin - else - active.available_to_users.available_to_admin - end - available_payment_methods.select do |p| - store.nil? || store.payment_methods.empty? || store.payment_methods.include?(p) - end - end - - def self.active? - where(type: to_s, active: true).count > 0 - end - # Used as partial name for your payment method # # Currently your payment method needs to provide these partials: @@ -151,10 +157,6 @@ def method_type type.demodulize.downcase end - def self.find_with_destroyed(*args) - unscoped { find(*args) } - end - def payment_profiles_supported? false end From 858e8c056f606d30403aab8c054fb1af4e6e2e69 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 31 May 2017 11:23:54 +0200 Subject: [PATCH 08/11] Rename Gateway into PaymentMethod::CreditCard The `Spree::Gateway` is an implementation of a credit card payment method. The current name is confusing. With moving this class into the `PaymentMethod` namespace and changing its name to `CreditCard` it is now more obvious what this class actually is. Several hints inside the class prove that: - It inherits from `Spree::PaymentMethod` - The `source_class` is `Spree::CreditCard` - In `supports?` it asks the gateway if it supports a certain credit card brand Further actions are required. First on foremost `solidus_gateway` needs to be changed in a way that all credit card based payment methods inherit from `Spree::PaymentMethod::CreditCard` from now on and all non credit card payment methods inherit from `Spree::PaymentMethod` directly. --- CHANGELOG.md | 3 +++ .../models/spree/{gateway.rb => payment_method/credit_card.rb} | 2 +- core/lib/solidus/migrations/rename_gateways.rb | 1 + .../{gateway_spec.rb => payment_method/credit_card_spec.rb} | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) rename core/app/models/spree/{gateway.rb => payment_method/credit_card.rb} (95%) rename core/spec/models/spree/{gateway_spec.rb => payment_method/credit_card_spec.rb} (97%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f0a8e9053b..1413f510f79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Solidus 2.3.0 (master, unreleased) +- Renamed `Spree::Gateway` payment method into `Spree::PaymentMethod::CreditCard` [\#2001](https://github/com/solidusio/solidus/pull/2001) ([tvdeyen](https://github.com/tvdeyen)) + Run `rake solidus:migrations:rename_gateways:up` to migrate your data. + - Renamed bogus payment methods [\#2000](https://github.com/solidusio/solidus/pull/2000) ([tvdeyen](https://github.com/tvdeyen)) `Spree::Gateway::BogusSimple` and `Spree::Gateway::Bogus` were renamed into `Spree::PaymentMethod::SimpleBogusCreditCard` and `Spree::PaymentMethod::BogusCreditCard` Run `rake solidus:migrations:rename_gateways:up` to migrate your data. diff --git a/core/app/models/spree/gateway.rb b/core/app/models/spree/payment_method/credit_card.rb similarity index 95% rename from core/app/models/spree/gateway.rb rename to core/app/models/spree/payment_method/credit_card.rb index 3783640617c..01e1ee6c48e 100644 --- a/core/app/models/spree/gateway.rb +++ b/core/app/models/spree/payment_method/credit_card.rb @@ -3,7 +3,7 @@ module Spree # base for extension. See https://github.com/solidusio/solidus_gateway/ for # offically supported payment gateway implementations. # - class Gateway < PaymentMethod + class PaymentMethod::CreditCard < PaymentMethod def payment_source_class CreditCard end diff --git a/core/lib/solidus/migrations/rename_gateways.rb b/core/lib/solidus/migrations/rename_gateways.rb index fef103cb9e9..2655597356e 100644 --- a/core/lib/solidus/migrations/rename_gateways.rb +++ b/core/lib/solidus/migrations/rename_gateways.rb @@ -4,6 +4,7 @@ module Solidus module Migrations class RenameGateways DEFAULT_MAPPING = { + 'Spree::Gateway' => 'Spree::PaymentMethod::CreditCard', 'Spree::Gateway::Bogus' => 'Spree::PaymentMethod::BogusCreditCard', 'Spree::Gateway::BogusSimple' => 'Spree::PaymentMethod::SimpleBogusCreditCard' } diff --git a/core/spec/models/spree/gateway_spec.rb b/core/spec/models/spree/payment_method/credit_card_spec.rb similarity index 97% rename from core/spec/models/spree/gateway_spec.rb rename to core/spec/models/spree/payment_method/credit_card_spec.rb index e7183d6c57b..ad8bced9694 100644 --- a/core/spec/models/spree/gateway_spec.rb +++ b/core/spec/models/spree/payment_method/credit_card_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Spree::Gateway, type: :model do +describe Spree::PaymentMethod::CreditCard, type: :model do context "fetching payment sources" do let(:store) { create :store } let(:user) { create :user } From 83b823b2f2fe063bf329f6803cc726c2930d694d Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 31 May 2017 11:34:58 +0200 Subject: [PATCH 09/11] Deprecate Spree::Gateway Use `Spree::PaymentMethod::CreditCard` instead. --- core/README.md | 4 ++-- core/app/models/spree/gateway.rb | 12 ++++++++++++ core/app/models/spree/payment_method.rb | 2 +- .../models/spree/payment_method/bogus_credit_card.rb | 2 +- core/spec/models/spree/gateway_spec.rb | 12 ++++++++++++ core/spec/models/spree/payment_method_spec.rb | 2 +- core/spec/support/test_gateway.rb | 2 +- 7 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 core/app/models/spree/gateway.rb create mode 100644 core/spec/models/spree/gateway_spec.rb diff --git a/core/README.md b/core/README.md index da3208ea11d..aee281ba964 100644 --- a/core/README.md +++ b/core/README.md @@ -39,8 +39,8 @@ integration. source (e.g. `Spree::CreditCard`) using a specific payment method (e.g `Solidus::Gateway::Braintree`). * `Spree::PaymentMethod` - An abstract class which is implemented most commonly -as a `Spree::Gateway`. -* `Spree::Gateway` - A concrete implementation of `Spree::PaymentMethod` +as a `Spree::PaymentMethod::CreditCard`. +* `Spree::PaymentMethod::CreditCard` - A concrete implementation of `Spree::PaymentMethod` intended to provide a base for extension. See https://github.com/solidusio/solidus_gateway/ for offically supported payment gateway implementations. diff --git a/core/app/models/spree/gateway.rb b/core/app/models/spree/gateway.rb new file mode 100644 index 00000000000..ac02cc262fc --- /dev/null +++ b/core/app/models/spree/gateway.rb @@ -0,0 +1,12 @@ +module Spree + # @deprecated Use Spree::PaymentMethod::CreditCard or Spree::PaymentMethod instead + class Gateway < PaymentMethod::CreditCard + def initialize(*args) + Spree::Deprecation.warn \ + "Using Spree::Gateway as parent class of payment methods is deprecated. " \ + "Please use Spree::PaymentMethod::CreditCard for credit card based payment methods " \ + "or Spree::PaymentMethod for non credit card payment methods instead." + super + end + end +end diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb index fa4e5835ecd..8ae03ab5008 100644 --- a/core/app/models/spree/payment_method.rb +++ b/core/app/models/spree/payment_method.rb @@ -179,7 +179,7 @@ def auto_capture? # # Please implement validation logic in your payment method implementation # - # @see Spree::Gateway#supports? + # @see Spree::PaymentMethod::CreditCard#supports? def supports?(_source) true end diff --git a/core/app/models/spree/payment_method/bogus_credit_card.rb b/core/app/models/spree/payment_method/bogus_credit_card.rb index d9bcdc33bd4..4c374f73a93 100644 --- a/core/app/models/spree/payment_method/bogus_credit_card.rb +++ b/core/app/models/spree/payment_method/bogus_credit_card.rb @@ -1,5 +1,5 @@ module Spree - class PaymentMethod::BogusCreditCard < Gateway + class PaymentMethod::BogusCreditCard < PaymentMethod::CreditCard TEST_VISA = ['4111111111111111', '4012888888881881', '4222222222222'] TEST_MC = ['5500000000000004', '5555555555554444', '5105105105105100'] TEST_AMEX = ['378282246310005', '371449635398431', '378734493671000', '340000000000009'] diff --git a/core/spec/models/spree/gateway_spec.rb b/core/spec/models/spree/gateway_spec.rb new file mode 100644 index 00000000000..f2c9c64f6af --- /dev/null +++ b/core/spec/models/spree/gateway_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe Spree::Gateway do + it 'is deprecated' do + expect(Spree::Deprecation).to receive(:warn) + described_class.new + end + + it 'has Spree::PaymentMethod::CreditCard as superclass' do + expect(described_class.ancestors).to include(Spree::PaymentMethod::CreditCard) + end +end diff --git a/core/spec/models/spree/payment_method_spec.rb b/core/spec/models/spree/payment_method_spec.rb index eca67b6b2e5..4b21d210697 100644 --- a/core/spec/models/spree/payment_method_spec.rb +++ b/core/spec/models/spree/payment_method_spec.rb @@ -180,7 +180,7 @@ end describe '#auto_capture?' do - class TestGateway < Spree::Gateway + class TestGateway < Spree::PaymentMethod::CreditCard def gateway_class Provider end diff --git a/core/spec/support/test_gateway.rb b/core/spec/support/test_gateway.rb index f6cf9ea0a00..75c9524c846 100644 --- a/core/spec/support/test_gateway.rb +++ b/core/spec/support/test_gateway.rb @@ -1,2 +1,2 @@ -class Spree::Gateway::Test < Spree::Gateway +class Spree::Gateway::Test < Spree::PaymentMethod::CreditCard end From 01df1bcc7af1a51accfb46b3421302faa01dacd7 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 31 May 2017 13:36:04 +0200 Subject: [PATCH 10/11] Update documentation of PaymentMethod::CreditCard --- core/app/models/spree/payment_method.rb | 4 ++-- core/app/models/spree/payment_method/credit_card.rb | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/core/app/models/spree/payment_method.rb b/core/app/models/spree/payment_method.rb index 8ae03ab5008..db5abe7a3f8 100644 --- a/core/app/models/spree/payment_method.rb +++ b/core/app/models/spree/payment_method.rb @@ -7,7 +7,7 @@ module Spree # Uses STI (single table inheritance) to store all implemented payment methods # in one table (+spree_payment_methods+). # - # This class is not ment to be instantiated. Please create instances of concrete payment methods. + # This class is not meant to be instantiated. Please create instances of concrete payment methods. # class PaymentMethod < Spree::Base preference :server, :string, default: 'test' @@ -145,7 +145,7 @@ def display_on # The form your customer enters the payment information in during checkout # # 2. app/views/spree/checkout/existing_payment/_{method_type}.html.erb - # The payment information of your customers resuable sources during checkout + # The payment information of your customers reusable sources during checkout # # 3. app/views/spree/admin/payments/source_forms/_{method_type}.html.erb # The form an admin enters payment information in when creating orders in the backend diff --git a/core/app/models/spree/payment_method/credit_card.rb b/core/app/models/spree/payment_method/credit_card.rb index 01e1ee6c48e..31187e84ab1 100644 --- a/core/app/models/spree/payment_method/credit_card.rb +++ b/core/app/models/spree/payment_method/credit_card.rb @@ -1,7 +1,10 @@ module Spree - # A concrete implementation of `Spree::PaymentMethod` intended to provide a - # base for extension. See https://github.com/solidusio/solidus_gateway/ for - # offically supported payment gateway implementations. + # An implementation of a `Spree::PaymentMethod` for credit card payments. + # + # It's a good candidate as base class for other credit card based payment methods. + # + # See https://github.com/solidusio/solidus_gateway/ for + # officially supported payment method implementations. # class PaymentMethod::CreditCard < PaymentMethod def payment_source_class From d39c1e91de4f93aeb493b52edb2117bb026805e9 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 31 May 2017 13:40:50 +0200 Subject: [PATCH 11/11] Update The Payment Sub-System section of README --- core/README.md | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/core/README.md b/core/README.md index aee281ba964..f048031a5d3 100644 --- a/core/README.md +++ b/core/README.md @@ -38,13 +38,10 @@ integration. * `Spree::Payment` - Manage and process a payment for an order, from a specific source (e.g. `Spree::CreditCard`) using a specific payment method (e.g `Solidus::Gateway::Braintree`). -* `Spree::PaymentMethod` - An abstract class which is implemented most commonly -as a `Spree::PaymentMethod::CreditCard`. -* `Spree::PaymentMethod::CreditCard` - A concrete implementation of `Spree::PaymentMethod` -intended to provide a base for extension. See -https://github.com/solidusio/solidus_gateway/ for offically supported payment -gateway implementations. -* `Spree::CreditCard` - The default `source` of a `Spree::Payment`. +* `Spree::PaymentMethod` - A base class which is used for implementing payment methods. +* `Spree::PaymentMethod::CreditCard` - An implementation of a `Spree::PaymentMethod` for credit card payments. +See https://github.com/solidusio/solidus_gateway/ for officially supported payment method implementations. +* `Spree::CreditCard` - The `source` of a `Spree::Payment` using `Spree::PaymentMethod::CreditCard` as payment method. ## The Inventory Sub-System * `Spree::ReturnAuthorization` - Models the return of Inventory Units to