Merge pull request #5619 from edx/sanchez/transfer-student-command

Update the student transfer command to transfer to multiple courses.

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

+"""
+Transfer Student Management Command
+"""
+from django.db import transaction
+from opaque_keys.edx.keys import CourseKey
 from optparse import make_option
-from django.core.management.base import BaseCommand
 from django.contrib.auth.models import User
 from student.models import CourseEnrollment
 from shoppingcart.models import CertificateItem
-from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from track.management.tracked_command import TrackedCommand
 
 
-class Command(BaseCommand):
+class TransferStudentError(Exception):
+    """Generic Error when handling student transfers."""
+    pass
+
+
+class Command(TrackedCommand):
+    """Management Command for transferring students from one course to new courses."""
     help = """
     This command takes two course ids as input and transfers
     all students enrolled in one course into the other.  This will
-    remove them from the first class and enroll them in the second
-    class in the same mode as the first one. eg. honor, verified,
+    remove them from the first class and enroll them in the specified
+    class(es) in the same mode as the first one. eg. honor, verified,
     audit.
 
     example:
         # Transfer students from the old demoX class to a new one.
         manage.py ... transfer_students -f edX/Open_DemoX/edx_demo_course -t edX/Open_DemoX/new_demoX
+
+        # Transfer students from old course to new, with original certificate items.
+        manage.py ... transfer_students -f edX/Open_DemoX/edx_demo_course -t edX/Open_DemoX/new_demoX -c true
+
+        # Transfer students from the old demoX class into two new classes.
+        manage.py ... transfer_students -f edX/Open_DemoX/edx_demo_course
+            -t edX/Open_DemoX/new_demoX,edX/Open_DemoX/edX_Insider
+
     """
 
-    option_list = BaseCommand.option_list + (
+    option_list = TrackedCommand.option_list + (
         make_option('-f', '--from',
                     metavar='SOURCE_COURSE',
                     dest='source_course',
                     help='The course to transfer students from.'),
         make_option('-t', '--to',
-                    metavar='DEST_COURSE',
-                    dest='dest_course',
-                    help='The new course to enroll the student into.'),
+                    metavar='DEST_COURSE_LIST',
+                    dest='dest_course_list',
+                    help='The new course(es) to enroll the student into.'),
+        make_option('-c', '--transfer-certificates',
+                    metavar='TRANSFER_CERTIFICATES',
+                    dest='transfer_certificates',
+                    help="If True, try to transfer certificate items to the new course.")
     )
 
-    def handle(self, *args, **options):
-        source_key = SlashSeparatedCourseKey.from_deprecated_string(options['source_course'])
-        dest_key = SlashSeparatedCourseKey.from_deprecated_string(options['dest_course'])
+    @transaction.commit_manually
+    def handle(self, *args, **options):  # pylint: disable=unused-argument
+        source_key = CourseKey.from_string(options.get('source_course', ''))
+        dest_keys = []
+        for course_key in options.get('dest_course_list', '').split(','):
+            dest_keys.append(CourseKey.from_string(course_key))
+
+        if not source_key or not dest_keys:
+            raise TransferStudentError(u"Must have a source course and destination course specified.")
+
+        tc_option = options.get('transfer_certificates', '')
+        transfer_certificates = ('true' == tc_option.lower()) if tc_option else False
+        if transfer_certificates and len(dest_keys) != 1:
+            raise TransferStudentError(u"Cannot transfer certificate items from one course to many.")
 
         source_students = User.objects.filter(
             courseenrollment__course_id=source_key
         )
 
         for user in source_students:
-            if CourseEnrollment.is_enrolled(user, dest_key):
-                # Un Enroll from source course but don't mess
-                # with the enrollment in the destination course.
-                CourseEnrollment.unenroll(user, source_key)
-                print("Unenrolled {} from {}".format(user.username, source_key.to_deprecated_string()))
-                msg = "Skipping {}, already enrolled in destination course {}"
-                print(msg.format(user.username, dest_key.to_deprecated_string()))
-                continue
-
-            print("Moving {}.".format(user.username))
-            # Find the old enrollment.
-            enrollment = CourseEnrollment.objects.get(
-                user=user,
-                course_id=source_key
+            with transaction.commit_on_success():
+                print("Moving {}.".format(user.username))
+                # Find the old enrollment.
+                enrollment = CourseEnrollment.objects.get(
+                    user=user,
+                    course_id=source_key
+                )
+
+                # Move the Student between the classes.
+                mode = enrollment.mode
+                old_is_active = enrollment.is_active
+                CourseEnrollment.unenroll(user, source_key, emit_unenrollment_event=False)
+                print(u"Unenrolled {} from {}".format(user.username, unicode(source_key)))
+
+                for dest_key in dest_keys:
+                    if CourseEnrollment.is_enrolled(user, dest_key):
+                        # Un Enroll from source course but don't mess
+                        # with the enrollment in the destination course.
+                        msg = u"Skipping {}, already enrolled in destination course {}"
+                        print(msg.format(user.username, unicode(dest_key)))
+                    else:
+                        new_enrollment = CourseEnrollment.enroll(user, dest_key, mode=mode)
+
+                        # 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)
+
+                        if transfer_certificates:
+                            self._transfer_certificate_item(source_key, enrollment, user, dest_keys, new_enrollment)
+
+    @staticmethod
+    def _transfer_certificate_item(source_key, enrollment, user, dest_keys, new_enrollment):
+        """ Transfer the certificate item from one course to another.
+
+        Do not use this generally, since certificate items are directly associated with a particular purchase.
+        This should only be used when a single course to a new location. This cannot be used when transferring
+        from one course to many.
+
+        Args:
+            source_key (str): The course key string representation for the original course.
+            enrollment (CourseEnrollment): The original enrollment to move the certificate item from.
+            user (User): The user to transfer the item for.
+            dest_keys (list): A list of course key strings to transfer the item to.
+            new_enrollment (CourseEnrollment): The new enrollment to associate the certificate item with.
+
+        Returns:
+            None
+
+        """
+        try:
+            certificate_item = CertificateItem.objects.get(
+                course_id=source_key,
+                course_enrollment=enrollment
             )
+        except CertificateItem.DoesNotExist:
+            print(u"No certificate for {}".format(user))
+            return
 
-            # Move the Student between the classes.
-            mode = enrollment.mode
-            old_is_active = enrollment.is_active
-            CourseEnrollment.unenroll(user, source_key)
-            new_enrollment = CourseEnrollment.enroll(user, dest_key, mode=mode)
-
-            # Unenroll from the new coures if the user had unenrolled
-            # form the old course.
-            if not old_is_active:
-                new_enrollment.update_enrollment(is_active=False)
-
-            if mode == 'verified':
-                try:
-                    certificate_item = CertificateItem.objects.get(
-                        course_id=source_key,
-                        course_enrollment=enrollment
-                    )
-                except CertificateItem.DoesNotExist:
-                    print("No certificate for {}".format(user))
-                    continue
-
-                certificate_item.course_id = dest_key
-                certificate_item.course_enrollment = new_enrollment
-                certificate_item.save()
+        certificate_item.course_id = dest_keys[0]
+        certificate_item.course_enrollment = new_enrollment

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

+"""Tests for Student Management Commands."""

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

+"""
+Tests the transfer student management command
+"""
+from django.conf import settings
+from opaque_keys.edx import locator
+import unittest
+import ddt
+from student.management.commands import transfer_students
+from student.models import CourseEnrollment
+from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
+@ddt.ddt
+class TestTransferStudents(ModuleStoreTestCase):
+    """Tests for transferring students between courses."""
+
+    PASSWORD = 'test'
+
+    def test_transfer_students(self):
+        student = UserFactory()
+        student.set_password(self.PASSWORD)  # pylint: disable=E1101
+        student.save()   # pylint: disable=E1101
+
+        # 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")
+
+        # New Course 1
+        course_location_one = locator.CourseLocator('Org1', 'Course1', 'Run1')
+        new_course_one = self._create_course(course_location_one)
+
+        # New Course 2
+        course_location_two = locator.CourseLocator('Org2', 'Course2', 'Run2')
+        new_course_two = self._create_course(course_location_two)
+        original_key = unicode(course.id)
+        new_key_one = unicode(new_course_one.id)
+        new_key_two = unicode(new_course_two.id)
+
+        # Run the actual management command
+        transfer_students.Command().handle(
+            source_course=original_key, dest_course_list=new_key_one + "," + new_key_two
+        )
+
+        # 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))
+
+    def _create_course(self, course_location):
+        """ Creates a course """
+        return CourseFactory.create(
+            org=course_location.org,
+            number=course_location.course,
+            run=course_location.run
+        )

common/djangoapps/student/models.py

@@ -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):
+    def update_enrollment(self, mode=None, is_active=None, emit_unenrollment_event=True):
         """
         Updates an enrollment for a user in a class.  This includes options
         like changing the mode, toggling is_active True/False, etc.
@@ -784,6 +784,7 @@ class CourseEnrollment(models.Model):
         Also emits relevant events for analytics purposes.
 
         This saves immediately.
+
         """
         activation_changed = False
         # if is_active is None, then the call to update_enrollment didn't specify
@@ -813,7 +814,7 @@ class CourseEnrollment(models.Model):
                           u"mode:{}".format(self.mode)]
                 )
 
-            else:
+            elif emit_unenrollment_event:
                 UNENROLL_DONE.send(sender=None, course_enrollment=self)
 
                 self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
@@ -987,7 +988,7 @@ class CourseEnrollment(models.Model):
             raise
 
     @classmethod
-    def unenroll(cls, user, course_id):
+    def unenroll(cls, user, course_id, emit_unenrollment_event=True):
         """
         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.
@@ -997,10 +998,12 @@ class CourseEnrollment(models.Model):
                adding an enrollment for it.
 
         `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.
         """
         try:
             record = CourseEnrollment.objects.get(user=user, course_id=course_id)
-            record.update_enrollment(is_active=False)
+            record.update_enrollment(is_active=False, emit_unenrollment_event=emit_unenrollment_event)
 
         except cls.DoesNotExist:
             err_msg = u"Tried to unenroll student {} from {} but they were not enrolled"