OrchardCore.Commerce
OrchardCore.Commerce copied to clipboard
Inventory management (OCC-37)
Concept
Content items which have ProductPart
may also have an InventoryPart
. If a type has InventoryPart
, then additional conditions apply before the product can be added to the cart.
Fields or Properties
Should this use OrchardCore.ContentFields? It would be nicer, I think.
- Inventory (int/NumericField) The number of items in stock.
- AllowsBackOrder (bool/BooleanField) If true, ordering is allowed even if Inventory is less or equal to zero.
- OutOfStockMessage (string/MarkdownField/HtmlField) Enables a specific message for an out of stock product (can be used to give an ETA).
-
IgnoreInventory (bool/BooleanField) If true, inventory is ignored same as not having an
InventoryPart
at all. This is useful for digital products for example. - MinimumOrderQuantity (int/NumericField) The minimum number of this item one can order; ignored if zero or negative.
- MaximumOrderQuantity (int/NumericField) The maximum number of this item one can order (per day? ever?); ignored if zero or negative.
I have changed the ConsiderInventory from the original proposal into IgnoreInventory so it can be default false.
Events
Should have an IInventoryEvents
interface with at least
- AddingToCart: before item is added to cart; add custom logic e.g. authentication check and cancel adding to cart if needed.
- OrderCheckedOut: after payment; should contain a default handler for updating the inventory, this is where we'll trigger workflows too.
Services and Providers
-
IProductInventoryService
&ProductInventoryService
(Product prefix so the implementation doesn't start with the letter I.)-
Task<int> QueryInventoryAsync(int sku)
: return the current inventory count. (also suggest aTask<bool> IsAvailableAsync(int sku)
as an extension method) -
Task UpdateInventoryAsync(int sku, int difference, bool reset = false)
: Normally you want to change the inventory by decrementing with sales amount or incrementing with shipment amount. Reset sets the value to zero first. This should be locking/atomic to prevent problems with simultaneous orders.
-
-
IProductInventoryProvider
&LocalInventoryProvider
:- Include the actual implementations of the two methods above.
-
Order
property. -
Task<bool> IsApplicableAsync(string sku)
: check if it product can use this provider. ForLocalInventoryProvider
this means the presence of theInventoryPart
.
Workflows
They are great, but it should be part of a followup issue to tighten the scope.
Project Structure
The events, interfaces and services (excluding LocalInventoryProvider
) should go to OrchardCore.Commerce.
LocalInventoryProvider
and InventoryPart
should go it a new module, e.g. OrchardCore.Comerce.Inventory.Local. Future ERP-based implementations would go to OrchardCore.Comerce.Inventory.{ProductName} projects accordingly.
The simple implementation should simply store the info on the part, but the abstraction it implements should assume remote data. In all but the most simple shops, inventory won't be managed in the Orchard app, but will be an existing system that Orchard Commerce will have to integrate with. The abstraction may also separate cart/order integration from inventory management, which should have lower-level APIs to increment, decrement, change, query, etc. inventory levels for a given SKU.
Nwazet links for reference:
- https://github.com/bleroy/Nwazet.Commerce/blob/master/Services/IProductInventoryService.cs
- https://github.com/bleroy/Nwazet.Commerce/blob/master/Services/ProductInventoryServiceBase.cs
Good point, an external inventory manager sounds more likely. The internal inventory management service would be a reference/fallback implementation of the same abstraction and creating additional implementations for specific ERP integration would be an exercise for the library consumer.
As for cart management, why is it important to make it detailed like that? I thought just query and change/reserve should be enough, as numeric management of the cart really happens on the client side and we just have to make sure no more is offered than what's available.
Not sure what you mean by cart management. One thing that may seem unfamiliar in the Nwazet implementation is bundles. Those are composite products that are made of an aggregation of products. When one of the products composing the bundle becomes unavailable, you want the bundle to also become unavailable. Since bundles are not implemented in Orchard.Commerce, this part can be skipped (but might be a thing to keep in mind as an example of interesting extensions that it should be possible to implement).
@DAud-IcI, a few questions:
- Is it only the Product content type that should have an inventory part?
- SKUs are normally strings — are they meant to be used as integers instead in the proposed methods, or is that just a typo?
-
UpdateInventoryAsync()
is supposed to be locking; is there a simple way to achieve this in OC? For reference, this is the implementation I have added. It seems to work as expected, but I'm not sure if it's locking: -
IInventoryEvents: these two methods (
OrderCheckedOut()
,AddingToCart()
), where should they be used and what parameters should they take? If you could, please elaborate on this part a bit more in general as I'm a bit confused. -
IProductInventoryProvider and LocalInventoryProvider, and their relation to IProductInventoryService's
QueryInventoryAsync()
andUpdateInventoryAsync()
methods: what do you mean by including the implementation of the two above mentioned methods? These two methods are already implemented in ProductInventoryService (on the issue branch, that is). Should both inventory providers be derived from IProductInventoryService just to make it so the methods are available here as well? . I also noticed thatIsApplicableAsync()
is a method (andOrder
is a property) of ISortableUpdaterProvider, should these new providers be based on this (much like IPriceProvider or ITaxProvider)? Just like above, a little more elaboration on the goal of these additions would be helpful.
- Should any of the inventory part's fields be displayed when viewing the product on the user-facing site?
- When should inventory be decreased — upon being added to cart, or upon completing checkout?
- I'd say no by default, but it should be possible to create a template that does show it. As usual different businesses use different rules so you need to account for all possibilities. Most businesses will tell you if a product is unavailable or not, some will tell you how many items are in stock, and some -gasp- will lie about it (which is probably illegal in quite a few places) Inventory management will not even be on for all businesses, so even if you go with displaying it, it should be gracefully disappearing when the feature is not on.
- That will also depend on the business. doing it on checkout seems like a reasonable default, but ultimately this looks like something that should be orchestrated with a workflow. There's in fact a security reason not to do it on adding to cart: a business' competitor could create a bot to create fake visitors to your site and add products to the cart until your inventory is virtually depleted, then never complete these purchases. You would never be able to sell anything. Doing it on checkout at least creates the hurdle that you need to pay for depleting the inventory.
- No, but you can assume that the content type that has
InventoryPart
also hasProductPart
. - Looks like a typo, fixed.
- I don't think you can, locking and async are kind of opposites. I agree that this process should be locked so feel free to replace
UpdateInventoryAsync
with a synchronous method. - I will write a followup comment in a bit.
- Same.
- I suggest adding blank template cshtml files for most of them, so they are hidden by default but can be still overridden with admin templates. Please add a nice template for the
Inventory
field that shows remaining stock in a similar format to the Price above. Also if the business doesn't need inventory info they should be able to simply remove the inventory part without error. - I think we can assume decreasing inventory on the checkout completion (so after payment) for the MVP. We can add a setting later to disable that e.g. if the business wants to do it via workflows when we have more support for those.
Async can be combined with locking when using SemaphoreSlim
using System.Threading;
namespace MyNamespace
{
public class AsyncWithLock
{
// SemaphoreSlim with initialCount of 1 means only
// one thread may enter (lock) it at a time
private readonly SemaphoreSlim _lock = new(initialCount: 1);
public async void LockingMethodAsync()
{
// The first thread to access WaitAsync() will return immediately;
// all subsequent threads will be blocked until the first thread
// calls Release()
await _lock.WaitAsync();
try
{
// single-thread-accessible code here
}
finally
{
_lock.Release();
}
}
}
}
IInventoryEvents: these two methods (OrderCheckedOut(), AddingToCart()), where should they be used and what parameters should they take? If you could, please elaborate on this part a bit more in general as I'm a bit confused.
I was thinking about adding some extension points, but now I feel like this was a bit over-engineered for the MVP. We can skip the events for this issue.
IProductInventoryProvider and LocalInventoryProvider, and their relation to IProductInventoryService's QueryInventoryAsync() and UpdateInventoryAsync() methods: what do you mean by including the implementation of the two above mentioned methods? These two methods are already implemented in ProductInventoryService (on the issue branch, that is). Should both inventory providers be derived from IProductInventoryService just to make it so the methods are available here as well?
The IProductInventoryProvider
implementation should contain the business logic. the IProductInventoryService
should just iterate over the IProductInventoryProvider
services. The IProductInventoryService
is basically a container for all its providers. See the PriceService.AddPricesAsync()
for reference.
I also noticed that IsApplicableAsync() is a method (and Order is a property) of ISortableUpdaterProvider, should these new providers be based on this (much like IPriceProvider or ITaxProvider)?
Yes, IProductInventoryProvider
should inherit from ISortableUpdaterProvider<T>
,