diff --git a/lms/djangoapps/program_enrollments/management/commands/link_program_enrollments.py b/lms/djangoapps/program_enrollments/management/commands/link_program_enrollments.py index a91d43d48e..14c1236263 100644 --- a/lms/djangoapps/program_enrollments/management/commands/link_program_enrollments.py +++ b/lms/djangoapps/program_enrollments/management/commands/link_program_enrollments.py @@ -3,7 +3,7 @@ import logging from django.contrib.auth import get_user_model from django.core.management.base import BaseCommand, CommandError -from django.db import transaction +from django.db import IntegrityError, transaction from lms.djangoapps.program_enrollments.models import ProgramEnrollment from student.models import CourseEnrollmentException @@ -46,8 +46,11 @@ class Command(BaseCommand): enrollments updated. If there is an error while enrolling a user in a waiting program course enrollment, the error will be - logged, but we will continue attempting to enroll the user in courses, and we will process all other - input users + logged, and we will roll back all transactions for that user so that their db state will be the same as + it was before this command was run. This is to allow the re-running of the same command again to correctly enroll + the user once the issue preventing the enrollment has been resolved. + + No other users will be affected, they will be processed normally. """ help = u'Manually links ProgramEnrollment records to LMS users' @@ -82,8 +85,15 @@ class Command(BaseCommand): if not user: logger.warning(NO_LMS_USER_TPL.format(username)) continue - - self.link_program_enrollment(program_enrollment, user) + try: + with transaction.atomic(): + self.link_program_enrollment(program_enrollment, user) + except (CourseEnrollmentException, IntegrityError): + logger.exception(u"Rolling back all operations for {}:{}".format( + external_student_key, + username, + )) + continue # transaction rolled back def parse_user_items(self, user_items): """ @@ -136,6 +146,23 @@ class Command(BaseCommand): """ Attempts to link the given program enrollment to the given user If the enrollment has any program course enrollments, enroll the user in those courses as well + + Raises: CourseEnrollmentException if there is an error enrolling user in a waiting + program course enrollment + IntegrityError if we try to create invalid records. + """ + try: + self._link_program_enrollment(program_enrollment, user) + self._link_course_enrollments(program_enrollment, user) + except IntegrityError: + logger.exception("Integrity error while linking program enrollments") + raise + + def _link_program_enrollment(self, program_enrollment, user): + """ + Links program enrollment to user. + + Raises IntegrityError if ProgramEnrollment is invalid """ if program_enrollment.user: logger.warning(get_existing_user_message(program_enrollment, user)) @@ -147,14 +174,23 @@ class Command(BaseCommand): program_enrollment.user = user program_enrollment.save() - for program_course_enrollment in program_enrollment.program_course_enrollments.all(): - try: + def _link_course_enrollments(self, program_enrollment, user): + """ + Enrolls user in waiting program course enrollments + + Raises: + IntegrityError if a constraint is violated + CourseEnrollmentException if there is an issue enrolling the user in a course + """ + try: + for program_course_enrollment in program_enrollment.program_course_enrollments.all(): program_course_enrollment.enroll(user) - except CourseEnrollmentException: - logger.warning(COURSE_ENROLLMENT_ERR_TPL.format( - user=user.username, - course=program_course_enrollment.course_key - )) + except CourseEnrollmentException: + logger.exception(COURSE_ENROLLMENT_ERR_TPL.format( + user=user.username, + course=program_course_enrollment.course_key + )) + raise def get_existing_user_message(program_enrollment, user): diff --git a/lms/djangoapps/program_enrollments/management/commands/tests/test_link_program_enrollments.py b/lms/djangoapps/program_enrollments/management/commands/tests/test_link_program_enrollments.py index 160086248e..24ad84c64a 100644 --- a/lms/djangoapps/program_enrollments/management/commands/tests/test_link_program_enrollments.py +++ b/lms/djangoapps/program_enrollments/management/commands/tests/test_link_program_enrollments.py @@ -35,6 +35,7 @@ class TestLinkProgramEnrollmentsMixin(object): def setUpTestData(cls): # pylint: disable=missing-docstring cls.command = Command() cls.program = uuid4() + cls.curriculum = uuid4() cls.other_program = uuid4() cls.fruit_course = CourseKey.from_string('course-v1:edX+Oranges+Apples') cls.animal_course = CourseKey.from_string('course-v1:edX+Cats+Dogs') @@ -62,6 +63,7 @@ class TestLinkProgramEnrollmentsMixin(object): return ProgramEnrollmentFactory.create( user=None, program_uuid=program_uuid, + curriculum_uuid=self.curriculum, external_user_key=external_user_key, ) @@ -274,19 +276,39 @@ class TestLinkProgramEnrollmentsErrors(TestLinkProgramEnrollmentsMixin, TestCase nonexistant_course = CourseKey.from_string('course-v1:edX+Zilch+Bupkis') program_enrollment_1 = self._create_waiting_enrollment(self.program, '0001') - self._create_waiting_course_enrollment(program_enrollment_1, nonexistant_course) - self._create_waiting_course_enrollment(program_enrollment_1, self.animal_course) + course_enrollment_1 = self._create_waiting_course_enrollment(program_enrollment_1, nonexistant_course) + course_enrollment_2 = self._create_waiting_course_enrollment(program_enrollment_1, self.animal_course) program_enrollment_2 = self._create_waiting_enrollment(self.program, '0002') - self._create_waiting_course_enrollment(program_enrollment_2, nonexistant_course) + self._create_waiting_course_enrollment(program_enrollment_2, self.fruit_course) self._create_waiting_course_enrollment(program_enrollment_2, self.animal_course) - msg_1 = COURSE_ENROLLMENT_ERR_TPL.format(user=self.user_1.username, course=nonexistant_course) - msg_2 = COURSE_ENROLLMENT_ERR_TPL.format(user=self.user_2.username, course=nonexistant_course) + msg = COURSE_ENROLLMENT_ERR_TPL.format(user=self.user_1.username, course=nonexistant_course) with LogCapture() as logger: self.call_command(self.program, ('0001', self.user_1.username), ('0002', self.user_2.username)) - logger.check_present((COMMAND_PATH, 'WARNING', msg_1)) - logger.check_present((COMMAND_PATH, 'WARNING', msg_2)) + logger.check_present((COMMAND_PATH, 'ERROR', msg)) - self._assert_user_enrolled_in_program_courses(self.user_1, self.program, self.animal_course) - self._assert_user_enrolled_in_program_courses(self.user_2, self.program, self.animal_course) + self._assert_no_program_enrollment(self.user_1, self.program) + self._assert_no_user(program_enrollment_1) + course_enrollment_1.refresh_from_db() + self.assertIsNone(course_enrollment_1.course_enrollment) + course_enrollment_2.refresh_from_db() + self.assertIsNone(course_enrollment_2.course_enrollment) + + self._assert_user_enrolled_in_program_courses(self.user_2, self.program, self.animal_course, self.fruit_course) + + def test_integrity_error(self): + existing_program_enrollment = self._create_waiting_enrollment(self.program, 'learner-0') + existing_program_enrollment.user = self.user_1 + existing_program_enrollment.save() + + program_enrollment_1 = self._create_waiting_enrollment(self.program, '0001') + self._create_waiting_enrollment(self.program, '0002') + + msg = 'Integrity error while linking program enrollments' + with LogCapture() as logger: + self.call_command(self.program, ('0001', self.user_1.username), ('0002', self.user_2.username)) + logger.check_present((COMMAND_PATH, 'ERROR', msg)) + + self._assert_no_user(program_enrollment_1) + self._assert_program_enrollment(self.user_2, self.program, '0002')