From 983c89d808198f4a2892d7fb7933fd1e0ddf2864 Mon Sep 17 00:00:00 2001 From: Eric Kolb Date: Sun, 1 Nov 2026 14:44:38 -0500 Subject: [PATCH 1/3] Find parent issues by variant UPC and SKU Extend API and website identifier filters to include variant covers without duplicate issues. Cover exact UPC/SKU and API UPC prefix searches with regression tests and document the behavior. Refs Metron-Project/metron#669 --- api/README.md | 12 ++- comicsdb/filters/issue.py | 34 ++++++-- tests/comicsdb/test_issue_sku_filters.py | 98 ++++++++++++++++++++++ tests/comicsdb/test_issue_upc_filters.py | 102 +++++++++++++++++++++++ 4 files changed, 235 insertions(+), 11 deletions(-) create mode 100644 tests/comicsdb/test_issue_sku_filters.py create mode 100644 tests/comicsdb/test_issue_upc_filters.py diff --git a/api/README.md b/api/README.md index dc88ad92..a1cf519f 100644 --- a/api/README.md +++ b/api/README.md @@ -366,15 +366,21 @@ The Issue endpoint supports the most comprehensive filtering options: - `number` - Issue number (case-insensitive, exact match) - `alt_number` - Alternate number (case-insensitive, exact match) -- `sku` - Distributor SKU (exact match) -- `upc` - UPC code (exact match) -- `upc_starts_with` - UPC code prefix match. Useful for mobile barcode scanners (e.g. Google ML Kit, AVFoundation) that only read the 12-digit UPC-A and drop the 5-digit EAN supplemental — pass just those 12 digits to match issues whose stored `upc` begins with them. +- `sku` - Distributor SKU (case-insensitive exact match against the main cover or any variant cover). +- `upc` - UPC code (exact match against the main cover or any variant cover). +- `upc_starts_with` - UPC code prefix match against the main cover or any variant cover. Useful for mobile barcode scanners (e.g. Google ML Kit, AVFoundation) that only read the 12-digit UPC-A and drop the 5-digit EAN supplemental — pass just those 12 digits to find matching parent issues. - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `missing_cv_id` - Boolean, issues without Comic Vine ID - `missing_gcd_id` - Boolean, issues without GCD ID - `cover_hash` - Perceptual hash for cover matching +The SKU and UPC filters return each matching parent issue once, even when multiple covers match. +The list response keeps the issue's main cover; retrieve the issue detail to inspect its +`variants` and identify the cover with the matching SKU or UPC. Other supplied filters still apply. +The website's issue search also supports exact SKU and UPC matches against main and variant covers. +SKU searches require the complete value; they do not match prefixes or substrings. + **Content Filters:** - `rating` - Content rating (exact match) diff --git a/comicsdb/filters/issue.py b/comicsdb/filters/issue.py index 5034edb5..0b163969 100644 --- a/comicsdb/filters/issue.py +++ b/comicsdb/filters/issue.py @@ -40,6 +40,18 @@ class NumberInFilter(df.rest_framework.BaseInFilter, df.rest_framework.NumberFil pass +class IssueCoverIdentifier(df.CharFilter): + """Find each parent issue once when its main cover or a variant matches.""" + + def filter(self, qs, value): + if not value: + return super().filter(qs, value) + return qs.filter( + Q(**{f"{self.field_name}__{self.lookup_expr}": value}) + | Q(**{f"variants__{self.field_name}__{self.lookup_expr}": value}) + ).distinct() + + class IssueFilter(df.rest_framework.FilterSet): cover_year = df.rest_framework.NumberFilter( label="Cover Year", field_name="cover_date", lookup_expr="year" @@ -92,13 +104,15 @@ class IssueFilter(df.rest_framework.FilterSet): rating = df.rest_framework.CharFilter( label="Rating", field_name="rating__name", lookup_expr="iexact" ) - sku = df.rest_framework.CharFilter( - label="Distributor SKU", field_name="sku", lookup_expr="iexact" + sku = IssueCoverIdentifier( + label="Distributor SKU (main or variant cover)", field_name="sku", lookup_expr="iexact" ) - upc = df.rest_framework.CharFilter(label="UPC Code", field_name="upc", lookup_expr="iexact") - upc_starts_with = df.rest_framework.CharFilter( - label="UPC Code starts with (e.g. the 12-digit UPC-A read by a mobile scanner " - "that strips the 5-digit EAN supplemental)", + upc = IssueCoverIdentifier( + label="UPC Code (main or variant cover)", field_name="upc", lookup_expr="iexact" + ) + upc_starts_with = IssueCoverIdentifier( + label="Main or variant UPC Code starts with (e.g. the 12-digit UPC-A read by a " + "mobile scanner that strips the 5-digit EAN supplemental)", field_name="upc", lookup_expr="startswith", ) @@ -172,8 +186,12 @@ class IssueViewFilter(df.FilterSet): series_year_began = df.NumberFilter( label="Series Beginning Year", field_name="series__year_began", lookup_expr="exact" ) - sku = df.CharFilter(label="Distributor SKU", field_name="sku", lookup_expr="iexact") - upc = df.CharFilter(label="UPC Code", field_name="upc", lookup_expr="iexact") + sku = IssueCoverIdentifier( + label="Distributor SKU (main or variant cover)", field_name="sku", lookup_expr="iexact" + ) + upc = IssueCoverIdentifier( + label="UPC Code (main or variant cover)", field_name="upc", lookup_expr="iexact" + ) cv_id = df.rest_framework.NumberFilter( label="Comic Vine ID", field_name="cv_id", lookup_expr="exact" ) diff --git a/tests/comicsdb/test_issue_sku_filters.py b/tests/comicsdb/test_issue_sku_filters.py new file mode 100644 index 00000000..bb802f97 --- /dev/null +++ b/tests/comicsdb/test_issue_sku_filters.py @@ -0,0 +1,98 @@ +"""Exact SKU searches include variant covers in both the API and website.""" + +import pytest +from django.urls import reverse + +from comicsdb.models import Issue, Variant + +MAIN_SKU = "1022DC019" +VARIANT_SKU = "1022DC020" +OTHER_SKU = "1022DC021" + + +@pytest.fixture(params=["api", "website"]) +def sku_search(request, api_client_with_credentials, auto_login_user): + if request.param == "website": + client, _ = auto_login_user() + url = reverse("issue:list") + else: + client = api_client_with_credentials + url = reverse("api:issue-list") + + def search(value, **filters): + response = client.get(url, {"sku": value, **filters}) + assert response.status_code == 200 + if request.param == "website": + return [issue.pk for issue in response.context["issue_list"]] + data = response.json() + ids = [row["id"] for row in data["results"]] + assert data["count"] == len(ids) + return ids + + return search + + +@pytest.mark.parametrize( + ("main_sku", "variant_skus", "query", "matches"), + [ + (MAIN_SKU, [], MAIN_SKU, True), + (MAIN_SKU, [VARIANT_SKU, OTHER_SKU], MAIN_SKU, True), + (MAIN_SKU, [VARIANT_SKU], VARIANT_SKU, True), + ("", [VARIANT_SKU], VARIANT_SKU, True), + ("", [VARIANT_SKU, VARIANT_SKU], VARIANT_SKU, True), + (VARIANT_SKU, [VARIANT_SKU], VARIANT_SKU, True), + (MAIN_SKU, [VARIANT_SKU], OTHER_SKU, False), + (MAIN_SKU, [VARIANT_SKU], "", True), + (MAIN_SKU, [VARIANT_SKU], VARIANT_SKU.lower(), True), + (MAIN_SKU, [VARIANT_SKU], "1022DC", False), + (MAIN_SKU, [VARIANT_SKU], "DC020", False), + (MAIN_SKU, [], MAIN_SKU.lower(), True), + ], + ids=[ + "main_only", + "main_with_variants", + "variant_only", + "blank_main", + "duplicate_variants", + "main_and_variant", + "no_match", + "empty_query", + "variant_case_insensitive", + "no_prefix_match", + "no_substring_match", + "main_case_insensitive", + ], +) +def test_exact_sku_matching(sku_search, basic_issue, main_sku, variant_skus, query, matches): + basic_issue.sku = main_sku + basic_issue.save() + for index, sku in enumerate(variant_skus): + Variant.objects.create(issue=basic_issue, name=f"Variant {index}", sku=sku) + assert sku_search(query) == ([basic_issue.pk] if matches else []) + + +def test_variant_sku_respects_other_filters(sku_search, basic_issue): + Variant.objects.create(issue=basic_issue, name="B", sku=VARIANT_SKU) + assert sku_search(VARIANT_SKU, number=basic_issue.number) == [basic_issue.pk] + assert sku_search(VARIANT_SKU, number="999") == [] + + +def test_sku_keeps_distinct_matching_issues(sku_search, basic_issue): + basic_issue.sku = MAIN_SKU + basic_issue.save() + second = Issue.objects.create( + series=basic_issue.series, + number="2", + cover_date=basic_issue.cover_date, + created_by=basic_issue.created_by, + edited_by=basic_issue.edited_by, + ) + Variant.objects.create(issue=second, name="B", sku=MAIN_SKU) + assert set(sku_search(MAIN_SKU)) == {basic_issue.pk, second.pk} + + +def test_sku_and_upc_filters_can_match_different_covers(sku_search, basic_issue): + Variant.objects.create(issue=basic_issue, name="B", sku=VARIANT_SKU) + Variant.objects.create(issue=basic_issue, name="C", upc="76194137738400121") + assert sku_search(VARIANT_SKU, upc="76194137738400121") == [basic_issue.pk] + assert sku_search(VARIANT_SKU, upc="75960609558200111") == [] diff --git a/tests/comicsdb/test_issue_upc_filters.py b/tests/comicsdb/test_issue_upc_filters.py new file mode 100644 index 00000000..fb8bd1d1 --- /dev/null +++ b/tests/comicsdb/test_issue_upc_filters.py @@ -0,0 +1,102 @@ +"""UPC lookups return parent issues for main and variant covers without duplicates.""" + +import pytest +from django.urls import reverse + +from comicsdb.models import Issue, Variant + +MAIN_UPC = "76194137738400111" +VARIANT_UPC = "76194137738400121" +OTHER_UPC = "75960609558200111" + + +@pytest.fixture(params=["api_exact", "api_prefix", "website_exact"]) +def upc_search(request, api_client_with_credentials, auto_login_user): + """Exercise both API filters and the website's exact filter via real requests.""" + if request.param == "website_exact": + client, _ = auto_login_user() + url = reverse("issue:list") + else: + client = api_client_with_credentials + url = reverse("api:issue-list") + prefix = request.param == "api_prefix" + field = "upc_starts_with" if prefix else "upc" + + def search(value, **filters): + value = value[:12] if prefix else value + response = client.get(url, {field: value, **filters}) + assert response.status_code == 200 + if request.param == "website_exact": + return [issue.pk for issue in response.context["issue_list"]] + data = response.json() + ids = [row["id"] for row in data["results"]] + assert data["count"] == len(ids) + return ids + + return search + + +def test_upc_main_cover_without_variants(upc_search, basic_issue): + basic_issue.upc = MAIN_UPC + basic_issue.save() + assert upc_search(MAIN_UPC) == [basic_issue.pk] + + +def test_upc_main_cover_with_multiple_variants_is_unique(upc_search, basic_issue): + basic_issue.upc = MAIN_UPC + basic_issue.save() + Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC) + Variant.objects.create(issue=basic_issue, name="C", upc=OTHER_UPC) + assert upc_search(MAIN_UPC) == [basic_issue.pk] + + +@pytest.mark.parametrize("main_upc", [OTHER_UPC, ""]) +def test_upc_variant_cover_only(upc_search, basic_issue, main_upc): + basic_issue.upc = main_upc + basic_issue.save() + Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC) + assert upc_search(VARIANT_UPC) == [basic_issue.pk] + + +def test_upc_multiple_matching_variants_are_unique(upc_search, basic_issue): + Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC) + Variant.objects.create(issue=basic_issue, name="C", upc=VARIANT_UPC) + assert upc_search(VARIANT_UPC) == [basic_issue.pk] + + +def test_upc_main_and_variant_matching_are_unique(upc_search, basic_issue): + basic_issue.upc = VARIANT_UPC + basic_issue.save() + Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC) + assert upc_search(VARIANT_UPC) == [basic_issue.pk] + + +def test_upc_no_matching_cover(upc_search, basic_issue): + basic_issue.upc = MAIN_UPC + basic_issue.save() + Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC) + assert upc_search(OTHER_UPC) == [] + + +def test_upc_empty_does_not_filter(upc_search, basic_issue): + assert upc_search("") == [basic_issue.pk] + + +def test_upc_variant_match_respects_other_filters(upc_search, basic_issue): + Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC) + assert upc_search(VARIANT_UPC, number=basic_issue.number) == [basic_issue.pk] + assert upc_search(VARIANT_UPC, number="999") == [] + + +def test_upc_keeps_distinct_matching_issues(upc_search, basic_issue): + basic_issue.upc = MAIN_UPC + basic_issue.save() + second = Issue.objects.create( + series=basic_issue.series, + number="2", + cover_date=basic_issue.cover_date, + created_by=basic_issue.created_by, + edited_by=basic_issue.edited_by, + ) + Variant.objects.create(issue=second, name="B", upc=MAIN_UPC) + assert set(upc_search(MAIN_UPC)) == {basic_issue.pk, second.pk} From 63daba5559d4a2605f6f4d637786c7dbe129fdc6 Mon Sep 17 00:00:00 2001 From: Eric Kolb Date: Tue, 3 Nov 2026 09:36:26 -0500 Subject: [PATCH 2/3] Keep UPC prefix searches scoped to the main cover Address maintainer feedback by restoring the original UPC prefix filter and documentation. Limit variant tests to exact lookups and cover 12-digit main-cover prefix searches separately. Refs Metron-Project/metron#670 --- api/README.md | 5 +++-- comicsdb/filters/issue.py | 6 +++--- tests/comicsdb/test_issue_upc_filters.py | 22 ++++++++++++++++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/api/README.md b/api/README.md index a1cf519f..4e17e2d8 100644 --- a/api/README.md +++ b/api/README.md @@ -368,14 +368,15 @@ The Issue endpoint supports the most comprehensive filtering options: - `alt_number` - Alternate number (case-insensitive, exact match) - `sku` - Distributor SKU (case-insensitive exact match against the main cover or any variant cover). - `upc` - UPC code (exact match against the main cover or any variant cover). -- `upc_starts_with` - UPC code prefix match against the main cover or any variant cover. Useful for mobile barcode scanners (e.g. Google ML Kit, AVFoundation) that only read the 12-digit UPC-A and drop the 5-digit EAN supplemental — pass just those 12 digits to find matching parent issues. +- `upc_starts_with` - UPC code prefix match. Useful for mobile barcode scanners (e.g. Google ML Kit, AVFoundation) that only read the 12-digit UPC-A and drop the 5-digit EAN supplemental — pass just those 12 digits to match issues whose stored `upc` begins with them. - `cv_id` - Comic Vine ID - `gcd_id` - Grand Comics Database ID - `missing_cv_id` - Boolean, issues without Comic Vine ID - `missing_gcd_id` - Boolean, issues without GCD ID - `cover_hash` - Perceptual hash for cover matching -The SKU and UPC filters return each matching parent issue once, even when multiple covers match. +The `sku` and `upc` filters match main and variant covers and return each matching parent issue once, +even when multiple covers match. Variant matching applies only to `upc` and `sku`, not `upc_starts_with`. The list response keeps the issue's main cover; retrieve the issue detail to inspect its `variants` and identify the cover with the matching SKU or UPC. Other supplied filters still apply. The website's issue search also supports exact SKU and UPC matches against main and variant covers. diff --git a/comicsdb/filters/issue.py b/comicsdb/filters/issue.py index 0b163969..76a56512 100644 --- a/comicsdb/filters/issue.py +++ b/comicsdb/filters/issue.py @@ -110,9 +110,9 @@ class IssueFilter(df.rest_framework.FilterSet): upc = IssueCoverIdentifier( label="UPC Code (main or variant cover)", field_name="upc", lookup_expr="iexact" ) - upc_starts_with = IssueCoverIdentifier( - label="Main or variant UPC Code starts with (e.g. the 12-digit UPC-A read by a " - "mobile scanner that strips the 5-digit EAN supplemental)", + upc_starts_with = df.rest_framework.CharFilter( + label="UPC Code starts with (e.g. the 12-digit UPC-A read by a mobile scanner " + "that strips the 5-digit EAN supplemental)", field_name="upc", lookup_expr="startswith", ) diff --git a/tests/comicsdb/test_issue_upc_filters.py b/tests/comicsdb/test_issue_upc_filters.py index fb8bd1d1..a4010efc 100644 --- a/tests/comicsdb/test_issue_upc_filters.py +++ b/tests/comicsdb/test_issue_upc_filters.py @@ -10,21 +10,18 @@ OTHER_UPC = "75960609558200111" -@pytest.fixture(params=["api_exact", "api_prefix", "website_exact"]) +@pytest.fixture(params=["api_exact", "website_exact"]) def upc_search(request, api_client_with_credentials, auto_login_user): - """Exercise both API filters and the website's exact filter via real requests.""" + """Exercise the API and website's exact filters via real requests.""" if request.param == "website_exact": client, _ = auto_login_user() url = reverse("issue:list") else: client = api_client_with_credentials url = reverse("api:issue-list") - prefix = request.param == "api_prefix" - field = "upc_starts_with" if prefix else "upc" def search(value, **filters): - value = value[:12] if prefix else value - response = client.get(url, {field: value, **filters}) + response = client.get(url, {"upc": value, **filters}) assert response.status_code == 200 if request.param == "website_exact": return [issue.pk for issue in response.context["issue_list"]] @@ -36,6 +33,19 @@ def search(value, **filters): return search +def test_upc_prefix_matches_main_cover(api_client_with_credentials, basic_issue): + basic_issue.upc = MAIN_UPC + basic_issue.save() + Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC) + response = api_client_with_credentials.get( + reverse("api:issue-list"), {"upc_starts_with": MAIN_UPC[:12]} + ) + assert response.status_code == 200 + data = response.json() + assert data["count"] == 1 + assert [row["id"] for row in data["results"]] == [basic_issue.pk] + + def test_upc_main_cover_without_variants(upc_search, basic_issue): basic_issue.upc = MAIN_UPC basic_issue.save() From b9788a7a4b84d22ecb9e4029589a6ef638c86fe3 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Tue, 3 Nov 2026 12:21:19 -0500 Subject: [PATCH 3/3] Use a shared UPC-A base in variant-only UPC test The variant-only test gave the parent a UPC with an entirely different 12-digit base from its variant, which doesn't happen with direct-market comics. Use the main cover's UPC instead, so the parent and variant share the UPC-A and differ only in the 5-digit add-on. The full variant UPC still matches only through the variant. Also add readable parametrize IDs. --- tests/comicsdb/test_issue_upc_filters.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/comicsdb/test_issue_upc_filters.py b/tests/comicsdb/test_issue_upc_filters.py index a4010efc..4fdab249 100644 --- a/tests/comicsdb/test_issue_upc_filters.py +++ b/tests/comicsdb/test_issue_upc_filters.py @@ -60,8 +60,10 @@ def test_upc_main_cover_with_multiple_variants_is_unique(upc_search, basic_issue assert upc_search(MAIN_UPC) == [basic_issue.pk] -@pytest.mark.parametrize("main_upc", [OTHER_UPC, ""]) +@pytest.mark.parametrize("main_upc", [MAIN_UPC, ""], ids=["shared_base", "blank_main"]) def test_upc_variant_cover_only(upc_search, basic_issue, main_upc): + # Variants share the main cover's 12-digit UPC-A and differ only in the add-on, + # so the full 17-digit variant UPC matches only through the variant. basic_issue.upc = main_upc basic_issue.save() Variant.objects.create(issue=basic_issue, name="B", upc=VARIANT_UPC)