More response to Dave's PR

common/djangoapps/student/models.py

@@ -822,7 +822,6 @@ class CourseEnrollment(models.Model):
 
         `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
         """
-        refund_error = "Refund Error"
         try:
             record = CourseEnrollment.objects.get(user=user, course_id=course_id)
             record.is_active = False

lms/djangoapps/shoppingcart/models.py

@@ -401,31 +401,27 @@ class CertificateItem(OrderItem):
     mode = models.SlugField()
 
     @receiver(unenroll_done, sender=CourseEnrollment)
-    def refund_cert_callback(sender, **kwargs):
+    def refund_cert_callback(sender, course_enrollment=None, **kwargs):
         """
         When a CourseEnrollment object calls its unenroll method, this function checks to see if that unenrollment
         occurred in a verified certificate that was within the refund deadline.  If so, it actually performs the
         refund.
 
-        Returns the refunded certificate on a successful refund.  If no refund is necessary, it returns nothing.
-        If an error that the user should see occurs, it returns a string specifying the error.
+        Returns the refunded certificate on a successful refund; else, it returns nothing.
         """
-        mode = kwargs['course_enrollment'].mode
-        course_id = kwargs['course_enrollment'].course_id
-        user = kwargs['course_enrollment'].user
 
         # Only refund verified cert unenrollments that are within bounds of the expiration date
-        if (mode != 'verified'):
+        if course_enrollment.mode != 'verified':
             return
 
-        if (CourseMode.mode_for_course(course_id, 'verified') is None):
+        if CourseMode.mode_for_course(course_enrollment.course_id, 'verified') is None:
             return
 
-        target_certs = CertificateItem.objects.filter(course_id=course_id, user_id=user, status='purchased', mode='verified')
+        target_certs = CertificateItem.objects.filter(course_id=course_enrollment.course_id, user_id=course_enrollment.user, status='purchased', mode='verified')
         try:
             target_cert = target_certs[0]
         except IndexError:
-            log.error("Matching CertificateItem not found while trying to refund.  User %s, Course %s", user, course_id)
+            log.error("Matching CertificateItem not found while trying to refund.  User %s, Course %s", course_enrollment.user, course_enrollment.course_id)
             return
         target_cert.status = 'refunded'
         target_cert.save()
@@ -433,13 +429,20 @@ class CertificateItem(OrderItem):
         order_number = target_cert.order_id
         # send billing an email so they can handle refunding
         subject = _("[Refund] User-Requested Refund")
-        message = "User {user} ({user_email}) has requested a refund on Order #{order_number}.".format(user=user, user_email=user.email, order_number=order_number)
+        message = "User {user} ({user_email}) has requested a refund on Order #{order_number}.".format(user=course_enrollment.user,
+                                                                                                       user_email=course_enrollment.user.email,
+                                                                                                       order_number=order_number)
         to_email = [settings.PAYMENT_SUPPORT_EMAIL]
         from_email = [settings.PAYMENT_SUPPORT_EMAIL]
         try:
             send_mail(subject, message, from_email, to_email, fail_silently=False)
         except (smtplib.SMTPException, BotoServerError):
-            log.error('Failed sending email to billing request a refund for verified certiciate (User %s, Course %s)', user, course_id)
+            err_str = 'Failed sending email to billing request a refund for verified certiciate (User {user}, Course {course}, CourseEnrollmentID {ce_id}, Order #{order})'
+            log.error(err_str.format(
+                user=course_enrollment.user,
+                course=course_enrollment.course_id,
+                ce_id=course_enrollment.id,
+                order=order_number))
 
         return target_cert
 

lms/djangoapps/shoppingcart/tests/test_models.py

@@ -364,18 +364,18 @@ class CertificateItemTest(ModuleStoreTestCase):
                           'shoppingcart/receipt.html')
 
     def test_refund_cert_callback_no_expiration(self):
-        # enroll and buy; dup from test_existing_enrollment
+        # When there is no expiration date on a verified mode, the user can always get a refund
         CourseEnrollment.enroll(self.user, self.course_id, 'verified')
         cart = Order.get_cart_for_user(user=self.user)
         CertificateItem.add_to_order(cart, self.course_id, self.cost, 'verified')
         cart.purchase()
-        # now that it's there, let's try refunding it
+
         CourseEnrollment.unenroll(self.user, self.course_id)
         target_certs = CertificateItem.objects.filter(course_id=self.course_id, user_id=self.user, status='refunded', mode='verified')
         self.assertTrue(target_certs[0])
 
     def test_refund_cert_callback_before_expiration(self):
-        # enroll and buy; dup from test_existing_enrollment
+        # If the expiration date has not yet passed on a verified mode, the user can be refunded
         course_id = "refund_before_expiration/test/one"
         many_days = datetime.timedelta(days=60)
 
@@ -392,13 +392,13 @@ class CertificateItemTest(ModuleStoreTestCase):
         CertificateItem.add_to_order(cart, course_id, self.cost, 'verified')
         cart.purchase()
 
-        # now that it's there, let's try refunding it
         CourseEnrollment.unenroll(self.user, course_id)
         target_certs = CertificateItem.objects.filter(course_id=course_id, user_id=self.user, status='refunded', mode='verified')
         self.assertTrue(target_certs[0])
 
     @patch('shoppingcart.models.log.error')
     def test_refund_cert_callback_before_expiration_email_error(self, error_logger):
+        # If there's an error sending an email to billing, we need to log this error
         course_id = "refund_before_expiration/test/one"
         many_days = datetime.timedelta(days=60)
 
@@ -415,14 +415,15 @@ class CertificateItemTest(ModuleStoreTestCase):
         CertificateItem.add_to_order(cart, course_id, self.cost, 'verified')
         cart.purchase()
 
-        # now that it's there, let's try refunding it
         with patch('shoppingcart.models.send_mail', side_effect=smtplib.SMTPException):
             CourseEnrollment.unenroll(self.user, course_id)
             self.assertTrue(error_logger.called)
 
     def test_refund_cert_callback_after_expiration(self):
-        # Enroll and buy
+        # If the expiration date has passed, the user cannot get a refund
         course_id = "refund_after_expiration/test/two"
+        many_days = datetime.timedelta(days=60)
+
         CourseFactory.create(org='refund_after_expiration', number='test', run='course', display_name='two')
         course_mode = CourseMode(course_id=course_id,
                                  mode_slug="verified",
@@ -434,14 +435,16 @@ class CertificateItemTest(ModuleStoreTestCase):
         cart = Order.get_cart_for_user(user=self.user)
         CertificateItem.add_to_order(cart, course_id, self.cost, 'verified')
         cart.purchase()
+
+        course_mode.expiration_date = (datetime.datetime.now(UTC()).date() - many_days)
         course_mode.save()
 
-        # now that it's there, let's try refunding it
         CourseEnrollment.unenroll(self.user, course_id)
         target_certs = CertificateItem.objects.filter(course_id=course_id, user_id=self.user, status='refunded', mode='verified')
-        self.assertTrue(target_certs[0])
+        self.assertEqual(len(target_certs),0)
 
     def test_refund_cert_no_cert_exists(self):
+        # If there is no paid certificate, the refund callback should return nothing
         CourseEnrollment.enroll(self.user, self.course_id, 'verified')
         ret_val = CourseEnrollment.unenroll(self.user, self.course_id)
         self.assertFalse(ret_val)

lms/envs/dev.py

@@ -19,7 +19,7 @@ DEBUG = True
 USE_I18N = True
 TEMPLATE_DEBUG = True
 
-MITX_FEATURES['ENABLE_PAYMENT_FAKE'] = True
+
 MITX_FEATURES['DISABLE_START_DATES'] = False
 MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True
 MITX_FEATURES['SUBDOMAIN_COURSE_LISTINGS'] = False  # Enable to test subdomains--otherwise, want all courses to show up
@@ -269,7 +269,7 @@ if SEGMENT_IO_LMS_KEY:
 CC_PROCESSOR['CyberSource']['SHARED_SECRET'] = os.environ.get('CYBERSOURCE_SHARED_SECRET', '')
 CC_PROCESSOR['CyberSource']['MERCHANT_ID'] = os.environ.get('CYBERSOURCE_MERCHANT_ID', '')
 CC_PROCESSOR['CyberSource']['SERIAL_NUMBER'] = os.environ.get('CYBERSOURCE_SERIAL_NUMBER', '')
-CC_PROCESSOR['CyberSource']['PURCHASE_ENDPOINT'] = '/shoppingcart/payment_fake/'
+CC_PROCESSOR['CyberSource']['PURCHASE_ENDPOINT'] = os.environ.get('CYBERSOURCE_PURCHASE_ENDPOINT', '')
 
 
 ########################## USER API ########################