Merge pull request #7708 from andrewpbrett/payment-states

Add `requires_authorization` Payment state
This commit is contained in:
Pau Pérez Fabregat
2021-06-18 17:04:03 +02:00
committed by GitHub
28 changed files with 133 additions and 50 deletions

View File

@@ -84,6 +84,8 @@ $color-ste-sold-bg: $color-success !default;
$color-ste-sold-text: $color-1 !default;
$color-ste-pending-bg: $color-notice !default;
$color-ste-pending-text: $color-1 !default;
$color-ste-requires_authorization-bg: $color-notice !default;
$color-ste-requires_authorization-text: $color-1 !default;
$color-ste-awaiting_return-bg: $color-notice !default;
$color-ste-awaiting_return-text: $color-1 !default;
$color-ste-returned-bg: $color-notice !default;
@@ -124,9 +126,9 @@ $color-ste-inactive-bg: $color-notice !default;
$color-ste-inactive-text: $color-1 !default;
// Available states
$states: completed, complete, sold, pending, awaiting_return, returned, credit_owed, paid, shipped, balance_due, backorder, checkout, cart, address, delivery, payment, confirm, canceled, ready, void, active, inactive !default;
$states-bg-colors: $color-ste-completed-bg, $color-ste-complete-bg, $color-ste-sold-bg, $color-ste-pending-bg, $color-ste-awaiting_return-bg, $color-ste-returned-bg, $color-ste-credit_owed-bg, $color-ste-paid-bg, $color-ste-shipped-bg, $color-ste-balance_due-bg, $color-ste-backorder-bg, $color-ste-checkout-bg, $color-ste-cart-bg, $color-ste-address-bg, $color-ste-delivery-bg, $color-ste-payment-bg, $color-ste-confirm-bg, $color-ste-canceled-bg, $color-ste-ready-bg, $color-ste-void-bg, $color-ste-active-bg, $color-ste-inactive-bg !default;
$states-text-colors: $color-ste-completed-text, $color-ste-complete-text, $color-ste-sold-text, $color-ste-pending-text, $color-ste-awaiting_return-text, $color-ste-returned-text, $color-ste-credit_owed-text, $color-ste-paid-text, $color-ste-shipped-text, $color-ste-balance_due-text, $color-ste-backorder-text, $color-ste-checkout-text, $color-ste-cart-text, $color-ste-address-text, $color-ste-delivery-text, $color-ste-payment-text, $color-ste-confirm-text, $color-ste-canceled-text, $color-ste-ready-text, $color-ste-void-text, $color-ste-active-text, $color-ste-inactive-text !default;
$states: completed, complete, sold, pending, awaiting_return, returned, credit_owed, paid, shipped, balance_due, backorder, checkout, cart, address, delivery, payment, confirm, canceled, ready, void, requires_authorization, active, inactive !default;
$states-bg-colors: $color-ste-completed-bg, $color-ste-complete-bg, $color-ste-sold-bg, $color-ste-pending-bg, $color-ste-awaiting_return-bg, $color-ste-returned-bg, $color-ste-credit_owed-bg, $color-ste-paid-bg, $color-ste-shipped-bg, $color-ste-balance_due-bg, $color-ste-backorder-bg, $color-ste-checkout-bg, $color-ste-cart-bg, $color-ste-address-bg, $color-ste-delivery-bg, $color-ste-payment-bg, $color-ste-confirm-bg, $color-ste-canceled-bg, $color-ste-ready-bg, $color-ste-void-bg, $color-ste-requires_authorization-bg, $color-ste-active-bg, $color-ste-inactive-bg !default;
$states-text-colors: $color-ste-completed-text, $color-ste-complete-text, $color-ste-sold-text, $color-ste-pending-text, $color-ste-awaiting_return-text, $color-ste-returned-text, $color-ste-credit_owed-text, $color-ste-paid-text, $color-ste-shipped-text, $color-ste-balance_due-text, $color-ste-backorder-text, $color-ste-checkout-text, $color-ste-cart-text, $color-ste-address-text, $color-ste-delivery-text, $color-ste-payment-text, $color-ste-confirm-text, $color-ste-canceled-text, $color-ste-ready-text, $color-ste-void-text, $color-ste-requires_authorization-text, $color-ste-active-text, $color-ste-inactive-text !default;
// Available actions
$actions: edit, clone, remove, void, capture, save, cancel, mail !default;

View File

@@ -137,7 +137,7 @@ class CheckoutController < ::BaseController
last_payment = OrderPaymentFinder.new(@order).last_payment
@order.state == "payment" &&
last_payment&.state == "pending" &&
last_payment&.state == "requires_authorization" &&
last_payment&.response_code == params["payment_intent"]
end

View File

@@ -175,9 +175,11 @@ module Spree
@payment.authorize!(full_order_path(@payment.order))
raise Spree::Core::GatewayError, I18n.t('authorization_failure') unless @payment.pending?
unless @payment.pending? || @payment.requires_authorization?
raise Spree::Core::GatewayError, I18n.t('authorization_failure')
end
return unless @payment.authorization_action_required?
return unless @payment.requires_authorization?
PaymentMailer.authorize_payment(@payment).deliver_later
raise Spree::Core::GatewayError, I18n.t('action_required')

View File

@@ -83,12 +83,12 @@ module Spree
end
def can_resend_authorization_email?(payment)
payment.pending? && payment.authorization_action_required?
payment.requires_authorization?
end
# Indicates whether its possible to capture the payment
def can_capture?(payment)
return false if payment.authorization_action_required?
return false if payment.requires_authorization?
payment.pending? || payment.checkout?
end

View File

@@ -366,6 +366,7 @@ module Spree
# These are both valid states to process the payment
def pending_payments
(payments.select(&:pending?) +
payments.select(&:requires_authorization?) +
payments.select(&:processing?) +
payments.select(&:checkout?)).uniq
end

View File

@@ -50,17 +50,18 @@ module Spree
scope :failed, -> { with_state('failed') }
scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) }
scope :authorization_action_required, -> { where.not(cvv_response_message: nil) }
scope :requires_authorization, -> { with_state("requires_authorization") }
scope :with_payment_intent, ->(code) { where(response_code: code) }
# order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details)
state_machine initial: :checkout do
# With card payments, happens before purchase or authorization happens
event :started_processing do
transition from: [:checkout, :pending, :completed, :processing], to: :processing
transition from: [:checkout, :pending, :completed, :processing, :requires_authorization], to: :processing
end
# When processing during checkout fails
event :failure do
transition from: [:pending, :processing], to: :failed
transition from: [:pending, :processing, :requires_authorization], to: :failed
end
# With card payments this represents authorizing the payment
event :pend do
@@ -68,7 +69,7 @@ module Spree
end
# With card payments this represents completing a purchase or capture transaction
event :complete do
transition from: [:processing, :pending, :checkout], to: :completed
transition from: [:processing, :pending, :checkout, :requires_authorization], to: :completed
end
event :void do
transition from: [:pending, :completed, :checkout], to: :void
@@ -77,6 +78,15 @@ module Spree
event :invalidate do
transition from: [:checkout], to: :invalid
end
event :require_authorization do
transition from: [:checkout, :processing], to: :requires_authorization
end
event :failed_authorization do
transition from: [:requires_authorization], to: :failed
end
event :completed_authorization do
transition from: [:requires_authorization], to: :completed
end
end
def money
@@ -115,7 +125,7 @@ module Spree
end
def resend_authorization_email!
return unless authorization_action_required?
return unless requires_authorization?
PaymentMailer.authorize_payment(self).deliver_later
end
@@ -143,7 +153,7 @@ module Spree
I18n.t('payment_method_fee')
end
def mark_as_processed
def clear_authorization_url
update_attribute(:cvv_response_message, nil)
end

View File

@@ -11,7 +11,7 @@ module Spree
def process_offline!
return unless validate!
return if authorization_action_required?
return if requires_authorization?
if preauthorized?
capture!
@@ -182,10 +182,6 @@ module Spree
options
end
def authorization_action_required?
cvv_response_message.present?
end
private
def preauthorized?
@@ -240,6 +236,9 @@ module Spree
if response.cvv_result
self.cvv_response_code = response.cvv_result['code']
self.cvv_response_message = response.cvv_result['message']
if self.cvv_response_message.present?
return self.require_authorization!
end
end
end
__send__("#{success_state}!")

View File

@@ -7,7 +7,7 @@ class PaymentsRequiringAction
def query
Spree::Payment.joins(:order).where("spree_orders.user_id = ?", user.id).
authorization_action_required
requires_authorization
end
private

View File

@@ -41,7 +41,7 @@ module Api
# This methods requires to eager load the payment association (with its required WHERE
# constraints) so as not to cause and N+1.
def ready_to_capture
pending_payments = object.pending_payments.reject(&:authorization_action_required?)
pending_payments = object.pending_payments.reject(&:requires_authorization?)
object.payment_required? && pending_payments.any?
end

View File

@@ -26,7 +26,7 @@ class ProcessPaymentIntent
def initialize(payment_intent, order)
@payment_intent = payment_intent
@order = order
@payment = order.payments.pending.with_payment_intent(payment_intent).first
@payment = order.payments.requires_authorization.with_payment_intent(payment_intent).first
end
def call!
@@ -36,10 +36,13 @@ class ProcessPaymentIntent
process_payment
if payment.reload.completed?
payment.mark_as_processed
payment.completed_authorization
payment.clear_authorization_url
Result.new(ok: true)
else
payment.failed_authorization
payment.clear_authorization_url
Result.new(ok: false, error: I18n.t("payment_could_not_complete"))
end
rescue Stripe::StripeError => e

View File

@@ -13,7 +13,7 @@
%td.align-center= payment.display_amount.to_html
%td.align-center= link_to payment_method_name(payment), spree.admin_order_payment_path(@order, payment)
%td.align-center
%span{class: "state #{payment.state}"}= t(payment.state, scope: :payment_states, default: payment.state.capitalize)
%span{class: "state #{payment.state}"}= t(payment.state, scope: "spree.payment_states", default: payment.state.capitalize)
%td.actions
- payment.actions.each do |action|
= link_to_with_icon "icon-#{action}", Spree.t(action), fire_admin_order_payment_path(@order, payment, e: action), method: :put, no_text: true, data: {action: action, disable_with: ""}

View File

@@ -2789,6 +2789,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
failed: "failed"
paid: "paid"
pending: "pending"
requires_authorization: "authorization required"
processing: "processing"
void: "void"
invalid: "invalid"
@@ -3782,6 +3783,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
paid: paid
pending: pending
processing: processing
requires_authorization: "authorization required"
void: void
invalid: invalid
authorise: authorise

View File

@@ -0,0 +1,10 @@
class MigratePaymentsToAuthState < ActiveRecord::Migration[5.2]
class Spree::Payment < ApplicationRecord
scope :with_state, ->(s) { where(state: s.to_s) }
end
def change
Spree::Payment.where.not(cvv_response_message: nil).with_state("pending").
update_all(state: "requires_authorization")
end
end

View File

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2021_04_15_052410) do
ActiveRecord::Schema.define(version: 2021_05_27_201938) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

View File

@@ -6,7 +6,7 @@ module OrderManagement
def call!(redirect_url = full_order_path(@order))
super(redirect_url)
return unless @payment.authorization_action_required?
return unless @payment.requires_authorization?
PaymentMailer.authorize_payment(@payment).deliver_now
PaymentMailer.authorization_required(@payment).deliver_now

View File

@@ -15,7 +15,9 @@ module OrderManagement
@payment.authorize!(redirect_url)
@order.errors.add(:base, I18n.t('authorization_failure')) unless @payment.pending?
unless @payment.pending? || @payment.requires_authorization?
@order.errors.add(:base, I18n.t('authorization_failure'))
end
@payment
end

View File

@@ -172,6 +172,8 @@ module OrderManagement
'failed'
elsif canceled_and_not_paid_for?
'void'
elsif requires_authorization?
'requires_authorization'
else
infer_payment_state_from_balance
end
@@ -220,6 +222,10 @@ module OrderManagement
order.create_tax_charge!
end
def requires_authorization?
payments.requires_authorization.any? && payments.completed.empty?
end
end
end
end

View File

@@ -65,7 +65,7 @@ module OrderManagement
allow(PaymentMailer).to receive(:authorize_payment) { mail_mock }
allow(PaymentMailer).to receive(:authorization_required) { mail_mock }
allow(payment).to receive(:authorize!) {
payment.state = "pending"
payment.state = "requires_authorization"
payment.cvv_response_message = "https://stripe.com/redirect"
}
end

View File

@@ -153,6 +153,26 @@ module OrderManagement
end
end
context "when the order has a payment that requires authorization" do
let!(:payment) { create(:payment, order: order, state: "requires_authorization") }
it "returns requires_authorization" do
expect {
updater.update_payment_state
}.to change { order.payment_state }.to 'requires_authorization'
end
end
context "when the order has a payment that requires authorization and a completed payment" do
let!(:payment) { create(:payment, order: order, state: "requires_authorization") }
let!(:completed_payment) { create(:payment, order: order, state: "completed") }
it "returns paid" do
updater.update_payment_state
expect(order.payment_state).to_not eq("requires_authorization")
end
end
context "payment total is greater than order total" do
it "is credit_owed" do
order.payment_total = 2
@@ -206,6 +226,7 @@ module OrderManagement
order.total = 30
allow(order).to receive_message_chain(:payments, :valid, :empty?) { false }
allow(order).to receive_message_chain(:payments, :completed, :empty?) { false }
allow(order).to receive_message_chain(:payments, :requires_authorization, :any?) { false }
expect {
updater.update_payment_state

View File

@@ -147,7 +147,7 @@ describe CheckoutController, type: :controller do
create(
:payment,
amount: order.total,
state: "pending",
state: "requires_authorization",
payment_method: payment_method,
response_code: "pi_123"
)

View File

@@ -95,7 +95,7 @@ describe Spree::Admin::PaymentsController, type: :controller do
before do
allow_any_instance_of(Spree::Payment).to receive(:authorize!) do |payment|
payment.update cvv_response_message: "https://www.stripe.com/authorize"
payment.update state: "pending"
payment.update state: "requires_authorization"
end
end
it "redirects to new payment page with flash error" do
@@ -251,6 +251,7 @@ describe Spree::Admin::PaymentsController, type: :controller do
request.env["HTTP_REFERER"] = "http://foo.com"
allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
allow(payment).to receive(:cvv_response_message).and_return("https://www.stripe.com/authorize")
allow(payment).to receive(:requires_authorization?) { true }
end
it "resends the authorization email" do

View File

@@ -97,7 +97,7 @@ describe Spree::OrdersController, type: :controller do
cvv_response_message: "https://stripe.com/redirect",
response_code: "pi_123",
order: order,
state: "pending"
state: "requires_authorization"
)
}
@@ -172,7 +172,7 @@ describe Spree::OrdersController, type: :controller do
expect(flash[:error]).to eq("#{I18n.t('payment_could_not_process')}. error message")
payment.reload
expect(payment.cvv_response_message).to eq("https://stripe.com/redirect")
expect(payment.state).to eq("pending")
expect(payment.state).to eq("requires_authorization")
end
end
@@ -197,7 +197,7 @@ describe Spree::OrdersController, type: :controller do
expect(flash[:error]).to eq("#{I18n.t('payment_could_not_process')}. ")
payment.reload
expect(payment.cvv_response_message).to eq("https://stripe.com/redirect")
expect(payment.state).to eq("pending")
expect(payment.state).to eq("requires_authorization")
end
end
end

View File

@@ -73,7 +73,7 @@ feature '
redirect_url: "https://www.stripe.com/authorize"
end
it "fails to add a payment due to card error" do
it "adds the payment and it is in the requires_authorization state" do
login_as_admin_and_visit spree.new_admin_order_payment_path order
fill_in "payment_amount", with: order.total.to_s
@@ -81,8 +81,8 @@ feature '
click_button "Update"
expect(page).to have_link "StripeSCA"
expect(page).to have_content "PENDING"
expect(OrderPaymentFinder.new(order.reload).last_payment.state).to eq "pending"
expect(page).to have_content "AUTHORIZATION REQUIRED"
expect(OrderPaymentFinder.new(order.reload).last_payment.state).to eq "requires_authorization"
end
end
end

View File

@@ -15,7 +15,11 @@ feature "Payments requiring action", js: true do
context "there is a payment requiring authorization" do
let!(:payment) do
create(:payment, order: order, cvv_response_message: "https://stripe.com/redirect")
create(:payment,
order: order,
cvv_response_message: "https://stripe.com/redirect",
state: "requires_authorization"
)
end
it "shows a table of payments requiring authorization" do

View File

@@ -27,7 +27,7 @@ describe Spree::Payment do
double('success_response', success?: true,
authorization: '123',
avs_result: { 'code' => 'avs-code' },
cvv_result: { 'code' => 'cvv-code', 'message' => "CVV Result" })
cvv_result: { code: nil, message: nil })
end
let(:failed_response) { double('gateway_response', success?: false) }
@@ -161,8 +161,6 @@ describe Spree::Payment do
payment.authorize!
expect(payment.response_code).to eq('123')
expect(payment.avs_response).to eq('avs-code')
expect(payment.cvv_response_code).to eq('cvv-code')
expect(payment.cvv_response_message).to eq('CVV Result')
end
it "should make payment pending" do
@@ -171,6 +169,23 @@ describe Spree::Payment do
end
end
context "authorization is required" do
before do
allow(success_response).to receive(:cvv_result) {
{ 'code' => "123",
'message' => "https://stripe.com/redirect" }
}
expect(payment.payment_method).to receive(:authorize).with(
amount_in_cents, card, anything
).and_return(success_response)
end
it "should move the payment to requires_authorization" do
expect(payment).to receive(:require_authorization!)
payment.authorize!
end
end
context "if unsuccessful" do
it "should mark payment as failed" do
gateway.stub(:authorize).and_return(failed_response)
@@ -970,11 +985,11 @@ describe Spree::Payment do
end
end
describe "#mark_as_processed" do
describe "#clear_authorization_url" do
let(:payment) { create(:payment, cvv_response_message: "message") }
it "removes the cvv_response_message" do
payment.mark_as_processed
payment.clear_authorization_url
expect(payment.cvv_response_message).to eq(nil)
end
end

View File

@@ -10,7 +10,11 @@ describe PaymentsRequiringAction do
describe '#query' do
context "payment has a cvv_response_message" do
let(:payment) do
create(:payment, order: order, cvv_response_message: "https://stripe.com/redirect")
create(:payment,
order: order,
cvv_response_message: "https://stripe.com/redirect",
state: "requires_authorization"
)
end
it "finds the payment" do

View File

@@ -37,12 +37,12 @@ describe Api::Admin::OrderSerializer do
allow(order).to receive(:payment_required?) { true }
end
context "there is a pending payment requiring authorization" do
let!(:pending_payment) do
context "there is a payment requiring authorization" do
let!(:payment) do
create(
:payment,
order: order,
state: 'pending',
state: 'requires_authorization',
amount: 123.45,
cvv_response_message: "https://stripe.com/redirect"
)

View File

@@ -12,6 +12,7 @@ describe ProcessPaymentIntent do
state: "payment")
}
let(:payment_method) { create(:stripe_sca_payment_method) }
let!(:payment) {
create(
:payment,
@@ -19,7 +20,7 @@ describe ProcessPaymentIntent do
cvv_response_message: "https://stripe.com/redirect",
response_code: "pi_123",
order: order,
state: "pending"
state: "requires_authorization"
)
}
let(:validator) { instance_double(Stripe::PaymentIntentValidator) }
@@ -44,7 +45,7 @@ describe ProcessPaymentIntent do
it "does not complete the payment" do
service.call!
expect(payment.reload.state).to eq("pending")
expect(payment.reload.state).to eq("requires_authorization")
end
end
@@ -64,7 +65,7 @@ describe ProcessPaymentIntent do
it "does not complete the payment" do
service.call!
expect(payment.reload.state).to eq("pending")
expect(payment.reload.state).to eq("requires_authorization")
end
end
end
@@ -175,9 +176,9 @@ describe ProcessPaymentIntent do
expect(result.error).to eq(I18n.t("payment_could_not_complete"))
end
it "does not complete the payment" do
it "does fails the payment" do
service.call!
expect(payment.reload.state).to eq("pending")
expect(payment.reload.state).to eq("failed")
end
end
end