Ticket #2539 :: Task: Track documentation contributions and derive the Documenter achievement from them

badges/admin.py

@@ -845,13 +845,15 @@ class UserAchievementAdmin(
         "user__display_name",
         "achievement__name",
         "grant_notes",
+        "dedup_info",
     )
     autocomplete_fields = ("achievement", "user")
     readonly_fields = (
         "created_at",
         "invalidated_by",
         "invalidated_at",
         "granted_by",
+        "dedup_info",
     )
     actions = ["invalidate", "revalidate"]
     add_fieldsets = ((None, {"fields": ("user", "achievement", "grant_notes")}),)

badges/migrations/0004_userachievement_dedup_info.py

@@ -0,0 +1,40 @@
+# Generated by Django 6.0.2 on 2026-08-19 14:21
+
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("badges", "0003_achievementsyncrun"),
+        ("contenttypes", "0002_remove_content_type_name"),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.RemoveConstraint(
+            model_name="userachievement",
+            name="unique_automatic_user_achievement_source",
+        ),
+        migrations.AddField(
+            model_name="userachievement",
+            name="dedup_info",
+            field=models.TextField(
+                blank=True,
+                help_text="For automatic grants: the source's stable id for the evidence.",
+                null=True,
+                verbose_name="dedup info",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="userachievement",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(
+                    ("dedup_info__isnull", False), ("source_type", "automatic")
+                ),
+                fields=("user", "achievement", "dedup_info"),
+                name="unique_automatic_user_achievement_dedup",
+            ),
+        ),
+    ]

badges/models.py

@@ -120,6 +120,15 @@ class UserAchievement(models.Model):
     # Big, not plain: the models these grants point at use BigAutoField keys.
     source_object_id = models.PositiveBigIntegerField(null=True, blank=True)
     source = GenericForeignKey("source_content_type", "source_object_id")
+    # The source's own name for the evidence, which a primary key is not: the
+    # commit importer deletes and re-creates rows, and one commit is stored once
+    # per library version covering it. Null for manual grants.
+    dedup_info = models.TextField(
+        _("dedup info"),
+        null=True,
+        blank=True,
+        help_text=_("For automatic grants: the source's stable id for the evidence."),
+    )
 
     granted_by = models.ForeignKey(
         settings.AUTH_USER_MODEL,
@@ -142,14 +151,9 @@ class Meta:
         ordering = ("-created_at",)
         constraints = [
             models.UniqueConstraint(
-                fields=[
-                    "user",
-                    "achievement",
-                    "source_content_type",
-                    "source_object_id",
-                ],
-                condition=models.Q(source_type="automatic"),
-                name="unique_automatic_user_achievement_source",
+                fields=["user", "achievement", "dedup_info"],
+                condition=models.Q(source_type="automatic", dedup_info__isnull=False),
+                name="unique_automatic_user_achievement_dedup",
             )
         ]
 

badges/services.py

@@ -11,8 +11,9 @@
 
 This module also owns the achievement-side writes that feed recalculation:
 ``sync_source``, which makes the stored automatic grants for one source agree with
-that source in both directions, and ``discard_source_achievements``, for source
-rows about to be deleted outright.
+that source in both directions, ``discard_source_achievements``, for source rows
+about to be deleted outright, and ``relink_source_achievements``, for rows about to
+be replaced by the same evidence under new ids.
 
 Both delete in bulk, and both recalculate their own members rather than leaving it
 to the ``post_delete`` signal, which fires per row: see ``owns_recalculation``.
@@ -50,7 +51,7 @@
     "the {rank} threshold of {threshold}."
 )
 
-# Rows per DELETE ... WHERE pk IN (...) and per bulk_create. The unmatched set can
+# Rows per DELETE ... WHERE pk IN (...) and per bulk_create. The stored key set can
 # be as large as the achievement table.
 SYNC_BATCH_SIZE = 1000
 
@@ -138,6 +139,47 @@ def discard_source_achievements(model, object_ids):
             recalculate_badges(user_id, achievement_id)
 
 
+def relink_source_achievements(model, ids_by_key):
+    """Re-point automatic grants at rows re-created under the same dedup key.
+
+    A caller that deletes and re-inserts the same evidence - the commit importer
+    running destructively - leaves every pointer to it dangling, because a generic
+    foreign key carries no referential integrity. The dedup key is what survives
+    that swap, so the link is rebuilt from it rather than the grant thrown away:
+    discarding a grant revokes the badge it justifies, records that revocation
+    permanently, and re-earns the badge with today's date on the next sync, so a
+    re-import would rewrite history that nothing actually changed.
+
+    No grant appears or disappears, so no count moves and nothing is recalculated.
+
+    Args:
+        model: The model the grants point at.
+        ids_by_key: The new row id for each dedup key, as the source names it.
+
+    Returns:
+        How many grants were re-pointed.
+    """
+    if not ids_by_key:
+        return 0
+    content_type = ContentType.objects.get_for_model(model)
+    grants = UserAchievement.objects.filter(
+        source_content_type=content_type,
+        source_type=SourceType.AUTOMATIC,
+        dedup_info__in=list(ids_by_key),
+    ).only("pk", "dedup_info", "source_object_id")
+    moved = []
+    for grant in grants.iterator(chunk_size=2000):
+        object_id = ids_by_key[grant.dedup_info]
+        if object_id != grant.source_object_id:
+            grant.source_object_id = object_id
+            moved.append(grant)
+    if moved:
+        UserAchievement.objects.bulk_update(
+            moved, ["source_object_id"], batch_size=SYNC_BATCH_SIZE
+        )
+    return len(moved)
+
+
 class SourceSync(NamedTuple):
     """What syncing one source found, and what it was allowed to do about it.
 
@@ -355,26 +397,25 @@ def _sync_source(
     if user_ids is not None:
         stored = stored.filter(user_id__in=user_ids)
 
-    # Every stored key, keyed by what the iterator can reconstruct and valued by
-    # the rows carrying it. Whatever survives the walk is stale, and a key the
-    # walk cannot find here is a grant that does not exist yet - so one dict
-    # answers both halves and the walk needs no per-batch lookup of its own.
-    # Bounded by this achievement's row count rather than by the source's, so a
-    # scoped run holds one member's grants in memory and not every commit.
+    # Every stored grant, keyed the way its source names the evidence. Whatever the
+    # walk never yields is stale, and a key it yields but cannot find here is a
+    # grant that does not exist yet, so one dict answers both halves. Bounded by
+    # this achievement's row count rather than by the source's, so a scoped run
+    # holds one member's grants in memory and not every commit.
     #
-    # A list of rows per key, not one: the source pointer is nullable, so several
-    # automatic rows can share ``(user, NULL, NULL)``, and one slot per key would
-    # clear all but the last of them per run. A key with a real pointer can only
-    # ever hold one row - ``unique_automatic_user_achievement_source`` says so.
-    unmatched = {}
-    for pk, user_id, content_type_id, object_id in stored.values_list(
-        "pk", "user_id", "source_content_type_id", "source_object_id"
+    # A list of rows per key, not one: rows written before a source was keyed all
+    # share ``(user, NULL)``. A real key can only ever hold one row -
+    # ``unique_automatic_user_achievement_dedup`` says so.
+    stored_keys = {}
+    for pk, user_id, dedup_info in stored.values_list(
+        "pk", "user_id", "dedup_info"
     ).iterator(chunk_size=2000):
-        unmatched.setdefault((user_id, content_type_id, object_id), []).append(pk)
+        stored_keys.setdefault((user_id, dedup_info), []).append(pk)
 
     scope = None if user_ids is None else set(user_ids)
     yielded = added = 0
     changed = set()
+    seen = set()
     pending = {}
 
     def flush():
@@ -383,49 +424,58 @@ def flush():
         if not pending:
             return
         if not dry_run:
-            # ignore_conflicts because ``unmatched`` is a snapshot: a concurrent
+            # ignore_conflicts because ``stored_keys`` is a snapshot: a concurrent
             # run of this same function may have inserted the row since.
             UserAchievement.objects.bulk_create(
                 list(pending.values()), ignore_conflicts=True
             )
         added += len(pending)
+        # An inserted row is not stale, and a key repeated in a later batch must
+        # not be inserted a second time.
+        for key in pending:
+            stored_keys.setdefault(key, [])
         pending = {}
 
-    for user, source in sources.BACKFILL_ITERATORS[slug]():
+    for user, source, dedup_key in sources.BACKFILL_ITERATORS[slug]():
         yielded += 1
+        if dedup_key is None:
+            # Without a key the engine cannot tell this grant from a new one, so
+            # every sweep would re-add it and every reconcile would remove it.
+            raise ValueError(f"Source '{slug}' yielded no dedup key for {source!r}.")
         # A deactivated account is skipped for every source at once, rather than
         # in each iterator, so a source wired later cannot forget the rule. It
         # sits after ``yielded`` on purpose: the refusal below asks whether the
         # source read empty, and "everyone it named is gone" is not that.
         #
         # Skipping is also what removes the grants such an account already holds,
-        # since its key stays in ``unmatched`` and reads as stale. That matters:
+        # since its key is never yielded and so reads as stale. That matters:
         # deleting an account scrubs its grants, but the libraries and commits
         # they derive from name it still, so without this the next sweep would
         # award them all back.
         if not user.is_active:
             continue
-        # The scope is applied here as well as on ``unmatched``: an out-of-scope
+        # The scope is applied here as well as on ``stored_keys``: an out-of-scope
         # member's key is absent from it, which on the additive side is
         # indistinguishable from a grant that needs creating.
         if scope is not None and user.pk not in scope:
             continue
-        content_type = ContentType.objects.get_for_model(source)
-        key = (user.pk, content_type.pk, source.pk)
-        if unmatched.pop(key, None) is not None:
+        key = (user.pk, dedup_key)
+        seen.add(key)
+        if key in stored_keys:
             continue
-        # ``pending`` is keyed, so an iterator that yields the same pair twice
-        # inside one batch counts it once. Across a flush the unique constraint
-        # is what catches it, and only the count is then optimistic.
+        # ``pending`` is keyed, so an iterator naming the same evidence twice
+        # inside one batch counts it once, and ``flush`` carries the key over so
+        # that holds across batches too.
         if not add or key in pending:
             continue
         changed.add(user.pk)
         pending[key] = UserAchievement(
             user_id=user.pk,
             achievement=achievement,
             source_type=SourceType.AUTOMATIC,
-            source_content_type=content_type,
+            source_content_type=ContentType.objects.get_for_model(source),
             source_object_id=source.pk,
+            dedup_info=dedup_key,
         )
         if len(pending) >= batch_size:
             flush()
@@ -434,11 +484,11 @@ def flush():
     # Paired with the member each row belongs to, so a chunk can recalculate the
     # members it just emptied without going back to the database to ask who they
     # were.
-    stale = (
-        [(pk, user_id) for (user_id, _, _), pks in unmatched.items() for pk in pks]
-        if remove
-        else []
-    )
+    stale = []
+    if remove:
+        for key, pks in stored_keys.items():
+            if key not in seen:
+                stale.extend((pk, key[0]) for pk in pks)
     if stale and not yielded and not allow_empty:
         logger.warning(
             "Refusing to remove %s stale grant(s) for '%s': the source yielded "
@@ -454,7 +504,7 @@ def flush():
         )
 
     if remove:
-        changed.update(user_id for user_id, _, _ in unmatched)
+        changed.update(user_id for _, user_id in stale)
 
     recalculated = set()
     if stale and not dry_run: