Improved basket usage.

* Less DB queries
* Added basic tests

oscar/apps/basket/abstract_models.py

@@ -63,6 +63,8 @@ class AbstractBasket(models.Model):
         This is important for offers as they alter the line models and you don't
         want to reload them from the DB.
         """
+        if self.id is None:
+            return []
         if self._lines is None:
             self._lines = self.lines.all()
         return self._lines
@@ -237,8 +239,10 @@ class AbstractBasket(models.Model):
 
     @property
     def is_empty(self):
-        """Return bool based on basket having 0 lines"""
-        return self.num_lines == 0
+        """
+        Test if this basket is empty
+        """
+        return self.id is None or self.num_lines == 0
 
     @property
     def total_excl_tax(self):
@@ -319,7 +323,7 @@ class AbstractBasket(models.Model):
     @property
     def num_lines(self):
         """Return number of lines"""
-        return self.all_lines().count()
+        return len(self.all_lines())
 
     @property
     def num_items(self):

oscar/apps/basket/middleware.py

@@ -13,7 +13,7 @@ Basket = get_model('basket', 'basket')
 class BasketMiddleware(object):
 
     def process_request(self, request):
-        self.cookies_to_delete = []
+        request.cookies_to_delete = []
         basket = self.get_basket(request)
         self.apply_offers_to_basket(request, basket)
         request.basket = basket
@@ -23,7 +23,7 @@ class BasketMiddleware(object):
         cookie_basket = self.get_cookie_basket(settings.OSCAR_BASKET_COOKIE_OPEN,
                                                request, manager)
 
-        if request.user.is_authenticated():
+        if hasattr(request, 'user') and request.user.is_authenticated():
             # Signed-in user: if they have a cookie basket too, it means
             # that they have just signed in and we need to merge their cookie
             # basket into their user basket, then delete the cookie
@@ -31,7 +31,7 @@ class BasketMiddleware(object):
                 basket, _ = manager.get_or_create(owner=request.user)
             except Basket.MultipleObjectsReturned:
                 # Not sure quite how we end up here with multiple baskets
-                # We merge any  them and create a fresh one
+                # We merge them and create a fresh one
                 old_baskets = list(manager.filter(owner=request.user))
                 basket = old_baskets[0]
                 for other_basket in old_baskets[1:]:
@@ -39,7 +39,7 @@ class BasketMiddleware(object):
 
             if cookie_basket:
                 self.merge_baskets(basket, cookie_basket)
-                self.cookies_to_delete.append(settings.OSCAR_BASKET_COOKIE_OPEN)
+                request.cookies_to_delete.append(settings.OSCAR_BASKET_COOKIE_OPEN)
         elif cookie_basket:
             # Anonymous user with a basket tied to the cookie
             basket = cookie_basket
@@ -59,8 +59,8 @@ class BasketMiddleware(object):
 
     def process_response(self, request, response):
         # Delete any surplus cookies
-        if hasattr(self, 'cookies_to_delete'):
-            for cookie_key in self.cookies_to_delete:
+        if hasattr(request, 'cookies_to_delete'):
+            for cookie_key in request.cookies_to_delete:
                 response.delete_cookie(cookie_key)
 
         # If a basket has had products added to it, but the user is anonymous
@@ -100,9 +100,9 @@ class BasketMiddleware(object):
                     basket = Basket.objects.get(pk=basket_id, owner=None,
                                                 status=OPEN)
                 except Basket.DoesNotExist:
-                    self.cookies_to_delete.append(cookie_key)
+                    request.cookies_to_delete.append(cookie_key)
             else:
-                self.cookies_to_delete.append(cookie_key)
+                request.cookies_to_delete.append(cookie_key)
         return basket
 
     def apply_offers_to_basket(self, request, basket):

oscar/templates/oscar/partials/nav_alternate.html

@@ -11,7 +11,7 @@
             </a>
             <a class="btn btn-navbar app-ico ico_shop_bag btn-cart hidden" href="{% url checkout:index %}">
 				<strong>{% trans "Basket" %}
-                {% if request.basket.lines.count %}
+                {% if not request.basket.is_empty %}
 				{% trans "Total:" %} {{ basket.total_incl_tax|currency }}
                 {% endif %}
                 </strong>

tests/unit/basket_tests.py

@@ -1,39 +1,49 @@
 import datetime
 
 from django.test import TestCase
+from django.test.client import RequestFactory
 
-from oscar.apps.basket.models import Basket, Line
+from oscar.apps.basket.models import Basket
+from oscar.apps.basket.middleware import BasketMiddleware
 from oscar.test.helpers import create_product
 from oscar.apps.basket.reports import (
     OpenBasketReportGenerator, SubmittedBasketReportGenerator)
 
 
-class BasketModelTest(TestCase):
+class TestBasketModel(TestCase):
 
     def setUp(self):
         self.basket = Basket.objects.create()
-        self.dummy_product = create_product()
+        self.product = create_product()
 
-    def test_empty_baskets_have_zero_lines(self):
-        self.assertTrue(Basket().num_lines == 0)
+    def test_an_empty_basket_has_zero_lines(self):
+        self.assertEqual(0, self.basket.num_lines)
 
     def test_new_baskets_are_empty(self):
-        self.assertTrue(Basket().is_empty)
+        self.assertTrue(self.basket.is_empty)
 
-    def test_basket_have_with_one_line(self):
-        Line.objects.create(basket=self.basket, product=self.dummy_product)
-        self.assertTrue(self.basket.num_lines == 1)
-
-    def test_add_product_creates_line(self):
-        self.basket.add_product(self.dummy_product)
-        self.assertTrue(self.basket.num_lines == 1)
+    def test_adding_product_creates_line(self):
+        self.basket.add_product(self.product)
+        self.assertEqual(1, self.basket.num_lines)
 
     def test_adding_multiproduct_line_returns_correct_number_of_items(self):
-        self.basket.add_product(self.dummy_product, 10)
+        self.basket.add_product(self.product, 10)
         self.assertEqual(self.basket.num_items, 10)
+        self.assertEqual(self.basket.num_lines, 1)
+
+
+class TestBasketMiddleware(TestCase):
+
+    def setUp(self):
+        self.middleware = BasketMiddleware()
+
+    def test_basket_is_attached_to_request(self):
+        req = RequestFactory().get('/')
+        self.middleware.process_request(req)
+        self.assertTrue(hasattr(req, 'basket'))
 
 
-class BasketReportTests(TestCase):
+class TestBasketReports(TestCase):
 
     def test_open_report_doesnt_error(self):
         data = {