boostorg / boostorg/website-v2

Task: Clean up achievements and badges when a user deletes their account

Open
#2,561 0 comments 0 reactions 0 assignees View on GitHub
Phase 2
Dominant language
HTML
Stars
18
Forks
28
Avg merge
2d 12h
Merged PRs (30d)
77

Description

## Context

PR #2537 reworks account deletion for V3. Deletion **anonymizes in place** rather than dropping the
row: `User.delete_account(extended_scrub=True)` sets `is_active=False`, scrubs the name, email,
avatar, GitHub username and profile links, and deletes the linked `SocialAccount`, `EmailAddress`,
`Preferences`, `LastSeen` and `UserMailingListSubscription` rows. The `User` row survives so authored
content stays attributed.

That PR was written against `develop`, where `User.badges` is still the **legacy** `users.Badge`
many-to-many. Its scrub therefore contains:

```python
self.badges.clear()
```

The badges stack deletes that legacy model and re-uses the same accessor name: `UserBadge.user` is a
`ForeignKey(..., related_name="badges")`. Two consequences, and both need this ticket.

### 1. It breaks outright (hard collision, must be fixed by whichever PR merges second)

`self.badges` becomes a reverse-FK `RelatedManager` over a **non-nullable** FK. Django only puts
`clear()` and `remove()` on reverse managers whose FK is nullable, so after both changes land every V3
account deletion raises:

```
AttributeError: 'RelatedManager' object has no attribute 'clear'
```

`delete_account` is wrapped in `@transaction.atomic`, so the whole deletion rolls back. The user is
told nothing was scheduled, or the nightly `do_scheduled_user_deletions` task dies on the first
affected row and stops processing the rest of the batch.

### 2. Even fixed, it cleans the wrong layer

`UserBadge` is not the source of truth. `UserAchievement` is - `recalculate_badges` derives badge
state from the count of valid `UserAchievement` rows, and it is the only writer of `UserBadge`.
Deleting badges without touching achievements means the next `recalculate_badges` call re-awards
every badge it just removed.

And achievements themselves regenerate. `badges/sources.py` iterates `Library.authors`,
`LibraryVersion.maintainers`, `Commit.author.user`, `Review.submitters.user` and `Entry.author` with
**no filter on account state** - because until now there was no reason for one. Those relations all
still point at the anonymized user, so `backfill_achievements` (which runs after every
`update_authors_and_maintainers`, `update_commits` and weekly `release_tasks`) re-grants the
achievements, the `post_save` signal recalculates, and the badges come back. **A scrub that only
deletes rows is silently undone within a week.**

Badges are rendered on the public profile (`get_earned_badges` / `featured_badge`) and on news
post-author cards, so this is user-visible data on an account the user asked to have erased.

## Scope

1. **A persistent "this account was deleted" marker.** Add `User.deleted_at` (nullable datetime,
`editable=False`), set by `delete_account()`. Do not infer deletion from `is_active=False` - that
flag is also how an admin deactivates a live account, and conflating the two would silently strip
badges from a suspended user. `deletion_extended_scrub` is not a substitute either; it records
*which* scrub ran, not *that* one ran.
2. **Exclude deleted accounts at the achievement source.** Filter in `backfill_achievements` where
the `(user, source)` pairs are consumed, not inside each of the six iterators in
`badges/sources.py` - one guard, no way to forget it when a seventh source is added. Add the same
guard to `grant_automatic()` so any future live path inherits it.
3. **Delete the user's achievement and badge rows** inside `delete_account`:
- `UserAchievement.objects.filter(user=self).delete()`
- `UserBadge.objects.filter(user=self).delete()`

Hard delete, not soft. Soft-revoke exists to preserve an audit trail *about a user*, and the point
here is that the user is being erased. `UserAchievement` rows also carry a generic FK to the
record that justified the grant (a specific commit, a specific review), which is re-identifying
data on an otherwise anonymized account. `UserBadge.tier` is `PROTECT`, but that protects the
*tier* from deletion; deleting the `UserBadge` row itself is unaffected.
4. **Force `hide_badges = True`** in the scrub. Cheap defence in depth: if any future code path
re-awards a badge to a deleted account, it still will not render.
5. **Replace `self.badges.clear()`** with the above. Whichever of the two PRs merges second carries
this line change even if the rest of this ticket is deferred - it is a crash, not a nicety.
6. **Make the notification suppression explicit.** `send_achievement_awarded_email` currently skips
deleted users only by accident: `getattr(user, "preferences", None)` returns `None` because
`RelatedObjectDoesNotExist` subclasses `AttributeError`, and `delete_account` deleted the
`Preferences` row. That is load-bearing behaviour resting on an exception hierarchy. Add an
explicit `if user.deleted_at: return False`.
7. **Tests** - see acceptance criteria.

## Out of scope

- **Changing deletion from anonymize-in-place to a hard delete.** That is #2438's design decision and
is not reopened here.
- **Unlinking `CommitAuthor.user`, `Library.authors`, `Entry.author` etc.** Deliberately left intact -
#2537's whole premise is that authored content stays attributed to the anonymized user. This ticket
stops those links *feeding achievements*; it does not sever them.
- **Dangling generic FKs from other causes.** `UserAchievement.source_object_id` is a plain integer,
so hard-deleting a `Commit` or a `Review` leaves an orphan row that still counts toward a threshold.
Real, but a separate data-integrity ticket, not a deletion concern.
- **The two other unenforced privacy toggles** (`hide_github_activity`,
`hide_mailing_list_activity`) - gap **K**.

## Decisions to confirm with product

1. **Legacy (flag-off) deletions.** #2537 keeps the narrow legacy scrub byte-identical to production
and gates everything new behind `extended_scrub`. Following that rule literally means a legacy
deletion leaves achievements and badges fully intact and publicly rendered. Since the `v3` flag is
an environment-wide rollback switch and this is a privacy obligation rather than a visual change,
the recommendation is to run the badge cleanup **unconditionally**, outside the `extended_scrub`
block. That is a deliberate deviation from the legacy-verbatim rule and needs a sign-off.
2. **Audit back-references.** `UserAchievement.granted_by` / `invalidated_by` and
`UserBadge.revoked_by` point at the **admin** who acted, not at the badge holder. When an admin
deletes their own account those rows survive and now read "John Doe". No PII leaks (the name is
scrubbed and these fields are admin-only), so the recommendation is **no action**. Stated
explicitly so it does not get re-litigated in review.
3. **Alternative if an audit trail of badge history is wanted.** Soft-revoke instead of delete, with a
third `RevocationSource.ACCOUNT_DELETION` treated like `MANUAL` in `_award_tier` so recalculation
cannot resurrect it. This is the weaker option: it keeps the re-identifying source links alive,
and it needs a migration plus a change to the reinstate admin action. Only worth it if someone
actually needs to answer "which badges did this deleted account hold".

## Also worth flagging

`versions/migrations/0015_drop_review_generated_stub_users.py` (already on `develop`) scheduled every
review-generated stub user for deletion by setting `delete_permanently_at`. Those stubs are precisely
the population that the `library-review` achievement source draws from. Because
`deletion_extended_scrub` defaults to `False`, `do_scheduled_user_deletions` will run the **narrow**
scrub on all of them - so under decision 1 above they would keep accumulating achievements forever.
Check the size of that cohort before the first production backfill.

## Acceptance criteria

- [ ] V3 account deletion completes without raising, with the badges stack merged
- [ ] Deleting an account removes all of its `UserAchievement` and `UserBadge` rows
- [ ] `hide_badges` is `True` after deletion
- [ ] Running `backfill_achievements` after a deletion grants the deleted user **zero** achievements,
proven by a test that gives the user a library authorship and a linked commit first
- [ ] Running `recalculate_badges` for a deleted user awards **zero** badges
- [ ] The deleted user's public profile renders no badges, and their news post-author card renders no
badge
- [ ] `send_achievement_awarded_email` returns `False` for a deleted user even when a `Preferences`
row exists
- [ ] `do_scheduled_user_deletions` processes a batch containing several users with badges without
aborting partway
- [ ] Deletion remains idempotent - running it twice is a no-op (matching #2537's existing guarantee)
- [ ] Full suite green, pre-commit clean

## Risks

- **The `.badges` accessor collision is a merge-order landmine.** Both PRs pass their own suites in
isolation; the failure only appears once they are in the same tree. Neither branch's CI will catch
it. Whoever merges second must run `users/tests/test_delete_account.py` against the merged result,
not against their own branch. A rebase after the first one gets into `develop` also solves this.
- **This is destructive.** The cleanup hard-deletes rows during a transaction that also emails the
user. Verify it runs inside `delete_account`'s existing `@transaction.atomic` so a failure rolls the
deletes back with everything else.
- **Order matters at deploy time.** If deletions ship before this cleanup and a production backfill
runs in between, already-deleted accounts will have badges. A one-off cleanup command over
`User.objects.filter(deleted_at__isnull=False)` would then be needed. Since the deletions PR will definitely arrive earlier than this one, we should double check that the backfill step is not hydrating deleted users' profiles.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with User.delete_account(), backfill_achievements and grant_automatic(), then inspect badges/sources.py and the notification path for send_achievement_awarded_email. Use users/tests/test_delete_account.py and the stated acceptance criteria to verify deleted users retain no achievements or badges, deletion is idempotent, scheduled batches continue, and the full suite passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
django, python
Domain
backend, databases, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.