Add forums renaming to bulk username rehash command

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)))

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)