diff --git a/lms/djangoapps/shoppingcart/exceptions.py b/lms/djangoapps/shoppingcart/exceptions.py index a5fa8492db..714cc0323c 100644 --- a/lms/djangoapps/shoppingcart/exceptions.py +++ b/lms/djangoapps/shoppingcart/exceptions.py @@ -55,3 +55,11 @@ class ReportException(Exception): class ReportTypeDoesNotExistException(ReportException): pass + + +class InvalidStatusToRetire(Exception): + pass + + +class UnexpectedOrderItemStatus(Exception): + pass diff --git a/lms/djangoapps/shoppingcart/management/__init__.py b/lms/djangoapps/shoppingcart/management/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/shoppingcart/management/commands/__init__.py b/lms/djangoapps/shoppingcart/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/shoppingcart/management/commands/retire_order.py b/lms/djangoapps/shoppingcart/management/commands/retire_order.py new file mode 100644 index 0000000000..40932e0591 --- /dev/null +++ b/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: ") + + 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, + ) diff --git a/lms/djangoapps/shoppingcart/management/tests/__init__.py b/lms/djangoapps/shoppingcart/management/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/shoppingcart/management/tests/test_retire_order.py b/lms/djangoapps/shoppingcart/management/tests/test_retire_order.py new file mode 100644 index 0000000000..40ccdd53fb --- /dev/null +++ b/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 diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 79f5b5931e..7c5851731e 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -38,10 +38,17 @@ from xmodule_django.models import CourseKeyField 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 @@ ORDER_STATUSES = ( # 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 @@ class Order(models.Model): 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 @@ class OrderItem(TimeStampedModel): '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): """ diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index 0fa9f6cf72..83d412c4fc 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -9,6 +9,7 @@ from boto.exception import BotoServerError # this is a super-class of SESError 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 shoppingcart.models import ( 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 @@ MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, incl @override_settings(MODULESTORE=MODULESTORE_CONFIG) +@ddt.ddt class OrderTest(ModuleStoreTestCase): def setUp(self): self.user = UserFactory.create() @@ -153,6 +161,62 @@ class OrderTest(ModuleStoreTestCase): 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 @@ class OrderTest(ModuleStoreTestCase): ((_, 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):