Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Spree::Wallet & Non credit card payment sources #1707

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
515a405
Move common payment source specs into shared example file
jordan-brough Apr 6, 2016
c08f334
Move common code from CreditCard into PaymentSource
jordan-brough Apr 21, 2016
a4434e0
Introduce Wallet and WalletSource classes
jordan-brough Apr 20, 2016
cb64c89
Add `PaymentSource#reusable?` concept
jordan-brough Apr 21, 2016
1f4dbf6
StoreCredit temporary workaround
jordan-brough Apr 21, 2016
2dd5293
Rename temporary_credit_card -> temporary_payment_source
jordan-brough Apr 21, 2016
8cdcfc2
Update persist_user_credit_card for the Wallet
jordan-brough Apr 21, 2016
9beab0a
Update assign_default_credit_card for the Wallet
jordan-brough Apr 21, 2016
e9c86f5
Allow passing `wallet_source_id` instead of `existing_card_id`
jordan-brough Apr 21, 2016
2435109
Use `wallet_source_id` instead of `existing_card` in frontend
jordan-brough Apr 21, 2016
f516b90
Deprecate existing_card and existing_card_id parameters
jordan-brough Apr 21, 2016
b43e95e
Allow payment sources that don't have `belongs_to :user`
jordan-brough Apr 21, 2016
458a947
Migrate `CreditCard#default` to Wallet, migrate data, drop column
jordan-brough Apr 21, 2016
97d3f73
Deprecate User#payment_sources
jordan-brough Apr 21, 2016
7dd840d
Update add_after_order_complete for non-PaymentSource sources
jordan-brough May 25, 2016
60ef289
Update Spree::StoreCredit to inherit from Spree::PaymentSource
peterberkenbosch Jun 1, 2016
abb73a5
Rename the wallet_source to wallet_payment_source
peterberkenbosch Jul 25, 2016
c0329e0
Add validation for payment_source on the wallet_payment_source.
peterberkenbosch Jul 25, 2016
5881475
Add another spec for WalletPaymentSource
jordan-brough Mar 10, 2017
224a9bc
Fetch reusable_sources from the wallet
Nov 7, 2016
e4611c0
Update comment and fix issue on payment builder
Nov 7, 2016
547d459
Update Spree::Gateway for payment sources
Nov 8, 2016
ad2a966
Fix deprecation warnings in PaymentSpec
Nov 8, 2016
b66c08e
Add specs for `Spree::Wallet`
Nov 9, 2016
88b3097
Update credit_cards relation to be in UserPaymentSource module
chrisradford Feb 16, 2017
a2a2b5c
Update deprecations to include replacement and deprecator
chrisradford Feb 16, 2017
a4585c2
Update specs to use Spree::Deprecation.silence
chrisradford Feb 16, 2017
1b7bba7
Rename wallet #default to #default_wallet_payment_source
chrisradford Feb 17, 2017
b11cf28
Update Wallet deprecations to use Spree::Deprecation
jordan-brough Feb 24, 2017
2e3bccf
Update Wallet#default_wallet_payment_source= to accept a wallet payme…
jordan-brough Feb 24, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions api/spec/controllers/spree/api/checkouts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ module Spree
payments_attributes: [
{
source_attributes: {
existing_card_id: credit_card.id.to_s,
wallet_payment_source_id: wallet_payment_source.id.to_param,
verification_value: '456'
}
}
Expand All @@ -277,7 +277,11 @@ module Spree
}
end

let!(:credit_card) do
let!(:wallet_payment_source) do
order.user.wallet.add(credit_card)
end

let(:credit_card) do
create(:credit_card, user_id: order.user_id, payment_method_id: @payment_method.id)
end

Expand All @@ -294,6 +298,34 @@ module Spree
expect(response.status).to eq 200
expect(order.credit_cards).to match_array [credit_card]
end

context 'with deprecated existing_card_id param' do
let(:params) do
{
id: order.to_param,
order_token: order.guest_token,
order: {
payments_attributes: [
{
source_attributes: {
existing_card_id: credit_card.id.to_param,
verification_value: '456'
}
}
]
}
}
end

it 'succeeds' do
Spree::Deprecation.silence do
api_put(:update, params)
end

expect(response.status).to eq 200
expect(order.credit_cards).to match_array [credit_card]
end
end
end

it "returns the order if the order is already complete" do
Expand Down
5 changes: 4 additions & 1 deletion backend/spec/features/admin/orders/new_order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,12 @@

# Regression test for https://github.com/spree/spree/issues/5327
context "customer with default credit card", js: true do
let!(:credit_card) { create(:credit_card, user: user) }

before do
create(:credit_card, default: true, user: user)
user.wallet.add(credit_card)
end

it "transitions to delivery not to complete" do
click_on 'Cart'

Expand Down
7 changes: 5 additions & 2 deletions backend/spec/features/admin/orders/payments_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,13 @@

context "user existing card" do
let!(:cc) do
create(:credit_card, user_id: order.user_id, payment_method: payment_method, gateway_customer_profile_id: "BGS-RFRE")
create(:credit_card, payment_method: payment_method, gateway_customer_profile_id: "BGS-RFRE")
end

before { visit spree.admin_order_payments_path(order) }
before do
order.user.wallet.add(cc)
visit spree.admin_order_payments_path(order)
end

it "is able to reuse customer payment source" do
expect(find("#card_#{cc.id}")).to be_checked
Expand Down
7 changes: 7 additions & 0 deletions core/app/models/concerns/spree/user_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ module UserMethods
has_many :store_credit_events, through: :store_credits
money_methods :total_available_store_credit

has_many :credit_cards, class_name: "Spree::CreditCard", foreign_key: :user_id
has_many :wallet_payment_sources, foreign_key: 'user_id', class_name: 'Spree::WalletPaymentSource', inverse_of: :user

after_create :auto_generate_spree_api_key

include Spree::RansackableAttributes unless included_modules.include?(Spree::RansackableAttributes)
Expand All @@ -31,6 +34,10 @@ module UserMethods
self.whitelisted_ransackable_attributes = %w[id email]
end

def wallet
Spree::Wallet.new(self)
end

# has_spree_role? simply needs to return true or false whether a user has a role or not.
def has_spree_role?(role_in_question)
spree_roles.any? { |role| role.name == role_in_question.to_s }
Expand Down
17 changes: 12 additions & 5 deletions core/app/models/concerns/spree/user_payment_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,22 @@ module Spree
module UserPaymentSource
extend ActiveSupport::Concern

included do
has_many :credit_cards, class_name: "Spree::CreditCard", foreign_key: :user_id
end

def default_credit_card
credit_cards.default.first
Spree::Deprecation.warn(
"user.default_credit_card is deprecated. Please use user.wallet.default_wallet_payment_source instead.",
caller
)
default = wallet.default_wallet_payment_source
if default && default.payment_source.is_a?(Spree::CreditCard)
default.payment_source
end
end

def payment_sources
Spree::Deprecation.warn(
"user.payment_sources is deprecated. Please use user.wallet.wallet_payment_sources instead.",
caller
)
credit_cards.with_payment_profile
end
end
Expand Down
70 changes: 29 additions & 41 deletions core/app/models/spree/credit_card.rb
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
module Spree
# The default `source` of a `Spree::Payment`.
#
class CreditCard < Spree::Base
belongs_to :payment_method
class CreditCard < Spree::PaymentSource
belongs_to :user, class_name: Spree::UserClassHandle.new, foreign_key: 'user_id'
belongs_to :address
has_many :payments, as: :source

before_save :set_last_digits

after_save :ensure_one_default

accepts_nested_attributes_for :address

attr_reader :number, :verification_value
attr_accessor :encrypted_data, :imported
attr_reader :number
attr_accessor :encrypted_data, :verification_value

validates :month, :year, numericality: { only_integer: true }, if: :require_card_numbers?, on: :create
validates :number, presence: true, if: :require_card_numbers?, on: :create, unless: :imported
validates :name, presence: true, if: :require_card_numbers?, on: :create
validates :verification_value, presence: true, if: :require_card_numbers?, on: :create, unless: :imported

scope :with_payment_profile, -> { where('gateway_customer_profile_id IS NOT NULL') }
scope :default, -> { where(default: true) }

def self.default
Spree::Deprecation.warn("CreditCard.default is deprecated. Please use Spree::Wallet instead.")
joins(:wallet_payment_sources).where(spree_wallet_payment_sources: { default: true })
end

# needed for some of the ActiveMerchant gateways (eg. SagePay)
alias_attribute :brand, :cc_type
Expand All @@ -44,6 +44,25 @@ class CreditCard < Spree::Base
'laser' => /^(6304|6706|6709|6771(?!89))\d{8}(\d{4}|\d{6,7})?$/
}.freeze

def default
Spree::Deprecation.warn("CreditCard.default is deprecated. Please use user.wallet.default_wallet_payment_source instead.", caller)
user.wallet.default_wallet_payment_source.payment_source == self
end

def default=(set_as_default)
Spree::Deprecation.warn("CreditCard.default= is deprecated. Please use user.wallet.default_wallet_payment_source= instead.", caller)
if set_as_default # setting this card as default
wallet_payment_source = user.wallet.add(self)
user.wallet.default_wallet_payment_source = wallet_payment_source
true
else # removing this card as default
if user.wallet.default_wallet_payment_source.try!(:payment_source) == self
user.wallet.default_wallet_payment_source = nil
end
false
end
end

def address_attributes=(attributes)
self.address = Spree::Address.immutable_merge(address, attributes)
end
Expand Down Expand Up @@ -123,31 +142,8 @@ def display_number
"XXXX-XXXX-XXXX-#{last_digits}"
end

# @return [Array<String>] the actions available on this credit card
def actions
%w{capture void credit}
end

# @param payment [Spree::Payment] the payment we want to know if can be captured
# @return [Boolean] true when the payment is in the pending or checkout states
def can_capture?(payment)
payment.pending? || payment.checkout?
end

# @param payment [Spree::Payment] the payment we want to know if can be voided
# @return [Boolean] true when the payment is not failed or voided
def can_void?(payment)
!payment.failed? && !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.
#
# @param payment [Spree::Payment] the payment we want to know if can be credited
# @return [Boolean] true when the payment is completed and can be credited
def can_credit?(payment)
payment.completed? && payment.credit_allowed > 0
def reusable?
has_payment_profile?
end

# @return [Boolean] true when there is a gateway customer or payment
Expand Down Expand Up @@ -192,13 +188,5 @@ def to_active_merchant
def require_card_numbers?
!encrypted_data.present? && !has_payment_profile?
end

def ensure_one_default
if user_id && default
Spree::CreditCard.where(default: true).where.not(id: id).where(user_id: user_id).each do |ucc|
ucc.update_columns(default: false, updated_at: Time.current)
end
end
end
end
end
10 changes: 6 additions & 4 deletions core/app/models/spree/gateway.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,18 @@ def supports?(source)
source.has_payment_profile?
end

def sources_by_order(order)
def reusable_sources_by_order(order)
source_ids = order.payments.where(payment_method_id: id).pluck(:source_id).uniq
payment_source_class.where(id: source_ids).with_payment_profile
payment_source_class.where(id: source_ids).select(&:reusable?)
end
alias_method :sources_by_order, :reusable_sources_by_order
deprecate sources_by_order: :reusable_sources_by_order, deprecator: Spree::Deprecation

def reusable_sources(order)
if order.completed?
sources_by_order(order)
reusable_sources_by_order(order)
elsif order.user_id
credit_cards.where(user_id: order.user_id).with_payment_profile
order.user.wallet.wallet_payment_sources.map(&:payment_source).select(&:reusable?)
else
[]
end
Expand Down
14 changes: 11 additions & 3 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,13 @@ class CannotRebuildShipments < StandardError; end
self.whitelisted_ransackable_attributes = %w[completed_at created_at email number state payment_state shipment_state total store_id]

attr_reader :coupon_code
attr_accessor :temporary_address, :temporary_credit_card
attr_accessor :temporary_address

attr_accessor :temporary_payment_source
alias_method :temporary_credit_card, :temporary_payment_source
alias_method :temporary_credit_card=, :temporary_payment_source=
deprecate temporary_credit_card: :temporary_payment_source, deprecator: Spree::Deprecation
deprecate :temporary_credit_card= => :temporary_payment_source=, deprecator: Spree::Deprecation

# Customer info
belongs_to :user, class_name: Spree::UserClassHandle.new
Expand Down Expand Up @@ -713,9 +719,9 @@ def add_payment_sources_to_wallet
add_to_wallet
end
alias_method :persist_user_credit_card, :add_payment_sources_to_wallet
deprecate :persist_user_credit_card
deprecate persist_user_credit_card: :add_payment_sources_to_wallet, deprecator: Spree::Deprecation

def assign_default_credit_card
def add_default_payment_from_wallet
builder = Spree::Config.default_payment_builder_class.new(self)

if payment = builder.build
Expand All @@ -728,6 +734,8 @@ def assign_default_credit_card
end
end
end
alias_method :assign_default_credit_card, :add_default_payment_from_wallet
deprecate assign_default_credit_card: :add_default_payment_from_wallet, deprecator: Spree::Deprecation

private

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order/checkout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def define_state_machine!

after_transition to: :complete, do: :add_payment_sources_to_wallet
before_transition to: :payment, do: :set_shipments_cost
before_transition to: :payment, do: :assign_default_credit_card
before_transition to: :payment, do: :add_default_payment_from_wallet

before_transition to: :confirm, do: :add_store_credit_payments

Expand Down
30 changes: 25 additions & 5 deletions core/app/models/spree/payment_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ class PaymentCreate
# @param attributes [Hash,ActionController::Parameters] attributes which are assigned to the new payment
# * :payment_method_id Id of payment method used for this payment
# * :source_attributes Attributes used to build the source of this payment. Usually a {CreditCard}
# * :existing_card_id (Integer) The id of an existing {CreditCard} object to use
# * :existing_card_id (Integer) Deprecated: The id of an existing {CreditCard} object to use
# * :wallet_payment_source_id (Integer): The id of a {WalletPaymentSource} to use
# @param request_env [Hash] rack env of user creating the payment
# @param payment [Payment] Internal use only. Instead of making a new payment, change the attributes for an existing one.
def initialize(order, attributes, payment: nil, request_env: {})
Expand All @@ -27,7 +28,13 @@ def build
@payment.attributes = @attributes

if source_attributes[:existing_card_id].present?
Spree::Deprecation.warn(
"Passing existing_card_id to PaymentCreate is deprecated. Use wallet_payment_source_id instead.",
caller,
)
build_existing_card
elsif source_attributes[:wallet_payment_source_id].present?
build_from_wallet_payment_source
else
build_source
end
Expand All @@ -44,20 +51,33 @@ def build_source
if source_attributes.present? && payment_method.try(:payment_source_class)
payment.source = payment_method.payment_source_class.new(source_attributes)
payment.source.payment_method_id = payment_method.id
payment.source.user_id = order.user_id if order
if order && payment.source.respond_to?(:user=)
payment.source.user = order.user
end
end
end

def build_from_wallet_payment_source
wallet_payment_source_id = source_attributes.fetch(:wallet_payment_source_id)
raise(ActiveRecord::RecordNotFound) if order.user.nil?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't raise here. An order does not necessarily have a user (guest orders don't have).

Copy link
Contributor

@jordan-brough jordan-brough Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method will only be used when there is a user for the order. The next line will fail with a NoMethodError: undefined method 'wallet' for nil:NilClass if there isn't a user.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but why raise then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to start of method with:

return if user.order.nil?

wallet_payment_source = order.user.wallet.find(wallet_payment_source_id)
raise(ActiveRecord::RecordNotFound) if wallet_payment_source.nil?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a find! method to the Wallet class instead? Then raise in that class rather than here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to a return if wallet_payment_source.nil?

build_from_payment_source(wallet_payment_source.payment_source)
end

def build_existing_card
credit_card = available_cards.find(source_attributes[:existing_card_id])
build_from_payment_source(credit_card)
end

def build_from_payment_source(payment_source)
# FIXME: does this work?
if source_attributes[:verification_value]
credit_card.verification_value = source_attributes[:verification_value]
payment_source.verification_value = source_attributes[:verification_value]
end

payment.source = credit_card
payment.payment_method_id = credit_card.payment_method_id
payment.source = payment_source
payment.payment_method_id = payment_source.payment_method_id
end

def available_cards
Expand Down
Loading