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
Products with options can be oversold #2637
Comments
We could refactor It gets a bit tricky when you consider that it is possible to have different purchase strategies for different lines - I think we would have to make some assumptions about this and make it easy to override them. |
Thanks for the response. Yeah, that could work. The basket would need to be checked in a number of places to prevent basket errors:
For item 3 it gets a bit tricky as you said. Would the validation have to rely on the availability policy, i.e., |
Not sure if this is the proper way but I am putting my solution here in case anybody else runs into this problem. What I ended up doing was taking the suggestion of using class Basket(AbstractBasket):
def is_quantity_allowed(self, qty, line=None):
"""Override to check basket level validation.
Add some validation to make sure the same products on different lines
are not oversold.
"""
#Call the original and stop if there are any problems found
is_allowed, reason = super(Basket, self).is_quantity_allowed(qty)
if not is_allowed:
return is_allowed, reason
#Check the basket totals
if line:
is_permitted, reason = line.purchase_info.availability.is_purchase_permitted(self.basket_quantity(line) + qty)
if not is_permitted:
reason = _(
"{reason} (there may be others in your basket)"
).format(
reason=reason,
)
return False, reason
return True, None
def basket_quantity(self, line):
"""Return the quantity of similar lines in the basket.
The basket can contain multiple lines with the same product and
stockrecord, but different options. Those quantities are summed up.
"""
matching_lines = self.lines.filter(stockrecord=line.stockrecord)
quantity = matching_lines.aggregate(Sum('quantity'))['quantity__sum']
return quantity or 0 This has to be called somewhere during the class BasketLineForm(CoreBasketLineForm):
def check_max_allowed_quantity(self, qty):
"""Override to pass the line.
"""
qty_delta = qty - self.instance.quantity
is_allowed, reason = self.instance.basket.is_quantity_allowed(qty_delta, line=self.instance)
if not is_allowed:
raise forms.ValidationError(reason) I also added a similar check to def check_basket_is_valid(self, request):
"""Checks to maker sure the basket is still valid. Called at each
checkout step.
Add some validation to make sure the same products on different lines
are not oversold.
"""
super(CheckoutSessionMixin, self).check_basket_is_valid(request)
messages = []
strategy = request.strategy
for line in request.basket.all_lines():
result = strategy.fetch_for_line(line)
basket_quantity = request.basket.basket_quantity(line)
is_permitted, reason = result.availability.is_purchase_permitted(basket_quantity)
if not is_permitted:
msg = _(
"'{title}' is no longer available to buy (there are {basket_quantity} in your basket - {reason}). "
"Please adjust your basket to continue"
).format(
title=line.product.get_title(),
basket_quantity=basket_quantity,
reason=reason,
)
messages.append(msg)
if messages:
raise exceptions.FailedPreCondition(url=reverse("basket:summary"), messages=messages) This will make sure the basket is valid at checkout. Once again, not sure if this is the best way but it should work until this gets addressed in subsequent versions. |
@ad-65 would you submit a proper PR and include unit tests? Thanks. |
Is this fixed ? Metadata-Version: 2.1 |
Fixed by #4096 |
When a product is configured with class or product options (not attributes) and tracks stock it possible to oversell the product. This behaviour is visible in the sandbox and in Oscar 1.5.
To reproduce:
If I am not mistaken this is because the availability validation happens at the basket line level and does not take the overall basket state into account. The
is_purchase_permitted()
check passes for each line item but should not pass as a whole because there are only 10 t-shirts available for purchase but 13 in the basket.The options force the same product to be a separate line item (as it should be) but this means that line item validation will not work because there can be multiple lines with the same product. I am not sure the best way to fix this as it requires basket level validation and I had a quick look at the code but did not see a hook for adding this. Any suggestions?
The text was updated successfully, but these errors were encountered: