Do not perform stock allocation manipulations on non-stock-tracked products (#2567)

* Silently skip stock allocation manipulations if the product class doesn't track stock.

* Make it possible to cancel/consume stock allocations without line args.

You can now do:
consume_stock_allocations(order)
cancel_stock_allocations(order)
without the lines / line_quantities arguments.

src/oscar/apps/order/processing.py

@@ -180,27 +180,49 @@ class EventHandler(object):
         """
         Check whether stock records still have enough stock to honour the
         requested allocations.
+
+        Lines whose product doesn't track stock are disregarded, which means
+        this method will return True if only non-stock-tracking-lines are
+        passed.
+        This means you can just throw all order lines to this method, without
+        checking whether stock tracking is enabled or not.
+        This is okay, as calling consume_stock_allocations() has no effect for
+        non-stock-tracking lines.
         """
         for line, qty in zip(lines, line_quantities):
             record = line.stockrecord
             if not record:
                 return False
+            if not record.can_track_allocations:
+                continue
             if not record.is_allocation_consumption_possible(qty):
                 return False
         return True
 
-    def consume_stock_allocations(self, order, lines, line_quantities):
+    def consume_stock_allocations(self, order, lines=None, line_quantities=None):
         """
-        Consume the stock allocations for the passed lines
+        Consume the stock allocations for the passed lines.
+
+        If no lines/quantities are passed, do it for all lines.
         """
+        if not lines:
+            lines = order.lines.all()
+        if not line_quantities:
+            line_quantities = [line.quantity for line in lines]
         for line, qty in zip(lines, line_quantities):
             if line.stockrecord:
                 line.stockrecord.consume_allocation(qty)
 
-    def cancel_stock_allocations(self, order, lines, line_quantities):
+    def cancel_stock_allocations(self, order, lines=None, line_quantities=None):
         """
-        Cancel the stock allocations for the passed lines
+        Cancel the stock allocations for the passed lines.
+
+        If no lines/quantities are passed, do it for all lines.
         """
+        if not lines:
+            lines = order.lines.all()
+        if not line_quantities:
+            line_quantities = [line.quantity for line in lines]
         for line, qty in zip(lines, line_quantities):
             if line.stockrecord:
                 line.stockrecord.cancel_allocation(qty)

src/oscar/apps/partner/abstract_models.py

@@ -2,6 +2,7 @@ from django.db import models, router
 from django.db.models import F, Value, signals
 from django.db.models.functions import Coalesce
 from django.utils.encoding import python_2_unicode_compatible
+from django.utils.functional import cached_property
 from django.utils.timezone import now
 from django.utils.translation import ugettext_lazy as _
 from django.utils.translation import pgettext_lazy
@@ -180,6 +181,11 @@ class AbstractStockRecord(models.Model):
             return self.num_in_stock
         return self.num_in_stock - self.num_allocated
 
+    @cached_property
+    def can_track_allocations(self):
+        """Return True if the Product is set for stock tracking."""
+        return self.product.product_class.track_stock
+
     # 2-stage stock management model
 
     def allocate(self, quantity):
@@ -190,6 +196,9 @@ class AbstractStockRecord(models.Model):
         product is actually shipped, then we 'consume' the allocation.
 
         """
+        # Doesn't make sense to allocate if stock tracking is off.
+        if not self.can_track_allocations:
+            return
         # Send the pre-save signal
         signals.pre_save.send(
             sender=self.__class__,
@@ -232,6 +241,8 @@ class AbstractStockRecord(models.Model):
         This is used when an item is shipped.  We remove the original
         allocation and adjust the number in stock accordingly
         """
+        if not self.can_track_allocations:
+            return
         if not self.is_allocation_consumption_possible(quantity):
             raise InvalidStockAdjustment(
                 _('Invalid stock consumption request'))
@@ -241,6 +252,8 @@ class AbstractStockRecord(models.Model):
     consume_allocation.alters_data = True
 
     def cancel_allocation(self, quantity):
+        if not self.can_track_allocations:
+            return
         # We ignore requests that request a cancellation of more than the
         # amount already allocated.
         self.num_allocated -= min(self.num_allocated, quantity)

tests/integration/order/test_event_handler.py

@@ -57,6 +57,189 @@ class TestEventHandler(TestCase):
             self.handler.handle_shipping_event(
                 order, self.shipped, lines, [4])
 
+    def test_are_stock_allocations_available(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=True)
+        product = factories.ProductFactory(product_class=product_class)
+
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        line = order.lines.get()
+        self.assertEqual(
+            self.handler.are_stock_allocations_available(
+                [line], [line.quantity]),
+            True,
+        )
+
+        self.assertEqual(
+            self.handler.are_stock_allocations_available(
+                [line], [105]),
+            False,
+        )
+
+    def test_are_stock_allocations_available_track_stock_off(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=False)
+        product = factories.ProductFactory(product_class=product_class)
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        line = order.lines.get()
+        self.assertEqual(
+            self.handler.are_stock_allocations_available(
+                [line], [105]),
+            True,
+        )
+
+    def test_consume_stock_allocations_track_stock_on(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=True)
+        product = factories.ProductFactory(product_class=product_class)
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        stockrecord = product.stockrecords.get()
+        num_in_stock = stockrecord.num_in_stock
+        num_allocated = stockrecord.num_allocated
+
+        lines = order.lines.all()
+        self.handler.consume_stock_allocations(
+            order, lines, [line.quantity for line in lines])
+
+        stockrecord.refresh_from_db()
+        self.assertEqual(
+            stockrecord.num_allocated,
+            num_allocated - 5,
+            "Allocated stock should have decreased, but didn't."
+        )
+        self.assertEqual(
+            stockrecord.num_in_stock,
+            num_in_stock - 5,
+            "Stock should have decreased, but didn't."
+        )
+
+    def test_consume_stock_allocations_track_stock_off(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=False)
+        product = factories.ProductFactory(product_class=product_class)
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        stockrecord = product.stockrecords.get()
+        num_in_stock = stockrecord.num_in_stock
+        num_allocated = stockrecord.num_allocated
+
+        lines = order.lines.all()
+        self.handler.consume_stock_allocations(
+            order, lines, [line.quantity for line in lines])
+
+        stockrecord.refresh_from_db()
+        self.assertEqual(
+            stockrecord.num_allocated,
+            num_allocated,
+            "Allocated stock shouldn't have changed, but it did."
+        )
+        self.assertEqual(
+            stockrecord.num_in_stock,
+            num_in_stock,
+            "Stock shouldn't have changed, but it did."
+        )
+
+    def test_consume_stock_allocations_without_line_arguments(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=True)
+        product = factories.ProductFactory(product_class=product_class)
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        stockrecord = product.stockrecords.get()
+        num_in_stock = stockrecord.num_in_stock
+        num_allocated = stockrecord.num_allocated
+
+        self.handler.consume_stock_allocations(order)
+
+        stockrecord.refresh_from_db()
+        self.assertEqual(
+            stockrecord.num_allocated,
+            num_allocated - 5,
+            "Allocated stock should have decreased, but didn't."
+        )
+        self.assertEqual(
+            stockrecord.num_in_stock,
+            num_in_stock - 5,
+            "Stock should have decreased, but didn't."
+        )
+
+    def test_cancel_stock_allocations_track_stock_on(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=True)
+        product = factories.ProductFactory(product_class=product_class)
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        stockrecord = product.stockrecords.get()
+        num_allocated = stockrecord.num_allocated
+
+        lines = order.lines.all()
+        self.handler.cancel_stock_allocations(
+            order, lines, [line.quantity for line in lines])
+
+        stockrecord.refresh_from_db()
+        self.assertEqual(
+            stockrecord.num_allocated,
+            num_allocated - 5,
+            "Allocated stock should have decreased, but didn't."
+        )
+
+    def test_cancel_stock_allocations_track_stock_off(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=False)
+        product = factories.ProductFactory(product_class=product_class)
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        stockrecord = product.stockrecords.get()
+        num_allocated = stockrecord.num_allocated
+
+        lines = order.lines.all()
+        self.handler.cancel_stock_allocations(
+            order, lines, [line.quantity for line in lines])
+
+        stockrecord.refresh_from_db()
+        self.assertEqual(
+            stockrecord.num_allocated,
+            num_allocated,
+            "Allocated stock shouldn't have changed, but it did."
+        )
+
+    def test_cancel_stock_allocations_without_line_arguments(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=True)
+        product = factories.ProductFactory(product_class=product_class)
+        basket = factories.create_basket(empty=True)
+        add_product(basket, D('10.00'), 5, product=product)
+        order = factories.create_order(basket=basket)
+
+        stockrecord = product.stockrecords.get()
+        num_allocated = stockrecord.num_allocated
+
+        self.handler.cancel_stock_allocations(order)
+
+        stockrecord.refresh_from_db()
+        self.assertEqual(
+            stockrecord.num_allocated,
+            num_allocated - 5,
+            "Allocated stock should have decreased, but didn't."
+        )
+
 
 class TestTotalCalculation(TestCase):
 

tests/integration/partner/test_models.py

@@ -55,6 +55,20 @@ class TestStockRecord(TestCase):
         self.assertEqual(10, self.stockrecord.num_in_stock)
 
 
+class TestStockRecordNoStockTrack(TestCase):
+
+    def setUp(self):
+        product_class = factories.ProductClassFactory(
+            requires_shipping=False, track_stock=False)
+        self.product = factories.ProductFactory(product_class=product_class)
+        self.stockrecord = factories.create_stockrecord(
+            self.product, price_excl_tax=D('10.00'), num_in_stock=10)
+
+    def test_allocate_does_nothing(self):
+        self.stockrecord.allocate(5)
+        self.assertEqual(self.stockrecord.num_allocated, None)
+
+
 class TestPartnerAddress(TestCase):
 
     def setUp(self):