diff --git a/common/djangoapps/student/management/commands/transfer_students.py b/common/djangoapps/student/management/commands/transfer_students.py index 041402e19e..8e8557ff88 100644 --- a/common/djangoapps/student/management/commands/transfer_students.py +++ b/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) diff --git a/common/djangoapps/student/management/tests/test_transfer_students.py b/common/djangoapps/student/management/tests/test_transfer_students.py index caebeeace2..45376eb148 100644 --- a/common/djangoapps/student/management/tests/test_transfer_students.py +++ b/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() diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 09967cf346..08d5c0ffb7 100644 --- a/common/djangoapps/student/models.py +++ b/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" diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 37a32dfe0c..d06624b2c1 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/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') diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index e1624ee730..69449b701a 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/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)