From d9902146dc5758e50a322b8bb41ed343912be778 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Jun 2026 18:33:33 +0000 Subject: [PATCH] Clean up label-embedding architecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move {id,label} stripping into _SetCriterion.from_json() so both MultiCriterion and ChoiceCriterion normalise at the parse boundary; the querying layer stays typed (list[int] / list[str]) and clean. - Revert MultiCriterion to a thin _extra_q() override; _SetCriterion.to_q() is no longer duplicated. - JS: readSearchSelect always emits {id, label} objects — no conditional mixed-type arrays. filter_bar.js stores them as-is for all fields, removing the fragile isIdField hardcoded list. - Update tests to use the {id, label} filter format. https://claude.ai/code/session_01EyAJcMoDktLrY9tSbdHViA --- common/criteria.py | 59 +++++++++----------------------- games/static/js/filter_bar.js | 39 ++++----------------- games/static/js/search_select.js | 5 ++- tests/test_filter_bars.py | 2 +- tests/test_filters.py | 9 +++++ 5 files changed, 36 insertions(+), 78 deletions(-) diff --git a/common/criteria.py b/common/criteria.py index 64d400a..b3bd7c2 100644 --- a/common/criteria.py +++ b/common/criteria.py @@ -310,57 +310,32 @@ class _SetCriterion(_Criterion): """Hook for subclass-specific modifiers; ``None`` means unsupported.""" return None + @classmethod + def from_json(cls, data: dict | None) -> Self | None: + result = super().from_json(data) + if result is None: + return None + # Labels embedded as {id, label} dicts are display-only; strip to bare ids + # so the querying layer stays clean and typed. + result.value = [item["id"] if isinstance(item, dict) else item for item in result.value] + result.excludes = [item["id"] if isinstance(item, dict) else item for item in result.excludes] + return result + @dataclass class MultiCriterion(_SetCriterion): - """Filter on a many-to-many or ForeignKey relationship by ID list. + """Filter on a many-to-many or ForeignKey relationship by ID list.""" - Each entry in ``value`` and ``excludes`` may be either a bare integer id or - a ``{"id": , "label": }`` dict (Stash-style embedded label). The - label is display-only; only the id is used for querying. - """ - - value: list = field(default_factory=list) - excludes: list = field(default_factory=list) - - def _ids(self, items: list) -> list[int]: - """Extract integer ids from a mixed list of bare ints or {id, label} dicts.""" - result = [] - for item in items: - if isinstance(item, dict): - result.append(int(item["id"])) - else: - result.append(int(item)) - return result - - def to_q(self, field_name: str) -> Q: - modifier = self.modifier - value_ids = self._ids(self.value) - excludes_ids = self._ids(self.excludes) - if modifier in (Modifier.INCLUDES, Modifier.EQUALS): - q = Q() - if value_ids: - q &= Q(**{f"{field_name}__in": value_ids}) - if excludes_ids: - q &= ~Q(**{f"{field_name}__in": excludes_ids}) - return q - if modifier == Modifier.IS_NULL: - return Q(**{f"{field_name}__isnull": True}) - if modifier == Modifier.NOT_NULL: - return Q(**{f"{field_name}__isnull": False}) - extra = self._extra_q(field_name) - if extra is not None: - return extra - raise ValueError(f"Unsupported modifier {modifier} for MultiCriterion") + value: list[int] = field(default_factory=list) + excludes: list[int] = field(default_factory=list) def _extra_q(self, field_name: str) -> Q | None: - value_ids = self._ids(self.value) if self.modifier == Modifier.EXCLUDES: - return ~Q(**{f"{field_name}__in": value_ids}) + return ~Q(**{f"{field_name}__in": self.value}) if self.modifier == Modifier.INCLUDES_ALL: q = Q() - for id_val in value_ids: - q &= Q(**{field_name: id_val}) + for value in self.value: + q &= Q(**{field_name: value}) return q return None diff --git a/games/static/js/filter_bar.js b/games/static/js/filter_bar.js index 64dcc23..54e3a7f 100644 --- a/games/static/js/filter_bar.js +++ b/games/static/js/filter_bar.js @@ -71,38 +71,13 @@ if (modifier === "NOT_NULL" || modifier === "IS_NULL") { filter[field] = { modifier: modifier }; } else if (included.length > 0 || excluded.length > 0) { - var isIdField = - field === "platform" || - field === "game" || - field === "device" || - field === "games"; - if (isIdField) { - // Store {id, label} objects so the filter URL/preset is self-describing - // and pills can be rendered without a DB lookup (Stash-style). - filter[field] = { - value: included.map(function (item) { - return typeof item === "object" - ? {id: parseInt(item.id, 10), label: item.label || ""} - : {id: parseInt(item, 10), label: ""}; - }), - excludes: excluded.map(function (item) { - return typeof item === "object" - ? {id: parseInt(item.id, 10), label: item.label || ""} - : {id: parseInt(item, 10), label: ""}; - }), - modifier: modifier || "INCLUDES", - }; - } else { - filter[field] = { - value: included.map(function (item) { - return typeof item === "object" ? item.id : item; - }), - excludes: excluded.map(function (item) { - return typeof item === "object" ? item.id : item; - }), - modifier: modifier || "INCLUDES", - }; - } + // All filter pills carry {id, label}; store them as-is so the filter + // URL and saved presets are self-describing (Stash-style). + filter[field] = { + value: included.map(function (item) { return {id: item.id, label: item.label}; }), + excludes: excluded.map(function (item) { return {id: item.id, label: item.label}; }), + modifier: modifier || "INCLUDES", + }; } }); diff --git a/games/static/js/search_select.js b/games/static/js/search_select.js index a476a7c..50325f3 100644 --- a/games/static/js/search_select.js +++ b/games/static/js/search_select.js @@ -436,11 +436,10 @@ } var value = pill.getAttribute("data-value"); var label = pill.getAttribute("data-label") || ""; - var entry = label ? {id: value, label: label} : value; if (pill.getAttribute("data-search-select-type") === "exclude") { - excluded.push(entry); + excluded.push({id: value, label: label}); } else { - included.push(entry); + included.push({id: value, label: label}); } }); } diff --git a/tests/test_filter_bars.py b/tests/test_filter_bars.py index 5766f94..55e99f3 100644 --- a/tests/test_filter_bars.py +++ b/tests/test_filter_bars.py @@ -94,7 +94,7 @@ class FilterBarRenderingTest(TestCase): def test_game_filter_bar_roundtrips_selected_status(self): """A status in filter_json renders as an include pill in the widget.""" - filter_json = json.dumps({"status": {"value": ["f"], "modifier": ""}}) + filter_json = json.dumps({"status": {"value": [{"id": "f", "label": "Finished"}], "modifier": "INCLUDES"}}) html = str( FilterBar( filter_json=filter_json, preset_list_url="/l", preset_save_url="/s" diff --git a/tests/test_filters.py b/tests/test_filters.py index 84363f3..5a50d4d 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -121,6 +121,15 @@ class TestMultiCriterion: c = MultiCriterion(value=[], modifier=Modifier.IS_NULL) assert c.to_q("device_id") == Q(device_id__isnull=True) + def test_from_json_strips_embedded_labels(self): + """from_json normalises {id, label} dicts to bare ids.""" + c = MultiCriterion.from_json( + {"value": [{"id": 797, "label": "Hollow Knight"}], "excludes": [{"id": 11, "label": "Steam Deck"}]} + ) + assert c.value == [797] + assert c.excludes == [11] + assert c.to_q("game_id") == Q(game_id__in=[797]) & ~Q(game_id__in=[11]) + class TestChoiceCriterionAgainstDB: """Verify ChoiceCriterion produces correct DB results."""