Skip to content

Commit

Permalink
Allow shipping_address being 'None' for an order
Browse files Browse the repository at this point in the history
I've had an issue with orders that contain only items that don't require
shipping. In this case, the shipping address could be ``None`` which
breaks the ``create_shipping_address`` method.

I've added a test case for it and provided a simple fix. I wasn't quit
sure if the check should be in ``create_shipping_address`` or in
``place_order`` and only call it for an address that is not ``None``.
I've decided to go with the first because it is also simpler to test
this in isolation but am not fussed about moving it up.
  • Loading branch information
Sebastian Vetter committed Nov 5, 2013
1 parent a662141 commit de96e78
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
4 changes: 4 additions & 0 deletions oscar/apps/checkout/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ def create_shipping_address(self, user, shipping_address):
Compared to self.get_shipping_address(), ShippingAddress is saved and
makes sure that appropriate UserAddress exists.
"""
# For an order that only contains items that don't require shipping we
# won't have a shipping address, so we have to check for it.
if not shipping_address:
return None
shipping_address.save()
if user.is_authenticated():
self.update_address_book(user, shipping_address)
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/checkout/mixins_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import mock

from django.test import TestCase

from oscar.apps.checkout.mixins import OrderPlacementMixin


class TestOrderPlacementMixin(TestCase):

def test_can_create_shipping_address_for_empty_address(self):
address = OrderPlacementMixin().create_shipping_address(
user=mock.Mock(), shipping_address=None)
self.assertEquals(address, None)

0 comments on commit de96e78

Please sign in to comment.