From de96e7815e7417ed60dbbebf91e70d1a73a1f702 Mon Sep 17 00:00:00 2001 From: Sebastian Vetter Date: Wed, 6 Nov 2013 10:04:58 +1100 Subject: [PATCH] Allow shipping_address being 'None' for an order 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. --- oscar/apps/checkout/mixins.py | 4 ++++ tests/unit/checkout/mixins_tests.py | 13 +++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 tests/unit/checkout/mixins_tests.py diff --git a/oscar/apps/checkout/mixins.py b/oscar/apps/checkout/mixins.py index 0eea5ab62f6..8456a0ef988 100644 --- a/oscar/apps/checkout/mixins.py +++ b/oscar/apps/checkout/mixins.py @@ -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) diff --git a/tests/unit/checkout/mixins_tests.py b/tests/unit/checkout/mixins_tests.py new file mode 100644 index 00000000000..562858b28ad --- /dev/null +++ b/tests/unit/checkout/mixins_tests.py @@ -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)