diff --git a/CLAUDE.md b/CLAUDE.md index 820343a9c..ddb99bfe8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,7 +58,8 @@ only) for deployment, hosting, and domain specifics. - [`docs/lessons.md`](docs/lessons.md) — terse, reusable cross-session lessons (CI vs. local venv trust, worktree port collisions, debug-color verification, cyclic-animation sampling, verifying cross-session claims, - ES mapping drift, test-factory isolation, sticky/overflow CSS gotchas). + ES mapping drift, test-factory isolation, sticky/overflow CSS gotchas, + migration-vs-command tag seeding). - [`docs/features/image-cdn.md`](docs/features/image-cdn.md) — the Worker + R2 bucket image CDN. - [`docs/features/pdf-generator.md`](docs/features/pdf-generator.md) — PDF diff --git a/MPCAutofill/cardpicker/management/commands/seed_no_match_reason_tags.py b/MPCAutofill/cardpicker/management/commands/seed_no_match_reason_tags.py new file mode 100644 index 000000000..838a1056a --- /dev/null +++ b/MPCAutofill/cardpicker/management/commands/seed_no_match_reason_tags.py @@ -0,0 +1,13 @@ +from typing import Any + +from django.core.management.base import BaseCommand + +from cardpicker.reason_tags import seed_no_match_reason_tags + + +class Command(BaseCommand): + help = "Seeds the 'why no match?' reason-code Tag taxonomy (custom-art, altered-frame, ...). Safe to re-run." + + def handle(self, *args: Any, **kwargs: Any) -> None: + stats = seed_no_match_reason_tags() + print(f"No-match reason tags: {stats['created']} created.") diff --git a/MPCAutofill/cardpicker/models.py b/MPCAutofill/cardpicker/models.py index b54e3eff3..a37cd2dee 100755 --- a/MPCAutofill/cardpicker/models.py +++ b/MPCAutofill/cardpicker/models.py @@ -124,6 +124,7 @@ def serialise_as_printing_candidate(self) -> PrintingCandidate: smallThumbnailUrl=self.small_thumbnail_url, mediumThumbnailUrl=self.medium_thumbnail_url, fullArt=metadata.full_art if metadata is not None else False, + isBorderless=metadata.border_color == "borderless" if metadata is not None else False, frame=metadata.frame if metadata is not None else "", releasedAt=metadata.released_at.isoformat() if metadata is not None and metadata.released_at else None, ) diff --git a/MPCAutofill/cardpicker/reason_tags.py b/MPCAutofill/cardpicker/reason_tags.py new file mode 100644 index 000000000..303e2caed --- /dev/null +++ b/MPCAutofill/cardpicker/reason_tags.py @@ -0,0 +1,53 @@ +""" +The "why no match?" reason-code taxonomy - shown as a follow-up strip in the printing-tag +queue after a human casts an explicit "No match" printing vote (see +docs/features/printing-tags.md, "no-match reason tags"). Kept in its own module and its own +management command (mirroring cardpicker.default_tags/seed_default_tags exactly) rather than +a data migration: a data migration would run automatically at DB-setup time (including the +test database), unconditionally seeding these rows into every fresh DB - which breaks every +test that asserts on the *complete* set of `Tag` rows (e.g. test_views.py::TestGetTags, which +documents that a fresh DB has zero real Tag rows besides the synthetic never-persisted "NSFW" +pseudo-tag - see cardpicker.tags). A manual, idempotent command avoids that coupling, exactly +like the existing descriptor taxonomy already does. + +These six tag names are a federation interchange contract (see docs/features/printing-tags.md) +- other instances that consume our vote export are expected to recognise these exact strings. +Renaming any of them is a breaking data migration, not a refactor. + +Deliberately a separate, lowercase-kebab-case taxonomy from `cardpicker.default_tags`'s Title +Case DEFAULT_TAGS (which parses filename bracket content at upload time, e.g. "Upscaled", +"Custom", "AI-Generated"). `upscaled`/`custom-art`/`ai-art` below cover near-identical +concepts to those but are cast by a human as the *reason* they picked "no match" in the +printing-tag queue, not inferred from a filename - kept as distinct rows rather than reusing +the existing tags so the two vote populations (upload-time inference vs. human no-match +reasoning) don't get silently merged into one consensus. +""" + +from cardpicker.models import Tag + +NO_MATCH_REASON_TAGS: list[tuple[str, str]] = [ + ("custom-art", "Original or alternate artwork - does not depict a real printing"), + ("altered-frame", "Real printing's art in a modified frame"), + ("upscaled", "AI-upscaled version of an official image"), + ("ai-art", "AI-generated artwork"), + ("no-collector-line", "No legible collector line on the card face"), + ("non-english", "Non-English printing"), +] + + +def seed_no_match_reason_tags() -> dict[str, int]: + """ + Idempotent - safe to re-run. Creates any tag that doesn't exist yet. `Tag` has no + description field (see cardpicker.models.Tag) - the descriptions above are documentation + only, mirrored as display copy in the frontend's NoMatchReasonStrip.tsx. + """ + + created = 0 + for name, _description in NO_MATCH_REASON_TAGS: + _tag, was_created = Tag.objects.get_or_create(name=name, defaults={"aliases": []}) + if was_created: + created += 1 + return {"created": created} + + +__all__ = ["seed_no_match_reason_tags", "NO_MATCH_REASON_TAGS"] diff --git a/MPCAutofill/cardpicker/schema_types.py b/MPCAutofill/cardpicker/schema_types.py index b4399cfe1..3574f9e85 100644 --- a/MPCAutofill/cardpicker/schema_types.py +++ b/MPCAutofill/cardpicker/schema_types.py @@ -1187,6 +1187,7 @@ class PrintingCandidate(BaseModel): frame: str fullArt: bool identifier: str + isBorderless: bool mediumThumbnailUrl: str smallThumbnailUrl: str releasedAt: Optional[str] = None @@ -1202,6 +1203,7 @@ def from_dict(obj: Any) -> "PrintingCandidate": frame = from_str(obj.get("frame")) fullArt = from_bool(obj.get("fullArt")) identifier = from_str(obj.get("identifier")) + isBorderless = from_bool(obj.get("isBorderless")) mediumThumbnailUrl = from_str(obj.get("mediumThumbnailUrl")) smallThumbnailUrl = from_str(obj.get("smallThumbnailUrl")) releasedAt = from_union([from_none, from_str], obj.get("releasedAt")) @@ -1214,6 +1216,7 @@ def from_dict(obj: Any) -> "PrintingCandidate": frame, fullArt, identifier, + isBorderless, mediumThumbnailUrl, smallThumbnailUrl, releasedAt, @@ -1229,6 +1232,7 @@ def to_dict(self) -> dict: result["frame"] = from_str(self.frame) result["fullArt"] = from_bool(self.fullArt) result["identifier"] = from_str(self.identifier) + result["isBorderless"] = from_bool(self.isBorderless) result["mediumThumbnailUrl"] = from_str(self.mediumThumbnailUrl) result["smallThumbnailUrl"] = from_str(self.smallThumbnailUrl) if self.releasedAt is not None: diff --git a/MPCAutofill/cardpicker/tests/test_printing_tags_views.py b/MPCAutofill/cardpicker/tests/test_printing_tags_views.py index 690e86888..96d62abde 100644 --- a/MPCAutofill/cardpicker/tests/test_printing_tags_views.py +++ b/MPCAutofill/cardpicker/tests/test_printing_tags_views.py @@ -118,7 +118,9 @@ def test_explicit_query_searches_by_name_even_when_a_link_exists(self, client, d def test_candidate_shape_includes_printing_metadata_fields(self, client, django_settings): card = CardFactory(name="Brainstorm") printing = CanonicalCardFactory(name="Brainstorm") - CanonicalPrintingMetadataFactory(canonical_card=printing, full_art=True, frame="1997") + CanonicalPrintingMetadataFactory( + canonical_card=printing, full_art=True, frame="1997", border_color="borderless" + ) response = client.post( reverse(views.post_printing_candidates), @@ -128,9 +130,24 @@ def test_candidate_shape_includes_printing_metadata_fields(self, client, django_ [result] = response.json()["results"] assert result["fullArt"] is True + assert result["isBorderless"] is True assert result["frame"] == "1997" assert result["artist"] == printing.artist.name + def test_candidate_with_non_borderless_border_color(self, client, django_settings): + card = CardFactory(name="Brainstorm") + printing = CanonicalCardFactory(name="Brainstorm") + CanonicalPrintingMetadataFactory(canonical_card=printing, border_color="black") + + response = client.post( + reverse(views.post_printing_candidates), + {"identifier": card.identifier}, + content_type="application/json", + ) + + [result] = response.json()["results"] + assert result["isBorderless"] is False + def test_candidate_without_printing_metadata_uses_defaults(self, client, django_settings): card = CardFactory(name="Brainstorm") CanonicalCardFactory(name="Brainstorm") # no CanonicalPrintingMetadataFactory for this one @@ -143,6 +160,7 @@ def test_candidate_without_printing_metadata_uses_defaults(self, client, django_ [result] = response.json()["results"] assert result["fullArt"] is False + assert result["isBorderless"] is False assert result["frame"] == "" assert result["releasedAt"] is None diff --git a/MPCAutofill/cardpicker/tests/test_reason_tags.py b/MPCAutofill/cardpicker/tests/test_reason_tags.py new file mode 100644 index 000000000..c0e303a1c --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_reason_tags.py @@ -0,0 +1,39 @@ +from cardpicker.default_tags import DEFAULT_TAGS +from cardpicker.models import Tag +from cardpicker.reason_tags import NO_MATCH_REASON_TAGS, seed_no_match_reason_tags + + +class TestSeedNoMatchReasonTags: + def test_creates_all_six_reason_tags(self, db): + stats = seed_no_match_reason_tags() + assert stats["created"] == len(NO_MATCH_REASON_TAGS) + + names = set( + Tag.objects.filter(name__in=[name for name, _description in NO_MATCH_REASON_TAGS]).values_list( + "name", flat=True + ) + ) + assert names == {name for name, _description in NO_MATCH_REASON_TAGS} + + def test_rerunning_does_not_duplicate(self, db): + seed_no_match_reason_tags() + count_after_first_run = Tag.objects.filter( + name__in=[name for name, _description in NO_MATCH_REASON_TAGS] + ).count() + + stats = seed_no_match_reason_tags() + + count_after_second_run = Tag.objects.filter( + name__in=[name for name, _description in NO_MATCH_REASON_TAGS] + ).count() + assert stats["created"] == 0 + assert count_after_first_run == count_after_second_run == len(NO_MATCH_REASON_TAGS) + + def test_reason_tags_are_case_distinct_from_default_tags(self, db): + # "upscaled" (reason tag) and "Upscaled" (DEFAULT_TAGS) are deliberately two separate + # rows covering related but distinct vote populations - see reason_tags.py's header + # comment. Exact-string collision (not just case-insensitive overlap) would mean + # seeding silently reused an existing row instead of creating a new one. + default_tag_names = {name for name, _aliases in DEFAULT_TAGS} + reason_tag_names = {name for name, _description in NO_MATCH_REASON_TAGS} + assert default_tag_names.isdisjoint(reason_tag_names) diff --git a/docs/features/printing-tags.md b/docs/features/printing-tags.md index 82c9d17c7..1c98c2459 100644 --- a/docs/features/printing-tags.md +++ b/docs/features/printing-tags.md @@ -248,6 +248,90 @@ field (`inferred_canonical_artist`/`artist_vote_status` only, serialise-time-only) — confirmed against `documents.py`'s field list, no hook needed. +## Stage 4: no-match reason tags + post-vote follow-up strips + +A resolved printing vote and an explicit "No match" vote both used to +advance the queue with zero follow-up (or, for no-match, the general +`AttributeVotingPanel`) — no fast way to capture _why_ a card had no +match, and no prompt to confirm full-art/borderless while the card was +still on screen. Two new, narrowly-scoped strips render in +`PrintingTagQueue.tsx` between a vote submitting and the queue +auto-advancing, both a brief, skippable dwell that never blocks +advancing: + +- `PrintingConfirmStrip` (after a vote resolves a printing) — two chips, + "Full art"/"Borderless", pre-filled (highlighted) from the resolved + candidate's own `fullArt`/`isBorderless` flags. A tap casts one + `CardTagVote` for the existing `Full Art`/`Borderless` tags (seeded by + `cardpicker.default_tags`, not new) with polarity matching the + preview. No new tags, no new endpoint. +- `NoMatchReasonStrip` (after an explicit "No match" vote) — six chips + for a new reason-code taxonomy (below). One tap casts one positive + `CardTagVote` and advances. Replaces `AttributeVotingPanel` only in + this specific branch — a card that's still contested from a candidate + pick (not an explicit no-match) keeps showing `AttributeVotingPanel` + unchanged. + +Both strips reuse the existing `CardTagVote`/`submitTagVote` machinery +end to end — no new backend endpoints or vote types, just narrower UI +over what Stage "attribute voting" already shipped. + +**`isBorderless` added to `PrintingCandidate`**: the schema had +`fullArt` but no border-color-derived field, so `PrintingConfirmStrip` +couldn't pre-fill a borderless preview from data "already in the +payload" as originally assumed — it wasn't. Added via the quicktype +regeneration step (`schemas/schemas/PrintingCandidate.json` → +`cd schemas && npm run build`), wired in +`CanonicalCard.serialise_as_printing_candidate()` as +`metadata.border_color == "borderless"`. Verified against live data +first (read-only `CanonicalPrintingMetadata.objects.values("border_color").annotate(...)` +query) rather than assumed from general Scryfall knowledge: the stored +values are exactly `black`/`borderless`/`white`/`gold`/`silver`/`yellow`. + +**Reason-code taxonomy — six new `Tag` rows, seeded by a management +command, not a migration**: `custom-art`, `altered-frame`, `upscaled`, +`ai-art`, `no-collector-line`, `non-english` +(`cardpicker/reason_tags.py`, `manage.py seed_no_match_reason_tags`, +mirroring the existing `cardpicker/default_tags.py`/ +`seed_default_tags` pattern exactly). **These names are a federation +interchange contract** — other instances consuming our vote export +expect these exact strings; renaming any of them is a breaking change, +not a refactor. + +A first pass seeded these via a data migration instead, per the +original task spec. That broke 5 unrelated tests +(`test_views.py::TestGetTags::*`, `test_tag_votes.py:: TestPostTagConsensus::test_returns_an_entry_for_every_seeded_tag`) — +they assert the _complete_ set of `Tag` rows, and document that a fresh +DB has zero real `Tag` rows besides the synthetic, never-persisted +`"NSFW"` pseudo-tag (`cardpicker/tags.py`). `seed_default_tags` is +deliberately **not** wired into any migration for the same reason: a +migration runs unconditionally at DB-setup time (including the test +DB), permanently seeding rows nothing asked for. Switched to a command +to match that established convention; suite back to the known 4-failure +baseline (2 unrelated `moxfield` network tests, 2 unrelated +`test_sources.py` path issues) afterward. + +**Deliberately a separate taxonomy from `DEFAULT_TAGS`**, not a reuse: +`upscaled`/`custom-art`/`ai-art` cover near-identical concepts to the +existing `Upscaled`/`Custom`/`AI-Generated` (which parse filename +bracket content at _upload_ time), but these are cast by a _human_ as +the reason they picked "no match" in the queue — kept as distinct rows +(exact-string-distinct, case included) so the two vote populations +don't silently merge into one consensus. + +`Tag` has no `description` field — the descriptions given in the task +spec live as documentation only (`reason_tags.py`'s module comment, +mirrored as frontend display copy in `NoMatchReasonStrip.tsx`), not a +new DB column or serializer field. + +**Activation note**: `manage.py seed_no_match_reason_tags` must be run +once after this deploys, or `NoMatchReasonStrip` votes 400 +(`post_submit_tag_vote` does `Tag.objects.get(name=...)`, not +`get_or_create` — a miss raises `BadRequestException`, not a silent +no-op). Confirmed live: `Full Art`/`Borderless` (used by +`PrintingConfirmStrip`) already exist in production — `seed_default_tags` +has been run there before — so that strip needs no activation step. + ## Key files - Backend: `cardpicker/printing_consensus.py`, @@ -256,11 +340,14 @@ hook needed. `0050_canonicalprintingmetadata_cardprintingtag_and_more.py`), `cardpicker/search/search_functions.py` (Stage 3 re-rank/filter), `cardpicker/documents.py` (Stage 3 widened indexing; Stage 3.5 - `reindex_card_safely`), `cardpicker/tag_consensus.py` (Stage 3.5) + `reindex_card_safely`), `cardpicker/tag_consensus.py` (Stage 3.5), + `cardpicker/reason_tags.py`, `cardpicker/management/commands/ seed_no_match_reason_tags.py` (Stage 4) - Frontend: `frontend/src/features/printingTags/` (`PrintingTagQueue.tsx`, `PrintingTagPicker.tsx`, `starburstShape.ts`, `useStickyTop`), `frontend/src/features/filters/ResolvedAttributeFilter.tsx` (Stage 3), - `frontend/src/common/processing.ts::getPrintingMatchLabel` (Stage 3) + `frontend/src/common/processing.ts::getPrintingMatchLabel` (Stage 3), + `frontend/src/features/attributeVoting/` (`ChipCard.tsx`, + `NoMatchReasonStrip.tsx`, `PrintingConfirmStrip.tsx` — Stage 4) - `docs/upstreaming/vote-system.md` ## Known gaps @@ -271,3 +358,7 @@ hook needed. - Client-side (Orama) search has no Stage 3 parity — see above. - Upstreaming this feature is deprioritized — see [[../infrastructure.md]]'s Upstreaming section. +- Stage numbering here may need reconciling if PR #11 (deductive + printing-tag backfill, also labelled "Stage 4" on its own branch) + lands separately from this one — whichever merges second should + renumber to avoid two unrelated "Stage 4"s. diff --git a/docs/lessons.md b/docs/lessons.md index 27763ddf4..686514a21 100644 --- a/docs/lessons.md +++ b/docs/lessons.md @@ -184,3 +184,24 @@ artifact in a real browser (Chromium at `executablePath: /opt/pw-browsers/chromium`; default Playwright download is absent). State clearly in the report that live behavior itself was not observed. + +## Seeding `Tag` rows via a data migration breaks tests that assert on the whole table — use a management command instead + +Tried seeding six new `Tag` rows via a Django data migration (RunPython). +Broke 5 unrelated tests (`test_views.py::TestGetTags::*`, +`test_tag_votes.py::TestPostTagConsensus::test_returns_an_entry_for_every_seeded_tag`) +because they assert the _complete_ `Tag` table is empty in a fresh DB +(besides the synthetic, never-persisted `"NSFW"` pseudo-tag from +`cardpicker/tags.py`) — a migration runs unconditionally at DB-setup time, +including the test database, so any migration-seeded row becomes +permanent baseline state for every test in the suite, not just the ones +that care about it. The repo's existing 13-tag `DEFAULT_TAGS` taxonomy +(`cardpicker/default_tags.py`) is deliberately _not_ wired into any +migration for exactly this reason — it's a manual, idempotent +`seed_default_tags` management command only. Any future tag/taxonomy +seeding should follow that same pattern (a `..._tags.py` module + +`get_or_create` + a thin management-command wrapper), never a migration, +regardless of how the request is phrased ("data migration" in a task spec +should be read as "a repeatable seeding step," not literally +`migrations.RunPython`, when the target table has DB-wide list-all +consumers). diff --git a/frontend/src/common/schema_types.ts b/frontend/src/common/schema_types.ts index b7ea5ae59..7f001d159 100644 --- a/frontend/src/common/schema_types.ts +++ b/frontend/src/common/schema_types.ts @@ -463,6 +463,7 @@ export interface PrintingCandidate { frame: string; fullArt: boolean; identifier: string; + isBorderless: boolean; mediumThumbnailUrl: string; releasedAt?: null | string; smallThumbnailUrl: string; @@ -1863,6 +1864,7 @@ const typeMap: any = { { json: "frame", js: "frame", typ: "" }, { json: "fullArt", js: "fullArt", typ: true }, { json: "identifier", js: "identifier", typ: "" }, + { json: "isBorderless", js: "isBorderless", typ: true }, { json: "mediumThumbnailUrl", js: "mediumThumbnailUrl", typ: "" }, { json: "releasedAt", js: "releasedAt", typ: u(undefined, u(null, "")) }, { json: "smallThumbnailUrl", js: "smallThumbnailUrl", typ: "" }, diff --git a/frontend/src/common/test-constants.ts b/frontend/src/common/test-constants.ts index 086abea81..1a9be1897 100644 --- a/frontend/src/common/test-constants.ts +++ b/frontend/src/common/test-constants.ts @@ -520,6 +520,7 @@ export const printingCandidate1: PrintingCandidate = { smallThumbnailUrl: "https://example.com/small1.png", mediumThumbnailUrl: "https://example.com/medium1.png", fullArt: false, + isBorderless: false, frame: "2015", releasedAt: "2020-01-01", }; @@ -534,6 +535,7 @@ export const printingCandidate2: PrintingCandidate = { smallThumbnailUrl: "https://example.com/small2.png", mediumThumbnailUrl: "https://example.com/medium2.png", fullArt: true, + isBorderless: true, frame: "2003", releasedAt: "2010-06-15", }; diff --git a/frontend/src/features/attributeVoting/ChipCard.tsx b/frontend/src/features/attributeVoting/ChipCard.tsx new file mode 100644 index 000000000..743d8e6c0 --- /dev/null +++ b/frontend/src/features/attributeVoting/ChipCard.tsx @@ -0,0 +1,77 @@ +/** + * Small tap-target "card" chip, styled with the same blue (#4d8ddf) used for the "resolved + * consensus" highlight and the "No match" placeholder over in PrintingTagQueue.tsx's + * candidate grid (see CandidateButton/ArtPlaceholder there) - reused here so the post-vote + * follow-up strips (NoMatchReasonStrip, PrintingConfirmStrip) read as the same visual + * language as the picker a user just interacted with, rather than introducing a second + * unrelated chip style. Deliberately lighter than CandidateButton: no starburst/hover-zoom, + * since these chips are small and numerous rather than one large focal candidate. + */ + +import styled from "@emotion/styled"; +import React from "react"; +import Button from "react-bootstrap/Button"; + +import { STARBURST_OUTER_COLOR } from "@/features/printingTags/starburstShape"; + +const StyledChipButton = styled(Button)` + border: 2px solid ${STARBURST_OUTER_COLOR}; + border-radius: 0.5rem; + background-color: transparent; + color: inherit; + width: 100%; + padding: 0.5rem 0.25rem; + text-align: center; + + &:hover, + &:focus { + background-color: rgba(77, 141, 223, 0.15); + border-color: ${STARBURST_OUTER_COLOR}; + color: inherit; + } + + &.highlighted { + background-color: ${STARBURST_OUTER_COLOR}; + color: #000000; + } + + &.highlighted:hover, + &.highlighted:focus { + background-color: ${STARBURST_OUTER_COLOR}; + } + + &:disabled { + opacity: 0.6; + } +`; + +interface ChipCardProps { + label: string; + sublabel?: string; + highlighted?: boolean; + disabled?: boolean; + onClick: () => void; + "data-testid"?: string; +} + +export function ChipCard({ + label, + sublabel, + highlighted = false, + disabled = false, + onClick, + "data-testid": dataTestId, +}: ChipCardProps) { + return ( + +
{label}
+ {sublabel != null &&
{sublabel}
} +
+ ); +} diff --git a/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx b/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx new file mode 100644 index 000000000..c547332e9 --- /dev/null +++ b/frontend/src/features/attributeVoting/NoMatchReasonStrip.tsx @@ -0,0 +1,124 @@ +/** + * "Why no match?" follow-up shown in PrintingTagQueue.tsx immediately after a user submits + * an explicit "No match" printing vote (not shown for a still-contested candidate pick - + * that case keeps using the general AttributeVotingPanel, see the call site). One tap on a + * reason chip casts a single positive CardTagVote for that reason and advances; Skip + * advances without voting. Deliberately not the full TagVotePicker grid - this is a + * narrower, faster "why" prompt matched to the moment right after a no-match tap, not a + * general tagging surface. + * + * Keep the six tagName values below in sync with cardpicker/reason_tags.py (seeded via the + * `seed_no_match_reason_tags` management command, not a migration - see that module's + * header comment for why) - and see the same file for why these are a separate taxonomy + * from cardpicker.default_tags.DEFAULT_TAGS and why renaming any of them is a breaking + * change. + * + * Graceful degradation for an instance where that command hasn't been run yet: filters the + * six chips down to whichever tags `useGetTagsQuery` (the existing, already-cached `2/tags/` + * query used elsewhere for the search-filter tag tree - no new endpoint/fetch introduced + * here) actually reports. While that query is still loading, shows all six optimistically + * rather than flashing an empty strip - a stale-positive chip just fails the same way an + * unseeded one always would (a caught, toasted "Vote failed"), it's not a worse outcome than + * today's baseline. Once loaded, unseeded chips are hidden entirely rather than shown + * disabled, since there's nothing useful for a voter to do with one that will only ever 400. + */ + +import React, { useState } from "react"; +import Button from "react-bootstrap/Button"; +import Col from "react-bootstrap/Col"; +import Row from "react-bootstrap/Row"; + +import { getOrCreateAnonymousId } from "@/common/cookies"; +import { useAppDispatch } from "@/common/types"; +import { ChipCard } from "@/features/attributeVoting/ChipCard"; +import { APISubmitTagVote, useGetTagsQuery } from "@/store/api"; +import { setNotification } from "@/store/slices/toastsSlice"; + +const APPLY = 1; + +const NO_MATCH_REASONS: Array<{ tagName: string; label: string }> = [ + { tagName: "custom-art", label: "Custom art" }, + { tagName: "altered-frame", label: "Altered frame" }, + { tagName: "upscaled", label: "Upscaled" }, + { tagName: "ai-art", label: "AI art" }, + { tagName: "no-collector-line", label: "No collector line" }, + { tagName: "non-english", label: "Non-English" }, +]; + +interface NoMatchReasonStripProps { + backendURL: string; + cardIdentifier: string; + /** Called once a reason has been submitted, or the user skips. */ + onDone: () => void; +} + +export function NoMatchReasonStrip({ + backendURL, + cardIdentifier, + onDone, +}: NoMatchReasonStripProps) { + const dispatch = useAppDispatch(); + const [submittingTagName, setSubmittingTagName] = useState( + null + ); + const { data: existingTags } = useGetTagsQuery(); + const existingTagNames = + existingTags != null ? new Set(existingTags.map((tag) => tag.name)) : null; + const visibleReasons = NO_MATCH_REASONS.filter( + (reason) => existingTagNames == null || existingTagNames.has(reason.tagName) + ); + + const choose = (tagName: string) => { + setSubmittingTagName(tagName); + APISubmitTagVote( + backendURL, + cardIdentifier, + getOrCreateAnonymousId(), + tagName, + APPLY + ) + .then(() => onDone()) + .catch(() => + dispatch( + setNotification([ + Math.random().toString(), + { + name: "Vote failed", + message: + "Something went wrong submitting your vote - please try again.", + level: "error", + }, + ]) + ) + ) + .finally(() => setSubmittingTagName(null)); + }; + + return ( +
+
Why no match?
+ + {visibleReasons.map((reason) => ( + + choose(reason.tagName)} + data-testid={`no-match-reason-${reason.tagName}`} + /> + + ))} + +
+ +
+
+ ); +} diff --git a/frontend/src/features/attributeVoting/PrintingConfirmStrip.tsx b/frontend/src/features/attributeVoting/PrintingConfirmStrip.tsx new file mode 100644 index 000000000..b0035b9de --- /dev/null +++ b/frontend/src/features/attributeVoting/PrintingConfirmStrip.tsx @@ -0,0 +1,140 @@ +/** + * "Confirm what you see" follow-up shown in PrintingTagQueue.tsx immediately after a vote + * resolves a card's printing. Two chips - Full art / Borderless - pre-filled (highlighted) + * from the resolved candidate's own metadata (already present on the PrintingCandidate + * payload as `fullArt`/`isBorderless`), so the chip's resting state previews the vote a tap + * would cast. Tapping a chip confirms that preview by casting one CardTagVote for the + * existing "Full Art"/"Borderless" tags (seeded by cardpicker.default_tags, not new tags) + * with polarity matching the previewed state; Skip/Continue moves on without voting. + * Deliberately reuses the existing Full Art/Borderless taxonomy rather than minting new + * tags - this strip is just a fast, pre-filled way to cast the same votes TagVotePicker + * already supports. + */ + +import React, { useState } from "react"; +import Button from "react-bootstrap/Button"; +import Col from "react-bootstrap/Col"; +import Row from "react-bootstrap/Row"; + +import { getOrCreateAnonymousId } from "@/common/cookies"; +import { PrintingCandidate } from "@/common/schema_types"; +import { useAppDispatch } from "@/common/types"; +import { ChipCard } from "@/features/attributeVoting/ChipCard"; +import { APISubmitTagVote } from "@/store/api"; +import { setNotification } from "@/store/slices/toastsSlice"; + +const APPLY = 1; +const NOT_APPLICABLE = -1; + +interface ConfirmToggle { + tagName: string; + label: string; + previewValue: boolean; +} + +interface PrintingConfirmStripProps { + backendURL: string; + cardIdentifier: string; + candidate: PrintingCandidate; + /** Called once the user has confirmed both toggles (or skipped). */ + onDone: () => void; +} + +export function PrintingConfirmStrip({ + backendURL, + cardIdentifier, + candidate, + onDone, +}: PrintingConfirmStripProps) { + const dispatch = useAppDispatch(); + const [confirmedTagNames, setConfirmedTagNames] = useState>( + new Set() + ); + const [submittingTagName, setSubmittingTagName] = useState( + null + ); + + const toggles: ConfirmToggle[] = [ + { tagName: "Full Art", label: "Full art", previewValue: candidate.fullArt }, + { + tagName: "Borderless", + label: "Borderless", + previewValue: candidate.isBorderless, + }, + ]; + + const confirm = (toggle: ConfirmToggle) => { + setSubmittingTagName(toggle.tagName); + APISubmitTagVote( + backendURL, + cardIdentifier, + getOrCreateAnonymousId(), + toggle.tagName, + toggle.previewValue ? APPLY : NOT_APPLICABLE + ) + .then(() => + setConfirmedTagNames((previous) => + new Set(previous).add(toggle.tagName) + ) + ) + .catch(() => + dispatch( + setNotification([ + Math.random().toString(), + { + name: "Vote failed", + message: + "Something went wrong submitting your vote - please try again.", + level: "error", + }, + ]) + ) + ) + .finally(() => setSubmittingTagName(null)); + }; + + return ( +
+
Confirm what you see
+ + {toggles.map((toggle) => ( + + confirm(toggle)} + data-testid={`printing-confirm-${toggle.tagName + .toLowerCase() + .replace(" ", "-")}`} + /> + + ))} + +
+ + +
+
+ ); +} diff --git a/frontend/src/features/printingTags/PrintingTagQueue.tsx b/frontend/src/features/printingTags/PrintingTagQueue.tsx index d5316d800..e6ec98442 100644 --- a/frontend/src/features/printingTags/PrintingTagQueue.tsx +++ b/frontend/src/features/printingTags/PrintingTagQueue.tsx @@ -27,6 +27,8 @@ import { import { CardDocument, useAppDispatch, useAppSelector } from "@/common/types"; import { Spinner } from "@/components/Spinner"; import { AttributeVotingPanel } from "@/features/attributeVoting/AttributeVotingPanel"; +import { NoMatchReasonStrip } from "@/features/attributeVoting/NoMatchReasonStrip"; +import { PrintingConfirmStrip } from "@/features/attributeVoting/PrintingConfirmStrip"; import { STARBURST_INNER_COLOR, STARBURST_INNER_FRAMES, @@ -365,6 +367,11 @@ export function PrintingTagQueue() { // card this session - the attribute-voting follow-up panel is step 2 after that action, // not shown alongside the initial printing picker on a card the user hasn't touched yet. const [votedThisCard, setVotedThisCard] = useState(false); + // which follow-up to show below the candidate grid once votedThisCard is true - tracks + // the *submitted* vote's own isNoMatch flag directly (not consensus.isNoMatch, which + // reflects the aggregate resolved outcome and can lag a single vote), so a no-match tap + // always gets the reason strip even before/without flipping the card's consensus. + const [lastVoteWasNoMatch, setLastVoteWasNoMatch] = useState(false); // guards the fetch effect below against React 18 Strict Mode's dev-time double-invoke, // which would otherwise append the same page's cards twice const fetchedPagesRef = React.useRef>(new Set()); @@ -410,6 +417,7 @@ export function PrintingTagQueue() { useEffect(() => { setRevealed(false); setVotedThisCard(false); + setLastVoteWasNoMatch(false); }, [currentCard?.identifier]); useEffect(() => { @@ -461,11 +469,10 @@ export function PrintingTagQueue() { .then((response) => { setConsensus(response); setVotedThisCard(true); - // if this vote itself resolved the printing (e.g. it broke a tie), there's nothing - // left to ask about - advance immediately rather than showing the attribute panel. - if (response.resolvedPrinting != null) { - advance(); - } + setLastVoteWasNoMatch(isNoMatch); + // No immediate advance here even if this vote resolved the printing (e.g. it broke + // a tie) - PrintingConfirmStrip gets a brief, skippable dwell below to confirm + // full-art/borderless before the queue moves on. }) .catch(() => dispatch( @@ -659,8 +666,35 @@ export function PrintingTagQueue() { ))} + {votedThisCard && + consensus?.resolvedPrinting != null && + backendURL != null && ( +
+
+ +
+ )} {votedThisCard && consensus?.resolvedPrinting == null && + lastVoteWasNoMatch && + backendURL != null && ( +
+
+ +
+ )} + {votedThisCard && + consensus?.resolvedPrinting == null && + !lastVoteWasNoMatch && backendURL != null && (

@@ -678,11 +712,13 @@ export function PrintingTagQueue() { > Skip - {votedThisCard && consensus?.resolvedPrinting == null && ( - - )} + {votedThisCard && + consensus?.resolvedPrinting == null && + !lastVoteWasNoMatch && ( + + )}
)} diff --git a/frontend/src/mocks/handlers.ts b/frontend/src/mocks/handlers.ts index d5e57a102..21e2643d2 100644 --- a/frontend/src/mocks/handlers.ts +++ b/frontend/src/mocks/handlers.ts @@ -545,6 +545,40 @@ export const tagsTwoResults = http.get(buildRoute("2/tags/"), () => HttpResponse.json({ tags: ["Tag 1", "Tag 2"] }, { status: 200 }) ); +const serialisedTag = (name: string) => ({ + name, + aliases: [], + isEnabledByDefault: true, + parent: null, + children: [], +}); + +// all six no-match reason tags exist server-side - NoMatchReasonStrip shows every chip +export const tagsAllNoMatchReasonTags = http.get(buildRoute("2/tags/"), () => + HttpResponse.json( + { + tags: [ + "custom-art", + "altered-frame", + "upscaled", + "ai-art", + "no-collector-line", + "non-english", + ].map(serialisedTag), + }, + { status: 200 } + ) +); + +// only two of the six reason tags exist server-side (seed_no_match_reason_tags hasn't fully +// run, or ran on an older version of the taxonomy) - NoMatchReasonStrip should hide the rest +export const tagsSomeNoMatchReasonTags = http.get(buildRoute("2/tags/"), () => + HttpResponse.json( + { tags: ["custom-art", "ai-art"].map(serialisedTag) }, + { status: 200 } + ) +); + //# endregion //# region sample cards @@ -729,6 +763,36 @@ export const submitPrintingTagResolvesToPrintingCandidate1 = http.post( ) ); +// printingCandidate2 (unlike printingCandidate1) has fullArt/isBorderless both true - used to +// exercise PrintingConfirmStrip's pre-fill-from-candidate-metadata behaviour in both states. +export const submitPrintingTagResolvesToPrintingCandidate2 = http.post( + buildRoute("2/submitPrintingTag/"), + () => + HttpResponse.json( + { + resolvedPrinting: printingCandidate2, + isNoMatch: false, + voteTally: [ + { printing: printingCandidate2, isNoMatch: false, count: 1 }, + ], + }, + { status: 200 } + ) +); + +export const submitPrintingTagNoMatch = http.post( + buildRoute("2/submitPrintingTag/"), + () => + HttpResponse.json( + { + resolvedPrinting: null, + isNoMatch: true, + voteTally: [{ isNoMatch: true, count: 1 }], + }, + { status: 200 } + ) +); + export const printingTagQueueOneResult = http.get( buildRoute("2/printingTagQueue/"), () => diff --git a/frontend/tests/NoMatchReasonStrip.spec.ts b/frontend/tests/NoMatchReasonStrip.spec.ts new file mode 100644 index 000000000..0be57838e --- /dev/null +++ b/frontend/tests/NoMatchReasonStrip.spec.ts @@ -0,0 +1,131 @@ +import { expect } from "@playwright/test"; + +import { + defaultHandlers, + printingCandidatesTwoResults, + printingConsensusUnresolved, + printingTagQueueOneResult, + submitPrintingTagNoMatch, + submitTagVoteResolvesToApply, + tagsAllNoMatchReasonTags, + tagsSomeNoMatchReasonTags, +} from "@/mocks/handlers"; + +import { test } from "../playwright.setup"; +import { loadPageWithDefaultBackend } from "./test-utils"; + +test.describe("NoMatchReasonStrip tests", () => { + test("shows the reason strip (not the general attribute panel) after a no-match vote", async ({ + page, + network, + }) => { + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagNoMatch, + tagsAllNoMatchReasonTags, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByText("No match").click(); + + const strip = page.getByTestId("no-match-reason-strip"); + await expect(strip).toBeVisible(); + await expect(strip.getByText("Custom art")).toBeVisible(); + await expect(strip.getByText("Altered frame")).toBeVisible(); + await expect(strip.getByText("Upscaled")).toBeVisible(); + await expect(strip.getByText("AI art")).toBeVisible(); + await expect(strip.getByText("No collector line")).toBeVisible(); + await expect(strip.getByText("Non-English")).toBeVisible(); + await expect(page.getByTestId("attribute-voting-panel")).not.toBeVisible(); + }); + + test("hides chips for reason tags that don't exist server-side yet", async ({ + page, + network, + }) => { + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagNoMatch, + tagsSomeNoMatchReasonTags, // only custom-art and ai-art exist + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByText("No match").click(); + + const strip = page.getByTestId("no-match-reason-strip"); + await expect(strip).toBeVisible(); + await expect(strip.getByText("Custom art")).toBeVisible(); + await expect(strip.getByText("AI art")).toBeVisible(); + await expect(strip.getByText("Altered frame")).not.toBeVisible(); + await expect(strip.getByText("Upscaled")).not.toBeVisible(); + await expect(strip.getByText("No collector line")).not.toBeVisible(); + await expect(strip.getByText("Non-English")).not.toBeVisible(); + }); + + test("tapping a reason chip submits a positive tag vote and advances the queue", async ({ + page, + network, + }) => { + let submittedBody: { tagName?: string; polarity?: number } = {}; + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagNoMatch, + submitTagVoteResolvesToApply, + tagsAllNoMatchReasonTags, + ...defaultHandlers + ); + page.on("request", async (request) => { + if (request.url().includes("/2/submitTagVote/")) { + submittedBody = request.postDataJSON(); + } + }); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByText("No match").click(); + await page.getByTestId("no-match-reason-ai-art").click(); + + await expect( + page.getByText("You're all caught up - no cards left to tag right now!") + ).toBeVisible(); + expect(submittedBody.tagName).toBe("ai-art"); + expect(submittedBody.polarity).toBe(1); + }); + + test("skip advances without submitting any tag vote", async ({ + page, + network, + }) => { + let tagVoteSubmitted = false; + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagNoMatch, + tagsAllNoMatchReasonTags, + ...defaultHandlers + ); + page.on("request", (request) => { + if (request.url().includes("/2/submitTagVote/")) { + tagVoteSubmitted = true; + } + }); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByText("No match").click(); + await expect(page.getByTestId("no-match-reason-strip")).toBeVisible(); + await page.getByTestId("no-match-reason-skip").click(); + + await expect( + page.getByText("You're all caught up - no cards left to tag right now!") + ).toBeVisible(); + expect(tagVoteSubmitted).toBe(false); + }); +}); diff --git a/frontend/tests/PrintingConfirmStrip.spec.ts b/frontend/tests/PrintingConfirmStrip.spec.ts new file mode 100644 index 000000000..4d631f1dc --- /dev/null +++ b/frontend/tests/PrintingConfirmStrip.spec.ts @@ -0,0 +1,119 @@ +import { expect } from "@playwright/test"; + +import { + defaultHandlers, + printingCandidatesTwoResults, + printingConsensusUnresolved, + printingTagQueueOneResult, + submitPrintingTagResolvesToPrintingCandidate1, + submitPrintingTagResolvesToPrintingCandidate2, + submitTagVoteResolvesToApply, +} from "@/mocks/handlers"; + +import { test } from "../playwright.setup"; +import { loadPageWithDefaultBackend } from "./test-utils"; + +test.describe("PrintingConfirmStrip tests", () => { + test("pre-fills chips from the resolved candidate's own fullArt/isBorderless flags", async ({ + page, + network, + }) => { + // printingCandidate1 has fullArt: false, isBorderless: false + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagResolvesToPrintingCandidate1, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByAltText("abc 1").click(); + + const strip = page.getByTestId("printing-confirm-strip"); + await expect(strip).toBeVisible(); + await expect(page.getByTestId("printing-confirm-full-art")).not.toHaveClass( + /highlighted/ + ); + await expect( + page.getByTestId("printing-confirm-borderless") + ).not.toHaveClass(/highlighted/); + }); + + test("pre-fills chips as highlighted when the candidate's flags are true", async ({ + page, + network, + }) => { + // printingCandidate2 has fullArt: true, isBorderless: true + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagResolvesToPrintingCandidate2, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByAltText("xyz 42").click(); + + await expect(page.getByTestId("printing-confirm-full-art")).toHaveClass( + /highlighted/ + ); + await expect(page.getByTestId("printing-confirm-borderless")).toHaveClass( + /highlighted/ + ); + }); + + test("tapping a chip submits a tag vote and marks it confirmed", async ({ + page, + network, + }) => { + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagResolvesToPrintingCandidate1, + submitTagVoteResolvesToApply, + ...defaultHandlers + ); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByAltText("abc 1").click(); + await page.getByTestId("printing-confirm-full-art").click(); + + await expect(page.getByTestId("printing-confirm-full-art")).toContainText( + "Confirmed" + ); + // the strip stays put - confirming a chip doesn't itself advance the queue + await expect(page.getByTestId("printing-confirm-strip")).toBeVisible(); + }); + + test("skip advances without submitting any tag vote", async ({ + page, + network, + }) => { + let tagVoteSubmitted = false; + network.use( + printingTagQueueOneResult, + printingCandidatesTwoResults, + printingConsensusUnresolved, + submitPrintingTagResolvesToPrintingCandidate1, + ...defaultHandlers + ); + page.on("request", (request) => { + if (request.url().includes("/2/submitTagVote/")) { + tagVoteSubmitted = true; + } + }); + await loadPageWithDefaultBackend(page, "printingQueue"); + + await page.getByAltText("abc 1").click(); + await expect(page.getByTestId("printing-confirm-strip")).toBeVisible(); + await page.getByTestId("printing-confirm-skip").click(); + + await expect( + page.getByText("You're all caught up - no cards left to tag right now!") + ).toBeVisible(); + expect(tagVoteSubmitted).toBe(false); + }); +}); diff --git a/frontend/tests/PrintingTagQueue.spec.ts b/frontend/tests/PrintingTagQueue.spec.ts index 40513e29b..d1c368410 100644 --- a/frontend/tests/PrintingTagQueue.spec.ts +++ b/frontend/tests/PrintingTagQueue.spec.ts @@ -100,7 +100,7 @@ test.describe("PrintingTagQueue tests", () => { ).toBeVisible(); }); - test("submitting a vote shows flavor text and advances the queue", async ({ + test("submitting a resolving vote shows the confirm strip, then advances on continue", async ({ page, network, }) => { @@ -115,7 +115,15 @@ test.describe("PrintingTagQueue tests", () => { await page.getByAltText("abc 1").click(); - // that was the only card in the queue - submitting advances past the end of it + // a resolving vote no longer advances immediately - the confirm strip gets a beat first + await expect(page.getByTestId("printing-confirm-strip")).toBeVisible(); + await expect( + page.getByText("You're all caught up - no cards left to tag right now!") + ).not.toBeVisible(); + + await page.getByTestId("printing-confirm-continue").click(); + + // that was the only card in the queue - continuing advances past the end of it await expect( page.getByText("You're all caught up - no cards left to tag right now!") ).toBeVisible(); diff --git a/schemas/schemas/PrintingCandidate.json b/schemas/schemas/PrintingCandidate.json index 21c8d9e18..6702cfa65 100644 --- a/schemas/schemas/PrintingCandidate.json +++ b/schemas/schemas/PrintingCandidate.json @@ -12,6 +12,7 @@ "smallThumbnailUrl": { "type": "string" }, "mediumThumbnailUrl": { "type": "string" }, "fullArt": { "type": "boolean" }, + "isBorderless": { "type": "boolean" }, "frame": { "type": "string" }, "releasedAt": { "type": ["string", "null"] } }, @@ -25,6 +26,7 @@ "smallThumbnailUrl", "mediumThumbnailUrl", "fullArt", + "isBorderless", "frame" ], "additionalProperties": false