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

Fix restocking and unstocking backordered items in FulfilmentChanger #2286

Conversation

DanielePalombo
Copy link
Contributor

This PR fixes issues described in #2285.

Current code is not taking into account some important scenarios, like:

  • Desired stock item as negative count on hand amount
  • Restock of inventory in backordered state

Some specs was incorrectly passing since they are not taking into
account setting coherent value of the stock item's count on hand.

If the shipment has 6 `backordered` inventory unit the related stock
item must have at least -6 in the count on hand.
All inventory units which are not shipped can be restocked.
Even backordered inventory units can be restocked.
If the stock item is backorderable unstock all quantity, else unstock
only the max available quantity in the stock item
@jhawthorn jhawthorn requested a review from mamhoff October 13, 2017 18:32
@mamhoff
Copy link
Contributor

mamhoff commented Oct 13, 2017

I don't think stock item's should have a negative count on hand. In my understanding, backordered stock items don't affect the count on hand.

Desired stock item's count on hand could even be negative.
If this happens availability should be considered 0.
@DanielePalombo DanielePalombo force-pushed the nebulab/fix-stock-item-fulfilment-changer branch from 57f6d99 to 10499d3 Compare October 13, 2017 22:41
@mamhoff
Copy link
Contributor

mamhoff commented Oct 17, 2017

I was wrong, just re-checked the way Spree::OrderInventory handles these cases. In fact, negative stock items are desired in order to account for the case that inventory is added and used up by backordered stock items.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked with @DanielePalombo in real life about this issue as well and everything done here seems to make sense to me. 👍

@jhawthorn jhawthorn added this to the 2.4.0 milestone Oct 18, 2017
@mamhoff mamhoff merged commit c81b9d1 into solidusio:master Oct 24, 2017
@jhawthorn jhawthorn changed the title Fix restocking and unstocking items in FulfilmentChanger Fix restocking and unstocking backordered items in FulfilmentChanger Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants