Skip to content

Commit

Permalink
Update order payment_total when voiding a payment
Browse files Browse the repository at this point in the history
As is, `payment_total` is only increased after successfully processing
a payment and never updated. This inconsistency breaks
`CustomerWithBalance` which relies on it.

Needless to say that if we keep this denormalized column, we have it
fixed.

I took the chance to rearrange tests a bit.
  • Loading branch information
sauloperez committed Nov 25, 2020
1 parent 09f7fb2 commit 80ab1ca
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 83 deletions.
5 changes: 5 additions & 0 deletions app/models/spree/payment/processing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,18 @@ def void_transaction!

if response.success?
self.response_code = response.authorization
update_payment_total
void
else
gateway_error(response)
end
end
end

def update_payment_total
order.update_attribute(:payment_total, order.payment_total - amount)
end

def credit!(credit_amount = nil)
protect_from_connection_error do
check_environment
Expand Down
65 changes: 62 additions & 3 deletions spec/controllers/admin/customers_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,68 @@ module Admin
spree_get :index, params
end

it 'includes the customer balance in the response' do
spree_get :index, params
expect(json_response.first["balance"]).to eq("$0.00")
context 'when the customer has no orders' do
it 'includes the customer balance in the response' do
spree_get :index, params
expect(json_response.first["balance"]).to eq("$0.00")
end
end

context 'when the customer has complete orders' do
let(:order) { create(:order, customer: customer, state: 'complete') }
let!(:line_item) { create(:line_item, order: order, price: 10.0) }

it 'includes the customer balance in the response' do
spree_get :index, params
expect(json_response.first["balance"]).to eq("$-10.00")
end
end

context 'when the customer has canceled orders' do
let(:order) { create(:order, customer: customer) }
let!(:line_item) { create(:line_item, order: order, price: 10.0) }
let!(:payment) { create(:payment, order: order, amount: order.total) }

before do
allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true)
order.process_payments!

order.update_attribute(:state, 'canceled')
end

it 'includes the customer balance in the response' do
spree_get :index, params
expect(json_response.first["balance"]).to eq("$10.00")
end
end

context 'when the customer has cart orders' do
let(:order) { create(:order, customer: customer, state: 'cart') }
let!(:line_item) { create(:line_item, order: order, price: 10.0) }

it 'includes the customer balance in the response' do
spree_get :index, params
expect(json_response.first["balance"]).to eq("$-10.00")
end
end

context 'when the customer has an order with a void payment' do
let(:order) { create(:order, customer: customer) }
let!(:line_item) { create(:line_item, order: order, price: 10.0) }
let!(:payment) { create(:payment, order: order, amount: order.total) }

before do
allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true)
order.process_payments!

payment.void_transaction!
end

it 'includes the customer balance in the response' do
expect(order.payment_total).to eq(0)
spree_get :index, params
expect(json_response.first["balance"]).to eq('$-10.00')
end
end
end

Expand Down
99 changes: 19 additions & 80 deletions spec/services/customers_with_balance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,14 @@
let(:total) { 200.00 }
let(:order_total) { 100.00 }

context 'when non-guest order' do
before do
create(:order, customer: customer, total: order_total, payment_total: 0)
create(:order, customer: customer, total: order_total, payment_total: 0)
end

it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(-total)
end
before do
create(:order, customer: customer, total: order_total, payment_total: 0)
create(:order, customer: customer, total: order_total, payment_total: 0)
end

context 'when guest order' do
before do
create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
end

it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(-total)
end
it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(-total)
end
end

Expand All @@ -42,78 +28,31 @@
let(:order_total) { 100.00 }
let(:payment_total) { order_total }

context 'when non-guest order' do
before do
create(:order, customer: customer, total: order_total, payment_total: 0)
create(:order, customer: customer, total: order_total, payment_total: payment_total)
end

it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(payment_total - total)
end
before do
create(:order, customer: customer, total: order_total, payment_total: 0)
create(:order, customer: customer, total: order_total, payment_total: payment_total)
end

context 'when guest order' do
before do
create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
create(
:order,
customer: customer,
user: nil,
total: order_total,
payment_total: payment_total
)
end

it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(payment_total - total)
end
it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(payment_total - total)
end
end

context 'when an order is canceled' do
let(:total) { 200.00 }
let(:order_total) { 100.00 }
let(:payment_total) { 100.00 }
let(:complete_orders_total) { order_total }

context 'when non-guest order' do
before do
create(:order, customer: customer, total: order_total, payment_total: 0)
create(
:order,
customer: customer,
total: order_total,
payment_total: order_total,
state: 'canceled'
)
end
let(:non_canceled_orders_total) { order_total }

it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(payment_total - complete_orders_total)
end
before do
create(:order, customer: customer, total: order_total, payment_total: 0)
create(:order, customer: customer, total: order_total, payment_total: order_total, state: 'canceled')
end

context 'when guest order' do
before do
create(:order, customer: customer, user: nil, total: order_total, payment_total: 0)
create(
:order,
customer: customer,
user: nil,
total: order_total,
payment_total: order_total,
state: 'canceled'
)
end

it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(payment_total - complete_orders_total)
end
it 'returns the customer balance' do
customer = customers_with_balance.query.first
expect(customer.balance_value).to eq(payment_total - non_canceled_orders_total)
end
end

Expand Down

0 comments on commit 80ab1ca

Please sign in to comment.