Makes apply_voucher() idempotent, adds _test_voucher to validate_cart, and updates tests.

registrasion/controllers/cart.py

@@ -178,6 +178,10 @@ class CartController(object):
         # Try and find the voucher
         voucher = rego.Voucher.objects.get(code=voucher_code.upper())
 
+        # Re-applying vouchers should be idempotent
+        if voucher in self.cart.vouchers.all():
+            return
+
         self._test_voucher(voucher)
 
         # If successful...
@@ -193,18 +197,32 @@ class CartController(object):
 
         # It's invalid for a user to enter a voucher that's exhausted
         carts_with_voucher = active_carts.filter(vouchers=voucher)
+        carts_with_voucher = carts_with_voucher.exclude(pk=self.cart.id)
         if len(carts_with_voucher) >= voucher.limit:
             raise ValidationError("Voucher %s is no longer available" % voucher.code)
 
         # It's not valid for users to re-enter a voucher they already have
-        user_carts_with_voucher = rego.Cart.objects.filter(
+        user_carts_with_voucher = carts_with_voucher.filter(
             user=self.cart.user,
-            released=False,
-            vouchers=voucher,
         )
+
         if len(user_carts_with_voucher) > 0:
             raise ValidationError("You have already entered this voucher.")
 
+    def _test_vouchers(self, vouchers):
+        ''' Tests each of the vouchers against self._test_voucher() and raises
+        the collective ValidationError.
+        Future work will refactor _test_voucher in terms of this, and save some
+        queries. '''
+        errors = []
+        for voucher in vouchers:
+            try:
+                self._test_voucher(voucher)
+            except ValidationError as ve:
+                errors.append(ve)
+
+        if errors:
+            raise(ValidationError(ve))
 
     def validate_cart(self):
         ''' Determines whether the status of the current cart is valid;
@@ -214,7 +232,10 @@ class CartController(object):
         user = self.cart.user
         errors = []
 
-        # TODO: validate vouchers
+        try:
+            self._test_vouchers(self.cart.vouchers.all())
+        except ValidationError as ve:
+            errors.append(ve)
 
         items = rego.ProductItem.objects.filter(cart=cart)
         products = set(i.product for i in items)

registrasion/tests/test_voucher.py

@@ -37,11 +37,11 @@ class VoucherTestCases(RegistrationCartTestCase):
         cart_2.cart.active = False
         cart_2.cart.save()
 
-        # After the reservation duration, user 1 should not be able to apply
+        # After the reservation duration, even though the voucher has applied,
-        # voucher, as user 2 has paid for their cart.
+        # it exceeds the number of vouchers available.
         self.add_timedelta(rego.Voucher.RESERVATION_DURATION * 2)
         with self.assertRaises(ValidationError):
-            cart_1.apply_voucher(voucher.code)
+            cart_1.validate_cart()
 
     def test_voucher_enables_item(self):
         voucher = self.new_voucher()
@@ -103,9 +103,11 @@ class VoucherTestCases(RegistrationCartTestCase):
         voucher = self.new_voucher(limit=2)
         current_cart = TestingCartController.for_user(self.USER_1)
         current_cart.apply_voucher(voucher.code)
-        with self.assertRaises(ValidationError):
         current_cart.apply_voucher(voucher.code)
 
+        # You can apply the code twice, but it will only add to the cart once.
+        self.assertEqual(1, current_cart.cart.vouchers.count())
+
     def test_voucher_can_only_be_applied_once_across_multiple_carts(self):
         voucher = self.new_voucher(limit=2)
         current_cart = TestingCartController.for_user(self.USER_1)