Merge pull request #5720 from edx/sanchez/avoid_event_suppression

Changing event suppression to signal suppression.

common/djangoapps/student/management/commands/transfer_students.py

@@ -83,7 +83,7 @@ class Command(TrackedCommand):
                 # Move the Student between the classes.
                 mode = enrollment.mode
                 old_is_active = enrollment.is_active
-                CourseEnrollment.unenroll(user, source_key, emit_unenrollment_event=False)
+                CourseEnrollment.unenroll(user, source_key, skip_refund=True)
                 print(u"Unenrolled {} from {}".format(user.username, unicode(source_key)))
 
                 for dest_key in dest_keys:
@@ -98,7 +98,7 @@ class Command(TrackedCommand):
                         # Un-enroll from the new course if the user had un-enrolled
                         # form the old course.
                         if not old_is_active:
-                            new_enrollment.update_enrollment(is_active=False, emit_unenrollment_event=False)
+                            new_enrollment.update_enrollment(is_active=False, skip_refund=True)
 
                         if transfer_certificates:
                             self._transfer_certificate_item(source_key, enrollment, user, dest_keys, new_enrollment)

common/djangoapps/student/management/tests/test_transfer_students.py

@@ -2,11 +2,16 @@
 Tests the transfer student management command
 """
 from django.conf import settings
+from mock import patch, call
 from opaque_keys.edx import locator
 import unittest
 import ddt
+
+from shoppingcart.models import Order, CertificateItem  # pylint: disable=F0401
+from course_modes.models import CourseMode
 from student.management.commands import transfer_students
-from student.models import CourseEnrollment
+from student.models import CourseEnrollment, UNENROLL_DONE, EVENT_NAME_ENROLLMENT_DEACTIVATED, \
+    EVENT_NAME_ENROLLMENT_ACTIVATED, EVENT_NAME_ENROLLMENT_MODE_CHANGED
 from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -18,18 +23,40 @@ class TestTransferStudents(ModuleStoreTestCase):
     """Tests for transferring students between courses."""
 
     PASSWORD = 'test'
+    signal_fired = False
+
+    def setUp(self, **kwargs):
+        """Connect a stub receiver, and analytics event tracking."""
+        UNENROLL_DONE.connect(self.assert_unenroll_signal)
+        patcher = patch('student.models.tracker')
+        self.mock_tracker = patcher.start()
+        self.addCleanup(patcher.stop)
+
+    def tearDown(self):
+        """Disconnects the UNENROLL stub receiver."""
+        UNENROLL_DONE.disconnect(self.assert_unenroll_signal)
+
+    def assert_unenroll_signal(self, skip_refund=False, **kwargs):   # pylint: disable=W0613
+        """ Signal Receiver stub for testing that the unenroll signal was fired. """
+        self.assertFalse(self.signal_fired)
+        self.assertTrue(skip_refund)
+        self.signal_fired = True
 
     def test_transfer_students(self):
-        student = UserFactory()
+        """ Verify the transfer student command works as intended. """
+        student = UserFactory.create()
         student.set_password(self.PASSWORD)  # pylint: disable=E1101
         student.save()   # pylint: disable=E1101
-
+        mode = 'verified'
         # Original Course
         original_course_location = locator.CourseLocator('Org0', 'Course0', 'Run0')
         course = self._create_course(original_course_location)
         # Enroll the student in 'verified'
         CourseEnrollment.enroll(student, course.id, mode="verified")
 
+        # Create and purchase a verified cert for the original course.
+        self._create_and_purchase_verified(student, course.id)
+
         # New Course 1
         course_location_one = locator.CourseLocator('Org1', 'Course1', 'Run1')
         new_course_one = self._create_course(course_location_one)
@@ -45,11 +72,55 @@ class TestTransferStudents(ModuleStoreTestCase):
         transfer_students.Command().handle(
             source_course=original_key, dest_course_list=new_key_one + "," + new_key_two
         )
+        self.assertTrue(self.signal_fired)
+
+        # Confirm the analytics event was emitted.
+        self.mock_tracker.emit.assert_has_calls(  # pylint: disable=E1103
+            [
+                call(
+                    EVENT_NAME_ENROLLMENT_ACTIVATED,
+                    {'course_id': original_key, 'user_id': student.id, 'mode': mode}
+                ),
+                call(
+                    EVENT_NAME_ENROLLMENT_MODE_CHANGED,
+                    {'course_id': original_key, 'user_id': student.id, 'mode': mode}
+                ),
+                call(
+                    EVENT_NAME_ENROLLMENT_DEACTIVATED,
+                    {'course_id': original_key, 'user_id': student.id, 'mode': mode}
+                ),
+                call(
+                    EVENT_NAME_ENROLLMENT_ACTIVATED,
+                    {'course_id': new_key_one, 'user_id': student.id, 'mode': mode}
+                ),
+                call(
+                    EVENT_NAME_ENROLLMENT_MODE_CHANGED,
+                    {'course_id': new_key_one, 'user_id': student.id, 'mode': mode}
+                ),
+                call(
+                    EVENT_NAME_ENROLLMENT_ACTIVATED,
+                    {'course_id': new_key_two, 'user_id': student.id, 'mode': mode}
+                ),
+                call(
+                    EVENT_NAME_ENROLLMENT_MODE_CHANGED,
+                    {'course_id': new_key_two, 'user_id': student.id, 'mode': mode}
+                )
+            ]
+        )
+        self.mock_tracker.reset_mock()
 
         # Confirm the enrollment mode is verified on the new courses, and enrollment is enabled as appropriate.
-        self.assertEquals(('verified', False), CourseEnrollment.enrollment_mode_for_user(student, course.id))
-        self.assertEquals(('verified', True), CourseEnrollment.enrollment_mode_for_user(student, new_course_one.id))
-        self.assertEquals(('verified', True), CourseEnrollment.enrollment_mode_for_user(student, new_course_two.id))
+        self.assertEquals((mode, False), CourseEnrollment.enrollment_mode_for_user(student, course.id))
+        self.assertEquals((mode, True), CourseEnrollment.enrollment_mode_for_user(student, new_course_one.id))
+        self.assertEquals((mode, True), CourseEnrollment.enrollment_mode_for_user(student, new_course_two.id))
+
+        # Confirm the student has not be refunded.
+        target_certs = CertificateItem.objects.filter(
+            course_id=course.id, user_id=student, status='purchased', mode=mode
+        )
+        self.assertTrue(target_certs[0])
+        self.assertFalse(target_certs[0].refund_requested_time)
+        self.assertEquals(target_certs[0].order.status, 'purchased')
 
     def _create_course(self, course_location):
         """ Creates a course """
@@ -58,3 +129,15 @@ class TestTransferStudents(ModuleStoreTestCase):
             number=course_location.course,
             run=course_location.run
         )
+
+    def _create_and_purchase_verified(self, student, course_id):
+        """ Creates a verified mode for the course and purchases it for the student. """
+        course_mode = CourseMode(course_id=course_id,
+                                 mode_slug="verified",
+                                 mode_display_name="verified cert",
+                                 min_price=50)
+        course_mode.save()
+        # When there is no expiration date on a verified mode, the user can always get a refund
+        cart = Order.get_cart_for_user(user=student)
+        CertificateItem.add_to_order(cart, course_id, 50, 'verified')
+        cart.purchase()

common/djangoapps/student/models.py

@@ -56,7 +56,7 @@ from ratelimitbackend import admin
 
 import analytics
 
-UNENROLL_DONE = Signal(providing_args=["course_enrollment"])
+UNENROLL_DONE = Signal(providing_args=["course_enrollment", "skip_refund"])
 log = logging.getLogger(__name__)
 AUDIT_LOG = logging.getLogger("audit")
 SessionStore = import_module(settings.SESSION_ENGINE).SessionStore  # pylint: disable=invalid-name
@@ -780,7 +780,7 @@ class CourseEnrollment(models.Model):
             is_course_full = cls.num_enrolled_in(course.id) >= course.max_student_enrollments_allowed
         return is_course_full
 
-    def update_enrollment(self, mode=None, is_active=None, emit_unenrollment_event=True):
+    def update_enrollment(self, mode=None, is_active=None, skip_refund=False):
         """
         Updates an enrollment for a user in a class.  This includes options
         like changing the mode, toggling is_active True/False, etc.
@@ -818,8 +818,8 @@ class CourseEnrollment(models.Model):
                           u"mode:{}".format(self.mode)]
                 )
 
-            elif emit_unenrollment_event:
-                UNENROLL_DONE.send(sender=None, course_enrollment=self)
+            else:
+                UNENROLL_DONE.send(sender=None, course_enrollment=self, skip_refund=skip_refund)
 
                 self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
 
@@ -992,7 +992,7 @@ class CourseEnrollment(models.Model):
             raise
 
     @classmethod
-    def unenroll(cls, user, course_id, emit_unenrollment_event=True):
+    def unenroll(cls, user, course_id, skip_refund=False):
         """
         Remove the user from a given course. If the relevant `CourseEnrollment`
         object doesn't exist, we log an error but don't throw an exception.
@@ -1003,11 +1003,11 @@ class CourseEnrollment(models.Model):
 
         `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall)
 
-        `emit_unenrollment_events` can be set to False to suppress events firing.
+        `skip_refund` can be set to True to avoid the refund process.
         """
         try:
             record = CourseEnrollment.objects.get(user=user, course_id=course_id)
-            record.update_enrollment(is_active=False, emit_unenrollment_event=emit_unenrollment_event)
+            record.update_enrollment(is_active=False, skip_refund=skip_refund)
 
         except cls.DoesNotExist:
             err_msg = u"Tried to unenroll student {} from {} but they were not enrolled"

lms/djangoapps/shoppingcart/models.py

@@ -1038,7 +1038,7 @@ class CertificateItem(OrderItem):
     mode = models.SlugField()
 
     @receiver(UNENROLL_DONE)
-    def refund_cert_callback(sender, course_enrollment=None, **kwargs):
+    def refund_cert_callback(sender, course_enrollment=None, skip_refund=False, **kwargs):  # pylint: disable=E0213,W0613
         """
         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
@@ -1048,7 +1048,7 @@ class CertificateItem(OrderItem):
         """
 
         # Only refund verified cert unenrollments that are within bounds of the expiration date
-        if not course_enrollment.refundable():
+        if (not course_enrollment.refundable()) or skip_refund:
             return
 
         target_certs = CertificateItem.objects.filter(course_id=course_enrollment.course_id, user_id=course_enrollment.user, status='purchased', mode='verified')

lms/djangoapps/shoppingcart/tests/test_models.py

@@ -496,6 +496,24 @@ class CertificateItemTest(ModuleStoreTestCase):
         self.assertTrue(target_certs[0].refund_requested_time)
         self.assertEquals(target_certs[0].order.status, 'refunded')
 
+    def test_no_refund_on_cert_callback(self):
+        # If we explicitly skip refunds, the unenroll action should not modify the purchase.
+        CourseEnrollment.enroll(self.user, self.course_key, 'verified')
+        cart = Order.get_cart_for_user(user=self.user)
+        CertificateItem.add_to_order(cart, self.course_key, self.cost, 'verified')
+        cart.purchase()
+
+        CourseEnrollment.unenroll(self.user, self.course_key, skip_refund=True)
+        target_certs = CertificateItem.objects.filter(
+            course_id=self.course_key,
+            user_id=self.user,
+            status='purchased',
+            mode='verified'
+        )
+        self.assertTrue(target_certs[0])
+        self.assertFalse(target_certs[0].refund_requested_time)
+        self.assertEquals(target_certs[0].order.status, 'purchased')
+
     def test_refund_cert_callback_before_expiration(self):
         # If the expiration date has not yet passed on a verified mode, the user can be refunded
         many_days = datetime.timedelta(days=60)