Verification of `Order` might fail if it contains `OrderLine`s exceeding the stock, but compensating `OrderLine`s exist

[Deric-W]

While implementing methods for checking if a UniqueInventory has sufficient stock for completing orders I noticed that there are a few edge cases that might be useful to solve here:

• Order can have multiple OrderLines per Product -> quantities have to be added
• Order can have OrderLines with negative Quantity -> solved like the one above
• UniqueInventoryItems can be missing -> missing Products are assumed to have a Quantity of zero

I implemented two new methods on UniqueInventory (hasSufficientQuantity(ProductIdentifier, Quantity) and hasSufficientQuantity(Order)) and can create a PR if you agree with my interpretation.

[odrotbohm]

Can you elaborate on the premise? Why did you implement the methods in the first place? The entire check whether sufficient stock is available to satisfy an Order is taken care of by the inventory (InventoryManagement in particular). This is the case to encapsulate that logic within the module. Exposing methods on the repository interfaces would incentivize clients to perform these checks themselves, which subverts the goal of the original design.

Fundamentally, the verifications of the Inventory are triggered when you complete the order, and on the surface, I don't quite see anything actually broken (which doesn't necessarily mean I am right 😬). Let me iterate over the issues you brought up:

• Multiple OrderLines per Product – this can be improved by tweaking our verification algorithm to not simply iterate over the OrderLines but grouping them by Product first to calculate the proper amounts to be reduced. But I actually think that our current algorithm should even catch this, as it would reduce the stock for each of the OrderLine instances and – if insufficient – would eventually end up, rejecting the entire order. I guess an integration test case for that case would help us find out.

• OrderLines with negative Quantity – I guess we could prevent negative quantities right away, but that would prevent projects to implement returns by e.g. allowing an admin to create a return order that would use these negative values to indicate that some customer has returned a few items. The negative values would then still update the inventory properly, AFAICT. What problem did you see with that scenario in the first place? Also, I'd like to see a failing test case against the current API to indicate we actually have a problem to solve.

• UII missing – in that case, no MII present would cause the error to be rejected, too (see InventoryManagement.verify(…)) and thus indicate the bug that the developer has failed to create InventoryItems of either kind when adding a Product. I'd have to reiterate on whether we could do something about this, but we so far haven't done anything about that as ultimately, the UniqueInventory and MultiInventory exist within an app at the same time and to automate the inventory update, we currently consider the sole presence of an II in either of the flavors to indicate, which of them to interact with.

To me, it feels like it might make sense to discuss these items individually as they each leave a few things to discuss and explore and trying to elaborate on them in parallel might get a bit confusing. I'd still like to hear about your immediate response before deciding how to proceed in that regard.

[Deric-W]

Thank you for your response.

Use Case

My use case was highlighting orders in the front end which can be completed.
I tought that exposing this logic would prevent users from reinventing the wheel.

negative Quantities

I do have no problems with negative quantities (please dont remove them) but I noticed that completing an Order with mixed negative and positive quantities can fail even if the sum is in stock depending on the order in which the OrderLines are completed, for example if the quantites 11 and -4 are applied to an inventory with 10 in stock.

UII missing

You are right.

[odrotbohm]

I guess the way to go would be to use the currently existing Inventory interfaces to call ….findByProductIdentifier(…) to avoid the user adding more elements to the Cart in the first place. That's essentially what completing an order does eventually, anyway. But there's no reason that either some presentation logic or the logic adding items to the Cart could not check that beforehand.

You're right regarding the processing of the individual quantities. A single OrderLine exceeding the currently available quantity would cause the order to rollback, even if there was another compensating OrderLine that prevents the threshold from being met. I guess it makes sense to tweak the verification algorithm to take that into account.