Merge branch 'fix_cart'

Resolves #13.

registrasion/controllers/cart.py

@@ -120,14 +120,12 @@ class CartController(object):
         # Test each product limit here
         for product, quantity in product_quantities:
             if quantity < 0:
-                # TODO: batch errors
                 errors.append((product, "Value must be zero or greater."))
 
             prod = ProductController(product)
             limit = prod.user_quantity_remaining(self.cart.user)
 
             if quantity > limit:
-                # TODO: batch errors
                 errors.append((
                     product,
                     "You may only have %d of product: %s" % (
@@ -149,7 +147,6 @@ class CartController(object):
             to_add = sum(i[1] for i in by_cat[category])
 
             if to_add > limit:
-                # TODO: batch errors
                 errors.append((
                     category,
                     "You may only have %d items in category: %s" % (
@@ -164,10 +161,8 @@ class CartController(object):
         )
 
         if errs:
-            # TODO: batch errors
+            for error in errs:
-            errors.append(
+                errors.append(error)
-                ("enabling_conditions", "An enabling condition failed")
-            )
 
         if errors:
             raise CartValidationError(errors)
@@ -175,43 +170,79 @@ class CartController(object):
     def apply_voucher(self, voucher_code):
         ''' Applies the voucher with the given code to this cart. '''
 
-        # Is voucher exhausted?
-        active_carts = rego.Cart.reserved_carts()
-
         # Try and find the voucher
         voucher = rego.Voucher.objects.get(code=voucher_code.upper())
 
-        # It's invalid for a user to enter a voucher that's exhausted
+        # Re-applying vouchers should be idempotent
-        carts_with_voucher = active_carts.filter(vouchers=voucher)
+        if voucher in self.cart.vouchers.all():
-        if len(carts_with_voucher) >= voucher.limit:
+            return
-            raise ValidationError("This voucher is no longer available")
 
-        # It's not valid for users to re-enter a voucher they already have
+        self._test_voucher(voucher)
-        user_carts_with_voucher = rego.Cart.objects.filter(
-            user=self.cart.user,
-            released=False,
-            vouchers=voucher,
-        )
-        if len(user_carts_with_voucher) > 0:
-            raise ValidationError("You have already entered this voucher.")
 
         # If successful...
         self.cart.vouchers.add(voucher)
         self.end_batch()
 
+    def _test_voucher(self, voucher):
+        ''' Tests whether this voucher is allowed to be applied to this cart.
+        Raises ValidationError if not. '''
+
+        # Is voucher exhausted?
+        active_carts = rego.Cart.reserved_carts()
+
+        # 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 carts_with_voucher.count() >= 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 = carts_with_voucher.filter(
+            user=self.cart.user,
+        )
+
+        if user_carts_with_voucher.count() > 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;
         this is normally called before generating or paying an invoice '''
 
-        # TODO: validate vouchers
+        cart = self.cart
+        user = self.cart.user
+        errors = []
 
-        items = rego.ProductItem.objects.filter(cart=self.cart)
+        try:
+            self._test_vouchers(self.cart.vouchers.all())
+        except ValidationError as ve:
+            errors.append(ve)
+
+        items = rego.ProductItem.objects.filter(cart=cart)
 
         product_quantities = list((i.product, i.quantity) for i in items)
-        self._test_limits(product_quantities)
+        try:
+            self._test_limits(product_quantities)
+        except ValidationError as ve:
+            for error in ve.error_list:
+                errors.append(error.message[1])
 
         # Validate the discounts
-        discount_items = rego.DiscountItem.objects.filter(cart=self.cart)
+        discount_items = rego.DiscountItem.objects.filter(cart=cart)
         seen_discounts = set()
 
         for discount_item in discount_items:
@@ -223,12 +254,49 @@ class CartController(object):
                 pk=discount.pk)
             cond = ConditionController.for_condition(real_discount)
 
-            if not cond.is_met(self.cart.user):
+            if not cond.is_met(user):
-                raise ValidationError("Discounts are no longer available")
+                errors.append(
+                    ValidationError("Discounts are no longer available")
+                )
 
+        if errors:
+            raise ValidationError(errors)
+
+    @transaction.atomic
+    def fix_simple_errors(self):
+        ''' This attempts to fix the easy errors raised by ValidationError.
+        This includes removing items from the cart that are no longer
+        available, recalculating all of the discounts, and removing voucher
+        codes that are no longer available. '''
+
+        # Fix vouchers first (this affects available discounts)
+        active_carts = rego.Cart.reserved_carts()
+        to_remove = []
+        for voucher in self.cart.vouchers.all():
+            try:
+                self._test_voucher(voucher)
+            except ValidationError as ve:
+                to_remove.append(voucher)
+
+        for voucher in to_remove:
+            self.cart.vouchers.remove(voucher)
+
+        # Fix products and discounts
+        items = rego.ProductItem.objects.filter(cart=self.cart)
+        products = set(i.product for i in items)
+        available = set(ProductController.available_products(
+            self.cart.user,
+            products=products,
+        ))
+
+        not_available = products - available
+        zeros = [(product, 0) for product in not_available]
+
+        self.set_quantities(zeros)
+
+    @transaction.atomic
     def recalculate_discounts(self):
         ''' Calculates all of the discounts available for this product.
-        NB should be transactional, and it's terribly inefficient.
         '''
 
         # Delete the existing entries.

registrasion/controllers/conditions.py

@@ -44,6 +44,26 @@ class ConditionController(object):
         except KeyError:
             return ConditionController()
 
+
+    SINGLE = True
+    PLURAL = False
+    NONE = True
+    SOME = False
+    MESSAGE = {
+        NONE: {
+            SINGLE:
+                "%(items)s is no longer available to you",
+            PLURAL:
+                "%(items)s are no longer available to you",
+        },
+        SOME: {
+            SINGLE:
+                "Only %(remainder)d of the following item remains: %(items)s",
+            PLURAL:
+                "Only %(remainder)d of the following items remain: %(items)s"
+        },
+    }
+
     @classmethod
     def test_enabling_conditions(
             cls, user, products=None, product_quantities=None):
@@ -83,6 +103,8 @@ class ConditionController(object):
         # if there are no mandatory conditions
         non_mandatory = defaultdict(lambda: False)
 
+        messages = {}
+
         for condition in all_conditions:
             cond = cls.for_condition(condition)
             remainder = cond.user_quantity_remaining(user)
@@ -104,12 +126,21 @@ class ConditionController(object):
                 consumed = 1
             met = consumed <= remainder
 
+            if not met:
+                items = ", ".join(str(product) for product in all_products)
+                base = cls.MESSAGE[remainder == 0][len(all_products) == 1]
+                message = base % {"items": items, "remainder": remainder}
+
             for product in all_products:
                 if condition.mandatory:
                     mandatory[product] &= met
                 else:
                     non_mandatory[product] |= met
 
+                if not met and product not in messages:
+                    messages[product] = message
+
+
         valid = defaultdict(lambda: True)
         for product in itertools.chain(mandatory, non_mandatory):
             if product in mandatory:
@@ -119,7 +150,11 @@ class ConditionController(object):
                 # Otherwise, we need just one non-mandatory condition met
                 valid[product] = non_mandatory[product]
 
-        error_fields = [product for product in valid if not valid[product]]
+        error_fields = [
+            (product, messages[product])
+            for product in valid if not valid[product]
+        ]
+
         return error_fields
 
     def user_quantity_remaining(self, user):

registrasion/controllers/product.py

@@ -38,9 +38,10 @@ class ProductController(object):
             if cls(product).user_quantity_remaining(user) > 0
         )
 
-        failed_conditions = set(ConditionController.test_enabling_conditions(
+        failed_and_messages = ConditionController.test_enabling_conditions(
             user, products=passed_limits
-        ))
+        )
+        failed_conditions = set(i[0] for i in failed_and_messages)
 
         out = list(passed_limits - failed_conditions)
         out.sort(key=lambda product: product.order)

registrasion/tests/test_enabling_condition.py

@@ -293,3 +293,48 @@ class EnablingConditionTestCases(RegistrationCartTestCase):
 
         self.assertTrue(self.CAT_1 in cats)
         self.assertTrue(self.CAT_2 in cats)
+
+    def test_validate_cart_when_enabling_conditions_become_unmet(self):
+        self.add_product_enabling_condition(mandatory=False)
+
+        cart = TestingCartController.for_user(self.USER_1)
+        cart.add_to_cart(self.PROD_2, 1)
+        cart.add_to_cart(self.PROD_1, 1)
+
+        # Should pass
+        cart.validate_cart()
+
+        cart.set_quantity(self.PROD_2, 0)
+
+        # Should fail
+        with self.assertRaises(ValidationError):
+            cart.validate_cart()
+
+    def test_fix_simple_errors_resolves_unavailable_products(self):
+        self.test_validate_cart_when_enabling_conditions_become_unmet()
+        cart = TestingCartController.for_user(self.USER_1)
+
+        # Should just remove all of the unavailable products
+        cart.fix_simple_errors()
+        # Should now succeed
+        cart.validate_cart()
+
+        # Should keep PROD_2 in the cart
+        items = rego.ProductItem.objects.filter(cart=cart.cart)
+        self.assertFalse([i for i in items if i.product == self.PROD_1])
+
+    def test_fix_simple_errors_does_not_remove_limited_items(self):
+        cart = TestingCartController.for_user(self.USER_1)
+
+        cart.add_to_cart(self.PROD_2, 1)
+        cart.add_to_cart(self.PROD_1, 10)
+
+        # Should just remove all of the unavailable products
+        cart.fix_simple_errors()
+        # Should now succeed
+        cart.validate_cart()
+
+        # Should keep PROD_2 in the cart
+        # and also PROD_1, which is now exhausted for user.
+        items = rego.ProductItem.objects.filter(cart=cart.cart)
+        self.assertTrue([i for i in items if i.product == self.PROD_1])

registrasion/tests/test_voucher.py

@@ -37,11 +37,23 @@ 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_fix_simple_errors_resolves_unavailable_voucher(self):
+        self.test_apply_voucher()
+
+        # User has an exhausted voucher leftover from test_apply_voucher
+        cart_1 = TestingCartController.for_user(self.USER_1)
+        with self.assertRaises(ValidationError):
+            cart_1.validate_cart()
+
+        cart_1.fix_simple_errors()
+        # This should work now.
+        cart_1.validate_cart()
 
     def test_voucher_enables_item(self):
         voucher = self.new_voucher()
@@ -103,8 +115,10 @@ 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)
-            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)
@@ -114,6 +128,8 @@ class VoucherTestCases(RegistrationCartTestCase):
         inv = InvoiceController.for_cart(current_cart.cart)
         inv.pay("Hello!", inv.invoice.value)
 
+        current_cart = TestingCartController.for_user(self.USER_1)
+
         with self.assertRaises(ValidationError):
             current_cart.apply_voucher(voucher.code)
 
@@ -133,3 +149,11 @@ class VoucherTestCases(RegistrationCartTestCase):
 
         inv.refund("Hello!", inv.invoice.value)
         current_cart.apply_voucher(voucher.code)
+
+    def test_fix_simple_errors_does_not_remove_limited_voucher(self):
+        voucher = self.new_voucher(code="VOUCHER")
+        current_cart = TestingCartController.for_user(self.USER_1)
+        current_cart.apply_voucher(voucher.code)
+
+        current_cart.fix_simple_errors()
+        self.assertEqual(1, current_cart.cart.vouchers.count())

registrasion/views.py

@@ -337,6 +337,8 @@ def set_quantities_from_products_form(products_form, current_cart):
             product, message = ve_field.message
             if product in field_names:
                 field = field_names[product]
+            elif isinstance(product, rego.Product):
+                continue
             else:
                 field = None
             products_form.add_error(field, message)
@@ -377,10 +379,30 @@ def checkout(request):
     invoice. '''
 
     current_cart = CartController.for_user(request.user)
-    current_invoice = InvoiceController.for_cart(current_cart.cart)
+
+    if "fix_errors" in request.GET and request.GET["fix_errors"] == "true":
+        current_cart.fix_simple_errors()
+
+    try:
+        current_invoice = InvoiceController.for_cart(current_cart.cart)
+    except ValidationError as ve:
+        return checkout_errors(request, ve)
 
     return redirect("invoice", current_invoice.invoice.id)
 
+def checkout_errors(request, errors):
+
+    error_list = []
+    for error in errors.error_list:
+        if isinstance(error, tuple):
+            error = error[1]
+        error_list.append(error)
+
+    data = {
+        "error_list": error_list,
+    }
+
+    return render(request, "registrasion/checkout_errors.html", data)
 
 @login_required
 def invoice(request, invoice_id):