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

Do not allow successful checkout when order has only a void payment #3123

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 69 additions & 21 deletions backend/spec/features/admin/orders/payments_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,24 @@
describe 'Payments', type: :feature do
stub_authorization!

context "with a pre-existing payment" do
let!(:payment) do
create(:payment,
order: order,
amount: order.outstanding_balance,
let(:state) { 'checkout' }

def create_payment(opts = {})
create(
:payment,
{
order: order,
amount: order.outstanding_balance,
payment_method: create(:credit_card_payment_method),
state: state)
end
state: state
}.merge(opts)
)
end

context "with a pre-existing payment" do
let!(:payment) { create_payment }

let(:order) { create(:completed_order_with_totals, number: 'R100', line_items_price: 50) }
let(:state) { 'checkout' }

before do
visit "/admin/orders/#{order.number}/payments"
Expand All @@ -24,12 +31,7 @@
# Regression tests for https://github.com/spree/spree/issues/1453
context 'with a check payment', js: true do
let(:order) { create(:completed_order_with_totals, number: 'R100') }
let!(:payment) do
create(:payment,
order: order,
amount: order.outstanding_balance,
payment_method: create(:check_payment_method, available_to_admin: true)) # Check
end
let!(:payment) { create_payment(payment_method: create(:check_payment_method, available_to_admin: true)) }

it 'capturing a check payment from a new order' do
click_icon(:capture)
Expand Down Expand Up @@ -62,6 +64,57 @@
expect(page).to have_content 'my cc address'
end

context 'when there are multiple pending payments', :js do
context 'while marking all payments as void' do
let(:card_payment_method) { create(:credit_card_payment_method) }

let!(:payment) do
create_payment(
payment_method: card_payment_method,
state: :pending
)
end

let!(:second_payment) do
create_payment(
payment_method: card_payment_method,
state: :pending
)
end

it 'updates the order payment state correctly at each iteration' do
visit current_path
expect(page).to have_css('#payment_status', text: 'Balance due')

within '#payments' do
expect(page).to have_selector('.pill-pending', count: 2)
within "#payment_#{payment.id}" do
find('.fa-void').click
end
end

expect(page).to have_css('#payment_status', text: 'Balance due')

within '#payments' do
expect(page).to have_selector('.pill-pending', count: 1)
within "#payment_#{payment.id}" do
expect(page).to have_selector('.pill-void', count: 1)
end
end

within "#payment_#{second_payment.id}" do
find('.fa-void').click
end

within '#payments' do
expect(page).not_to have_selector('.pill-pending')
expect(page).to have_selector('.pill-void', count: 2)
end
expect(page).to have_css('#payment_status', text: 'Failed')
end
end
end

it 'lists, updates and creates payments for an order', js: true do
within_row(1) do
expect(column_text(3)).to eq('Credit Card')
Expand All @@ -70,7 +123,7 @@
end

click_icon :void
expect(page).to have_css('#payment_status', text: 'Balance due')
expect(page).to have_css('#payment_status', text: 'Failed')
expect(page).to have_content('Payment Updated')

within_row(1) do
Expand Down Expand Up @@ -211,12 +264,7 @@
context 'with a soft-deleted payment method' do
let(:order) { create(:completed_order_with_totals, line_items_count: 1) }
let!(:payment_method) { create(:check_payment_method) }
let!(:payment) do
create(:payment,
order: order,
amount: order.outstanding_balance,
payment_method: payment_method)
end
let!(:payment) { create_payment(payment_method: payment_method) }

before do
payment_method.discard
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/payment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class Payment < Spree::Base
scope :failed, -> { with_state('failed') }

scope :risky, -> { where("avs_response IN (?) OR (cvv_response_code IS NOT NULL and cvv_response_code != 'M') OR state = 'failed'", RISKY_AVS_CODES) }
scope :valid, -> { where.not(state: %w(failed invalid)) }
scope :valid, -> { where.not(state: %w(failed invalid void)) }

scope :store_credits, -> { where(source_type: Spree::StoreCredit.to_s) }
scope :not_store_credits, -> { where(arel_table[:source_type].not_eq(Spree::StoreCredit.to_s).or(arel_table[:source_type].eq(nil))) }
Expand Down
12 changes: 12 additions & 0 deletions core/spec/models/spree/payment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1255,4 +1255,16 @@
end
end
end

describe '::valid scope' do
before do
create :payment, state: :void
create :payment, state: :failed
create :payment, state: :invalid
end

it 'does not include void, failed and invalid payments' do
expect(described_class.valid).to be_empty
end
end
end
20 changes: 20 additions & 0 deletions frontend/spec/features/checkout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,26 @@
end
end

context "when order has only a void payment" do
let(:order) { Spree::TestingSupport::OrderWalkthrough.up_to(:payment) }

before do
user = create(:user)
order.user = user
order.recalculate

allow_any_instance_of(Spree::CheckoutController).to receive_messages(current_order: order)
allow_any_instance_of(Spree::CheckoutController).to receive_messages(try_spree_current_user: user)
end

it "does not allow successful order submission" do
visit spree.checkout_path
order.payments.first.update state: :void
click_button 'Place Order'
expect(page).to have_current_path spree.checkout_state_path(:payment)
end
spaghetticode marked this conversation as resolved.
Show resolved Hide resolved
end

# Regression test for https://github.com/spree/spree/issues/2694 and https://github.com/spree/spree/issues/4117
context "doesn't allow bad credit card numbers" do
let!(:payment_method) { create(:credit_card_payment_method) }
Expand Down