Merge pull request #14935 from edx/uzairr/ECOM-7252-refund

ECOM-7252-fix refund discrepancy after the course mode expiry

common/djangoapps/course_modes/models.py

@@ -340,7 +340,7 @@ class CourseMode(models.Model):
         return {mode.slug: mode for mode in modes}
 
     @classmethod
-    def mode_for_course(cls, course_id, mode_slug, modes=None):
+    def mode_for_course(cls, course_id, mode_slug, modes=None, include_expired=False):
         """Returns the mode for the course corresponding to mode_slug.
 
         Returns only non-expired modes.
@@ -356,12 +356,15 @@ class CourseMode(models.Model):
                 of course modes.  This can be used to avoid an additional
                 database query if you have already loaded the modes list.
 
+            include_expired (bool): If True, expired course modes will be included
+                in the returned values. If False, these modes will be omitted.
+
         Returns:
             Mode
 
         """
         if modes is None:
-            modes = cls.modes_for_course(course_id)
+            modes = cls.modes_for_course(course_id, include_expired=include_expired)
 
         matched = [m for m in modes if m.slug == mode_slug]
         if matched:

common/djangoapps/student/models.py

@@ -1557,6 +1557,7 @@ class CourseEnrollment(models.Model):
         # which calls this method to determine whether to refund the order.
         # This can't be set directly because refunds currently happen as a side-effect of unenrolling.
         # (side-effects are bad)
+
         if getattr(self, 'can_refund', None) is not None:
             return True
 
@@ -1570,10 +1571,13 @@ class CourseEnrollment(models.Model):
 
         # If it is after the refundable cutoff date they should not be refunded.
         refund_cutoff_date = self.refund_cutoff_date()
-        if refund_cutoff_date and datetime.now(UTC) > refund_cutoff_date:
+        # `refund_cuttoff_date` will be `None` if there is no order. If there is no order return `False`.
+        if refund_cutoff_date is None:
+            return False
+        if datetime.now(UTC) > refund_cutoff_date:
             return False
 
-        course_mode = CourseMode.mode_for_course(self.course_id, 'verified')
+        course_mode = CourseMode.mode_for_course(self.course_id, 'verified', include_expired=True)
         if course_mode is None:
             return False
         else:

common/djangoapps/student/tests/test_refunds.py

@@ -55,23 +55,24 @@ class RefundableTest(SharedModuleStoreTestCase):
             mode_display_name='Verified',
             expiration_datetime=datetime.now(pytz.UTC) + timedelta(days=1)
         )
+
         self.enrollment = CourseEnrollment.enroll(self.user, self.course.id, mode='verified')
 
         self.client = Client()
         cache.clear()
 
-    def test_refundable(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_refundable(self, cutoff_date):
         """ Assert base case is refundable"""
+        cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(days=1)
         self.assertTrue(self.enrollment.refundable())
 
-    def test_refundable_expired_verification(self):
-        """ Assert that enrollment is not refundable if course mode has expired."""
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_refundable_expired_verification(self, cutoff_date):
+        """ Assert that enrollment is refundable if course mode has expired."""
+        cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(days=1)
         self.verified_mode.expiration_datetime = datetime.now(pytz.UTC) - timedelta(days=1)
         self.verified_mode.save()
-        self.assertFalse(self.enrollment.refundable())
-
-        # Assert that can_refund overrides this and allows refund
-        self.enrollment.can_refund = True
         self.assertTrue(self.enrollment.refundable())
 
     def test_refundable_of_purchased_course(self):
@@ -94,8 +95,12 @@ class RefundableTest(SharedModuleStoreTestCase):
         resp = self.client.post(reverse('student.views.dashboard', args=[]))
         self.assertIn('You will not be refunded the amount you paid.', resp.content)
 
-    def test_refundable_when_certificate_exists(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_refundable_when_certificate_exists(self, cutoff_date):
         """ Assert that enrollment is not refundable once a certificat has been generated."""
+
+        cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(days=1)
+
         self.assertTrue(self.enrollment.refundable())
 
         GeneratedCertificateFactory.create(
@@ -121,16 +126,17 @@ class RefundableTest(SharedModuleStoreTestCase):
             )
         )
 
-    def test_refundable_with_cutoff_date(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_refundable_with_cutoff_date(self, cutoff_date):
         """ Assert enrollment is refundable before cutoff and not refundable after."""
+        cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(days=1)
         self.assertTrue(self.enrollment.refundable())
 
-        with patch('student.models.CourseEnrollment.refund_cutoff_date') as cutoff_date:
-            cutoff_date.return_value = datetime.now(pytz.UTC) - timedelta(minutes=5)
-            self.assertFalse(self.enrollment.refundable())
+        cutoff_date.return_value = datetime.now(pytz.UTC) - timedelta(minutes=5)
+        self.assertFalse(self.enrollment.refundable())
 
-            cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(minutes=5)
-            self.assertTrue(self.enrollment.refundable())
+        cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(minutes=5)
+        self.assertTrue(self.enrollment.refundable())
 
     @ddt.data(
         (timedelta(days=1), timedelta(days=2), timedelta(days=2), 14),

lms/djangoapps/shoppingcart/tests/test_models.py

@@ -911,9 +911,10 @@ class CertificateItemTest(ModuleStoreTestCase):
             'STORE_BILLING_INFO': True,
         }
     )
-    def test_refund_cert_callback_no_expiration(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_refund_cert_callback_no_expiration(self, cutoff_date):
         # When there is no expiration date on a verified mode, the user can always get a refund
-
+        cutoff_date.return_value = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1)
         # need to prevent analytics errors from appearing in stderr
         with patch('sys.stderr', sys.stdout.write):
             CourseEnrollment.enroll(self.user, self.course_key, 'verified')
@@ -952,7 +953,8 @@ class CertificateItemTest(ModuleStoreTestCase):
             'STORE_BILLING_INFO': True,
         }
     )
-    def test_refund_cert_callback_before_expiration(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_refund_cert_callback_before_expiration(self, cutoff_date):
         # If the expiration date has not yet passed on a verified mode, the user can be refunded
         many_days = datetime.timedelta(days=60)
 
@@ -965,6 +967,7 @@ class CertificateItemTest(ModuleStoreTestCase):
                                  expiration_datetime=(datetime.datetime.now(pytz.utc) + many_days))
         course_mode.save()
 
+        cutoff_date.return_value = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1)
         # need to prevent analytics errors from appearing in stderr
         with patch('sys.stderr', sys.stdout.write):
             CourseEnrollment.enroll(self.user, self.course_key, 'verified')
@@ -979,7 +982,8 @@ class CertificateItemTest(ModuleStoreTestCase):
         self.assertEquals(target_certs[0].order.status, 'refunded')
         self._assert_refund_tracked()
 
-    def test_refund_cert_callback_before_expiration_email(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_refund_cert_callback_before_expiration_email(self, cutoff_date):
         """ Test that refund emails are being sent correctly. """
         course = CourseFactory.create()
         course_key = course.id
@@ -998,6 +1002,7 @@ class CertificateItemTest(ModuleStoreTestCase):
         cart.purchase()
 
         mail.outbox = []
+        cutoff_date.return_value = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1)
         with patch('shoppingcart.models.log.error') as mock_error_logger:
             CourseEnrollment.unenroll(self.user, course_key)
             self.assertFalse(mock_error_logger.called)
@@ -1006,8 +1011,9 @@ class CertificateItemTest(ModuleStoreTestCase):
             self.assertEquals(settings.PAYMENT_SUPPORT_EMAIL, mail.outbox[0].from_email)
             self.assertIn('has requested a refund on Order', mail.outbox[0].body)
 
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
     @patch('shoppingcart.models.log.error')
-    def test_refund_cert_callback_before_expiration_email_error(self, error_logger):
+    def test_refund_cert_callback_before_expiration_email_error(self, error_logger, cutoff_date):
         # If there's an error sending an email to billing, we need to log this error
         many_days = datetime.timedelta(days=60)
 
@@ -1026,6 +1032,7 @@ class CertificateItemTest(ModuleStoreTestCase):
         CertificateItem.add_to_order(cart, course_key, self.cost, 'verified')
         cart.purchase()
 
+        cutoff_date.return_value = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1)
         with patch('shoppingcart.models.send_mail', side_effect=smtplib.SMTPException):
             CourseEnrollment.unenroll(self.user, course_key)
             self.assertTrue(error_logger.call_args[0][0].startswith('Failed sending email'))

lms/djangoapps/shoppingcart/tests/test_reports.py

@@ -7,6 +7,7 @@ import datetime
 import pytz
 import StringIO
 from textwrap import dedent
+from mock import patch
 
 from django.conf import settings
 
@@ -26,9 +27,10 @@ class ReportTypeTests(ModuleStoreTestCase):
     """
     FIVE_MINS = datetime.timedelta(minutes=5)
 
-    def setUp(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def setUp(self, cutoff_date):
         super(ReportTypeTests, self).setUp()
-
+        cutoff_date.return_value = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1)
         # Need to make a *lot* of users for this one
         self.first_verified_user = UserFactory.create(profile__name="John Doe")
         self.second_verified_user = UserFactory.create(profile__name="Jane Deer")

lms/djangoapps/support/tests/test_refund.py

@@ -8,6 +8,8 @@ to the E-Commerce service is complete.
 
 """
 import datetime
+import pytz
+from mock import patch
 
 from django.test.client import Client
 
@@ -91,10 +93,12 @@ class RefundTests(ModuleStoreTestCase):
         response = self.client.post('/support/refund/', {'course_id': str(self.course_id), 'user': 'unknown@foo.com'})
         self.assertContains(response, 'User not found')
 
-    def test_not_refundable(self):
+    @patch('student.models.CourseEnrollment.refund_cutoff_date')
+    def test_not_refundable(self, cutoff_date):
         self._enroll()
         self.course_mode.expiration_datetime = datetime.datetime(2033, 4, 6)
         self.course_mode.save()
+        cutoff_date.return_value = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1)
         response = self.client.post('/support/refund/', self.form_pars)
         self.assertContains(response, 'not past the refund window')