-
Notifications
You must be signed in to change notification settings - Fork 2.2k
refactor: updates to inventory counts and statuses #4859
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
Conversation
@aldeed all comments addressed, this is ready for another look
| // we can map over the unique productIds at the end, and update each one once | ||
| orderItems.forEach(async (item) => { | ||
| // Update supplied item inventory | ||
| const updatedItem = await collections.Products.findOneAndUpdate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless you set the returnOriginal option to false, findOneAndUpdate default behavior is to return the original, not the updated. In this case it doesn't cause an issue, but updatedItem is an inaccurate variable name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the return is technically an object where value is the document, either original or updated. So it would be better to do
const { value: originalProduct } =There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment for the other similar calls in this PR, but it doesn't seem to be anything more than a variable name issue in all places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't really matter, but do you think it might be best to have the updated document for use in the future, to expand on this, if needed: 09e2b4c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that's best, but the option is called returnOriginal and needs to be set to false: http://mongodb.github.io/node-mongodb-native/3.1/api/Collection.html#findOneAndUpdate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it...updated. Was running off of the MongoDB Shell docs: https://docs.mongodb.com/manual/reference/method/db.collection.findOneAndUpdate/
Resolves #4337
Impact: minor
Type: refactor
Issue
Some inventory statuses are confusing and cause issues when trying to easily determine a current status. Inventory also does not correctly update on variants, depending on if a variant has options or not.
Solution
This PR helps to clarify some statuses by adding new statuses, and defining older statuses better. It also fixes the issues concerning out dated inventory numbers on variants that have option children.
Add
canBackorderboolean. canBackorder represents the ability to backorder products:inventoryPolicy === false && inventoryManagement === true. Added to Variants and Options in the Catalog Schema and the Product Schema.Better define (via comments)
isBackorderas being the current state of backorder, not the ability to backorder (which iscanBackorder). PreviouslyinventoryPolicy === false && inventoryManagement === true && currentQuantity === 0, nowcanBackorder === true && inventoryAvailableToSell === 0Depreciate
currentQuantityfrom cart, and addinventoryAvailableToSellandinventoryInStockAdd
inventoryAvailableToSellinventory count to determine how many products are available for sale, i.e. not reserved.inventoryAvailableToSellis updated when a customer completes their order. This field is added on Options, Variants, and Product levels, in both the Catalog and the Products collections. If avariantorproduct, this is not an input number, but rather a number calculated based of the summed up total of its children's inventory.Add
inventoryInStockin the Catalog collection. This is an existing field in the Products collection, calledinventoryQuantity. This is updated when an operator processes an order.Add calculations that automatically update the
inventoryAvailableToSellandinventoryInStocknumbers on Products and Variants, based on the summed inventory count of it's children. These functions areupdateParentVariantsInventoryAvailableToSellQuantityandupdateParentVariantsInventoryInStockQuantity. For example,Variant Ahas childrenOption AandOption B. In the past, we would stop updating inventory onVariant A, and in the UI show a client side calculation of the sum of inventory ofOption AandOption B. This update calculates the sum ofOption AandOption Bwhenever they are updated, and saves the number as an actual inventory number onVariant Ain the database. This way we can trust the number we are provided by the data, instead of doing a client side calculation at runtime.Breaking changes
inventoryAvailableToSellto all products / variants, to correctly calculate the numbers on parent products / variants, and to publish this data to already published Catalog items.currentQuantityhas been marked withdepreciatedin the cart. This isn't a breaking change at the moment, but lays the path to remove this field and replace withinventoryAvailableToSellandinventoryInStockin the future.Catalog.getVariantQuantityandReactionProduct.getVariantQuantityhave been removed. Custom plugins using these methods will need to be updated. The same data returned by these methods is now on the object that was being passed into these methods as the fieldinventoryQuantityorinventoryAvailableToSellisBackorder,isLowQuantity, andisSoldOutfunctions from thecatalogplugin to the newinventoryplugin. Custom plugins using these methods will need to update their import path.Testing
canBackorder,inventoryAvailableToSellandinventoryInStockare present on products, variants, and options on ourcatalogItemProductquery.You can use this GQL query to only see the relevent info and get rid of the noise in GraphiQL:
Add various combinations of products with only variants, only options, etc, and see that the
inventoryAvailableToSellis correctly decremented when an order is placed.Process the order, and see that the same / correct updates are made to
inventoryInStock.** Inventory numbers will be different between the time an order is placed, and it is processed. They should be the same if there are no current orders waiting to be processed.
When an order is cancelled BEFORE it's approved, only the
inventoryAvailableToSellnumber should be adjusted.When an order is cancelled AFTER it's been approved, both inventory counts should be adjusted, IF the user chooses "restock" on the cancellation.
As a customer, add
xproducts to your cart and complete checkout. Go to PDP as admin, Adjust inventory via the operator UI, and see that the both inventory numbers adjust appropriately on all options, variants, and top level products.Hook up to the Cart endpoint via GraphiQL and see that
inventoryAvailableToSellandinventoryInStockhave been added.Migration 51 has two parts: the first adds the new inventory field to the
Productscollection product, and the second publishes those changes to published catalog products. If you are running Reaction from scratch, this second part will not do anything on the first run, since the Basic Reaction Product isn't published to the catalog by default. The best way to test the second part is to run the app from scratch, go to the PDP and publish the product, then go into Mongo and deleteinventoryAvailableToSellandinventoryInStockfields from all products / variants / options inside theCatalogcollection. Then, inside theMigrationscollection, change the migration version to50. Then, restart the core docker container.