Skip to content

fix: ensure PII is cleared from historical certificate records during retirement#38671

Merged
robrap merged 21 commits into
openedx:masterfrom
ttak-apphelix:ttak-apphelix/BOMS-590
Jun 11, 2026
Merged

fix: ensure PII is cleared from historical certificate records during retirement#38671
robrap merged 21 commits into
openedx:masterfrom
ttak-apphelix:ttak-apphelix/BOMS-590

Conversation

@ttak-apphelix

@ttak-apphelix ttak-apphelix commented May 28, 2026

Copy link
Copy Markdown
Contributor

Description

GeneratedCertificate uses django-simple-history, which mirrors every row change into certificates_historicalgeneratedcertificate. This table stores the learner's full name in every snapshot. The existing retirement pipeline only blanked name in the live table — all prior audit snapshots retained the real name indefinitely.

This PR closes that gap:

  • clear_pii_from_certificate_records_for_user() now also blanks name in the history table via GeneratedCertificate.history.filter(...).update(name="")
  • purge_pii_from_generatedcertificates management command extended to also backfill the history table for all previously retired users(flag-gated)
  • [config.py] — added [ENABLE_REDACT_HISTORICAL_PII_RETIREMENT] setting toggle
  • pii_retirement annotation on GeneratedCertificate corrected from retained to local_api
  • Tests updated to assert history table rows are cleared in both paths

Testing:
pytest lms/djangoapps/certificates/tests/test_api.py::CertificatesLearnerRetirementFunctionality
pytest lms/djangoapps/certificates/management/commands/tests/test_purge_pii_from_generatedcertificates.py

Jira ticket:
https://2u-internal.atlassian.net/browse/BOMS-590

@ttak-apphelix ttak-apphelix requested a review from a team as a code owner May 28, 2026 10:21
Comment thread lms/djangoapps/certificates/config.py Outdated

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Comment thread lms/djangoapps/certificates/models.py
Comment thread lms/djangoapps/certificates/models.py

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of simple changes. I gave it a bit more attention. Not everything would be blocking, but simple enough in most cases.

Comment thread lms/djangoapps/certificates/config.py Outdated
Comment thread lms/djangoapps/certificates/config.py Outdated
Comment thread lms/djangoapps/certificates/models.py Outdated
Comment thread lms/djangoapps/certificates/api.py Outdated
Comment thread lms/djangoapps/certificates/models.py Outdated

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job. I made two minor updates before merging.

@robrap robrap enabled auto-merge (squash) June 11, 2026 21:29
@robrap robrap merged commit 7536eff into openedx:master Jun 11, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants