Merge pull request #917 from dannyvfilms/claude/list-filter-option-nhvrcc

Add list filter support to smart list rules

Author: dannyvfilms

src/app/signals.py

@@ -8,7 +8,7 @@
 from django.apps import apps
 from django.conf import settings
 from django.db import transaction
-from django.db.models.signals import post_delete, post_save, pre_save
+from django.db.models.signals import m2m_changed, post_delete, post_save, pre_save
 from django.db.utils import OperationalError
 from django.dispatch import receiver
 from django.utils import timezone
@@ -47,6 +47,7 @@
     Season,
     Sources,
 )
+from lists.models import CustomList, CustomListItem
 from lists.smart_rules import sync_smart_lists_for_item
 
 logger = logging.getLogger(__name__)
@@ -393,6 +394,48 @@ def sync_smart_lists_on_item_tag_change(sender, instance, **kwargs):
     _sync_owner_smart_lists_for_items(owner, [item])
 
 
+@receiver([post_save, post_delete], sender=CustomListItem)
+def sync_smart_lists_on_list_membership_change(sender, instance, **kwargs):
+    """Re-evaluate referencing smart lists when a manual list's membership changes.
+
+    A smart list's "List" filter includes the full contents of any linked
+    (non-smart) list. Membership changes on that manual list happen outside
+    the normal media-tracking signals below, so they need their own hook.
+    """
+    if kwargs.get("raw"):
+        return
+    custom_list = getattr(instance, "custom_list", None)
+    item = getattr(instance, "item", None)
+    if not custom_list or not item or custom_list.is_smart:
+        return
+
+    owners = {custom_list.owner}
+    owners.update(custom_list.collaborators.all())
+    for owner in owners:
+        _sync_owner_smart_lists_for_items(owner, [item])
+
+
+@receiver(m2m_changed, sender=CustomList.items.through)
+def sync_smart_lists_on_list_items_m2m_change(
+    sender, instance, action, reverse, pk_set, **kwargs
+):
+    """Catch list-membership writes that bypass CustomListItem's save/delete.
+
+    `CustomList.items.add()` creates `CustomListItem` rows via the m2m
+    manager's `bulk_create()`, which doesn't call `save()`, so
+    `sync_smart_lists_on_list_membership_change` above never fires for it.
+    `m2m_changed` fires regardless of how the through rows were written.
+    """
+    if action != "post_add" or reverse or instance.is_smart or not pk_set:
+        return
+
+    items = list(Item.objects.filter(id__in=pk_set))
+    owners = {instance.owner}
+    owners.update(instance.collaborators.all())
+    for owner in owners:
+        _sync_owner_smart_lists_for_items(owner, items)
+
+
 @receiver(post_save, sender=Item)
 def sync_smart_lists_on_watch_providers_change(
     sender, instance, update_fields=None, **kwargs

src/lists/imports/mdblist.py

@@ -17,6 +17,7 @@
 from app.models import Item, MediaTypes, Sources
 from app.providers import services
 from integrations.imports import helpers
+from lists.smart_rules import sync_smart_lists_for_item
 
 # Shared MDBList client helpers live with the full-account importer; keep the
 # old private names so existing call sites and test patch targets still work.
@@ -173,6 +174,8 @@ def _sync_list(user, api_key, list_info):
             continue
         desired_item_ids.add(item.id)
 
+    changed_item_ids = set()
+
     def _apply():
         with transaction.atomic():
             custom_list, _ = CustomList.objects.update_or_create(
@@ -184,22 +187,41 @@ def _apply():
                     "description": list_info.get("description") or "",
                 },
             )
-            custom_list.customlistitem_set.exclude(
-                item_id__in=desired_item_ids,
+            removed_item_ids = set(
+                custom_list.customlistitem_set.exclude(
+                    item_id__in=desired_item_ids,
+                ).values_list("item_id", flat=True),
+            )
+            custom_list.customlistitem_set.filter(
+                item_id__in=removed_item_ids,
             ).delete()
             existing_item_ids = set(
                 custom_list.customlistitem_set.values_list("item_id", flat=True),
             )
+            added_item_ids = desired_item_ids - existing_item_ids
             CustomListItem.objects.bulk_create(
                 CustomListItem(
                     custom_list=custom_list,
                     item_id=item_id,
                     added_by=user,
                 )
-                for item_id in desired_item_ids - existing_item_ids
+                for item_id in added_item_ids
             )
+            changed_item_ids.update(removed_item_ids, added_item_ids)
 
     helpers.retry_on_lock(_apply)
+
+    # bulk_create() and the bulk .delete() above bypass CustomListItem's
+    # save()/delete(), so the smart-list membership signal never fires for
+    # them; resync explicitly for whatever actually changed.
+    for item_id in changed_item_ids:
+        try:
+            sync_smart_lists_for_item(owner=user, item=Item(id=item_id))
+        except Exception:
+            logger.exception(
+                "Failed to sync smart lists for item %s after MDBList list sync",
+                item_id,
+            )
     logger.info(
         "Synced MDBList list %s (%s) for %s: %s items, %s skipped",
         list_id,

src/lists/smart_rules.py

@@ -41,6 +41,7 @@
     "provider",
     "tag",
     "tag_mode",
+    "list",
 )
 
 TAG_MODE_CHOICES = {"and", "or", "not"}
@@ -72,6 +73,7 @@
     "provider": "",
     "tag": [],
     "tag_mode": "or",
+    "list": [],
 }
 
 MAX_RATING = 10.0
@@ -242,6 +244,22 @@ def _payload_getlist(payload, key: str) -> list[str]:
     return []
 
 
+def _valid_linked_list_ids(owner, raw_values: list[str]) -> list[int]:
+    """Return ids from raw_values that are non-smart lists owner can access."""
+    candidate_ids = {int(value) for value in raw_values if str(value).strip().isdigit()}
+    if not candidate_ids or not owner:
+        return []
+
+    from lists.models import CustomList
+
+    return list(
+        CustomList.objects.filter(id__in=candidate_ids, is_smart=False)
+        .filter(Q(owner=owner) | Q(collaborators=owner))
+        .distinct()
+        .values_list("id", flat=True),
+    )
+
+
 def get_available_media_types(owner) -> list[str]:
     """Return enabled media types that can participate in smart rules."""
     if owner and hasattr(owner, "get_enabled_media_types"):
@@ -368,6 +386,8 @@ def normalize_rule_payload(payload, owner):
         seen_tags.add(key)
         deduped_tags.append(value)
 
+    list_ids = _valid_linked_list_ids(owner, _payload_getlist(payload, "list"))
+
     return {
         "media_types": normalized_media_types,
         "status": normalized_statuses,
@@ -396,6 +416,7 @@ def normalize_rule_payload(payload, owner):
         "provider": str(_payload_get(payload, "provider", "") or "").strip(),
         "tag": deduped_tags,
         "tag_mode": tag_mode,
+        "list": list_ids,
     }
 
 
@@ -836,6 +857,21 @@ def _resolve_tag_id_sets(
     return set().union(*per_tag_id_sets), None
 
 
+def _resolve_list_membership_item_ids(list_ids: list[int]) -> set[int]:
+    """Return the union of item ids belonging to the given linked lists."""
+    if not list_ids:
+        return set()
+
+    from lists.models import CustomListItem
+
+    return set(
+        CustomListItem.objects.filter(custom_list_id__in=list_ids).values_list(
+            "item_id",
+            flat=True,
+        ),
+    )
+
+
 def collect_matching_item_ids(
     owner,
     normalized_rules: dict,
@@ -981,6 +1017,8 @@ def _tag_filter_excludes(item_id: int) -> bool:
                             continue
                         matched_ids.add(item.id)
 
+    matched_ids |= _resolve_list_membership_item_ids(normalized_rules.get("list") or [])
+
     return matched_ids
 
 
@@ -995,6 +1033,16 @@ def item_matches_rules(
     if not owner or not item:
         return False
 
+    list_ids = normalized_rules.get("list") or []
+    if list_ids:
+        from lists.models import CustomListItem
+
+        if CustomListItem.objects.filter(
+            custom_list_id__in=list_ids,
+            item_id=item.id,
+        ).exists():
+            return True
+
     target_media_types = _target_media_types(
         owner, normalized_rules.get("media_types", [])
     )
@@ -1151,6 +1199,7 @@ def build_rule_filter_data(
     *,
     include_collection_only_untracked: bool = False,
     precomputed_tags: list[str] | None = None,
+    include_list_options: bool = True,
 ):
     """Build menu options for smart-rule filters from matched candidate media."""
     target_media_types = _target_media_types(owner, media_types)
@@ -1345,4 +1394,15 @@ def build_rule_filter_data(
             .order_by("name")
         )
 
+    filter_data["lists"] = []
+    if include_list_options:
+        from lists.models import CustomList
+
+        filter_data["lists"] = [
+            {"id": custom_list.id, "label": custom_list.name}
+            for custom_list in CustomList.objects.get_user_lists(owner)
+            .filter(is_smart=False)
+            .order_by("name")
+        ]
+
     return filter_data

src/lists/tests/test_models.py

@@ -304,6 +304,119 @@ def test_smart_rules_support_multi_status_and_tag_modes(self):
                 expected_ids,
             )
 
+    def test_normalize_rule_payload_restricts_list_filter_to_accessible_manual_lists(
+        self,
+    ):
+        """The list filter should only accept non-smart lists owner can access."""
+        inaccessible_list = CustomList.objects.create(
+            name="Not Mine",
+            owner=self.other_user,
+        )
+        smart_target = CustomList.objects.create(
+            name="Smart Target",
+            owner=self.user,
+            is_smart=True,
+        )
+
+        rules = smart_rules.normalize_rule_payload(
+            {
+                "list": [
+                    self.list1.id,
+                    self.list2.id,
+                    inaccessible_list.id,
+                    smart_target.id,
+                ],
+            },
+            self.user,
+        )
+
+        self.assertCountEqual(rules["list"], [self.list1.id, self.list2.id])
+
+    def test_collect_matching_item_ids_unions_linked_list_contents(self):
+        """List filter should include linked lists' items even if other filters fail."""
+        linked_item = Item.objects.create(
+            title="Untracked Linked Game",
+            media_id="link-1",
+            media_type=MediaTypes.GAME.value,
+            source=Sources.IGDB.value,
+        )
+        CustomListItem.objects.create(
+            custom_list=self.list1,
+            item=linked_item,
+            added_by=self.user,
+        )
+        other_item = Item.objects.create(
+            title="Unrelated Movie",
+            media_id="link-2",
+            media_type=MediaTypes.MOVIE.value,
+            source=Sources.TMDB.value,
+        )
+        Movie.objects.create(
+            item=other_item, user=self.user, status=Status.COMPLETED.value
+        )
+
+        rules = smart_rules.normalize_rule_payload(
+            {
+                "media_types": [MediaTypes.MOVIE.value],
+                "genre": "Nonexistent Genre",
+                "list": [self.list1.id],
+            },
+            self.user,
+        )
+
+        matched_ids = smart_rules.collect_matching_item_ids(self.user, rules)
+
+        self.assertIn(linked_item.id, matched_ids)
+        self.assertNotIn(other_item.id, matched_ids)
+
+    def test_item_matches_rules_short_circuits_for_linked_list_membership(self):
+        """A linked list's item should match regardless of other active filters."""
+        linked_item = Item.objects.create(
+            title="Linked Game",
+            media_id="link-3",
+            media_type=MediaTypes.GAME.value,
+            source=Sources.IGDB.value,
+        )
+        CustomListItem.objects.create(
+            custom_list=self.list1,
+            item=linked_item,
+            added_by=self.user,
+        )
+
+        rules = smart_rules.normalize_rule_payload(
+            {"genre": "Nonexistent Genre", "list": [self.list1.id]},
+            self.user,
+        )
+
+        self.assertTrue(smart_rules.item_matches_rules(self.user, linked_item, rules))
+
+    def test_manual_list_membership_change_syncs_referencing_smart_lists(self):
+        """Adding/removing an item on a linked manual list should resync smart lists."""
+        linked_item = Item.objects.create(
+            title="Freshly Linked Game",
+            media_id="link-4",
+            media_type=MediaTypes.GAME.value,
+            source=Sources.IGDB.value,
+        )
+        smart_list = CustomList.objects.create(
+            name="All Owned Games",
+            owner=self.user,
+            is_smart=True,
+            smart_filters={"list": [self.list1.id]},
+        )
+
+        membership = CustomListItem.objects.create(
+            custom_list=self.list1,
+            item=linked_item,
+            added_by=self.user,
+        )
+
+        self.assertTrue(smart_list.items.filter(id=linked_item.id).exists())
+
+        membership.delete()
+
+        self.assertFalse(smart_list.items.filter(id=linked_item.id).exists())
+
     def test_smart_list_collection_filter_uses_episode_collection_for_tv(self):
         """Collected TV rules should match when related episodes are collected."""
         tv_item = Item.objects.create(