From 8c93e424c0cdf0e97ec6e966a0ab9d734e784a72 Mon Sep 17 00:00:00 2001 From: bmedx Date: Fri, 10 Aug 2018 14:51:01 -0400 Subject: [PATCH] Add forums renaming to bulk username rehash command --- .../commands/bulk_rehash_retired_usernames.py | 47 ++++++++++++-- .../test_bulk_rehash_retired_usernames.py | 64 ++++++++++++++++++- 2 files changed, 101 insertions(+), 10 deletions(-) diff --git a/openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py b/openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py index 68ec4037b7..cc1ecf1653 100644 --- a/openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py +++ b/openedx/core/djangoapps/user_api/management/commands/bulk_rehash_retired_usernames.py @@ -12,9 +12,13 @@ command likely cannot be re-used in the future because eventually we will need to clean out the UserRetirementStatus table. """ from __future__ import print_function + from django.conf import settings from django.db import transaction from django.core.management.base import BaseCommand +from six import text_type + +from lms.lib import comment_client from openedx.core.djangoapps.user_api.models import UserRetirementStatus from user_util import user_util @@ -36,6 +40,8 @@ class Command(BaseCommand): """ dry_run = options['dry_run'] retirements = UserRetirementStatus.objects.all().select_related('user') + + failed_retirements = [] for retirement in retirements: original_username = retirement.original_username old_retired_username = retirement.retired_username @@ -64,7 +70,7 @@ class Command(BaseCommand): retirement.user.id, ) ) - # Found an username to update: + # Found an username to update else: print( 'Updating UserRetirementStatus ID {} / User ID {} ' @@ -76,9 +82,36 @@ class Command(BaseCommand): ) ) if not dry_run: - # Update and save both the user table and retirement queue table: - with transaction.atomic(): - retirement.user.username = new_retired_username - retirement.user.save() - retirement.retired_username = new_retired_username - retirement.save() + try: + # Update the forums first, that way if it fails the user can + # be re-run. It does not need to be in the same transaction, + # 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) + + # Update and save both the user table and retirement queue table: + with transaction.atomic(): + 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 + print( + 'UserRetirementStatus ID {} User ID {} failed rename'.format( + retirement.id, retirement.user.id + ) + ) + print(text_type(exc)) + failed_retirements.append(retirement) + + if failed_retirements: + print('------------------------------------------------------------') + print( + 'FAILED! {} retirements failed to rehash. Retirement IDs:\n{}'.format( + len(failed_retirements), + '\n'.join([text_type(r.id) for r in failed_retirements]) + ) + ) + else: + print('Success! {} retirements examined.'.format(len(retirements))) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py b/openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py index 90d949db2c..6164a7f25c 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_bulk_rehash_retired_usernames.py @@ -1,14 +1,16 @@ """ Test the bulk_rehash_retired_usernames management command """ +from mock import call, patch import pytest +from django.conf import settings from django.core.management import call_command +from user_util.user_util import get_retired_username from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import RetirementTestCase, fake_retirement from openedx.core.djangoapps.user_api.models import UserRetirementStatus from openedx.core.djangolib.testing.utils import skip_unless_lms -from student.models import get_retired_username_by_username from student.tests.factories import UserFactory pytestmark = pytest.mark.django_db @@ -56,27 +58,83 @@ def _setup_users(): @skip_unless_lms -def test_successful_rehash(capsys): +@patch('lms.lib.comment_client.User.retire') +def test_successful_rehash(retire_user_forums, capsys): """ Run the command with users of all different hash statuses, expect success """ RetirementTestCase.setup_states() users_skipped, users_faked, users_needing_rehash, retirements = _setup_users() + 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 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 = [] for user in users_needing_rehash: retirement = retirements[user.id] user.refresh_from_db() retirement.refresh_from_db() - new_retired_username = get_retired_username_by_username(retirement.original_username) + 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 assert new_retired_username == user.username assert new_retired_username == retirement.retired_username + + retire_user_forums.assert_has_calls(expected_username_calls) + + +@skip_unless_lms +@patch('lms.lib.comment_client.User.retire') +def test_forums_failed(retire_user_forums, capsys): + """ + Run the command with users of all different hash statuses, expect success + """ + RetirementTestCase.setup_states() + users_skipped, users_faked, users_needing_rehash, retirements = _setup_users() + retire_user_forums.side_effect = Exception('something bad happened with forums') + + 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 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 = [] + 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 *not* updated, due to the forums error + assert new_retired_username != user.username + assert new_retired_username != retirement.retired_username + + assert "FAILED! 2 retirements failed to rehash. Retirement IDs:" in output + retire_user_forums.assert_has_calls(expected_username_calls)