From abaa66cc14f13b85661f726cbc9a95046f859000 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 13:06:16 +0100 Subject: [PATCH 1/9] Bring models from spree_core --- app/models/spree/credit_card.rb | 115 +++++++++++++ app/models/spree/gateway.rb | 54 +++++++ app/models/spree/gateway/bogus.rb | 85 ++++++++++ app/models/spree/gateway/bogus_simple.rb | 26 +++ app/models/spree/payment_method.rb | 62 +++++++ app/models/spree/payment_method/check.rb | 29 ++++ spec/models/spree/credit_card_spec.rb | 197 +++++++++++++++++++++++ spec/models/spree/gateway_spec.rb | 24 +++ spec/models/spree/payment_method_spec.rb | 33 ++++ 9 files changed, 625 insertions(+) create mode 100644 app/models/spree/credit_card.rb create mode 100644 app/models/spree/gateway.rb create mode 100644 app/models/spree/gateway/bogus.rb create mode 100644 app/models/spree/gateway/bogus_simple.rb create mode 100644 app/models/spree/payment_method.rb create mode 100644 app/models/spree/payment_method/check.rb create mode 100644 spec/models/spree/gateway_spec.rb diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb new file mode 100644 index 0000000000..21bc709ec1 --- /dev/null +++ b/app/models/spree/credit_card.rb @@ -0,0 +1,115 @@ +module Spree + class CreditCard < ActiveRecord::Base + has_many :payments, as: :source + + before_save :set_last_digits + + attr_accessor :number, :verification_value + + validates :month, :year, numericality: { only_integer: true } + validates :number, presence: true, unless: :has_payment_profile?, on: :create + validates :verification_value, presence: true, unless: :has_payment_profile?, on: :create + validate :expiry_not_in_the_past + + scope :with_payment_profile, -> { where('gateway_customer_profile_id IS NOT NULL') } + + # needed for some of the ActiveMerchant gateways (eg. SagePay) + alias_attribute :brand, :cc_type + + def expiry=(expiry) + self[:month], self[:year] = expiry.split(" / ") + self[:year] = "20" + self[:year] + end + + def number=(num) + @number = num.gsub(/[^0-9]/, '') rescue nil + end + + # cc_type is set by jquery.payment, which helpfully provides different + # types from Active Merchant. Converting them is necessary. + def cc_type=(type) + real_type = case type + when 'mastercard', 'maestro' + 'master' + when 'amex' + 'american_express' + when 'dinersclub' + 'diners_club' + else + type + end + self[:cc_type] = real_type + end + + def set_last_digits + number.to_s.gsub!(/\s/,'') + verification_value.to_s.gsub!(/\s/,'') + self.last_digits ||= number.to_s.length <= 4 ? number : number.to_s.slice(-4..-1) + end + + def name? + first_name? && last_name? + end + + def name + "#{first_name} #{last_name}" + end + + def verification_value? + verification_value.present? + end + + # Show the card number, with all but last 4 numbers replace with "X". (XXXX-XXXX-XXXX-4338) + def display_number + "XXXX-XXXX-XXXX-#{last_digits}" + end + + def actions + %w{capture void credit} + end + + # Indicates whether its possible to capture the payment + def can_capture?(payment) + payment.pending? || payment.checkout? + end + + # Indicates whether its possible to void the payment. + def can_void?(payment) + !payment.void? + end + + # Indicates whether its possible to credit the payment. Note that most gateways require that the + # payment be settled first which generally happens within 12-24 hours of the transaction. + def can_credit?(payment) + return false unless payment.completed? + return false unless payment.order.payment_state == 'credit_owed' + payment.credit_allowed > 0 + end + + def has_payment_profile? + gateway_customer_profile_id.present? + end + + def to_active_merchant + ActiveMerchant::Billing::CreditCard.new( + :number => number, + :month => month, + :year => year, + :verification_value => verification_value, + :first_name => first_name, + :last_name => last_name + ) + end + + private + + def expiry_not_in_the_past + if year.present? && month.present? + time = "#{year}-#{month}-1".to_time + if time < Time.zone.now.to_time.beginning_of_month + errors.add(:base, :card_expired) + end + end + end + end +end diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb new file mode 100644 index 0000000000..4e1cbaa368 --- /dev/null +++ b/app/models/spree/gateway.rb @@ -0,0 +1,54 @@ +module Spree + class Gateway < PaymentMethod + delegate_belongs_to :provider, :authorize, :purchase, :capture, :void, :credit + + validates :name, :type, presence: true + + preference :server, :string, default: 'test' + preference :test_mode, :boolean, default: true + + def payment_source_class + CreditCard + end + + # instantiates the selected gateway and configures with the options stored in the database + def self.current + super + end + + def provider + gateway_options = options + gateway_options.delete :login if gateway_options.has_key?(:login) and gateway_options[:login].nil? + if gateway_options[:server] + ActiveMerchant::Billing::Base.gateway_mode = gateway_options[:server].to_sym + end + @provider ||= provider_class.new(gateway_options) + end + + def options + self.preferences.inject({}){ |memo, (key, value)| memo[key.to_sym] = value; memo } + end + + def method_missing(method, *args) + if @provider.nil? || !@provider.respond_to?(method) + super + else + provider.send(method, *args) + end + end + + def payment_profiles_supported? + false + end + + def method_type + 'gateway' + end + + def supports?(source) + return true unless provider_class.respond_to? :supports? + return false unless source.brand + provider_class.supports?(source.brand) + end + end +end diff --git a/app/models/spree/gateway/bogus.rb b/app/models/spree/gateway/bogus.rb new file mode 100644 index 0000000000..de241848ad --- /dev/null +++ b/app/models/spree/gateway/bogus.rb @@ -0,0 +1,85 @@ +module Spree + class Gateway::Bogus < Gateway + TEST_VISA = ['4111111111111111','4012888888881881','4222222222222'] + TEST_MC = ['5500000000000004','5555555555554444','5105105105105100'] + TEST_AMEX = ['378282246310005','371449635398431','378734493671000','340000000000009'] + TEST_DISC = ['6011000000000004','6011111111111117','6011000990139424'] + + VALID_CCS = ['1', TEST_VISA, TEST_MC, TEST_AMEX, TEST_DISC].flatten + + attr_accessor :test + + def provider_class + self.class + end + + def preferences + {} + end + + def create_profile(payment) + # simulate the storage of credit card profile using remote service + success = VALID_CCS.include? payment.source.number + payment.source.update_attributes(:gateway_customer_profile_id => generate_profile_id(success)) + end + + def authorize(money, credit_card, options = {}) + profile_id = credit_card.gateway_customer_profile_id + if VALID_CCS.include? credit_card.number or (profile_id and profile_id.starts_with? 'BGS-') + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { :message => 'Bogus Gateway: Forced failure' }, :test => true) + end + end + + def purchase(money, credit_card, options = {}) + profile_id = credit_card.gateway_customer_profile_id + if VALID_CCS.include? credit_card.number or (profile_id and profile_id.starts_with? 'BGS-') + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', :message => 'Bogus Gateway: Forced failure', :test => true) + end + end + + def credit(money, credit_card, response_code, options = {}) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345') + end + + def capture(authorization, credit_card, gateway_options) + if authorization.response_code == '12345' + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '67890') + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', :error => 'Bogus Gateway: Forced failure', :test => true) + end + + end + + def void(response_code, credit_card, options = {}) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345') + end + + def test? + # Test mode is not really relevant with bogus gateway (no such thing as live server) + true + end + + def payment_profiles_supported? + true + end + + def actions + %w(capture void credit) + end + + private + def generate_profile_id(success) + record = true + prefix = success ? 'BGS' : 'FAIL' + while record + random = "#{prefix}-#{Array.new(6){rand(6)}.join}" + record = CreditCard.where(:gateway_customer_profile_id => random).first + end + random + end + end +end diff --git a/app/models/spree/gateway/bogus_simple.rb b/app/models/spree/gateway/bogus_simple.rb new file mode 100644 index 0000000000..8b0757b220 --- /dev/null +++ b/app/models/spree/gateway/bogus_simple.rb @@ -0,0 +1,26 @@ +# Bogus Gateway that doesn't support payment profiles +module Spree + class Gateway::BogusSimple < Gateway::Bogus + + def payment_profiles_supported? + false + end + + def authorize(money, credit_card, options = {}) + if VALID_CCS.include? credit_card.number + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { :message => 'Bogus Gateway: Forced failure' }, :test => true) + end + end + + def purchase(money, credit_card, options = {}) + if VALID_CCS.include? credit_card.number + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', :message => 'Bogus Gateway: Forced failure', :test => true) + end + end + + end +end diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb new file mode 100644 index 0000000000..aba5a7ee9c --- /dev/null +++ b/app/models/spree/payment_method.rb @@ -0,0 +1,62 @@ +module Spree + class PaymentMethod < ActiveRecord::Base + acts_as_paranoid + DISPLAY = [:both, :front_end, :back_end] + default_scope -> { where(deleted_at: nil) } + + scope :production, -> { where(environment: 'production') } + + validates :name, presence: true + + def self.providers + Rails.application.config.spree.payment_methods + end + + def provider_class + raise 'You must implement provider_class method for this gateway.' + 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 + def payment_source_class + raise 'You must implement payment_source_class method for this gateway.' + end + + def self.available(display_on = 'both') + all.select do |p| + p.active && + (p.display_on == display_on.to_s || p.display_on.blank?) && + (p.environment == Rails.env || p.environment.blank?) + end + end + + def self.active? + where(type: self.to_s, environment: Rails.env, active: true).count > 0 + end + + def method_type + type.demodulize.downcase + end + + def self.find_with_destroyed *args + unscoped { find(*args) } + end + + def payment_profiles_supported? + false + end + + def source_required? + true + end + + def auto_capture? + Spree::Config[:auto_capture] + end + + def supports?(source) + true + end + end +end diff --git a/app/models/spree/payment_method/check.rb b/app/models/spree/payment_method/check.rb new file mode 100644 index 0000000000..108d8317d1 --- /dev/null +++ b/app/models/spree/payment_method/check.rb @@ -0,0 +1,29 @@ +module Spree + class PaymentMethod::Check < PaymentMethod + def actions + %w{capture void} + end + + # Indicates whether its possible to capture the payment + def can_capture?(payment) + ['checkout', 'pending'].include?(payment.state) + end + + # Indicates whether its possible to void the payment. + def can_void?(payment) + payment.state != 'void' + end + + def capture(*args) + ActiveMerchant::Billing::Response.new(true, "", {}, {}) + end + + def void(*args) + ActiveMerchant::Billing::Response.new(true, "", {}, {}) + end + + def source_required? + false + end + end +end diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index 845b1dd84a..a2b812fe33 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -2,6 +2,203 @@ require 'spec_helper' module Spree describe CreditCard do + describe "original specs from Spree" do + let(:valid_credit_card_attributes) { { number: '4111111111111111', verification_value: '123', month: 12, year: Time.now.year + 1 } } + + def self.payment_states + Spree::Payment.state_machine.states.keys + end + + def stub_rails_env(environment) + Rails.stub(env: ActiveSupport::StringInquirer.new(environment)) + end + + let(:credit_card) { Spree::CreditCard.new } + + before(:each) do + + @order = create(:order) + @payment = Spree::Payment.create(:amount => 100, :order => @order) + + @success_response = double('gateway_response', success?: true, authorization: '123', avs_result: { 'code' => 'avs-code' }) + @fail_response = double('gateway_response', success?: false) + + @payment_gateway = mock_model(Spree::PaymentMethod, + payment_profiles_supported?: true, + authorize: @success_response, + purchase: @success_response, + capture: @success_response, + void: @success_response, + credit: @success_response, + environment: 'test' + ) + + @payment.stub payment_method: @payment_gateway + end + + context "#can_capture?" do + it "should be true if payment is pending" do + payment = mock_model(Spree::Payment, pending?: true, created_at: Time.now) + credit_card.can_capture?(payment).should be_true + end + + it "should be true if payment is checkout" do + payment = mock_model(Spree::Payment, pending?: false, checkout?: true, created_at: Time.now) + credit_card.can_capture?(payment).should be_true + end + end + + context "#can_void?" do + it "should be true if payment is not void" do + payment = mock_model(Spree::Payment, void?: false) + credit_card.can_void?(payment).should be_true + end + end + + context "#can_credit?" do + it "should be false if payment is not completed" do + payment = mock_model(Spree::Payment, completed?: false) + credit_card.can_credit?(payment).should be_false + end + + it "should be false when order payment_state is not 'credit_owed'" do + payment = mock_model(Spree::Payment, completed?: true, order: mock_model(Spree::Order, payment_state: 'paid')) + credit_card.can_credit?(payment).should be_false + end + + it "should be false when credit_allowed is zero" do + payment = mock_model(Spree::Payment, completed?: true, credit_allowed: 0, order: mock_model(Spree::Order, payment_state: 'credit_owed')) + credit_card.can_credit?(payment).should be_false + end + end + + context "#valid?" do + it "should validate presence of number" do + credit_card.attributes = valid_credit_card_attributes.except(:number) + credit_card.should_not be_valid + credit_card.errors[:number].should == ["can't be blank"] + end + + it "should validate presence of security code" do + credit_card.attributes = valid_credit_card_attributes.except(:verification_value) + credit_card.should_not be_valid + credit_card.errors[:verification_value].should == ["can't be blank"] + end + + it "should validate expiration is not in the past" do + credit_card.month = 1.month.ago.month + credit_card.year = 1.month.ago.year + credit_card.should_not be_valid + credit_card.errors[:base].should == ["Card has expired"] + end + + it "does not run expiration in the past validation if month is not set" do + credit_card.month = nil + credit_card.year = Time.now.year + credit_card.should_not be_valid + credit_card.errors[:base].should be_blank + end + + it "does not run expiration in the past validation if year is not set" do + credit_card.month = Time.now.month + credit_card.year = nil + credit_card.should_not be_valid + credit_card.errors[:base].should be_blank + end + + it "does not run expiration in the past validation if year and month are empty" do + credit_card.year = "" + credit_card.month = "" + credit_card.should_not be_valid + credit_card.errors[:card].should be_blank + end + + it "should only validate on create" do + credit_card.attributes = valid_credit_card_attributes + credit_card.save + credit_card.should be_valid + end + end + + context "#save" do + before do + credit_card.attributes = valid_credit_card_attributes + credit_card.save! + end + + let!(:persisted_card) { Spree::CreditCard.find(credit_card.id) } + + it "should not actually store the number" do + persisted_card.number.should be_blank + end + + it "should not actually store the security code" do + persisted_card.verification_value.should be_blank + end + end + + context "#number=" do + it "should strip non-numeric characters from card input" do + credit_card.number = "6011000990139424" + credit_card.number.should == "6011000990139424" + + credit_card.number = " 6011-0009-9013-9424 " + credit_card.number.should == "6011000990139424" + end + + it "should not raise an exception on non-string input" do + credit_card.number = Hash.new + credit_card.number.should be_nil + end + end + + context "#cc_type=" do + it "converts between the different types" do + credit_card.cc_type = 'mastercard' + credit_card.cc_type.should == 'master' + + credit_card.cc_type = 'maestro' + credit_card.cc_type.should == 'master' + + credit_card.cc_type = 'amex' + credit_card.cc_type.should == 'american_express' + + credit_card.cc_type = 'dinersclub' + credit_card.cc_type.should == 'diners_club' + + credit_card.cc_type = 'some_outlandish_cc_type' + credit_card.cc_type.should == 'some_outlandish_cc_type' + end + end + + context "#associations" do + it "should be able to access its payments" do + expect { credit_card.payments.to_a }.not_to raise_error + end + end + + context "#to_active_merchant" do + before do + credit_card.number = "4111111111111111" + credit_card.year = Time.now.year + credit_card.month = Time.now.month + credit_card.first_name = "Bob" + credit_card.last_name = "Boblaw" + credit_card.verification_value = 123 + end + + it "converts to an ActiveMerchant::Billing::CreditCard object" do + am_card = credit_card.to_active_merchant + am_card.number.should == "4111111111111111" + am_card.year.should == Time.now.year + am_card.month.should == Time.now.month + am_card.first_name.should == "Bob" + am_card.last_name = "Boblaw" + am_card.verification_value.should == 123 + end + end + end + describe "setting default credit card for a user" do let(:user) { create(:user) } let(:onetime_card_attrs) do diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb new file mode 100644 index 0000000000..cdedccd730 --- /dev/null +++ b/spec/models/spree/gateway_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Spree::Gateway do + class Provider + def initialize(options) + end + + def imaginary_method + + end + end + + class TestGateway < Spree::Gateway + def provider_class + Provider + end + end + + it "passes through all arguments on a method_missing call" do + gateway = TestGateway.new + gateway.provider.should_receive(:imaginary_method).with('foo') + gateway.imaginary_method('foo') + end +end diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index 396b9f1ab0..e61fd8252c 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -2,6 +2,39 @@ require 'spec_helper' module Spree describe PaymentMethod do + describe "#available" do + before(:all) do + Spree::PaymentMethod.delete_all + + [nil, 'both', 'front_end', 'back_end'].each do |display_on| + Spree::Gateway::Test.create( + :name => 'Display Both', + :display_on => display_on, + :active => true, + :environment => 'test', + :description => 'foofah' + ) + end + Spree::PaymentMethod.all.size.should == 4 + end + + it "should return all methods available to front-end/back-end when no parameter is passed" do + Spree::PaymentMethod.available.size.should == 2 + end + + it "should return all methods available to front-end/back-end when display_on = :both" do + Spree::PaymentMethod.available(:both).size.should == 2 + end + + it "should return all methods available to front-end when display_on = :front_end" do + Spree::PaymentMethod.available(:front_end).size.should == 2 + end + + it "should return all methods available to back-end when display_on = :back_end" do + Spree::PaymentMethod.available(:back_end).size.should == 2 + end + end + it "orders payment methods by name" do pm1 = create(:payment_method, name: 'ZZ') pm2 = create(:payment_method, name: 'AA') From 142bab8c35771efce02d0e9fcd1f9077d97acb09 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 13:13:43 +0100 Subject: [PATCH 2/9] Merge decorators with original spree files --- app/models/spree/credit_card.rb | 37 ++++++++- app/models/spree/credit_card_decorator.rb | 47 ----------- app/models/spree/gateway.rb | 9 ++- app/models/spree/gateway_decorator.rb | 9 --- app/models/spree/payment_method.rb | 84 +++++++++++++++++++- app/models/spree/payment_method_decorator.rb | 84 -------------------- 6 files changed, 126 insertions(+), 144 deletions(-) delete mode 100644 app/models/spree/credit_card_decorator.rb delete mode 100644 app/models/spree/gateway_decorator.rb delete mode 100644 app/models/spree/payment_method_decorator.rb diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 21bc709ec1..46718b5ad8 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -1,16 +1,26 @@ module Spree class CreditCard < ActiveRecord::Base + # Should be able to remove once we reach Spree v2.2.0 + # https://github.com/spree/spree/commit/411010f3975c919ab298cb63962ee492455b415c + belongs_to :payment_method + belongs_to :user + has_many :payments, as: :source before_save :set_last_digits attr_accessor :number, :verification_value + # For holding customer preference in memory + attr_writer :save_requested_by_customer validates :month, :year, numericality: { only_integer: true } validates :number, presence: true, unless: :has_payment_profile?, on: :create validates :verification_value, presence: true, unless: :has_payment_profile?, on: :create validate :expiry_not_in_the_past + after_create :ensure_single_default_card + after_save :ensure_single_default_card, if: :default_card_needs_updating? + scope :with_payment_profile, -> { where('gateway_customer_profile_id IS NOT NULL') } # needed for some of the ActiveMerchant gateways (eg. SagePay) @@ -86,8 +96,9 @@ module Spree payment.credit_allowed > 0 end + # Allows us to use a gateway_payment_profile_id to store Stripe Tokens def has_payment_profile? - gateway_customer_profile_id.present? + gateway_customer_profile_id.present? || gateway_payment_profile_id.present? end def to_active_merchant @@ -101,6 +112,10 @@ module Spree ) end + def save_requested_by_customer? + !!@save_requested_by_customer + end + private def expiry_not_in_the_past @@ -111,5 +126,25 @@ module Spree end end end + + def reusable? + gateway_customer_profile_id.present? + end + + def default_missing? + !user.credit_cards.exists?(is_default: true) + end + + def default_card_needs_updating? + is_default_changed? || gateway_customer_profile_id_changed? + end + + def ensure_single_default_card + return unless user + return unless is_default? || (reusable? && default_missing?) + + user.credit_cards.update_all(['is_default=(id=?)', id]) + self.is_default = true + end end end diff --git a/app/models/spree/credit_card_decorator.rb b/app/models/spree/credit_card_decorator.rb deleted file mode 100644 index b8c78f01b7..0000000000 --- a/app/models/spree/credit_card_decorator.rb +++ /dev/null @@ -1,47 +0,0 @@ -Spree::CreditCard.class_eval do - # For holding customer preference in memory - attr_writer :save_requested_by_customer - - # Should be able to remove once we reach Spree v2.2.0 - # https://github.com/spree/spree/commit/411010f3975c919ab298cb63962ee492455b415c - belongs_to :payment_method - - belongs_to :user - - after_create :ensure_single_default_card - after_save :ensure_single_default_card, if: :default_card_needs_updating? - - # Allows us to use a gateway_payment_profile_id to store Stripe Tokens - # Should be able to remove once we reach Spree v2.2.0 - # Commit: https://github.com/spree/spree/commit/5a4d690ebc64b264bf12904a70187e7a8735ef3f - # See also: https://github.com/spree/spree_gateway/issues/111 - def has_payment_profile? # rubocop:disable Naming/PredicateName - gateway_customer_profile_id.present? || gateway_payment_profile_id.present? - end - - def save_requested_by_customer? - !!@save_requested_by_customer - end - - private - - def reusable? - gateway_customer_profile_id.present? - end - - def default_missing? - !user.credit_cards.exists?(is_default: true) - end - - def default_card_needs_updating? - is_default_changed? || gateway_customer_profile_id_changed? - end - - def ensure_single_default_card - return unless user - return unless is_default? || (reusable? && default_missing?) - - user.credit_cards.update_all(['is_default=(id=?)', id]) - self.is_default = true - end -end diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 4e1cbaa368..165fffd91a 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -1,11 +1,16 @@ +require 'spree/concerns/payment_method_distributors' + module Spree class Gateway < PaymentMethod + include Spree::PaymentMethodDistributors + delegate_belongs_to :provider, :authorize, :purchase, :capture, :void, :credit validates :name, :type, presence: true - preference :server, :string, default: 'test' - preference :test_mode, :boolean, default: true + # Default to live + preference :server, :string, default: 'live' + preference :test_mode, :boolean, default: false def payment_source_class CreditCard diff --git a/app/models/spree/gateway_decorator.rb b/app/models/spree/gateway_decorator.rb deleted file mode 100644 index c62702a321..0000000000 --- a/app/models/spree/gateway_decorator.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'spree/concerns/payment_method_distributors' - -Spree::Gateway.class_eval do - include Spree::PaymentMethodDistributors - - # Default to live - preference :server, :string, default: 'live' - preference :test_mode, :boolean, default: false -end diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index aba5a7ee9c..01a9be0ce5 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -1,12 +1,56 @@ +require 'spree/concerns/payment_method_distributors' + module Spree class PaymentMethod < ActiveRecord::Base + include Spree::Core::CalculatedAdjustments + include Spree::PaymentMethodDistributors + + acts_as_taggable acts_as_paranoid + DISPLAY = [:both, :front_end, :back_end] default_scope -> { where(deleted_at: nil) } - scope :production, -> { where(environment: 'production') } + has_many :credit_cards, class_name: "Spree::CreditCard" # from Spree v.2.3.0 d470b31798f37 validates :name, presence: true + validate :distributor_validation + + after_initialize :init + + scope :production, -> { where(environment: 'production') } + + # -- Scopes + scope :managed_by, lambda { |user| + if user.has_spree_role?('admin') + where(nil) + else + joins(:distributors). + where('distributors_payment_methods.distributor_id IN (?)', user.enterprises.select(&:id)). + select('DISTINCT spree_payment_methods.*') + end + } + + scope :for_distributors, ->(distributors) { + non_unique_matches = unscoped.joins(:distributors).where(enterprises: { id: distributors }) + where(id: non_unique_matches.map(&:id)) + } + + scope :for_distributor, lambda { |distributor| + joins(:distributors). + where('enterprises.id = ?', distributor) + } + + scope :for_subscriptions, -> { where(type: Subscription::ALLOWED_PAYMENT_METHOD_TYPES) } + + scope :by_name, -> { order('spree_payment_methods.name ASC') } + + # Rewrite Spree's ruby-land class method as a scope + scope :available, lambda { |display_on = 'both'| + where(active: true). + where('spree_payment_methods.display_on=? OR spree_payment_methods.display_on=? OR spree_payment_methods.display_on IS NULL', display_on, ''). + where('spree_payment_methods.environment=? OR spree_payment_methods.environment=? OR spree_payment_methods.environment IS NULL', Rails.env, '') + } def self.providers Rails.application.config.spree.payment_methods @@ -58,5 +102,43 @@ module Spree def supports?(source) true end + + def init + unless reflections.key?(:calculator) + self.class.include Spree::Core::CalculatedAdjustments + end + + self.calculator ||= Calculator::FlatRate.new(preferred_amount: 0) + end + + def has_distributor?(distributor) + distributors.include?(distributor) + end + + def self.clean_name + case name + when "Spree::PaymentMethod::Check" + "Cash/EFT/etc. (payments for which automatic validation is not required)" + when "Spree::Gateway::Migs" + "MasterCard Internet Gateway Service (MIGS)" + when "Spree::Gateway::Pin" + "Pin Payments" + when "Spree::Gateway::StripeConnect" + "Stripe" + when "Spree::Gateway::StripeSCA" + "Stripe SCA" + when "Spree::Gateway::PayPalExpress" + "PayPal Express" + else + i = name.rindex('::') + 2 + name[i..-1] + end + end + + private + + def distributor_validation + validates_with DistributorsValidator + end end end diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb deleted file mode 100644 index 4c2770daea..0000000000 --- a/app/models/spree/payment_method_decorator.rb +++ /dev/null @@ -1,84 +0,0 @@ -require 'spree/concerns/payment_method_distributors' - -Spree::PaymentMethod.class_eval do - include Spree::Core::CalculatedAdjustments - include Spree::PaymentMethodDistributors - - acts_as_taggable - - has_many :credit_cards, class_name: "Spree::CreditCard" # from Spree v.2.3.0 d470b31798f37 - - after_initialize :init - - validate :distributor_validation - - # -- Scopes - scope :managed_by, lambda { |user| - if user.has_spree_role?('admin') - where(nil) - else - joins(:distributors). - where('distributors_payment_methods.distributor_id IN (?)', user.enterprises.select(&:id)). - select('DISTINCT spree_payment_methods.*') - end - } - - scope :for_distributors, ->(distributors) { - non_unique_matches = unscoped.joins(:distributors).where(enterprises: { id: distributors }) - where(id: non_unique_matches.map(&:id)) - } - - scope :for_distributor, lambda { |distributor| - joins(:distributors). - where('enterprises.id = ?', distributor) - } - - scope :for_subscriptions, -> { where(type: Subscription::ALLOWED_PAYMENT_METHOD_TYPES) } - - scope :by_name, -> { order('spree_payment_methods.name ASC') } - - # Rewrite Spree's ruby-land class method as a scope - scope :available, lambda { |display_on = 'both'| - where(active: true). - where('spree_payment_methods.display_on=? OR spree_payment_methods.display_on=? OR spree_payment_methods.display_on IS NULL', display_on, ''). - where('spree_payment_methods.environment=? OR spree_payment_methods.environment=? OR spree_payment_methods.environment IS NULL', Rails.env, '') - } - - def init - unless reflections.key?(:calculator) - self.class.include Spree::Core::CalculatedAdjustments - end - - self.calculator ||= Calculator::FlatRate.new(preferred_amount: 0) - end - - def has_distributor?(distributor) - distributors.include?(distributor) - end - - def self.clean_name - case name - when "Spree::PaymentMethod::Check" - "Cash/EFT/etc. (payments for which automatic validation is not required)" - when "Spree::Gateway::Migs" - "MasterCard Internet Gateway Service (MIGS)" - when "Spree::Gateway::Pin" - "Pin Payments" - when "Spree::Gateway::StripeConnect" - "Stripe" - when "Spree::Gateway::StripeSCA" - "Stripe SCA" - when "Spree::Gateway::PayPalExpress" - "PayPal Express" - else - i = name.rindex('::') + 2 - name[i..-1] - end - end - - private - - def distributor_validation - validates_with DistributorsValidator - end -end From 621e2a3132de8a27651bfa2ab035c000fea4f9d8 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 13:16:38 +0100 Subject: [PATCH 3/9] Run rubocop autocorrect --- app/models/spree/credit_card.rb | 41 +++++++++------- app/models/spree/gateway.rb | 7 ++- app/models/spree/gateway/bogus.rb | 60 ++++++++++++------------ app/models/spree/gateway/bogus_simple.rb | 16 +++---- app/models/spree/payment_method.rb | 14 +++--- app/models/spree/payment_method/check.rb | 6 ++- spec/models/spree/credit_card_spec.rb | 44 +++++++++-------- spec/models/spree/gateway_spec.rb | 9 ++-- spec/models/spree/payment_method_spec.rb | 12 ++--- 9 files changed, 111 insertions(+), 98 deletions(-) diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 46718b5ad8..f075de0e14 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class CreditCard < ActiveRecord::Base # Should be able to remove once we reach Spree v2.2.0 @@ -32,28 +34,32 @@ module Spree end def number=(num) - @number = num.gsub(/[^0-9]/, '') rescue nil + @number = begin + num.gsub(/[^0-9]/, '') + rescue StandardError + nil + end end # cc_type is set by jquery.payment, which helpfully provides different # types from Active Merchant. Converting them is necessary. def cc_type=(type) real_type = case type - when 'mastercard', 'maestro' - 'master' - when 'amex' - 'american_express' - when 'dinersclub' - 'diners_club' - else - type + when 'mastercard', 'maestro' + 'master' + when 'amex' + 'american_express' + when 'dinersclub' + 'diners_club' + else + type end self[:cc_type] = real_type end def set_last_digits - number.to_s.gsub!(/\s/,'') - verification_value.to_s.gsub!(/\s/,'') + number.to_s.gsub!(/\s/, '') + verification_value.to_s.gsub!(/\s/, '') self.last_digits ||= number.to_s.length <= 4 ? number : number.to_s.slice(-4..-1) end @@ -93,6 +99,7 @@ module Spree def can_credit?(payment) return false unless payment.completed? return false unless payment.order.payment_state == 'credit_owed' + payment.credit_allowed > 0 end @@ -103,12 +110,12 @@ module Spree def to_active_merchant ActiveMerchant::Billing::CreditCard.new( - :number => number, - :month => month, - :year => year, - :verification_value => verification_value, - :first_name => first_name, - :last_name => last_name + number: number, + month: month, + year: year, + verification_value: verification_value, + first_name: first_name, + last_name: last_name ) end diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 165fffd91a..283a8458de 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spree/concerns/payment_method_distributors' module Spree @@ -23,7 +25,7 @@ module Spree def provider gateway_options = options - gateway_options.delete :login if gateway_options.has_key?(:login) and gateway_options[:login].nil? + gateway_options.delete :login if gateway_options.key?(:login) && gateway_options[:login].nil? if gateway_options[:server] ActiveMerchant::Billing::Base.gateway_mode = gateway_options[:server].to_sym end @@ -31,7 +33,7 @@ module Spree end def options - self.preferences.inject({}){ |memo, (key, value)| memo[key.to_sym] = value; memo } + preferences.each_with_object({}){ |(key, value), memo| memo[key.to_sym] = value; } end def method_missing(method, *args) @@ -53,6 +55,7 @@ module Spree def supports?(source) return true unless provider_class.respond_to? :supports? return false unless source.brand + provider_class.supports?(source.brand) end end diff --git a/app/models/spree/gateway/bogus.rb b/app/models/spree/gateway/bogus.rb index de241848ad..2efa234fe9 100644 --- a/app/models/spree/gateway/bogus.rb +++ b/app/models/spree/gateway/bogus.rb @@ -1,9 +1,11 @@ +# frozen_string_literal: true + module Spree class Gateway::Bogus < Gateway - TEST_VISA = ['4111111111111111','4012888888881881','4222222222222'] - TEST_MC = ['5500000000000004','5555555555554444','5105105105105100'] - TEST_AMEX = ['378282246310005','371449635398431','378734493671000','340000000000009'] - TEST_DISC = ['6011000000000004','6011111111111117','6011000990139424'] + TEST_VISA = ['4111111111111111', '4012888888881881', '4222222222222'].freeze + TEST_MC = ['5500000000000004', '5555555555554444', '5105105105105100'].freeze + TEST_AMEX = ['378282246310005', '371449635398431', '378734493671000', '340000000000009'].freeze + TEST_DISC = ['6011000000000004', '6011111111111117', '6011000990139424'].freeze VALID_CCS = ['1', TEST_VISA, TEST_MC, TEST_AMEX, TEST_DISC].flatten @@ -20,42 +22,41 @@ module Spree def create_profile(payment) # simulate the storage of credit card profile using remote service success = VALID_CCS.include? payment.source.number - payment.source.update_attributes(:gateway_customer_profile_id => generate_profile_id(success)) + payment.source.update(gateway_customer_profile_id: generate_profile_id(success)) end - def authorize(money, credit_card, options = {}) + def authorize(_money, credit_card, _options = {}) profile_id = credit_card.gateway_customer_profile_id - if VALID_CCS.include? credit_card.number or (profile_id and profile_id.starts_with? 'BGS-') - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + if VALID_CCS.include?(credit_card.number) || profile_id&.starts_with?('BGS-') + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { :message => 'Bogus Gateway: Forced failure' }, :test => true) + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { message: 'Bogus Gateway: Forced failure' }, test: true) end end - def purchase(money, credit_card, options = {}) + def purchase(_money, credit_card, _options = {}) profile_id = credit_card.gateway_customer_profile_id - if VALID_CCS.include? credit_card.number or (profile_id and profile_id.starts_with? 'BGS-') - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + if VALID_CCS.include?(credit_card.number) || profile_id&.starts_with?('BGS-') + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', :message => 'Bogus Gateway: Forced failure', :test => true) + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', message: 'Bogus Gateway: Forced failure', test: true) end end - def credit(money, credit_card, response_code, options = {}) - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345') + def credit(_money, _credit_card, _response_code, _options = {}) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345') end - def capture(authorization, credit_card, gateway_options) + def capture(authorization, _credit_card, _gateway_options) if authorization.response_code == '12345' - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '67890') + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '67890') else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', :error => 'Bogus Gateway: Forced failure', :test => true) + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', error: 'Bogus Gateway: Forced failure', test: true) end - end - def void(response_code, credit_card, options = {}) - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345') + def void(_response_code, _credit_card, _options = {}) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345') end def test? @@ -72,14 +73,15 @@ module Spree end private - def generate_profile_id(success) - record = true - prefix = success ? 'BGS' : 'FAIL' - while record - random = "#{prefix}-#{Array.new(6){rand(6)}.join}" - record = CreditCard.where(:gateway_customer_profile_id => random).first - end - random + + def generate_profile_id(success) + record = true + prefix = success ? 'BGS' : 'FAIL' + while record + random = "#{prefix}-#{Array.new(6){ rand(6) }.join}" + record = CreditCard.where(gateway_customer_profile_id: random).first end + random + end end end diff --git a/app/models/spree/gateway/bogus_simple.rb b/app/models/spree/gateway/bogus_simple.rb index 8b0757b220..0a9d64c1a0 100644 --- a/app/models/spree/gateway/bogus_simple.rb +++ b/app/models/spree/gateway/bogus_simple.rb @@ -1,26 +1,26 @@ +# frozen_string_literal: true + # Bogus Gateway that doesn't support payment profiles module Spree class Gateway::BogusSimple < Gateway::Bogus - def payment_profiles_supported? false end - def authorize(money, credit_card, options = {}) + def authorize(_money, credit_card, _options = {}) if VALID_CCS.include? credit_card.number - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { :message => 'Bogus Gateway: Forced failure' }, :test => true) + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { message: 'Bogus Gateway: Forced failure' }, test: true) end end - def purchase(money, credit_card, options = {}) + def purchase(_money, credit_card, _options = {}) if VALID_CCS.include? credit_card.number - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, :test => true, :authorization => '12345', :avs_result => { :code => 'A' }) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', :message => 'Bogus Gateway: Forced failure', :test => true) + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', message: 'Bogus Gateway: Forced failure', test: true) end end - end end diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index 01a9be0ce5..c659c9a47a 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spree/concerns/payment_method_distributors' module Spree @@ -8,7 +10,7 @@ module Spree acts_as_taggable acts_as_paranoid - DISPLAY = [:both, :front_end, :back_end] + DISPLAY = [:both, :front_end, :back_end].freeze default_scope -> { where(deleted_at: nil) } has_many :credit_cards, class_name: "Spree::CreditCard" # from Spree v.2.3.0 d470b31798f37 @@ -70,20 +72,20 @@ module Spree def self.available(display_on = 'both') all.select do |p| p.active && - (p.display_on == display_on.to_s || p.display_on.blank?) && - (p.environment == Rails.env || p.environment.blank?) + (p.display_on == display_on.to_s || p.display_on.blank?) && + (p.environment == Rails.env || p.environment.blank?) end end def self.active? - where(type: self.to_s, environment: Rails.env, active: true).count > 0 + where(type: to_s, environment: Rails.env, active: true).count > 0 end def method_type type.demodulize.downcase end - def self.find_with_destroyed *args + def self.find_with_destroyed(*args) unscoped { find(*args) } end @@ -99,7 +101,7 @@ module Spree Spree::Config[:auto_capture] end - def supports?(source) + def supports?(_source) true end diff --git a/app/models/spree/payment_method/check.rb b/app/models/spree/payment_method/check.rb index 108d8317d1..8ebdd19fc7 100644 --- a/app/models/spree/payment_method/check.rb +++ b/app/models/spree/payment_method/check.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class PaymentMethod::Check < PaymentMethod def actions @@ -14,11 +16,11 @@ module Spree payment.state != 'void' end - def capture(*args) + def capture(*_args) ActiveMerchant::Billing::Response.new(true, "", {}, {}) end - def void(*args) + def void(*_args) ActiveMerchant::Billing::Response.new(true, "", {}, {}) end diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index a2b812fe33..930cefa995 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' module Spree describe CreditCard do describe "original specs from Spree" do - let(:valid_credit_card_attributes) { { number: '4111111111111111', verification_value: '123', month: 12, year: Time.now.year + 1 } } + let(:valid_credit_card_attributes) { { number: '4111111111111111', verification_value: '123', month: 12, year: Time.zone.now.year + 1 } } def self.payment_states Spree::Payment.state_machine.states.keys @@ -16,34 +16,32 @@ module Spree let(:credit_card) { Spree::CreditCard.new } before(:each) do - @order = create(:order) - @payment = Spree::Payment.create(:amount => 100, :order => @order) + @payment = Spree::Payment.create(amount: 100, order: @order) @success_response = double('gateway_response', success?: true, authorization: '123', avs_result: { 'code' => 'avs-code' }) @fail_response = double('gateway_response', success?: false) @payment_gateway = mock_model(Spree::PaymentMethod, - payment_profiles_supported?: true, - authorize: @success_response, - purchase: @success_response, - capture: @success_response, - void: @success_response, - credit: @success_response, - environment: 'test' - ) + payment_profiles_supported?: true, + authorize: @success_response, + purchase: @success_response, + capture: @success_response, + void: @success_response, + credit: @success_response, + environment: 'test') @payment.stub payment_method: @payment_gateway end context "#can_capture?" do it "should be true if payment is pending" do - payment = mock_model(Spree::Payment, pending?: true, created_at: Time.now) + payment = mock_model(Spree::Payment, pending?: true, created_at: Time.zone.now) credit_card.can_capture?(payment).should be_true end it "should be true if payment is checkout" do - payment = mock_model(Spree::Payment, pending?: false, checkout?: true, created_at: Time.now) + payment = mock_model(Spree::Payment, pending?: false, checkout?: true, created_at: Time.zone.now) credit_card.can_capture?(payment).should be_true end end @@ -94,24 +92,24 @@ module Spree it "does not run expiration in the past validation if month is not set" do credit_card.month = nil - credit_card.year = Time.now.year + credit_card.year = Time.zone.now.year credit_card.should_not be_valid credit_card.errors[:base].should be_blank end it "does not run expiration in the past validation if year is not set" do - credit_card.month = Time.now.month + credit_card.month = Time.zone.now.month credit_card.year = nil credit_card.should_not be_valid credit_card.errors[:base].should be_blank end - + it "does not run expiration in the past validation if year and month are empty" do credit_card.year = "" credit_card.month = "" credit_card.should_not be_valid credit_card.errors[:card].should be_blank - end + end it "should only validate on create" do credit_card.attributes = valid_credit_card_attributes @@ -147,7 +145,7 @@ module Spree end it "should not raise an exception on non-string input" do - credit_card.number = Hash.new + credit_card.number = ({}) credit_card.number.should be_nil end end @@ -176,12 +174,12 @@ module Spree expect { credit_card.payments.to_a }.not_to raise_error end end - + context "#to_active_merchant" do before do credit_card.number = "4111111111111111" - credit_card.year = Time.now.year - credit_card.month = Time.now.month + credit_card.year = Time.zone.now.year + credit_card.month = Time.zone.now.month credit_card.first_name = "Bob" credit_card.last_name = "Boblaw" credit_card.verification_value = 123 @@ -190,8 +188,8 @@ module Spree it "converts to an ActiveMerchant::Billing::CreditCard object" do am_card = credit_card.to_active_merchant am_card.number.should == "4111111111111111" - am_card.year.should == Time.now.year - am_card.month.should == Time.now.month + am_card.year.should == Time.zone.now.year + am_card.month.should == Time.zone.now.month am_card.first_name.should == "Bob" am_card.last_name = "Boblaw" am_card.verification_value.should == 123 diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb index cdedccd730..f68155edaf 100644 --- a/spec/models/spree/gateway_spec.rb +++ b/spec/models/spree/gateway_spec.rb @@ -1,13 +1,12 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Gateway do class Provider - def initialize(options) - end + def initialize(options); end - def imaginary_method - - end + def imaginary_method; end end class TestGateway < Spree::Gateway diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index e61fd8252c..79920f62e0 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -8,11 +8,11 @@ module Spree [nil, 'both', 'front_end', 'back_end'].each do |display_on| Spree::Gateway::Test.create( - :name => 'Display Both', - :display_on => display_on, - :active => true, - :environment => 'test', - :description => 'foofah' + name: 'Display Both', + display_on: display_on, + active: true, + environment: 'test', + description: 'foofah' ) end Spree::PaymentMethod.all.size.should == 4 @@ -34,7 +34,7 @@ module Spree Spree::PaymentMethod.available(:back_end).size.should == 2 end end - + it "orders payment methods by name" do pm1 = create(:payment_method, name: 'ZZ') pm2 = create(:payment_method, name: 'AA') From d746ae3d9e273a2aa14fee07407bf701b4a62720 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 13:28:09 +0100 Subject: [PATCH 4/9] Fix easy rubocop issues --- app/models/spree/credit_card.rb | 26 ++-- app/models/spree/gateway.rb | 2 +- app/models/spree/gateway/bogus.rb | 157 +++++++++++++---------- app/models/spree/gateway/bogus_simple.rb | 42 +++--- app/models/spree/payment_method.rb | 5 +- app/models/spree/payment_method/check.rb | 42 +++--- spec/models/spree/credit_card_spec.rb | 55 +++++--- 7 files changed, 186 insertions(+), 143 deletions(-) diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index f075de0e14..7f6b41bcb4 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -11,9 +11,9 @@ module Spree before_save :set_last_digits - attr_accessor :number, :verification_value - # For holding customer preference in memory - attr_writer :save_requested_by_customer + attr_accessor :verification_value + attr_reader :number + attr_writer :save_requested_by_customer # For holding customer preference in memory validates :month, :year, numericality: { only_integer: true } validates :number, presence: true, unless: :has_payment_profile?, on: :create @@ -53,7 +53,7 @@ module Spree 'diners_club' else type - end + end self[:cc_type] = real_type end @@ -94,13 +94,13 @@ module Spree !payment.void? end - # Indicates whether its possible to credit the payment. Note that most gateways require that the - # payment be settled first which generally happens within 12-24 hours of the transaction. + # Indicates whether its possible to credit the payment. Note that most gateways require that the + # payment be settled first which generally happens within 12-24 hours of the transaction. def can_credit?(payment) return false unless payment.completed? return false unless payment.order.payment_state == 'credit_owed' - payment.credit_allowed > 0 + payment.credit_allowed.positive? end # Allows us to use a gateway_payment_profile_id to store Stripe Tokens @@ -126,12 +126,12 @@ module Spree private def expiry_not_in_the_past - if year.present? && month.present? - time = "#{year}-#{month}-1".to_time - if time < Time.zone.now.to_time.beginning_of_month - errors.add(:base, :card_expired) - end - end + return unless year.present? && month.present? + + time = "#{year}-#{month}-1".to_time + return unless time < Time.zone.now.to_time.beginning_of_month + + errors.add(:base, :card_expired) end def reusable? diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 283a8458de..216f0cfe6b 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -40,7 +40,7 @@ module Spree if @provider.nil? || !@provider.respond_to?(method) super else - provider.send(method, *args) + provider.__send__(method, *args) end end diff --git a/app/models/spree/gateway/bogus.rb b/app/models/spree/gateway/bogus.rb index 2efa234fe9..2a844a0ace 100644 --- a/app/models/spree/gateway/bogus.rb +++ b/app/models/spree/gateway/bogus.rb @@ -1,87 +1,102 @@ # frozen_string_literal: true module Spree - class Gateway::Bogus < Gateway - TEST_VISA = ['4111111111111111', '4012888888881881', '4222222222222'].freeze - TEST_MC = ['5500000000000004', '5555555555554444', '5105105105105100'].freeze - TEST_AMEX = ['378282246310005', '371449635398431', '378734493671000', '340000000000009'].freeze - TEST_DISC = ['6011000000000004', '6011111111111117', '6011000990139424'].freeze + module Gateway + class Bogus < Spree::Gateway + TEST_VISA = ['4111111111111111', '4012888888881881', '4222222222222'].freeze + TEST_MC = ['5500000000000004', '5555555555554444', '5105105105105100'].freeze + TEST_AMEX = ['378282246310005', '371449635398431', + '378734493671000', '340000000000009'].freeze + TEST_DISC = ['6011000000000004', '6011111111111117', '6011000990139424'].freeze - VALID_CCS = ['1', TEST_VISA, TEST_MC, TEST_AMEX, TEST_DISC].flatten + VALID_CCS = ['1', TEST_VISA, TEST_MC, TEST_AMEX, TEST_DISC].flatten - attr_accessor :test + attr_accessor :test - def provider_class - self.class - end - - def preferences - {} - end - - def create_profile(payment) - # simulate the storage of credit card profile using remote service - success = VALID_CCS.include? payment.source.number - payment.source.update(gateway_customer_profile_id: generate_profile_id(success)) - end - - def authorize(_money, credit_card, _options = {}) - profile_id = credit_card.gateway_customer_profile_id - if VALID_CCS.include?(credit_card.number) || profile_id&.starts_with?('BGS-') - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) - else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { message: 'Bogus Gateway: Forced failure' }, test: true) + def provider_class + self.class end - end - def purchase(_money, credit_card, _options = {}) - profile_id = credit_card.gateway_customer_profile_id - if VALID_CCS.include?(credit_card.number) || profile_id&.starts_with?('BGS-') - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) - else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', message: 'Bogus Gateway: Forced failure', test: true) + def preferences + {} end - end - def credit(_money, _credit_card, _response_code, _options = {}) - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345') - end - - def capture(authorization, _credit_card, _gateway_options) - if authorization.response_code == '12345' - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '67890') - else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', error: 'Bogus Gateway: Forced failure', test: true) + def create_profile(payment) + # simulate the storage of credit card profile using remote service + success = VALID_CCS.include? payment.source.number + payment.source.update(gateway_customer_profile_id: generate_profile_id(success)) end - end - def void(_response_code, _credit_card, _options = {}) - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345') - end - - def test? - # Test mode is not really relevant with bogus gateway (no such thing as live server) - true - end - - def payment_profiles_supported? - true - end - - def actions - %w(capture void credit) - end - - private - - def generate_profile_id(success) - record = true - prefix = success ? 'BGS' : 'FAIL' - while record - random = "#{prefix}-#{Array.new(6){ rand(6) }.join}" - record = CreditCard.where(gateway_customer_profile_id: random).first + def authorize(_money, credit_card, _options = {}) + profile_id = credit_card.gateway_customer_profile_id + if VALID_CCS.include?(credit_card.number) || profile_id&.starts_with?('BGS-') + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, + test: true, authorization: '12345', + avs_result: { code: 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', + { message: 'Bogus Gateway: Forced failure' }, + test: true) + end + end + + def purchase(_money, credit_card, _options = {}) + profile_id = credit_card.gateway_customer_profile_id + if VALID_CCS.include?(credit_card.number) || profile_id&.starts_with?('BGS-') + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, + test: true, authorization: '12345', + avs_result: { code: 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', + message: 'Bogus Gateway: Forced failure', + test: true) + end + end + + def credit(_money, _credit_card, _response_code, _options = {}) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, + test: true, authorization: '12345') + end + + def capture(authorization, _credit_card, _gateway_options) + if authorization.response_code == '12345' + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, + test: true, authorization: '67890') + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', + error: 'Bogus Gateway: Forced failure', test: true) + end + end + + def void(_response_code, _credit_card, _options = {}) + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, + test: true, authorization: '12345') + end + + def test? + # Test mode is not really relevant with bogus gateway (no such thing as live server) + true + end + + def payment_profiles_supported? + true + end + + def actions + %w(capture void credit) + end + + private + + def generate_profile_id(success) + record = true + prefix = success ? 'BGS' : 'FAIL' + while record + random = "#{prefix}-#{Array.new(6){ rand(6) }.join}" + record = CreditCard.find_by(gateway_customer_profile_id: random) + end + random end - random end end end diff --git a/app/models/spree/gateway/bogus_simple.rb b/app/models/spree/gateway/bogus_simple.rb index 0a9d64c1a0..62938e438f 100644 --- a/app/models/spree/gateway/bogus_simple.rb +++ b/app/models/spree/gateway/bogus_simple.rb @@ -2,24 +2,34 @@ # Bogus Gateway that doesn't support payment profiles module Spree - class Gateway::BogusSimple < Gateway::Bogus - def payment_profiles_supported? - false - end - - def authorize(_money, credit_card, _options = {}) - if VALID_CCS.include? credit_card.number - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) - else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', { message: 'Bogus Gateway: Forced failure' }, test: true) + module Gateway + class BogusSimple < Spree::Gateway::Bogus + def payment_profiles_supported? + false end - end - def purchase(_money, credit_card, _options = {}) - if VALID_CCS.include? credit_card.number - ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, test: true, authorization: '12345', avs_result: { code: 'A' }) - else - ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', message: 'Bogus Gateway: Forced failure', test: true) + def authorize(_money, credit_card, _options = {}) + if VALID_CCS.include? credit_card.number + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, + test: true, authorization: '12345', + avs_result: { code: 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', + { message: 'Bogus Gateway: Forced failure' }, + test: true) + end + end + + def purchase(_money, credit_card, _options = {}) + if VALID_CCS.include? credit_card.number + ActiveMerchant::Billing::Response.new(true, 'Bogus Gateway: Forced success', {}, + test: true, authorization: '12345', + avs_result: { code: 'A' }) + else + ActiveMerchant::Billing::Response.new(false, 'Bogus Gateway: Forced failure', + message: 'Bogus Gateway: Forced failure', + test: true) + end end end end diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index c659c9a47a..a799eaebdd 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -28,7 +28,8 @@ module Spree where(nil) else joins(:distributors). - where('distributors_payment_methods.distributor_id IN (?)', user.enterprises.select(&:id)). + where('distributors_payment_methods.distributor_id IN (?)', + user.enterprises.select(&:id)). select('DISTINCT spree_payment_methods.*') end } @@ -78,7 +79,7 @@ module Spree end def self.active? - where(type: to_s, environment: Rails.env, active: true).count > 0 + where(type: to_s, environment: Rails.env, active: true).count.positive? end def method_type diff --git a/app/models/spree/payment_method/check.rb b/app/models/spree/payment_method/check.rb index 8ebdd19fc7..15be0202b1 100644 --- a/app/models/spree/payment_method/check.rb +++ b/app/models/spree/payment_method/check.rb @@ -1,31 +1,33 @@ # frozen_string_literal: true module Spree - class PaymentMethod::Check < PaymentMethod - def actions - %w{capture void} - end + module PaymentMethod + class Check < Spree::PaymentMethod + def actions + %w{capture void} + end - # Indicates whether its possible to capture the payment - def can_capture?(payment) - ['checkout', 'pending'].include?(payment.state) - end + # Indicates whether its possible to capture the payment + def can_capture?(payment) + ['checkout', 'pending'].include?(payment.state) + end - # Indicates whether its possible to void the payment. - def can_void?(payment) - payment.state != 'void' - end + # Indicates whether its possible to void the payment. + def can_void?(payment) + payment.state != 'void' + end - def capture(*_args) - ActiveMerchant::Billing::Response.new(true, "", {}, {}) - end + def capture(*_args) + ActiveMerchant::Billing::Response.new(true, "", {}, {}) + end - def void(*_args) - ActiveMerchant::Billing::Response.new(true, "", {}, {}) - end + def void(*_args) + ActiveMerchant::Billing::Response.new(true, "", {}, {}) + end - def source_required? - false + def source_required? + false + end end end end diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index 930cefa995..87b5daf62d 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -3,7 +3,14 @@ require 'spec_helper' module Spree describe CreditCard do describe "original specs from Spree" do - let(:valid_credit_card_attributes) { { number: '4111111111111111', verification_value: '123', month: 12, year: Time.zone.now.year + 1 } } + let(:valid_credit_card_attributes) { + { + number: '4111111111111111', + verification_value: '123', + month: 12, + year: Time.zone.now.year + 1 + } + } def self.payment_states Spree::Payment.state_machine.states.keys @@ -19,7 +26,9 @@ module Spree @order = create(:order) @payment = Spree::Payment.create(amount: 100, order: @order) - @success_response = double('gateway_response', success?: true, authorization: '123', avs_result: { 'code' => 'avs-code' }) + @success_response = double('gateway_response', success?: true, + authorization: '123', + avs_result: { 'code' => 'avs-code' }) @fail_response = double('gateway_response', success?: false) @payment_gateway = mock_model(Spree::PaymentMethod, @@ -41,7 +50,9 @@ module Spree end it "should be true if payment is checkout" do - payment = mock_model(Spree::Payment, pending?: false, checkout?: true, created_at: Time.zone.now) + payment = mock_model(Spree::Payment, pending?: false, + checkout?: true, + created_at: Time.zone.now) credit_card.can_capture?(payment).should be_true end end @@ -60,12 +71,16 @@ module Spree end it "should be false when order payment_state is not 'credit_owed'" do - payment = mock_model(Spree::Payment, completed?: true, order: mock_model(Spree::Order, payment_state: 'paid')) + payment = mock_model(Spree::Payment, + completed?: true, + order: mock_model(Spree::Order, payment_state: 'paid')) credit_card.can_credit?(payment).should be_false end it "should be false when credit_allowed is zero" do - payment = mock_model(Spree::Payment, completed?: true, credit_allowed: 0, order: mock_model(Spree::Order, payment_state: 'credit_owed')) + payment = mock_model(Spree::Payment, + completed?: true, credit_allowed: 0, + order: mock_model(Spree::Order, payment_state: 'credit_owed')) credit_card.can_credit?(payment).should be_false end end @@ -74,20 +89,20 @@ module Spree it "should validate presence of number" do credit_card.attributes = valid_credit_card_attributes.except(:number) credit_card.should_not be_valid - credit_card.errors[:number].should == ["can't be blank"] + expect(credit_card.errors[:number]).to eq ["can't be blank"] end it "should validate presence of security code" do credit_card.attributes = valid_credit_card_attributes.except(:verification_value) credit_card.should_not be_valid - credit_card.errors[:verification_value].should == ["can't be blank"] + expect(credit_card.errors[:verification_value]).to eq ["can't be blank"] end it "should validate expiration is not in the past" do credit_card.month = 1.month.ago.month credit_card.year = 1.month.ago.year credit_card.should_not be_valid - credit_card.errors[:base].should == ["Card has expired"] + expect(credit_card.errors[:base]).to eq ["Card has expired"] end it "does not run expiration in the past validation if month is not set" do @@ -138,10 +153,10 @@ module Spree context "#number=" do it "should strip non-numeric characters from card input" do credit_card.number = "6011000990139424" - credit_card.number.should == "6011000990139424" + expect(credit_card.number).to eq "6011000990139424" credit_card.number = " 6011-0009-9013-9424 " - credit_card.number.should == "6011000990139424" + expect(credit_card.number).to eq "6011000990139424" end it "should not raise an exception on non-string input" do @@ -153,19 +168,19 @@ module Spree context "#cc_type=" do it "converts between the different types" do credit_card.cc_type = 'mastercard' - credit_card.cc_type.should == 'master' + expect(credit_card.cc_type).to eq 'master' credit_card.cc_type = 'maestro' - credit_card.cc_type.should == 'master' + expect(credit_card.cc_type).to eq 'master' credit_card.cc_type = 'amex' - credit_card.cc_type.should == 'american_express' + expect(credit_card.cc_type).to eq 'american_express' credit_card.cc_type = 'dinersclub' - credit_card.cc_type.should == 'diners_club' + expect(credit_card.cc_type).to eq 'diners_club' credit_card.cc_type = 'some_outlandish_cc_type' - credit_card.cc_type.should == 'some_outlandish_cc_type' + expect(credit_card.cc_type).to eq 'some_outlandish_cc_type' end end @@ -187,12 +202,12 @@ module Spree it "converts to an ActiveMerchant::Billing::CreditCard object" do am_card = credit_card.to_active_merchant - am_card.number.should == "4111111111111111" - am_card.year.should == Time.zone.now.year - am_card.month.should == Time.zone.now.month - am_card.first_name.should == "Bob" + expect(am_card.number).to eq "4111111111111111" + expect(am_card.year).to eq Time.zone.now.year + expect(am_card.month).to eq Time.zone.now.month + expect(am_card.first_name).to eq "Bob" am_card.last_name = "Boblaw" - am_card.verification_value.should == 123 + expect(am_card.verification_value).to eq 123 end end end From b21a969502ddb66827a3f54df338dbe603b41c7e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 13:45:25 +0100 Subject: [PATCH 5/9] Fix new credit_card_spec --- app/models/spree/gateway/bogus.rb | 2 +- app/models/spree/gateway/bogus_simple.rb | 2 +- app/models/spree/payment_method/check.rb | 2 +- config/initializers/spree.rb | 3 - spec/models/spree/credit_card_spec.rb | 88 +++++++++++++----------- 5 files changed, 49 insertions(+), 48 deletions(-) diff --git a/app/models/spree/gateway/bogus.rb b/app/models/spree/gateway/bogus.rb index 2a844a0ace..76dde5f211 100644 --- a/app/models/spree/gateway/bogus.rb +++ b/app/models/spree/gateway/bogus.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Spree - module Gateway + class Gateway class Bogus < Spree::Gateway TEST_VISA = ['4111111111111111', '4012888888881881', '4222222222222'].freeze TEST_MC = ['5500000000000004', '5555555555554444', '5105105105105100'].freeze diff --git a/app/models/spree/gateway/bogus_simple.rb b/app/models/spree/gateway/bogus_simple.rb index 62938e438f..2a7c9e7778 100644 --- a/app/models/spree/gateway/bogus_simple.rb +++ b/app/models/spree/gateway/bogus_simple.rb @@ -2,7 +2,7 @@ # Bogus Gateway that doesn't support payment profiles module Spree - module Gateway + class Gateway class BogusSimple < Spree::Gateway::Bogus def payment_profiles_supported? false diff --git a/app/models/spree/payment_method/check.rb b/app/models/spree/payment_method/check.rb index 15be0202b1..9819a52c6c 100644 --- a/app/models/spree/payment_method/check.rb +++ b/app/models/spree/payment_method/check.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Spree - module PaymentMethod + class PaymentMethod class Check < Spree::PaymentMethod def actions %w{capture void} diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 6ff230c5f5..be67509db8 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -17,9 +17,6 @@ Spree::Gateway.class_eval do acts_as_taggable end -require "#{Rails.root}/app/models/spree/payment_method_decorator" -require "#{Rails.root}/app/models/spree/gateway_decorator" - Spree.config do |config| config.shipping_instructions = true config.address_requires_state = true diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index 87b5daf62d..a245b4e64f 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -24,112 +24,116 @@ module Spree before(:each) do @order = create(:order) - @payment = Spree::Payment.create(amount: 100, order: @order) + @payment = create(:payment, amount: 100, order: @order) @success_response = double('gateway_response', success?: true, authorization: '123', avs_result: { 'code' => 'avs-code' }) @fail_response = double('gateway_response', success?: false) - @payment_gateway = mock_model(Spree::PaymentMethod, - payment_profiles_supported?: true, - authorize: @success_response, - purchase: @success_response, - capture: @success_response, - void: @success_response, - credit: @success_response, - environment: 'test') - + @payment_gateway = create(:payment_method, + environment: 'test') + allow(@payment_gateway).to receive_messages :payment_profiles_supported? => true, + :authorize => @success_response, + :purchase => @success_response, + :capture => @success_response, + :void => @success_response, + :credit => @success_response @payment.stub payment_method: @payment_gateway end context "#can_capture?" do it "should be true if payment is pending" do - payment = mock_model(Spree::Payment, pending?: true, created_at: Time.zone.now) - credit_card.can_capture?(payment).should be_true + payment = create(:payment, created_at: Time.zone.now) + allow(payment).to receive(:pending?) { true } + expect(credit_card.can_capture?(payment)).to be_truthy end it "should be true if payment is checkout" do - payment = mock_model(Spree::Payment, pending?: false, - checkout?: true, - created_at: Time.zone.now) - credit_card.can_capture?(payment).should be_true + payment = create(:payment, created_at: Time.zone.now) + allow(payment).to receive_messages :pending? => false, + :checkout? => true + expect(credit_card.can_capture?(payment)).to be_truthy end end context "#can_void?" do it "should be true if payment is not void" do - payment = mock_model(Spree::Payment, void?: false) - credit_card.can_void?(payment).should be_true + payment = create(:payment) + allow(payment).to receive(:void?) { false } + expect(credit_card.can_void?(payment)).to be_truthy end end context "#can_credit?" do it "should be false if payment is not completed" do - payment = mock_model(Spree::Payment, completed?: false) - credit_card.can_credit?(payment).should be_false + payment = create(:payment) + allow(payment).to receive(:completed?) { false } + expect(credit_card.can_credit?(payment)).to be_falsy end it "should be false when order payment_state is not 'credit_owed'" do - payment = mock_model(Spree::Payment, - completed?: true, - order: mock_model(Spree::Order, payment_state: 'paid')) - credit_card.can_credit?(payment).should be_false + payment = create(:payment, + order: create(:order, payment_state: 'paid')) + allow(payment).to receive(:completed?) { true } + expect(credit_card.can_credit?(payment)).to be_falsy end it "should be false when credit_allowed is zero" do - payment = mock_model(Spree::Payment, - completed?: true, credit_allowed: 0, - order: mock_model(Spree::Order, payment_state: 'credit_owed')) - credit_card.can_credit?(payment).should be_false + payment = create(:payment, + order: create(:order, payment_state: 'credit_owed')) + allow(payment).to receive_messages :completed? => true, + :credit_allowed => 0 + + expect(credit_card.can_credit?(payment)).to be_falsy end end context "#valid?" do it "should validate presence of number" do credit_card.attributes = valid_credit_card_attributes.except(:number) - credit_card.should_not be_valid + expect(credit_card).to_not be_valid expect(credit_card.errors[:number]).to eq ["can't be blank"] end it "should validate presence of security code" do credit_card.attributes = valid_credit_card_attributes.except(:verification_value) - credit_card.should_not be_valid + expect(credit_card).to_not be_valid expect(credit_card.errors[:verification_value]).to eq ["can't be blank"] end it "should validate expiration is not in the past" do credit_card.month = 1.month.ago.month credit_card.year = 1.month.ago.year - credit_card.should_not be_valid - expect(credit_card.errors[:base]).to eq ["Card has expired"] + expect(credit_card).to_not be_valid + expect(credit_card.errors[:base]).to eq ["has expired"] end it "does not run expiration in the past validation if month is not set" do credit_card.month = nil credit_card.year = Time.zone.now.year - credit_card.should_not be_valid - credit_card.errors[:base].should be_blank + expect(credit_card).to_not be_valid + expect(credit_card.errors[:base]).to be_blank end it "does not run expiration in the past validation if year is not set" do credit_card.month = Time.zone.now.month credit_card.year = nil - credit_card.should_not be_valid - credit_card.errors[:base].should be_blank + expect(credit_card).to_not be_valid + expect(credit_card.errors[:base]).to be_blank end it "does not run expiration in the past validation if year and month are empty" do credit_card.year = "" credit_card.month = "" - credit_card.should_not be_valid - credit_card.errors[:card].should be_blank + expect(credit_card).to_not be_valid + expect(credit_card.errors[:card]).to be_blank end it "should only validate on create" do credit_card.attributes = valid_credit_card_attributes credit_card.save - credit_card.should be_valid + expect(credit_card).to be_valid end end @@ -142,11 +146,11 @@ module Spree let!(:persisted_card) { Spree::CreditCard.find(credit_card.id) } it "should not actually store the number" do - persisted_card.number.should be_blank + expect(persisted_card.number).to be_blank end it "should not actually store the security code" do - persisted_card.verification_value.should be_blank + expect(persisted_card.verification_value).to be_blank end end @@ -161,7 +165,7 @@ module Spree it "should not raise an exception on non-string input" do credit_card.number = ({}) - credit_card.number.should be_nil + expect(credit_card.number).to be_nil end end From 798194c03ee03741204e8ad26d1029edd2443837 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 14:02:05 +0100 Subject: [PATCH 6/9] Fix payment_method spec --- spec/models/spree/payment_method_spec.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index 79920f62e0..3e0d99c5fa 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -1,9 +1,14 @@ require 'spec_helper' +class Spree::Gateway::Test < Spree::Gateway +end + module Spree describe PaymentMethod do describe "#available" do - before(:all) do + let(:enterprise) { create(:enterprise) } + + before do Spree::PaymentMethod.delete_all [nil, 'both', 'front_end', 'back_end'].each do |display_on| @@ -12,26 +17,27 @@ module Spree display_on: display_on, active: true, environment: 'test', - description: 'foofah' + description: 'foofah', + distributors: [enterprise] ) end - Spree::PaymentMethod.all.size.should == 4 + expect(Spree::PaymentMethod.all.size).to eq 4 end it "should return all methods available to front-end/back-end when no parameter is passed" do - Spree::PaymentMethod.available.size.should == 2 + expect(Spree::PaymentMethod.available.size).to eq 2 end it "should return all methods available to front-end/back-end when display_on = :both" do - Spree::PaymentMethod.available(:both).size.should == 2 + expect(Spree::PaymentMethod.available(:both).size).to eq 2 end it "should return all methods available to front-end when display_on = :front_end" do - Spree::PaymentMethod.available(:front_end).size.should == 2 + expect(Spree::PaymentMethod.available(:front_end).size).to eq 2 end it "should return all methods available to back-end when display_on = :back_end" do - Spree::PaymentMethod.available(:back_end).size.should == 2 + expect(Spree::PaymentMethod.available(:back_end).size).to eq 2 end end From 1b66a72c7fb173c65d918746f51b772b745e2027 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 14:03:15 +0100 Subject: [PATCH 7/9] Run transpec --- spec/models/spree/credit_card_spec.rb | 4 ++-- spec/models/spree/gateway_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index a245b4e64f..c1d8b8ae82 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -17,7 +17,7 @@ module Spree end def stub_rails_env(environment) - Rails.stub(env: ActiveSupport::StringInquirer.new(environment)) + allow(Rails).to receive_messages(env: ActiveSupport::StringInquirer.new(environment)) end let(:credit_card) { Spree::CreditCard.new } @@ -39,7 +39,7 @@ module Spree :capture => @success_response, :void => @success_response, :credit => @success_response - @payment.stub payment_method: @payment_gateway + allow(@payment).to receive_messages payment_method: @payment_gateway end context "#can_capture?" do diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb index f68155edaf..294073a559 100644 --- a/spec/models/spree/gateway_spec.rb +++ b/spec/models/spree/gateway_spec.rb @@ -17,7 +17,7 @@ describe Spree::Gateway do it "passes through all arguments on a method_missing call" do gateway = TestGateway.new - gateway.provider.should_receive(:imaginary_method).with('foo') + expect(gateway.provider).to receive(:imaginary_method).with('foo') gateway.imaginary_method('foo') end end From 49a60374e6f4c48202b1899b09b7c4a42fa0a027 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 14:14:46 +0100 Subject: [PATCH 8/9] Remove dead method in payment method, it's a scope in OFN and remove unnecessary comments about spree --- app/models/spree/credit_card.rb | 2 -- app/models/spree/payment_method.rb | 12 +----------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 7f6b41bcb4..948dcf829d 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -2,8 +2,6 @@ module Spree class CreditCard < ActiveRecord::Base - # Should be able to remove once we reach Spree v2.2.0 - # https://github.com/spree/spree/commit/411010f3975c919ab298cb63962ee492455b415c belongs_to :payment_method belongs_to :user diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index a799eaebdd..39df36486d 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -13,7 +13,7 @@ module Spree DISPLAY = [:both, :front_end, :back_end].freeze default_scope -> { where(deleted_at: nil) } - has_many :credit_cards, class_name: "Spree::CreditCard" # from Spree v.2.3.0 d470b31798f37 + has_many :credit_cards, class_name: "Spree::CreditCard" validates :name, presence: true validate :distributor_validation @@ -22,7 +22,6 @@ module Spree scope :production, -> { where(environment: 'production') } - # -- Scopes scope :managed_by, lambda { |user| if user.has_spree_role?('admin') where(nil) @@ -48,7 +47,6 @@ module Spree scope :by_name, -> { order('spree_payment_methods.name ASC') } - # Rewrite Spree's ruby-land class method as a scope scope :available, lambda { |display_on = 'both'| where(active: true). where('spree_payment_methods.display_on=? OR spree_payment_methods.display_on=? OR spree_payment_methods.display_on IS NULL', display_on, ''). @@ -70,14 +68,6 @@ module Spree raise 'You must implement payment_source_class method for this gateway.' end - def self.available(display_on = 'both') - all.select do |p| - p.active && - (p.display_on == display_on.to_s || p.display_on.blank?) && - (p.environment == Rails.env || p.environment.blank?) - end - end - def self.active? where(type: to_s, environment: Rails.env, active: true).count.positive? end From 09b7aa134b77eaee9b1d2e395baebde1a815799a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 7 Aug 2020 17:31:39 +0100 Subject: [PATCH 9/9] Ammend payment method spec and specify a calculator so that the default calculator is not the spree one that is based on a calculator that does not exist in OFN: Spree::Calculator::FlatRate --- spec/features/admin/payment_method_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/features/admin/payment_method_spec.rb b/spec/features/admin/payment_method_spec.rb index 2cdc226bdf..851c1be1a0 100644 --- a/spec/features/admin/payment_method_spec.rb +++ b/spec/features/admin/payment_method_spec.rb @@ -81,7 +81,8 @@ feature ' end scenario "updating a payment method", js: true do - payment_method = create(:payment_method, distributors: [@distributors[0]]) + payment_method = create(:payment_method, distributors: [@distributors[0]], + calculator: build(:calculator_flat_rate)) login_as_admin_and_visit spree.edit_admin_payment_method_path payment_method fill_in 'payment_method_name', with: 'New PM Name'