Skip to content

Commit

Permalink
feat(core): Improve customization of order process
Browse files Browse the repository at this point in the history
This commit enables composition of multiple new custom order process states, and also makes it possible to inject providers into the transition hooks. Relates to #401

BREAKING CHANGE: The way custom Order states are defined has changed. The `VendureConfig.orderOptions.process` property now accepts an **array** of objects implementing the `CustomerOrderProcess` interface. This interface is more-or-less the same as the old `OrderProcessOptions` object, but the use of an array now allows better composition, and since `CustomerOrderProcess` inherits from `InjectableStrategy`, this means providers can now be injected and used in the custom order process logic.
  • Loading branch information
michaelbromley committed Jul 9, 2020
1 parent 8ebe047 commit 0011ea9
Show file tree
Hide file tree
Showing 24 changed files with 509 additions and 100 deletions.
198 changes: 198 additions & 0 deletions packages/core/e2e/order-process.e2e-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
import { CustomOrderProcess, mergeConfig, OrderState } from '@vendure/core';
import { createTestEnvironment } from '@vendure/testing';
import path from 'path';

import { initialData } from '../../../e2e-common/e2e-initial-data';
import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config';

import {
AddItemToOrder,
GetNextOrderStates,
SetCustomerForOrder,
TransitionToState,
} from './graphql/generated-e2e-shop-types';
import {
ADD_ITEM_TO_ORDER,
GET_NEXT_STATES,
SET_CUSTOMER,
TRANSITION_TO_STATE,
} from './graphql/shop-definitions';

type TestOrderState = OrderState | 'ValidatingCustomer';

const initSpy = jest.fn();
const transitionStartSpy = jest.fn();
const transitionEndSpy = jest.fn();
const transitionEndSpy2 = jest.fn();
const transitionErrorSpy = jest.fn();

describe('Order process', () => {
const VALIDATION_ERROR_MESSAGE = 'Customer must have a company email address';
const customOrderProcess: CustomOrderProcess<'ValidatingCustomer'> = {
init(injector) {
initSpy(injector.getConnection().name);
},
transitions: {
AddingItems: {
to: ['ValidatingCustomer'],
mergeStrategy: 'replace',
},
ValidatingCustomer: {
to: ['ArrangingPayment', 'AddingItems'],
},
},
onTransitionStart(fromState, toState, data) {
transitionStartSpy(fromState, toState, data);
if (toState === 'ValidatingCustomer') {
if (!data.order.customer) {
return false;
}
if (!data.order.customer.emailAddress.includes('@company.com')) {
return VALIDATION_ERROR_MESSAGE;
}
}
},
onTransitionEnd(fromState, toState, data) {
transitionEndSpy(fromState, toState, data);
},
onError(fromState, toState, message) {
transitionErrorSpy(fromState, toState, message);
},
};

const customOrderProcess2: CustomOrderProcess<'ValidatingCustomer'> = {
transitions: {
ValidatingCustomer: {
to: ['Cancelled'],
},
},
onTransitionEnd(fromState, toState, data) {
transitionEndSpy2(fromState, toState, data);
},
};

const { server, adminClient, shopClient } = createTestEnvironment(
mergeConfig(testConfig, {
orderOptions: { process: [customOrderProcess, customOrderProcess2] },
}),
);

beforeAll(async () => {
await server.init({
initialData,
productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-full.csv'),
customerCount: 1,
});
await adminClient.asSuperAdmin();
}, TEST_SETUP_TIMEOUT_MS);

afterAll(async () => {
await server.destroy();
});

it('CustomOrderProcess is injectable', () => {
expect(initSpy).toHaveBeenCalledTimes(1);
expect(initSpy.mock.calls[0][0]).toBe('default');
});

it('replaced transition target', async () => {
await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(ADD_ITEM_TO_ORDER, {
productVariantId: 'T_1',
quantity: 1,
});

const { nextOrderStates } = await shopClient.query<GetNextOrderStates.Query>(GET_NEXT_STATES);

expect(nextOrderStates).toEqual(['ValidatingCustomer']);
});

it('custom onTransitionStart handler returning false', async () => {
transitionStartSpy.mockClear();
transitionEndSpy.mockClear();

const { transitionOrderToState } = await shopClient.query<
TransitionToState.Mutation,
TransitionToState.Variables
>(TRANSITION_TO_STATE, {
state: 'ValidatingCustomer',
});

expect(transitionStartSpy).toHaveBeenCalledTimes(1);
expect(transitionEndSpy).not.toHaveBeenCalled();
expect(transitionStartSpy.mock.calls[0].slice(0, 2)).toEqual(['AddingItems', 'ValidatingCustomer']);
expect(transitionOrderToState?.state).toBe('AddingItems');
});

it('custom onTransitionStart handler returning error message', async () => {
transitionStartSpy.mockClear();
transitionErrorSpy.mockClear();

await shopClient.query<SetCustomerForOrder.Mutation, SetCustomerForOrder.Variables>(SET_CUSTOMER, {
input: {
firstName: 'Joe',
lastName: 'Test',
emailAddress: '[email protected]',
},
});

try {
const { transitionOrderToState } = await shopClient.query<
TransitionToState.Mutation,
TransitionToState.Variables
>(TRANSITION_TO_STATE, {
state: 'ValidatingCustomer',
});
fail('Should have thrown');
} catch (e) {
expect(e.message).toContain(VALIDATION_ERROR_MESSAGE);
}

expect(transitionStartSpy).toHaveBeenCalledTimes(1);
expect(transitionErrorSpy).toHaveBeenCalledTimes(1);
expect(transitionEndSpy).not.toHaveBeenCalled();
expect(transitionErrorSpy.mock.calls[0]).toEqual([
'AddingItems',
'ValidatingCustomer',
VALIDATION_ERROR_MESSAGE,
]);
});

it('custom onTransitionStart handler allows transition', async () => {
transitionEndSpy.mockClear();

await shopClient.query<SetCustomerForOrder.Mutation, SetCustomerForOrder.Variables>(SET_CUSTOMER, {
input: {
firstName: 'Joe',
lastName: 'Test',
emailAddress: '[email protected]',
},
});

const { transitionOrderToState } = await shopClient.query<
TransitionToState.Mutation,
TransitionToState.Variables
>(TRANSITION_TO_STATE, {
state: 'ValidatingCustomer',
});

expect(transitionEndSpy).toHaveBeenCalledTimes(1);
expect(transitionEndSpy.mock.calls[0].slice(0, 2)).toEqual(['AddingItems', 'ValidatingCustomer']);
expect(transitionOrderToState?.state).toBe('ValidatingCustomer');
});

it('composes multiple CustomOrderProcesses', async () => {
transitionEndSpy.mockClear();
transitionEndSpy2.mockClear();

const { nextOrderStates } = await shopClient.query<GetNextOrderStates.Query>(GET_NEXT_STATES);

expect(nextOrderStates).toEqual(['ArrangingPayment', 'AddingItems', 'Cancelled']);

await shopClient.query<TransitionToState.Mutation, TransitionToState.Variables>(TRANSITION_TO_STATE, {
state: 'Cancelled',
});

expect(transitionEndSpy.mock.calls[0].slice(0, 2)).toEqual(['ValidatingCustomer', 'Cancelled']);
expect(transitionEndSpy2.mock.calls[0].slice(0, 2)).toEqual(['ValidatingCustomer', 'Cancelled']);
});
});
16 changes: 16 additions & 0 deletions packages/core/e2e/shop-order.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,22 @@ describe('Shop orders', () => {
expect(result2.activeOrder!.id).toBe(activeOrder.id);
});

it(
'cannot setCustomerForOrder when already logged in',
assertThrowsWithMessage(async () => {
await shopClient.query<SetCustomerForOrder.Mutation, SetCustomerForOrder.Variables>(
SET_CUSTOMER,
{
input: {
emailAddress: '[email protected]',
firstName: 'New',
lastName: 'Person',
},
},
);
}, 'Cannot set a Customer for the Order when already logged in'),
);

describe('shipping', () => {
let shippingMethods: GetShippingMethods.EligibleShippingMethods[];

Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/api/resolvers/shop/shop-order.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { QueryCountriesArgs } from '@vendure/common/lib/generated-types';
import ms from 'ms';

import { ForbiddenError, InternalServerError } from '../../../common/error/errors';
import { ForbiddenError, IllegalOperationError, InternalServerError } from '../../../common/error/errors';
import { Translated } from '../../../common/types/locale-types';
import { idsAreEqual } from '../../../common/utils';
import { Country } from '../../../entity';
Expand Down Expand Up @@ -303,6 +303,9 @@ export class ShopOrderResolver {
@Allow(Permission.Owner)
async setCustomerForOrder(@Ctx() ctx: RequestContext, @Args() args: MutationSetCustomerForOrderArgs) {
if (ctx.authorizedAsOwnerOnly) {
if (ctx.activeUserId) {
throw new IllegalOperationError('error.cannot-set-customer-for-order-when-logged-in');
}
const sessionOrder = await this.getOrderFromContext(ctx);
if (sessionOrder) {
const customer = await this.customerService.createOrUpdate(args.input, true);
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export class AppModule implements NestModule, OnApplicationBootstrap, OnApplicat
const { jobQueueStrategy } = this.configService.jobQueueOptions;
const { mergeStrategy, priceCalculationStrategy } = this.configService.orderOptions;
const { entityIdStrategy } = this.configService;
const { process } = this.configService.orderOptions;
return [
...adminAuthenticationStrategy,
...shopAuthenticationStrategy,
Expand All @@ -135,6 +136,7 @@ export class AppModule implements NestModule, OnApplicationBootstrap, OnApplicat
mergeStrategy,
entityIdStrategy,
priceCalculationStrategy,
...process,
];
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
import { Observable } from 'rxjs';

/**
* @description
* A type which is used to define all valid transitions and transition callbacks
*
* @docsCategory StateMachine
*/
export type Transitions<T extends string> = {
[S in T]: {
to: T[];
}
export type Transitions<State extends string, Target extends string = State> = {
[S in State]: {
to: Target[];
mergeStrategy?: 'merge' | 'replace';
};
};

/**
* @description
* The config object used to instantiate a new FSM instance.
*
* @docsCategory StateMachine
*/
export type StateMachineConfig<T extends string, Data = undefined> = {
transitions: Transitions<T>;
Expand All @@ -28,7 +35,10 @@ export type StateMachineConfig<T extends string, Data = undefined> = {
};

/**
* @description
* A simple type-safe finite state machine
*
* @docsCategory StateMachine
*/
export class FSM<T extends string, Data = any> {
private readonly _initialState: T;
Expand Down Expand Up @@ -65,7 +75,7 @@ export class FSM<T extends string, Data = any> {
if (typeof this.config.onTransitionStart === 'function') {
const transitionResult = this.config.onTransitionStart(this._currentState, state, data);
const canTransition = await (transitionResult instanceof Observable
? await transitionResult.toPromise()
? transitionResult.toPromise()
: transitionResult);
if (canTransition === false) {
return;

This comment has been minimized.

Copy link
@hendrik-advantitge

hendrik-advantitge Jul 11, 2020

Contributor

Should this.onError() not be called in this case as well?

This comment has been minimized.

Copy link
@michaelbromley

michaelbromley Jul 13, 2020

Author Member

There is a distinction between returning false and string from the onTransitionStart. false means cannot transition, but this is not an error. string means this is an error.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { Transitions } from './finite-state-machine';
import { mergeTransitionDefinitions } from './merge-transition-definitions';

describe('FSM mergeTransitionDefinitions()', () => {
it('handles no b', () => {
const a: Transitions<'Start' | 'End'> = {
Start: { to: ['End'] },
End: { to: [] },
};
const result = mergeTransitionDefinitions(a);

expect(result).toEqual(a);
});

it('adding new state, merge by default', () => {
const a: Transitions<'Start' | 'End'> = {
Start: { to: ['End'] },
End: { to: [] },
};
const b: Transitions<'Start' | 'Cancelled'> = {
Start: { to: ['Cancelled'] },
Cancelled: { to: [] },
};
const result = mergeTransitionDefinitions(a, b);

expect(result).toEqual({
Start: { to: ['End', 'Cancelled'] },
End: { to: [] },
Cancelled: { to: [] },
});
});

it('adding new state, replace', () => {
const a: Transitions<'Start' | 'End'> = {
Start: { to: ['End'] },
End: { to: [] },
};
const b: Transitions<'Start' | 'Cancelled'> = {
Start: { to: ['Cancelled'], mergeStrategy: 'replace' },
Cancelled: { to: ['Start'] },
};
const result = mergeTransitionDefinitions(a, b);

expect(result).toEqual({
Start: { to: ['Cancelled'] },
End: { to: [] },
Cancelled: { to: ['Start'] },
});
});

it('is an idempotent, pure function', () => {
const a: Transitions<'Start' | 'End'> = {
Start: { to: ['End'] },
End: { to: [] },
};
const aCopy = { ...a };
const b: Transitions<'Start' | 'Cancelled'> = {
Start: { to: ['Cancelled'] },
Cancelled: { to: ['Start'] },
};
let result = mergeTransitionDefinitions(a, b);
result = mergeTransitionDefinitions(a, b);

expect(a).toEqual(aCopy);
expect(result).toEqual({
Start: { to: ['End', 'Cancelled'] },
End: { to: [] },
Cancelled: { to: ['Start'] },
});
});
});
Loading

0 comments on commit 0011ea9

Please sign in to comment.