can_apply_benefit return wrong data when use apply offer for product child

kgilpin commented 1 month ago

Issue Summary

When there are product discountable true and variant discountable false in cart in the same time, discount is calculated for variant which is incorrect.

Steps to Reproduce

Product A (discountable=true)
Variant B (discountable=false)
Set an offer X with range that included Product A and Variant B

Issue

When Product A and Variant B are in the same cart, discount from offer X is calculated for both lines which is incorrect.

Relevant code

oscar/app/offer/abstract_models.py

def can_apply_benefit(self, line):
        """
        Determines whether the benefit can be applied to a given basket line
        """

Found in version

c49a21979

github-actions[bot] commented 1 month ago

Title: Fix can_apply_benefit to Exclude Undiscountable Variants from Discount Calculation

Problem: The can_apply_benefit method incorrectly allows discounts to be applied to product variants that are not set as discountable. When both a discountable product (e.g., Product A) and a non-discountable variant (e.g., Variant B) are in the same cart, the discount is applied to both items, which should not happen.

Analysis: The issue stems from the can_apply_benefit method not correctly differentiating between discountable products and their non-discountable variants. The method needs to ensure that it only returns true if the product or its variant is marked as discountable.

Proposed Changes:

src/oscar/apps/offer/abstract_models.py: Update the can_apply_benefit method
- Add a condition to ensure that the benefit is only applied if both the product and its variants are discountable.
tests/integration/offer/test_absolute_benefit.py: Add test cases
- Update existing test cases or add new test cases to ensure that the discount is not applied to non-discountable variants.

Detailed Steps:

src/oscar/apps/offer/abstract_models.py: Update the can_apply_benefit method

The can_apply_benefit method should be updated to include a check that ensures products are marked as discountable before applying benefits.

def can_apply_benefit(self, line):
   """
   Determines whether the benefit can be applied to a given basket line
   """
   product = line.product
   # Ensure product and its variants are discountable
   return line.stockrecord and product.is_discountable and product.get_parent().is_discountable

tests/integration/offer/test_absolute_benefit.py: Update/Add test cases

Enhance existing tests to verify that the discount is not applied to non-discountable variants.

parent_discountable_product = factories.create_product(
   structure="parent", is_discountable=True
)
child = factories.create_product(
   title="Undiscountable variant",
   structure="child",
   parent=parent_discountable_product,
   is_discountable=False,
   price=100,
)

benefit = SomeBenefitObject()  # Example Benefit Object
line = SomeBasketLineObject()  # Example Line Object
Applicator().apply(basket)

# Verify the behavior with the new logic
assert not benefit.can_apply_benefit(line.child)

These changes will ensure that discounts are correctly applied only to discountable products, preventing non-discountable variants from erroneously receiving discounts.

kgilpin commented 1 month ago

Original report: https://github.com/django-oscar/django-oscar/issues/4070

land-of-apps / django-oscar