Merge branch 'master' into aaich/BOMS-572

Author: Akanshu-2u

lms/djangoapps/certificates/api.py

@@ -24,6 +24,7 @@
 from common.djangoapps.student.models import CourseEnrollment
 from lms.djangoapps.branding import api as branding_api
 from lms.djangoapps.certificates.config import AUTO_CERTIFICATE_GENERATION as _AUTO_CERTIFICATE_GENERATION
+from lms.djangoapps.certificates.config import REDACT_CERTIFICATES_HISTORICAL_PII
 from lms.djangoapps.certificates.data import CertificateStatuses, GeneratedCertificateData
 from lms.djangoapps.certificates.generation_handler import generate_certificate_task as _generate_certificate_task
 from lms.djangoapps.certificates.generation_handler import is_on_certificate_allowlist as _is_on_certificate_allowlist
@@ -977,13 +978,17 @@ def clear_pii_from_certificate_records_for_user(user):
     model's custom `save()` function, nor fire any Django signals (which is desired at the time of writing). There is
     nothing to update in our external systems by this update.
 
+    If the ``REDACT_CERTIFICATES_HISTORICAL_PII`` toggle is enabled, the history audit table will also be redacted.
+
     Args:
         user (User): The User instance of the learner actively being retired.
 
     Returns:
         None
     """
     GeneratedCertificate.objects.filter(user=user).update(name="")
+    if REDACT_CERTIFICATES_HISTORICAL_PII.is_enabled():
+        GeneratedCertificate.history.filter(user=user).update(name="")
 
 
 def get_cert_history_for_course_id(course_id):

lms/djangoapps/certificates/config.py

@@ -2,7 +2,7 @@
 This module contains various configuration settings via
 waffle switches for the Certificates app.
 """
-from edx_toggles.toggles import WaffleSwitch
+from edx_toggles.toggles import SettingToggle, WaffleSwitch
 
 # Namespace
 WAFFLE_NAMESPACE = 'certificates'
@@ -14,3 +14,14 @@
 # .. toggle_use_cases: open_edx
 # .. toggle_creation_date: 2017-09-14
 AUTO_CERTIFICATE_GENERATION = WaffleSwitch(f"{WAFFLE_NAMESPACE}.auto_certificate_generation", __name__)
+
+# .. toggle_name: REDACT_CERTIFICATES_HISTORICAL_PII
+# .. toggle_implementation: SettingToggle
+# .. toggle_default: False
+# .. toggle_description: Clears the `name` field in the django-simple-history audit table for
+#      retiring users' certificate records.
+# .. toggle_use_cases: open_edx
+# .. toggle_creation_date: 2026-05-29
+REDACT_CERTIFICATES_HISTORICAL_PII = SettingToggle(
+    "REDACT_CERTIFICATES_HISTORICAL_PII", default=False, module_name=__name__
+)

lms/djangoapps/certificates/management/commands/purge_pii_from_generatedcertificates.py

@@ -1,6 +1,7 @@
 """
 A management command, designed to be run once by Open edX Operators, to obfuscate learner PII from the
-`Certificates_GeneratedCertificate` table that should have been purged during learner retirement.
+`certificates_generatedcertificate` and `certificates_historicalgeneratedcertificate` tables that should have been
+purged during learner retirement.
 
 A fix has been included in the retirement pipeline to properly purge this data during learner retirement. This can be
 used to purge PII from accounts that have already been retired.
@@ -11,6 +12,7 @@
 from django.contrib.auth import get_user_model
 from django.core.management.base import BaseCommand
 
+from lms.djangoapps.certificates.config import REDACT_CERTIFICATES_HISTORICAL_PII
 from lms.djangoapps.certificates.models import GeneratedCertificate
 from openedx.core.djangoapps.user_api.api import get_retired_user_ids
 
@@ -20,11 +22,12 @@
 
 class Command(BaseCommand):
     """
-    This management command performs a bulk update on `GeneratedCertificate` instances. This means that it will not
-    invoke the custom save() function defined as part of the `GeneratedCertificate` model, and thus will not emit any
-    Django signals throughout the system after the update occurs. This is desired behavior. We are using this
-    management command to purge remnant PII, retired elsewhere in the system, that should have already been removed
-    from the Certificates tables. We don't need updates to propogate to external systems (like the Credentials IDA).
+    This management command performs bulk updates on `GeneratedCertificate` instances and their django-simple-history
+    audit rows in `certificates_historicalgeneratedcertificate`. This means that it will not invoke the custom save()
+    function defined as part of the `GeneratedCertificate` model, and thus will not emit any Django signals throughout
+    the system after the update occurs. This is desired behavior. We are using this management command to purge remnant
+    PII, retired elsewhere in the system, that should have already been removed from the Certificates tables. We don't
+    need updates to propogate to external systems (like the Credentials IDA).
 
     This management command functions by requesting a list of learners' user_ids whom have completed their journey
     through the retirement pipeline. The `get_retired_user_ids` utility function is responsible for filtering out any
@@ -41,8 +44,8 @@ class Command(BaseCommand):
     """
 
     help = """
-    Purges learners' full names from the `Certificates_GeneratedCertificate` table if their account has been
-    successfully retired.
+    Purges learners' full names from the `certificates_generatedcertificate` table and conditionally the
+    `certificates_historicalgeneratedcertificate` table if their account has been successfully retired.
     """
 
     def add_arguments(self, parser):
@@ -59,6 +62,11 @@ def handle(self, *args, **options):
                 f"Purging `name` from the certificate records of the following users: {retired_user_ids}"
             )
             GeneratedCertificate.objects.filter(user_id__in=retired_user_ids).update(name="")
+            if REDACT_CERTIFICATES_HISTORICAL_PII.is_enabled():
+                log.info(
+                    f"Purging `name` from the historical certificate records of the following users: {retired_user_ids}"
+                )
+                GeneratedCertificate.history.filter(user_id__in=retired_user_ids).update(name="")
         else:
             log.info(
                 "DRY RUN: running this management command would purge `name` data from the following users: "

lms/djangoapps/certificates/management/commands/tests/test_purge_pii_from_generatedcertificates.py

@@ -2,8 +2,9 @@
 Tests for the `purge_pii_from_generatedcertificates` management command.
 """
 
-
+import ddt
 from django.core.management import call_command
+from django.test import override_settings
 from testfixtures import LogCapture
 
 from common.djangoapps.student.tests.factories import UserFactory
@@ -20,6 +21,7 @@
 from xmodule.modulestore.tests.factories import CourseFactory
 
 
+@ddt.ddt
 class PurgePiiFromCertificatesTests(ModuleStoreTestCase):
     """
     Tests for the `purge_pii_from_generatedcertificates` management command.
@@ -72,7 +74,8 @@ def setUp(self):
         )
         UserRetirementRequestFactory(user=self.user_retired)
 
-    def test_management_command(self):
+    @ddt.data(True, False)
+    def test_management_command(self, redact_history_toggle_enabled):
         """
         Verify the management command purges expected data from a GeneratedCertificate instance if a learner has
         successfully had their account retired.
@@ -82,13 +85,31 @@ def test_management_command(self):
         cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
         assert cert_for_retired_user.name == self.user_retired_name
 
-        call_command("purge_pii_from_generatedcertificates")
+        with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=redact_history_toggle_enabled):
+            call_command("purge_pii_from_generatedcertificates")
 
         cert_for_active_user = GeneratedCertificate.objects.get(user_id=self.user_active)
         assert cert_for_active_user.name == self.user_active_name
         cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
         assert cert_for_retired_user.name == ""
 
+        active_history_names = list(
+            GeneratedCertificate.history.filter(user=self.user_active).values_list("name", flat=True)
+        )
+        assert len(active_history_names) > 0
+        assert all(n == self.user_active_name for n in active_history_names)
+
+        retired_history_names = list(
+            GeneratedCertificate.history.filter(user=self.user_retired).values_list("name", flat=True)
+        )
+        assert len(retired_history_names) > 0
+        if redact_history_toggle_enabled:
+            assert all(n == "" for n in retired_history_names), "Names in the history table should have been redacted."
+        else:
+            assert all(n == self.user_retired_name for n in retired_history_names), (
+                "Names in the history table should not have been redacted."
+            )
+
     def test_management_command_dry_run(self):
         """
         Verify that the management command does not purge any data when invoked with the `--dry-run` flag
@@ -111,4 +132,10 @@ def test_management_command_dry_run(self):
         cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
         assert cert_for_retired_user.name == self.user_retired_name
 
+        retired_history_names = list(
+            GeneratedCertificate.history.filter(user=self.user_retired).values_list("name", flat=True)
+        )
+        assert len(retired_history_names) > 0
+        assert all(n == self.user_retired_name for n in retired_history_names)
+
         assert logger.records[0].msg == expected_log_msg

lms/djangoapps/certificates/models.py

@@ -170,9 +170,9 @@ class GeneratedCertificate(models.Model):  # noqa: DJ008
     """
     Base model for generated course certificates
 
-    .. pii: PII can exist in the generated certificate linked to in this model. Certificate data is currently retained.
-    .. pii_types: name, username
-    .. pii_retirement: retained
+    .. pii: PII can exist in the generated certificate linked to in this model.
+    .. pii_types: name
+    .. pii_retirement: local_api
 
     course_id       - Course run key
     created_date    - Date and time the certificate was created
@@ -248,6 +248,10 @@ class GeneratedCertificate(models.Model):  # noqa: DJ008
     # imports this model's code. Simple History will attempt to connect to the installed
     # model in the certificates app, which will fail.
     if 'certificates' in apps.app_configs:
+        # The PII is retained by default, but can be removed by enabling ``REDACT_CERTIFICATES_HISTORICAL_PII``.
+        # .. pii: The auto-generated ``HistoricalGeneratedCertificate`` table mirrors all fields of this model.
+        # .. pii_types: name
+        # .. pii_retirement: retained
         history = HistoricalRecords()
 
     class Meta:

lms/djangoapps/certificates/tests/test_api.py

@@ -1277,6 +1277,32 @@ def test_clear_pii_from_certificate_records(self):
         assert cert_course1.name == ""
         assert cert_course2.name == ""
 
+    def test_clear_pii_from_certificate_records_clears_history_table(self):
+        """
+        Verify that `clear_pii_from_certificate_records_for_user` blanks `name` in the
+        django-simple-history audit table only when the ``REDACT_CERTIFICATES_HISTORICAL_PII``
+        setting toggle is enabled, and leaves it untouched when the toggle is disabled.
+        """
+        with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=False):
+            clear_pii_from_certificate_records_for_user(self.user)
+
+        history_names = list(
+            GeneratedCertificate.history.filter(user=self.user).values_list("name", flat=True)
+        )
+        assert all(n == self.user_full_name for n in history_names), (
+            "History rows should be untouched when the waffle flag is disabled."
+        )
+
+        with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=True):
+            clear_pii_from_certificate_records_for_user(self.user)
+
+        history_names_after = list(
+            GeneratedCertificate.history.filter(user=self.user).values_list("name", flat=True)
+        )
+        assert all(n == "" for n in history_names_after), (
+            "Expected all history rows to have name blanked after retirement."
+        )
+
 
 class GetCourseIdsForUsernameTests(TestCase):
     """