Merge pull request #6151 from edx/adam/add-defunct-states-for-cart

Adam/add defunct states for cart

lms/djangoapps/shoppingcart/exceptions.py

@@ -55,3 +55,11 @@ class ReportException(Exception):
 
 class ReportTypeDoesNotExistException(ReportException):
     pass
+
+
+class InvalidStatusToRetire(Exception):
+    pass
+
+
+class UnexpectedOrderItemStatus(Exception):
+    pass

lms/djangoapps/shoppingcart/management/commands/retire_order.py

@@ -0,0 +1,44 @@
+"""
+Script for retiring order that went through cybersource but weren't
+marked as "purchased" in the db
+"""
+
+from django.core.management.base import BaseCommand, CommandError
+from shoppingcart.models import Order
+from shoppingcart.exceptions import UnexpectedOrderItemStatus, InvalidStatusToRetire
+
+
+class Command(BaseCommand):
+    """
+    Retire orders that went through cybersource but weren't updated
+    appropriately in the db
+    """
+    help = """
+    Retire orders that went through cybersource but weren't updated appropriately in the db.
+    Takes a file of orders to be retired, one order per line
+    """
+
+    def handle(self, *args, **options):
+        "Execute the command"
+        if len(args) != 1:
+            raise CommandError("retire_order requires one argument: <orders file>")
+
+        with open(args[0]) as orders_file:
+            order_ids = [int(line.strip()) for line in orders_file.readlines()]
+
+        orders = Order.objects.filter(id__in=order_ids)
+
+        for order in orders:
+            old_status = order.status
+            try:
+                order.retire()
+            except (UnexpectedOrderItemStatus, InvalidStatusToRetire) as err:
+                print "Did not retire order {order}: {message}".format(
+                    order=order.id, message=err.message
+                )
+            else:
+                print "retired order {order_id} from status {old_status} to status {new_status}".format(
+                    order_id=order.id,
+                    old_status=old_status,
+                    new_status=order.status,
+                )

lms/djangoapps/shoppingcart/management/tests/test_retire_order.py

@@ -0,0 +1,76 @@
+"""Tests for the retire_order command"""
+
+from tempfile import NamedTemporaryFile
+from django.core.management import call_command
+
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+from shoppingcart.models import Order, CertificateItem
+from student.tests.factories import UserFactory
+
+
+class TestRetireOrder(ModuleStoreTestCase):
+    """Test the retire_order command"""
+    def setUp(self):
+        course = CourseFactory.create()
+        self.course_key = course.id
+
+        # set up test carts
+        self.cart, __ = self._create_cart()
+
+        self.paying, __ = self._create_cart()
+        self.paying.start_purchase()
+
+        self.already_defunct_cart, __ = self._create_cart()
+        self.already_defunct_cart.retire()
+
+        self.purchased, self.purchased_item = self._create_cart()
+        self.purchased.status = "purchased"
+        self.purchased.save()
+        self.purchased_item.status = "purchased"
+        self.purchased.save()
+
+    def test_retire_order(self):
+        """Test the retire_order command"""
+        nonexistent_id = max(order.id for order in Order.objects.all()) + 1
+        order_ids = [
+            self.cart.id,
+            self.paying.id,
+            self.already_defunct_cart.id,
+            self.purchased.id,
+            nonexistent_id
+        ]
+
+        self._create_tempfile_and_call_command(order_ids)
+
+        self.assertEqual(
+            Order.objects.get(id=self.cart.id).status, "defunct-cart"
+        )
+        self.assertEqual(
+            Order.objects.get(id=self.paying.id).status, "defunct-paying"
+        )
+        self.assertEqual(
+            Order.objects.get(id=self.already_defunct_cart.id).status,
+            "defunct-cart"
+        )
+        self.assertEqual(
+            Order.objects.get(id=self.purchased.id).status, "purchased"
+        )
+
+    def _create_tempfile_and_call_command(self, order_ids):
+        """
+        Takes a list of order_ids, writes them to a tempfile, and then runs the
+        "retire_order" command on the tempfile
+        """
+        with NamedTemporaryFile() as temp:
+            temp.write("\n".join(str(order_id) for order_id in order_ids))
+            temp.seek(0)
+            call_command('retire_order', temp.name)
+
+    def _create_cart(self):
+        """Creates a cart and adds a CertificateItem to it"""
+        cart = Order.get_cart_for_user(UserFactory.create())
+        item = CertificateItem.add_to_order(
+            cart, self.course_key, 10, 'honor', currency='usd'
+        )
+        return cart, item

lms/djangoapps/shoppingcart/models.py

@@ -38,10 +38,17 @@
 from verify_student.models import SoftwareSecurePhotoVerification
 
 from .exceptions import (
-    InvalidCartItem, PurchasedCallbackException, ItemAlreadyInCartException,
-    AlreadyEnrolledInCourseException, CourseDoesNotExistException,
-    MultipleCouponsNotAllowedException, RegCodeAlreadyExistException,
-    ItemDoesNotExistAgainstRegCodeException, ItemNotAllowedToRedeemRegCodeException
+    InvalidCartItem,
+    PurchasedCallbackException,
+    ItemAlreadyInCartException,
+    AlreadyEnrolledInCourseException,
+    CourseDoesNotExistException,
+    MultipleCouponsNotAllowedException,
+    RegCodeAlreadyExistException,
+    ItemDoesNotExistAgainstRegCodeException,
+    ItemNotAllowedToRedeemRegCodeException,
+    InvalidStatusToRetire,
+    UnexpectedOrderItemStatus,
 )
 
 from microsite_configuration import microsite
@@ -62,8 +69,22 @@
 
     # The user's order has been refunded.
     ('refunded', 'refunded'),
+
+    # The user's order went through, but the order was erroneously left
+    # in 'cart'.
+    ('defunct-cart', 'defunct-cart'),
+
+    # The user's order went through, but the order was erroneously left
+    # in 'paying'.
+    ('defunct-paying', 'defunct-paying'),
 )
 
+# maps order statuses to their defunct states
+ORDER_STATUS_MAP = {
+    'cart': 'defunct-cart',
+    'paying': 'defunct-paying',
+}
+
 # we need a tuple to represent the primary key of various OrderItem subclasses
 OrderItemSubclassPK = namedtuple('OrderItemSubclassPK', ['cls', 'pk'])  # pylint: disable=invalid-name
 
@@ -484,6 +505,39 @@ def generate_receipt_instructions(self):
             instruction_set.update(set_of_html)
         return instruction_dict, instruction_set
 
+    def retire(self):
+        """
+        Method to "retire" orders that have gone through to the payment service
+        but have (erroneously) not had their statuses updated.
+        This method only works on orders that satisfy the following conditions:
+        1) the order status is either "cart" or "paying" (otherwise we raise
+           an InvalidStatusToRetire error)
+        2) the order's order item's statuses match the order's status (otherwise
+           we throw an UnexpectedOrderItemStatus error)
+        """
+        # if an order is already retired, no-op:
+        if self.status in ORDER_STATUS_MAP.values():
+            return
+
+        if self.status not in ORDER_STATUS_MAP.keys():
+            raise InvalidStatusToRetire(
+                "order status {order_status} is not 'paying' or 'cart'".format(
+                    order_status=self.status
+                )
+            )
+
+        for item in self.orderitem_set.all():  # pylint: disable=no-member
+            if item.status != self.status:
+                raise UnexpectedOrderItemStatus(
+                    "order_item status is different from order status"
+                )
+
+        self.status = ORDER_STATUS_MAP[self.status]
+        self.save()
+
+        for item in self.orderitem_set.all():  # pylint: disable=no-member
+            item.retire()
+
 
 class OrderItem(TimeStampedModel):
     """
@@ -616,6 +670,15 @@ def analytics_data(self):
             'category': 'N/A',
         }
 
+    def retire(self):
+        """
+        Called by the `retire` method defined in the `Order` class. Retires
+        an order item if its (and its order's) status was erroneously not
+        updated to "purchased" after the order was processed.
+        """
+        self.status = ORDER_STATUS_MAP[self.status]
+        self.save()
+
 
 class Invoice(models.Model):
     """

lms/djangoapps/shoppingcart/tests/test_models.py

@@ -9,6 +9,7 @@
 
 from mock import patch, MagicMock
 import pytz
+import ddt
 from django.core import mail
 from django.conf import settings
 from django.db import DatabaseError
@@ -28,8 +29,14 @@
 from student.tests.factories import UserFactory
 from student.models import CourseEnrollment
 from course_modes.models import CourseMode
-from shoppingcart.exceptions import (PurchasedCallbackException, CourseDoesNotExistException,
-                                     ItemAlreadyInCartException, AlreadyEnrolledInCourseException)
+from shoppingcart.exceptions import (
+    PurchasedCallbackException,
+    CourseDoesNotExistException,
+    ItemAlreadyInCartException,
+    AlreadyEnrolledInCourseException,
+    InvalidStatusToRetire,
+    UnexpectedOrderItemStatus,
+)
 
 from opaque_keys.edx.locator import CourseLocator
 
@@ -39,6 +46,7 @@
 
 
 @override_settings(MODULESTORE=MODULESTORE_CONFIG)
+@ddt.ddt
 class OrderTest(ModuleStoreTestCase):
     def setUp(self):
         self.user = UserFactory.create()
@@ -153,6 +161,62 @@ def test_start_purchase(self):
         for item in cart.orderitem_set.all():
             self.assertEqual(item.status, 'purchased')
 
+    def test_retire_order_cart(self):
+        """Test that an order in cart can successfully be retired"""
+        cart = Order.get_cart_for_user(user=self.user)
+        CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd')
+
+        cart.retire()
+        self.assertEqual(cart.status, 'defunct-cart')
+        self.assertEqual(cart.orderitem_set.get().status, 'defunct-cart')
+
+    def test_retire_order_paying(self):
+        """Test that an order in "paying" can successfully be retired"""
+        cart = Order.get_cart_for_user(user=self.user)
+        CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd')
+        cart.start_purchase()
+
+        cart.retire()
+        self.assertEqual(cart.status, 'defunct-paying')
+        self.assertEqual(cart.orderitem_set.get().status, 'defunct-paying')
+
+    @ddt.data(
+        ("cart", "paying", UnexpectedOrderItemStatus),
+        ("purchased", "purchased", InvalidStatusToRetire),
+    )
+    @ddt.unpack
+    def test_retire_order_error(self, order_status, item_status, exception):
+        """
+        Test error cases for retiring an order:
+        1) Order item has a different status than the order
+        2) The order's status isn't in "cart" or "paying"
+        """
+        cart = Order.get_cart_for_user(user=self.user)
+        item = CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd')
+
+        cart.status = order_status
+        cart.save()
+        item.status = item_status
+        item.save()
+
+        with self.assertRaises(exception):
+            cart.retire()
+
+    @ddt.data('defunct-paying', 'defunct-cart')
+    def test_retire_order_already_retired(self, status):
+        """
+        Check that orders that have already been retired noop when the method
+        is called on them again.
+        """
+        cart = Order.get_cart_for_user(user=self.user)
+        item = CertificateItem.add_to_order(cart, self.course_key, self.cost, 'honor', currency='usd')
+        cart.status = item.status = status
+        cart.save()
+        item.save()
+        cart.retire()
+        self.assertEqual(cart.status, status)
+        self.assertEqual(item.status, status)
+
     @override_settings(
         SEGMENT_IO_LMS_KEY="foobar",
         FEATURES={
@@ -291,20 +355,20 @@ def test_billing_info_storage_off(self, render):
         ((_, context), _) = render.call_args
         self.assertFalse(context['has_billing_info'])
 
-    mock_gen_inst = MagicMock(return_value=(OrderItemSubclassPK(OrderItem, 1), set([])))
-
     def test_generate_receipt_instructions_callchain(self):
         """
         This tests the generate_receipt_instructions call chain (ie calling the function on the
         cart also calls it on items in the cart
         """
+        mock_gen_inst = MagicMock(return_value=(OrderItemSubclassPK(OrderItem, 1), set([])))
+
         cart = Order.get_cart_for_user(self.user)
         item = OrderItem(user=self.user, order=cart)
         item.save()
         self.assertTrue(cart.has_items())
-        with patch.object(OrderItem, 'generate_receipt_instructions', self.mock_gen_inst):
+        with patch.object(OrderItem, 'generate_receipt_instructions', mock_gen_inst):
             cart.generate_receipt_instructions()
-            self.mock_gen_inst.assert_called_with()
+            mock_gen_inst.assert_called_with()
 
 
 class OrderItemTest(TestCase):