Update basket deletion management command

Avoid deletion of invoiced baskets, and sleep between each batch deletion. Also do a better job explaining why we used batched deletes. In service of ECOM-5386.

ecommerce/extensions/basket/management/commands/delete_ordered_baskets.py

@@ -4,6 +4,9 @@ Management command that deletes baskets associated with orders.
 These baskets don't have much value once the order is placed, and unnecessarily take up space.
 """
 from __future__ import unicode_literals
+
+import time
+
 from django.core.management import BaseCommand
 from django.db import transaction
 from oscar.core.loading import get_model
@@ -15,12 +18,20 @@ class Command(BaseCommand):
     help = 'Delete baskets for which orders have been placed.'
 
     def add_arguments(self, parser):
+        # Batched deletion prevents the entire table from locking up as the command executes.
         parser.add_argument('-b', '--batch-size',
                             action='store',
                             dest='batch_size',
                             default=1000,
                             type=int,
                             help='Size of each batch of baskets to be deleted.')
+        # Sleeping between each batch deletion gives MySQL time to process other connections.
+        parser.add_argument('-s', '--sleep-seconds',
+                            action='store',
+                            dest='sleep_seconds',
+                            default=3,
+                            type=int,
+                            help='Seconds to sleep between each batch deletion.')
         parser.add_argument('--commit',
                             action='store_true',
                             dest='commit',
@@ -28,21 +39,29 @@ class Command(BaseCommand):
                             help='Actually delete the baskets.')
 
     def handle(self, *args, **options):
-        queryset = Basket.objects.filter(order__isnull=False)
+        # Only select those baskets linked to an order, and those not linked to an invoice.
+        # TODO: Simplify this query when the foreign key to Basket is removed from Invoice.
+        queryset = Basket.objects.filter(order__isnull=False, invoice__isnull=True)
         count = queryset.count()
 
         if options['commit']:
             if count:
-                self.stderr.write('Deleting [{}] baskets...'.format(count))
+                self.stderr.write('Deleting [{}] baskets.'.format(count))
+
                 batch_size = options['batch_size']
+                sleep_seconds = options['sleep_seconds']
+
                 max_id = queryset.order_by('-id')[0].id
                 for start in range(0, max_id, batch_size):
                     end = min(start + batch_size, max_id)
-                    self.stderr.write('...deleting baskets [{start}] through [{end}]...'.format(start=start, end=end))
+                    self.stderr.write('Deleting baskets [{start}] through [{end}].'.format(start=start, end=end))
                     with transaction.atomic():
                         queryset.filter(pk__gte=start, pk__lte=end).delete()
 
-                self.stderr.write('Done.')
+                    self.stderr.write('Complete. Sleeping.'.format(start=start, end=end))
+                    time.sleep(sleep_seconds)
+
+                self.stderr.write('All baskets deleted.')
             else:
                 self.stderr.write('No baskets to delete.')
         else:

ecommerce/extensions/basket/tests/test_commands.py

@@ -6,6 +6,7 @@ from django.core.management import call_command, CommandError
 from oscar.core.loading import get_model
 from oscar.test import factories
 
+from ecommerce.invoice.models import Invoice
 from ecommerce.tests.testcases import TestCase
 
 Basket = get_model('basket', 'Basket')
@@ -21,6 +22,12 @@ class DeleteOrderedBasketsCommandTests(TestCase):
         self.orders = [factories.create_order() for __ in range(0, 2)]
         self.unordered_baskets = [factories.BasketFactory() for __ in range(0, 3)]
 
+        # Create invoiced baskets.
+        self.invoiced_orders = [factories.create_order() for __ in range(0, 2)]
+        self.invoiced_baskets = [order.basket for order in self.invoiced_orders]
+        for order in self.invoiced_orders:
+            Invoice.objects.create(basket=order.basket, order=order)
+
     def test_without_commit(self):
         """ Verify the command does not delete baskets if the commit flag is not set. """
         expected = Basket.objects.count()
@@ -40,19 +47,22 @@ class DeleteOrderedBasketsCommandTests(TestCase):
     def test_with_commit(self):
         """ Verify the command, when called with the commit flag, deletes baskets with orders. """
         # Verify we have baskets with orders
-        self.assertEqual(Basket.objects.filter(order__isnull=False).count(), len(self.orders))
+        self.assertEqual(Basket.objects.filter(order__isnull=False).count(), len(self.orders + self.invoiced_orders))
+
+        # Verify we have invoiced baskets
+        self.assertEqual(Basket.objects.filter(invoice__isnull=False).count(), len(self.invoiced_baskets))
 
         # Call the command with the commit flag
         out = StringIO()
         call_command(self.command, commit=True, stderr=out)
 
-        # Verify baskets with orders deleted
-        self.assertEqual(list(Basket.objects.all()), self.unordered_baskets)
+        # Verify baskets with orders deleted, except for those which are invoiced
+        self.assertEqual(list(Basket.objects.all()), self.unordered_baskets + self.invoiced_baskets)
 
         # Verify info was output to stderr
         actual = out.getvalue().strip()
-        self.assertTrue(actual.startswith('Deleting [{}] baskets...'.format(len(self.orders))))
-        self.assertTrue(actual.endswith('Done.'))
+        self.assertTrue(actual.startswith('Deleting [{}] baskets.'.format(len(self.orders))))
+        self.assertTrue(actual.endswith('All baskets deleted.'))
 
     def test_commit_without_baskets(self):
         """ Verify the command does nothing if there are no baskets to delete. """