Distinguish between pre- and skip-conditions for checkout views

Terminology:

- A *pre*-condition is a condition that must be met in order for a view
  to be available. If it isn't then the customer will be redirected to
  a view *earlier* in the chain.

- A *skip*-condition is a condition that must NOT be met in order for a
  view to be available. If the condition is met, this means the view can
  be skipped and the customer should be redirected to a view *later* in
  the chain.

This change implements the above by introducing skip conditions for
checkout views. They allow the previous payment pre-conditions to be
cleanly separated.

An existing basket pre-condition is also changed to be a skip-condition.

oscar/apps/checkout/exceptions.py

@@ -8,3 +8,12 @@ class FailedPreCondition(Exception):
             self.messages = messages
         else:
             self.messages = []
+
+
+class PassedSkipCondition(Exception):
+    """
+    To be raised when a skip condition has been passed and the current view
+    should be skipped. The passed URL dictates where to.
+    """
+    def __init__(self, url):
+        self.url = url

oscar/apps/checkout/session.py

@@ -1,3 +1,5 @@
+from decimal import Decimal as D
+
 from django.contrib import messages
 from django import http
 from django.core.exceptions import ImproperlyConfigured
@@ -25,9 +27,15 @@ class CheckoutSessionMixin(object):
     checkout information is available in the template context.
     """
     # This should be list of method names that get executed before the normal
-    # flow of the view.
+    # flow of the view. Each method should check some condition has been met.
+    # If not, then an exception is raised that indicates the URL the customer
+    # should be redirect to.
     pre_conditions = None
 
+    # Skip conditions check whether the view should be skipped. They work in
+    # the same way as pre-conditions.
+    skip_conditions = None
+
     def dispatch(self, request, *args, **kwargs):
         # Assign the checkout session manager so it's available in all checkout
         # views.
@@ -41,6 +49,12 @@ class CheckoutSessionMixin(object):
                 messages.warning(request, message)
             return http.HttpResponseRedirect(e.url)
 
+        # Check if this view should be skipped
+        try:
+            self.check_skip_conditions(request)
+        except exceptions.PassedSkipCondition as e:
+            return http.HttpResponseRedirect(e.url)
+
         return super(CheckoutSessionMixin, self).dispatch(
             request, *args, **kwargs)
 
@@ -61,6 +75,23 @@ class CheckoutSessionMixin(object):
             return []
         return self.pre_conditions
 
+    def check_skip_conditions(self, request):
+        skip_conditions = self.get_skip_conditions(request)
+        for method_name in skip_conditions:
+            if not hasattr(self, method_name):
+                raise ImproperlyConfigured(
+                    "There is no method '%s' to call as a skip-condition" % (
+                        method_name))
+            getattr(self, method_name)(request)
+
+    def get_skip_conditions(self, request):
+        """
+        Return the skip-condition method names to run for this view
+        """
+        if self.skip_conditions is None:
+            return []
+        return self.skip_conditions
+
     # Re-usable pre-condition validators
 
     def check_basket_is_not_empty(self, request):
@@ -107,14 +138,6 @@ class CheckoutSessionMixin(object):
                     "Please either sign in or enter your email address")
             )
 
-    def check_basket_requires_shipping(self, request):
-        # Check to see that a shipping address is actually required.  It may
-        # not be if the basket is purely downloads
-        if not request.basket.is_shipping_required():
-            raise exceptions.FailedPreCondition(
-                url=reverse('checkout:shipping-method')
-            )
-
     def check_shipping_data_is_captured(self, request):
         if not request.basket.is_shipping_required():
             return
@@ -134,32 +157,35 @@ class CheckoutSessionMixin(object):
                 message=_("Please choose a shipping method")
             )
 
-    def check_payment_is_necessary(self, request):
-        shipping_address = self.get_shipping_address(request.basket)
-        shipping_method = self.get_shipping_method(
-            request.basket, shipping_address)
-        total = self.get_order_totals(request.basket, shipping_method)
-        if total.excl_tax == 0:
-            raise exceptions.FailedPreCondition(
-                url=reverse('checkout:preview'),
-                message=_("Payment is not necessary for this order")
-            )
-
     def check_payment_data_is_captured(self, request):
         # We don't collect payment data by default so we don't have anything to
         # validate here. If your shop requires forms to be submitted on the
         # payment details page, then override this method to check that the
         # relevant data is available. Often just enforcing that the preview
         # view is only accessible from a POST request is sufficient.
-
-        # Skip check if payment isn't necessary
-        try:
-            self.check_payment_is_necessary(request)
-        except exceptions.FailedPreCondition:
-            return
-        # Validate payment data here
         pass
 
+    # Re-usable skip conditions
+
+    def skip_unless_basket_requires_shipping(self, request):
+        # Check to see that a shipping address is actually required.  It may
+        # not be if the basket is purely downloads
+        if not request.basket.is_shipping_required():
+            raise exceptions.PassedSkipCondition(
+                url=reverse('checkout:payment-method')
+            )
+
+    def skip_unless_payment_is_required(self, request):
+        # Check to see if payment is actually required for this order.
+        shipping_address = self.get_shipping_address(request.basket)
+        shipping_method = self.get_shipping_method(
+            request.basket, shipping_address)
+        total = self.get_order_totals(request.basket, shipping_method)
+        if total.excl_tax == D('0.00'):
+            raise exceptions.PassedSkipCondition(
+                url=reverse('checkout:preview')
+            )
+
     # Helpers
 
     def get_context_data(self, **kwargs):

oscar/apps/checkout/views.py

@@ -134,8 +134,8 @@ class ShippingAddressView(CheckoutSessionMixin, generic.FormView):
     success_url = reverse_lazy('checkout:shipping-method')
     pre_conditions = ['check_basket_is_not_empty',
                       'check_basket_is_valid',
-                      'check_user_email_is_captured',
-                      'check_basket_requires_shipping']
+                      'check_user_email_is_captured']
+    skip_conditions = ['skip_unless_basket_requires_shipping']
 
     def get_initial(self):
         return self.checkout_session.new_shipping_address_fields()
@@ -244,7 +244,7 @@ class ShippingMethodView(CheckoutSessionMixin, generic.TemplateView):
     template_name = 'checkout/shipping_methods.html'
     pre_conditions = ['check_basket_is_not_empty',
                       'check_basket_is_valid',
-                      'check_user_email_is_captured', ]
+                      'check_user_email_is_captured',]
 
     def get(self, request, *args, **kwargs):
         # These pre-conditions can't easily be factored out into the normal
@@ -338,8 +338,8 @@ class PaymentMethodView(CheckoutSessionMixin, generic.TemplateView):
         'check_basket_is_not_empty',
         'check_basket_is_valid',
         'check_user_email_is_captured',
-        'check_shipping_data_is_captured',
-        'check_payment_is_necessary']
+        'check_shipping_data_is_captured']
+    skip_conditions = ['skip_unless_payment_is_required']
 
     def get(self, request, *args, **kwargs):
         # By default we redirect straight onto the payment details view. Shops
@@ -391,6 +391,8 @@ class PaymentDetailsView(OrderPlacementMixin, generic.TemplateView):
     template_name = 'checkout/payment_details.html'
     template_name_preview = 'checkout/preview.html'
 
+    # These conditions are extended at runtime depending on whether we are in
+    # 'preview' mode or not.
     pre_conditions = [
         'check_basket_is_not_empty',
         'check_basket_is_valid',
@@ -406,9 +408,13 @@ class PaymentDetailsView(OrderPlacementMixin, generic.TemplateView):
             # The preview view needs to ensure payment information has been
             # correctly captured.
             return self.pre_conditions + ['check_payment_data_is_captured']
-        else:
+        return super(PaymentDetailsView, self).get_pre_conditions(request)
+
+    def get_skip_conditions(self, request):
+        if not self.preview:
             # Payment details should only be collected if necessary
-            return self.pre_conditions + ['check_payment_is_necessary']
+            return ['skip_unless_payment_is_required']
+        return super(PaymentDetailsView, self).get_skip_conditions(request)
 
     def post(self, request, *args, **kwargs):
         # Posting to payment-details isn't the right thing to do.  Form