fix(core): Fix refunds after failures & with multiple payments

Fixes #873. This commit also includes corrected logic for dealing with refunds on orders
with multiple payments.

packages/core/e2e/fixtures/test-payment-methods.ts

@@ -1,4 +1,4 @@
-import { PaymentMethodHandler } from '@vendure/core';
+import { Payment, PaymentMethodHandler, TransactionalConnection } from '@vendure/core';
 
 import { LanguageCode } from '../graphql/generated-e2e-admin-types';
 
@@ -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.
  */

packages/core/e2e/order.e2e-spec.ts

@@ -24,6 +24,7 @@ import {
     onTransitionSpy,
     partialPaymentMethod,
     singleStageRefundablePaymentMethod,
+    singleStageRefundFailingPaymentMethod,
     twoStagePaymentMethod,
 } from './fixtures/test-payment-methods';
 import { FULFILLMENT_FRAGMENT } from './graphql/fragments';
@@ -113,6 +114,7 @@ describe('Orders resolver', () => {
                     failsToSettlePaymentMethod,
                     singleStageRefundablePaymentMethod,
                     partialPaymentMethod,
+                    singleStageRefundFailingPaymentMethod,
                 ],
             },
         }),
@@ -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: [] },
@@ -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,
@@ -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,
@@ -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', () => {
@@ -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(
@@ -1821,6 +1875,7 @@ describe('Orders resolver', () => {
                 password,
             );
             orderId = result.orderId;
+            productInOrder = result.product;
         });
 
         it('adds a partial payment', async () => {
@@ -1879,7 +1934,7 @@ 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,
             });
@@ -1887,8 +1942,8 @@ describe('Orders resolver', () => {
                 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,
@@ -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,
             );
         });
 

packages/core/src/service/services/order.service.ts

@@ -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;
@@ -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();
         }
@@ -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);
         }

packages/core/src/service/services/payment.service.ts

@@ -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,
@@ -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: {},
             });
@@ -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!;