diff --git a/api/README.md b/api/README.md index dc88ad92..4e17e2d8 100644 --- a/api/README.md +++ b/api/README.md @@ -366,8 +366,8 @@ 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) +- `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. 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 @@ -375,6 +375,13 @@ The Issue endpoint supports the most comprehensive filtering options: - `missing_gcd_id` - Boolean, issues without GCD ID - `cover_hash` - Perceptual hash for cover matching +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. +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..76a56512 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,10 +104,12 @@ 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 = IssueCoverIdentifier( + label="UPC Code (main or variant cover)", field_name="upc", 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)", @@ -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..4fdab249 --- /dev/null +++ b/tests/comicsdb/test_issue_upc_filters.py @@ -0,0 +1,114 @@ +"""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", "website_exact"]) +def upc_search(request, api_client_with_credentials, auto_login_user): + """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") + + def search(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"]] + data = response.json() + ids = [row["id"] for row in data["results"]] + assert data["count"] == len(ids) + return ids + + 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() + 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", [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) + 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}