Rework the clean() method of the add-to-basket form

This fixes a bug with adding customised products to the basket, where
the clean method would get a MultipleObjectsReturned exception as the
lookup wasn't taking options into account.

Also fixed a display issue with required options in basket forms.

oscar/apps/basket/abstract_models.py

@@ -393,6 +393,10 @@ class AbstractBasket(models.Model):
     def contains_a_voucher(self):
         return self.vouchers.all().count() > 0
 
+    # =============
+    # Query methods
+    # =============
+
     def contains_voucher(self, code):
         """
         Test whether the basket contains a voucher with a given code
@@ -404,6 +408,16 @@ class AbstractBasket(models.Model):
         else:
             return True
 
+    def line_quantity(self, product, options=None):
+        """
+        Return the current quantity of a specific product and options
+        """
+        ref = self._create_line_reference(product, options)
+        try:
+            return self.lines.get(line_reference=ref).quantity
+        except ObjectDoesNotExist:
+            return 0
+
 
 class AbstractLine(models.Model):
     """

oscar/apps/basket/forms.py

@@ -126,7 +126,20 @@ class AddToBasketForm(forms.Form):
             else:
                 self._create_product_fields(instance)
 
+    def cleaned_options(self):
+        """
+        Return submitted options in a clean format
+        """
+        options = []
+        for option in self.instance.options:
+            if option.code in self.cleaned_data:
+                options.append({
+                    'option': option,
+                    'value': self.cleaned_data[option.code]})
+        return options
+
     def clean(self):
+        # Check product exists
         try:
             product = Product.objects.get(
                 id=self.cleaned_data.get('product_id', None))
@@ -134,13 +147,9 @@ class AddToBasketForm(forms.Form):
             raise forms.ValidationError(
                 _("Please select a valid product"))
 
-        qty = self.cleaned_data.get('quantity', 1)
-        try:
-            line = self.basket.lines.get(product=product)
-        except Line.DoesNotExist:
-            desired_qty = qty
-        else:
-            desired_qty = qty + line.quantity
+        current_qty = self.basket.line_quantity(product,
+                                                self.cleaned_options())
+        desired_qty = current_qty + self.cleaned_data.get('quantity', 1)
 
         is_available, reason = product.is_purchase_permitted(
             user=self.user, quantity=desired_qty)
@@ -183,7 +192,9 @@ class AddToBasketForm(forms.Form):
                                                       label=_("Variant"))
 
     def _create_product_fields(self, item):
-        """Add the product option fields."""
+        """
+        Add the product option fields.
+        """
         for option in item.options:
             self._add_option_field(item, option)
 
@@ -194,7 +205,8 @@ class AddToBasketForm(forms.Form):
         This is designed to be overridden so that specific widgets can be used
         for certain types of options.
         """
-        self.fields[option.code] = forms.CharField()
+        kwargs = {'required': option.is_required}
+        self.fields[option.code] = forms.CharField(**kwargs)
 
 
 class SimpleAddToBasketForm(AddToBasketForm):

oscar/apps/basket/views.py

@@ -173,14 +173,9 @@ class BasketAddView(FormView):
         return url
 
     def form_valid(self, form):
-        options = []
-        for option in form.instance.options:
-            if option.code in form.cleaned_data:
-                options.append({
-                    'option': option,
-                    'value': form.cleaned_data[option.code]})
         self.request.basket.add_product(
-            form.instance, form.cleaned_data['quantity'], options)
+            form.instance, form.cleaned_data['quantity'],
+            form.cleaned_options())
         messages.success(self.request, self.get_success_message(form))
 
         # Send signal for basket addition

oscar/apps/catalogue/abstract_models.py

@@ -777,15 +777,16 @@ class AbstractAttributeEntityType(models.Model):
 
 
 class AbstractOption(models.Model):
-    u"""
+    """
     An option that can be selected for a particular item when the product
     is added to the basket.
 
-    Eg a list ID for an SMS message send, or a personalised message to
+    For example,  a list ID for an SMS message send, or a personalised message to
     print on a T-shirt.
 
-    This is not the same as an attribute as options do not have a fixed value for
-    a particular item - options, they need to be specified by the customer.
+    This is not the same as an 'attribute' as options do not have a fixed value
+    for a particular item.  Instead, option need to be specified by a customer
+    when add the item to their basket.
     """
     name = models.CharField(_("Name"), max_length=128)
     code = models.SlugField(_("Code"), max_length=128)
@@ -795,7 +796,8 @@ class AbstractOption(models.Model):
         (REQUIRED, _("Required - a value for this option must be specified")),
         (OPTIONAL, _("Optional - a value for this option can be omitted")),
     )
-    type = models.CharField(_("Status"), max_length=128, default=REQUIRED, choices=TYPE_CHOICES)
+    type = models.CharField(_("Status"), max_length=128, default=REQUIRED,
+                            choices=TYPE_CHOICES)
 
     class Meta:
         abstract = True
@@ -810,6 +812,10 @@ class AbstractOption(models.Model):
             self.code = slugify(self.name)
         super(AbstractOption, self).save(*args, **kwargs)
 
+    @property
+    def is_required(self):
+        return self.type == self.REQUIRED
+
 
 class MissingProductImage(object):
 

oscar/templatetags/basket_tags.py

@@ -53,8 +53,6 @@ class BasketFormNode(template.Node):
             initial = {}
             if not product.is_group:
                 initial['product_id'] = product.id
-            context[self.form_var] = self.form_class(basket,
-                                                     user=None,
-                                                     instance=product,
-                                                     initial=initial)
+            context[self.form_var] = self.form_class(
+                basket, user=None, instance=product, initial=initial)
         return ''

tests/unit/basket_tests.py

@@ -9,9 +9,10 @@ from oscar.apps.basket.middleware import BasketMiddleware
 from oscar.test.helpers import create_product
 from oscar.apps.basket.reports import (
     OpenBasketReportGenerator, SubmittedBasketReportGenerator)
+from oscar.apps.catalogue.models import Option
 
 
-class TestBasketModel(TestCase):
+class TestABasket(TestCase):
 
     def setUp(self):
         self.basket = Basket()
@@ -48,6 +49,21 @@ class TestBasketModel(TestCase):
         self.basket.flush()
         self.assertEqual(self.basket.num_items, 0)
 
+    def test_returns_correct_quantity_for_missing_product(self):
+        self.assertEqual(0, self.basket.line_quantity(self.product))
+
+    def test_returns_correct_quantity_for_existing_product(self):
+        self.basket.add_product(self.product)
+        self.assertEqual(1, self.basket.line_quantity(self.product))
+
+    def test_returns_correct_quantity_for_existing_product_with_options(self):
+        option = Option.objects.create(name="Message")
+        options = [{"option": option, "value": "2"}]
+        self.basket.add_product(self.product, options=options)
+        self.assertEqual(0, self.basket.line_quantity(self.product))
+        self.assertEqual(1, self.basket.line_quantity(self.product, options))
+
+
 
 class TestBasketMiddleware(TestCase):