diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 130e09b9e5..6bf764268d 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -2168,7 +2168,7 @@ class ManualEnrollmentAudit(models.Model): @classmethod def get_manual_enrollment(cls, enrollment): """ - if matches returns the most recent entry in the table filtered by enrollment else returns None, + Returns the most recent entry for the given enrollment, or None if there are no matches """ try: manual_enrollment = cls.objects.filter(enrollment=enrollment).latest('time_stamp') @@ -2177,12 +2177,17 @@ class ManualEnrollmentAudit(models.Model): return manual_enrollment @classmethod - def retire_manual_enrollments(cls, enrollments, retired_email): + def retire_manual_enrollments(cls, user, retired_email): """ - Removes PII (enrolled_email and reason) from any rows corresponding to - the enrollment passed in. Bubbles up any exceptions. + Removes PII (enrolled_email and reason) associated with the User passed in. Bubbles up any exceptions. """ - return cls.objects.filter(enrollment__in=enrollments).update(reason="", enrolled_email=retired_email) + # This bit of ugliness is to fix a perfmance issue with Django using a slow + # sub-select that caused the original query to take several seconds (PLAT-2371). + # It is possible that this could also be bad if a user has thousands of manual + # enrollments, but currently that number tends to be very low. + manual_enrollment_ids = list(cls.objects.filter(enrollment__user=user).values_list('id', flat=True)) + + return cls.objects.filter(id__in=manual_enrollment_ids).update(reason="", enrolled_email=retired_email) class CourseEnrollmentAllowed(DeletableByUserValue, models.Model): diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 6b34bb7e5e..ee9f4e1d01 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -375,8 +375,7 @@ class TestManualEnrollmentAudit(SharedModuleStoreTestCase): ) self.assertTrue(ManualEnrollmentAudit.objects.filter(enrollment=enrollment).exists()) # retire the ManualEnrollmentAudit objects associated with the above enrollments - enrollments = CourseEnrollment.objects.filter(user=self.user) - ManualEnrollmentAudit.retire_manual_enrollments(enrollments=enrollments, retired_email="xxx") + ManualEnrollmentAudit.retire_manual_enrollments(user=self.user, retired_email="xxx") self.assertTrue(ManualEnrollmentAudit.objects.filter(enrollment=enrollment).exists()) self.assertFalse(ManualEnrollmentAudit.objects.filter(enrollment=enrollment).exclude( enrolled_email="xxx" diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index de003f460a..e5d35b955e 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -866,8 +866,7 @@ class LMSAccountRetirementView(ViewSet): ArticleRevision.retire_user(retirement.user) PendingNameChange.delete_by_user_value(retirement.user, field='user') PasswordHistory.retire_user(retirement.user.id) - course_enrollments = CourseEnrollment.objects.filter(user=retirement.user) - ManualEnrollmentAudit.retire_manual_enrollments(course_enrollments, retirement.retired_email) + ManualEnrollmentAudit.retire_manual_enrollments(retirement.user, retirement.retired_email) CreditRequest.retire_user(retirement) ApiAccessRequest.retire_user(retirement.user)