diff --git a/registrasion/controllers/cart.py b/registrasion/controllers/cart.py index a643863f..def22c17 100644 --- a/registrasion/controllers/cart.py +++ b/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 - errors.append( - ("enabling_conditions", "An enabling condition failed") - ) + for error in errs: + errors.append(error) 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 - carts_with_voucher = active_carts.filter(vouchers=voucher) - if len(carts_with_voucher) >= voucher.limit: - raise ValidationError("This voucher is no longer available") + # Re-applying vouchers should be idempotent + if voucher in self.cart.vouchers.all(): + return - # It's not valid for users to re-enter a voucher they already have - 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.") + self._test_voucher(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): - raise ValidationError("Discounts are no longer available") + if not cond.is_met(user): + 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. diff --git a/registrasion/controllers/conditions.py b/registrasion/controllers/conditions.py index 9a0b26df..55b1a7d5 100644 --- a/registrasion/controllers/conditions.py +++ b/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): diff --git a/registrasion/controllers/product.py b/registrasion/controllers/product.py index d25a79bb..b49d5569 100644 --- a/registrasion/controllers/product.py +++ b/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) diff --git a/registrasion/tests/test_enabling_condition.py b/registrasion/tests/test_enabling_condition.py index e6c090dd..6dd3d102 100644 --- a/registrasion/tests/test_enabling_condition.py +++ b/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]) diff --git a/registrasion/tests/test_voucher.py b/registrasion/tests/test_voucher.py index de47c864..e3ac0d0d 100644 --- a/registrasion/tests/test_voucher.py +++ b/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 - # voucher, as user 2 has paid for their cart. + # After the reservation duration, even though the voucher has applied, + # 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()) diff --git a/registrasion/views.py b/registrasion/views.py index 77c00bb3..1cb4ec57 100644 --- a/registrasion/views.py +++ b/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):