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

RFC: Remove or Optimize the OrderItem entity #1981

Closed
michaelbromley opened this issue Jan 10, 2023 · 3 comments
Closed

RFC: Remove or Optimize the OrderItem entity #1981

michaelbromley opened this issue Jan 10, 2023 · 3 comments
Labels
design 📐 This issue deals with high-level design of a feature @vendure/core

Comments

@michaelbromley
Copy link
Member

Here is a simplified diagram of the current data model for Orders:

image

In each OrderLine, a new OrderItem entity is created in an amount corresponding to the quantity of that line. There are a few reasons we do it like this:

  1. When pro-rating order-level discounts, it sometimes happens that when splitting a discount across items in an OrderLine, the discount amount does not evenly divide by the line quantity. In that case, 1 OrderItem in the OrderLine ends up with 1 penny difference in discount. Having separate OrderItems allows us to easily keep track of this.
  2. StockMovements like refunds, cancellations are directly related to individual OrderItems. The idea being that a single OrderItem represents a physical stock unit being sold.
  3. Promotions are applied on the OrderItem level, so we can do BOGOF-like promotions by reducing the price of an individual OrderItem to 0 while leaving the other one as is.

Problems

There are a few issues with the current model, all of which come down to computational & storage efficiency:

  1. Some shops have orders with very large quantities. E.g. a B2B parts wholesaler might have an order for 5,000 of a particular part. This will create 5,000 OrderItems.
  2. This means that every modification to the Order then may need to loop over all 5,000 OrderItems to perform checks and calculations. A lot of optimization work has been done to try to avoid this work, but even that fact alone is a red flag.
  3. Fetching the order now involves joining all those 5,000 OrderItems just to view basic info about the Order such as the price of each OrderLine. This is not just slow, it also uses a lot of memory.
  4. There is a lot of redundant data duplicated at the OrderLine level, such as TaxLines data. This adds a significant amount of disk space and memory resource requirements.
  5. Seemingly simple tasks like "show me all the Fulfillments associated with this Order" require querying all 5,000 OrderItems.

Proposal

I would like to explore how we can altogether remove the OrderItem entity from the Vendure data model.

The high-level tasks are:

  1. Get feedback on this issue from the community.
  2. Propose a design that relocates all functionality of the OrderItem directly to the OrderLine or Order entities.
  3. Identify breaking changes that this design would cause.
  4. Implement changes and produce a migration solution - automated script to migrate existing Orders, and migration guide for plugin developers.

I suspect that this change can have a very significant impact on the speed and efficiency of Vendure as a whole. I'm happy to hear your feedback!

@michaelbromley michaelbromley added @vendure/core design 📐 This issue deals with high-level design of a feature v2 labels Jan 10, 2023
@michaelbromley michaelbromley moved this to 🤔 Under consideration in Vendure OS Roadmap Jan 10, 2023
@martijnvdbrug
Copy link
Collaborator

Sounds like a good plan to get rid of the OrderItem entity. Not sure how other ecommerce frameworks handle this...

When pro-rating order-level discounts, it sometimes happens that when splitting a discount across items in an OrderLine, the discount amount does not evenly divide by the line quantity. In that case, 1 OrderItem in the OrderLine ends up with 1 penny difference in discount. Having separate OrderItems allows us to easily keep track of this.

How will we solve this? We can still have rounding differences with large quantities I think, even after this has been implemented?

StockMovements like refunds, cancellations are directly related to individual OrderItems. The idea being that a single OrderItem represents a physical stock unit being sold.

This is only an internal relation used for creating the StockMovements right? It shouldn't be to complicate to construct stock movements based on orderlines (famous last words)? In my plugins I really only use the variant that's related to the stockMovement.

@is0utfitters
Copy link

Good plan on getting rid of the OrderItem entity.
Should be able to create data models that quantify what is being done with each group of similar OrderItems be it a shipment, return, discount tier, whatever.

Migrations could include counting the OrderItems that match the specific data model and adding the count for that model.

In either case it seems that it would considerably improve efficiency going forward.

@casabian
Copy link
Contributor

Sound's good to me.
I took a look at the commercetools documentation they have a similar construct as OrderItems (they call it discountedPricePerQuantity) just for discounted OrderLines.
So maybe an approach would be to have calculated OrderItems only for discounted lines.

@michaelbromley michaelbromley moved this from 🤔 Under consideration to 🏗 In progress in Vendure OS Roadmap Jan 23, 2023
michaelbromley added a commit that referenced this issue Jan 27, 2023
Relates to #1981. All core e2e tests are passing as of this commit. Still need to fix Admin UI and
any broken plugin tests
michaelbromley added a commit that referenced this issue Feb 2, 2023
Relates to #1981. All core e2e tests are passing as of this commit. Still need to fix Admin UI and
any broken plugin tests
@michaelbromley michaelbromley moved this from 🏗 In progress to ✅ Done in Vendure OS Roadmap Jun 7, 2023
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 @vendure/core
Projects
Status: 🚀 Shipped
Development

No branches or pull requests

4 participants