From 737dd9275b19995dd2aa401c7283148720e6a4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Tue, 9 Jun 2026 00:07:36 +0200 Subject: [PATCH] Fix includes all query returning duplicates --- games/filters.py | 34 +++++++-- games/static/js/search_select.js | 115 +++++++++++++++++++++++++++++-- tests/test_filters.py | 27 ++++++++ 3 files changed, 166 insertions(+), 10 deletions(-) diff --git a/games/filters.py b/games/filters.py index 2b6de47..0cc04c5 100644 --- a/games/filters.py +++ b/games/filters.py @@ -395,13 +395,26 @@ class PurchaseFilter(OperatorFilter): join and would require a single link row to be both games. Instead chain a filter per game so each gets its own join, then match by ``pk``. ``INCLUDES_ONLY`` additionally excludes purchases that have - any game outside the specified set. The orthogonal ``excludes`` - channel is applied as a negative, consistent with every other - modifier. All other modifiers delegate to the criterion. - """ - if criterion.modifier in (Modifier.INCLUDES_ALL, Modifier.INCLUDES_ONLY) and criterion.value: - from games.models import Game, Purchase + any game outside the specified set. + ``INCLUDES`` (plain "any") also uses a subquery instead of a raw + ``games__in`` join because a single purchase linked to *n* of the + given games would appear *n* times in the result set (M2M join + duplicates). + + The orthogonal ``excludes`` channel is applied as a negative, + consistent with every other modifier. All other modifiers delegate + to the criterion. + """ + # Empty value means no constraint; still apply excludes if any + if not criterion.value: + if criterion.excludes: + return ~Q(games__in=criterion.excludes) + return Q() + + from games.models import Game, Purchase + + if criterion.modifier in (Modifier.INCLUDES_ALL, Modifier.INCLUDES_ONLY): subquery = Purchase.objects.all() for game_id in criterion.value: subquery = subquery.filter(games=game_id) @@ -417,6 +430,15 @@ class PurchaseFilter(OperatorFilter): if criterion.excludes: q &= ~Q(games__in=criterion.excludes) return q + + if criterion.modifier == Modifier.INCLUDES: + # Use subquery to avoid duplicate rows from M2M join + subquery = Purchase.objects.filter(games__in=criterion.value) + q = Q(pk__in=subquery.values("pk")) + if criterion.excludes: + q &= ~Q(games__in=criterion.excludes) + return q + return criterion.to_q("games") diff --git a/games/static/js/search_select.js b/games/static/js/search_select.js index a384fb4..4b9242b 100644 --- a/games/static/js/search_select.js +++ b/games/static/js/search_select.js @@ -65,6 +65,60 @@ if (noResults) noResults.classList.toggle("hidden", !visible); } + // ── Highlight tracking (filter mode) ── + var highlightedRow = null; + + function highlightOption(row) { + clearHighlight(); + if (!row) return; + row.style.backgroundColor = "var(--color-brand, rgba(59, 130, 246, 0.15))"; + row.style.outline = "1px solid var(--color-brand, #3b82f6)"; + highlightedRow = row; + row.scrollIntoView({ block: "nearest" }); + } + + function clearHighlight() { + if (highlightedRow) { + highlightedRow.style.backgroundColor = ""; + highlightedRow.style.outline = ""; + highlightedRow = null; + } + } + + function getVisibleOptions() { + var all = options.querySelectorAll("[data-search-select-option]"); + return Array.prototype.filter.call(all, function (row) { + return row.style.display !== "none"; + }); + } + + function autoHighlight(query) { + var visible = getVisibleOptions(); + if (visible.length === 0) { + clearHighlight(); + return; + } + var lower = query.toLowerCase(); + // 1. Starts-with match + for (var i = 0; i < visible.length; i++) { + var label = (visible[i].getAttribute("data-label") || "").toLowerCase(); + if (lower && label.startsWith(lower)) { + highlightOption(visible[i]); + return; + } + } + // 2. Substring match (fuzzy-lite) + for (var j = 0; j < visible.length; j++) { + var subLabel = (visible[j].getAttribute("data-label") || "").toLowerCase(); + if (lower && subLabel.indexOf(lower) !== -1) { + highlightOption(visible[j]); + return; + } + } + // 3. Fallback: first visible option + highlightOption(visible[0]); + } + // ── Render server-fetched rows into the panel ── function renderRows(items) { options.querySelectorAll("[data-search-select-option]").forEach(function (row) { @@ -140,6 +194,7 @@ renderRows(items); // Re-apply the live query: the box may hold more text than was sent. setNoResults(filterRows(search.value.trim()) === 0); + if (isFilter) autoHighlight(search.value.trim()); }) .catch(function (error) { if (error && error.name === "AbortError") return; // superseded @@ -166,6 +221,7 @@ } else { setNoResults(filterRows(query) === 0); } + if (isFilter) autoHighlight(query); } // ── Single-select combobox: the search box shows the committed label; @@ -188,12 +244,15 @@ // Show whatever is already loaded; the server decides no-results. filterRows(search.value.trim()); setNoResults(false); + if (isFilter) autoHighlight(search.value.trim()); } } else { setNoResults(filterRows(search.value.trim()) === 0); + if (isFilter) autoHighlight(search.value.trim()); } }); search.addEventListener("input", function () { + clearHighlight(); if (!multi) container._searchSelectDirty = true; runSearch(); }); @@ -215,6 +274,45 @@ }); } + // ── Keyboard navigation (filter mode) ── + search.addEventListener("keydown", function (event) { + if (!isFilter) return; + var key = event.key; + if (key === "ArrowDown" || key === "ArrowUp" || key === "Enter" || key === "Escape") { + var visible = getVisibleOptions(); + if (visible.length === 0) { + if (key === "Escape") hidePanel(); + return; + } + + if (key === "ArrowDown") { + event.preventDefault(); + showPanel(); + var idx = highlightedRow ? visible.indexOf(highlightedRow) : -1; + var next = visible[(idx + 1) % visible.length]; + highlightOption(next); + } else if (key === "ArrowUp") { + event.preventDefault(); + showPanel(); + var idx = highlightedRow ? visible.indexOf(highlightedRow) : -1; + var prev = visible[(idx - 1 + visible.length) % visible.length]; + highlightOption(prev); + } else if (key === "Enter") { + if (highlightedRow) { + event.preventDefault(); + var option = optionFromRow(highlightedRow); + addFilterPill(option, "include"); + search.value = ""; + clearHighlight(); + hidePanel(); + } + } else if (key === "Escape") { + clearHighlight(); + hidePanel(); + } + } + }); + // Clicking an option must not blur the input before the click selects. options.addEventListener("mousedown", function (event) { event.preventDefault(); @@ -243,10 +341,18 @@ } // Include / exclude button on a value row. var button = event.target.closest("[data-search-select-action]"); - if (!button) return; - var row = button.closest("[data-search-select-option]"); - if (!row) return; - addFilterPill(optionFromRow(row), button.getAttribute("data-search-select-action")); + if (button) { + var row = button.closest("[data-search-select-option]"); + if (!row) return; + addFilterPill(optionFromRow(row), button.getAttribute("data-search-select-action")); + return; + } + // Click on the option row itself → include. + var optionRow = event.target.closest("[data-search-select-option]"); + if (optionRow) { + addFilterPill(optionFromRow(optionRow), "include"); + return; + } } // Add (or re-type) an include/exclude pill for a value. Selecting any value @@ -258,6 +364,7 @@ ); if (existing) existing.remove(); pills.appendChild(buildFilterValuePill(option, kind)); + search.value = ""; emitChange(null); } diff --git a/tests/test_filters.py b/tests/test_filters.py index f7f7f83..0b8ad74 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -334,6 +334,33 @@ class TestPurchaseGamesIncludesAllAgainstDB: result = set(Purchase.objects.filter(pf.to_q())) assert result == {seeded["both"], seeded["only_a"], seeded["all_three"]} + @pytest.mark.django_db + def test_includes_any_no_duplicates(self): + """INCLUDES [A, B] must not return duplicate rows for a purchase linked + to both A and B — the M2M join must not inflate the result. + + Regression: ``games__in`` on a many-to-many field produces one row per + matching through-table entry, so a purchase linked to N of the selected + games would appear N times. The fix uses a subquery so each purchase + appears at most once. + """ + from games.filters import PurchaseFilter + from games.models import Purchase + + seeded = self._seed() + pf = PurchaseFilter.from_json( + { + "games": { + "value": [seeded["a"].id, seeded["b"].id], + "modifier": "INCLUDES", + } + } + ) + result = list(Purchase.objects.filter(pf.to_q())) + # Must have 3 distinct purchases, not duplicates + assert len(result) == 3 + assert set(result) == {seeded["both"], seeded["only_a"], seeded["all_three"]} + @pytest.mark.django_db def test_includes_all_strips_embedded_labels(self): """Stash-style {id, label} value items are normalised to bare ids."""