Fixed issue with voucher discounts not being recorded.

Also added tests for vouchers

oscar/apps/offer/models.py

@@ -251,8 +251,8 @@ class Benefit(models.Model):
             desc = _("%(value).2f discount on %(range)s") % {'value': float(self.value),
                                                              'range': unicode(self.range).lower()}
 
-        max_item_str = ungettext(" (max %d item)", " (max %d items)", self.max_affected_items)
-        desc += max_item_str % self.max_affected_items
+        if self.max_affected_items:
+            desc += ungettext(" (max 1 item)", " (max %d items)", self.max_affected_items) % self.max_affected_items
 
         return desc
 
@@ -752,7 +752,3 @@ class MultibuyDiscountBenefit(Benefit):
         else:
             free_line.discount(discount, 0)
         return self.round(discount)
-
-
-# We need to import receivers at the bottom of this script
-from oscar.apps.offer.receivers import receive_basket_voucher_change

oscar/apps/offer/receivers.py

@@ -16,15 +16,3 @@ def receive_basket_voucher_change(sender, **kwargs):
         voucher = Voucher._default_manager.get(pk=voucher_id)
         voucher.num_basket_additions += 1
         voucher.save()
-
-
-@receiver(post_save, sender=OrderDiscount)        
-def receive_order_discount_save(sender, instance, **kwargs):
-    # Record the amount of discount against the appropriate offers
-    # and vouchers
-    discount = instance
-    if discount.voucher:
-        discount.voucher.total_discount += discount.amount
-        discount.voucher.save()
-    if discount.offer:
-        discount.offer.record_usage(discount.amount)

oscar/apps/offer/utils.py

@@ -48,6 +48,7 @@ class Applicator(object):
         offers = self.get_offers(request, basket) 
         logger.debug("Found %d offers to apply to basket %d", len(offers), basket.id)
         discounts = self.get_basket_discounts(basket, offers)
+
         # Store this list of discounts with the basket so it can be 
         # rendered in templates
         basket.set_discounts(list(discounts.values()))

oscar/apps/order/utils.py

@@ -69,8 +69,11 @@ class OrderCreator(object):
         for line in basket.all_lines():
             self.create_line_models(order, line)
             self.update_stock_records(line)
+
         for discount in basket.get_discounts():
             self.create_discount_model(order, discount)
+            self.record_discount(discount)
+
         for voucher in basket.vouchers.all():
             self.record_voucher_usage(order, voucher, user)
         
@@ -78,6 +81,8 @@ class OrderCreator(object):
         order_placed.send(sender=self, order=order, user=user)
         
         return order
+
+
         
     def create_order_model(self, user, basket, shipping_address, shipping_method, 
                            billing_address, total_incl_tax, total_excl_tax, 
@@ -200,17 +205,21 @@ class OrderCreator(object):
         Creates an order discount model for each discount attached to the basket.
         """
         order_discount = OrderDiscount(order=order,
-                                       offer_id=discount['offer'].id, 
+                                       offer_id=discount['offer'].id,
                                        amount=discount['discount'])
-        if discount['voucher']:
-            order_discount.voucher_id = discount['voucher'].id
-            order_discount.voucher_code = discount['voucher'].code
+        voucher = discount.get('voucher', None)
+        if voucher:
+            order_discount.voucher_id = voucher.id
+            order_discount.voucher_code = voucher.code
         order_discount.save()
+
+    def record_discount(self, discount):
+        discount['offer'].record_usage(discount['discount'])
+        if 'voucher' in discount:
+            discount['voucher'].record_discount(discount['discount'])
         
     def record_voucher_usage(self, order, voucher, user):
         """
         Updates the models that care about this voucher.
         """
         voucher.record_usage(order, user)
-        voucher.num_orders += 1
-        voucher.save()

oscar/apps/voucher/abstract_models.py

@@ -8,7 +8,7 @@ from django.utils.translation import ugettext as _
 
 class AbstractVoucher(models.Model):
     """
-    A voucher.  This is simply a link to a collection of offers
+    A voucher.  This is simply a link to a collection of offers.
 
     Note that there are three possible "usage" models:
     (a) Single use
@@ -19,7 +19,7 @@ class AbstractVoucher(models.Model):
         help_text=_("""This will be shown in the checkout and basket once the voucher is entered"""))
     code = models.CharField(_("Code"), max_length=128, db_index=True, unique=True,
         help_text=_("""Case insensitive / No spaces allowed"""))
-    offers = models.ManyToManyField('offer.ConditionalOffer', related_name='vouchers', 
+    offers = models.ManyToManyField('offer.ConditionalOffer', related_name='vouchers',
                                     limit_choices_to={'offer_type': "Voucher"})
 
     SINGLE_USE, MULTI_USE, ONCE_PER_CUSTOMER = ('Single use', 'Multi-use', 'Once per customer')
@@ -33,7 +33,7 @@ class AbstractVoucher(models.Model):
     start_date = models.DateField(_('Start Date'))
     end_date = models.DateField(_('End Date'))
 
-    # Summary information
+    # Audit information
     num_basket_additions = models.PositiveIntegerField(_('Times added to basket'), default=0)
     num_orders = models.PositiveIntegerField(_('Times on orders'), default=0)
     total_discount = models.DecimalField(_('Total discount'), decimal_places=2, max_digits=12, default=Decimal('0.00'))
@@ -59,7 +59,7 @@ class AbstractVoucher(models.Model):
 
     def is_active(self, test_date=None):
         """
-        Tests whether this voucher is currently active.
+        Test whether this voucher is currently active.
         """
         if not test_date:
             test_date = datetime.date.today()
@@ -67,7 +67,7 @@ class AbstractVoucher(models.Model):
 
     def is_available_to_user(self, user=None):
         """
-        Tests whether this voucher is available to the passed user.
+        Test whether this voucher is available to the passed user.
         
         Returns a tuple of a boolean for whether it is successulf, and a message
         """
@@ -96,6 +96,15 @@ class AbstractVoucher(models.Model):
             self.applications.create(voucher=self, order=order, user=user)
         else:
             self.applications.create(voucher=self, order=order)
+        self.num_orders += 1
+        self.save()
+
+    def record_discount(self, discount):
+        """
+        Record a discount that this offer has given
+        """
+        self.total_discount += discount
+        self.save()
 
     @property
     def benefit(self):
@@ -120,4 +129,6 @@ class AbstractVoucherApplication(models.Model):
         verbose_name_plural = _("Voucher Applications")
 
     def __unicode__(self):
-        return _("'%(voucher)s' used by '%(user)s'") % {'voucher': self.voucher, 'user': self.user}
+        return _("'%(voucher)s' used by '%(user)s'") % {
+            'voucher': self.voucher,
+            'user': self.user}

oscar/apps/voucher/models.py

@@ -7,5 +7,3 @@ class Voucher(AbstractVoucher):
 
 class VoucherApplication(AbstractVoucherApplication):
     pass
-
-

tests/functional/checkout_tests.py

@@ -10,6 +10,7 @@ from oscar.test import ClientTestCase, patch_settings
 from oscar.apps.basket.models import Basket
 from oscar.apps.order.models import Order
 from oscar.apps.address.models import Country
+from oscar.apps.voucher.models import Voucher
 
 
 class CheckoutMixin(object):
@@ -19,8 +20,9 @@ class CheckoutMixin(object):
         self.client.post(reverse('basket:add'), {'product_id': product.id,
                                                  'quantity': 1})
 
-    def add_voucher_to_basket(self):
-        voucher = create_voucher()
+    def add_voucher_to_basket(self, voucher=None):
+        if voucher is None:
+            voucher = create_voucher()
         self.client.post(reverse('basket:vouchers-add'),
                          {'code': voucher.code})
 
@@ -41,7 +43,7 @@ class CheckoutMixin(object):
                                       'postcode': 'N1 9RT',
                                       'country': 'GB',
                                      })
-        self.assertIsRedirect(response)
+        self.assertRedirectUrlName(response, 'checkout:shipping-method')
 
     def complete_shipping_method(self):
         self.client.get(reverse('checkout:shipping-method'))
@@ -205,7 +207,7 @@ class TestPreviewView(ClientTestCase, CheckoutMixin):
         self.assertIsOk(response)
 
 
-class PaymentDetailsViewTests(ClientTestCase, CheckoutMixin):
+class TestPaymentDetailsView(ClientTestCase, CheckoutMixin):
 
     def test_user_must_have_a_nonempty_basket(self):
         response = self.client.get(reverse('checkout:payment-details'))
@@ -230,12 +232,12 @@ class PaymentDetailsViewTests(ClientTestCase, CheckoutMixin):
         self.assertRedirectUrlName(response, 'basket:summary')
 
 
-class OrderPlacementTests(ClientTestCase, CheckoutMixin):
+class TestOrderPlacement(ClientTestCase, CheckoutMixin):
 
     def setUp(self):
         Order.objects.all().delete()
 
-        super(OrderPlacementTests, self).setUp()
+        super(TestOrderPlacement, self).setUp()
         self.basket = Basket.objects.create(owner=self.user)
         self.basket.add_product(create_product(price=D('12.00')))
 
@@ -253,19 +255,25 @@ class OrderPlacementTests(ClientTestCase, CheckoutMixin):
         self.assertEqual(1, len(orders))
         
 
-class TestAnonUserOrderPlacementScenarios(ClientTestCase, CheckoutMixin):
+class TestPlacingOrderUsingAVoucher(ClientTestCase, CheckoutMixin):
 
-    def test_basic_submission_gets_redirect_to_thankyou(self):
+    def setUp(self):
+        self.login()
         self.add_product_to_basket()
+        voucher = create_voucher()
+        self.add_voucher_to_basket(voucher)
         self.complete_shipping_address()
         self.complete_shipping_method()
-        response = self.submit()
-        self.assertRedirectUrlName(response, 'checkout:thank-you')
+        self.response = self.submit()
 
-    def test_submission_using_voucher(self):
-        self.add_product_to_basket()
-        self.add_voucher_to_basket()
-        self.complete_shipping_address()
-        self.complete_shipping_method()
-        response = self.submit()
-        self.assertRedirectUrlName(response, 'checkout:thank-you')
+        # Reload voucher
+        self.voucher = Voucher.objects.get(id=voucher.id)
+
+    def test_is_successful(self):
+        self.assertRedirectUrlName(self.response, 'checkout:thank-you')
+
+    def test_records_use(self):
+        self.assertEquals(1, self.voucher.num_orders)
+
+    def test_records_discount(self):
+        self.assertEquals(1, self.voucher.num_orders)

tests/unit/voucher_tests.py

@@ -1,11 +1,36 @@
 import datetime
+from decimal import Decimal as D
 
 from django.test import TestCase
+from django.core import exceptions
+from django.contrib.auth.models import User
+from django_dynamic_fixture import G
 
 from oscar.apps.voucher.models import Voucher
+from oscar.apps.order.models import Order
+
+START_DATE = datetime.date(2011, 01, 01)
+END_DATE = datetime.date(2012, 01, 01)
 
 
 class TestVoucher(TestCase):
+
+    def test_saves_code_as_uppercase(self):
+        start = datetime.date(2011, 01, 01)
+        end = datetime.date(2012, 01, 01)
+        voucher = Voucher.objects.create(code='lower',
+                                         start_date=start,
+                                         end_date=end)
+        self.assertEqual('LOWER', voucher.code)
+
+    def test_checks_dates_are_sensible(self):
+        start = datetime.date(2011, 01, 01)
+        end = datetime.date(2012, 01, 01)
+        with self.assertRaises(exceptions.ValidationError):
+            voucher = Voucher.objects.create(code='lower',
+                                            start_date=end,
+                                            end_date=start)
+            voucher.clean()
     
     def test_is_active_between_start_and_end_dates(self):
         start = datetime.date(2011, 01, 01)
@@ -20,10 +45,41 @@ class TestVoucher(TestCase):
         end = datetime.date(2011, 02, 01)
         voucher = Voucher(start_date=start, end_date=end)
         self.assertFalse(voucher.is_active(test))
-        
-    def test_codes_are_saved_as_uppercase(self):
-        start = datetime.date(2011, 01, 01)
-        end = datetime.date(2011, 02, 01)
-        voucher = Voucher(name="Dummy voucher", code="lowercase", start_date=start, end_date=end)
-        voucher.save()
-        self.assertEquals("LOWERCASE", voucher.code)
+
+    def test_increments_total_discount_when_recording_usage(self):
+        voucher = G(Voucher)
+        voucher.record_discount(D('10.00'))
+        self.assertEqual(voucher.total_discount, D('10.00'))
+        voucher.record_discount(D('10.00'))
+        self.assertEqual(voucher.total_discount, D('20.00'))
+
+
+class TestMultiuseVoucher(TestCase):
+
+    def setUp(self):
+        self.voucher = G(Voucher, usage=Voucher.MULTI_USE)
+
+    def test_is_available_to_same_user_multiple_times(self):
+        user, order = G(User), G(Order)
+        for i in xrange(10):
+            self.voucher.record_usage(order, user)
+            self.assertTrue(self.voucher.is_available_to_user(user)[0])
+
+
+class TestOncePerCustomerVoucher(TestCase):
+
+    def setUp(self):
+        self.voucher = G(Voucher, usage=Voucher.ONCE_PER_CUSTOMER)
+
+    def test_is_available_to_a_user_once(self):
+        user, order = G(User), G(Order)
+        self.assertTrue(self.voucher.is_available_to_user(user)[0])
+        self.voucher.record_usage(order, user)
+        self.assertFalse(self.voucher.is_available_to_user(user)[0])
+
+    def test_is_available_to_different_users(self):
+        users, order = [G(User), G(User)], G(Order)
+        for user in users:
+            self.assertTrue(self.voucher.is_available_to_user(user)[0])
+            self.voucher.record_usage(order, user)
+            self.assertFalse(self.voucher.is_available_to_user(user)[0])