From 6a2cb4a8810c4395592a607469a998f6e6c04547 Mon Sep 17 00:00:00 2001
From: Colin Powell <colin@unbl.ink>
Date: Sat, 21 Mar 2026 13:44:37 -0400
Subject: [PATCH] [scrobbles] Add PersonScrobble junction table

---
 vrobbler/apps/people/admin.py                 |  10 +-
 .../commands/backfill_scrobble_counts.py      |  23 ++-
 .../0006_add_person_scrobble_junction.py      |  84 ++++++++
 vrobbler/apps/people/models.py                |  59 ++++--
 vrobbler/apps/people/signals.py               |  64 +++++--
 vrobbler/apps/people/tests/__init__.py        |   0
 .../apps/people/tests/test_person_scrobble.py | 180 ++++++++++++++++++
 vrobbler/apps/people/views.py                 |  29 ++-
 8 files changed, 401 insertions(+), 48 deletions(-)
 create mode 100644 vrobbler/apps/people/migrations/0006_add_person_scrobble_junction.py
 create mode 100644 vrobbler/apps/people/tests/__init__.py
 create mode 100644 vrobbler/apps/people/tests/test_person_scrobble.py

diff --git a/vrobbler/apps/people/admin.py b/vrobbler/apps/people/admin.py
index 4922336..20240b3 100644
--- a/vrobbler/apps/people/admin.py
+++ b/vrobbler/apps/people/admin.py
@@ -1,5 +1,5 @@
 from django.contrib import admin
-from people.models import Person
+from people.models import Person, PersonScrobble
 
 
 @admin.register(Person)
@@ -8,3 +8,11 @@ class PersonAdmin(admin.ModelAdmin):
     list_display = ("name", "bgg_username", "bgstats_id")
     ordering = ("-created",)
     search_fields = ("name",)
+
+
+@admin.register(PersonScrobble)
+class PersonScrobbleAdmin(admin.ModelAdmin):
+    list_display = ("person", "user", "scrobble", "created")
+    list_filter = ("created",)
+    ordering = ("-created",)
+    raw_id_fields = ("person", "user", "scrobble")
diff --git a/vrobbler/apps/people/management/commands/backfill_scrobble_counts.py b/vrobbler/apps/people/management/commands/backfill_scrobble_counts.py
index a89805f..6aa3039 100644
--- a/vrobbler/apps/people/management/commands/backfill_scrobble_counts.py
+++ b/vrobbler/apps/people/management/commands/backfill_scrobble_counts.py
@@ -1,15 +1,24 @@
 from django.core.management.base import BaseCommand
 
-from people.models import Person
+from people.models import PersonScrobble
+from people.signals import get_person_ids_from_log
+from scrobbles.models import Scrobble
 
 
 class Command(BaseCommand):
-    help = "Backfill scrobble counts for all people"
+    help = "Backfill PersonScrobble junction table from existing scrobbles"
 
     def handle(self, *args, **options):
-        for person in Person.objects.all():
-            person.update_scrobble_count()
-            self.stdout.write(
-                f"{person.name}: {person.scrobble_count} scrobbles"
-            )
+        count = 0
+        for scrobble in Scrobble.objects.all():
+            person_ids = get_person_ids_from_log(scrobble.log)
+            for person_id in person_ids:
+                obj, created = PersonScrobble.objects.get_or_create(
+                    person_id=person_id,
+                    user=scrobble.user,
+                    scrobble=scrobble,
+                )
+                if created:
+                    count += 1
+        self.stdout.write(f"Created {count} PersonScrobble records")
         self.stdout.write(self.style.SUCCESS("Done!"))
diff --git a/vrobbler/apps/people/migrations/0006_add_person_scrobble_junction.py b/vrobbler/apps/people/migrations/0006_add_person_scrobble_junction.py
new file mode 100644
index 0000000..da24b28
--- /dev/null
+++ b/vrobbler/apps/people/migrations/0006_add_person_scrobble_junction.py
@@ -0,0 +1,84 @@
+# Generated by Django 4.2.29 on 2026-03-21 17:14
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+import django_extensions.db.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("scrobbles", "0072_add_scrobble_indexes"),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ("people", "0005_person_boardgamearena_id"),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name="person",
+            name="scrobble_count",
+        ),
+        migrations.CreateModel(
+            name="PersonScrobble",
+            fields=[
+                (
+                    "id",
+                    models.BigAutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                (
+                    "created",
+                    django_extensions.db.fields.CreationDateTimeField(
+                        auto_now_add=True, verbose_name="created"
+                    ),
+                ),
+                (
+                    "modified",
+                    django_extensions.db.fields.ModificationDateTimeField(
+                        auto_now=True, verbose_name="modified"
+                    ),
+                ),
+                (
+                    "person",
+                    models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        related_name="scrobble_associations",
+                        to="people.person",
+                    ),
+                ),
+                (
+                    "scrobble",
+                    models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        to="scrobbles.scrobble",
+                    ),
+                ),
+                (
+                    "user",
+                    models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        related_name="person_scrobbles",
+                        to=settings.AUTH_USER_MODEL,
+                    ),
+                ),
+            ],
+            options={
+                "indexes": [
+                    models.Index(
+                        fields=["person", "user"],
+                        name="people_pers_person__b7ebc7_idx",
+                    ),
+                    models.Index(
+                        fields=["user", "scrobble"],
+                        name="people_pers_user_id_4f5fe7_idx",
+                    ),
+                ],
+                "unique_together": {("person", "scrobble")},
+            },
+        ),
+    ]
diff --git a/vrobbler/apps/people/models.py b/vrobbler/apps/people/models.py
index 68e4236..0c03c66 100644
--- a/vrobbler/apps/people/models.py
+++ b/vrobbler/apps/people/models.py
@@ -1,5 +1,6 @@
 from django.contrib.auth import get_user_model
 from django.db import models
+from django.shortcuts import get_object_or_404
 from django_extensions.db.models import TimeStampedModel
 
 User = get_user_model()
@@ -22,25 +23,51 @@ class Person(TimeStampedModel):
     bgg_username = models.CharField(max_length=100, **BNULL)
     lichess_username = models.CharField(max_length=100, **BNULL)
     bio = models.TextField(**BNULL)
-    scrobble_count = models.IntegerField(default=0)
 
     def __str__(self):
         return self.name
 
-    def update_scrobble_count(self):
+    def get_scrobble_count(self, user=None):
+        if user is None:
+            user = self.created_by
+        return PersonScrobble.objects.filter(person=self, user=user).count()
+
+    def get_scrobbles(self, user=None):
+        if user is None:
+            user = self.created_by
         from scrobbles.models import Scrobble
 
-        count = 0
-        for scrobble in Scrobble.objects.filter(user=self.created_by):
-            if scrobble.log and isinstance(scrobble.log, dict):
-                person_ids = scrobble.log.get("with_people_ids") or []
-                if person_ids and self.id in person_ids:
-                    count += 1
-                    continue
-                players = scrobble.log.get("players") or []
-                for player in players:
-                    if isinstance(player, dict) and player.get("person_id") == self.id:
-                        count += 1
-                        break
-        self.scrobble_count = count
-        self.save(update_fields=["scrobble_count"])
+        return Scrobble.objects.filter(
+            id__in=PersonScrobble.objects.filter(person=self, user=user).values_list(
+                "scrobble_id", flat=True
+            )
+        ).order_by("-timestamp")
+
+
+class PersonScrobble(TimeStampedModel):
+    """Tracks the relationship between a Person and a Scrobble."""
+
+    person = models.ForeignKey(
+        Person,
+        on_delete=models.CASCADE,
+        related_name="scrobble_associations",
+    )
+    user = models.ForeignKey(
+        User,
+        on_delete=models.CASCADE,
+        related_name="person_scrobbles",
+    )
+    scrobble = models.ForeignKey(
+        "scrobbles.Scrobble",
+        on_delete=models.CASCADE,
+    )
+
+    class Meta:
+        unique_together = ["person", "scrobble"]
+        indexes = [
+            models.Index(fields=["person", "user"]),
+            models.Index(fields=["user", "scrobble"]),
+        ]
+
+    def __str__(self):
+        return f"{self.person.name} - {self.scrobble_id}"
diff --git a/vrobbler/apps/people/signals.py b/vrobbler/apps/people/signals.py
index 86b3f24..11b69a5 100644
--- a/vrobbler/apps/people/signals.py
+++ b/vrobbler/apps/people/signals.py
@@ -4,25 +4,55 @@ from django.dispatch import receiver
 from scrobbles.models import Scrobble
 
 
-@receiver(post_save, sender=Scrobble)
-def update_person_scrobble_count_on_save(sender, instance, **kwargs):
-    if instance.log and isinstance(instance.log, dict):
-        person_ids = instance.log.get("with_people_ids", [])
-        for person_id in person_ids:
-            from people.models import Person
+def get_person_ids_from_log(log):
+    if not log or not isinstance(log, dict):
+        return set()
 
-            person = Person.objects.filter(id=person_id).first()
-            if person:
-                person.update_scrobble_count()
+    person_ids = set()
+
+    with_people_ids = log.get("with_people_ids") or []
+    if isinstance(with_people_ids, list):
+        person_ids.update(with_people_ids)
+
+    players = log.get("players") or []
+    if isinstance(players, list):
+        for player in players:
+            if isinstance(player, dict) and player.get("person_id"):
+                person_ids.add(player["person_id"])
+
+    return person_ids
+
+
+@receiver(post_save, sender=Scrobble)
+def sync_person_scrobbles_on_save(sender, instance, **kwargs):
+    from people.models import Person, PersonScrobble
+
+    person_ids = get_person_ids_from_log(instance.log)
+
+    existing = set(
+        PersonScrobble.objects.filter(scrobble=instance).values_list(
+            "person_id", flat=True
+        )
+    )
+
+    for person_id in person_ids:
+        person = Person.objects.filter(id=person_id).first()
+        if person and person.user and person.user == instance.user:
+            continue
+        if person_id not in existing:
+            PersonScrobble.objects.get_or_create(
+                person_id=person_id,
+                user=instance.user,
+                scrobble=instance,
+            )
+
+    PersonScrobble.objects.filter(scrobble=instance).exclude(
+        person_id__in=person_ids
+    ).delete()
 
 
 @receiver(post_delete, sender=Scrobble)
-def update_person_scrobble_count_on_delete(sender, instance, **kwargs):
-    if instance.log and isinstance(instance.log, dict):
-        person_ids = instance.log.get("with_people_ids", [])
-        for person_id in person_ids:
-            from people.models import Person
+def sync_person_scrobbles_on_delete(sender, instance, **kwargs):
+    from people.models import PersonScrobble
 
-            person = Person.objects.filter(id=person_id).first()
-            if person:
-                person.update_scrobble_count()
+    PersonScrobble.objects.filter(scrobble=instance).delete()
diff --git a/vrobbler/apps/people/tests/__init__.py b/vrobbler/apps/people/tests/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/vrobbler/apps/people/tests/test_person_scrobble.py b/vrobbler/apps/people/tests/test_person_scrobble.py
new file mode 100644
index 0000000..ba746c7
--- /dev/null
+++ b/vrobbler/apps/people/tests/test_person_scrobble.py
@@ -0,0 +1,180 @@
+import pytest
+
+from django.contrib.auth import get_user_model
+from scrobbles.models import Scrobble
+
+from people.models import Person, PersonScrobble
+
+User = get_user_model()
+
+
+@pytest.mark.django_db
+def test_signal_creates_person_scrobble_on_save_with_people_ids():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Alice", created_by=creator, user=creator)
+
+    scrobble = Scrobble.objects.create(
+        user=other_user,
+        log={"with_people_ids": [person.id]},
+    )
+
+    assert PersonScrobble.objects.filter(person=person, user=other_user).count() == 1
+
+
+@pytest.mark.django_db
+def test_signal_creates_person_scrobble_on_save_with_players():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Bob", created_by=creator, user=creator)
+
+    scrobble = Scrobble.objects.create(
+        user=other_user,
+        log={
+            "players": [
+                {"person_id": person.id, "win": True, "rank": 1},
+            ]
+        },
+    )
+
+    assert PersonScrobble.objects.filter(person=person, user=other_user).count() == 1
+
+
+@pytest.mark.django_db
+def test_signal_removes_person_scrobble_when_person_removed_from_with_people_ids():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Charlie", created_by=creator, user=creator)
+
+    scrobble = Scrobble.objects.create(
+        user=other_user,
+        log={"with_people_ids": [person.id]},
+    )
+    assert PersonScrobble.objects.filter(person=person).count() == 1
+
+    scrobble.log = {}
+    scrobble.save()
+
+    assert PersonScrobble.objects.filter(person=person).count() == 0
+
+
+@pytest.mark.django_db
+def test_signal_removes_person_scrobble_when_person_removed_from_players():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Diana", created_by=creator, user=creator)
+
+    scrobble = Scrobble.objects.create(
+        user=other_user,
+        log={
+            "players": [
+                {"person_id": person.id, "win": True, "rank": 1},
+            ]
+        },
+    )
+    assert PersonScrobble.objects.filter(person=person).count() == 1
+
+    scrobble.log = {"players": []}
+    scrobble.save()
+
+    assert PersonScrobble.objects.filter(person=person).count() == 0
+
+
+@pytest.mark.django_db
+def test_signal_deletes_person_scrobble_on_scrobble_delete():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Eve", created_by=creator, user=creator)
+
+    scrobble = Scrobble.objects.create(
+        user=other_user,
+        log={"with_people_ids": [person.id]},
+    )
+    assert PersonScrobble.objects.filter(person=person).count() == 1
+
+    scrobble.delete()
+
+    assert PersonScrobble.objects.filter(person=person).count() == 0
+
+
+@pytest.mark.django_db
+def test_signal_skips_when_person_user_matches_scrobble_user():
+    user = User.objects.create_user(username="testuser", password="testpass123")
+    person = Person.objects.create(name="Frank", created_by=user, user=user)
+
+    scrobble = Scrobble.objects.create(
+        user=user,
+        log={"with_people_ids": [person.id]},
+    )
+
+    assert PersonScrobble.objects.filter(person=person).count() == 0
+
+
+@pytest.mark.django_db
+def test_signal_works_with_different_users():
+    user1 = User.objects.create_user(username="user1", password="testpass123")
+    user2 = User.objects.create_user(username="user2", password="testpass123")
+    person = Person.objects.create(name="Grace", created_by=user1, user=user1)
+
+    scrobble1 = Scrobble.objects.create(
+        user=user1,
+        log={"with_people_ids": [person.id]},
+    )
+    scrobble2 = Scrobble.objects.create(
+        user=user2,
+        log={"with_people_ids": [person.id]},
+    )
+
+    assert PersonScrobble.objects.filter(person=person, user=user1).count() == 0
+    assert PersonScrobble.objects.filter(person=person, user=user2).count() == 1
+
+
+@pytest.mark.django_db
+def test_person_get_scrobble_count():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Henry", created_by=creator, user=creator)
+
+    Scrobble.objects.create(user=other_user, log={"with_people_ids": [person.id]})
+    Scrobble.objects.create(user=other_user, log={"with_people_ids": [person.id]})
+    Scrobble.objects.create(user=other_user, log={})
+
+    assert person.get_scrobble_count(user=other_user) == 2
+
+
+@pytest.mark.django_db
+def test_person_get_scrobbles():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Iris", created_by=creator, user=creator)
+
+    scrobble1 = Scrobble.objects.create(
+        user=other_user, log={"with_people_ids": [person.id]}
+    )
+    scrobble2 = Scrobble.objects.create(
+        user=other_user, log={"with_people_ids": [person.id]}
+    )
+    Scrobble.objects.create(user=other_user, log={})
+
+    scrobbles = person.get_scrobbles(user=other_user)
+
+    assert scrobbles.count() == 2
+    assert scrobble1 in scrobbles
+    assert scrobble2 in scrobbles
+
+
+@pytest.mark.django_db
+def test_person_scrobble_unique_together():
+    creator = User.objects.create_user(username="creator", password="testpass123")
+    other_user = User.objects.create_user(username="other", password="testpass123")
+    person = Person.objects.create(name="Jack", created_by=creator, user=creator)
+
+    scrobble = Scrobble.objects.create(
+        user=other_user,
+        log={
+            "with_people_ids": [person.id],
+            "players": [{"person_id": person.id}],
+        },
+    )
+
+    assert PersonScrobble.objects.filter(person=person, scrobble=scrobble).count() == 1
diff --git a/vrobbler/apps/people/views.py b/vrobbler/apps/people/views.py
index 0f7f5c8..4700494 100644
--- a/vrobbler/apps/people/views.py
+++ b/vrobbler/apps/people/views.py
@@ -1,7 +1,8 @@
 from django.views import generic
 from django.urls import reverse_lazy
+from django.db.models import Count, Q
 
-from people.models import Person
+from people.models import Person, PersonScrobble
 
 
 class PersonCreateUpdateView(generic.CreateView):
@@ -24,9 +25,16 @@ class PersonCreateUpdateView(generic.CreateView):
 
     def get_context_data(self, **kwargs):
         context_data = super().get_context_data(**kwargs)
-        context_data["people"] = Person.objects.filter(
-            created_by=self.request.user
-        ).order_by("-scrobble_count", "name")
+        context_data["people"] = (
+            Person.objects.filter(created_by=self.request.user)
+            .annotate(
+                scrobble_count=Count(
+                    "scrobble_associations",
+                    filter=Q(scrobble_associations__user=self.request.user),
+                )
+            )
+            .order_by("-scrobble_count", "name")
+        )
         return context_data
 
     def form_valid(self, form):
@@ -49,7 +57,14 @@ class PersonUpdateView(generic.UpdateView):
 
     def get_context_data(self, **kwargs):
         context_data = super().get_context_data(**kwargs)
-        context_data["people"] = Person.objects.filter(
-            created_by=self.request.user
-        ).order_by("-scrobble_count", "name")
+        context_data["people"] = (
+            Person.objects.filter(created_by=self.request.user)
+            .annotate(
+                scrobble_count=Count(
+                    "scrobble_associations",
+                    filter=Q(scrobble_associations__user=self.request.user),
+                )
+            )
+            .order_by("-scrobble_count", "name")
+        )
         return context_data