fix(core): Correctly call PaymentMethodHandler.onStateTransitionStart

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

@@ -14,11 +14,12 @@ export const testSuccessfulPaymentMethod = new PaymentMethodHandler({
             metadata,
         };
     },
-    settlePayment: order => ({
+    settlePayment: (order) => ({
         success: true,
     }),
 });
 
+export const onTransitionSpy = jest.fn();
 /**
  * A two-stage (authorize, capture) payment method, with no createRefund method.
  */
@@ -42,6 +43,9 @@ export const twoStagePaymentMethod = new PaymentMethodHandler({
             },
         };
     },
+    onStateTransitionStart: (fromState, toState, data) => {
+        onTransitionSpy(fromState, toState, data);
+    },
 });
 
 /**
@@ -104,7 +108,7 @@ export const testFailingPaymentMethod = new PaymentMethodHandler({
             metadata,
         };
     },
-    settlePayment: order => ({
+    settlePayment: (order) => ({
         success: true,
     }),
 });
@@ -120,7 +124,7 @@ export const testErrorPaymentMethod = new PaymentMethodHandler({
             metadata,
         };
     },
-    settlePayment: order => ({
+    settlePayment: (order) => ({
         success: true,
     }),
 });

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

@@ -9,6 +9,7 @@ import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-conf
 
 import {
     failsToSettlePaymentMethod,
+    onTransitionSpy,
     singleStageRefundablePaymentMethod,
     twoStagePaymentMethod,
 } from './fixtures/test-payment-methods';
@@ -149,12 +150,16 @@ describe('Orders resolver', () => {
             expect(result.order!.state).toBe('PaymentAuthorized');
         });
 
-        it('settlePayment succeeds', async () => {
+        it('settlePayment succeeds, onStateTransitionStart called', async () => {
+            onTransitionSpy.mockClear();
             await shopClient.asUserWithCredentials(customers[1].emailAddress, password);
             await proceedToArrangingPayment(shopClient);
             const order = await addPaymentToOrder(shopClient, twoStagePaymentMethod);
 
             expect(order.state).toBe('PaymentAuthorized');
+            expect(onTransitionSpy).toHaveBeenCalledTimes(1);
+            expect(onTransitionSpy.mock.calls[0][0]).toBe('Created');
+            expect(onTransitionSpy.mock.calls[0][1]).toBe('Authorized');
 
             const payment = order.payments![0];
             const { settlePayment } = await adminClient.query<
@@ -171,6 +176,9 @@ describe('Orders resolver', () => {
                 baz: 'quux',
                 moreData: 42,
             });
+            expect(onTransitionSpy).toHaveBeenCalledTimes(2);
+            expect(onTransitionSpy.mock.calls[1][0]).toBe('Authorized');
+            expect(onTransitionSpy.mock.calls[1][1]).toBe('Settled');
 
             const result = await adminClient.query<GetOrder.Query, GetOrder.Variables>(GET_ORDER, {
                 id: order.id,

packages/core/src/common/finite-state-machine/finite-state-machine.ts

@@ -4,7 +4,8 @@ import { StateMachineConfig } from './types';
 
 /**
  * @description
- * A simple type-safe finite state machine
+ * A simple type-safe finite state machine. This is used internally to control the Order process, ensuring that
+ * the state of Orders, Payments and Refunds follows a well-defined behaviour.
  *
  * @docsCategory StateMachine
  */

packages/core/src/common/finite-state-machine/types.ts

@@ -80,7 +80,7 @@ export type OnTransitionEndFn<T extends string, Data> = (
 
 /**
  * @description
- * The config object used to instantiate a new FSM instance.
+ * The config object used to instantiate a new {@link FSM} instance.
  *
  * @docsCategory StateMachine
  * @docsPage StateMachineConfig

packages/core/src/config/payment-method/payment-method-handler.ts

@@ -9,7 +9,7 @@ import {
     ConfigurableOperationDefOptions,
     LocalizedStringArray,
 } from '../../common/configurable-operation';
-import { StateMachineConfig } from '../../common/finite-state-machine/types';
+import { OnTransitionStartFn, StateMachineConfig } from '../../common/finite-state-machine/types';
 import { Order } from '../../entity/order/order.entity';
 import { Payment, PaymentMetadata } from '../../entity/payment/payment.entity';
 import {
@@ -20,24 +20,9 @@ import { RefundState } from '../../service/helpers/refund-state-machine/refund-s
 
 export type PaymentMethodArgType = ConfigArgSubset<'int' | 'string' | 'boolean'>;
 export type PaymentMethodArgs = ConfigArgs<PaymentMethodArgType>;
-export type OnTransitionStartReturnType = ReturnType<Required<StateMachineConfig<any>>['onTransitionStart']>;
-
-/**
- * @description
- * The signature of the function defined by `onStateTransitionStart` in {@link PaymentMethodConfigOptions}.
- *
- * This function is called before the state of a Payment is transitioned. Its
- * return value used to determine whether the transition can occur.
- *
- * @docsCategory payment
- * @docsPage Payment Method Types
- */
-export type OnTransitionStartFn<T extends PaymentMethodArgs> = (
-    fromState: PaymentState,
-    toState: PaymentState,
-    args: ConfigArgValues<T>,
-    data: PaymentTransitionData,
-) => OnTransitionStartReturnType;
+export type OnPaymentTransitionStartReturnType = ReturnType<
+    Required<StateMachineConfig<any>>['onTransitionStart']
+>;
 
 /**
  * @description
@@ -172,7 +157,7 @@ export interface PaymentMethodConfigOptions<T extends PaymentMethodArgs = Paymen
      * This function, when specified, will be invoked before any transition from one {@link PaymentState} to another.
      * The return value (a sync / async `boolean`) is used to determine whether the transition is permitted.
      */
-    onStateTransitionStart?: OnTransitionStartFn<T>;
+    onStateTransitionStart?: OnTransitionStartFn<PaymentState, PaymentTransitionData>;
 }
 
 /**
@@ -233,7 +218,7 @@ export class PaymentMethodHandler<
     private readonly createPaymentFn: CreatePaymentFn<T>;
     private readonly settlePaymentFn: SettlePaymentFn<T>;
     private readonly createRefundFn?: CreateRefundFn<T>;
-    private readonly onTransitionStartFn?: OnTransitionStartFn<T>;
+    private readonly onTransitionStartFn?: OnTransitionStartFn<PaymentState, PaymentTransitionData>;
 
     constructor(config: PaymentMethodConfigOptions<T>) {
         super(config);
@@ -297,11 +282,10 @@ export class PaymentMethodHandler<
     onStateTransitionStart(
         fromState: PaymentState,
         toState: PaymentState,
-        args: ConfigArg[],
         data: PaymentTransitionData,
-    ): OnTransitionStartReturnType {
+    ): OnPaymentTransitionStartReturnType {
         if (typeof this.onTransitionStartFn === 'function') {
-            return this.onTransitionStartFn(fromState, toState, argsArrayToHash(args), data);
+            return this.onTransitionStartFn(fromState, toState, data);
         } else {
             return true;
         }

packages/core/src/service/helpers/payment-state-machine/payment-state-machine.ts

@@ -5,41 +5,22 @@ import { RequestContext } from '../../../api/common/request-context';
 import { IllegalOperationError } from '../../../common/error/errors';
 import { FSM } from '../../../common/finite-state-machine/finite-state-machine';
 import { StateMachineConfig } from '../../../common/finite-state-machine/types';
+import { awaitPromiseOrObservable } from '../../../common/utils';
 import { ConfigService } from '../../../config/config.service';
 import { Order } from '../../../entity/order/order.entity';
 import { Payment } from '../../../entity/payment/payment.entity';
 import { HistoryService } from '../../services/history.service';
+import { OrderState, OrderTransitionData } from '../order-state-machine/order-state';
 
 import { PaymentState, paymentStateTransitions, PaymentTransitionData } from './payment-state';
 
 @Injectable()
 export class PaymentStateMachine {
-    private readonly config: StateMachineConfig<PaymentState, PaymentTransitionData> = {
-        transitions: paymentStateTransitions,
-        onTransitionStart: async (fromState, toState, data) => {
-            return true;
-        },
-        onTransitionEnd: async (fromState, toState, data) => {
-            await this.historyService.createHistoryEntryForOrder({
-                ctx: data.ctx,
-                orderId: data.order.id,
-                type: HistoryEntryType.ORDER_PAYMENT_TRANSITION,
-                data: {
-                    paymentId: data.payment.id,
-                    from: fromState,
-                    to: toState,
-                },
-            });
-        },
-        onError: (fromState, toState, message) => {
-            throw new IllegalOperationError(message || 'error.cannot-transition-payment-from-to', {
-                fromState,
-                toState,
-            });
-        },
-    };
+    private readonly config: StateMachineConfig<PaymentState, PaymentTransitionData>;
 
-    constructor(private configService: ConfigService, private historyService: HistoryService) {}
+    constructor(private configService: ConfigService, private historyService: HistoryService) {
+        this.config = this.initConfig();
+    }
 
     getNextStates(payment: Payment): ReadonlyArray<PaymentState> {
         const fsm = new FSM(this.config, payment.state);
@@ -51,4 +32,41 @@ export class PaymentStateMachine {
         await fsm.transitionTo(state, { ctx, order, payment });
         payment.state = state;
     }
+
+    private initConfig(): StateMachineConfig<PaymentState, PaymentTransitionData> {
+        const { paymentMethodHandlers } = this.configService.paymentOptions;
+        return {
+            transitions: paymentStateTransitions,
+            onTransitionStart: async (fromState, toState, data) => {
+                for (const handler of paymentMethodHandlers) {
+                    if (data.payment.method === handler.code) {
+                        const result = await awaitPromiseOrObservable(
+                            handler.onStateTransitionStart(fromState, toState, data),
+                        );
+                        if (result !== true) {
+                            return result;
+                        }
+                    }
+                }
+            },
+            onTransitionEnd: async (fromState, toState, data) => {
+                await this.historyService.createHistoryEntryForOrder({
+                    ctx: data.ctx,
+                    orderId: data.order.id,
+                    type: HistoryEntryType.ORDER_PAYMENT_TRANSITION,
+                    data: {
+                        paymentId: data.payment.id,
+                        from: fromState,
+                        to: toState,
+                    },
+                });
+            },
+            onError: (fromState, toState, message) => {
+                throw new IllegalOperationError(message || 'error.cannot-transition-payment-from-to', {
+                    fromState,
+                    toState,
+                });
+            },
+        };
+    }
 }