feat: redact PII fields before delete in user retirement flows

common/djangoapps/student/models/course_enrollment.py

@@ -1597,7 +1597,9 @@ class CourseEnrollmentAllowed(DeletableByUserValue, models.Model):
     the object is marked with the student who enrolled, to prevent students from changing e-mails and
     enrolling many accounts through the same e-mail.
 
-    .. no_pii:
+    .. pii: The email field stores PII.
+    .. pii_types: email_address
+    .. pii_retirement: local_api
     """
     email = models.CharField(max_length=255, db_index=True)
     course_id = CourseKeyField(max_length=255, db_index=True)
@@ -1646,6 +1648,13 @@ def may_enroll_and_unenrolled(cls, course_id):
         """
         return CourseEnrollmentAllowed.objects.filter(course_id=course_id, user__isnull=True)
 
+    @classmethod
+    def redact_before_delete_fields(cls):
+        """
+        Redact email before deleting records for downstream soft-delete systems.
+        """
+        return {'email': 'redacted@retired.invalid'}
+
 
 class CourseEnrollmentAttribute(models.Model):
     """

common/djangoapps/student/models/user.py

@@ -904,14 +904,21 @@ class PendingEmailChange(DeletableByUserValue, models.Model):  # noqa: DJ008
     """
     This model keeps track of pending requested changes to a user's email address.
 
-    .. pii: Contains new_email, retired in AccountRetirementView
+    .. pii: Contains new_email, redacted then deleted in AccountRetirementView
     .. pii_types: email_address
     .. pii_retirement: local_api
     """
     user = models.OneToOneField(User, unique=True, db_index=True, on_delete=models.CASCADE)
     new_email = models.CharField(blank=True, max_length=255, db_index=True)
     activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True)
 
+    @classmethod
+    def redact_before_delete_fields(cls):
+        """
+        Redact PII fields before delete in downstream soft-delete systems.
+        """
+        return {'new_email': 'redacted@retired.invalid'}
+
     def request_change(self, email):
         """Request a change to a user's email.
 

common/djangoapps/student/tests/test_models.py

@@ -10,8 +10,10 @@
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser, User  # pylint: disable=imported-auth-user
 from django.core.cache import cache
+from django.db import connection
 from django.db.models.functions import Lower
 from django.test import TestCase, override_settings
+from django.test.utils import CaptureQueriesContext
 from edx_toggles.toggles.testutils import override_waffle_flag
 from freezegun import freeze_time
 from opaque_keys.edx.keys import CourseKey
@@ -42,6 +44,7 @@
 from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
 from openedx.core.djangoapps.schedules.models import Schedule
 from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
+from openedx.core.djangolib.testing.sql_assertions import assert_update_before_delete
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from xmodule.modulestore import ModuleStoreEnum  # pylint: disable=wrong-import-order
 from xmodule.modulestore.tests.django_utils import (  # pylint: disable=wrong-import-order
@@ -600,6 +603,18 @@ def test_delete_by_user_no_effect_for_user_with_no_email_change(self):
         assert not record_was_deleted
         assert 1 == len(PendingEmailChange.objects.all())
 
+    def test_delete_by_user_value_redacts_pending_email_before_deletion(self):
+        """
+        Verify that delete_by_user_value redacts new_email before deletion.
+        """
+        table = PendingEmailChange._meta.db_table
+        with CaptureQueriesContext(connection) as ctx:
+            record_was_deleted = PendingEmailChange.delete_by_user_value(self.user, field='user')
+
+        assert_update_before_delete([q['sql'] for q in ctx], table=table)
+        assert record_was_deleted
+        assert not PendingEmailChange.objects.filter(user=self.user).exists()
+
 
 class TestCourseEnrollmentAllowed(ModuleStoreTestCase):  # pylint: disable=missing-class-docstring
 
@@ -636,6 +651,33 @@ def test_retiring_nonexistent_user_doesnt_modify_records(self):
         )
         assert user_search_results.exists()
 
+    def test_email_redacted_before_delete(self):
+        """
+        Verify email redaction runs before delete for downstream soft-delete systems.
+        """
+        other_course_key = CourseKey.from_string('course-v1:edX+OtherX+Other_Course')
+        other_record = CourseEnrollmentAllowed.objects.create(
+            email=self.email,
+            course_id=other_course_key,
+        )
+
+        with CaptureQueriesContext(connection) as ctx:
+            is_successful = CourseEnrollmentAllowed.delete_by_user_value(
+                value=self.email,
+                field='email'
+            )
+
+        assert is_successful
+        assert_update_before_delete(
+            [q['sql'] for q in ctx],
+            table=CourseEnrollmentAllowed._meta.db_table,
+            require_id_filter=True,
+            expected_redacted_value='redacted@retired.invalid',
+        )
+        assert not CourseEnrollmentAllowed.objects.filter(
+            id__in=[self.allowed_enrollment.id, other_record.id]
+        ).exists()
+
     def test_may_enroll_and_unenrolled_result_is_based_on_unmarked_user_field(self):
         """
         Make sure that if an allowed student has no assigned user in its user field,

common/djangoapps/student/views/management.py

@@ -963,14 +963,17 @@ def confirm_email_change(request, key):
 
         user.email = pec.new_email
         user.save()
-        pec.delete()
+
+        # Redacts pending email before deletion
+        PendingEmailChange.delete_by_user_value(user, field="user")
+
         # And send it to the new email...
-        msg.recipient = Recipient(user.id, pec.new_email)
+        msg.recipient = Recipient(user.id, user.email)
         try:
             ace.send(msg)
         except Exception:  # pylint: disable=broad-except
             log.warning('Unable to send confirmation email to new address', exc_info=True)
-            response = render_to_response("email_change_failed.html", {'email': pec.new_email})
+            response = render_to_response("email_change_failed.html", {'email': user.email})
             transaction.set_rollback(True)
             return response
 

openedx/core/djangoapps/course_groups/models.py

@@ -311,3 +311,10 @@ class Meta:
     course_user_group = models.ForeignKey(CourseUserGroup, on_delete=models.CASCADE)  # noqa: DJ012
     email = models.CharField(blank=True, max_length=255, db_index=True)
     course_id = CourseKeyField(max_length=255)
+
+    @classmethod
+    def redact_before_delete_fields(cls):
+        """
+        Redact email before deleting records for downstream soft-delete systems.
+        """
+        return {'email': 'redacted@retired.invalid'}

openedx/core/djangoapps/course_groups/tests/test_cohorts.py

@@ -8,15 +8,17 @@
 import ddt
 import pytest
 from django.contrib.auth.models import AnonymousUser, User  # pylint: disable=imported-auth-user
-from django.db import IntegrityError
+from django.db import IntegrityError, connection
 from django.http import Http404
 from django.test import TestCase
+from django.test.utils import CaptureQueriesContext
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import CourseLocator
 from openedx_events.testing import OpenEdxEventsTestMixin
 
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.tests.factories import UserFactory
+from openedx.core.djangolib.testing.sql_assertions import assert_update_before_delete
 from xmodule.modulestore.django import modulestore  # pylint: disable=wrong-import-order
 from xmodule.modulestore.tests.django_utils import (  # pylint: disable=wrong-import-order
     TEST_DATA_SPLIT_MODULESTORE,
@@ -807,3 +809,36 @@ def test_retired_user_with_no_cohort_returns_false(self):
 
         assert not was_retired
         assert self.cohort_assignment.email == known_learner_email
+
+    def test_email_redacted_before_delete(self):
+        """
+        Verify email redaction runs before delete for downstream soft-delete systems.
+        """
+        other_course_key = CourseKey.from_string('course-v1:edX+OtherX+Other_Course')
+        other_cohort = CourseUserGroup.objects.create(
+            name='OtherCohort',
+            course_id=other_course_key,
+            group_type=CourseUserGroup.COHORT,
+        )
+        other_assignment = UnregisteredLearnerCohortAssignments.objects.create(
+            course_user_group=other_cohort,
+            course_id=other_course_key,
+            email='learner@example.com',
+        )
+
+        with CaptureQueriesContext(connection) as ctx:
+            was_retired = UnregisteredLearnerCohortAssignments.delete_by_user_value(
+                value='learner@example.com',
+                field='email'
+            )
+
+        assert was_retired
+        assert_update_before_delete(
+            [q['sql'] for q in ctx],
+            table=UnregisteredLearnerCohortAssignments._meta.db_table,
+            require_id_filter=True,
+            expected_redacted_value='redacted@retired.invalid',
+        )
+        assert not UnregisteredLearnerCohortAssignments.objects.filter(
+            id__in=[self.cohort_assignment.id, other_assignment.id]
+        ).exists()

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

@@ -67,6 +67,7 @@
     UserRetirementPartnerReportingStatus,
     UserRetirementStatus,
 )
+from openedx.core.djangolib.testing.sql_assertions import assert_update_before_delete
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -1469,7 +1470,10 @@ def test_retire_user_where_username_not_provided(self):
     @mock.patch('openedx.core.djangoapps.user_api.accounts.views.remove_profile_images')
     def test_retire_user(self, mock_remove_profile_images, mock_get_profile_image_names):
         data = {'username': self.original_username}
-        self.post_and_assert_status(data)
+        with CaptureQueriesContext(connection) as ctx:
+            self.post_and_assert_status(data)
+
+        assert_update_before_delete([q['sql'] for q in ctx], table=PendingEmailChange._meta.db_table)
 
         self.test_user.refresh_from_db()
         self.test_user.profile.refresh_from_db()  # pylint: disable=no-member

openedx/core/djangolib/model_mixins.py

@@ -20,14 +20,27 @@ class DeletableByUserValue:
     associated with some specified user.
     """
 
+    @classmethod
+    def redact_before_delete_fields(cls):
+        """
+        Returns dict of PII fields and their redacted values.
+
+        Always returns an empty dict unless overridden by the inheriting model.
+        Results are used by ``delete_by_user_value`` to redact PII before delete.
+        """
+        return {}
+
     @classmethod
     def delete_by_user_value(cls, value, field):
         """
-        Deletes instances of this model where ``field`` equals ``value``.
+        Redacts and deletes instances of this model where ``field`` equals ``value``.
 
         e.g.
             ``delete_by_user_value(value='learner@example.com', field='email')``
 
+        If ``redact_before_delete_fields`` returns a non-empty dict, matching
+        records are bulk-updated with those redacted values before delete.
+
         Returns True if any instances were deleted.
         Returns False otherwise.
         """
@@ -37,5 +50,12 @@ def delete_by_user_value(cls, value, field):
         if not records_matching_user_value.exists():
             return False
 
-        records_matching_user_value.delete()
+        record_ids = list(records_matching_user_value.values_list('id', flat=True))
+        records_matching_ids = cls.objects.filter(id__in=record_ids)
+
+        redact_fields = cls.redact_before_delete_fields()
+        if redact_fields:
+            records_matching_ids.update(**redact_fields)
+
+        records_matching_ids.delete()
         return True

openedx/core/djangolib/testing/sql_assertions.py

@@ -0,0 +1,46 @@
+"""
+SQL assertion helpers for tests.
+"""
+
+
+def assert_update_before_delete(
+    sql_list,
+    table,
+    num_redact_delete_pairs=1,
+    require_id_filter=False,
+    expected_redacted_value=None,
+):
+    """
+    Assert that UPDATE and DELETE queries for ``table`` occur in consecutive pairs.
+
+    Optionally assert ID-based filtering and an expected redacted value in UPDATE SQL.
+    """
+    table_key = table.upper()
+    expected_sql_list = [
+        sql for sql in sql_list
+        if table_key in sql.upper() and ('UPDATE' in sql.upper() or 'DELETE' in sql.upper())
+    ]
+
+    assert len(expected_sql_list) == num_redact_delete_pairs * 2, (
+        f'Expected {num_redact_delete_pairs * 2} UPDATE/DELETE queries on {table}, '
+        f'got {len(expected_sql_list)}'
+    )
+
+    for index in range(0, len(expected_sql_list), 2):
+        update_sql = expected_sql_list[index]
+        delete_sql = expected_sql_list[index + 1]
+        assert 'UPDATE' in update_sql.upper(), f'Expected UPDATE at position {index} for {table}'
+        assert 'DELETE' in delete_sql.upper(), f'Expected DELETE at position {index + 1} for {table}'
+
+        if require_id_filter:
+            assert '"ID" IN' in update_sql.upper() or 'WHERE "ID"' in update_sql.upper(), (
+                f'Expected UPDATE to use ID-based filtering for {table}'
+            )
+            assert '"ID" IN' in delete_sql.upper() or 'WHERE "ID"' in delete_sql.upper(), (
+                f'Expected DELETE to use ID-based filtering for {table}'
+            )
+
+        if expected_redacted_value is not None:
+            assert expected_redacted_value.upper() in update_sql.upper(), (
+                f'Expected UPDATE to set redacted value {expected_redacted_value} for {table}'
+            )