feat: simplify retirement cleanup by removing custom redaction value parameters

openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py

@@ -1072,14 +1072,11 @@ def cleanup_and_assert_status(self, data=None, expected_status=status.HTTP_204_N
         assert response.status_code == expected_status
         return response
 
-    def _assert_redacted_update_delete_queries(self, queries, redacted_username, redacted_email, redacted_name):
+    def _assert_redacted_update_delete_queries(self, queries):
         """
         Helper method to verify UPDATE and DELETE queries use ID-based filtering and correct field-value assignments.
         Args:
             queries: List of captured query dicts from CaptureQueriesContext
-            redacted_username: Expected redacted username value
-            redacted_email: Expected redacted email value
-            redacted_name: Expected redacted name value
         """
         update_queries = [q for q in queries if 'UPDATE' in q['sql'] and 'user_api_userretirementstatus' in q['sql']]
         delete_queries = [q for q in queries if 'DELETE' in q['sql'] and 'user_api_userretirementstatus' in q['sql']]
@@ -1090,14 +1087,14 @@ def _assert_redacted_update_delete_queries(self, queries, redacted_username, red
         update_query = update_queries[0]
         sql_lower = update_query['sql']
         # Ensure original_username, original_email, and original_name are set to redacted values
-        assert f'"original_username" = \'{redacted_username}\'' in sql_lower, (
-            f"UPDATE query missing '\"original_username\" = {redacted_username}': {sql_lower}"
+        assert '"original_username" = \'redacted\'' in sql_lower, (
+            f"UPDATE query missing '\"original_username\" = redacted': {sql_lower}"
         )
-        assert f'"original_email" = \'{redacted_email}\'' in sql_lower, (
-            f"UPDATE query missing '\"original_email\" = {redacted_email}': {sql_lower}"
+        assert '"original_email" = \'redacted\'' in sql_lower, (
+            f"UPDATE query missing '\"original_email\" = redacted': {sql_lower}"
         )
-        assert f'"original_name" = \'{redacted_name}\'' in sql_lower, (
-            f"UPDATE query missing '\"original_name\" = {redacted_name}': {sql_lower}"
+        assert '"original_name" = \'redacted\'' in sql_lower, (
+            f"UPDATE query missing '\"original_name\" = redacted': {sql_lower}"
         )
         # Ensure UPDATE uses ID-based filtering
         assert '"id" IN' in sql_lower or 'WHERE "id"' in sql_lower, (
@@ -1113,9 +1110,9 @@ def _assert_redacted_update_delete_queries(self, queries, redacted_username, red
             f"DELETE query should use ID filtering to prevent over-deletion, but got: {sql_lower}"
         )
 
-    def test_default_redacted_values(self):
+    def test_redact_and_delete(self):
         """
-        Test basic cleanup with default redacted values.
+        Test basic cleanup with redacted values.
         Verify that redaction (UPDATE) happens before deletion (DELETE).
         Captures actual SQL queries to ensure UPDATE queries contain correct field-value assignments.
         """
@@ -1126,33 +1123,8 @@ def test_default_redacted_values(self):
         retirements = UserRetirementStatus.objects.all()
         assert retirements.count() == 0
 
-        # Verify UPDATE and DELETE queries with default 'redacted' value
-        self._assert_redacted_update_delete_queries(context.captured_queries, 'redacted', 'redacted', 'redacted')
-
-    def test_custom_redacted_values(self):
-        """Test that custom redacted values are applied before deletion."""
-        custom_username = 'username-redacted-12345'
-        custom_email = 'email-redacted-67890'
-        custom_name = 'name-redacted-abcde'
-
-        data = {
-            'usernames': self.usernames,
-            'redacted_username': custom_username,
-            'redacted_email': custom_email,
-            'redacted_name': custom_name
-        }
-
-        with CaptureQueriesContext(connection) as context:
-            self.cleanup_and_assert_status(data=data)
-
-        # Verify records are deleted after redaction
-        retirements = UserRetirementStatus.objects.all()
-        assert retirements.count() == 0
-
-        # Verify UPDATE and DELETE queries with custom redacted values
-        self._assert_redacted_update_delete_queries(
-            context.captured_queries, custom_username, custom_email, custom_name
-        )
+        # Verify UPDATE and DELETE queries with 'redacted' value
+        self._assert_redacted_update_delete_queries(context.captured_queries)
 
     def test_does_not_delete_unrelated_redacted_records(self):
         """

openedx/core/djangoapps/user_api/accounts/views.py

@@ -1024,20 +1024,14 @@ def cleanup(self, request):
 
         ```
         {
-            'usernames': ['user1', 'user2', ...],
-            'redacted_username': 'Value to store in username field',
-            'redacted_email': 'Value to store in email field',
-            'redacted_name': 'Value to store in name field'
+            'usernames': ['user1', 'user2', ...]
         }
         ```
 
         Redacts and then deletes a batch of retirement requests by username.
         """
         try:
             usernames = request.data["usernames"]
-            redacted_username = request.data.get("redacted_username", "redacted")
-            redacted_email = request.data.get("redacted_email", "redacted")
-            redacted_name = request.data.get("redacted_name", "redacted")
 
             if not isinstance(usernames, list):
                 raise TypeError("Usernames should be an array.")
@@ -1058,9 +1052,9 @@ def cleanup(self, request):
             retirement_ids = list(retirements.values_list('id', flat=True))
             # Update by IDs
             UserRetirementStatus.objects.filter(id__in=retirement_ids).update(
-                original_username=redacted_username,
-                original_email=redacted_email,
-                original_name=redacted_name
+                original_username="redacted",
+                original_email="redacted",
+                original_name="redacted"
             )
             # Delete by IDs
             UserRetirementStatus.objects.filter(id__in=retirement_ids, current_state=complete_state).delete()

scripts/user_retirement/retirement_archive_and_cleanup.py

@@ -196,17 +196,14 @@ def _archive_retirements_or_exit(config, learners, dry_run=False):
         FAIL_EXCEPTION(ERR_ARCHIVING, 'Unexpected error occurred archiving retirements!', exc)
 
 
-def _cleanup_retirements_or_exit(config, learners, redacted_username='redacted',
-                                 redacted_email='redacted', redacted_name='redacted'):
+def _cleanup_retirements_or_exit(config, learners):
     """
     Bulk deletes the retirements for this run after redacting PII fields
     """
     LOG('Cleaning up retirements for {} learners'.format(len(learners)))  # noqa: UP032
     try:
         usernames = [l['original_username'] for l in learners]
-        config['LMS'].bulk_cleanup_retirements(
-            usernames, redacted_username, redacted_email, redacted_name
-        )
+        config['LMS'].bulk_cleanup_retirements(usernames)
     except Exception as exc:  # pylint: disable=broad-except
         FAIL_EXCEPTION(ERR_DELETING, 'Unexpected error occurred redacting/deleting retirements!', exc)
 
@@ -262,26 +259,7 @@ def _get_utc_now():
     help='Number of user retirements to process',
     type=int
 )
-@click.option(
-    '--redacted_username',
-    help='Value to use for redacted username field',
-    type=str,
-    default='redacted'
-)
-@click.option(
-    '--redacted_email',
-    help='Value to use for redacted email field',
-    type=str,
-    default='redacted'
-)
-@click.option(
-    '--redacted_name',
-    help='Value to use for redacted name field',
-    type=str,
-    default='redacted'
-)
-def archive_and_cleanup(config_file, cool_off_days, dry_run, start_date, end_date, batch_size,
-                        redacted_username, redacted_email, redacted_name):
+def archive_and_cleanup(config_file, cool_off_days, dry_run, start_date, end_date, batch_size):
     """
     Cleans up UserRetirementStatus rows in LMS by:
     1- Getting all rows currently in COMPLETE that were created --cool_off_days ago or more,
@@ -336,9 +314,7 @@ def archive_and_cleanup(config_file, cool_off_days, dry_run, start_date, end_dat
                 if dry_run:
                     LOG('This is a dry-run. Exiting before any retirements are cleaned up')
                 else:
-                    _cleanup_retirements_or_exit(
-                        config, batch, redacted_username, redacted_email, redacted_name
-                    )
+                    _cleanup_retirements_or_exit(config, batch)
                     LOG('Archive and cleanup complete for batch #{}'.format(str(index + 1)))  # noqa: UP032
                     time.sleep(DELAY)
         else:

scripts/user_retirement/tests/test_retirement_archive_and_cleanup.py

@@ -28,8 +28,7 @@
 FAKE_BUCKET_NAME = "fake_test_bucket"
 
 
-def _call_script(cool_off_days=37, batch_size=None, dry_run=None, start_date=None, end_date=None,
-                 redacted_username=None, redacted_email=None, redacted_name=None):
+def _call_script(cool_off_days=37, batch_size=None, dry_run=None, start_date=None, end_date=None):
     """
     Call the archive script with the given params and a generic config file.
     Returns the CliRunner.invoke results
@@ -51,12 +50,6 @@ def _call_script(cool_off_days=37, batch_size=None, dry_run=None, start_date=Non
             base_args += ['--start_date', start_date]
         if end_date:
             base_args += ['--end_date', end_date]
-        if redacted_username:
-            base_args += ['--redacted_username', redacted_username]
-        if redacted_email:
-            base_args += ['--redacted_email', redacted_email]
-        if redacted_name:
-            base_args += ['--redacted_name', redacted_name]
 
         result = runner.invoke(archive_and_cleanup, args=base_args)
     print(result)
@@ -113,7 +106,7 @@ def test_successful(*args, **kwargs):
     assert mock_get_access_token.call_count == 1
     mock_get_learners.assert_called_once()
     mock_bulk_cleanup_retirements.assert_called_once_with(
-        ['test1', 'test2', 'test3'], 'redacted', 'redacted', 'redacted')
+        ['test1', 'test2', 'test3'])
 
     assert result.exit_code == 0
     assert 'Archive and cleanup complete' in result.output
@@ -141,44 +134,14 @@ def test_successful_with_batching(*args, **kwargs):
     # Called once to get the LMS token
     assert mock_get_access_token.call_count == 1
     mock_get_learners.assert_called_once()
-    get_learner_calls = [call(['test1', 'test2'], 'redacted', 'redacted', 'redacted'),
-                         call(['test3'], 'redacted', 'redacted', 'redacted')]
+    get_learner_calls = [call(['test1', 'test2']),
+                         call(['test3'])]
     mock_bulk_cleanup_retirements.assert_has_calls(get_learner_calls)
 
     assert result.exit_code == 0
     assert 'Archive and cleanup complete for batch #1' in result.output
     assert 'Archive and cleanup complete for batch #2' in result.output
 
-
-@patch('scripts.user_retirement.utils.edx_api.BaseApiClient.get_access_token', return_value=('THIS_IS_A_JWT', None))
-@patch.multiple(
-    'scripts.user_retirement.utils.edx_api.LmsApi',
-    get_learners_by_date_and_status=DEFAULT,
-    bulk_cleanup_retirements=DEFAULT
-)
-@mock_aws
-def test_successful_with_custom_redaction_values(*args, **kwargs):
-    conn = boto3.resource('s3')
-    conn.create_bucket(Bucket=FAKE_BUCKET_NAME)
-
-    mock_get_access_token = args[0]
-    mock_get_learners = kwargs['get_learners_by_date_and_status']
-    mock_bulk_cleanup_retirements = kwargs['bulk_cleanup_retirements']
-
-    mock_get_learners.return_value = fake_learners_to_retire()
-
-    result = _call_script(
-        redacted_username='custom_user',
-        redacted_email='custom@example.com',
-        redacted_name='Custom Name'
-    )
-
-    # Called once to get the LMS token
-    assert mock_get_access_token.call_count == 1
-    mock_get_learners.assert_called_once()
-    mock_bulk_cleanup_retirements.assert_called_once_with(
-        ['test1', 'test2', 'test3'], 'custom_user', 'custom@example.com', 'Custom Name')
-
     assert result.exit_code == 0
     assert 'Archive and cleanup complete' in result.output
 

scripts/user_retirement/utils/edx_api.py

@@ -352,19 +352,11 @@ def retirement_retire_proctoring_backend_data(self, learner):
         return self._request("POST", api_url)
 
     @_retry_lms_api()
-    def bulk_cleanup_retirements(self, usernames, redacted_username=None,
-                                 redacted_email=None, redacted_name=None):
+    def bulk_cleanup_retirements(self, usernames):
         """
         Redacts and then deletes the retirements for all given usernames.
-        Optionally pass caller-defined redacted values for each PII field before deletion.
         """
         data = {'usernames': usernames}
-        if redacted_username is not None:
-            data['redacted_username'] = redacted_username
-        if redacted_email is not None:
-            data['redacted_email'] = redacted_email
-        if redacted_name is not None:
-            data['redacted_name'] = redacted_name
         api_url = self.get_api_url("api/user/v1/accounts/retirement_cleanup")
         return self._request("POST", api_url, json=data)