Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Promotions Free Gift support #1798

Closed
michaelbromley opened this issue Sep 23, 2022 · 22 comments
Closed

Better Promotions Free Gift support #1798

michaelbromley opened this issue Sep 23, 2022 · 22 comments
Labels
design 📐 This issue deals with high-level design of a feature type: feature ✨ @vendure/core
Milestone

Comments

@michaelbromley
Copy link
Member

michaelbromley commented Sep 23, 2022

Is your feature request related to a problem? Please describe.
It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like
Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.
@michaelbromley michaelbromley added @vendure/core type: feature ✨ design 📐 This issue deals with high-level design of a feature labels Sep 23, 2022
@skid
Copy link
Contributor

skid commented Sep 23, 2022

In our case (online supermarket), this is a very common promotion. Here's the ideal implementation:

  1. The item is automatically added - this is A MUST because customers get confused and ask "where's my gift"
  2. The item is automatically removed if the condition is no longer met. Otherwise customers don't read their final statement and sometimes pay the full order then ask for refunds.
  3. Support for BOGO (buy one, get one free) promotions with auto add / remove. This is trickier than it sounds and maybe even impossible to do. The way we do this now is that we list the "free" items as a separate SKU which as a custom field "hidden" flag, which prevents it from being purchased regularly.

Here's how we solve the auto add / remove:

@VendurePlugin({
  imports: [PluginCommonModule],
  entities: [],
  providers: [PromoService],
})
export class PromoPlugin implements OnApplicationBootstrap {
  constructor(private orderService: OrderService, private promoService: PromoService) {}

  async onApplicationBootstrap() {
    const addItemToOrder = this.orderService.addItemToOrder;
    const adjustOrderLine = this.orderService.adjustOrderLine;
    const removeItemFromOrder = this.orderService.removeItemFromOrder;

    // TODO: Prevent adding of hidden items
    this.orderService.addItemToOrder = async (...args: Parameters<typeof addItemToOrder>) => {
      let order = await addItemToOrder.apply(this.orderService, args);

      if (order instanceof Order) {
        order = await this.promoService.autoAddPromoItems(args[0], addItemToOrder, adjustOrderLine, order);
      }

      if (order instanceof Order) {
        order.lines = order.lines.sort((a, b) => +a.createdAt - +b.createdAt);
      }

      return order;
    };

    // TODO: Prevent adding of hidden items
    this.orderService.adjustOrderLine = async (...args: Parameters<typeof adjustOrderLine>) => {
      let order = await adjustOrderLine.apply(this.orderService, args);

      if (order instanceof Order) {
        order = await this.promoService.autoAddPromoItems(args[0], addItemToOrder, adjustOrderLine, order);
      }

      if (order instanceof Order) {
        order.lines = order.lines.sort((a, b) => +a.createdAt - +b.createdAt);
      }

      return order;
    };

    this.orderService.removeItemFromOrder = async (...args: Parameters<typeof removeItemFromOrder>) => {
      let order = await removeItemFromOrder.apply(this.orderService, args);

      if (order instanceof Order) {
        order = await this.promoService.autoAddPromoItems(args[0], addItemToOrder, adjustOrderLine, order);
      }

      if (order instanceof Order) {
        order.lines = order.lines.sort((a, b) => +a.createdAt - +b.createdAt);
      }

      return order;
    };
  }
}

Here's the autoAdd method. As you can see it depends directly on having specific conditions and actions active.

@Injectable()
export class PromoService {
  constructor(
    private orderService: OrderService,
    private promotionService: PromotionService,
    private conn: TransactionalConnection,
  ) {}

  async autoAddPromoItems(
    ctx: RequestContext,
    addItemToOrderFn: typeof this.orderService.addItemToOrder,
    adjustOrderLineFn: typeof this.orderService.adjustOrderLine,
    order: Order,
  ) {
    const { items: promotions } = await this.promotionService.findAll(ctx, {
      filter: { enabled: { eq: true } },
      sort: { priorityScore: 'ASC' },
    });

    // Tracks which variantIds should be added or removed
    const updates: { [key: string]: number } = {};

    for (const promotion of promotions) {
      const condition = promotion.conditions.find(c => c.code === 'buy_x_of_y');
      const action = promotion.actions.find(a => a.code === 'get_x_free');

      if (!condition || !action) {
        // Auto-adding and removing only works for BOGO-type promotions
        continue;
      }

      const { amount, variantIds, autoAdd } = getXFreeArgParser(action.args);

      if (!autoAdd) {
        // No need to adjust quantities without autoadd
        continue;
      }

      const state = await promotion.test(ctx, order);
      if (!state || typeof state !== 'object') {
        continue;
      }

      const appliedDiscounts = state.buy_x_of_y.discounts as number;
      const purchasedVaiants = state.buy_x_of_y.variants as { [key: string]: number };
      const totalQuantity = appliedDiscounts * amount;

      for (const variantId of variantIds) {
        const line = order.lines.find(line => line.productVariant.id === variantId);

        // For BOGO discounts (buy 1 milk, get 1 milk free) we do not auto add
        // It causes a weird interaction where adding 1 beer adds 2.
        // We solve this by making another SKU for the same product and make it hidden.
        if ((totalQuantity === 0 && !line) || purchasedVaiants[variantId]) {
          continue;
        }
        updates[variantId] = (updates[variantId] || 0) + totalQuantity;
      }
    }

    const variantIds = Object.keys(updates);

    const hidden =
      variantIds.length === 0
        ? []
        : await this.conn
            .getRepository(ctx, Product)
            .createQueryBuilder('p')
            .select('p.customFieldsHidden', 'h')
            .addSelect('pv.id', 'id')
            .innerJoin(ProductVariant, 'pv', 'pv.productId = p.id AND pv.id IN (:...variantIds)', { variantIds })
            .getRawMany();

    for (const variantId of variantIds) {
      const line = order.lines.find(line => line.productVariant.id === variantId);
      const newQuantity = updates[variantId];
      const delta = newQuantity - (line?.quantity || 0);
      const autoRemove = !!hidden.find(l => l.id === variantId);

      let result: any;

      if (line && (delta > 0 || (autoRemove && delta < 0))) {
        result = await adjustOrderLineFn.call(this.orderService, ctx, order.id, line.id, newQuantity);
      } else if (!line && delta > 0) {
        result = await addItemToOrderFn.call(this.orderService, ctx, order.id, variantId, newQuantity);
      }

      if (result instanceof Order) {
        order = result; // Small optimization to prvent another fetch
      }
    }

    return await this.orderService.applyPriceAdjustments(ctx, order);
  }
}

And here's the implementation of some promotion actions and conditions:

import compact from 'lodash/compact';
import { PromotionCondition, LanguageCode, FacetValueChecker, TransactionalConnection, PromotionItemAction } from '@vendure/core';

let facetValueChecker: FacetValueChecker;

export const buyXofY = new PromotionCondition({
  code: 'buy_x_of_y',

  description: [
    {
      languageCode: LanguageCode.en,
      value: 'Buy items/value of variants/facets',
    },
  ],

  args: {
    amount: {
      type: 'int',
      defaultValue: 0,
      required: false,
      label: [{ languageCode: LanguageCode.en, value: 'At least this many items (0 to ignore)' }],
    },
    value: {
      type: 'int',
      defaultValue: 0,
      required: false,
      ui: { component: 'currency-form-input' },
      label: [{ languageCode: LanguageCode.en, value: 'At least this much in value (0 to ignore)' }],
    },
    variantIds: {
      type: 'ID',
      list: true,
      required: false,
      ui: { component: 'product-selector-form-input' },
      label: [{ languageCode: LanguageCode.en, value: 'Specific variants' }],
    },
    facetIds: {
      type: 'ID',
      list: true,
      required: false,
      ui: { component: 'facet-value-form-input' },
      label: [{ languageCode: LanguageCode.en, value: 'Facet values' }],
    },
  },

  init(injector) {
    facetValueChecker = new FacetValueChecker(injector.get(TransactionalConnection));
  },

  async check(ctx, order, args) {
    if (!order || !order.lines) {
      return false;
    }

    // All order lines that contain
    const lines = compact(
      await Promise.all(
        order.lines.map(async line => {
          const hasVariant = args.variantIds.length > 0 && args.variantIds.includes(line.productVariant.id);
          const hasFacets = args.facetIds.length > 0 && (await facetValueChecker.hasFacetValues(line, args.facetIds));
          return (hasVariant || hasFacets) && line;
        }),
      ),
    );

    const { quantity, value } = lines.reduce(
      (acc, line) => {
        acc.value += ctx.channel.pricesIncludeTax ? line.linePriceWithTax : line.linePrice;
        acc.quantity += line.quantity;
        return acc;
      },
      { quantity: 0, value: 0 },
    );

    const discountsToApplyBasedOnQuantity = args.amount && Math.floor(quantity / args.amount);
    const discountsToApplyBasedOnValue = args.value && Math.floor(value / args.value);

    return {
      triggerQuantity: args.amount || 0,
      variants: Object.fromEntries(lines.map(line => [line.productVariant.id, line.quantity])),
      discounts: Math.max(discountsToApplyBasedOnQuantity, discountsToApplyBasedOnValue),
    };
  },
});

export const getXFree = new PromotionItemAction({
  code: 'get_x_free',
  description: [
    {
      languageCode: LanguageCode.en,
      value: 'Get a number of selected variants for free',
    },
  ],
  args: {
    amount: {
      type: 'int',
      defaultValue: 1,
      label: [{ languageCode: LanguageCode.en, value: 'Free amount of EACH selected variant' }],
    },
    autoAdd: {
      type: 'boolean',
      defaultValue: false,
      label: [{ languageCode: LanguageCode.en, value: 'Auto-add to cart' }],
    },
    autoRemove: {
      type: 'boolean',
      defaultValue: false,
      label: [{ languageCode: LanguageCode.en, value: 'No regular purchase (only select for hidden items)' }],
    },
    variantIds: {
      type: 'ID',
      list: true,
      ui: { component: 'product-selector-form-input' },
      label: [{ languageCode: LanguageCode.en, value: 'Product Variants' }],
    },
  },
  conditions: [buyXofY],
  execute(_ctx, item, line, args, state) {
    if (!state || !line) {
      return 0;
    }
    const { discounts, variants, triggerQuantity } = state.buy_x_of_y;
    let totalDiscountedItems = discounts * args.amount;

    if (totalDiscountedItems > 0 && args.variantIds.includes(line.productVariant.id)) {
      const itemIndex = line.items.findIndex(i => i.id === item.id);

      // Number of already purchased items OF THE SAME variant that is discounted
      // This is important for BOGO discount (buy 1 milk, get 1 milk free)
      // We don't want to Apply a "GET 1 FREE" discount to the only item in the cart
      const purchasedVariants = variants[line.productVariant.id] || 0;
      if (purchasedVariants && triggerQuantity) {
        // The number of items including the free ones. Eg for "Buy 2 Get 1 free" it's 3.
        // This doesn't work with "Buy 3 Get 2 free" entirely, because you need at least 5 items
        // but you really should get a discount on "Buy 3 get 1 free".
        const discountPackSize = triggerQuantity + args.amount;

        totalDiscountedItems = 0;
        let quant = line.quantity;
        while (true) {
          if (quant >= discountPackSize) {
            quant -= discountPackSize;
            totalDiscountedItems += args.amount;
            continue;
          }
          if (quant > triggerQuantity) {
            const diff = quant - triggerQuantity;
            quant -= diff;
            totalDiscountedItems += diff;
          }
          break;
        }
      }

      // This method gets called once per OrderItem
      // This is how we decide if we've discounted enough items
      if (itemIndex < totalDiscountedItems) {
        return -item.listPrice;
      }
    }

    return 0;
  },
});

@skid
Copy link
Contributor

skid commented Sep 24, 2022

Also, if you're touching the promotions, you might wanna look into allowing promotion actions do other stuff than applying a discount. For example, I'm working on a loyalty point scheme right now, and we want certain products instead of being discounted, to add loyalty points upon successful order payment.

To do this via promotions, I need:

  1. To be sure that I can write stuff to a custom order field from a promotion action when the conditions pass. This is probably possible, even if maybe not recommended.
  2. I need to be sure that I can "undo" what I wrote to the custom order field once the conditions stop applying. I don't think that there's a way to do this now.
  3. This would also be a general solution for "auto-add" items.

For example, you might extend the promotion action interface with 2 methods:

onValid: async (ctx, order, item) => void,
onInvalid: async (ctx, order, item) => void,

If these are defined, then you can execute them instead of exec.

@michaelbromley
Copy link
Member Author

@skid thanks for all this very valuable input!

I've thought a bit about this "onValid, onInvalid" API you suggest.

The basic concept I think this captures is that of side effects. So right now, the execute functions are pure and simply return a number representing the amount by which to discount. That's a nice simple API IMO and I want to keep that.

But we could allow an optional side-effect API which is along the lines of your suggestion. I'll explore some designs along these lines.

@tianyingchun
Copy link
Contributor

How to determine how gifts should be stored in the Product list? as standard product item? or special "gift" tagged products?
how to determine when a special rule matched, which gift should be auto added?

@tianyingchun
Copy link
Contributor

another question , if promotionactions been applied event if return 0 the order.discounts always will list this discount item, It's a little bit confusing i think

@skid
Copy link
Contributor

skid commented Sep 27, 2022

The basic concept I think this captures is that of side effects. So right now, the execute functions are pure and simply return a number representing the amount by which to discount. That's a nice simple API IMO and I want to keep that.

Technically, since the execute method is async, you can also use it to do side-effects. For example, right now I'm thinking of using it to set a custom order field called awardedLoyaltyPoints and return 0. Upon transitioning to PaymentSettled I plan to use that field to award points to the customer. So, I'm using it to do a side-effect because the alternative would be to build my own system of configurable operations and promo condition evaluation, which is an even worse idea :)

Point is - if you want to allow rich extensibility to Vendure - you can't do that just with pure functions. Pure functions require that the entire model is known at design time.

@skid
Copy link
Contributor

skid commented Sep 27, 2022

How to determine how gifts should be stored in the Product list? as standard product item? or special "gift" tagged products? how to determine when a special rule matched, which gift should be auto added?

You can set a facet to the product and filter by that facet in the promotion condition.

@michaelbromley
Copy link
Member Author

Technically, since the execute method is async, you can also use it to do side-effects.

Yes, sure you can already do side-effects, but I really mean the intention is that it is pure, and then we can have an explicit API for side-effects only (i.e. no return value).

How to determine how gifts should be stored in the Product list? as standard product item? or special "gift" tagged products? how to determine when a special rule matched, which gift should be auto added?

I think this will need to be solved in the inplementation of the PromotionAction itself, but another way would be to just look up the variant ID, since we should already know the ID of the variant being added as a free gift from the args object.

@michaelbromley
Copy link
Member Author

OK I have a promising proof-of-concept design running locally. Here's what a free gift promotion looks like:

let orderService: OrderService;
export const freeGiftAction = new PromotionItemAction({
    code: 'free_gift',
    description: [{ languageCode: LanguageCode.en, value: 'Add free gifts to the order' }],
    args: {
        productVariantIds: {
            type: 'ID',
            list: true,
            ui: { component: 'product-selector-form-input' },
            label: [{ languageCode: LanguageCode.en, value: 'Gift product variants' }],
        },
    },
    init(injector) {
        orderService = injector.get(OrderService);
    },
    execute(ctx, orderItem, orderLine, args) {
        if (lineContainsIds(args.productVariantIds, orderLine)) {
            const unitPrice = ctx.channel.pricesIncludeTax ? orderLine.unitPriceWithTax : orderLine.unitPrice;
            return -unitPrice;
        }
        return 0;
    },
    async onActivate(ctx, order, args) {
        for (const id of args.productVariantIds) {
            if (!order.lines.find(line => idsAreEqual(line.productVariant.id, id))) {
                // The order does not yet contain this free gift, so add it
                await orderService.addItemToOrder(ctx, order.id, id, 1);
            }
        }
    },
    async onDeactivate(ctx, order, args) {
        for (const id of args.productVariantIds) {
            const lineWithFreeGift = order.lines.find(line => idsAreEqual(line.productVariant.id, id));
            if (lineWithFreeGift) {
                // Remove the free gift
                await orderService.adjustOrderLine(
                    ctx,
                    order.id,
                    lineWithFreeGift.id,
                    lineWithFreeGift.quantity - 1,
                );
            }
        }
    },
});

Going to do some more testing and make sure this is not interfering with any existing processes.

Note that in order for this API to work, I needed to make a functional change:

  • Previously, the Order.promotions relation would only get populated upon order completion (checkout).
  • Now, in order to correctly track changes to active promotions, I need to populate this relation as soon as an order becomes active.

I don't think this should be a breaking change, but who knows whether someone for some reason relied on the former behaviour 🤷 . I cannot remember the reasoning for only adding the relation upon order completion, but in any case it seems better to add the relation as soon as a Promotion activates.

@tianyingchun
Copy link
Contributor

when onDeactivate will be triggered?

@tianyingchun
Copy link
Contributor

and then could add associate Promotion as parameter for PromotionAction?

@michaelbromley
Copy link
Member Author

when onDeactivate will be triggered?
It gets triggered whenever a promotion which was active, becomes no longer active.

Example:

  • add couponCode, onActivate is invoked
  • remove couponCode, onDeactivate is invoked.

and then could add associate Promotion as parameter for PromotionAction?

You mean this issue?

yes I will probably be able to add that.

@tianyingchun
Copy link
Contributor

yes, :). adjustOrderline will invoke onActivate & onDeactivate at the same time?

@skid
Copy link
Contributor

skid commented Sep 29, 2022

Previously, the Order.promotions relation would only get populated upon order completion (checkout).

What do you mean by "upon order completion" ? Actually - I'm not quite sure what populating the relation means.
I think the important thing is to make the onActivate and onDeactivate checks run whenever the order changes, namely the following methods:

addItemToOrder
adjustOrderLine
removeItemFromOrder
applyCouponCode
removeCouponCode
addSurchargeToOrder
removeSurchargeFromOrder

Any one of these can validate or invalidate a promotion.

Another caveat with free items is that the user might manually remove an automatically added free item, but if the promotion is still valid, it would just add it back leading to a confusing UX. The way we solve this is by adding a custom flag to free items which the frontend uses to disable the add/remove to cart buttons on that specific product.

There is no straightforward solution to this I think without tracking the user's actions, but even then - the behaviour is not easy to define from a product aspect.

@michaelbromley
Copy link
Member Author

What do you mean by "upon order completion" ?

I'm referring to this: https://github.com/vendure-ecommerce/vendure/blob/master/packages/core/src/service/helpers/order-state-machine/order-state-machine.ts#L187

Namely, until the order is placed (transitions to PaymentAuthorized/PaymentSettled by default), the order.promotions array will always be empty, because no Promotions have been related to the order yet. Under the existing system, this relation is only established (i.e. order.promotions = activePromotions, save order) upon placing the order.

In my POC implementation, I am doing the side effect checks in the OrderService.applyPriceAdjustments() method, which I believe is called by all of the above.

Another caveat with free items is that the user might manually remove an automatically added free item, but if the promotion is still valid, it would just add it back leading to a confusing UX. The way we solve this is by adding a custom flag to free items which the frontend uses to disable the add/remove to cart buttons on that specific product.

This is a very good point, and can be accommodated with this POC design - you'd need to add the customField to the OrderLine and set it in the onActivate addItemToOrder call.

@tianyingchun
Copy link
Contributor

tianyingchun commented Sep 29, 2022

it seems that onActivate, onDeActivate it seems it hard to tell what phase of this two states , why not conbine these to one event like onChange? becase in facet for onActive, onDeactive we always need to throught all order information to determine the workflow

@tianyingchun
Copy link
Contributor

tianyingchun commented Sep 29, 2022

for above sample, gift product need to be remove/added should be determined what my shopping cart have rather than we are in onActivate or onDeActivate phase, BTW should we make sure that onActive, onDeatvie always exec completed before execute() method invoke?

@michaelbromley
Copy link
Member Author

it seems that onActivate, onDeActivate it seems it hard to tell what phase of this two states , why not conbine these to one event like onChange?

I'm not sure about what difference this makes? Is there some capability which would be possible with a single handler rather than 2? I tend to like 2 explicitly-named handlers better.

BTW should we make sure that onActive, onDeatvie always exec completed before execute() method invoke?

In the current POC design, onActivated and onDeactivated are always called after all order price calculations (including all execute functions) have completed.

@tianyingchun
Copy link
Contributor

tianyingchun commented Sep 29, 2022

yes agree, However, I still don't understand when onActive should occur, when onDeactive is triggered, and how these two methods are associated with the execution order of PromotionAction, And what kind of side effect logic should I do in these two methods

@michaelbromley
Copy link
Member Author

The flow is like this:

1 .order change (add item, change quantity, apply coupon etc)
2. Snapshot current active promotions on the order.
3. OrderService.applyPriceAdjustments(), invoke all execute() functions on PromotionActions where the conditions are satisfied.
4. Compare active promotions with the snapshot
5. For any promotions that are newly active, execute onActivated
6. For any promotions that were in the snapshot but no longer active, execute onDeactivated.

I've just published this POC on the major branch: 1a4a117

And released it in pre-release v2.0.0-next.18, so I can do some real-world testing with it.

@michaelbromley
Copy link
Member Author

First feedback from testing:

  • We need a way to handle errors that might arise in the side-effects. For example, I set up a free gift side-effect similar to the example above, but it wasn't working. I found out it was because there was insufficient stock of the selected ProductVariant, so the orderService.addItemToOrder() call was returning an ErrorResult.

I think we can handle this by wrapping the calls to onActivate and onDeactivate in a try-catch, and in the case of an error, we convert that into a new type of ErrorResult which we'll have to add to all the union types that can trigger a promotion side effect.

@tianyingchun
Copy link
Contributor

the flow very clearly: 1a4a117

:)

@michaelbromley michaelbromley moved this to 📅 Planned in Vendure OS Roadmap Oct 10, 2022
@michaelbromley michaelbromley moved this from 📅 Planned to 🏗 In progress in Vendure OS Roadmap Oct 10, 2022
@michaelbromley michaelbromley added this to the v1.8 milestone Oct 10, 2022
@michaelbromley michaelbromley moved this from 🏗 In progress to 🔖 Ready in Vendure OS Roadmap Oct 24, 2022
@michaelbromley michaelbromley moved this from 🔖 Ready to ✅ Done in Vendure OS Roadmap Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design 📐 This issue deals with high-level design of a feature type: feature ✨ @vendure/core
Projects
Archived in project
Development

No branches or pull requests

3 participants