Added fix for issue with users being able to bypass shipping.

Taking inspiration from Asia's pull request.

oscar/apps/basket/abstract_models.py

@@ -189,6 +189,16 @@ class AbstractBasket(models.Model):
         for line in self.all_lines():
             line.set_as_tax_exempt()
 
+    def is_shipping_required(self):
+        """
+        Test whether the basket contains physical products that require
+        shipping.
+        """
+        for line in self.all_lines():
+            if line.product.is_shipping_required:
+                return True
+        return False
+
     # =======
     # Helpers
     # =======

oscar/apps/checkout/tests.py

@@ -14,6 +14,11 @@ from oscar.apps.order.models import Order
 class CheckoutMixin(object):
     fixtures = ['countries.json']
 
+    def add_product_to_basket(self):
+        product = create_product(price=D('12.00'))
+        self.client.post(reverse('basket:add'), {'product_id': product.id,
+                                                 'quantity': 1})
+
     def complete_guest_email_form(self, email='test@example.com'):
         response = self.client.post(reverse('checkout:index'),
                                     {'username': email,
@@ -120,6 +125,7 @@ class ShippingMethodViewTests(ClientTestCase, CheckoutMixin):
     fixtures = ['countries.json']
 
     def test_shipping_method_view_redirects_if_no_shipping_address(self):
+        self.add_product_to_basket()
         response = self.client.get(reverse('checkout:shipping-method'))
         self.assertIsRedirect(response)
         self.assertRedirectUrlName(response, 'checkout:shipping-address')
@@ -133,6 +139,7 @@ class ShippingMethodViewTests(ClientTestCase, CheckoutMixin):
 class PaymentMethodViewTests(ClientTestCase, CheckoutMixin):
 
     def test_view_redirects_if_no_shipping_address(self):
+        self.add_product_to_basket() 
         response = self.client.get(reverse('checkout:payment-method'))
         self.assertIsRedirect(response)
         self.assertRedirectUrlName(response, 'checkout:shipping-address')
@@ -147,6 +154,7 @@ class PaymentMethodViewTests(ClientTestCase, CheckoutMixin):
 class PreviewViewTests(ClientTestCase, CheckoutMixin):
 
     def test_view_redirects_if_no_shipping_address(self):
+        self.add_product_to_basket()
         response = self.client.get(reverse('checkout:preview'))
         self.assertIsRedirect(response)
         self.assertRedirectUrlName(response, 'checkout:shipping-address')
@@ -167,11 +175,13 @@ class PreviewViewTests(ClientTestCase, CheckoutMixin):
 class PaymentDetailsViewTests(ClientTestCase, CheckoutMixin):
 
     def test_view_redirects_if_no_shipping_address(self):
+        self.add_product_to_basket()
         response = self.client.post(reverse('checkout:payment-details'))
         self.assertIsRedirect(response)
         self.assertRedirectUrlName(response, 'checkout:shipping-address')
 
     def test_view_redirects_if_no_shipping_method(self):
+        self.add_product_to_basket()
         self.complete_shipping_address()
         response = self.client.post(reverse('checkout:payment-details'))
         self.assertIsRedirect(response)

oscar/apps/checkout/utils.py

@@ -64,20 +64,6 @@ class CheckoutSessionData(object):
         self._unset('shipping', 'new_address_fields')
         self._unset('shipping', 'user_address_id')
 
-    def no_shipping_required(self):
-        """
-        Record fact that basket doesn't require a shipping address or method
-        """
-        self.reset_shipping_data()
-        self._set('shipping', 'is_required', False)
-
-    def shipping_required(self):
-        """
-        Record fact that basket does require a shipping address or method
-        """
-        self.reset_shipping_data()
-        self._set('shipping', 'is_required', True)
-
     def ship_to_user_address(self, address):
         """
         Set existing shipping address id to session and unset address fields from session
@@ -104,10 +90,12 @@ class CheckoutSessionData(object):
         """
         return self._get('shipping', 'user_address_id')
 
-    def is_shipping_required(self):
-        return self._get('shipping', 'is_required', True)
-
     def is_shipping_address_set(self):
+        """
+        Test whether a shipping address has been stored in the session.
+
+        This can be from a new address or re-using an existing address.
+        """
         new_fields = self.new_shipping_address_fields()
         has_new_address = new_fields is not None
         has_old_address = self.user_address_id() > 0

oscar/apps/checkout/views.py

@@ -183,24 +183,14 @@ class ShippingAddressView(CheckoutSessionMixin, FormView):
 
         # Check to see that a shipping address is actually required.  It may not be if
         # the basket is purely downloads
-        if not self.does_basket_require_shipping(request.basket):
+        if not request.basket.is_shipping_required():
             messages.info(request, _("Your basket does not require a shipping address to be submitted"))
-            self.checkout_session.no_shipping_required()
             return HttpResponseRedirect(self.get_success_url())
         else:
             self.checkout_session.shipping_required()
 
         return super(ShippingAddressView, self).get(request, *args, **kwargs)
 
-    def does_basket_require_shipping(self, basket):
-        """
-        Test whether the contents of the basket require shipping
-        """
-        for line in basket.all_lines():
-            if line.product.is_shipping_required:
-                return True
-        return False
-
     def get_initial(self):
         return self.checkout_session.new_shipping_address_fields()
 
@@ -322,7 +312,7 @@ class ShippingMethodView(CheckoutSessionMixin, TemplateView):
 
     def get(self, request, *args, **kwargs):
         # Check that shipping is required at all
-        if not self.checkout_session.is_shipping_required():
+        if not request.basket.is_shipping_required():
             self.checkout_session.use_shipping_method(NoShippingRequired().code)
             return self.get_success_response()
 
@@ -398,7 +388,7 @@ class PaymentMethodView(CheckoutSessionMixin, TemplateView):
 
     def get(self, request, *args, **kwargs):
         # Check that shipping address has been completed
-        if self.checkout_session.is_shipping_required() and not self.checkout_session.is_shipping_address_set():
+        if request.basket.is_shipping_required() and not self.checkout_session.is_shipping_address_set():
             messages.error(request, _("Please choose a shipping address"))
             return HttpResponseRedirect(reverse('checkout:shipping-address'))
         # Check that shipping method has been set
@@ -516,7 +506,7 @@ class OrderPlacementMixin(CheckoutSessionMixin):
         If the shipping address was selected from the user's address book,
         then we convert the UserAddress to a ShippingAddress.
         """
-        if not self.checkout_session.is_shipping_required():
+        if not self.request.basket.is_shipping_required():
             return None
 
         addr_data = self.checkout_session.new_shipping_address_fields()
@@ -674,7 +664,7 @@ class PaymentDetailsView(OrderPlacementMixin, TemplateView):
 
     def get_error_response(self):
         # Check that shipping address has been completed
-        if self.checkout_session.is_shipping_required() and not self.checkout_session.is_shipping_address_set():
+        if self.request.basket.is_shipping_required() and not self.checkout_session.is_shipping_address_set():
             messages.error(self.request, _("Please choose a shipping address"))
             return HttpResponseRedirect(reverse('checkout:shipping-address'))
         # Check that shipping method has been set
@@ -695,6 +685,7 @@ class PaymentDetailsView(OrderPlacementMixin, TemplateView):
         then the method can call submit()
         """
         error_response = self.get_error_response()
+        
         if error_response:
             return error_response
         if self.preview: