diff --git a/common/djangoapps/student/management/commands/bulk_change_enrollment.py b/common/djangoapps/student/management/commands/bulk_change_enrollment.py index 371a8a9e33..94bcfc4119 100644 --- a/common/djangoapps/student/management/commands/bulk_change_enrollment.py +++ b/common/djangoapps/student/management/commands/bulk_change_enrollment.py @@ -6,6 +6,7 @@ from django.db import transaction from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from optparse import make_option +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from course_modes.models import CourseMode from student.models import CourseEnrollment @@ -49,6 +50,12 @@ class Command(BaseCommand): default=None, help='the course to change enrollments in' ), + make_option( + '-o', '--org', + dest='org', + default=None, + help='all courses belonging to this org will be selected for changing the enrollments' + ), make_option( '--commit', action='store_true', @@ -60,43 +67,64 @@ class Command(BaseCommand): def handle(self, *args, **options): course_id = options.get('course') + org = options.get('org') from_mode = options.get('from_mode') to_mode = options.get('to_mode') commit = options.get('commit') - if course_id is None: - raise CommandError('No course ID given.') + if (not course_id and not org) or (course_id and org): + raise CommandError('You must provide either a course ID or an org, but not both.') + if from_mode is None or to_mode is None: raise CommandError('Both `from` and `to` course modes must be given.') - try: - course_key = CourseKey.from_string(course_id) - except InvalidKeyError: - raise CommandError('Course ID {} is invalid.'.format(course_id)) + course_keys = [] + if course_id: + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError: + raise CommandError('Course ID {} is invalid.'.format(course_id)) - if modulestore().get_course(course_key) is None: - raise CommandError('The given course {} does not exist.'.format(course_id)) + if modulestore().get_course(course_key) is None: + raise CommandError('The given course {} does not exist.'.format(course_id)) + course_keys.append(course_key) + else: + course_keys = [course.id for course in CourseOverview.get_all_courses(orgs=[org])] + if not course_keys: + raise CommandError('No courses exist for the org "{}".'.format(org)) + + for course_key in course_keys: + self.move_users_for_course(course_key, from_mode, to_mode, commit) + + if not commit: + logger.info('Dry run, changes have not been saved. Run again with "commit" argument to save changes') + + def move_users_for_course(self, course_key, from_mode, to_mode, commit): + """ + Change the enrollment mode of users for a course. + + Arguments: + course_key (CourseKey): to lookup the course. + from_mode (str): the enrollment mode to change. + to_mode (str): the enrollment mode to change to. + commit (bool): required to make the change to the database. Otherwise + just a count will be displayed. + """ if CourseMode.mode_for_course(course_key, to_mode) is None: raise CommandError('The given mode to move users into ({}) does not exist.'.format(to_mode)) course_key_str = unicode(course_key) - try: - course_enrollments = CourseEnrollment.objects.filter(course_id=course_key, mode=from_mode) - logger.info( - 'Moving %d users from %s to %s in course %s.', - course_enrollments.count(), from_mode, to_mode, course_key_str - ) - if not commit: - logger.info('Dry run, changes have not been saved. Run again with "commit" argument to save changes') - raise Exception('The --commit flag was not given; forcing rollback.') - + course_enrollments = CourseEnrollment.objects.filter(course_id=course_key, mode=from_mode) + logger.info( + 'Moving %d users from %s to %s in course %s.', + course_enrollments.count(), from_mode, to_mode, course_key_str + ) + if commit: # call `change_mode` which will change the mode and also emit tracking event for enrollment in course_enrollments: with transaction.atomic(): enrollment.change_mode(mode=to_mode) logger.info('Finished moving users from %s to %s in course %s.', from_mode, to_mode, course_key_str) - except Exception: # pylint: disable=broad-except - logger.info('No users moved.') diff --git a/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py b/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py index 8897c40547..dec5c637af 100644 --- a/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py +++ b/common/djangoapps/student/management/tests/test_bulk_change_enrollment.py @@ -9,6 +9,8 @@ from student.models import CourseEnrollment, EVENT_NAME_ENROLLMENT_MODE_CHANGED from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + @ddt.ddt class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): @@ -16,8 +18,10 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): def setUp(self): super(BulkChangeEnrollmentTests, self).setUp() - self.course = CourseFactory.create() + self.org = 'testX' + self.course = CourseFactory.create(org=self.org) self.users = UserFactory.create_batch(5) + CourseOverview.load_from_module_store(self.course.id) @patch('student.models.tracker') @ddt.data(('audit', 'honor'), ('honor', 'audit')) @@ -53,6 +57,69 @@ class BulkChangeEnrollmentTests(SharedModuleStoreTestCase): ] ) + @patch('student.models.tracker') + @ddt.data(('audit', 'no-id-professional'), ('no-id-professional', 'audit')) + @ddt.unpack + def test_bulk_convert_with_org(self, from_mode, to_mode, mock_tracker): + """Verify that enrollments are changed correctly when org was given.""" + self._enroll_users(from_mode) + CourseModeFactory(course_id=self.course.id, mode_slug=to_mode) + + # Verify that no users are in the `from` mode yet. + self.assertEqual(len(CourseEnrollment.objects.filter(mode=to_mode, course_id=self.course.id)), 0) + + call_command( + 'bulk_change_enrollment', + org=self.org, + from_mode=from_mode, + to_mode=to_mode, + commit=True, + ) + + # Verify that all users have been moved -- if not, this will + # raise CourseEnrollment.DoesNotExist + for user in self.users: + CourseEnrollment.objects.get(mode=to_mode, course_id=self.course.id, user=user) + + # Confirm the analytics event was emitted. + mock_tracker.emit.assert_has_calls( # pylint: disable=maybe-no-member + [ + call( + EVENT_NAME_ENROLLMENT_MODE_CHANGED, + {'course_id': unicode(self.course.id), 'user_id': user.id, 'mode': to_mode} + ), + ] + ) + + def test_with_org_and_course_key(self): + """Verify that command raises CommandError when `org` and `course_key` both are given.""" + self._enroll_users('audit') + CourseModeFactory(course_id=self.course.id, mode_slug='no-id-professional') + + with self.assertRaises(CommandError): + call_command( + 'bulk_change_enrollment', + org=self.org, + course=unicode(self.course.id), + from_mode='audit', + to_mode='no-id-professional', + commit=True, + ) + + def test_with_invalid_org(self): + """Verify that command raises CommandError when invalid `org` is given.""" + self._enroll_users('audit') + CourseModeFactory(course_id=self.course.id, mode_slug='no-id-professional') + + with self.assertRaises(CommandError): + call_command( + 'bulk_change_enrollment', + org='fakeX', + from_mode='audit', + to_mode='no-id-professional', + commit=True, + ) + def test_without_commit(self): """Verify that nothing happens when the `commit` flag is not given.""" self._enroll_users('audit')