From 666a30f06c212a350c3bc84bd915f979602ab3da Mon Sep 17 00:00:00 2001 From: rahulkanneri-2u Date: Fri, 4 Sep 2026 19:24:46 +0530 Subject: [PATCH] Revert "fix: free retired learner's email on retirement completion" --- .../accounts/tests/test_retirement_views.py | 43 ------------ .../user_api/accounts/tests/test_utils.py | 60 ----------------- .../djangoapps/user_api/accounts/utils.py | 42 +----------- .../djangoapps/user_api/accounts/views.py | 17 +---- .../commands/free_retired_user_email.py | 44 ------------- .../tests/test_free_retired_user_email.py | 66 ------------------- openedx/core/djangoapps/user_api/toggles.py | 27 -------- 7 files changed, 3 insertions(+), 296 deletions(-) delete mode 100644 openedx/core/djangoapps/user_api/management/commands/free_retired_user_email.py delete mode 100644 openedx/core/djangoapps/user_api/management/tests/test_free_retired_user_email.py delete mode 100644 openedx/core/djangoapps/user_api/toggles.py diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index 6f1c41735760..8d9b963a6177 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py @@ -16,7 +16,6 @@ from django.test import TestCase from django.test.utils import CaptureQueriesContext from django.urls import reverse -from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys.edx.keys import CourseKey from rest_framework import status from social_django.models import UserSocialAuth @@ -69,7 +68,6 @@ UserRetirementPartnerReportingStatus, UserRetirementStatus ) -from openedx.core.djangoapps.user_api.toggles import FREE_RETIRED_LEARNER_EMAIL_ON_COMPLETION from openedx.core.djangolib.testing.utils import assert_redact_before_delete, skip_unless_lms from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -1320,47 +1318,6 @@ def test_moves(self, start_state, move_to_state, force, expected_response_code): self.update_and_assert_status(data, expected_response_code) - def test_reaching_complete_does_not_free_email_when_flag_disabled(self): - """ - The auto-free behavior is opt-in via FREE_RETIRED_LEARNER_EMAIL_ON_COMPLETION, - which defaults to disabled. - """ - data = {'new_state': 'COMPLETE', 'response': 'accountretirementcomplete', 'force': True} - self.update_and_assert_status(data) - - self.test_user.refresh_from_db() - assert self.test_user.email == self.retirement.original_email - - @override_waffle_flag(FREE_RETIRED_LEARNER_EMAIL_ON_COMPLETION, active=True) - def test_reaching_complete_frees_retired_email_when_flag_enabled(self): - """ - Moving a retirement to COMPLETE should free up the learner's retired - email address so it can be reused for a new registration. - """ - data = {'new_state': 'COMPLETE', 'response': 'accountretirementcomplete', 'force': True} - self.update_and_assert_status(data) - - self.test_user.refresh_from_db() - assert self.test_user.email == f'{self.retirement.original_email}.freed.{self.test_user.id}' - - @override_waffle_flag(FREE_RETIRED_LEARNER_EMAIL_ON_COMPLETION, active=True) - def test_reaching_complete_email_free_failure_fails_request(self): - """ - If freeing the email unexpectedly errors, the request should report - the failure rather than silently swallowing it, and the state - transition to COMPLETE should be rolled back so a retry isn't stuck - needing `force`. - """ - data = {'new_state': 'COMPLETE', 'response': 'accountretirementcomplete', 'force': True} - with mock.patch( - 'openedx.core.djangoapps.user_api.accounts.views.free_retired_learner_email', - side_effect=Exception('boom'), - ): - self.update_and_assert_status(data, status.HTTP_500_INTERNAL_SERVER_ERROR) - - retirement = UserRetirementStatus.objects.get(id=self.retirement.id) - assert retirement.current_state == self.pending_state - @skip_unless_lms class TestAccountRetirementPost(RetirementTestCase): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py index 793cebff0fcf..1ca8711958ef 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -7,10 +7,8 @@ from contextlib import contextmanager import ddt -import pytest from completion import models from completion.test_utils import CompletionWaffleTestMixin -from django.conf import settings from django.db import connection from django.db.models.signals import pre_delete from django.test import TestCase @@ -23,12 +21,10 @@ from openedx.core.djangoapps.user_api.accounts.signals import redact_social_auth_pii_before_deletion from openedx.core.djangoapps.user_api.accounts.utils import ( REDACTED_SOCIAL_AUTH_UID_PREFIX, - free_retired_learner_email, redact_and_delete_historical_social_auth, redact_and_delete_social_auth, retrieve_last_sitewide_block_completed, ) -from openedx.core.djangoapps.user_api.models import RetirementState, RetirementStateError from openedx.core.djangolib.testing.utils import assert_redact_before_delete, skip_unless_lms from xmodule.modulestore.tests.django_utils import ( SharedModuleStoreTestCase, # pylint: disable=wrong-import-order @@ -39,11 +35,6 @@ ) from ..utils import format_social_link, validate_social_link -from .retirement_helpers import ( # pylint: disable=unused-import - RetirementTestCase, - create_retirement_status, - setup_retirement_states -) # Use a context manager to guarantee signal reconnection between tests. @@ -305,54 +296,3 @@ def test_historical_social_auth_redact_before_delete(self): ) assert not self.historical_social_auth_model.objects.filter(user=self.user).exists() assert self.historical_social_auth_model.objects.filter(user=other_user).exists() - - -class FreeRetiredLearnerEmailTest(RetirementTestCase): - """ - Tests for free_retired_learner_email(). - """ - - def _retire_user_to_state(self, user, state_name): - return create_retirement_status(user, state=RetirementState.objects.get(state_name=state_name)) - - def test_frees_email_when_retirement_complete(self): - user = UserFactory(email='retired__user_abc123@retired.invalid') - self._retire_user_to_state(user, 'COMPLETE') - - free_retired_learner_email(user) - - user.refresh_from_db() - assert user.email == f'retired__user_abc123@retired.invalid.freed.{user.id}' - - def test_raises_when_retirement_still_in_progress(self): - user = UserFactory(email='retired__user_abc123@retired.invalid') - self._retire_user_to_state(user, 'RETIRING_LMS') - - with pytest.raises(RetirementStateError): - free_retired_learner_email(user) - - def test_is_idempotent(self): - user = UserFactory(email='retired__user_abc123@retired.invalid') - self._retire_user_to_state(user, 'COMPLETE') - - free_retired_learner_email(user) - user.refresh_from_db() - freed_email = user.email - - free_retired_learner_email(user) - user.refresh_from_db() - assert user.email == freed_email - - def test_frees_email_when_status_row_archived(self): - user = UserFactory(email=f'retired__user_abc123@{settings.RETIRED_EMAIL_DOMAIN}') - - free_retired_learner_email(user) - - user.refresh_from_db() - assert user.email.endswith(f'.freed.{user.id}') - - def test_raises_when_user_does_not_appear_retired(self): - user = UserFactory(email='still.active@example.com') - - with pytest.raises(RetirementStateError): - free_retired_learner_email(user) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index f62d2ed21a0d..dad1f9fcf543 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -27,7 +27,7 @@ from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from ..models import RetirementStateError, UserRetirementStatus +from ..models import UserRetirementStatus # Prefix and suffix used to build a per-record redacted uid for UserSocialAuth. REDACTED_SOCIAL_AUTH_UID_PREFIX = 'redacted-before-delete-' @@ -311,43 +311,3 @@ def handle_retirement_cancellation(retirement, email_address=None): retirement.user.save() retirement.delete() - - -def _is_retired_email_format(email): - """ - Returns True if the given email address is in the retired-email domain - used by settings.RETIRED_EMAIL_DOMAIN. - """ - return email.endswith(f'@{settings.RETIRED_EMAIL_DOMAIN}') - - -def free_retired_learner_email(user): - """ - Lets a fully-retired learner reuse their original email address by appending - a suffix to the retired-hash email currently on their auth_user row. This - only mutates that one column - the row and its retirement history are kept - for compliance, and is_email_retired() will no longer match the freed value. - - Raises RetirementStateError if the user's retirement isn't in a state where - it's safe to free the email (still in progress, or doesn't look retired at all). - """ - freed_suffix = f'.freed.{user.id}' - if user.email.endswith(freed_suffix): - LOGGER.info(f"Email for user {user.id} was already freed, nothing to do.") - return - - try: - retirement = UserRetirementStatus.objects.select_related('current_state').get(user=user) - if retirement.current_state.state_name != 'COMPLETE': - raise RetirementStateError( - f"Cannot free email for user {user.id}: retirement is in state " - f"'{retirement.current_state.state_name}', not COMPLETE." - ) - except UserRetirementStatus.DoesNotExist: - if not _is_retired_email_format(user.email): - raise RetirementStateError(f"User {user.id} does not appear to be a retired user.") # lint-amnesty, pylint: disable=raise-missing-from - - old_email = user.email - user.email = old_email + freed_suffix - user.save(update_fields=['email']) - LOGGER.info(f"Freed retired email for user {user.id}: '{old_email}' -> '{user.email}'.") diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 5fdb5e0f22a3..4b515dc8c496 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -82,7 +82,6 @@ UserRetirementPartnerReportingStatus, UserRetirementStatus, ) -from ..toggles import should_free_retired_learner_email_on_completion from .api import get_account_settings, update_account_settings from .permissions import ( CanCancelUserRetirement, @@ -98,11 +97,7 @@ UserSearchEmailSerializer, ) from .signals import USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC, USER_RETIRE_MAILINGS -from .utils import ( - create_retirement_request_and_deactivate_account, - free_retired_learner_email, - username_suffix_generator, -) +from .utils import create_retirement_request_and_deactivate_account, username_suffix_generator log = logging.getLogger(__name__) @@ -1012,15 +1007,7 @@ def partial_update(self, request): if retirement is None: raise UserRetirementStatus.DoesNotExist() - with transaction.atomic(): - retirement.update_state(request.data) - - if ( - retirement.current_state.state_name == 'COMPLETE' and - should_free_retired_learner_email_on_completion() - ): - free_retired_learner_email(retirement.user) - + retirement.update_state(request.data) return Response(status=status.HTTP_204_NO_CONTENT) except UserRetirementStatus.DoesNotExist: return Response(status=status.HTTP_404_NOT_FOUND) diff --git a/openedx/core/djangoapps/user_api/management/commands/free_retired_user_email.py b/openedx/core/djangoapps/user_api/management/commands/free_retired_user_email.py deleted file mode 100644 index 33882029bbc6..000000000000 --- a/openedx/core/djangoapps/user_api/management/commands/free_retired_user_email.py +++ /dev/null @@ -1,44 +0,0 @@ -""" -Frees a retired learner's email address so it can be reused for a new -registration, without touching the archived UserRetirementStatus row. -""" -import logging - -from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user -from django.core.management.base import BaseCommand, CommandError - -from openedx.core.djangoapps.user_api.accounts.utils import free_retired_learner_email -from openedx.core.djangoapps.user_api.models import RetirementStateError - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - """ - Implementation of the free_retired_user_email command. - """ - help = "Frees a retired learner's email address so it can be reused for a new registration." - - def add_arguments(self, parser): - parser.add_argument('--username', type=str, help='Username of the retired learner to free.') - parser.add_argument('--user_id', type=int, help='User ID of the retired learner to free.') - - def handle(self, *args, **options): - username = options['username'] - user_id = options['user_id'] - - if bool(username) == bool(user_id): - raise CommandError('Please provide exactly one of --username or --user_id.') - - try: - user = User.objects.get(username=username) if username else User.objects.get(id=user_id) - except User.DoesNotExist: - raise CommandError(f'No user found for username={username!r} user_id={user_id!r}.') # lint-amnesty, pylint: disable=raise-missing-from - - try: - free_retired_learner_email(user) - except RetirementStateError as exc: - raise CommandError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - - logger.info(f'Successfully freed email for user {user.id}.') - print(f'Successfully freed email for user {user.id}.') diff --git a/openedx/core/djangoapps/user_api/management/tests/test_free_retired_user_email.py b/openedx/core/djangoapps/user_api/management/tests/test_free_retired_user_email.py deleted file mode 100644 index 0670adb1976a..000000000000 --- a/openedx/core/djangoapps/user_api/management/tests/test_free_retired_user_email.py +++ /dev/null @@ -1,66 +0,0 @@ -""" -Test the free_retired_user_email management command -""" - - -import pytest -from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user -from django.core.management import CommandError, call_command - -from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import ( # pylint: disable=unused-import - create_retirement_status, - setup_retirement_states -) -from openedx.core.djangoapps.user_api.models import RetirementState -from common.djangoapps.student.tests.factories import UserFactory - -pytestmark = pytest.mark.django_db - - -def _retire_user(user, state_name): - return create_retirement_status(user, state=RetirementState.objects.get(state_name=state_name)) - - -def test_frees_email_by_username(setup_retirement_states, capsys): # pylint: disable=redefined-outer-name, unused-argument - user = UserFactory(email='retired__user_abc123@retired.invalid') - _retire_user(user, 'COMPLETE') - - call_command('free_retired_user_email', username=user.username) - - user.refresh_from_db() - assert user.email == f'retired__user_abc123@retired.invalid.freed.{user.id}' - assert 'Successfully freed email' in capsys.readouterr().out - - -def test_frees_email_by_user_id(setup_retirement_states): # pylint: disable=redefined-outer-name, unused-argument - user = UserFactory(email='retired__user_abc123@retired.invalid') - _retire_user(user, 'COMPLETE') - - call_command('free_retired_user_email', user_id=user.id) - - user.refresh_from_db() - assert user.email.endswith(f'.freed.{user.id}') - - -def test_requires_exactly_one_identifier(): - with pytest.raises(CommandError, match=r'exactly one of --username or --user_id'): - call_command('free_retired_user_email') - - with pytest.raises(CommandError, match=r'exactly one of --username or --user_id'): - call_command('free_retired_user_email', username='someone', user_id=1) - - -def test_unknown_user(): - with pytest.raises(CommandError, match=r'No user found'): - call_command('free_retired_user_email', username='nonexistent') - - -def test_blocked_while_retirement_in_progress(setup_retirement_states): # pylint: disable=redefined-outer-name, unused-argument - user = UserFactory(email='retired__user_abc123@retired.invalid') - _retire_user(user, 'RETIRING_LMS') - - with pytest.raises(CommandError, match=r'not COMPLETE'): - call_command('free_retired_user_email', username=user.username) - - user.refresh_from_db() - assert User.objects.get(id=user.id).email == 'retired__user_abc123@retired.invalid' diff --git a/openedx/core/djangoapps/user_api/toggles.py b/openedx/core/djangoapps/user_api/toggles.py deleted file mode 100644 index 74c20bbfb6bf..000000000000 --- a/openedx/core/djangoapps/user_api/toggles.py +++ /dev/null @@ -1,27 +0,0 @@ -""" -Toggles for the user_api app. -""" - -from edx_toggles.toggles import WaffleFlag - -# .. toggle_name: user_api.free_retired_learner_email_on_completion -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: When enabled, a learner's retired email address is automatically freed -# (see free_retired_learner_email) as soon as their retirement reaches the COMPLETE state via -# PATCH /api/user/v1/accounts/update_retirement_status/. This lets the behavior be turned off -# without pausing the retirement pipeline itself; the free_retired_user_email management command -# is unaffected by this toggle. -# .. toggle_use_cases: opt_in -# .. toggle_creation_date: 2026-09-04 -FREE_RETIRED_LEARNER_EMAIL_ON_COMPLETION = WaffleFlag( - 'user_api.free_retired_learner_email_on_completion', __name__ -) - - -def should_free_retired_learner_email_on_completion(): - """ - Returns True if a learner's retired email should be automatically freed - when their retirement reaches the COMPLETE state. - """ - return FREE_RETIRED_LEARNER_EMAIL_ON_COMPLETION.is_enabled()