make link_program_enrollments atomic per user (#21439)
* make link_program_enrollments atomic per user
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user