diff --git a/common/djangoapps/student/management/commands/transfer_students.py b/common/djangoapps/student/management/commands/transfer_students.py index c1b46cdeff22ff175cfeed69f7a569a2538a1ca1..041402e19e5ebfbdd31f23498aef21b49fec8cd0 100644 --- a/common/djangoapps/student/management/commands/transfer_students.py +++ b/common/djangoapps/student/management/commands/transfer_students.py @@ -1,81 +1,135 @@ +""" +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 diff --git a/common/djangoapps/student/management/tests/__init__.py b/common/djangoapps/student/management/tests/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..b7d6ea0722d2506a8d9da2d1a2909a855c5244a3 --- /dev/null +++ b/common/djangoapps/student/management/tests/__init__.py @@ -0,0 +1 @@ +"""Tests for Student Management Commands.""" diff --git a/common/djangoapps/student/management/tests/test_transfer_students.py b/common/djangoapps/student/management/tests/test_transfer_students.py new file mode 100644 index 0000000000000000000000000000000000000000..caebeeace29d068c8788909ecf43efc3ddccc565 --- /dev/null +++ b/common/djangoapps/student/management/tests/test_transfer_students.py @@ -0,0 +1,60 @@ +""" +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 + ) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 70691adf289e32ac49b204a8fce0440ee0243d9a..dda87a80e5b2ceda06f1e1e5bde5883b6424cdf2 100644 --- a/common/djangoapps/student/models.py +++ b/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"