Skip to content

Commit

Permalink
fix(core): Fix refunds after failures & with multiple payments
Browse files Browse the repository at this point in the history 
Fixes #873. This commit also includes corrected logic for dealing with refunds on orders
with multiple payments.
  • Loading branch information
@michaelbromley
michaelbromley committed May 11, 2021
1 parent 1795f48 commit ed30874
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 41 deletions.
38 changes: 36 additions & 2 deletions packages/core/e2e/fixtures/test-payment-methods.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { PaymentMethodHandler } from '@vendure/core';
import { Payment, PaymentMethodHandler, TransactionalConnection } from '@vendure/core';

import { LanguageCode } from '../graphql/generated-e2e-admin-types';

Expand Down Expand Up @@ -91,13 +91,47 @@ export const singleStageRefundablePaymentMethod = new PaymentMethodHandler({
},
createRefund: (ctx, input, amount, order, payment, args) => {
return {
amount,
state: 'Settled',
transactionId: 'abc123',
};
},
});

let connection: TransactionalConnection;
/**
* A payment method where a Refund attempt will fail the first time
*/
export const singleStageRefundFailingPaymentMethod = new PaymentMethodHandler({
code: 'single-stage-refund-failing-payment-method',
description: [{ languageCode: LanguageCode.en, value: 'Test Payment Method' }],
args: {},
init: injector => {
connection = injector.get(TransactionalConnection);
},
createPayment: (ctx, order, amount, args, metadata) => {
return {
amount,
state: 'Settled',
transactionId: '12345',
metadata,
};
},
settlePayment: () => {
return { success: true };
},
createRefund: async (ctx, input, amount, order, payment, args) => {
const paymentWithRefunds = await connection
.getRepository(ctx, Payment)
.findOne(payment.id, { relations: ['refunds'] });
const isFirstRefundAttempt = paymentWithRefunds?.refunds.length === 0;
const metadata = isFirstRefundAttempt ? { errorMessage: 'Service temporarily unavailable' } : {};
return {
state: isFirstRefundAttempt ? 'Failed' : 'Settled',
metadata,
};
},
});

/**
* A payment method where calling `settlePayment` always fails.
*/
Expand Down
133 changes: 113 additions & 20 deletions packages/core/e2e/order.e2e-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
onTransitionSpy,
partialPaymentMethod,
singleStageRefundablePaymentMethod,
singleStageRefundFailingPaymentMethod,
twoStagePaymentMethod,
} from './fixtures/test-payment-methods';
import { FULFILLMENT_FRAGMENT } from './graphql/fragments';
Expand Down Expand Up @@ -113,6 +114,7 @@ describe('Orders resolver', () => {
failsToSettlePaymentMethod,
singleStageRefundablePaymentMethod,
partialPaymentMethod,
singleStageRefundFailingPaymentMethod,
],
},
}),
Expand Down Expand Up @@ -146,6 +148,10 @@ describe('Orders resolver', () => {
name: singleStageRefundablePaymentMethod.code,
handler: { code: singleStageRefundablePaymentMethod.code, arguments: [] },
},
{
name: singleStageRefundFailingPaymentMethod.code,
handler: { code: singleStageRefundFailingPaymentMethod.code, arguments: [] },
},
{
name: partialPaymentMethod.code,
handler: { code: partialPaymentMethod.code, arguments: [] },
Expand Down Expand Up @@ -1550,6 +1556,22 @@ describe('Orders resolver', () => {
refundId = refundOrder.id;
});

it('manually settle a Refund', async () => {
const { settleRefund } = await adminClient.query<SettleRefund.Mutation, SettleRefund.Variables>(
SETTLE_REFUND,
{
input: {
id: refundId,
transactionId: 'aaabbb',
},
},
);
refundGuard.assertSuccess(settleRefund);

expect(settleRefund.state).toBe('Settled');
expect(settleRefund.transactionId).toBe('aaabbb');
});

it('returns error result if attempting to refund the same item more than once', async () => {
const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, {
id: orderId,
Expand All @@ -1573,22 +1595,6 @@ describe('Orders resolver', () => {
expect(refundOrder.errorCode).toBe(ErrorCode.QUANTITY_TOO_GREAT_ERROR);
});

it('manually settle a Refund', async () => {
const { settleRefund } = await adminClient.query<SettleRefund.Mutation, SettleRefund.Variables>(
SETTLE_REFUND,
{
input: {
id: refundId,
transactionId: 'aaabbb',
},
},
);
refundGuard.assertSuccess(settleRefund);

expect(settleRefund.state).toBe('Settled');
expect(settleRefund.transactionId).toBe('aaabbb');
});

it('order history contains expected entries', async () => {
const { order } = await adminClient.query<GetOrderHistory.Query, GetOrderHistory.Variables>(
GET_ORDER_HISTORY,
Expand Down Expand Up @@ -1655,6 +1661,53 @@ describe('Orders resolver', () => {
},
]);
});

// https://github.com/vendure-ecommerce/vendure/issues/873
it('can add another refund if the first one fails', async () => {
const orderResult = await createTestOrder(
adminClient,
shopClient,
customers[0].emailAddress,
password,
);
await proceedToArrangingPayment(shopClient);
const order = await addPaymentToOrder(shopClient, singleStageRefundFailingPaymentMethod);
orderGuard.assertSuccess(order);

expect(order.state).toBe('PaymentSettled');

const { refundOrder: refund1 } = await adminClient.query<
RefundOrder.Mutation,
RefundOrder.Variables
>(REFUND_ORDER, {
input: {
lines: order!.lines.map(l => ({ orderLineId: l.id, quantity: l.quantity })),
shipping: order!.shipping,
adjustment: 0,
reason: 'foo',
paymentId: order.payments![0].id,
},
});
refundGuard.assertSuccess(refund1);
expect(refund1.state).toBe('Failed');
expect(refund1.total).toBe(order.totalWithTax);

const { refundOrder: refund2 } = await adminClient.query<
RefundOrder.Mutation,
RefundOrder.Variables
>(REFUND_ORDER, {
input: {
lines: order!.lines.map(l => ({ orderLineId: l.id, quantity: l.quantity })),
shipping: order!.shipping,
adjustment: 0,
reason: 'foo',
paymentId: order.payments![0].id,
},
});
refundGuard.assertSuccess(refund2);
expect(refund2.state).toBe('Settled');
expect(refund2.total).toBe(order.totalWithTax);
});
});

describe('order notes', () => {
Expand Down Expand Up @@ -1812,6 +1865,7 @@ describe('Orders resolver', () => {
let orderTotalWithTax: number;
let payment1Id: string;
let payment2Id: string;
let productInOrder: GetProductWithVariants.Product;

beforeAll(async () => {
const result = await createTestOrder(
Expand All @@ -1821,6 +1875,7 @@ describe('Orders resolver', () => {
password,
);
orderId = result.orderId;
productInOrder = result.product;
});

it('adds a partial payment', async () => {
Expand Down Expand Up @@ -1879,16 +1934,16 @@ describe('Orders resolver', () => {
payment2Id = order.payments![1].id;
});

it('refunding order with multiple payments', async () => {
it('partial refunding of order with multiple payments', async () => {
const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, {
id: orderId,
});
const { refundOrder } = await adminClient.query<RefundOrder.Mutation, RefundOrder.Variables>(
REFUND_ORDER,
{
input: {
lines: order!.lines.map(l => ({ orderLineId: l.id, quantity: l.quantity })),
shipping: order!.shipping,
lines: order!.lines.map(l => ({ orderLineId: l.id, quantity: 1 })),
shipping: 0,
adjustment: 0,
reason: 'foo',
paymentId: payment1Id,
Expand All @@ -1910,7 +1965,45 @@ describe('Orders resolver', () => {

expect(orderWithPayments?.payments![1].refunds.length).toBe(1);
expect(orderWithPayments?.payments![1].refunds[0].total).toBe(
orderTotalWithTax - PARTIAL_PAYMENT_AMOUNT,
productInOrder.variants[0].priceWithTax - PARTIAL_PAYMENT_AMOUNT,
);
});

it('refunding remaining amount of order with multiple payments', async () => {
const { order } = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, {
id: orderId,
});
const { refundOrder } = await adminClient.query<RefundOrder.Mutation, RefundOrder.Variables>(
REFUND_ORDER,
{
input: {
lines: order!.lines.map(l => ({ orderLineId: l.id, quantity: 1 })),
shipping: order!.shippingWithTax,
adjustment: 0,
reason: 'foo',
paymentId: payment1Id,
},
},
);
refundGuard.assertSuccess(refundOrder);
expect(refundOrder.total).toBe(order!.totalWithTax - order!.lines[0].unitPriceWithTax);

const { order: orderWithPayments } = await adminClient.query<
GetOrderWithPayments.Query,
GetOrderWithPayments.Variables
>(GET_ORDER_WITH_PAYMENTS, {
id: orderId,
});

expect(orderWithPayments?.payments![0].refunds.length).toBe(1);
expect(orderWithPayments?.payments![0].refunds[0].total).toBe(PARTIAL_PAYMENT_AMOUNT);

expect(orderWithPayments?.payments![1].refunds.length).toBe(2);
expect(orderWithPayments?.payments![1].refunds[0].total).toBe(
productInOrder.variants[0].priceWithTax - PARTIAL_PAYMENT_AMOUNT,
);
expect(orderWithPayments?.payments![1].refunds[1].total).toBe(
productInOrder.variants[0].priceWithTax + order!.shippingWithTax,
);
});

Expand Down
14 changes: 10 additions & 4 deletions packages/core/src/service/services/order.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,10 +817,10 @@ export class OrderService {
order: Order,
): Promise<Order | OrderStateTransitionError> {
const orderId = order.id;
if (orderTotalIsCovered(order, 'Settled')) {
if (orderTotalIsCovered(order, 'Settled') && order.state !== 'PaymentSettled') {
return this.transitionToState(ctx, orderId, 'PaymentSettled');
}
if (orderTotalIsCovered(order, ['Authorized', 'Settled'])) {
if (orderTotalIsCovered(order, ['Authorized', 'Settled']) && order.state !== 'PaymentAuthorized') {
return this.transitionToState(ctx, orderId, 'PaymentAuthorized');
}
return order;
Expand Down Expand Up @@ -1078,7 +1078,11 @@ export class OrderService {
) {
return new NothingToRefundError();
}
const ordersAndItems = await this.getOrdersAndItemsFromLines(ctx, input.lines, i => !i.refund);
const ordersAndItems = await this.getOrdersAndItemsFromLines(
ctx,
input.lines,
i => i.refund?.state !== 'Settled',
);
if (!ordersAndItems) {
return new QuantityTooGreatError();
}
Expand All @@ -1100,7 +1104,9 @@ export class OrderService {
) {
return new RefundOrderStateError(order.state);
}
const alreadyRefunded = items.find(i => !!i.refundId);
const alreadyRefunded = items.find(
i => i.refund?.state === 'Pending' || i.refund?.state === 'Settled',
);
if (alreadyRefunded) {
return new AlreadyRefundedError(alreadyRefunded.refundId as string);
}
Expand Down
45 changes: 30 additions & 15 deletions packages/core/src/service/services/payment.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,28 +192,43 @@ export class PaymentService {
input: RefundOrderInput,
order: Order,
items: OrderItem[],
payment: Payment,
selectedPayment: Payment,
): Promise<Refund | RefundStateTransitionError> {
const orderWithRefunds = await this.connection.getEntityOrThrow(ctx, Order, order.id, {
relations: ['payments', 'payments.refunds'],
});
const existingRefunds =
orderWithRefunds.payments?.reduce((refunds, p) => [...refunds, ...p.refunds], [] as Refund[]) ??
[];

function paymentRefundTotal(payment: Payment): number {
const nonFailedRefunds = payment.refunds?.filter(refund => refund.state !== 'Failed') ?? [];
return summate(nonFailedRefunds, 'total');
}
const existingNonFailedRefunds =
orderWithRefunds.payments
?.reduce((refunds, p) => [...refunds, ...p.refunds], [] as Refund[])
.filter(refund => refund.state !== 'Failed') ?? [];
const refundablePayments = orderWithRefunds.payments.filter(p => {
return paymentRefundTotal(p) < p.amount;
});
const itemAmount = summate(items, 'proratedUnitPriceWithTax');
const refundTotal = itemAmount + input.shipping + input.adjustment;
let primaryRefund: Refund | undefined;
const refundedPaymentIds: ID[] = [];
let primaryRefund: Refund;
let refundOutstanding = refundTotal - summate(existingRefunds, 'total');
const refundTotal = itemAmount + input.shipping + input.adjustment;
const refundMax =
orderWithRefunds.payments
?.map(p => p.amount - paymentRefundTotal(p))
.reduce((sum, amount) => sum + amount, 0) ?? 0;
let refundOutstanding = Math.min(refundTotal, refundMax);
do {
const paymentToRefund =
refundedPaymentIds.length === 0
? payment
: orderWithRefunds.payments.find(p => !refundedPaymentIds.includes(p.id));
(refundedPaymentIds.length === 0 &&
refundablePayments.find(p => idsAreEqual(p.id, selectedPayment.id))) ||
refundablePayments.find(p => !refundedPaymentIds.includes(p.id)) ||
refundablePayments[0];
if (!paymentToRefund) {
throw new InternalServerError(`Could not find a Payment to refund`);
}
const total = Math.min(paymentToRefund.amount, refundOutstanding);
const amountNotRefunded = paymentToRefund.amount - paymentRefundTotal(paymentToRefund);
const total = Math.min(amountNotRefunded, refundOutstanding);
let refund = new Refund({
payment: paymentToRefund,
total,
Expand All @@ -222,7 +237,7 @@ export class PaymentService {
reason: input.reason,
adjustment: input.adjustment,
shipping: input.shipping,
method: payment.method,
method: selectedPayment.method,
state: 'Pending',
metadata: {},
});
Expand Down Expand Up @@ -255,12 +270,12 @@ export class PaymentService {
new RefundStateTransitionEvent(fromState, createRefundResult.state, ctx, refund, order),
);
}
if (idsAreEqual(paymentToRefund.id, payment.id)) {
if (primaryRefund == null) {
primaryRefund = refund;
}
existingRefunds.push(refund);
existingNonFailedRefunds.push(refund);
refundedPaymentIds.push(paymentToRefund.id);
refundOutstanding = refundTotal - summate(existingRefunds, 'total');
refundOutstanding = refundTotal - summate(existingNonFailedRefunds, 'total');
} while (0 < refundOutstanding);
// tslint:disable-next-line:no-non-null-assertion
return primaryRefund!;
Expand Down

0 comments on commit ed30874

Please sign in to comment.