From c4ee7dacbce38ab607305c8579f7d5d0e567d5be Mon Sep 17 00:00:00 2001 From: bmedx Date: Wed, 8 Aug 2018 09:44:24 -0400 Subject: [PATCH] Update retirement to handle multiple UserRetirementStatus rows returned --- .../djangoapps/user_api/accounts/views.py | 41 +++++++++++++++++-- openedx/core/djangoapps/user_api/models.py | 17 +++++++- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 0d0d26c7cb..6980a737b0 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -624,9 +624,23 @@ class AccountRetirementPartnerReportView(ViewSet): original_username__in=usernames ) - if len(usernames) != len(retirement_statuses): + # Need to de-dupe usernames that differ only by case to find the exact right match + retirement_statuses_clean = [rs for rs in retirement_statuses if rs.original_username in usernames] + + # During a narrow window learners were able to re-use a username that had been retired if + # they altered the capitalization of one or more characters. Therefore we can have more + # than one row returned here (due to our MySQL collation being case-insensitive), and need + # to disambiguate them in Python, which will respect case in the comparison. + if len(usernames) != len(retirement_statuses_clean): return Response( - '{} original_usernames given, only {} found!'.format(len(usernames), len(retirement_statuses)), + '{} original_usernames given, {} found!\n' + 'Given usernames:\n{}\n' + 'Found UserRetirementReportingStatuses:\n{}'.format( + len(usernames), + len(retirement_statuses_clean), + usernames, + ', '.join([rs.original_username for rs in retirement_statuses_clean]) + ), status=status.HTTP_400_BAD_REQUEST ) @@ -765,7 +779,28 @@ class AccountRetirementStatusView(ViewSet): """ try: username = request.data['username'] - retirement = UserRetirementStatus.objects.get(original_username=username) + retirements = UserRetirementStatus.objects.filter(original_username=username) + + # During a narrow window learners were able to re-use a username that had been retired if + # they altered the capitalization of one or more characters. Therefore we can have more + # than one row returned here (due to our MySQL collation being case-insensitive), and need + # to disambiguate them in Python, which will respect case in the comparison. + retirement = None + if len(retirements) < 1: + raise UserRetirementStatus.DoesNotExist() + elif len(retirements) > 1: + for r in retirements: + if r.original_username == username: + retirement = r + break + # UserRetirementStatus was found, but it was the wrong case. + if retirement is None: + raise UserRetirementStatus.DoesNotExist() + else: + if username != retirements[0].original_username: + raise UserRetirementStatus.DoesNotExist() + retirement = retirements[0] + retirement.update_state(request.data) return Response(status=status.HTTP_204_NO_CONTENT) except UserRetirementStatus.DoesNotExist: diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index dca0eb6aba..882a3ef508 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -335,7 +335,22 @@ class UserRetirementStatus(TimeStampedModel): Can raise UserRetirementStatus.DoesNotExist or RetirementStateError, otherwise should return a UserRetirementStatus """ - retirement = cls.objects.get(original_username=username) + # During a narrow window learners were able to re-use a username that had been retired if + # they altered the capitalization of one or more characters. Therefore we can have more + # than one row returned here (due to our MySQL collation being case-insensitive), and need + # to disambiguate them in Python, which will respect case in the comparison. + retirements = cls.objects.filter(original_username=username) + + retirement = None + for r in retirements: + if r.original_username == username: + retirement = r + break + + if retirement is None: + raise UserRetirementStatus.DoesNotExist('{} does not have an exact match in UserRetirementStatus. ' + '{} similar rows found.'.format(username, len(retirements))) + state = retirement.current_state if state.required or state.state_name.endswith('_COMPLETE'):