Merge pull request #1922 from edoburu/fix/weightbased-discount

Fix errors with shipping discounts on free weightbased shipping.

src/oscar/apps/shipping/abstract_models.py

@@ -39,6 +39,13 @@ class AbstractBase(models.Model):
     def __str__(self):
         return self.name
 
+    def discount(self, basket):
+        """
+        Return the discount on the standard shipping charge
+        """
+        # This method is identical to the Base.discount().
+        return D('0.00')
+
 
 class AbstractOrderAndItemCharges(AbstractBase):
     """
@@ -95,7 +102,6 @@ class AbstractWeightBased(AbstractBase):
 
     code = 'weight-based-shipping'
     name = _('Weight-based shipping')
-    offer = None
 
     # The default weight to use (in kg) when a product doesn't have a weight
     # attribute.
@@ -179,10 +185,6 @@ class AbstractWeightBased(AbstractBase):
         except IndexError:
             return None
 
-    def discount(self, basket):
-        base_charge = self.calculate(basket)
-        return self.offer.shipping_discount(base_charge.incl_tax)
-
 
 @python_2_unicode_compatible
 class AbstractWeightBand(models.Model):

src/oscar/apps/shipping/methods.py

@@ -40,6 +40,9 @@ class Base(object):
         """
         Return the discount on the standard shipping charge
         """
+        # The regular shipping methods don't add a default discount.
+        # For offers and vouchers, the discount will be provided
+        # by a wrapper that Repository.apply_shipping_offer() adds.
         return D('0.00')
 
 

tests/integration/shipping/model_method_tests.py

@@ -2,7 +2,10 @@ from decimal import Decimal as D
 
 from django.test import TestCase
 
+from oscar.apps.offer.applicator import Applicator
+from oscar.apps.offer.models import Benefit
 from oscar.apps.shipping.models import OrderAndItemCharges, WeightBased
+from oscar.apps.shipping.repository import Repository
 from oscar.core.compat import get_user_model
 from oscar.test import factories
 
@@ -198,3 +201,24 @@ class WeightBasedMethodTests(TestCase):
         charge = self.standard.calculate(basket)
 
         self.assertEqual(D('1.00') + D('2.00'), charge.excl_tax)
+
+    def test_zero_charge_discount(self):
+        # Since Repository.apply_shipping_offer() returns the original
+        # shipping method object on a free shipping charge, it's discount()
+        # method should be callable and also indicate it won't add discounts.
+        basket = factories.create_basket()
+        self.assertEqual(D('0.00'), self.standard.discount(basket))
+
+    def test_zero_charge_with_shipping_discount(self):
+        offer = factories.create_offer(
+            benefit=Benefit.objects.create(
+                type=Benefit.SHIPPING_FIXED_PRICE, value=1),
+        )
+        basket = factories.create_basket()
+        Applicator().apply_offers(basket, [offer])
+
+        # Similar to test_zero_charge_discount(),
+        # but now test how the repository deals with it.
+        method = Repository().apply_shipping_offer(
+            basket, self.standard, offer)
+        self.assertEqual(D('0.00'), method.discount(basket))