Merge pull request #8272 from edx/renzo/merge-release-into-master

Merge release into master

lms/djangoapps/commerce/signals.py

@@ -39,7 +39,7 @@ def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, **kw
             # trapping the Exception and logging an error.
             log.exception(
                 "Unexpected exception while attempting to initiate refund for user [%s], course [%s]",
-                course_enrollment.user,
+                course_enrollment.user.id,
                 course_enrollment.course_id,
             )
 
@@ -90,7 +90,7 @@ def refund_seat(course_enrollment, request_user):
             # support the case of a non-superusers initiating a refund on
             # behalf of another user.
             log.warning("User [%s] was not authorized to initiate a refund for user [%s] "
-                        "upon unenrollment from course [%s]", request_user, unenrolled_user, course_key_str)
+                        "upon unenrollment from course [%s]", request_user.id, unenrolled_user.id, course_key_str)
             return []
         else:
             # no other error is anticipated, so re-raise the Exception
@@ -104,11 +104,27 @@ def refund_seat(course_enrollment, request_user):
             course_key_str,
             refund_ids,
         )
-        try:
-            send_refund_notification(course_enrollment, refund_ids)
-        except:  # pylint: disable=bare-except
-            # don't break, just log a warning
-            log.warning("Could not send email notification for refund.", exc_info=True)
+
+        # XCOM-371: this is a temporary measure to suppress refund-related email
+        # notifications to students and support@) for free enrollments.  This
+        # condition should be removed when the CourseEnrollment.refundable() logic
+        # is updated to be more correct, or when we implement better handling (and
+        # notifications) in Otto for handling reversal of $0 transactions.
+        if course_enrollment.mode != 'verified':
+            # 'verified' is the only enrollment mode that should presently
+            # result in opening a refund request.
+            log.info(
+                "Skipping refund email notification for non-verified mode for user [%s], course [%s], mode: [%s]",
+                course_enrollment.user.id,
+                course_enrollment.course_id,
+                course_enrollment.mode,
+            )
+        else:
+            try:
+                send_refund_notification(course_enrollment, refund_ids)
+            except:  # pylint: disable=bare-except
+                # don't break, just log a warning
+                log.warning("Could not send email notification for refund.", exc_info=True)
     else:
         # no refundable orders were found.
         log.debug("No refund opened for user [%s], course [%s]", unenrolled_user.id, course_key_str)

lms/djangoapps/commerce/tests/test_signals.py

@@ -4,6 +4,8 @@ Tests for signal handling in commerce djangoapp.
 from django.test import TestCase
 from django.test.utils import override_settings
 
+from course_modes.models import CourseMode
+import ddt
 import mock
 from opaque_keys.edx.keys import CourseKey
 from student.models import UNENROLL_DONE
@@ -14,6 +16,7 @@ from commerce.tests import TEST_PUBLIC_URL_ROOT, TEST_API_URL, TEST_API_SIGNING_
 from commerce.tests.mocks import mock_create_refund
 
 
+@ddt.ddt
 @override_settings(
     ECOMMERCE_PUBLIC_URL_ROOT=TEST_PUBLIC_URL_ROOT,
     ECOMMERCE_API_URL=TEST_API_URL,
@@ -31,6 +34,7 @@ class TestRefundSignal(TestCase):
         self.course_enrollment = CourseEnrollmentFactory(
             user=self.student,
             course_id=CourseKey.from_string('course-v1:org+course+run'),
+            mode=CourseMode.VERIFIED,
         )
         self.course_enrollment.refundable = mock.Mock(return_value=True)
 
@@ -111,7 +115,7 @@ class TestRefundSignal(TestCase):
         Ensure that expected authorization issues are logged as warnings.
         """
         with mock_create_refund(status=403):
-            refund_seat(self.course_enrollment, None)
+            refund_seat(self.course_enrollment, UserFactory())
             self.assertTrue(mock_log_warning.called)
 
     @mock.patch('commerce.signals.log.exception')
@@ -125,14 +129,14 @@ class TestRefundSignal(TestCase):
             self.assertTrue(mock_log_exception.called)
 
     @mock.patch('commerce.signals.send_refund_notification')
-    def test_notification(self, mock_send_notificaton):
+    def test_notification(self, mock_send_notification):
         """
         Ensure the notification function is triggered when refunds are
         initiated
         """
         with mock_create_refund(status=200, response=[1, 2, 3]):
             self.send_signal()
-            self.assertTrue(mock_send_notificaton.called)
+            self.assertTrue(mock_send_notification.called)
 
     @mock.patch('commerce.signals.send_refund_notification')
     def test_notification_no_refund(self, mock_send_notification):
@@ -144,6 +148,27 @@ class TestRefundSignal(TestCase):
             self.send_signal()
             self.assertFalse(mock_send_notification.called)
 
+    @mock.patch('commerce.signals.send_refund_notification')
+    @ddt.data(
+        CourseMode.HONOR,
+        CourseMode.PROFESSIONAL,
+        CourseMode.AUDIT,
+        CourseMode.NO_ID_PROFESSIONAL_MODE,
+        CourseMode.CREDIT_MODE,
+    )
+    def test_notification_not_verified(self, mode, mock_send_notification):
+        """
+        Ensure the notification function is NOT triggered when the
+        unenrollment is for any mode other than verified (i.e. any mode other
+        than one for which refunds are presently supported).  See the
+        TODO associated with XCOM-371 in the signals module in the commerce
+        package for more information.
+        """
+        self.course_enrollment.mode = mode
+        with mock_create_refund(status=200, response=[1, 2, 3]):
+            self.send_signal()
+            self.assertFalse(mock_send_notification.called)
+
     @mock.patch('commerce.signals.send_refund_notification', side_effect=Exception("Splat!"))
     @mock.patch('commerce.signals.log.warning')
     def test_notification_error(self, mock_log_warning, mock_send_notification):