Browse Source

fix(core): Fix error when applying multiple promotions

Attempts to optimize the order calculation resulted in bad logic that caused multiple simultaneous promotions to break on the OrderItem level.
Michael Bromley 5 years ago
parent
commit
c807d32e52

+ 69 - 2
packages/core/e2e/order-promotion.e2e-spec.ts

@@ -25,6 +25,7 @@ import {
 import {
 import {
     AddItemToOrder,
     AddItemToOrder,
     AdjustItemQuantity,
     AdjustItemQuantity,
+    AdjustmentType,
     ApplyCouponCode,
     ApplyCouponCode,
     GetActiveOrder,
     GetActiveOrder,
     GetOrderPromotionsByCode,
     GetOrderPromotionsByCode,
@@ -436,6 +437,72 @@ describe('Promotions applied to Orders', () => {
 
 
             await deletePromotion(promotion.id);
             await deletePromotion(promotion.id);
         });
         });
+
+        it('multiple promotions simultaneously', async () => {
+            const { facets } = await adminClient.query<GetFacetList.Query>(GET_FACET_LIST);
+            const saleFacetValue = facets.items[0].values[0];
+            const promotion1 = await createPromotion({
+                enabled: true,
+                name: 'item promo',
+                couponCode: 'CODE1',
+                conditions: [],
+                actions: [
+                    {
+                        code: discountOnItemWithFacets.code,
+                        arguments: [
+                            { name: 'discount', type: 'int', value: '50' },
+                            { name: 'facets', type: 'facetValueIds', value: `["${saleFacetValue.id}"]` },
+                        ],
+                    },
+                ],
+            });
+            const promotion2 = await createPromotion({
+                enabled: true,
+                name: 'order promo',
+                couponCode: 'CODE2',
+                conditions: [],
+                actions: [
+                    {
+                        code: orderPercentageDiscount.code,
+                        arguments: [{ name: 'discount', type: 'int', value: '50' }],
+                    },
+                ],
+            });
+
+            await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(ADD_ITEM_TO_ORDER, {
+                productVariantId: getVariantBySlug('item-sale-12').id,
+                quantity: 1,
+            });
+
+            // Apply the OrderItem-level promo
+            const { applyCouponCode: apply1 } = await shopClient.query<
+                ApplyCouponCode.Mutation,
+                ApplyCouponCode.Variables
+            >(APPLY_COUPON_CODE, {
+                couponCode: 'CODE1',
+            });
+
+            expect(apply1?.lines[0].adjustments.length).toBe(2);
+            expect(
+                apply1?.lines[0].adjustments.find(a => a.type === AdjustmentType.PROMOTION)?.description,
+            ).toBe('item promo');
+            expect(apply1?.adjustments.length).toBe(0);
+
+            // Apply the Order-level promo
+            const { applyCouponCode: apply2 } = await shopClient.query<
+                ApplyCouponCode.Mutation,
+                ApplyCouponCode.Variables
+            >(APPLY_COUPON_CODE, {
+                couponCode: 'CODE2',
+            });
+
+            expect(apply2?.lines[0].adjustments.length).toBe(2);
+            expect(
+                apply2?.lines[0].adjustments.find(a => a.type === AdjustmentType.PROMOTION)?.description,
+            ).toBe('item promo');
+            expect(apply2?.adjustments.length).toBe(1);
+            expect(apply2?.adjustments[0].description).toBe('order promo');
+        });
     });
     });
 
 
     describe('per-customer usage limit', () => {
     describe('per-customer usage limit', () => {
@@ -513,8 +580,8 @@ describe('Promotions applied to Orders', () => {
                 >(GET_ORDER_PROMOTIONS_BY_CODE, {
                 >(GET_ORDER_PROMOTIONS_BY_CODE, {
                     code: orderCode,
                     code: orderCode,
                 });
                 });
-                expect(orderByCode!.promotions.map(pick(['id', 'name']))).toEqual([
-                    { id: 'T_9', name: 'Free with test coupon' },
+                expect(orderByCode!.promotions.map(pick(['name']))).toEqual([
+                    { name: 'Free with test coupon' },
                 ]);
                 ]);
             });
             });
 
 

+ 14 - 9
packages/core/src/service/helpers/order-calculator/order-calculator.ts

@@ -177,12 +177,18 @@ export class OrderCalculator {
             // which affected the order price.
             // which affected the order price.
             const applicablePromotions = await filterAsync(promotions, p => p.test(order, utils));
             const applicablePromotions = await filterAsync(promotions, p => p.test(order, utils));
 
 
+            const lineHasExistingPromotions =
+                line.items[0].pendingAdjustments &&
+                !!line.items[0].pendingAdjustments.find(a => a.type === AdjustmentType.PROMOTION);
             const forceUpdateItems = this.orderLineHasInapplicablePromotions(applicablePromotions, line);
             const forceUpdateItems = this.orderLineHasInapplicablePromotions(applicablePromotions, line);
+
+            if (forceUpdateItems || lineHasExistingPromotions) {
+                line.clearAdjustments(AdjustmentType.PROMOTION);
+            }
             if (forceUpdateItems) {
             if (forceUpdateItems) {
                 // This OrderLine contains Promotion adjustments for Promotions that are no longer
                 // This OrderLine contains Promotion adjustments for Promotions that are no longer
                 // applicable. So we know for sure we will need to update these OrderItems in the
                 // applicable. So we know for sure we will need to update these OrderItems in the
                 // DB. Therefore add them to the `updatedOrderItems` set.
                 // DB. Therefore add them to the `updatedOrderItems` set.
-                line.clearAdjustments(AdjustmentType.PROMOTION);
                 line.items.forEach(i => updatedOrderItems.add(i));
                 line.items.forEach(i => updatedOrderItems.add(i));
             }
             }
 
 
@@ -193,12 +199,6 @@ export class OrderCalculator {
                 // as to render later promotions no longer applicable.
                 // as to render later promotions no longer applicable.
                 if (await promotion.test(order, utils)) {
                 if (await promotion.test(order, utils)) {
                     for (const item of line.items) {
                     for (const item of line.items) {
-                        const itemHasPromotions =
-                            item.pendingAdjustments &&
-                            !!item.pendingAdjustments.find(a => a.type === AdjustmentType.PROMOTION);
-                        if (itemHasPromotions) {
-                            item.clearAdjustments(AdjustmentType.PROMOTION);
-                        }
                         const adjustment = await promotion.apply({
                         const adjustment = await promotion.apply({
                             orderItem: item,
                             orderItem: item,
                             orderLine: line,
                             orderLine: line,
@@ -208,8 +208,6 @@ export class OrderCalculator {
                             item.pendingAdjustments = item.pendingAdjustments.concat(adjustment);
                             item.pendingAdjustments = item.pendingAdjustments.concat(adjustment);
                             priceAdjusted = true;
                             priceAdjusted = true;
                             updatedOrderItems.add(item);
                             updatedOrderItems.add(item);
-                        } else if (itemHasPromotions) {
-                            updatedOrderItems.add(item);
                         }
                         }
                     }
                     }
                     if (priceAdjusted) {
                     if (priceAdjusted) {
@@ -218,6 +216,13 @@ export class OrderCalculator {
                     }
                     }
                 }
                 }
             }
             }
+            const lineNoLongerHasPromotions =
+                !line.items[0].pendingAdjustments ||
+                !line.items[0].pendingAdjustments.find(a => a.type === AdjustmentType.PROMOTION);
+            if (lineHasExistingPromotions && lineNoLongerHasPromotions) {
+                line.items.forEach(i => updatedOrderItems.add(i));
+            }
+
             if (forceUpdateItems) {
             if (forceUpdateItems) {
                 // If we are forcing an update, we need to ensure that totals get
                 // If we are forcing an update, we need to ensure that totals get
                 // re-calculated *even if* there are no applicable promotions (i.e.
                 // re-calculated *even if* there are no applicable promotions (i.e.