Disallow selecting quantity of stock past what's available. #1905
Conversation
nerrad
requested review from
Aljullu and
mikejolley
and removed request for
a team
|
doh I need to fix php unit tests! |
| @@ -20,28 +20,93 @@ import ProductLowStockBadge from './product-low-stock-badge'; | |||
| /** | |||
| * Cart line item table row component. | |||
| */ | |||
| const CartLineItemRow = ( { lineItem = {} } ) => { | |||
| const CartLineItemRow = ( { | |||
| lineItem = { | |||
There was a problem hiding this comment.
Should we define these default objects somewhere for reuse?
There was a problem hiding this comment.
Let's see how often it comes up?
There was a problem hiding this comment.
I ended up removing this and making lineItem (via propTypes). That way there's no need for defaults because it's clear via the propTypes that the properties are required.
assets/js/blocks/cart-checkout/cart/full-cart/cart-line-item-row.js
Outdated
Show resolved
Hide resolved
assets/js/blocks/cart-checkout/cart/full-cart/cart-line-item-row.js
Outdated
Show resolved
Hide resolved
assets/js/blocks/cart-checkout/cart/full-cart/cart-line-item-row.js
Outdated
Show resolved
Hide resolved
it doesn’t need to do a lookup
- has correct default shape for line item - implements change to `useStoreCartItemQuantity` signature. - retrieves currency using currency object on price - accounts for remaining stock on quantity selector.
The rest response was not matching the schema for low_stock_threshold
nerrad
force-pushed
the
update/disallow-quantity-past-max-avaialble
branch
from
fef3019 to
75fdddd
Compare
|
Ok @mikejolley I updated the description of the pull to account for the changes and this is ready for another review. |
| * | ||
| * @param \WC_Product $product Product instance. | ||
| * @return integer|null | ||
| */ | ||
| protected function get_low_stock_remaining( \WC_Product $product ) { | ||
| protected function get_remaining_stock( \WC_Product $product ) { |
There was a problem hiding this comment.
This isn't needed now right?
There was a problem hiding this comment.
It abstracted out the logic from get_low_stock_remaining (which calls this). I thought it'd be fine to leave it. It makes get_low_stock_remaining) a bit easier to follow and if this is needed in the future (or if the logic for getting remaining stock changes) we already have this available.
This addresses #1887
This pull does the following:
useStoreCartItemQuantitynow receives the cartItem instead of thecartItemKeyas an argument. I didn't notice in previous reviews how it's used in the context where we already have a cartItem so implementing this reduces complexity and makes the hook more precise for it's purpose.backorders_allowedto the CartItem schema for the API. This allows for client to have correct logic for maximum quantity when this value is true.CartLineItemRowso that quantity picker is limited by the amount of stock remaining if that is available (lowStockRemaining) and thebackorders_allowedis false.99when other conditions don't apply (see related issue in Introduce per product setting for maximum quantity selected for product. #1913)To Test
backorders_allowedset to true that the quantity limit is 99.Potential followup.
Should we automatically reduce the quantity selected for an item in the cart if the stock has gone down? Since we're updating the cart on every request, it's possible for us to do this (and we could trigger a notice when this happens). This could entice folks to move quicker on high turnover items.
We could do the above as a stretch goal, but it's nice knowing it's doable.