Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Update bulk_rehash_retired_usernames

Browse Source 
- Handle forums 404s
- Update UserRetirementStatus whether or not the user has completed retirement
This commit is contained in:
bmedx
2018-08-20 13:25:57 -04:00
parent 1082bedb10
commit 745aadbcd5
2 changed files with 68 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py
View File

@@ -50,20 +50,10 @@ class Command(BaseCommand):
settings.RETIRED_USER_SALTS,
settings.RETIRED_USERNAME_FMT
)
# Sanity check:
if retirement.user.username != old_retired_username:
print(
'WARNING: Skipping UserRetirementStatus ID {} / User ID {} because the user does not appear to '
'have a retired username: {} != {}.'.format(
retirement.id,
retirement.user.id,
retirement.user.username,
old_retired_username
)
)
# If the original username was already normalized (or all lowercase), the old and new hashes would
# match:
elif old_retired_username == new_retired_username:
if old_retired_username == new_retired_username:
print(
'Skipping UserRetirementStatus ID {} / User ID {} because the hash would not change.'.format(
retirement.id,
@@ -88,12 +78,26 @@ class Command(BaseCommand):
# as the local db updates and can be slow, so keeping it
# outside to cut down on potential deadlocks.
cc_user = comment_client.User.from_django_user(retirement.user)
cc_user.retire(new_retired_username)
# The user may not exist in forums, if it doesn't that's not
# an error.
try:
cc_user.retire(new_retired_username)
except comment_client.utils.CommentClientRequestError as e:
if e.status_code != 404:
print(
'UserRetirementStatus ID {} User ID {} failed to rename in forums: {}'.format(
retirement.id, retirement.user.id, text_type(e)
)
)
raise
# Update and save both the user table and retirement queue table:
with transaction.atomic():
retirement.user.username = new_retired_username
retirement.user.save()
# Only rename them in auth_user if they've already been retired
if retirement.user.username == old_retired_username:
retirement.user.username = new_retired_username
retirement.user.save()
retirement.retired_username = new_retired_username
retirement.save()
except Exception as exc: # pylint: disable=broad-except

66
openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py
View File

@@ -8,6 +8,7 @@ from django.conf import settings
from django.core.management import call_command
from user_util.user_util import get_retired_username
from lms.lib import comment_client
from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import (
setup_retirement_states, fake_completed_retirement
)
@@ -21,16 +22,14 @@ pytestmark = pytest.mark.django_db
def _setup_users():
"""
Creates and returns test users in the different states of needing rehash:
- Skipped: has not yet been retired
- Faked: has been fake-retired, but the retired username does not require updating
- Needing rehash: has been fake-retired and name changed so it triggers a hash update
- Skipped: The retired username does not require updating, some of these are fake retired
- Needing rehash: Has been fake-retired and name changed so it triggers a hash update
"""
# When we loop through creating users, take additional action on these
user_indexes_to_be_fake_retired = (2, 4, 6, 8, 10)
user_indexes_to_be_rehashed = (4, 6)
users_skipped = []
users_faked = []
users_needing_rehash = []
retirements = {}
@@ -44,19 +43,18 @@ def _setup_users():
fake_completed_retirement(user)
if i in user_indexes_to_be_rehashed:
# In order to need a rehash user.username must be the same as
# retirement.retired_username and NOT the same as the hash
# generated when the script is run. So we force that here.
# In order to need a rehash user.username the new hash must be
# different, we force that here.
retirement.retired_username = retirement.retired_username.upper()
user.username = retirement.retired_username
retirement.save()
user.save()
users_needing_rehash.append(user)
else:
users_faked.append(user)
users_skipped.append(user)
else:
users_skipped.append(user)
return users_skipped, users_faked, users_needing_rehash, retirements
return users_skipped, users_needing_rehash, retirements
@skip_unless_lms
@@ -66,7 +64,7 @@ def test_successful_rehash(retire_user_forums, capsys):
"""
Run the command with users of all different hash statuses, expect success
"""
users_skipped, users_faked, users_needing_rehash, retirements = _setup_users()
users_skipped, users_needing_rehash, retirements = _setup_users()
call_command('bulk_rehash_retired_usernames')
output = capsys.readouterr().out
@@ -75,9 +73,6 @@ def test_successful_rehash(retire_user_forums, capsys):
assert retire_user_forums.call_count == 2
for user in users_skipped:
assert "User ID {} because the user does not appear to have a retired username:".format(user.id) in output
for user in users_faked:
assert "User ID {} because the hash would not change.".format(user.id) in output
expected_username_calls = []
@@ -106,7 +101,7 @@ def test_forums_failed(retire_user_forums, capsys):
"""
Run the command with users of all different hash statuses, expect success
"""
users_skipped, users_faked, users_needing_rehash, retirements = _setup_users()
users_skipped, users_needing_rehash, retirements = _setup_users()
retire_user_forums.side_effect = Exception('something bad happened with forums')
call_command('bulk_rehash_retired_usernames')
@@ -116,9 +111,6 @@ def test_forums_failed(retire_user_forums, capsys):
assert retire_user_forums.call_count == 2
for user in users_skipped:
assert "User ID {} because the user does not appear to have a retired username:".format(user.id) in output
for user in users_faked:
assert "User ID {} because the hash would not change.".format(user.id) in output
expected_username_calls = []
@@ -140,3 +132,43 @@ def test_forums_failed(retire_user_forums, capsys):
assert "FAILED! 2 retirements failed to rehash. Retirement IDs:" in output
retire_user_forums.assert_has_calls(expected_username_calls)
@skip_unless_lms
@pytest.mark.usefixtures("setup_retirement_states")
@patch('lms.lib.comment_client.User.retire')
def test_forums_404(retire_user_forums, capsys):
"""
Run the command with users of all different hash statuses, expect success
"""
users_skipped, users_needing_rehash, retirements = _setup_users()
retire_user_forums.side_effect = comment_client.utils.CommentClientRequestError('not found', status_codes=404)
call_command('bulk_rehash_retired_usernames')
output = capsys.readouterr().out
# Make sure forums was called the correct number of times
assert retire_user_forums.call_count == 2
for user in users_skipped:
assert "User ID {} because the hash would not change.".format(user.id) in output
expected_username_calls = []
for user in users_needing_rehash:
retirement = retirements[user.id]
user.refresh_from_db()
retirement.refresh_from_db()
new_retired_username = get_retired_username(
retirement.original_username,
settings.RETIRED_USER_SALTS,
settings.RETIRED_USERNAME_FMT
)
expected_username_calls.append(call(new_retired_username))
assert "User ID {} to rehash their retired username".format(user.id) in output
# Confirm that the usernames *are* updated, since this is a non-blocking forums error
assert new_retired_username == user.username
assert new_retired_username == retirement.retired_username
assert "Success!" in output
retire_user_forums.assert_has_calls(expected_username_calls)