Changing event suppression to signal suppression.

Code review changes.

Fix test and pylint.

pylint error.

Updating the test to verify the signal is fired.

Test for all analytics events

Import pylint error.

Pylint error

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
@@ -776,7 +776,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.
@@ -814,8 +814,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)
 
@@ -988,7 +988,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.
@@ -999,11 +999,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

@@ -495,6 +495,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)