fix(core): Publish state transition events after persisting entities

Relates to #245. Previously, the events would be published *prior* to the associated Order etc. entities being persisted to the DB. This meant that any event subscribers which then attempted to update that Order entity would have their changes clobbered by the subsequent call to `.save()`, resulting in vexing bugs.

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

@@ -6,8 +6,6 @@ import { IllegalOperationError } from '../../../common/error/errors';
 import { FSM, StateMachineConfig, Transitions } from '../../../common/finite-state-machine';
 import { ConfigService } from '../../../config/config.service';
 import { Order } from '../../../entity/order/order.entity';
-import { EventBus } from '../../../event-bus/event-bus';
-import { OrderStateTransitionEvent } from '../../../event-bus/events/order-state-transition-event';
 import { HistoryService } from '../../services/history.service';
 import { PromotionService } from '../../services/promotion.service';
 import { StockMovementService } from '../../services/stock-movement.service';
@@ -24,7 +22,6 @@ export class OrderStateMachine {
         private stockMovementService: StockMovementService,
         private historyService: HistoryService,
         private promotionService: PromotionService,
-        private eventBus: EventBus,
     ) {
         this.config = this.initConfig();
     }
@@ -75,7 +72,6 @@ export class OrderStateMachine {
         if (toState === 'Cancelled') {
             data.order.active = false;
         }
-        this.eventBus.publish(new OrderStateTransitionEvent(fromState, toState, data.ctx, data.order));
         await this.historyService.createHistoryEntryForOrder({
             orderId: data.order.id,
             type: HistoryEntryType.ORDER_STATE_TRANSITION,

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

@@ -7,22 +7,18 @@ import { FSM, StateMachineConfig } from '../../../common/finite-state-machine';
 import { ConfigService } from '../../../config/config.service';
 import { Order } from '../../../entity/order/order.entity';
 import { Payment } from '../../../entity/payment/payment.entity';
-import { EventBus } from '../../../event-bus/event-bus';
-import { PaymentStateTransitionEvent } from '../../../event-bus/events/payment-state-transition-event';
 import { HistoryService } from '../../services/history.service';
 
 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) => {
-            this.eventBus.publish(new PaymentStateTransitionEvent(fromState, toState, data.ctx, data.payment, data.order));
             await this.historyService.createHistoryEntryForOrder({
                 ctx: data.ctx,
                 orderId: data.order.id,
@@ -42,9 +38,7 @@ export class PaymentStateMachine {
         },
     };
 
-    constructor(private configService: ConfigService,
-                private historyService: HistoryService,
-                private eventBus: EventBus) {}
+    constructor(private configService: ConfigService, private historyService: HistoryService) {}
 
     getNextStates(payment: Payment): PaymentState[] {
         const fsm = new FSM(this.config, payment.state);

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

@@ -7,22 +7,18 @@ import { FSM, StateMachineConfig } from '../../../common/finite-state-machine';
 import { ConfigService } from '../../../config/config.service';
 import { Order } from '../../../entity/order/order.entity';
 import { Refund } from '../../../entity/refund/refund.entity';
-import { EventBus } from '../../../event-bus/event-bus';
-import { RefundStateTransitionEvent } from '../../../event-bus/events/refund-state-transition-event';
 import { HistoryService } from '../../services/history.service';
 
 import { RefundState, refundStateTransitions, RefundTransitionData } from './refund-state';
 
 @Injectable()
 export class RefundStateMachine {
-
     private readonly config: StateMachineConfig<RefundState, RefundTransitionData> = {
         transitions: refundStateTransitions,
         onTransitionStart: async (fromState, toState, data) => {
             return true;
         },
         onTransitionEnd: async (fromState, toState, data) => {
-            this.eventBus.publish(new RefundStateTransitionEvent(fromState, toState, data.ctx, data.refund, data.order));
             await this.historyService.createHistoryEntryForOrder({
                 ctx: data.ctx,
                 orderId: data.order.id,
@@ -43,9 +39,7 @@ export class RefundStateMachine {
         },
     };
 
-    constructor(private configService: ConfigService,
-                private historyService: HistoryService,
-                private eventBus: EventBus) {}
+    constructor(private configService: ConfigService, private historyService: HistoryService) {}
 
     getNextStates(refund: Refund): RefundState[] {
         const fsm = new FSM(this.config, refund.state);

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

@@ -38,6 +38,10 @@ import { ProductVariant } from '../../entity/product-variant/product-variant.ent
 import { Promotion } from '../../entity/promotion/promotion.entity';
 import { Refund } from '../../entity/refund/refund.entity';
 import { User } from '../../entity/user/user.entity';
+import { EventBus } from '../../event-bus/event-bus';
+import { OrderStateTransitionEvent } from '../../event-bus/events/order-state-transition-event';
+import { PaymentStateTransitionEvent } from '../../event-bus/events/payment-state-transition-event';
+import { RefundStateTransitionEvent } from '../../event-bus/events/refund-state-transition-event';
 import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder';
 import { OrderCalculator } from '../helpers/order-calculator/order-calculator';
 import { OrderMerger } from '../helpers/order-merger/order-merger';
@@ -76,6 +80,7 @@ export class OrderService {
         private refundStateMachine: RefundStateMachine,
         private historyService: HistoryService,
         private promotionService: PromotionService,
+        private eventBus: EventBus,
     ) {}
 
     findAll(ctx: RequestContext, options?: ListQueryOptions<Order>): Promise<PaginatedList<Order>> {
@@ -378,8 +383,10 @@ export class OrderService {
 
     async transitionToState(ctx: RequestContext, orderId: ID, state: OrderState): Promise<Order> {
         const order = await this.getOrderOrThrow(ctx, orderId);
+        const fromState = order.state;
         await this.orderStateMachine.transition(ctx, order, state);
         await this.connection.getRepository(Order).save(order, { reload: false });
+        this.eventBus.publish(new OrderStateTransitionEvent(fromState, state, ctx, order));
         return order;
     }
 
@@ -423,9 +430,14 @@ export class OrderService {
         const payment = await getEntityOrThrow(this.connection, Payment, paymentId, { relations: ['order'] });
         const settlePaymentResult = await this.paymentMethodService.settlePayment(payment, payment.order);
         if (settlePaymentResult.success) {
-            await this.paymentStateMachine.transition(ctx, payment.order, payment, 'Settled');
+            const fromState = payment.state;
+            const toState = 'Settled';
+            await this.paymentStateMachine.transition(ctx, payment.order, payment, toState);
             payment.metadata = { ...payment.metadata, ...settlePaymentResult.metadata };
             await this.connection.getRepository(Payment).save(payment, { reload: false });
+            this.eventBus.publish(
+                new PaymentStateTransitionEvent(fromState, toState, ctx, payment, payment.order),
+            );
             if (payment.amount === payment.order.total) {
                 await this.transitionToState(ctx, payment.order.id, 'PaymentSettled');
             }
@@ -642,8 +654,14 @@ export class OrderService {
             relations: ['payment', 'payment.order'],
         });
         refund.transactionId = input.transactionId;
-        await this.refundStateMachine.transition(ctx, refund.payment.order, refund, 'Settled');
-        return this.connection.getRepository(Refund).save(refund);
+        const fromState = refund.state;
+        const toState = 'Settled';
+        await this.refundStateMachine.transition(ctx, refund.payment.order, refund, toState);
+        await this.connection.getRepository(Refund).save(refund);
+        this.eventBus.publish(
+            new RefundStateTransitionEvent(fromState, toState, ctx, refund, refund.payment.order),
+        );
+        return refund;
     }
 
     async addCustomerToOrder(ctx: RequestContext, orderId: ID, customer: Customer): Promise<Order> {

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

@@ -9,7 +9,6 @@ import { Connection } from 'typeorm';
 import { RequestContext } from '../../api/common/request-context';
 import { UserInputError } from '../../common/error/errors';
 import { ListQueryOptions } from '../../common/types/common-types';
-import { getConfig } from '../../config/config-helpers';
 import { ConfigService } from '../../config/config.service';
 import {
     PaymentMethodArgs,
@@ -17,11 +16,13 @@ import {
     PaymentMethodHandler,
 } from '../../config/payment-method/payment-method-handler';
 import { OrderItem } from '../../entity/order-item/order-item.entity';
-import { OrderLine } from '../../entity/order-line/order-line.entity';
 import { Order } from '../../entity/order/order.entity';
 import { PaymentMethod } from '../../entity/payment-method/payment-method.entity';
 import { Payment, PaymentMetadata } from '../../entity/payment/payment.entity';
 import { Refund } from '../../entity/refund/refund.entity';
+import { EventBus } from '../../event-bus/event-bus';
+import { PaymentStateTransitionEvent } from '../../event-bus/events/payment-state-transition-event';
+import { RefundStateTransitionEvent } from '../../event-bus/events/refund-state-transition-event';
 import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder';
 import { PaymentStateMachine } from '../helpers/payment-state-machine/payment-state-machine';
 import { RefundStateMachine } from '../helpers/refund-state-machine/refund-state-machine';
@@ -36,6 +37,7 @@ export class PaymentMethodService {
         private listQueryBuilder: ListQueryBuilder,
         private paymentStateMachine: PaymentStateMachine,
         private refundStateMachine: RefundStateMachine,
+        private eventBus: EventBus,
     ) {}
 
     async initPaymentMethods() {
@@ -78,11 +80,15 @@ export class PaymentMethodService {
     ): Promise<Payment> {
         const { paymentMethod, handler } = await this.getMethodAndHandler(method);
         const result = await handler.createPayment(order, paymentMethod.configArgs, metadata || {});
+        const initialState = 'Created';
         const payment = await this.connection
             .getRepository(Payment)
-            .save(new Payment({ ...result, state: 'Created' }));
+            .save(new Payment({ ...result, state: initialState }));
         await this.paymentStateMachine.transition(ctx, order, payment, result.state);
         await this.connection.getRepository(Payment).save(payment, { reload: false });
+        this.eventBus.publish(
+            new PaymentStateTransitionEvent(initialState, result.state, ctx, payment, payment.order),
+        );
         return payment;
     }
 
@@ -126,8 +132,12 @@ export class PaymentMethodService {
         }
         refund = await this.connection.getRepository(Refund).save(refund);
         if (createRefundResult) {
+            const fromState = refund.state;
             await this.refundStateMachine.transition(ctx, order, refund, createRefundResult.state);
             await this.connection.getRepository(Refund).save(refund, { reload: false });
+            this.eventBus.publish(
+                new RefundStateTransitionEvent(fromState, createRefundResult.state, ctx, refund, order),
+            );
         }
         return refund;
     }