Browse Source

fix(core): Fix OrderMergeStrategy implementation

Relates to #669. The old implementation was not able to correctly handle OrderLine removal nor
correctly account for custom fields.

BREAKING CHANGE: The signature of the `OrderMergeStrategy.merge()` method has changed. If you have
implemented a custom OrderMergeStrategy, you'll need to update it to return the expected type.
Michael Bromley 5 years ago
parent
commit
31930801fb

+ 44 - 15
packages/core/src/config/order/merge-orders-strategy.spec.ts

@@ -1,6 +1,6 @@
 import { RequestContext } from '../../api/common/request-context';
 import { Order } from '../../entity/order/order.entity';
-import { createOrderFromLines, parseLines } from '../../testing/order-test-utils';
+import { createOrderFromLines } from '../../testing/order-test-utils';
 
 import { MergeOrdersStrategy } from './merge-orders-strategy';
 
@@ -24,7 +24,7 @@ describe('MergeOrdersStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(guestLines);
+        expect(result).toEqual([{ orderLineId: 1, quantity: 2 }]);
     });
 
     it('guestOrder empty', () => {
@@ -34,7 +34,7 @@ describe('MergeOrdersStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(existingLines);
+        expect(result).toEqual([{ orderLineId: 1, quantity: 2 }]);
     });
 
     it('both orders have non-conflicting lines', () => {
@@ -52,16 +52,16 @@ describe('MergeOrdersStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual([
-            { lineId: 21, quantity: 2, productVariantId: 201 },
-            { lineId: 22, quantity: 1, productVariantId: 202 },
-            { lineId: 1, quantity: 1, productVariantId: 101 },
-            { lineId: 2, quantity: 1, productVariantId: 102 },
-            { lineId: 3, quantity: 1, productVariantId: 103 },
+        expect(result).toEqual([
+            { orderLineId: 21, quantity: 2 },
+            { orderLineId: 22, quantity: 1 },
+            { orderLineId: 1, quantity: 1 },
+            { orderLineId: 2, quantity: 1 },
+            { orderLineId: 3, quantity: 1 },
         ]);
     });
 
-    it('both orders have conflicting lines, some of which conflict', () => {
+    it('both orders have lines, some of which conflict', () => {
         const guestLines = [
             { lineId: 21, quantity: 2, productVariantId: 102 },
             { lineId: 22, quantity: 1, productVariantId: 202 },
@@ -76,11 +76,40 @@ describe('MergeOrdersStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual([
-            { lineId: 22, quantity: 1, productVariantId: 202 },
-            { lineId: 1, quantity: 1, productVariantId: 101 },
-            { lineId: 2, quantity: 1, productVariantId: 102 },
-            { lineId: 3, quantity: 1, productVariantId: 103 },
+        expect(result).toEqual([
+            { orderLineId: 22, quantity: 1 },
+            { orderLineId: 1, quantity: 1 },
+            { orderLineId: 2, quantity: 2 },
+            { orderLineId: 3, quantity: 1 },
+        ]);
+    });
+
+    it('equivalent customFields are merged', () => {
+        const guestLines = [{ lineId: 21, quantity: 2, productVariantId: 102, customFields: { foo: 'bar' } }];
+        const existingLines = [
+            { lineId: 2, quantity: 1, productVariantId: 102, customFields: { foo: 'bar' } },
+        ];
+        const guestOrder = createOrderFromLines(guestLines);
+        const existingOrder = createOrderFromLines(existingLines);
+
+        const result = strategy.merge(ctx, guestOrder, existingOrder);
+
+        expect(result).toEqual([{ orderLineId: 2, quantity: 2, customFields: { foo: 'bar' } }]);
+    });
+
+    it('differing customFields are not merged', () => {
+        const guestLines = [{ lineId: 21, quantity: 2, productVariantId: 102, customFields: { foo: 'bar' } }];
+        const existingLines = [
+            { lineId: 2, quantity: 1, productVariantId: 102, customFields: { foo: 'quux' } },
+        ];
+        const guestOrder = createOrderFromLines(guestLines);
+        const existingOrder = createOrderFromLines(existingLines);
+
+        const result = strategy.merge(ctx, guestOrder, existingOrder);
+
+        expect(result).toEqual([
+            { orderLineId: 21, quantity: 2, customFields: { foo: 'bar' } },
+            { orderLineId: 2, quantity: 1, customFields: { foo: 'quux' } },
         ]);
     });
 

+ 14 - 5
packages/core/src/config/order/merge-orders-strategy.ts

@@ -2,7 +2,7 @@ import { RequestContext } from '../../api/common/request-context';
 import { OrderLine } from '../../entity/order-line/order-line.entity';
 import { Order } from '../../entity/order/order.entity';
 
-import { OrderMergeStrategy } from './order-merge-strategy';
+import { MergedOrderLine, OrderMergeStrategy, toMergedOrderLine } from './order-merge-strategy';
 
 /**
  * @description
@@ -13,19 +13,28 @@ import { OrderMergeStrategy } from './order-merge-strategy';
  * @docsPage Merge Strategies
  */
 export class MergeOrdersStrategy implements OrderMergeStrategy {
-    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): OrderLine[] {
-        const mergedLines = existingOrder.lines.slice();
+    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): MergedOrderLine[] {
+        const mergedLines: MergedOrderLine[] = existingOrder.lines.map(toMergedOrderLine);
         const guestLines = guestOrder.lines.slice();
         for (const guestLine of guestLines.reverse()) {
             const existingLine = this.findCorrespondingLine(existingOrder, guestLine);
             if (!existingLine) {
-                mergedLines.unshift(guestLine);
+                mergedLines.unshift(toMergedOrderLine(guestLine));
+            } else {
+                const matchingMergedLine = mergedLines.find(l => l.orderLineId === existingLine.id);
+                if (matchingMergedLine) {
+                    matchingMergedLine.quantity = guestLine.quantity;
+                }
             }
         }
         return mergedLines;
     }
 
     private findCorrespondingLine(existingOrder: Order, guestLine: OrderLine): OrderLine | undefined {
-        return existingOrder.lines.find(line => line.productVariant.id === guestLine.productVariant.id);
+        return existingOrder.lines.find(
+            line =>
+                line.productVariant.id === guestLine.productVariant.id &&
+                JSON.stringify(line.customFields) === JSON.stringify(guestLine.customFields),
+        );
     }
 }

+ 26 - 1
packages/core/src/config/order/order-merge-strategy.ts

@@ -1,8 +1,31 @@
+import { ID } from '@vendure/common/lib/shared-types';
+
 import { RequestContext } from '../../api/common/request-context';
 import { InjectableStrategy } from '../../common/types/injectable-strategy';
 import { OrderLine } from '../../entity/order-line/order-line.entity';
 import { Order } from '../../entity/order/order.entity';
 
+/**
+ * @description
+ * The result of the {@link OrderMergeStrategy} `merge` method.
+ *
+ * @docsCategory orders
+ * @docsPage OrderMergeStrategy
+ */
+export interface MergedOrderLine {
+    orderLineId: ID;
+    quantity: number;
+    customFields?: any;
+}
+
+export function toMergedOrderLine(line: OrderLine): MergedOrderLine {
+    return {
+        orderLineId: line.id,
+        quantity: line.quantity,
+        customFields: line.customFields,
+    };
+}
+
 /**
  * @description
  * An OrderMergeStrategy defines what happens when a Customer with an existing Order
@@ -12,6 +35,8 @@ import { Order } from '../../entity/order/order.entity';
  * of OrderLines. The OrderMergeStrategy defines the rules governing this reconciliation.
  *
  * @docsCategory orders
+ * @docsPage OrderMergeStrategy
+ * @docsWeight 0
  */
 export interface OrderMergeStrategy extends InjectableStrategy {
     /**
@@ -19,5 +44,5 @@ export interface OrderMergeStrategy extends InjectableStrategy {
      * Merges the lines of the guest Order with those of the existing Order which is associated
      * with the active customer.
      */
-    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): OrderLine[];
+    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): MergedOrderLine[];
 }

+ 13 - 17
packages/core/src/config/order/use-existing-strategy.spec.ts

@@ -1,6 +1,6 @@
 import { RequestContext } from '../../api/common/request-context';
 import { Order } from '../../entity/order/order.entity';
-import { createOrderFromLines, parseLines } from '../../testing/order-test-utils';
+import { createOrderFromLines } from '../../testing/order-test-utils';
 
 import { UseExistingStrategy } from './use-existing-strategy';
 
@@ -24,7 +24,7 @@ describe('UseExistingStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual([]);
+        expect(result).toEqual([]);
     });
 
     it('guestOrder empty', () => {
@@ -34,7 +34,7 @@ describe('UseExistingStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(existingLines);
+        expect(result).toEqual([{ orderLineId: 1, quantity: 2 }]);
     });
 
     it('both orders have non-conflicting lines', () => {
@@ -52,7 +52,11 @@ describe('UseExistingStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(existingLines);
+        expect(result).toEqual([
+            { orderLineId: 1, quantity: 1 },
+            { orderLineId: 2, quantity: 1 },
+            { orderLineId: 3, quantity: 1 },
+        ]);
     });
 
     it('both orders have conflicting lines, some of which conflict', () => {
@@ -70,18 +74,10 @@ describe('UseExistingStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(existingLines);
-    });
-
-    it('returns a new array', () => {
-        const guestLines = [{ lineId: 21, quantity: 2, productVariantId: 102 }];
-        const existingLines = [{ lineId: 1, quantity: 1, productVariantId: 101 }];
-        const guestOrder = createOrderFromLines(guestLines);
-        const existingOrder = createOrderFromLines(existingLines);
-
-        const result = strategy.merge(ctx, guestOrder, existingOrder);
-
-        expect(result).not.toBe(guestOrder.lines);
-        expect(result).not.toBe(existingOrder.lines);
+        expect(result).toEqual([
+            { orderLineId: 1, quantity: 1 },
+            { orderLineId: 2, quantity: 1 },
+            { orderLineId: 3, quantity: 1 },
+        ]);
     });
 });

+ 3 - 4
packages/core/src/config/order/use-existing-strategy.ts

@@ -1,8 +1,7 @@
 import { RequestContext } from '../../api/common/request-context';
-import { OrderLine } from '../../entity/order-line/order-line.entity';
 import { Order } from '../../entity/order/order.entity';
 
-import { OrderMergeStrategy } from './order-merge-strategy';
+import { MergedOrderLine, OrderMergeStrategy, toMergedOrderLine } from './order-merge-strategy';
 
 /**
  * @description
@@ -12,7 +11,7 @@ import { OrderMergeStrategy } from './order-merge-strategy';
  * @docsPage Merge Strategies
  */
 export class UseExistingStrategy implements OrderMergeStrategy {
-    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): OrderLine[] {
-        return existingOrder.lines.slice();
+    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): MergedOrderLine[] {
+        return existingOrder.lines.map(toMergedOrderLine);
     }
 }

+ 14 - 18
packages/core/src/config/order/use-guest-if-existing-empty-strategy.spec.ts

@@ -1,6 +1,6 @@
 import { RequestContext } from '../../api/common/request-context';
 import { Order } from '../../entity/order/order.entity';
-import { createOrderFromLines, parseLines } from '../../testing/order-test-utils';
+import { createOrderFromLines } from '../../testing/order-test-utils';
 
 import { UseGuestIfExistingEmptyStrategy } from './use-guest-if-existing-empty-strategy';
 
@@ -24,7 +24,7 @@ describe('UseGuestIfExistingEmptyStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(guestLines);
+        expect(result).toEqual([{ orderLineId: 1, quantity: 2 }]);
     });
 
     it('guestOrder empty', () => {
@@ -34,7 +34,7 @@ describe('UseGuestIfExistingEmptyStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(existingLines);
+        expect(result).toEqual([{ orderLineId: 1, quantity: 2 }]);
     });
 
     it('both orders have non-conflicting lines', () => {
@@ -52,10 +52,14 @@ describe('UseGuestIfExistingEmptyStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(existingLines);
+        expect(result).toEqual([
+            { orderLineId: 1, quantity: 1 },
+            { orderLineId: 2, quantity: 1 },
+            { orderLineId: 3, quantity: 1 },
+        ]);
     });
 
-    it('both orders have conflicting lines, some of which conflict', () => {
+    it('both orders have lines, some of which conflict', () => {
         const guestLines = [
             { lineId: 21, quantity: 2, productVariantId: 102 },
             { lineId: 22, quantity: 1, productVariantId: 202 },
@@ -70,18 +74,10 @@ describe('UseGuestIfExistingEmptyStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(existingLines);
-    });
-
-    it('returns a new array', () => {
-        const guestLines = [{ lineId: 21, quantity: 2, productVariantId: 102 }];
-        const existingLines = [{ lineId: 1, quantity: 1, productVariantId: 101 }];
-        const guestOrder = createOrderFromLines(guestLines);
-        const existingOrder = createOrderFromLines(existingLines);
-
-        const result = strategy.merge(ctx, guestOrder, existingOrder);
-
-        expect(result).not.toBe(guestOrder.lines);
-        expect(result).not.toBe(existingOrder.lines);
+        expect(result).toEqual([
+            { orderLineId: 1, quantity: 1 },
+            { orderLineId: 2, quantity: 1 },
+            { orderLineId: 3, quantity: 1 },
+        ]);
     });
 });

+ 5 - 4
packages/core/src/config/order/use-guest-if-existing-empty-strategy.ts

@@ -1,8 +1,7 @@
 import { RequestContext } from '../../api/common/request-context';
-import { OrderLine } from '../../entity/order-line/order-line.entity';
 import { Order } from '../../entity/order/order.entity';
 
-import { OrderMergeStrategy } from './order-merge-strategy';
+import { MergedOrderLine, OrderMergeStrategy, toMergedOrderLine } from './order-merge-strategy';
 
 /**
  * @description
@@ -12,7 +11,9 @@ import { OrderMergeStrategy } from './order-merge-strategy';
  * @docsPage Merge Strategies
  */
 export class UseGuestIfExistingEmptyStrategy implements OrderMergeStrategy {
-    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): OrderLine[] {
-        return existingOrder.lines.length ? existingOrder.lines.slice() : guestOrder.lines.slice();
+    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): MergedOrderLine[] {
+        return existingOrder.lines.length
+            ? existingOrder.lines.map(toMergedOrderLine)
+            : guestOrder.lines.map(toMergedOrderLine);
     }
 }

+ 11 - 17
packages/core/src/config/order/use-guest-strategy.spec.ts

@@ -1,6 +1,6 @@
 import { RequestContext } from '../../api/common/request-context';
 import { Order } from '../../entity/order/order.entity';
-import { createOrderFromLines, parseLines } from '../../testing/order-test-utils';
+import { createOrderFromLines } from '../../testing/order-test-utils';
 
 import { UseGuestStrategy } from './use-guest-strategy';
 
@@ -24,7 +24,7 @@ describe('UseGuestStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(guestLines);
+        expect(result).toEqual([{ orderLineId: 1, quantity: 2 }]);
     });
 
     it('guestOrder empty', () => {
@@ -34,7 +34,7 @@ describe('UseGuestStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual([]);
+        expect(result).toEqual([]);
     });
 
     it('both orders have non-conflicting lines', () => {
@@ -52,7 +52,10 @@ describe('UseGuestStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(guestLines);
+        expect(result).toEqual([
+            { orderLineId: 21, quantity: 2 },
+            { orderLineId: 22, quantity: 1 },
+        ]);
     });
 
     it('both orders have conflicting lines, some of which conflict', () => {
@@ -70,18 +73,9 @@ describe('UseGuestStrategy', () => {
 
         const result = strategy.merge(ctx, guestOrder, existingOrder);
 
-        expect(parseLines(result)).toEqual(guestLines);
-    });
-
-    it('returns a new array', () => {
-        const guestLines = [{ lineId: 21, quantity: 2, productVariantId: 102 }];
-        const existingLines = [{ lineId: 1, quantity: 1, productVariantId: 101 }];
-        const guestOrder = createOrderFromLines(guestLines);
-        const existingOrder = createOrderFromLines(existingLines);
-
-        const result = strategy.merge(ctx, guestOrder, existingOrder);
-
-        expect(result).not.toBe(guestOrder.lines);
-        expect(result).not.toBe(existingOrder.lines);
+        expect(result).toEqual([
+            { orderLineId: 21, quantity: 2 },
+            { orderLineId: 22, quantity: 1 },
+        ]);
     });
 });

+ 3 - 4
packages/core/src/config/order/use-guest-strategy.ts

@@ -1,8 +1,7 @@
 import { RequestContext } from '../../api/common/request-context';
-import { OrderLine } from '../../entity/order-line/order-line.entity';
 import { Order } from '../../entity/order/order.entity';
 
-import { OrderMergeStrategy } from './order-merge-strategy';
+import { MergedOrderLine, OrderMergeStrategy, toMergedOrderLine } from './order-merge-strategy';
 
 /**
  * @description
@@ -12,7 +11,7 @@ import { OrderMergeStrategy } from './order-merge-strategy';
  * @docsPage Merge Strategies
  */
 export class UseGuestStrategy implements OrderMergeStrategy {
-    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): OrderLine[] {
-        return guestOrder.lines.slice();
+    merge(ctx: RequestContext, guestOrder: Order, existingOrder: Order): MergedOrderLine[] {
+        return guestOrder.lines.map(toMergedOrderLine);
     }
 }

+ 120 - 76
packages/core/src/service/helpers/order-merger/order-merger.spec.ts

@@ -13,81 +13,125 @@ describe('OrderMerger', () => {
     let orderMerger: OrderMerger;
     const ctx = RequestContext.empty();
 
-    beforeEach(async () => {
-        const module = await Test.createTestingModule({
-            providers: [OrderMerger, { provide: ConfigService, useClass: MockConfigService }],
-        }).compile();
-        const mockConfigService = module.get<ConfigService, MockConfigService>(ConfigService);
-        mockConfigService.orderOptions = {
-            mergeStrategy: new MergeOrdersStrategy(),
-        };
-        orderMerger = module.get(OrderMerger);
-    });
-
-    it('both orders undefined', () => {
-        const guestOrder = new Order({ lines: [] });
-        const existingOrder = new Order({ lines: [] });
-
-        const result = orderMerger.merge(ctx);
-
-        expect(result.order).toBeUndefined();
-        expect(result.linesToInsert).toBeUndefined();
-        expect(result.orderToDelete).toBeUndefined();
-    });
-
-    it('guestOrder undefined', () => {
-        const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
-
-        const result = orderMerger.merge(ctx, undefined, existingOrder);
-
-        expect(result.order).toBe(existingOrder);
-        expect(result.linesToInsert).toBeUndefined();
-        expect(result.orderToDelete).toBeUndefined();
-    });
-
-    it('existingOrder undefined', () => {
-        const guestOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
-
-        const result = orderMerger.merge(ctx, guestOrder, undefined);
-
-        expect(result.order).toBe(guestOrder);
-        expect(result.linesToInsert).toBeUndefined();
-        expect(result.orderToDelete).toBeUndefined();
-    });
-
-    it('empty guestOrder', () => {
-        const guestOrder = createOrderFromLines([]);
-        guestOrder.id = 42;
-        const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
-
-        const result = orderMerger.merge(ctx, guestOrder, existingOrder);
-
-        expect(result.order).toBe(existingOrder);
-        expect(result.linesToInsert).toBeUndefined();
-        expect(result.orderToDelete).toBe(guestOrder);
-    });
-
-    it('empty existingOrder', () => {
-        const guestOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
-        const existingOrder = createOrderFromLines([]);
-        existingOrder.id = 42;
-
-        const result = orderMerger.merge(ctx, guestOrder, existingOrder);
-
-        expect(result.order).toBe(guestOrder);
-        expect(result.linesToInsert).toBeUndefined();
-        expect(result.orderToDelete).toBe(existingOrder);
-    });
-
-    it('new lines added by merge', () => {
-        const guestOrder = createOrderFromLines([{ lineId: 20, quantity: 2, productVariantId: 200 }]);
-        guestOrder.id = 42;
-        const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
-
-        const result = orderMerger.merge(ctx, guestOrder, existingOrder);
-
-        expect(result.order).toBe(existingOrder);
-        expect(result.linesToInsert).toEqual([{ productVariantId: 200, quantity: 2 }]);
-        expect(result.orderToDelete).toBe(guestOrder);
+    describe('MergeOrdersStrategy', () => {
+        beforeEach(async () => {
+            const module = await Test.createTestingModule({
+                providers: [OrderMerger, { provide: ConfigService, useClass: MockConfigService }],
+            }).compile();
+            const mockConfigService = module.get<ConfigService, MockConfigService>(ConfigService);
+            mockConfigService.orderOptions = {
+                mergeStrategy: new MergeOrdersStrategy(),
+            };
+            orderMerger = module.get(OrderMerger);
+        });
+
+        it('both orders undefined', () => {
+            const guestOrder = new Order({ lines: [] });
+            const existingOrder = new Order({ lines: [] });
+
+            const result = orderMerger.merge(ctx);
+
+            expect(result.order).toBeUndefined();
+            expect(result.linesToInsert).toBeUndefined();
+            expect(result.linesToModify).toBeUndefined();
+            expect(result.linesToDelete).toBeUndefined();
+            expect(result.orderToDelete).toBeUndefined();
+        });
+
+        it('guestOrder undefined', () => {
+            const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
+
+            const result = orderMerger.merge(ctx, undefined, existingOrder);
+
+            expect(result.order).toBe(existingOrder);
+            expect(result.linesToInsert).toBeUndefined();
+            expect(result.linesToModify).toBeUndefined();
+            expect(result.linesToDelete).toBeUndefined();
+            expect(result.orderToDelete).toBeUndefined();
+        });
+
+        it('existingOrder undefined', () => {
+            const guestOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
+
+            const result = orderMerger.merge(ctx, guestOrder, undefined);
+
+            expect(result.order).toBe(guestOrder);
+            expect(result.linesToInsert).toBeUndefined();
+            expect(result.linesToModify).toBeUndefined();
+            expect(result.linesToDelete).toBeUndefined();
+            expect(result.orderToDelete).toBeUndefined();
+        });
+
+        it('empty guestOrder', () => {
+            const guestOrder = createOrderFromLines([]);
+            guestOrder.id = 42;
+            const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
+
+            const result = orderMerger.merge(ctx, guestOrder, existingOrder);
+
+            expect(result.order).toBe(existingOrder);
+            expect(result.linesToInsert).toBeUndefined();
+            expect(result.linesToModify).toBeUndefined();
+            expect(result.linesToDelete).toBeUndefined();
+            expect(result.orderToDelete).toBe(guestOrder);
+        });
+
+        it('empty existingOrder', () => {
+            const guestOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
+            const existingOrder = createOrderFromLines([]);
+            existingOrder.id = 42;
+
+            const result = orderMerger.merge(ctx, guestOrder, existingOrder);
+
+            expect(result.order).toBe(guestOrder);
+            expect(result.linesToInsert).toBeUndefined();
+            expect(result.linesToModify).toBeUndefined();
+            expect(result.linesToDelete).toBeUndefined();
+            expect(result.orderToDelete).toBe(existingOrder);
+        });
+
+        it('new lines added by merge', () => {
+            const guestOrder = createOrderFromLines([{ lineId: 20, quantity: 2, productVariantId: 200 }]);
+            guestOrder.id = 42;
+            const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
+
+            const result = orderMerger.merge(ctx, guestOrder, existingOrder);
+
+            expect(result.order).toBe(existingOrder);
+            expect(result.linesToInsert).toEqual([{ productVariantId: 200, quantity: 2 }]);
+            expect(result.linesToModify).toEqual([]);
+            expect(result.linesToDelete).toEqual([]);
+            expect(result.orderToDelete).toBe(guestOrder);
+        });
+
+        it('guest quantity replaces existing quantity', () => {
+            const guestOrder = createOrderFromLines([{ lineId: 20, quantity: 2, productVariantId: 100 }]);
+            guestOrder.id = 42;
+            const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 4, productVariantId: 100 }]);
+
+            const result = orderMerger.merge(ctx, guestOrder, existingOrder);
+
+            expect(result.order).toBe(existingOrder);
+            expect(result.linesToInsert).toEqual([]);
+            expect(result.linesToModify).toEqual([{ orderLineId: 1, quantity: 2 }]);
+            expect(result.linesToDelete).toEqual([]);
+            expect(result.orderToDelete).toBe(guestOrder);
+        });
+
+        it('takes customFields into account', () => {
+            const guestOrder = createOrderFromLines([
+                { lineId: 20, quantity: 2, productVariantId: 200, customFields: { foo: 'bar' } },
+            ]);
+            guestOrder.id = 42;
+            const existingOrder = createOrderFromLines([{ lineId: 1, quantity: 2, productVariantId: 100 }]);
+
+            const result = orderMerger.merge(ctx, guestOrder, existingOrder);
+
+            expect(result.order).toBe(existingOrder);
+            expect(result.linesToInsert).toEqual([
+                { productVariantId: 200, quantity: 2, customFields: { foo: 'bar' } },
+            ]);
+            expect(result.orderToDelete).toBe(guestOrder);
+        });
     });
 });

+ 59 - 14
packages/core/src/service/helpers/order-merger/order-merger.ts

@@ -1,8 +1,11 @@
 import { Injectable } from '@nestjs/common';
 import { ID } from '@vendure/common/lib/shared-types';
+import { notNullOrUndefined } from '@vendure/common/lib/shared-utils';
 
 import { RequestContext } from '../../../api/common/request-context';
+import { idsAreEqual } from '../../../common/utils';
 import { ConfigService } from '../../../config/config.service';
+import { MergedOrderLine } from '../../../config/order/order-merge-strategy';
 import { OrderLine } from '../../../entity/order-line/order-line.entity';
 import { Order } from '../../../entity/order/order.entity';
 
@@ -11,7 +14,9 @@ export type OrderWithEmptyLines = Order & { lines: ArrayLike<OrderLine> & { leng
 export type EmptyOrder = OrderWithEmptyLines | OrderWithNoLines;
 export type MergeResult = {
     order?: Order;
-    linesToInsert?: Array<{ productVariantId: ID; quantity: number }>;
+    linesToInsert?: Array<{ productVariantId: ID; quantity: number; customFields?: any }>;
+    linesToModify?: Array<{ orderLineId: ID; quantity: number; customFields?: any }>;
+    linesToDelete?: Array<{ orderLineId: ID }>;
     orderToDelete?: Order;
 };
 
@@ -30,6 +35,8 @@ export class OrderMerger {
             return {
                 order: existingOrder,
                 linesToInsert: this.getLinesToInsert(guestOrder, existingOrder, mergedLines),
+                linesToModify: this.getLinesToModify(guestOrder, existingOrder, mergedLines),
+                linesToDelete: this.getLinesToDelete(guestOrder, existingOrder, mergedLines),
                 orderToDelete: guestOrder,
             };
         } else if (
@@ -52,19 +59,57 @@ export class OrderMerger {
     private getLinesToInsert(
         guestOrder: Order,
         existingOrder: Order,
-        mergedLines: OrderLine[],
-    ): Array<{ productVariantId: ID; quantity: number }> {
-        const linesToInsert: Array<{ productVariantId: ID; quantity: number }> = [];
-        for (const line of mergedLines) {
-            if (
-                !existingOrder.lines.find(
-                    existingLine => existingLine.productVariant.id === line.productVariant.id,
-                )
-            ) {
-                linesToInsert.push({ productVariantId: line.productVariant.id, quantity: line.quantity });
-            }
-        }
-        return linesToInsert;
+        mergedLines: MergedOrderLine[],
+    ): Array<{ productVariantId: ID; quantity: number; customFields?: any }> {
+        return guestOrder.lines
+            .map(guestLine => {
+                const mergedLine = mergedLines.find(ml => idsAreEqual(ml.orderLineId, guestLine.id));
+                if (!mergedLine) {
+                    return;
+                }
+                return {
+                    productVariantId: guestLine.productVariant.id,
+                    quantity: mergedLine.quantity,
+                    customFields: mergedLine.customFields,
+                };
+            })
+            .filter(notNullOrUndefined);
+    }
+
+    private getLinesToModify(
+        guestOrder: Order,
+        existingOrder: Order,
+        mergedLines: MergedOrderLine[],
+    ): Array<{ orderLineId: ID; quantity: number; customFields?: any }> {
+        return existingOrder.lines
+            .map(existingLine => {
+                const mergedLine = mergedLines.find(ml => idsAreEqual(ml.orderLineId, existingLine.id));
+                if (!mergedLine) {
+                    return;
+                }
+                const lineIsModified =
+                    mergedLine.quantity !== existingLine.quantity ||
+                    JSON.stringify(mergedLine.customFields) !== JSON.stringify(existingLine.customFields);
+                if (!lineIsModified) {
+                    return;
+                }
+                return {
+                    orderLineId: mergedLine.orderLineId,
+                    quantity: mergedLine.quantity,
+                    customFields: mergedLine.customFields,
+                };
+            })
+            .filter(notNullOrUndefined);
+    }
+
+    private getLinesToDelete(
+        guestOrder: Order,
+        existingOrder: Order,
+        mergedLines: MergedOrderLine[],
+    ): Array<{ orderLineId: ID }> {
+        return existingOrder.lines
+            .filter(existingLine => !mergedLines.find(ml => idsAreEqual(ml.orderLineId, existingLine.id)))
+            .map(existingLine => ({ orderLineId: existingLine.id }));
     }
 
     private orderEmpty(order: Order | EmptyOrder): order is EmptyOrder {

+ 32 - 2
packages/core/src/service/services/order.service.ts

@@ -1182,7 +1182,7 @@ export class OrderService {
             return existingOrder;
         }
         const mergeResult = await this.orderMerger.merge(ctx, guestOrder, existingOrder);
-        const { orderToDelete, linesToInsert } = mergeResult;
+        const { orderToDelete, linesToInsert, linesToDelete, linesToModify } = mergeResult;
         let { order } = mergeResult;
         if (orderToDelete) {
             await this.connection.getRepository(ctx, Order).delete(orderToDelete.id);
@@ -1190,7 +1190,37 @@ export class OrderService {
         if (order && linesToInsert) {
             const orderId = order.id;
             for (const line of linesToInsert) {
-                const result = await this.addItemToOrder(ctx, orderId, line.productVariantId, line.quantity);
+                const result = await this.addItemToOrder(
+                    ctx,
+                    orderId,
+                    line.productVariantId,
+                    line.quantity,
+                    line.customFields,
+                );
+                if (!isGraphQlErrorResult(result)) {
+                    order = result;
+                }
+            }
+        }
+        if (order && linesToModify) {
+            const orderId = order.id;
+            for (const line of linesToModify) {
+                const result = await this.adjustOrderLine(
+                    ctx,
+                    orderId,
+                    line.orderLineId,
+                    line.quantity,
+                    line.customFields,
+                );
+                if (!isGraphQlErrorResult(result)) {
+                    order = result;
+                }
+            }
+        }
+        if (order && linesToDelete) {
+            const orderId = order.id;
+            for (const line of linesToDelete) {
+                const result = await this.removeItemFromOrder(ctx, orderId, line.orderLineId);
                 if (!isGraphQlErrorResult(result)) {
                     order = result;
                 }

+ 3 - 12
packages/core/src/testing/order-test-utils.ts

@@ -12,15 +12,16 @@ import { TaxCategory } from '../entity/tax-category/tax-category.entity';
 import { TaxRate } from '../entity/tax-rate/tax-rate.entity';
 import { Zone } from '../entity/zone/zone.entity';
 
-export type SimpleLine = { productVariantId: ID; quantity: number; lineId: ID };
+export type SimpleLine = { productVariantId: ID; quantity: number; lineId: ID; customFields?: any };
 
 export function createOrderFromLines(simpleLines: SimpleLine[]): Order {
     const lines = simpleLines.map(
-        ({ productVariantId, quantity, lineId }) =>
+        ({ productVariantId, quantity, lineId, customFields }) =>
             new OrderLine({
                 id: lineId,
                 productVariant: new ProductVariant({ id: productVariantId }),
                 items: Array.from({ length: quantity }).map(() => new OrderItem({})),
+                ...(customFields ? { customFields } : {}),
             }),
     );
 
@@ -29,16 +30,6 @@ export function createOrderFromLines(simpleLines: SimpleLine[]): Order {
     });
 }
 
-export function parseLines(lines: OrderLine[]): SimpleLine[] {
-    return lines.map(line => {
-        return {
-            lineId: line.id,
-            productVariantId: line.productVariant.id,
-            quantity: line.quantity,
-        };
-    });
-}
-
 export function createRequestContext(options: { pricesIncludeTax: boolean }): RequestContext {
     const channel = new Channel({
         defaultTaxZone: zoneDefault,