fix(core): Do no de-allocate OrderItems that were not allocated

Fixes #1557

packages/core/e2e/graphql/generated-e2e-admin-types.ts

@@ -7117,6 +7117,12 @@ export type TransitionFulfillmentToStateMutation = {
         | Pick<FulfillmentStateTransitionError, 'errorCode' | 'message' | 'transitionError'>;
 };
 
+export type UpdateOrderCustomFieldsMutationVariables = Exact<{
+    input: UpdateOrderInput;
+}>;
+
+export type UpdateOrderCustomFieldsMutation = { setOrderCustomFields?: Maybe<Pick<Order, 'id'>> };
+
 export type GetTagListQueryVariables = Exact<{
     options?: Maybe<TagListOptions>;
 }>;
@@ -9581,6 +9587,12 @@ export namespace TransitionFulfillmentToState {
     >;
 }
 
+export namespace UpdateOrderCustomFields {
+    export type Variables = UpdateOrderCustomFieldsMutationVariables;
+    export type Mutation = UpdateOrderCustomFieldsMutation;
+    export type SetOrderCustomFields = NonNullable<UpdateOrderCustomFieldsMutation['setOrderCustomFields']>;
+}
+
 export namespace GetTagList {
     export type Variables = GetTagListQueryVariables;
     export type Query = GetTagListQuery;

packages/core/e2e/stock-control.e2e-spec.ts

@@ -1,5 +1,13 @@
 /* tslint:disable:no-non-null-assertion */
-import { manualFulfillmentHandler, mergeConfig, OrderState } from '@vendure/core';
+import {
+    DefaultOrderPlacedStrategy,
+    manualFulfillmentHandler,
+    mergeConfig,
+    Order,
+    OrderPlacedStrategy,
+    OrderState,
+    RequestContext,
+} from '@vendure/core';
 import { createErrorResultGuard, createTestEnvironment, ErrorResultGuard } from '@vendure/testing';
 import gql from 'graphql-tag';
 import path from 'path';
@@ -67,13 +75,43 @@ import {
 import { assertThrowsWithMessage } from './utils/assert-throws-with-message';
 import { addPaymentToOrder, proceedToArrangingPayment } from './utils/test-order-utils';
 
+class TestOrderPlacedStrategy extends DefaultOrderPlacedStrategy {
+    shouldSetAsPlaced(
+        ctx: RequestContext,
+        fromState: OrderState,
+        toState: OrderState,
+        order: Order,
+    ): boolean {
+        if ((order.customFields as any).test1557 === true) {
+            // This branch is used in testing https://github.com/vendure-ecommerce/vendure/issues/1557
+            // i.e. it will cause the Order to be set to `active: false` but without creating any
+            // Allocations for the OrderLines.
+            if (fromState === 'AddingItems' && toState === 'ArrangingPayment') {
+                return true;
+            }
+            return false;
+        }
+        return super.shouldSetAsPlaced(ctx, fromState, toState, order);
+    }
+}
+
 describe('Stock control', () => {
     const { server, adminClient, shopClient } = createTestEnvironment(
         mergeConfig(testConfig(), {
             paymentOptions: {
                 paymentMethodHandlers: [testSuccessfulPaymentMethod, twoStagePaymentMethod],
             },
+            orderOptions: {
+                orderPlacedStrategy: new TestOrderPlacedStrategy(),
+            },
             customFields: {
+                Order: [
+                    {
+                        name: 'test1557',
+                        type: 'boolean',
+                        defaultValue: false,
+                    },
+                ],
                 OrderLine: [{ name: 'customization', type: 'string', nullable: true }],
             },
         }),
@@ -1171,6 +1209,72 @@ describe('Stock control', () => {
                 const { activeOrder } = await shopClient.query<GetActiveOrder.Query>(GET_ACTIVE_ORDER);
                 expect(activeOrder!.lines.length).toBe(0);
             });
+
+            // https://github.com/vendure-ecommerce/vendure/issues/1557
+            it('cancelling an Order only creates Releases for OrderItems that have actually been allocated', async () => {
+                const product = await getProductWithStockMovement('T_2');
+                const variant6 = product!.variants.find(v => v.id === variant6Id)!;
+                expect(variant6.stockOnHand).toBe(3);
+                expect(variant6.stockAllocated).toBe(0);
+
+                await shopClient.asUserWithCredentials('trevor_donnelly96@hotmail.com', 'test');
+                const { addItemToOrder: add1 } = await shopClient.query<
+                    AddItemToOrder.Mutation,
+                    AddItemToOrder.Variables
+                >(ADD_ITEM_TO_ORDER, {
+                    productVariantId: variant6.id,
+                    quantity: 1,
+                });
+                orderGuard.assertSuccess(add1);
+
+                // Set this flag so that our custom OrderPlacedStrategy uses the special logic
+                // designed to test this scenario.
+                const res = await shopClient.query(UPDATE_ORDER_CUSTOM_FIELDS, {
+                    input: { customFields: { test1557: true } },
+                });
+
+                await shopClient.query<SetShippingAddress.Mutation, SetShippingAddress.Variables>(
+                    SET_SHIPPING_ADDRESS,
+                    {
+                        input: {
+                            streetLine1: '1 Test Street',
+                            countryCode: 'GB',
+                        } as CreateAddressInput,
+                    },
+                );
+                await setFirstEligibleShippingMethod();
+                const { transitionOrderToState } = await shopClient.query<
+                    TransitionToState.Mutation,
+                    TransitionToState.Variables
+                >(TRANSITION_TO_STATE, { state: 'ArrangingPayment' });
+                orderGuard.assertSuccess(transitionOrderToState);
+                expect(transitionOrderToState.state).toBe('ArrangingPayment');
+
+                const product2 = await getProductWithStockMovement('T_2');
+                const variant6_2 = product2!.variants.find(v => v.id === variant6Id)!;
+                expect(variant6_2.stockOnHand).toBe(3);
+                expect(variant6_2.stockAllocated).toBe(0);
+
+                const { cancelOrder } = await adminClient.query<CancelOrder.Mutation, CancelOrder.Variables>(
+                    CANCEL_ORDER,
+                    {
+                        input: {
+                            orderId: transitionOrderToState.id,
+                            lines: transitionOrderToState.lines.map(l => ({
+                                orderLineId: l.id,
+                                quantity: l.quantity,
+                            })),
+                            reason: 'Cancelled by test',
+                        },
+                    },
+                );
+                orderGuard.assertSuccess(cancelOrder);
+
+                const product3 = await getProductWithStockMovement('T_2');
+                const variant6_3 = product3!.variants.find(v => v.id === variant6Id)!;
+                expect(variant6_3.stockOnHand).toBe(3);
+                expect(variant6_3.stockAllocated).toBe(0);
+            });
         });
     });
 
@@ -1328,3 +1432,17 @@ export const TRANSITION_FULFILLMENT_TO_STATE = gql`
         }
     }
 `;
+
+export const UPDATE_ORDER_CUSTOM_FIELDS = gql`
+    mutation UpdateOrderCustomFields($input: UpdateOrderInput!) {
+        setOrderCustomFields(input: $input) {
+            ... on Order {
+                id
+            }
+            ... on ErrorResult {
+                errorCode
+                message
+            }
+        }
+    }
+`;

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

@@ -89,6 +89,7 @@ import { ProductVariant } from '../../entity/product-variant/product-variant.ent
 import { Promotion } from '../../entity/promotion/promotion.entity';
 import { Refund } from '../../entity/refund/refund.entity';
 import { ShippingLine } from '../../entity/shipping-line/shipping-line.entity';
+import { Allocation } from '../../entity/stock-movement/allocation.entity';
 import { Surcharge } from '../../entity/surcharge/surcharge.entity';
 import { User } from '../../entity/user/user.entity';
 import { EventBus } from '../../event-bus/event-bus';
@@ -908,10 +909,10 @@ export class OrderService {
      * * Shipping or billing address changes
      *
      * Setting the `dryRun` input property to `true` will apply all changes, including updating the price of the
-     * Order, except history entry and additional payment actions. 
-     * 
+     * Order, except history entry and additional payment actions.
+     *
      * __Using dryRun option, you must wrap function call in transaction manually.__
-     * 
+     *
      */
     async modifyOrder(
         ctx: RequestContext,
@@ -1320,7 +1321,7 @@ export class OrderService {
         const fullOrder = await this.findOne(ctx, order.id);
 
         const soldItems = items.filter(i => !!i.fulfillment);
-        const allocatedItems = items.filter(i => !i.fulfillment);
+        const allocatedItems = await this.getAllocatedItems(ctx, items);
         await this.stockMovementService.createCancellationsForOrderItems(ctx, soldItems);
         await this.stockMovementService.createReleasesForOrderItems(ctx, allocatedItems);
         items.forEach(i => (i.cancelled = true));
@@ -1358,6 +1359,26 @@ export class OrderService {
         return orderItemsAreAllCancelled(orderWithItems);
     }
 
+    private async getAllocatedItems(ctx: RequestContext, items: OrderItem[]): Promise<OrderItem[]> {
+        const allocatedItems: OrderItem[] = [];
+        const allocationMap = new Map<ID, Allocation | false>();
+        for (const item of items) {
+            let allocation = allocationMap.get(item.lineId);
+            if (!allocation) {
+                allocation = await this.connection
+                    .getRepository(ctx, Allocation)
+                    .createQueryBuilder('allocation')
+                    .where('allocation.orderLine = :lineId', { lineId: item.lineId })
+                    .getOne();
+                allocationMap.set(item.lineId, allocation || false);
+            }
+            if (allocation && !item.fulfillment) {
+                allocatedItems.push(item);
+            }
+        }
+        return allocatedItems;
+    }
+
     /**
      * @description
      * Creates a {@link Refund} against the order and in doing so invokes the `createRefund()` method of the