From f37c493aeea99b79593070f2df13ec32009bb8ee Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 23 Jul 2026 18:21:18 +0000 Subject: [PATCH 1/2] Render orphan Drive-ID cards from text/XML import (foreign-order resilience Phase 1) Fixes text-import [mpc:] tokens and unindexed XML slots (incl. the b:null back-face case) getting silently cleared or landing in Invalid Cards, by synthesizing a client-side orphan CardDocument and skipping the invalidation listener for identifiers that look like real Drive file IDs. Orphan images load direct from Google, never our CDN. Closes #324 (Phase 1 only - Phase 2 source-suggestion deferred). --- CLAUDE.md | 6 + docs/README.md | 5 + docs/features/foreign-order-resilience.md | 220 ++++++++++++++++++ docs/upstreaming/extractable-primitives.md | 1 + frontend/src/common/orphanCard.test.ts | Bin 0 -> 4316 bytes frontend/src/common/orphanCard.ts | 180 ++++++++++++++ frontend/src/common/processing.test.ts | 77 ++++++ frontend/src/common/processing.ts | 57 ++++- frontend/src/common/types.ts | 10 + frontend/src/features/card/Card.test.tsx | 66 ++++++ frontend/src/features/card/Card.tsx | 101 ++++++-- .../card/DeckbuilderConfirmAffordance.tsx | 4 + .../src/features/download/downloadXML.test.ts | 24 ++ frontend/src/features/import/ImportText.tsx | 8 + .../src/features/import/ImportXML.test.ts | 65 ++++++ frontend/src/features/pdf/pdfImage.ts | 63 +++++ frontend/src/store/listenerMiddleware.test.ts | 181 ++++++++++++++ frontend/src/store/listenerMiddleware.ts | 63 +++-- .../src/store/slices/cardDocumentsSlice.ts | 78 ++++++- frontend/tests/OrphanRendering.spec.ts | 123 ++++++++++ 20 files changed, 1292 insertions(+), 40 deletions(-) create mode 100644 docs/features/foreign-order-resilience.md create mode 100644 frontend/src/common/orphanCard.test.ts create mode 100644 frontend/src/common/orphanCard.ts create mode 100644 frontend/src/store/listenerMiddleware.test.ts create mode 100644 frontend/tests/OrphanRendering.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index 69fe79823..b1a068cb0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -218,6 +218,12 @@ external reader's orientation to the whole fork, see reusable, permission-triggered contextual consent toast (`useConsentToast` - `ConsentToast.tsx`), issue #204. No dependency on any consumer feature yet; issue #203 (client-side phash contribution) is the first planned one. +- [`docs/features/foreign-order-resilience.md`](docs/features/foreign-order-resilience.md) + — issue #324 Phase 1 (shipped 2026-07-23): rendering "orphan" cards + (Drive file IDs the catalog has never indexed) from text (`[mpc:]` + token) and XML import, direct-from-Google image fetch (never our own + CDN), the invalidation-listener root-cause fix, round-trip export, and + what's deferred to Phase 2. - [`docs/upstreaming/vote-system.md`](docs/upstreaming/vote-system.md) — cherry-pick extraction manifest for the vote system (companion to the Upstreaming workflow in `docs/infrastructure.md`); accurate through diff --git a/docs/README.md b/docs/README.md index c303e3a08..9c8d34bd1 100644 --- a/docs/README.md +++ b/docs/README.md @@ -136,6 +136,11 @@ The methodology and the systems it governs. bottom-corner accept/decline prompt shown only right before an action that needs it, per-permission-key session scoping, no dependency on any consumer feature yet. +- [`features/foreign-order-resilience.md`](features/foreign-order-resilience.md) + — issue #324 Phase 1: rendering "orphan" cards (Drive file IDs the + catalog has never indexed) from text/XML import, direct-from-Google + fetch with tiered sizing, the invalidation-listener root-cause fix, + round-trip export, and what's deferred to Phase 2. ## Using it diff --git a/docs/features/foreign-order-resilience.md b/docs/features/foreign-order-resilience.md new file mode 100644 index 000000000..aba9d201a --- /dev/null +++ b/docs/features/foreign-order-resilience.md @@ -0,0 +1,220 @@ +# Foreign-order resilience (issue #324) + +Owner-ratified direction, 2026-07-22, promoted to high priority 2026-07-23 +after two live symptoms: a text-import `[mpc:]` token not registering at +all, and an XML-imported unindexed back face landing in the Invalid Cards +modal as `Back | b:null | `. **Phase 1 (orphan rendering) shipped +2026-07-23** — this doc covers what shipped, what was adapted from the +original spec, and what's still deferred to Phase 2. + +## The problem + +The editor previously rendered only what the catalog already knows: a +project-member `selectedImage` that doesn't appear in the catalog's own +search results for its query got silently deselected (or, if search results +existed at all for the query, recorded as an "Invalid Card" and replaced +with the first real match) — regardless of whether that identifier was +genuinely garbage or a real, fetchable Google Drive file ID from an order +built against another mpc-autofill-lineage catalog. + +## What "orphan" means here + +An **orphan** is a `CardDocument` synthesized entirely client-side +(`frontend/src/common/orphanCard.ts`) for a project-member identifier the +catalog has never indexed. It carries `isOrphan: true` and deliberately +never sets `sourceType` — every consumer that must not route it through our +own image-CDN Worker/R2 bucket (`common/image.ts`'s `getBucketImageURL`/ +`getWorkerImageURL` both gate on `sourceType === SourceType.GoogleDrive`) or +offer it tag/consensus surfaces checks `isOrphan` (or, for the CDN-routing +case, gets that behaviour for free just by `sourceType` being absent). + +## How an identifier becomes an orphan + +1. **Text import**: a `[mpc:]` token anywhere after the query text (e.g. + `1x Kharn [mpc:1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn]`) is parsed by + `processing.ts`'s `extractDriveIdBracketToken`/`unpackLine` — an + _addition_ to the pre-existing `query@id` (`SelectedImageSeparator`) + syntax, not a replacement. Both forms end up in the same + `ProjectMember.selectedImage` field, so everything below applies + identically regardless of which syntax supplied it. +2. **XML import**: `ImportXML.tsx`'s `parseXmlImport` already read a + ``'s raw `` text verbatim before this feature — the parser + itself needed no change. The reported `b:null` case is + `parseXmlImport`'s own pre-existing fallback: a front slot with no + matching `` entry gets `{ query: { query: null, cardType: Cardback }, selectedImage: text> }` + — an entirely legitimate, name-less orphan. +3. Either way, `cardDocumentsSlice.ts`'s `fetchCardDocuments` thunk now + fetches against the **union** of the pre-existing search-derived + identifier set (`selectUniqueCardIdentifiers`) and every raw + `selectedImage` a project member actually references + (`selectProjectMemberIdentifiers`) — the search-derived set alone would + never even attempt to resolve an orphan, since by definition it never + appears in any search result. Anything still unresolved after both the + local (client-search) and remote (`/2/cards/`) lookups, that also passes + `orphanCard.ts`'s `isLikelyDriveFileId` allowlist + (`^[A-Za-z0-9_-]{10,200}$`, the owner's 2026-07-22 security-review + ruling), gets synthesized into a `CardDocument` and merged into + `cardDocuments.cardDocuments` — same map real catalog cards live in, so + every existing selector/consumer (`selectCardDocumentByIdentifier`, + `Card.tsx`, `downloadXML.ts`, `pdfImage.ts`, …) picks it up automatically. + +## The listener fix (the actual root cause) + +`listenerMiddleware.ts`'s pre-existing invalid-identifier listener +(triggered on `fetchSearchResults.fulfilled`/`fetchCardbacks.fulfilled`) +unconditionally cleared any `selectedImage` absent from the catalog's +search results for its query — this is what silently dropped both reported +symptoms, independent of anything in the parsers. It now also triggers on +`fetchCardDocuments.fulfilled` (needed because `cardDocuments.cardDocuments` +isn't settled yet on the earlier two triggers) and skips the +clear/Invalid-Cards path when the identifier is either already resolved to +an orphan (`selectCardDocumentByIdentifier(...).isOrphan === true`) or not +yet resolved either way but still looks like a real Drive file ID +(`isLikelyDriveFileId`) — erring towards "wait and see" rather than +prematurely clearing. A genuinely-known-but-currently-filtered/removed +catalog card (defined in `cardDocuments`, `isOrphan` false/undefined) still +falls through to the pre-existing Invalid Cards flow unchanged — this is a +regression-guarded addition, not a loosening of that existing behaviour +(see `listenerMiddleware.test.ts`'s explicit regression-guard case). + +## Rendering + +- **Image source**: direct from Google + (`https://lh4.googleusercontent.com/d/`), never through the image-CDN + Worker/R2 bucket — that cache stays catalog-only (issue's own Phase 1 + bullet). Two size tiers mirror the Worker's own `=h` URL shape exactly + (`image-cdn/src/service/GoogleDriveService.ts`'s `getLH4Params`/ + `image-cdn/src/types.ts`'s `ImageSizes`): 400px height for the editor + grid/preview tile, 800px for the (currently unused) "large" tier, and no + size suffix at all — the original file — for PDF export + (`pdfImage.ts`'s `getOrphanPDFImageURL`/`getPDFImageBlob`, only ever + invoked on an actual export action, never speculatively). +- **`referrerpolicy="no-referrer"`** on every orphan `` (owner + ruling — the fetch itself remaining a signal to the file's owner is an + accepted residual on this, an author-only, surface). +- **Visually distinct treatment**: a small corner badge + (`Card.tsx`'s `OrphanBadge`) showing the synthesized document's own + `sourceName` ("Your file" on the editor/author surface this Phase 1 pass + covers). **Real-browser-only bug caught during Playwright verification, + not by Jest**: Bootstrap's `.ratio > *` rule stretches every direct child + of the aspect-ratio card wrapper to `width:100%; height:100%; top:0; left:0` — the badge needs `width: auto; height: auto;` to opt back out, + the same defensive pattern the pre-existing `CardIcon`/`MatchIndicatorIcon` + corner icons already use. jsdom (Jest) never computes layout, so this + class of bug is invisible to unit tests; a real Playwright screenshot + caught it as a badge stretched to cover the entire card. +- **No tags/consensus surfaces**: `DeckbuilderConfirmAffordance.tsx` gates + off for `card?.isOrphan === true`; `Card.tsx` suppresses + click-to-open-detailed-view for orphans (`canShowDetailedView`) since that + modal's own surfaces (printing tags, reporting) don't apply to a card the + catalog has never indexed. +- **"Image unavailable" degrade, not a stuck spinner, on a genuine fetch + failure**: `useImageSrc`'s `onError` handler was fixed to check whether a + bucket URL is actually configured (`imageBucketURLValid`) before deciding + whether to retry via the fallback tier or go straight to `errored` — an + orphan (no bucket at all) was already loading its one-and-only URL from + the very first render, just still internally labelled + `"loading-from-bucket"` (this hook's fixed initial state); without the + fix, a single failure would relabel to `"loading-from-fallback"` and + re-render with the _same_ src string, which browsers don't re-fetch, + leaving the spinner stuck forever. This also fixes the identical latent + issue for any non-Google-Drive card with no bucket configured (e.g. AWS S3 + sources, or local dev with `NEXT_PUBLIC_IMAGE_BUCKET_URL` unset) — not + orphan-specific, just the same code path. + +## Round-trip (export/re-import) + +`downloadXML.ts`'s `createCardElement` already returned `null` (silently +dropping the slot) for any identifier absent from `cardDocuments` — since +orphans are now present there, this is unaffected and Just Works. The +synthesized orphan's `searchq` field (which `` re-export reads) +deliberately carries the _sanitized real stand-in query text_, never the +`"Unindexed card"` display fallback — the fallback name would otherwise +corrupt a re-exported file with fabricated text that was never the user's +actual search query. `cardDocumentsSlice.ts`'s `buildStandInQueryByIdentifier` +harvests each identifier's real stand-in name/cardType from the project +member(s) that reference it before synthesis, specifically so this +round-trip stays faithful. + +## Replacement suggestions (already-working infra, not new work) + +The owner's later scope addition ("orphan slot retains its XML search +query, offers 'find this card in our catalog'") needed **no new code**: an +orphan's `selectedImage` no longer gets forcibly cleared, but its +`SearchQuery` was never touched either — `CardSlot.tsx`'s existing +`searchResultsForQueryOrDefault`/grid-selector/version-picker machinery +already operates independently of whether the currently-selected image is +an orphan, so a real catalog match (if the query happens to find one) is +already offered exactly as it always was for any slot. + +## Deviations from the original issue-body spec + +- **Resolution-tier param shape**: the owner's own 2026-07-22 follow-up + comment loosely specified `=w` (width). Implemented instead using the + image-CDN Worker's own already-tested `=h` (height) convention and + `lh4.googleusercontent.com` host (not `lh3`) — exact parity with the + Worker's `GoogleDriveService.getImageURL`/`ImageSizes` rather than + introducing a second, unverified size-parameter shape for the same + visual-size discipline the owner asked for. +- **`[mpc:]` text-import syntax**: not present anywhere in the ratified + issue body or its comments — designed fresh to match the owner's own + exact reported repro line, as an _additional_ syntax alongside the + pre-existing `query@id` form (see "How an identifier becomes an orphan" + above), rather than the issue's proposed identifier-lookup-miss path + alone (which only covered XML import). + +## Explicitly deferred (not shipped in this pass) + +- **Phase 2 (source derivation + suggestion)**: the Drive `files.get` + parents-walk backend endpoint, the "suggest this drive as a source" + one-tap flow, and its credential-class verification — entirely + out of scope for Phase 1, per the issue's own phasing. +- **Per-surface consent ruling** (owner's second security-review round): + self-import/own saved decks allowed by default; shared decks viewed by + others deny-by-default behind an explicit per-deck recipient opt-in with + a reversible "Hide" control. **Only the editor/self-import surface is + wired up in this pass** — `SharedDeckViewer.tsx` and any other read-only + viewer were not touched, so they simply don't synthesize orphan + CardDocuments at all yet (safe-by-omission: deny-by-default is the + correct posture there, just not yet built as an explicit opt-in flow). + Building that opt-in UI is future work, not a regression. +- **Bleed normalization for orphans**: `PDF.tsx`'s + `isBleedNormalizationEligible` still gates on `sourceType === GoogleDrive || sourceType === LocalFile`, which an orphan (no + `sourceType`) never matches — an orphan's PDF embed uses the plain + scale-transform path, not bleed-measured/corrected geometry. Satisfies + the acceptance bar ("an exported PDF containing an orphan embeds the + high-res image") without extending bleed-measurement to a source this + catalog has no calibration data for. +- **"Download Images" bulk raw-file export** (`ExportImages.tsx`) still + filters to `sourceType === SourceType.GoogleDrive` only — orphans are + silently excluded from that surface. Not mentioned in the issue's + acceptance criteria (which is PDF-export-specific); flagged here as a + plausible related surface for a future pass, not built. +- **Policy/Terms note** (owner's "non-blocking" queue item: third-party + image display + takedown path) — not written in this pass. + +## Key files + +- `frontend/src/common/orphanCard.ts` — the allowlist regex, direct-Google + URL builder (small/large/full tiers), stand-in-name sanitizer, and + `synthesizeOrphanCardDocument`/`buildOrphanCardDocuments`. +- `frontend/src/common/processing.ts` — `extractDriveIdBracketToken`, + wired into `unpackLine`. +- `frontend/src/store/slices/cardDocumentsSlice.ts` — the broadened + identifier union and orphan synthesis in `fetchCardDocuments`. +- `frontend/src/store/listenerMiddleware.ts` — the invalid-identifier + listener's orphan-candidate skip. +- `frontend/src/features/card/Card.tsx` — `OrphanBadge`, the + bucket-validity-aware `onError` fix, click-to-detail suppression. +- `frontend/src/features/card/DeckbuilderConfirmAffordance.tsx` — the + consensus-surface gate. +- `frontend/src/features/pdf/pdfImage.ts` — `getOrphanPDFImageURL`, the + full-resolution PDF-export path. +- `frontend/src/common/types.ts` — the `isOrphan?: boolean` marker on the + frontend's own `Card`/`CardDocument` type (never present in the + quicktype-generated `schema_types.ts`). +- Tests: `orphanCard.test.ts`, `processing.test.ts` (bracket-token cases), + `listenerMiddleware.test.ts`, `ImportXML.test.ts` (front + the b:null + back-face case), `downloadXML.test.ts` (round-trip), `Card.test.tsx` + (badge/click-suppression/error-degrade), and the Playwright + `tests/OrphanRendering.spec.ts` (both reported symptoms, end to end, + with screenshots). diff --git a/docs/upstreaming/extractable-primitives.md b/docs/upstreaming/extractable-primitives.md index 1a0ef9915..45a8e1727 100644 --- a/docs/upstreaming/extractable-primitives.md +++ b/docs/upstreaming/extractable-primitives.md @@ -85,6 +85,7 @@ coupling to the vote system is. | Image-CDN URL helpers | `frontend/src/common/image.ts` | Resolves a card identifier to its bucket/worker CDN URL | upstream, proxies-at-home (with their own CDN) | entangled-with-image-cdn-infra | — | | Contextual consent toast | `frontend/src/features/consent/consentToast.ts`, `ConsentToast.tsx`, `useConsentToast.tsx` | General-purpose permission-triggered accept/decline toast (`useConsentToast().requestConsent(key, message)`), per-permission-key session-scoped decision, no consumer feature wired in yet (issue #204) | upstream, proxies-at-home | CLEAN | — | | Sliced-wordmark pop animation (`WhatsThatWords`) | `frontend/src/features/questionFeed/WhatsThatWords.tsx` | Inlines an SVG wordmark's path data once and crops it into N independently-animated bands via per-instance `viewBox`, staggered CSS keyframe pop + `prefers-reduced-motion` fallback — the technique (not the specific WHAT'S/THAT/CARD? content) generalizes to any multi-word SVG lockup | upstream, proxies-at-home | CLEAN | — | +| Orphan-card synthesis (foreign-order resilience) | `frontend/src/common/orphanCard.ts` | Synthesizes a minimal, renderable `CardDocument` for an image identifier the catalog hasn't indexed (Drive-ID allowlist regex, direct-from-source thumbnail/full-res URL builder, untrusted-name sanitizer) — no catalog/consensus lookup, no vote system, no auth (issue #324 Phase 1) | upstream, proxies-at-home (with their own CDN) | CLEAN | — | ## Frontend — PDF / export diff --git a/frontend/src/common/orphanCard.test.ts b/frontend/src/common/orphanCard.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..c57c2a27d42ab8abe4ae77b29d62a22ab87763bb GIT binary patch literal 4316 zcmcInZExE)5Z>qfiCwh;lQF*}?%S54!@8s$jHd0-rt60yXlaSISW~1!Qc2t*z<$ks z*nY|GNJ)`w<)vL`AaNocd3O(=d+sRmr8W*eZ2?RRA=4LTnK8A;OuDa=BInB4e*4`d zXLJ7gl|iA=MeomN_3uUHPUt7(St2f zf&zrY;cCfY!`>zt&)GYNFDR-%=4xQEOd7{dn)^Tu~e;w}}x`9!DJ@Mtz8T@bDLZ+>TopM=VMB(pg~Mo3KtK;C;dj zafiu%YYhNDTk^zVE*mElH^S}phED&+V`mE9dw_>@j=%9M_%!?j?twlP?hh_w4iR%& zgUt}In>&nxedTP%q`lS~BI}q?Ld_u+7OA99@r06$8J0M1>}Pma+Km{TFey?H>8)GQ z%>zM#Nmf`fl`D`MW3kR)NT1=HMMI<&XD%LZ@ASGod(bZk(WvjL*jf&&!*PXC77CT} znNSGMOdC2Xp{i7oqhcnwfdMFfg5RF5rtoJ9C=H>w#3e`B5l0*$m;p)*>CX0cnb7Mh zhyiRvdvv1Jbw--tmyy5>F8lNz9Myj!{bPC({T83UeAS;ma*yhe^ zWxIXs_W-bW z?~J$MN6$7cCE_FyDMZ#gZ*Rv{YTpQ##nC^1{awS}YIN(2xMC7%ArYvNFk=LYnXqAS zK@`(cXD#=g+Umk2+|MQPf^!6VS|khZMnUlaIpwMbH=n_q>L#2- z2?i}5RuQf@`LS`;x2EZBW06+PrQEb3nVTd-jSZrYt>4dtKIl6e_-BF$ZMd#5$sKMZ3DT6^<^sTrxO~H zkcNii2x@Q=9?)H6@Vg%y`emsTMD41!8C!NMCmrH{t7Jb*9cD#7RahIZjV$?alR;>c zr#VkWkzcjl6|)CzN911<(Z + DriveFileIdPattern.test(identifier); + +/** Mirrors image-cdn/src/types.ts's ImageSizes exactly - the small/large thumbnail tiers an + * orphan's editor-grid rendering must match in size discipline (owner ruling, 2026-07-22). */ +const OrphanImageHeightPx = { small: 400, large: 800 } as const; + +const DirectGoogleImageOrigin = "https://lh4.googleusercontent.com"; + +/** + * Build a direct-from-Google image URL for an orphan identifier - NEVER routed through our + * image-CDN Worker or R2 bucket (see module doc). `height` mirrors the Worker's own `=h` + * URL suffix (GoogleDriveService.getImageURL) for the small/large tiers; omitting it requests + * the original, unresized file - the "full" tier, used only for PDF export, never the editor + * grid (owner ruling: "the editor grid never requests it"). + * + * The identifier is validated against `DriveFileIdPattern` before it ever reaches URL + * construction, and is placed in the URL via the `URL` constructor (not string + * interpolation) - only the size suffix, which is always one of two code-fixed values, is + * appended as a literal. + */ +export const buildOrphanImageURL = ( + identifier: string, + height: number | undefined +): string | undefined => { + if (!isLikelyDriveFileId(identifier)) { + return undefined; + } + const base = new URL(`/d/${identifier}`, DirectGoogleImageOrigin).toString(); + return height !== undefined ? `${base}=h${height}` : base; +}; + +/** The editor grid / preview tile tier - never used for PDF export. */ +export const getOrphanSmallImageURL = ( + identifier: string +): string | undefined => + buildOrphanImageURL(identifier, OrphanImageHeightPx.small); + +/** Unused today (no orphan-specific "large" surface yet), kept for parity with the catalog + * path's own small/large/full tier triad. */ +export const getOrphanLargeImageURL = ( + identifier: string +): string | undefined => + buildOrphanImageURL(identifier, OrphanImageHeightPx.large); + +/** The PDF-export tier - original resolution, fetched only on an explicit export action, never + * speculatively (owner ruling). */ +export const getOrphanFullResolutionImageURL = ( + identifier: string +): string | undefined => buildOrphanImageURL(identifier, undefined); + +// Built from character codes rather than a literal escape sequence in this source +// file, to avoid embedding raw control bytes in the repo - equivalent to +// /[\x00-\x1F\x7F]/g. +const ControlCharPattern = new RegExp( + `[${String.fromCharCode(0)}-${String.fromCharCode(31)}${String.fromCharCode( + 127 + )}]`, + "g" +); +const StandInNameMaxLength = 120; + +/** + * The stand-in name Phase 1 shows for an orphan (its slot's own XML/text search query) is + * untrusted input from a file the user uploaded, not our own indexing pipeline - stripped of + * control characters and length-capped before it reaches any display or `data-card-*` sink + * (owner ruling, 2026-07-22 security review). React's JSX text nodes and `getCardDataAttributes`'s + * DOM-API attribute assignment are already immune to injection either way (see + * docs/features/card-dom-api.md), but the cap/strip still applies as defence in depth and to + * keep a maliciously huge query string from bloating the DOM. + */ +export const sanitizeStandInName = (rawName: string): string => { + const stripped = rawName.replace(ControlCharPattern, "").trim(); + return stripped.length > StandInNameMaxLength + ? `${stripped.slice(0, StandInNameMaxLength)}…` + : stripped; +}; + +/** Shown when an orphan's originating slot carried no usable query text (e.g. the reported + * `b:null` back-face case - the foreign XML's own `` element was empty). */ +export const OrphanFallbackName = "Unindexed card"; + +/** + * Synthesize a minimal CardDocument for an identifier the catalog has never indexed - Phase 1's + * "on identifier lookup miss" step. Only ever called for identifiers that already passed + * `isLikelyDriveFileId` (see cardDocumentsSlice.ts's fetchCardDocuments thunk, the sole caller). + * Deliberately never sets `sourceType` - leaving it `undefined` is what keeps + * `common/image.ts`'s bucket/Worker URL builders (which gate on + * `sourceType === SourceType.GoogleDrive`) and `pdfImage.ts`'s source-type switch from ever + * routing an orphan through our own CDN; `isOrphan: true` is the one field every consumer that + * needs to special-case an orphan actually checks. + */ +export const synthesizeOrphanCardDocument = ( + identifier: string, + standInQuery?: { name: string | null; cardType: CardType } | undefined +): CardDocument => { + const rawName = standInQuery?.name; + const sanitizedName = + rawName != null && rawName.length > 0 + ? sanitizeStandInName(rawName) + : undefined; + return { + cardType: standInQuery?.cardType ?? CardType.Card, + dateCreated: "", + dateModified: "", + dpi: 0, + extension: "", + identifier, + isOrphan: true, + language: "EN", + mediumThumbnailUrl: getOrphanLargeImageURL(identifier), + name: sanitizedName ?? OrphanFallbackName, + printingTagStatus: PrintingTagStatus.NoMatch, + priority: 0, + // Deliberately NOT the sanitized display name - this is the round-trip field + // downloadXML.ts's createCardElement reads to rebuild the `` element on re-export + // (see docs/features/foreign-order-resilience.md's round-trip section). Falling back to the + // fabricated OrphanFallbackName here would corrupt a re-exported file with text that was + // never actually the user's search query. + searchq: sanitizedName ?? "", + size: 0, + smallThumbnailUrl: getOrphanSmallImageURL(identifier), + source: "", + sourceId: -1, + sourceName: "Your file", + sourceVerbose: "Your file", + tags: [], + }; +}; + +/** + * Build orphan CardDocuments for every identifier in `identifiers` that looks like a real Drive + * file ID - anything else is left out entirely (genuinely invalid, not an orphan candidate). + * `standInQueryByIdentifier` supplies each identifier's own project-member query text/cardType + * when known (see cardDocumentsSlice.ts), so the synthesized name and the XML round-trip's + * `searchq` field reflect what the user actually asked for, not a generic placeholder. + */ +export const buildOrphanCardDocuments = ( + identifiers: Array, + standInQueryByIdentifier: Map< + string, + { name: string | null; cardType: CardType } + > = new Map() +): { [identifier: string]: CardDocument } => + Object.fromEntries( + identifiers + .filter(isLikelyDriveFileId) + .map((identifier) => [ + identifier, + synthesizeOrphanCardDocument( + identifier, + standInQueryByIdentifier.get(identifier) + ), + ]) + ); diff --git a/frontend/src/common/processing.test.ts b/frontend/src/common/processing.test.ts index f4a5d4ca3..efcd74fb5 100644 --- a/frontend/src/common/processing.test.ts +++ b/frontend/src/common/processing.test.ts @@ -6,6 +6,7 @@ import { Token, } from "@/common/constants"; import { + extractDriveIdBracketToken, parseCSVFileAsLines, processLine, processQuery, @@ -470,6 +471,82 @@ test("a line specifying the selected image ID for both faces is processed correc ]); }); +describe("foreign-order resilience Phase 1 - [mpc:] bracket token (issue #324)", () => { + const driveId = "1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn"; + + test("extractDriveIdBracketToken strips a valid token and returns the ID", () => { + expect(extractDriveIdBracketToken(`Kharn [mpc:${driveId}]`)).toEqual([ + "Kharn", + driveId, + ]); + }); + + test("extractDriveIdBracketToken leaves text unchanged when there's no token", () => { + expect(extractDriveIdBracketToken("Kharn")).toEqual(["Kharn", undefined]); + }); + + test("extractDriveIdBracketToken leaves text unchanged when the bracketed contents don't look like a real Drive file ID", () => { + expect(extractDriveIdBracketToken("Kharn [mpc:too-short]")).toEqual([ + "Kharn [mpc:too-short]", + undefined, + ]); + }); + + test("the owner's exact reported repro line registers a selected image", () => { + // Symptom (a) from issue #324's high-priority promotion comment: this exact line was + // reported as not registering at all. + expect(processLine(`1x Kharn [mpc:${driveId}]`, dfcPairs, false)).toEqual([ + 1, + { + query: { cardType: Card, query: "kharn" }, + selectedImage: driveId, + selected: false, + }, + null, + ]); + }); + + test("a bracket token on the back face is processed correctly", () => { + expect( + processLine( + `2x front card${FaceSeparator}back card [mpc:${driveId}]`, + dfcPairs, + false + ) + ).toEqual([ + 2, + { + query: { cardType: Card, query: "front card" }, + selectedImage: undefined, + selected: false, + }, + { + query: { cardType: Card, query: "back card" }, + selectedImage: driveId, + selected: false, + }, + ]); + }); + + test("a bracket token takes precedence over a trailing @id for the same face", () => { + expect( + processLine( + `opt${SelectedImageSeparator}legacyid [mpc:${driveId}]`, + dfcPairs, + false + ) + ).toEqual([ + 1, + { + query: { cardType: Card, query: "opt" }, + selectedImage: driveId, + selected: false, + }, + null, + ]); + }); +}); + describe("file path-like identifier handling", () => { test.each([ { diff --git a/frontend/src/common/processing.ts b/frontend/src/common/processing.ts index 2b101d942..1ecd817e2 100644 --- a/frontend/src/common/processing.ts +++ b/frontend/src/common/processing.ts @@ -18,6 +18,7 @@ import { SelectedImageSeparator, Token, } from "@/common/constants"; +import { isLikelyDriveFileId } from "@/common/orphanCard"; import { CardDocument, CSVRow, @@ -141,6 +142,41 @@ const extractQuantity = (line: string): [number, string] => { return [quantity, results[2]]; }; +// Foreign-order resilience Phase 1 (issue #324) - a bracketed `[mpc:]` token, e.g. +// `1x Kharn [mpc:1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn]`, is an ADDITIONAL way to pin a line's +// selected image alongside the existing `query@id` (SelectedImageSeparator) syntax above - not +// a replacement for it. Distinct from `@id` in one deliberate way: `@id` has always been read as +// "this identifier IS a catalog image" (the search-results-driven listener in +// listenerMiddleware.ts clears it if the catalog doesn't back it up), whereas `[mpc:id]` is the +// explicit "this identifier is a Drive file ID, indexed or not" declaration - useful for pasting +// a decklist that names cards by their exact Drive file, from an order built against another +// mpc-autofill instance. Once extracted, both forms flow into the exact same +// `ProjectMember.selectedImage` field, so every downstream orphan-rendering fix (the +// cardDocumentsSlice synthesis + listenerMiddleware invalidation-skip) applies identically +// regardless of which syntax supplied it. +const DriveIdBracketTokenRegex = /\[mpc:([^\]]+)\]/i; + +/** + * Strip a `[mpc:]` token from `text` if present and its captured ID passes + * `isLikelyDriveFileId` - returns the token-stripped text plus the extracted ID (or the + * original text unchanged and `undefined` if there's no token, or its contents don't look like + * a real Drive file ID - in that case it's left in place as ordinary query text rather than + * silently eaten). + */ +export const extractDriveIdBracketToken = ( + text: string +): [string, string | undefined] => { + const match = text.match(DriveIdBracketTokenRegex); + if (match == null) { + return [text, undefined]; + } + const candidateId = match[1].trim(); + if (!isLikelyDriveFileId(candidateId)) { + return [text, undefined]; + } + return [text.replace(match[0], "").trim(), candidateId]; +}; + /** * Unpack `line` into its constituents. * @@ -150,14 +186,21 @@ const extractQuantity = (line: string): [number, string] => { * * If quantity is not specified, we assume a quantity of 1. * Specifying a back query is optional. - * Specifying an image ID (for each face) is optional. + * Specifying an image ID (for each face) is optional. A `[mpc:]` token anywhere in either + * face's text (see `extractDriveIdBracketToken`) is another way to specify the image ID, and + * takes precedence over a trailing `@id` for that same face if somehow both are present. */ function unpackLine( line: string ): [number, [string, string | null] | null, [string, string | null] | null] { const [quantity, trimmedLine] = extractQuantity(line); - const [frontLine, backLine] = trimmedLine.split(FaceSeparator); + const [rawFrontLine, rawBackLine] = trimmedLine.split(FaceSeparator); + const [frontLine, frontBracketId] = extractDriveIdBracketToken(rawFrontLine); + const [backLine, backBracketId] = + rawBackLine !== undefined + ? extractDriveIdBracketToken(rawBackLine) + : [undefined, undefined]; const faceLineRegex = new RegExp( `^(.+?)(?:${SelectedImageSeparator}(${getPhrasesNotAllowedInIdentifiersNegativeLookahead()}))?$`, @@ -172,9 +215,15 @@ function unpackLine( } return [ quantity, - [frontLineResults[1]?.trim(), frontLineResults[2]?.trim()], + [ + frontLineResults[1]?.trim(), + frontBracketId ?? frontLineResults[2]?.trim(), + ], backLineResults !== null - ? [backLineResults[1]?.trim(), backLineResults[2]?.trim()] + ? [ + backLineResults[1]?.trim(), + backBracketId ?? backLineResults[2]?.trim(), + ] : null, ]; } diff --git a/frontend/src/common/types.ts b/frontend/src/common/types.ts index 4cc60ae2f..4a8c5be5c 100644 --- a/frontend/src/common/types.ts +++ b/frontend/src/common/types.ts @@ -65,6 +65,16 @@ export type Card = Omit< > & { smallThumbnailUrl: string | undefined; mediumThumbnailUrl: string | undefined; + /** + * Frontend-only marker (never sent to or received from the backend - CardSchema is + * quicktype-generated from the API's own serialisers, which have no concept of this) for + * foreign-order resilience Phase 1 (issue #324): true only for a CardDocument synthesized + * client-side (`common/orphanCard.ts`'s synthesizeOrphanCardDocument) to represent a Google + * Drive file ID this catalog has never indexed. Every consumer that must never route an + * orphan through our own image-CDN Worker/bucket, offer it tag/consensus surfaces, or treat + * it as a real catalog match checks this flag - see docs/features/foreign-order-resilience.md. + */ + isOrphan?: boolean; }; export type CardDocument = Card; diff --git a/frontend/src/features/card/Card.test.tsx b/frontend/src/features/card/Card.test.tsx index 31f7013aa..f56f0ba46 100644 --- a/frontend/src/features/card/Card.test.tsx +++ b/frontend/src/features/card/Card.test.tsx @@ -2,6 +2,8 @@ import { fireEvent, render, screen } from "@testing-library/react"; import React from "react"; import { Provider } from "react-redux"; +import { synthesizeOrphanCardDocument } from "@/common/orphanCard"; +import { CardType } from "@/common/schema_types"; import { cardDocument1 } from "@/common/test-constants"; import { ClientSearchContextProvider } from "@/features/clientSearch/clientSearchContext"; import { ClientSearchService } from "@/features/clientSearch/clientSearchService"; @@ -33,6 +35,70 @@ function renderCard(cardOnClick?: () => void) { ); } +// Foreign-order resilience Phase 1 (issue #324): an orphan is a synthesized CardDocument for a +// Drive file ID this catalog has never indexed - "visually distinct treatment", no click-to- +// detailed-view (no tags/consensus surfaces), no fabricated DPI in the source line. +describe("orphan rendering (issue #324)", () => { + const orphanId = "1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn"; + const orphanCardDocument = synthesizeOrphanCardDocument(orphanId, { + name: "Kharn", + cardType: CardType.Card, + }); + + function renderOrphanCard() { + const store = setupStore({}); + render( + + + + + + ); + return store; + } + + it("shows the 'Your file' badge", () => { + renderOrphanCard(); + expect(screen.getByTestId("orphan-badge")).toHaveTextContent("Your file"); + }); + + it("shows the sanitized stand-in name, not a fabricated DPI", () => { + renderOrphanCard(); + expect(screen.getByText("Kharn")).toBeInTheDocument(); + // "Your file" appears twice (the corner badge, plus the source line) - both are correct, + // this test is only checking the source line never fabricates "[0 DPI]". + expect(screen.getAllByText("Your file").length).toBe(2); + expect(screen.queryByText(/DPI/)).not.toBeInTheDocument(); + }); + + it("clicking the image does not open the detailed-view modal (no tags/consensus surfaces)", () => { + const store = renderOrphanCard(); + const image = screen.getByAltText("Kharn"); + fireEvent.click(image); + expect(store.getState().modals.shownModal).not.toBe("cardDetailedView"); + }); + + it("a single failed image fetch reaches the 'Image unavailable' placeholder, not a stuck spinner", () => { + // Regression test for the bucket-validity-aware onError fix in useImageSrc: an orphan has + // no bucket URL configured, so it's already loading its one-and-only direct-from-Google URL + // from the very first render (just still internally labelled "loading-from-bucket", this + // hook's fixed initial state) - a naive onError would relabel to "loading-from-fallback" and + // re-render with the SAME src string, which browsers don't re-fetch, leaving the image + // broken forever with the spinner never resolving. + renderOrphanCard(); + const image = screen.getByAltText("Kharn"); + fireEvent.error(image); + expect( + screen.getByTestId("card-image-error-placeholder") + ).toBeInTheDocument(); + expect(screen.getByText("Image unavailable")).toBeInTheDocument(); + }); +}); + describe("Card keyboard activation", () => { it("is focusable and has role=button when cardOnClick is provided", () => { renderCard(() => undefined); diff --git a/frontend/src/features/card/Card.tsx b/frontend/src/features/card/Card.tsx index d138eba13..01316d9a2 100644 --- a/frontend/src/features/card/Card.tsx +++ b/frontend/src/features/card/Card.tsx @@ -143,6 +143,36 @@ const MatchIndicatorIcon = styled(Icon)` -webkit-text-stroke: 2px black; `; +// Foreign-order resilience Phase 1 (issue #324) - "visually distinct treatment" for an orphan +// (a card addressed only by a Drive file ID this catalog has never indexed). Top corner, clear +// of the existing bottom-corner favorite-heart/printing-match icons. Text comes straight from +// the synthesized CardDocument's own sourceName ("Your file" on this - the author/editor - +// surface; a later phase's shared-deck recipient view can synthesize "Shared file" instead +// without this component needing to know the difference). +const OrphanBadge = styled.span` + // Bootstrap's .ratio > * rule (the aspect-ratio wrapper every card image sits inside) + // stretches EVERY direct child to width:100%/height:100%/top:0/left:0 by default - width/ + // height must be overridden back to auto here (same defensive pattern CardIcon/ + // MatchIndicatorIcon below already use), or this badge silently stretches to cover the whole + // card instead of sitting in its corner (caught only by a real browser render - Jest/jsdom + // doesn't compute layout, so this class of bug is invisible to unit tests). + position: absolute; + top: 8px; + left: 8px; + width: auto; + height: auto; + z-index: 2; + font-size: 0.65rem; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.03em; + padding: 2px 6px; + border-radius: 4px; + background: rgba(13, 110, 253, 0.85); + color: #fff; + pointer-events: none; +`; + type ImageState = | "loading-from-bucket" | "loading-from-fallback" @@ -220,14 +250,36 @@ export const useImageSrc = ( setImageState("loaded-from-local-file"); } }; + + // attempt to load directly from bucket first + const thumbnailBucketURL = getBucketImageURL( + cardDocument, + small ? "small" : "large" + ); + const imageBucketURLValid = thumbnailBucketURL !== undefined; + const loadFromBucket = + imageBucketURLValid && + (imageState === "loading-from-bucket" || + imageState === "loaded-from-bucket"); + const onError: React.ReactEventHandler = (img) => { img.preventDefault(); img.currentTarget.onerror = null; - setImageState((value) => - value === "loading-from-bucket" || value === "loaded-from-bucket" - ? "loading-from-fallback" - : "errored" - ); + setImageState((value) => { + // The "bucket" tier only really exists as a distinct retry step when there's an actual + // second URL to fall back to (imageBucketURLValid) - a card with no bucket configured + // (orphans always; also AWS S3/dev-without-bucket-configured cards) is ALREADY loading + // its one and only "fallback" URL from the very first render, just still labelled + // "loading-from-bucket" because that's this hook's fixed initial state. Without this + // check, a failure here would relabel to "loading-from-fallback" and re-render with the + // exact same src string, which browsers don't re-fetch - the image would stay broken + // forever with the spinner never resolving to the "Image unavailable" placeholder. See + // docs/features/foreign-order-resilience.md for the orphan case this was written for. + if (value === "loading-from-bucket" || value === "loaded-from-bucket") { + return imageBucketURLValid ? "loading-from-fallback" : "errored"; + } + return "errored"; + }); }; if (localFileImageSrc !== undefined) { @@ -241,17 +293,6 @@ export const useImageSrc = ( }; } - // attempt to load directly from bucket first - const thumbnailBucketURL = getBucketImageURL( - cardDocument, - small ? "small" : "large" - ); - const imageBucketURLValid = thumbnailBucketURL !== undefined; - const loadFromBucket = - imageBucketURLValid && - (imageState === "loading-from-bucket" || - imageState === "loaded-from-bucket"); - // if image is unavailable in bucket, fall back on loading from worker if possible const imageWorkerURL = getWorkerImageURL( cardDocument, @@ -305,8 +346,12 @@ function CardImage({ priority = false, }: CardImageProps) { const dispatch = useAppDispatch(); + // Foreign-order resilience Phase 1 (issue #324): orphans get no version picker, tags, or + // consensus surfaces - the detailed-view modal exposes exactly those (printing tags, + // reporting), so clicking an orphan's image is a no-op rather than opening it. + const canShowDetailedView = showDetailedViewOnClick && !cardDocument.isOrphan; const handleShowDetailedView = () => { - if (showDetailedViewOnClick) { + if (canShowDetailedView) { dispatch(showCardDetailedViewModal({ card: cardDocument })); } }; @@ -371,6 +416,11 @@ function CardImage({ onErrorCapture={onError} alt={imageAlt} fill={true} + // Orphan images are fetched direct from Google, never our own CDN - owner ruling + // (2026-07-22 security review) accepts the fetch itself remaining a signal to the + // file's owner on author-only surfaces, but still asks for no-referrer so we're not + // additionally leaking this site's own URL to Google on every request. + referrerPolicy={cardDocument.isOrphan ? "no-referrer" : undefined} /> ) : ( <> @@ -381,6 +431,11 @@ function CardImage({ ) : ( <> + {cardDocument.isOrphan && ( + + {cardDocument.sourceName} + + )} {isFavorite && small && ( )} @@ -405,13 +460,16 @@ function CardImage({ loading={priority ? undefined : "lazy"} priority={priority} imageIsLoading={imageIsLoading} - showDetailedViewOnClick={showDetailedViewOnClick} + showDetailedViewOnClick={canShowDetailedView} src={imageSrc} onLoad={onLoad} onErrorCapture={onError} onClick={handleShowDetailedView} alt={imageAlt} fill={true} + referrerPolicy={ + cardDocument.isOrphan ? "no-referrer" : undefined + } /> )} @@ -609,7 +667,12 @@ export function Card({
{maybeCardDocument != null && - `${maybeCardDocument.sourceName} [${maybeCardDocument.dpi} DPI]`} + // An orphan's dpi/sourceName are placeholders (issue #324's synthesized + // CardDocument, never fetched from anywhere) - showing "[0 DPI]" would read as + // real catalog data we don't actually have. + (maybeCardDocument.isOrphan + ? maybeCardDocument.sourceName + : `${maybeCardDocument.sourceName} [${maybeCardDocument.dpi} DPI]`)} {maybeCardDocument == null && searchQuery != undefined && "Your search query"} diff --git a/frontend/src/features/card/DeckbuilderConfirmAffordance.tsx b/frontend/src/features/card/DeckbuilderConfirmAffordance.tsx index 18e840d82..6dd9b6cd4 100644 --- a/frontend/src/features/card/DeckbuilderConfirmAffordance.tsx +++ b/frontend/src/features/card/DeckbuilderConfirmAffordance.tsx @@ -138,6 +138,10 @@ export function DeckbuilderConfirmAffordance({ const [submitting, setSubmitting] = useState(false); const isUnconfirmedCanonicalImport = + // Foreign-order resilience Phase 1 (issue #324): orphans get no consensus surfaces - this + // affordance exists to collect printing-tag votes against a real, indexed card, which an + // orphan (by definition) isn't. + card?.isOrphan !== true && searchQuery?.expansionCode != null && getPrintingMatchLabel( searchQuery, diff --git a/frontend/src/features/download/downloadXML.test.ts b/frontend/src/features/download/downloadXML.test.ts index 9e3db2803..a29e7dcbe 100644 --- a/frontend/src/features/download/downloadXML.test.ts +++ b/frontend/src/features/download/downloadXML.test.ts @@ -1,3 +1,4 @@ +import { synthesizeOrphanCardDocument } from "@/common/orphanCard"; import { CardType } from "@/common/schema_types"; import { cardDocument1, cardDocument12 } from "@/common/test-constants"; import { SlotProjectMembers } from "@/common/types"; @@ -94,4 +95,27 @@ describe("generateXML", () => { ); expect(card?.querySelector("query")).not.toBeNull(); }); + + // Foreign-order resilience Phase 1 (issue #324) - the round-trip requirement: an orphan + // (a synthesized CardDocument for a Drive file ID the catalog never indexed) must still + // export with its raw preserved, since createCardElement previously returned null + // entirely for any identifier missing from cardDocuments. + it("re-exports an orphan's raw , unchanged", () => { + const orphanId = "1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn"; + const orphanCard = synthesizeOrphanCardDocument(orphanId, { + name: "Kharn", + cardType: CardType.Card, + }); + const xml = generateXML( + [buildSlot("slot-1", orphanId)], + { [orphanId]: orphanCard }, + null, + 1, + finishSettings + ); + const doc = new DOMParser().parseFromString(xml, "text/xml"); + const card = doc.querySelector("fronts > card"); + expect(card?.querySelector("id")?.textContent).toBe(orphanId); + expect(card?.querySelector("query")?.textContent).toBe("Kharn"); + }); }); diff --git a/frontend/src/features/import/ImportText.tsx b/frontend/src/features/import/ImportText.tsx index ba819906f..52d9c8451 100644 --- a/frontend/src/features/import/ImportText.tsx +++ b/frontend/src/features/import/ImportText.tsx @@ -185,6 +185,14 @@ export function ImportText({ . +
  • + Alternatively, pin an exact Google Drive image by putting{" "} + [mpc:image ID] anywhere after your search query — + for example, your card name [mpc:image ID]. This + works even if we haven't indexed that image yet (e.g. an + order built against another {ProjectName}-style site) — + it'll render using the image ID directly. +
  • You may specify queries for both the front and the back by separating them with {FaceSeparator} — for example,{" "} diff --git a/frontend/src/features/import/ImportXML.test.ts b/frontend/src/features/import/ImportXML.test.ts index 3837523c2..a39bb5691 100644 --- a/frontend/src/features/import/ImportXML.test.ts +++ b/frontend/src/features/import/ImportXML.test.ts @@ -97,3 +97,68 @@ describe("parseXmlImport", () => { expect(foil).toBe(false); }); }); + +// Foreign-order resilience Phase 1 (issue #324) - the reported symptom: an order XML +// referencing a Drive file ID this catalog never indexed. parseXmlImport already read raw +// / text verbatim before this feature landed - what's under test here is that this +// stays true even for an ID with no catalog match, INCLUDING the exact "b:null" back-face shape +// from the owner's screenshot (an empty element on the back, i.e. no name text at all). +// The actual fix (not clearing/invalidating an unresolved-but-plausible identifier) lives in +// listenerMiddleware.ts and cardDocumentsSlice.ts - this only covers the parser's own half. +describe("foreign-order resilience Phase 1 - unindexed identifiers (issue #324)", () => { + const orphanId = "1LrVX0pUcye9n_0RtaDNVl2xPrQgn7CYf"; + + it("reads a front the catalog doesn't know about, unchanged", () => { + const xml = ` + +
    + 1 + (S30) Standard Smooth + false +
    + + + ${orphanId} + google_drive + 0 + Kharn.png + kharn + + + +    `; + const { members } = parseXmlImport(xml, 0, null, true); + expect(members[0].front?.selectedImage).toBe(orphanId); + expect(members[0].front?.query.query).toBe("kharn"); + }); + + it("reads an unindexed as the implicit back face (the reported b:null case), unchanged", () => { + // The owner's screenshot showed an Invalid Cards row "Back | b:null | " - + // stringifySearchQuery renders "b:null" specifically when query.query is the literal JS + // `null`, which only happens via THIS path: a front slot with no matching entry + // falls back to the order's own root-level element (parseXmlImport's own + // `newMembers[slot].back == null` branch), carrying `query: { query: null, cardType: + // Cardback }` - not an empty/blank query string. + const xml = ` + +
    + 1 + (S30) Standard Smooth + false +
    + + + abc123 + google_drive + 0 + Kharn.png + kharn + + + ${orphanId} +    `; + const { members } = parseXmlImport(xml, 0, null, true); + expect(members[0].back?.selectedImage).toBe(orphanId); + expect(members[0].back?.query.query).toBeNull(); + }); +}); diff --git a/frontend/src/features/pdf/pdfImage.ts b/frontend/src/features/pdf/pdfImage.ts index bbd8e5e21..a612fae7a 100644 --- a/frontend/src/features/pdf/pdfImage.ts +++ b/frontend/src/features/pdf/pdfImage.ts @@ -1,4 +1,5 @@ import { getBucketImageURL, getWorkerImageURL } from "@/common/image"; +import { getOrphanFullResolutionImageURL } from "@/common/orphanCard"; import { SourceType } from "@/common/schema_types"; import { Semaphore } from "@/common/semaphore"; import { CardDocument } from "@/common/types"; @@ -170,6 +171,52 @@ const getThumbnailURL = async ( return URL.createObjectURL(await fetchAsBlob(workerURL)); }; +/** + * Foreign-order resilience Phase 1 (issue #324): an orphan's PDF-export path, always direct + * from Google - never through our image-CDN Worker (see common/orphanCard.ts's module doc). + * Thumbnail tiers reuse the small/large URLs already computed at synthesis time (the same + * 400px/800px height params the Worker itself uses); "full-resolution" fetches the original, + * unresized file, paced by the same semaphore/retry logic as the catalog full-resolution path + * (owner ruling: "PDF export: original-resolution fetch... an exported PDF containing an + * orphan embeds the high-res image; the editor grid never requests it" - satisfied by this + * function only ever running when a PDF is actually being generated). + */ +const getOrphanPDFImageURL = async ( + cardDocument: CardDocument, + imageQuality: PDFImageQuality +): Promise => { + switch (imageQuality) { + case "small-thumbnail": + case "large-thumbnail": { + const thumbnailURL = + imageQuality === "small-thumbnail" + ? cardDocument.smallThumbnailUrl + : cardDocument.mediumThumbnailUrl; + if (thumbnailURL === undefined) { + throw new Error( + `no orphan image source configured for card ${cardDocument.identifier}` + ); + } + return URL.createObjectURL(await fetchAsBlob(thumbnailURL)); + } + case "full-resolution": { + const fullResolutionURL = getOrphanFullResolutionImageURL( + cardDocument.identifier + ); + if (fullResolutionURL === undefined) { + throw new Error( + `no orphan image source configured for card ${cardDocument.identifier}` + ); + } + return URL.createObjectURL( + await fetchFullResolutionImageAsBlob(fullResolutionURL) + ); + } + default: + throw new Error(`invalid imageQuality ${imageQuality}`); + } +}; + /** * Resolve the image source for a card in a PDF, honouring the requested quality * tier and the card's source (Google Drive image worker, or a local file). @@ -185,6 +232,9 @@ export const getPDFImageURL = async ( jpgQuality: number, fileHandles: { [identifier: string]: FileSystemFileHandle } ): Promise => { + if (cardDocument.isOrphan) { + return getOrphanPDFImageURL(cardDocument, imageQuality); + } switch (cardDocument.sourceType) { case SourceType.GoogleDrive: switch (imageQuality) { @@ -243,6 +293,19 @@ export const getPDFImageBlob = async ( jpgQuality: number, fileHandles: { [identifier: string]: FileSystemFileHandle } ): Promise => { + if (cardDocument.isOrphan) { + // Foreign-order resilience Phase 1 (issue #324) - see getOrphanPDFImageURL's own comment + // above for why this is always direct-from-Google, full resolution, never our Worker. + const fullResolutionURL = getOrphanFullResolutionImageURL( + cardDocument.identifier + ); + if (fullResolutionURL === undefined) { + throw new Error( + `no orphan image source configured for card ${cardDocument.identifier}` + ); + } + return fetchFullResolutionImageAsBlob(fullResolutionURL); + } switch (cardDocument.sourceType) { case SourceType.GoogleDrive: { const workerURL = getWorkerImageURL( diff --git a/frontend/src/store/listenerMiddleware.test.ts b/frontend/src/store/listenerMiddleware.test.ts new file mode 100644 index 000000000..ada708df5 --- /dev/null +++ b/frontend/src/store/listenerMiddleware.test.ts @@ -0,0 +1,181 @@ +/** + * Foreign-order resilience Phase 1 (issue #324): the invalid-identifier listener in + * listenerMiddleware.ts must NOT clear/invalidate a project member's selectedImage just + * because it's absent from the catalog's own search results, when that identifier is (or might + * still turn out to be) an orphan - a Drive file ID this catalog has never indexed. This is the + * root cause of both symptoms from the owner's 2026-07-23 high-priority promotion: the text + * import token not registering, and the XML "Back | b:null | " Invalid Cards row - in both + * cases this listener was unconditionally clearing the selectedImage the moment it noticed the + * catalog's search didn't back it up, before this feature. + */ + +import { Card as CardTypeConst } from "@/common/constants"; +import { synthesizeOrphanCardDocument } from "@/common/orphanCard"; +import { computeSearchQueryHashKey } from "@/common/processing"; +import { CardType, PrintingTagStatus } from "@/common/schema_types"; +import { SlotProjectMembers } from "@/common/types"; +import { fetchCardDocuments } from "@/store/slices/cardDocumentsSlice"; +import { selectInvalidIdentifiers } from "@/store/slices/invalidIdentifiersSlice"; +import { fetchSearchResults } from "@/store/slices/searchResultsSlice"; +import { RootState, setupStore } from "@/store/store"; + +const driveId = "1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn"; +const query = "kharn"; +const hashKey = computeSearchQueryHashKey({ + query, + cardType: CardTypeConst, +}); + +function buildOneFrontMemberState( + selectedImage: string, + cardDocuments?: RootState["cardDocuments"]["cardDocuments"] +): Partial { + const members: Array = [ + { + id: "slot-1", + front: { + query: { query, cardType: CardTypeConst }, + selectedImage, + selected: false, + }, + back: null, + }, + ]; + return { + project: { + members, + nextMemberId: 1, + cardback: null, + mostRecentlySelectedSlot: null, + manualOverrides: {}, + }, + cardDocuments: { + cardDocuments: cardDocuments ?? {}, + status: "idle", + error: null, + }, + }; +} + +describe("foreign-order resilience Phase 1 - invalid-identifier listener (issue #324)", () => { + it("keeps a Drive-ID-shaped selectedImage selected when it's not yet resolved either way (early pass)", () => { + const store = setupStore(buildOneFrontMemberState(driveId)); + + store.dispatch( + fetchSearchResults.fulfilled( + { results: { [hashKey]: [] }, degradedQueryHashKeys: [] }, + "request-1", + undefined + ) + ); + + expect(store.getState().project.members[0].front?.selectedImage).toBe( + driveId + ); + expect(selectInvalidIdentifiers(store.getState())).toEqual([]); + }); + + it("keeps a selectedImage selected once it's resolved to a synthesized orphan CardDocument", () => { + const orphanDoc = synthesizeOrphanCardDocument(driveId, { + name: query, + cardType: CardType.Card, + }); + const store = setupStore(buildOneFrontMemberState(driveId)); + + // Populate search results too (empty, i.e. the catalog's search genuinely doesn't back + // this identifier up) so the listener's per-member block actually runs, same as it would + // once fetchSearchResults itself has resolved in a real load cycle. + store.dispatch( + fetchSearchResults.fulfilled( + { results: { [hashKey]: [] }, degradedQueryHashKeys: [] }, + "request-2a", + undefined + ) + ); + store.dispatch( + fetchCardDocuments.fulfilled( + { [driveId]: orphanDoc }, + "request-2b", + undefined + ) + ); + + expect(store.getState().project.members[0].front?.selectedImage).toBe( + driveId + ); + expect( + store.getState().cardDocuments.cardDocuments[driveId]?.isOrphan + ).toBe(true); + expect(selectInvalidIdentifiers(store.getState())).toEqual([]); + }); + + it("still clears and records a genuinely invalid (non-Drive-ID-shaped) identifier", () => { + const store = setupStore(buildOneFrontMemberState("too-short")); + + store.dispatch( + fetchSearchResults.fulfilled( + { + results: { [hashKey]: ["a-different-real-result-id"] }, + degradedQueryHashKeys: [], + }, + "request-3", + undefined + ) + ); + + // Not preserved as-is: an invalidated selection gets deselected and, since real search + // results exist for this query, replaced with the first of them - same pre-existing + // behaviour as before this feature, unaffected for a genuinely invalid identifier. + expect(store.getState().project.members[0].front?.selectedImage).toBe( + "a-different-real-result-id" + ); + expect(selectInvalidIdentifiers(store.getState())[0]?.front?.[1]).toBe( + "too-short" + ); + }); + + it("still clears and records a known catalog card that's since been filtered out of search results (regression guard)", () => { + const knownCard = { + cardType: CardType.Card, + dateCreated: "", + dateModified: "", + dpi: 100, + extension: "png", + identifier: driveId, + language: "EN", + mediumThumbnailUrl: "", + name: "Kharn", + printingTagStatus: PrintingTagStatus.Unresolved, + priority: 0, + searchq: query, + size: 0, + smallThumbnailUrl: "", + source: "some-source", + sourceId: 1, + sourceName: "Some Source", + sourceVerbose: "Some Source", + tags: [], + }; + const store = setupStore( + buildOneFrontMemberState(driveId, { [driveId]: knownCard }) + ); + + store.dispatch( + fetchSearchResults.fulfilled( + { + results: { [hashKey]: ["a-different-real-result-id"] }, + degradedQueryHashKeys: [], + }, + "request-4", + undefined + ) + ); + + expect(store.getState().project.members[0].front?.selectedImage).toBe( + "a-different-real-result-id" + ); + expect(selectInvalidIdentifiers(store.getState())[0]?.front?.[1]).toBe( + driveId + ); + }); +}); diff --git a/frontend/src/store/listenerMiddleware.ts b/frontend/src/store/listenerMiddleware.ts index c2133ac50..199a23144 100644 --- a/frontend/src/store/listenerMiddleware.ts +++ b/frontend/src/store/listenerMiddleware.ts @@ -10,6 +10,7 @@ import { setLocalStorageFavorites, setLocalStorageManualOverrides, } from "@/common/cookies"; +import { isLikelyDriveFileId } from "@/common/orphanCard"; import { computeSearchQueryHashKey } from "@/common/processing"; import { Faces } from "@/common/types"; import { api } from "@/store/api"; @@ -23,7 +24,11 @@ import { fetchCardbacksAndReportError, selectCardbacks, } from "@/store/slices/cardbackSlice"; -import { fetchCardDocumentsAndReportError } from "@/store/slices/cardDocumentsSlice"; +import { + fetchCardDocuments, + fetchCardDocumentsAndReportError, + selectCardDocumentByIdentifier, +} from "@/store/slices/cardDocumentsSlice"; import { clearFavoriteRenders, removeFavoriteRender, @@ -271,7 +276,17 @@ startAppListening({ }); startAppListening({ - matcher: isAnyOf(fetchSearchResults.fulfilled, fetchCardbacks.fulfilled), + // Foreign-order resilience Phase 1 (issue #324) added fetchCardDocuments.fulfilled to this + // matcher: this listener needs to know whether an unmatched selectedImage is a genuinely + // invalid identifier or an orphan (a Drive file ID the catalog's search doesn't back up but + // hasn't necessarily failed either), and that isn't settled until + // cardDocuments.cardDocuments actually reflects it - see the isOrphanCandidate comment below + // for the full ordering rationale. The original two triggers are unchanged. + matcher: isAnyOf( + fetchSearchResults.fulfilled, + fetchCardbacks.fulfilled, + fetchCardDocuments.fulfilled + ), /** * Whenever search results change, this listener will inspect each card slot * and ensure that their selected images are valid. @@ -297,22 +312,44 @@ startAppListening({ if (searchResultsForQueryOrDefault != null) { let mutatedSelectedImage = projectMember.selectedImage; - // If an image is selected and it's not in the search results, deselect the image and let the user know about it + // If an image is selected and it's not in the search results, deselect the image and let the user know about it - + // UNLESS it's a foreign-order-resilience orphan (issue #324), which is neither invalid nor cleared. if ( mutatedSelectedImage != null && !searchResultsForQueryOrDefault.includes(mutatedSelectedImage) ) { - if (searchResultsForQueryOrDefault.length > 0) { - dispatch( - recordInvalidIdentifier({ - slot, - face, - searchQuery, - identifier: mutatedSelectedImage, - }) - ); + const knownCardDocument = selectCardDocumentByIdentifier( + state, + mutatedSelectedImage + ); + // An identifier the search doesn't back up isn't automatically invalid: if it's + // already resolved to a synthesized orphan CardDocument, or hasn't been resolved + // either way yet but still looks like a real Drive file ID, treat it as an + // orphan candidate and leave it selected. `knownCardDocument == null` covers the + // early passes of this listener (fired by fetchSearchResults.fulfilled/ + // fetchCardbacks.fulfilled, both of which can run before cardDocuments knows + // anything) - erring towards NOT clearing yet is safe, because this same listener + // re-runs once fetchCardDocuments.fulfilled settles cardDocuments definitively, + // at which point a genuinely-known-but-filtered/removed catalog card ( + // knownCardDocument defined, isOrphan false/undefined) still falls through to the + // existing Invalid Cards flow below exactly as before. + const isOrphanCandidate = + knownCardDocument?.isOrphan === true || + (knownCardDocument == null && + isLikelyDriveFileId(mutatedSelectedImage)); + if (!isOrphanCandidate) { + if (searchResultsForQueryOrDefault.length > 0) { + dispatch( + recordInvalidIdentifier({ + slot, + face, + searchQuery, + identifier: mutatedSelectedImage, + }) + ); + } + mutatedSelectedImage = undefined; } - mutatedSelectedImage = undefined; } // If no image is selected and there are search results, select the first favorited image or the first image diff --git a/frontend/src/store/slices/cardDocumentsSlice.ts b/frontend/src/store/slices/cardDocumentsSlice.ts index 69bf364d5..8f7fb5e39 100644 --- a/frontend/src/store/slices/cardDocumentsSlice.ts +++ b/frontend/src/store/slices/cardDocumentsSlice.ts @@ -4,12 +4,15 @@ import { createSelector } from "@reduxjs/toolkit"; -import { CardEndpointPageSize } from "@/common/constants"; +import { Back, CardEndpointPageSize, Front } from "@/common/constants"; +import { buildOrphanCardDocuments } from "@/common/orphanCard"; +import { CardType } from "@/common/schema_types"; import { CardDocument, CardDocumentsState, createAppAsyncThunk, createAppSlice, + Faces, OramaCardDocument, useAppSelector, } from "@/common/types"; @@ -25,6 +28,37 @@ import { fetchSearchResultsAndReportError } from "@/store/slices/searchResultsSl import { setNotification } from "@/store/slices/toastsSlice"; import { AppDispatch, RootState } from "@/store/store"; +/** + * For every project member's selectedImage, remember the query text/cardType that slot + * actually asked for - foreign-order resilience Phase 1 (issue #324) needs this so an orphan's + * synthesized CardDocument (see buildOrphanCardDocuments below) can carry the user's own stand-in + * name and the correct front/back-consistent card type, rather than a generic placeholder. First + * slot to reference a given identifier wins if more than one slot happens to share it under + * different query text - an edge case, not the common path. + */ +const buildStandInQueryByIdentifier = ( + projectMembers: RootState["project"]["members"] +): Map => { + const standInQueryByIdentifier = new Map< + string, + { name: string | null; cardType: CardType } + >(); + for (const member of projectMembers) { + for (const face of [Front, Back] as Array) { + const projectMemberAtFace = member[face]; + const identifier = projectMemberAtFace?.selectedImage; + const query = projectMemberAtFace?.query; + if (identifier != null && !standInQueryByIdentifier.has(identifier)) { + standInQueryByIdentifier.set(identifier, { + name: query?.query ?? null, + cardType: query?.cardType ?? CardType.Card, + }); + } + } + } + return standInQueryByIdentifier; +}; + //# region async thunk const typePrefix = "cardDocuments/fetchCardDocuments"; @@ -59,7 +93,14 @@ export const getCardDocumentRequestPromiseChain = async ( } }; -const fetchCardDocuments = createAppAsyncThunk( +// Exported (not just via fetchCardDocumentsAndReportError below) so listenerMiddleware.ts can +// match on fetchCardDocuments.fulfilled directly - foreign-order resilience Phase 1 (issue +// #324) needs its own invalid-identifier listener to re-run once cardDocuments.cardDocuments +// actually reflects whether an identifier is a real catalog card, an orphan, or neither, which +// isn't settled yet by the time fetchSearchResults.fulfilled/fetchCardbacks.fulfilled (the +// listener's original triggers) fire - see that listener's own comment for the full ordering +// rationale. +export const fetchCardDocuments = createAppAsyncThunk( typePrefix, /** * This function queries card documents (entire database rows) from the backend. It only queries cards which have @@ -79,7 +120,15 @@ const fetchCardDocuments = createAppAsyncThunk( const state = getState() as RootState; - const allIdentifiers = selectUniqueCardIdentifiers(state); + // Union of the search-derived identifier set (the pre-existing source of truth) with every + // raw selectedImage the project actually references, including ones the fork of the site's + // search index never matched (see this thunk's own foreign-order-resilience comment below). + // selectUniqueCardIdentifiers alone would never even attempt to resolve an orphan's + // identifier, since by definition it never appears in any search result. + const allIdentifiers = new Set([ + ...selectUniqueCardIdentifiers(state), + ...selectProjectMemberIdentifiers(state), + ]); const identifiersWithKnownData = new Set( Object.keys(state.cardDocuments.cardDocuments) ); @@ -99,7 +148,28 @@ const fetchCardDocuments = createAppAsyncThunk( ? getCardDocumentRequestPromiseChain(identifiersToSearch, backendURL) : new Promise(async (resolve) => resolve({})); return await Promise.all([localResultsPromise, remoteResultsPromise]).then( - ([localResults, remoteResults]) => ({ ...remoteResults, ...localResults }) + ([localResults, remoteResults]) => { + const resolvedDocuments = { ...remoteResults, ...localResults }; + // Foreign-order resilience Phase 1 (issue #324): anything still unresolved after both + // lookups, that also looks like a real Drive file ID, gets a synthesized orphan + // CardDocument instead of being left out entirely - see common/orphanCard.ts's module + // doc for the full rationale. Genuinely invalid identifiers (garbage, or a real + // catalog ID whose source got disabled/removed) are unaffected - buildOrphanCardDocuments + // only emits entries for identifiers that pass isLikelyDriveFileId, so anything else + // stays exactly as absent as it always was, and listenerMiddleware.ts's existing + // Invalid Cards flow still catches it. + const stillUnresolved = identifiersToSearch.filter( + (identifier) => resolvedDocuments[identifier] == null + ); + const orphanDocuments = + stillUnresolved.length > 0 + ? buildOrphanCardDocuments( + stillUnresolved, + buildStandInQueryByIdentifier(state.project.members) + ) + : {}; + return { ...orphanDocuments, ...resolvedDocuments }; + } ); } ); diff --git a/frontend/tests/OrphanRendering.spec.ts b/frontend/tests/OrphanRendering.spec.ts new file mode 100644 index 000000000..a5022a1d6 --- /dev/null +++ b/frontend/tests/OrphanRendering.spec.ts @@ -0,0 +1,123 @@ +/** + * Foreign-order resilience Phase 1 (issue #324) - end-to-end coverage for the owner's own + * 2026-07-23 high-priority repro: importing a reference to a Drive file ID this catalog has + * never indexed must render an orphan tile (badge, image, "find this card" search still live), + * not silently drop the selection or land it in the Invalid Cards modal. + */ + +import { expect } from "@playwright/test"; + +import { defaultHandlers } from "@/mocks/handlers"; + +import { test } from "../playwright.setup"; +import { + expectCardSlotToExist, + importText, + importXML, + loadPageWithDefaultBackend, + toggleFace, +} from "./test-utils"; + +// A syntactically real-looking Drive file ID this catalog's mocked backend never returns from +// /2/cards/ or /3/editorSearch/ (see defaultHandlers - cardDocumentsNoResults/ +// searchResultsNoResults) - exercises the genuinely-unresolved-forever path, not a race with a +// mock that happens to resolve it eventually. +const orphanId = "1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn"; +const orphanBackId = "1LrVX0pUcye9n_0RtaDNVl2xPrQgn7CYf"; + +test.describe("orphan rendering (issue #324)", () => { + test("the owner's exact reported text-import line registers and renders an orphan tile", async ({ + page, + network, + }) => { + network.use(...defaultHandlers); + await loadPageWithDefaultBackend(page); + + await importText(page, `1x Kharn [mpc:${orphanId}]`); + + await expectCardSlotToExist(page, 1); + const frontSlot = page.getByTestId("front-slot0"); + // The stand-in name comes from the parsed SearchQuery, which - same as every other text + // import - lowercases the query text (processQuery); "Kharn" the user typed, "kharn" here + // is expected, not a bug. + await expect(frontSlot).toContainText("kharn"); + await expect(frontSlot.getByTestId("orphan-badge")).toBeVisible(); + await expect(frontSlot.getByTestId("orphan-badge")).toHaveText("Your file"); + + // Never lands in the Invalid Cards flow - the whole point of this feature. + await expect(page.getByText("Review Invalid Cards")).not.toBeVisible(); + + // Wait for the real direct-from-Google fetch to actually resolve before screenshotting - + // otherwise the shot just captures the spinner mid-flight. The extra fixed wait lets the + // image's own 0.3s CSS opacity fade-in (card-img-fade-in) finish too, purely cosmetic. + await expect(frontSlot.getByRole("status")).not.toBeVisible({ + timeout: 15_000, + }); + await page.waitForTimeout(500); + await page.screenshot({ + path: "test-results/orphan-text-import-desktop.png", + }); + }); + + test("an XML order referencing an unindexed front id and an unindexed implicit cardback (the reported b:null case) renders both as orphans", async ({ + page, + network, + }) => { + network.use(...defaultHandlers); + await loadPageWithDefaultBackend(page); + + await importXML( + page, + ` +
    + 1 + (S30) Standard Smooth + false +
    + + + ${orphanId} + google_drive + 0 + Kharn.png + kharn + + + ${orphanBackId} +     ` + ); + + await expectCardSlotToExist(page, 1); + const frontSlot = page.getByTestId("front-slot0"); + await expect(frontSlot.getByTestId("orphan-badge")).toBeVisible(); + + // No element covers this slot, so parseXmlImport falls back to the order's own + // root-level - the exact "b:null" mechanism from the owner's screenshot (see + // ImportXML.test.ts's own comment on this). That fallback is per-slot (back-slot0's own + // selectedImage) - it's a SEPARATE concept from the shared "Common Cardback" panel (which + // only reflects state.project.cardback, auto-selected from the indexed cardbacks list, and + // stays empty here since the mocked backend has none) - so the back-slot0 tile is what's + // under test, not Common Cardback. The small viewport this suite runs at collapses the + // editor to a front/back toggle, so it must be switched to before back-slot0 is visible. + await toggleFace(page); + const backSlot = page.getByTestId("back-slot0"); + await expect(backSlot.getByTestId("orphan-badge")).toBeVisible(); + await expect(backSlot.getByRole("status")).not.toBeVisible({ + timeout: 15_000, + }); + await page.waitForTimeout(500); + await page.screenshot({ + path: "test-results/orphan-xml-import-backs-desktop.png", + }); + await toggleFace(page); + + await expect(page.getByText("Review Invalid Cards")).not.toBeVisible(); + await expect(frontSlot.getByRole("status")).not.toBeVisible({ + timeout: 15_000, + }); + await page.waitForTimeout(500); + await page.screenshot({ + path: "test-results/orphan-xml-import-desktop.png", + }); + }); +}); From e6eb051204fbe3bd8ff809c9b8ca946bddca38c7 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 23 Jul 2026 19:14:56 +0000 Subject: [PATCH 2/2] Fix orphan cardback rendering and re-point orphan e2e coverage to /display Common Cardback panel showed "Card not found" for an orphan XML cardback (ImportXML.tsx now seeds state.project.cardback when unset; listenerMiddleware.ts's cardback listener gets the same orphan carve-out the per-slot listener already had). OrphanRendering.spec.ts now verifies rendering on the unified /display page per owner ruling, not the legacy /editor route. Co-Authored-By: Claude Fable 5 --- docs/features/foreign-order-resilience.md | 79 +++++++- .../src/features/import/ImportXML.test.ts | 34 ++++ frontend/src/features/import/ImportXML.tsx | 42 +++- frontend/src/store/listenerMiddleware.test.ts | 117 +++++++++++ frontend/src/store/listenerMiddleware.ts | 27 ++- frontend/tests/ImportXML.spec.ts | 49 +++++ frontend/tests/OrphanRendering.spec.ts | 182 +++++++++++------- 7 files changed, 446 insertions(+), 84 deletions(-) diff --git a/docs/features/foreign-order-resilience.md b/docs/features/foreign-order-resilience.md index aba9d201a..91219f6f5 100644 --- a/docs/features/foreign-order-resilience.md +++ b/docs/features/foreign-order-resilience.md @@ -121,6 +121,64 @@ regression-guarded addition, not a loosening of that existing behaviour sources, or local dev with `NEXT_PUBLIC_IMAGE_BUCKET_URL` unset) — not orphan-specific, just the same code path. +## Rendering surfaces & acceptance (2026-07-23 owner review round) + +- **Acceptance surface**: the classic `/editor` page is a legacy route held + behind the route-swap PR #389 — owner ruling, this review round: the + UNIFIED `/display` page (nav "Editor") is the only acceptance surface for + frontend rendering work. `OrphanRendering.spec.ts`'s two Playwright cases + (originally verified against `/editor`) were moved to run against + `/display` instead, screenshotting `test-results/orphan-text-import-desktop.png`/ + `orphan-xml-import-desktop.png`/`orphan-xml-import-backs-desktop.png`. +- **The `/display` sheet needed no code change**: `PagePreview.tsx` (the + unified page's own sheet-cell renderer, a DIFFERENT component from + `Card.tsx`) reads `cardDocument.mediumThumbnailUrl`/`isOrphan` straight off + the same shared `cardDocuments` store slice — since `synthesizeOrphanCardDocument` + already sets both, an orphan's image renders correctly there for free, for + both fronts (text import) and backs (the XML `b:null` case, once the page's + own Fronts/Backs toggle is switched — the "cardback corner" from the + owner's report; see the next bullet for the surface that PagePreview.tsx + is NOT). One gap this surfaces: `PagePreview.tsx` has no `OrphanBadge` + equivalent (`Card.tsx`'s corner label) — a pre-existing, page-scoped visual + gap the sheet already had for every other card attribute it doesn't badge, + not a regression from this pass. +- **REAL BUG, fixed**: the classic editor's "Common Cardback" panel + (`CommonCardback.tsx`'s right-panel mount, `/editor` only — `/display` has + no equivalent persistent tile, only the `CardbackToolbarButton` picker) + showed "Card not found" after importing an order whose own `` + was an orphan, even though the very same identifier rendered correctly one + panel over as the imported slot's own per-slot back. Root cause was two + separate gaps, both now fixed: + 1. `ImportXML.tsx`'s `parseXmlImport` read the file's own root-level + `` into each individual backless front's own per-slot + fallback, but never fed it back to the caller to also initialise + `state.project.cardback` (the Common Cardback panel's own selection) — + so a BRAND NEW project (nothing selected yet) never picked it up at + all. Fixed by returning the raw `` text as `cardback` on + `ParsedXmlImport`, and having `ImportXML`'s `parseXMLFile` dispatch + `setSelectedCardback` with it — but ONLY when `state.project.cardback` + was `null` beforehand (`projectCardback == null`, read from the + component's own pre-import selector snapshot). That gate is load- + bearing: an EXISTING non-null project cardback deliberately stays + untouched by a later import even with "Use XML Cardback" on — + `ImportXML.spec.ts`'s pre-existing "import an XML and use its + cardback"/"...use the project cardback" tests assert exactly this, and + the fix must not (and does not) regress them. + 2. `listenerMiddleware.ts`'s `fetchCardbacks.fulfilled` listener (which + deselects `state.project.cardback` the moment it's absent from the + catalog's own indexed cardbacks list) had no orphan-candidate carve-out + at all, unlike its sibling per-slot invalid-identifier listener above — + so even after fix 1 initialised an orphan cardback, this listener would + immediately clear it right back out. Given the same `isOrphan`/ + `isLikelyDriveFileId` carve-out as the per-slot listener, now also + re-triggered on `fetchCardDocuments.fulfilled` for the same + not-yet-resolved ordering reason. + - Coverage: `listenerMiddleware.test.ts`'s new "project cardback listener" + describe block (both the orphan carve-out and its own regression + guards), `ImportXML.test.ts`'s new `cardback` field cases, and + `ImportXML.spec.ts`'s new "brand new project" Playwright case + (`common-cardback`'s `orphan-badge` visible, no "Card Not Found" text). + ## Round-trip (export/re-import) `downloadXML.ts`'s `createCardElement` already returned `null` (silently @@ -202,7 +260,9 @@ already offered exactly as it always was for any slot. - `frontend/src/store/slices/cardDocumentsSlice.ts` — the broadened identifier union and orphan synthesis in `fetchCardDocuments`. - `frontend/src/store/listenerMiddleware.ts` — the invalid-identifier - listener's orphan-candidate skip. + listener's orphan-candidate skip, and (2026-07-23 follow-up) the SEPARATE + `fetchCardbacks.fulfilled` project-cardback listener's own matching + carve-out. - `frontend/src/features/card/Card.tsx` — `OrphanBadge`, the bucket-validity-aware `onError` fix, click-to-detail suppression. - `frontend/src/features/card/DeckbuilderConfirmAffordance.tsx` — the @@ -212,9 +272,16 @@ already offered exactly as it always was for any slot. - `frontend/src/common/types.ts` — the `isOrphan?: boolean` marker on the frontend's own `Card`/`CardDocument` type (never present in the quicktype-generated `schema_types.ts`). +- `frontend/src/features/import/ImportXML.tsx` — (2026-07-23 follow-up) + `ParsedXmlImport`'s new `cardback` field and `parseXMLFile`'s gated + `setSelectedCardback` dispatch, fixing the Common Cardback panel bug (see + "Rendering surfaces & acceptance" above). - Tests: `orphanCard.test.ts`, `processing.test.ts` (bracket-token cases), - `listenerMiddleware.test.ts`, `ImportXML.test.ts` (front + the b:null - back-face case), `downloadXML.test.ts` (round-trip), `Card.test.tsx` - (badge/click-suppression/error-degrade), and the Playwright - `tests/OrphanRendering.spec.ts` (both reported symptoms, end to end, - with screenshots). + `listenerMiddleware.test.ts` (both the per-slot AND, as of 2026-07-23, the + project-cardback listener), `ImportXML.test.ts` (front + the b:null + back-face case, plus the new `cardback` field cases), `downloadXML.test.ts` + (round-trip), `Card.test.tsx` (badge/click-suppression/error-degrade), + `ImportXML.spec.ts`'s "brand new project" Playwright case (the Common + Cardback fix), and the Playwright `tests/OrphanRendering.spec.ts` (both + reported symptoms, end to end, on the unified `/display` page as of the + 2026-07-23 acceptance-surface correction, with screenshots). diff --git a/frontend/src/features/import/ImportXML.test.ts b/frontend/src/features/import/ImportXML.test.ts index a39bb5691..7e555f380 100644 --- a/frontend/src/features/import/ImportXML.test.ts +++ b/frontend/src/features/import/ImportXML.test.ts @@ -132,6 +132,40 @@ describe("foreign-order resilience Phase 1 - unindexed identifiers (issue #324)" expect(members[0].front?.query.query).toBe("kharn"); }); + it("also surfaces the file's own root text via the return value's `cardback` field (regardless of catalog/orphan status), for the caller to propagate into state.project.cardback", () => { + // Follow-up fix (owner-observed 2026-07-23): parseXmlImport previously only ever fed the + // file's own into each individual backless front's own per-slot fallback - never + // back out to the caller for state.project.cardback itself, which is why the "Common + // Cardback" panel (CommonCardback.tsx) kept showing "Card not found" after an XML import + // instead of the file's actual cardback (orphan or not) - see ImportXML.tsx's own + // parseXMLFile comment for the caller-side half of this fix. + const xml = ` + +
    + 1 + (S30) Standard Smooth + false +
    + + + abc123 + google_drive + 0 + Kharn.png + kharn + + + ${orphanId} +    `; + const { cardback } = parseXmlImport(xml, 0, null, true); + expect(cardback).toBe(orphanId); + }); + + it("`cardback` is undefined when the file has no element (or an empty one) at all", () => { + const { cardback } = parseXmlImport(XML_1_0, 0, null, true); + expect(cardback).toBeUndefined(); + }); + it("reads an unindexed as the implicit back face (the reported b:null case), unchanged", () => { // The owner's screenshot showed an Invalid Cards row "Back | b:null | " - // stringifySearchQuery renders "b:null" specifically when query.query is the literal JS diff --git a/frontend/src/features/import/ImportXML.tsx b/frontend/src/features/import/ImportXML.tsx index 833e7150e..8ce90b21e 100644 --- a/frontend/src/features/import/ImportXML.tsx +++ b/frontend/src/features/import/ImportXML.tsx @@ -49,12 +49,22 @@ import { addMembers, selectProjectCardback, selectProjectSize, + setSelectedCardback, } from "@/store/slices/projectSlice"; export interface ParsedXmlImport { members: Array>; stock?: string; foil?: boolean; + /** The file's own root-level `` text, verbatim - `undefined` when the file carries + * no `` element (or an empty one) at all, distinct from the internal + * `xmlCardback` variable below (which additionally falls back to the CURRENT project + * cardback, purely for populating each backless front's own per-slot fallback). Foreign-order + * resilience Phase 1 (issue #324) follow-up: the caller (`ImportXML`'s `parseXMLFile`) uses + * this to initialise `state.project.cardback` when the project doesn't have one selected yet - + * see that call site's own comment for why, and why it's gated on the project's own cardback + * still being unset. */ + cardback?: string; } /** @@ -104,9 +114,13 @@ export function parseXmlImport( ? backsElement.getElementsByTagName("card") : undefined; - const xmlCardback = - rootElement.getElementsByTagName("cardback")[0]?.textContent ?? - projectCardback; + // The file's own literal `` text - `undefined` for a missing OR empty element ("" is + // not a usable identifier - see the XML_1_0 test fixture's ``), kept + // separate from `xmlCardback` below (which additionally falls back to the CURRENT project + // cardback, purely for populating each individual backless front's own per-slot fallback). + const xmlCardbackElementText = + rootElement.getElementsByTagName("cardback")[0]?.textContent || undefined; + const xmlCardback = xmlCardbackElementText ?? projectCardback; // `newMembers` is initialised with the maximum length it might need to contain all cards // the project can hold, then is truncated later according to `lastNonNullSlot` @@ -197,6 +211,7 @@ export function parseXmlImport( members: newMembers.slice(0, lastNonNullSlot + 1), stock: stock ?? undefined, foil, + cardback: xmlCardbackElementText, }; } @@ -221,7 +236,7 @@ export function ImportXML({ onImportComplete }: ImportXMLProps) { } // TODO: throw a user-visible error if the xml doc is malformed - const { members, stock, foil } = parseXmlImport( + const { members, stock, foil, cardback } = parseXmlImport( fileContents, projectSize, projectCardback, @@ -229,6 +244,25 @@ export function ImportXML({ onImportComplete }: ImportXMLProps) { ); dispatch(addMembers({ members })); + // Foreign-order resilience Phase 1 (issue #324) follow-up (owner-observed 2026-07-23): a + // BRAND NEW project (state.project.cardback still null - nothing auto-selected yet, e.g. the + // catalog has zero indexed cardbacks at all) never got state.project.cardback initialised + // from the very XML file just imported, even when the user opted to use that file's own + // cardback - so the "Common Cardback" panel (CommonCardback.tsx, the classic editor's right + // panel - a SEPARATE, project-wide concept from any individual slot's own back, see + // cardDocumentsSlice.ts's own comment on that distinction) kept showing "Card not found" + // right next to a perfectly-rendered orphan back-face slot tile, independent of whether the + // file's own cardback identifier is a real catalog cardback or an orphan Drive file ID this + // catalog has never indexed. Deliberately gated on `projectCardback == null` (the value + // already read above, before this import): an EXISTING non-null project cardback must stay + // untouched by a later XML import even with useXMLCardback=true - that's a real, deliberate, + // already-tested distinction (ImportXML.spec.ts's "import an XML and use its cardback" - + // useXMLCardback only ever governed each backless front's OWN per-slot fallback, never this + // project-wide setting) which this fix must not regress. + if (useXMLCardback && cardback != null && projectCardback == null) { + dispatch(setSelectedCardback({ selectedImage: cardback })); + } + if ( useXMLFinishSettings && stock != null && diff --git a/frontend/src/store/listenerMiddleware.test.ts b/frontend/src/store/listenerMiddleware.test.ts index ada708df5..81fdaefb6 100644 --- a/frontend/src/store/listenerMiddleware.test.ts +++ b/frontend/src/store/listenerMiddleware.test.ts @@ -14,8 +14,10 @@ import { synthesizeOrphanCardDocument } from "@/common/orphanCard"; import { computeSearchQueryHashKey } from "@/common/processing"; import { CardType, PrintingTagStatus } from "@/common/schema_types"; import { SlotProjectMembers } from "@/common/types"; +import { fetchCardbacks } from "@/store/slices/cardbackSlice"; import { fetchCardDocuments } from "@/store/slices/cardDocumentsSlice"; import { selectInvalidIdentifiers } from "@/store/slices/invalidIdentifiersSlice"; +import { selectProjectCardback } from "@/store/slices/projectSlice"; import { fetchSearchResults } from "@/store/slices/searchResultsSlice"; import { RootState, setupStore } from "@/store/store"; @@ -179,3 +181,118 @@ describe("foreign-order resilience Phase 1 - invalid-identifier listener (issue ); }); }); + +// Foreign-order resilience Phase 1 follow-up (issue #324, owner-observed 2026-07-23) - the +// "Common Cardback" panel (CommonCardback.tsx, the classic editor's right panel; also the source +// of truth ImportXML.tsx's own project-cardback propagation feeds) showed "Card not found" for +// an imported orphan back face. Root cause traced to THIS listener: unlike the per-slot +// invalid-identifier listener above (already fixed), it unconditionally cleared +// state.project.cardback the moment it wasn't in the catalog's own indexed cardbacks list, with +// no orphan-candidate carve-out at all - a SEPARATE concept from any individual slot's own back +// (see cardDocumentsSlice.ts's own comment on that distinction). +describe("foreign-order resilience Phase 1 follow-up - project cardback listener (issue #324)", () => { + function buildCardbackState( + cardback: string | null, + cardDocuments?: RootState["cardDocuments"]["cardDocuments"] + ): Partial { + return { + project: { + members: [], + nextMemberId: 0, + cardback, + mostRecentlySelectedSlot: null, + manualOverrides: {}, + }, + cardDocuments: { + cardDocuments: cardDocuments ?? {}, + status: "idle", + error: null, + }, + }; + } + + it("keeps an orphan-looking project cardback selected when the catalog's own cardbacks list doesn't include it", () => { + const store = setupStore(buildCardbackState(driveId)); + + store.dispatch(fetchCardbacks.fulfilled([], "request-cb-1", undefined)); + + expect(selectProjectCardback(store.getState())).toBe(driveId); + }); + + it("keeps a project cardback already resolved to a synthesized orphan CardDocument selected", () => { + const orphanDoc = synthesizeOrphanCardDocument(driveId, { + name: null, + cardType: CardType.Cardback, + }); + const store = setupStore( + buildCardbackState(driveId, { [driveId]: orphanDoc }) + ); + + store.dispatch(fetchCardbacks.fulfilled([], "request-cb-2", undefined)); + store.dispatch( + fetchCardDocuments.fulfilled( + { [driveId]: orphanDoc }, + "request-cb-2b", + undefined + ) + ); + + expect(selectProjectCardback(store.getState())).toBe(driveId); + }); + + it("still clears and replaces a genuinely invalid (non-Drive-ID-shaped) project cardback with the first real cardback", () => { + const store = setupStore(buildCardbackState("too-short")); + + store.dispatch( + fetchCardbacks.fulfilled(["a-real-cardback"], "request-cb-3", undefined) + ); + + expect(selectProjectCardback(store.getState())).toBe("a-real-cardback"); + }); + + it("still clears a known catalog cardback that's since been removed from the indexed list (regression guard)", () => { + const knownCardback = { + cardType: CardType.Cardback, + dateCreated: "", + dateModified: "", + dpi: 100, + extension: "png", + identifier: driveId, + language: "EN", + mediumThumbnailUrl: "", + name: "Some Cardback", + printingTagStatus: PrintingTagStatus.Unresolved, + priority: 0, + searchq: "", + size: 0, + smallThumbnailUrl: "", + source: "some-source", + sourceId: 1, + sourceName: "Some Source", + sourceVerbose: "Some Source", + tags: [], + }; + const store = setupStore( + buildCardbackState(driveId, { [driveId]: knownCardback }) + ); + + store.dispatch( + fetchCardbacks.fulfilled( + ["a-different-real-cardback"], + "request-cb-4", + undefined + ) + ); + store.dispatch( + fetchCardDocuments.fulfilled( + { [driveId]: knownCardback }, + "request-cb-4b", + undefined + ) + ); + + expect(selectProjectCardback(store.getState())).toBe( + "a-different-real-cardback" + ); + }); +}); diff --git a/frontend/src/store/listenerMiddleware.ts b/frontend/src/store/listenerMiddleware.ts index 199a23144..229ad592a 100644 --- a/frontend/src/store/listenerMiddleware.ts +++ b/frontend/src/store/listenerMiddleware.ts @@ -184,7 +184,18 @@ startAppListening({ }); startAppListening({ - actionCreator: fetchCardbacks.fulfilled, + // Foreign-order resilience Phase 1 (issue #324) follow-up - the owner's own repro traced this + // exact listener as "the CARDBACK display path [that] doesn't resolve orphan identifiers": it + // unconditionally cleared state.project.cardback (the "Common Cardback" panel/toolbar picker's + // own selection - a SEPARATE concept from any individual slot's own back, see + // cardDocumentsSlice.ts's own comment on that distinction) whenever it wasn't a real indexed + // cardback, with no orphan-candidate carve-out at all - unlike the sibling per-slot listener + // below, which already got that fix. fetchCardDocuments.fulfilled added to the trigger set + // (same ordering rationale as the per-slot listener's own comment): cardDocuments isn't settled + // yet on the original fetchCardbacks.fulfilled-only trigger, so this listener needs a second + // pass once it is, to let a genuinely-known-but-filtered/removed cardback (isOrphan + // false/undefined) still fall through to being cleared exactly as before. + matcher: isAnyOf(fetchCardbacks.fulfilled, fetchCardDocuments.fulfilled), /** * Whenever the list of cardbacks changes, this listener will deselect the cardback * if it's no longer valid, then select the first cardback in the list if there are @@ -199,7 +210,19 @@ startAppListening({ let newCardback = currentCardback; if (newCardback != null && !cardbacks.includes(newCardback)) { - newCardback = undefined; + // Same "wait and see" carve-out as the per-slot listener below: an identifier already + // resolved to a synthesized orphan CardDocument, or not resolved either way yet but still + // looks like a real Drive file ID, is left selected rather than cleared. + const knownCardDocument = selectCardDocumentByIdentifier( + state, + newCardback + ); + const isOrphanCandidate = + knownCardDocument?.isOrphan === true || + (knownCardDocument == null && isLikelyDriveFileId(newCardback)); + if (!isOrphanCandidate) { + newCardback = undefined; + } } if (newCardback == null && cardbacks.length > 0) { newCardback = selectFirstFavoritedOrFirst( diff --git a/frontend/tests/ImportXML.spec.ts b/frontend/tests/ImportXML.spec.ts index 47f8b37f0..32f99d11e 100644 --- a/frontend/tests/ImportXML.spec.ts +++ b/frontend/tests/ImportXML.spec.ts @@ -1,3 +1,5 @@ +import { expect } from "@playwright/test"; + import { S30, SelectedImageSeparator } from "@/common/constants"; import { cardDocument1, @@ -739,4 +741,51 @@ test.describe("ImportXML", () => { ); await expectCardbackSlotState(page, cardDocument2.name, 1, 2); // cardback should not have changed }); + + // Foreign-order resilience Phase 1 follow-up (issue #324, owner-observed 2026-07-23): a BRAND + // NEW project (no cardback selected yet - here because the mocked catalog has zero indexed + // cardbacks at all, same as OrphanRendering.spec.ts's own repro) previously left the "Common + // Cardback" panel (CommonCardback.tsx, the classic editor's right panel) showing "Card not + // found" forever after an XML import, right next to a perfectly-rendered orphan back-face slot + // tile - even though the import's own (an orphan Drive file ID here, but this fix + // applies identically to a real catalog cardback) was sitting right there unused. See + // ImportXML.tsx's own parseXMLFile comment for the fix (state.project.cardback is now + // initialised from the import when nothing was selected before it) and its own gate against + // regressing the "should not have changed" tests above (an EXISTING project cardback is never + // touched by a later import). + test("importing an XML into a brand new project with no cardback yet initialises the Common Cardback panel from the file's own - even an orphan", async ({ + page, + network, + }) => { + network.use(...defaultHandlers); + await loadPageWithDefaultBackend(page); + + const orphanFrontId = "1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn"; + const orphanBackId = "1LrVX0pUcye9n_0RtaDNVl2xPrQgn7CYf"; + + await importXML( + page, + ` +
    + 1 + ${S30} + false +
    + + + ${orphanFrontId} + google_drive + 0 + Kharn.png + kharn + + + ${orphanBackId} +     ` + ); + + const commonCardback = page.getByTestId("common-cardback"); + await expect(commonCardback.getByTestId("orphan-badge")).toBeVisible(); + await expect(commonCardback).not.toContainText("Card Not Found"); + }); }); diff --git a/frontend/tests/OrphanRendering.spec.ts b/frontend/tests/OrphanRendering.spec.ts index a5022a1d6..40346de93 100644 --- a/frontend/tests/OrphanRendering.spec.ts +++ b/frontend/tests/OrphanRendering.spec.ts @@ -1,8 +1,27 @@ /** * Foreign-order resilience Phase 1 (issue #324) - end-to-end coverage for the owner's own * 2026-07-23 high-priority repro: importing a reference to a Drive file ID this catalog has - * never indexed must render an orphan tile (badge, image, "find this card" search still live), - * not silently drop the selection or land it in the Invalid Cards modal. + * never indexed must render an orphan tile (image, "find this card" search still live), not + * silently drop the selection or land it in the Invalid Cards modal. + * + * Acceptance surface (owner ruling, 2026-07-23 review round): the classic /editor page is a + * legacy route held behind the route-swap PR #389 - the UNIFIED /display page (nav "Editor") + * is the only acceptance surface for frontend rendering work, so both cases below run there, not + * on /editor. The shared parse/import layers (processing.ts, cardDocumentsSlice.ts, + * listenerMiddleware.ts) are unaffected by which page mounts them - orphan rendering on /display + * needed NO code change (it already reads `cardDocument.mediumThumbnailUrl`/`isOrphan` off the + * same shared `cardDocuments` store slice PagePreview.tsx's own sheet-cell renderer uses), so + * this file is proof, not a fix. Note this is a DIFFERENT rendering path from Card.tsx (the + * classic editor's own card component, which additionally shows an "orphan-badge" corner label - + * see Card.test.tsx for that unit-level coverage): PagePreview.tsx (the /display sheet's own + * renderer) has no badge equivalent today, so no badge assertion appears below - that's a + * pre-existing, page-scoped gap in the sheet's own visual language, not a regression this file + * introduces or hides. + * + * The classic editor's own "Common Cardback" panel (CommonCardback.tsx, /editor only - /display + * has no equivalent persistent tile, only the CardbackToolbarButton picker) is covered + * separately by ImportXML.spec.ts's "brand new project" test, since that panel/bug is + * editor-specific. */ import { expect } from "@playwright/test"; @@ -10,13 +29,7 @@ import { expect } from "@playwright/test"; import { defaultHandlers } from "@/mocks/handlers"; import { test } from "../playwright.setup"; -import { - expectCardSlotToExist, - importText, - importXML, - loadPageWithDefaultBackend, - toggleFace, -} from "./test-utils"; +import { loadPageWithDefaultBackend } from "./test-utils"; // A syntactically real-looking Drive file ID this catalog's mocked backend never returns from // /2/cards/ or /3/editorSearch/ (see defaultHandlers - cardDocumentsNoResults/ @@ -25,99 +38,124 @@ import { const orphanId = "1FItgPw7VK_Tbv6dMiqdy5zd-jAoEC9mn"; const orphanBackId = "1LrVX0pUcye9n_0RtaDNVl2xPrQgn7CYf"; -test.describe("orphan rendering (issue #324)", () => { +// Whichever test in this file is first to actually hit /display pays Next dev mode's on-demand +// page-compile cost for a brand-new route (DisplayPage.spec.ts's own precedent comment) - +// comfortably over the default 30s test timeout when this file runs in isolation. +test.describe.configure({ timeout: 60_000 }); + +test.describe("orphan rendering (issue #324) - unified /display page", () => { test("the owner's exact reported text-import line registers and renders an orphan tile", async ({ page, network, }) => { network.use(...defaultHandlers); - await loadPageWithDefaultBackend(page); + await loadPageWithDefaultBackend(page, "display"); + await expect(page.getByTestId("display-empty-state")).toBeVisible({ + timeout: 20_000, + }); - await importText(page, `1x Kharn [mpc:${orphanId}]`); + await page + .getByRole("textbox", { name: "import-text" }) + .fill(`1x Kharn [mpc:${orphanId}]`); + await page.getByRole("button", { name: "import-text-submit" }).click(); - await expectCardSlotToExist(page, 1); - const frontSlot = page.getByTestId("front-slot0"); - // The stand-in name comes from the parsed SearchQuery, which - same as every other text - // import - lowercases the query text (processQuery); "Kharn" the user typed, "kharn" here - // is expected, not a bug. - await expect(frontSlot).toContainText("kharn"); - await expect(frontSlot.getByTestId("orphan-badge")).toBeVisible(); - await expect(frontSlot.getByTestId("orphan-badge")).toHaveText("Your file"); + await expect(page.getByTestId("display-page")).toBeVisible(); // Never lands in the Invalid Cards flow - the whole point of this feature. await expect(page.getByText("Review Invalid Cards")).not.toBeVisible(); - // Wait for the real direct-from-Google fetch to actually resolve before screenshotting - - // otherwise the shot just captures the spinner mid-flight. The extra fixed wait lets the - // image's own 0.3s CSS opacity fade-in (card-img-fade-in) finish too, purely cosmetic. - await expect(frontSlot.getByRole("status")).not.toBeVisible({ - timeout: 15_000, - }); + const slot = page.getByTestId("page-preview-slot").first(); + // The stand-in name comes from the parsed SearchQuery, which - same as every other text + // import - lowercases the query text (processQuery); "Kharn" the user typed, "kharn" here + // is expected, not a bug. + const image = slot.locator("img"); + // Generous timeout: this is the same real direct-from-Google fetch URL construction the + // classic editor's own orphan test waits out, plus this page's own first-compile cost. + await expect(image).toHaveCount(1, { timeout: 45_000 }); + await expect(image).toHaveAttribute("alt", "kharn"); + await expect(image).toHaveAttribute( + "src", + `https://lh4.googleusercontent.com/d/${orphanId}=h800` + ); + await page.waitForTimeout(500); await page.screenshot({ path: "test-results/orphan-text-import-desktop.png", }); }); - test("an XML order referencing an unindexed front id and an unindexed implicit cardback (the reported b:null case) renders both as orphans", async ({ + test("an XML order referencing an unindexed front id and an unindexed implicit cardback (the reported b:null case) renders both as orphans, including the cardback corner", async ({ page, network, }) => { network.use(...defaultHandlers); - await loadPageWithDefaultBackend(page); - - await importXML( - page, - ` -
    - 1 - (S30) Standard Smooth - false -
    - - - ${orphanId} - google_drive - 0 - Kharn.png - kharn - - - ${orphanBackId} -     ` + await loadPageWithDefaultBackend(page, "display"); + await expect(page.getByTestId("display-empty-state")).toBeVisible({ + timeout: 20_000, + }); + + await page.getByRole("button", { name: "XML" }).click(); + const fileInput = page + .getByLabel("import-xml") + .locator('input[type="file"]') + .first(); + await fileInput.setInputFiles({ + name: "test.xml", + mimeType: "text/xml;charset=utf-8", + buffer: Buffer.from( + ` +
    + 1 + (S30) Standard Smooth + false +
    + + + ${orphanId} + google_drive + 0 + Kharn.png + kharn + + + ${orphanBackId} +     ` + ), + }); + + await expect(page.getByTestId("display-page")).toBeVisible(); + await expect(page.getByText("Review Invalid Cards")).not.toBeVisible(); + + const slot = page.getByTestId("page-preview-slot").first(); + const frontImage = slot.locator("img"); + await expect(frontImage).toHaveCount(1, { timeout: 45_000 }); + await expect(frontImage).toHaveAttribute( + "src", + `https://lh4.googleusercontent.com/d/${orphanId}=h800` ); - await expectCardSlotToExist(page, 1); - const frontSlot = page.getByTestId("front-slot0"); - await expect(frontSlot.getByTestId("orphan-badge")).toBeVisible(); + await page.waitForTimeout(500); + await page.screenshot({ + path: "test-results/orphan-xml-import-desktop.png", + }); // No element covers this slot, so parseXmlImport falls back to the order's own // root-level - the exact "b:null" mechanism from the owner's screenshot (see // ImportXML.test.ts's own comment on this). That fallback is per-slot (back-slot0's own - // selectedImage) - it's a SEPARATE concept from the shared "Common Cardback" panel (which - // only reflects state.project.cardback, auto-selected from the indexed cardbacks list, and - // stays empty here since the mocked backend has none) - so the back-slot0 tile is what's - // under test, not Common Cardback. The small viewport this suite runs at collapses the - // editor to a front/back toggle, so it must be switched to before back-slot0 is visible. - await toggleFace(page); - const backSlot = page.getByTestId("back-slot0"); - await expect(backSlot.getByTestId("orphan-badge")).toBeVisible(); - await expect(backSlot.getByRole("status")).not.toBeVisible({ - timeout: 15_000, - }); - await page.waitForTimeout(500); - await page.screenshot({ - path: "test-results/orphan-xml-import-backs-desktop.png", - }); - await toggleFace(page); + // selectedImage), rendered here via the SAME sheet cell once the page is toggled to show + // backs - the "cardback corner" from the owner's fix request. This is a DIFFERENT concept + // from the "Common Cardback" panel (ImportXML.spec.ts's own coverage, /editor only). + await page.getByRole("button", { name: /Showing: Fronts/ }).click(); + const backImage = slot.locator("img"); + await expect(backImage).toHaveCount(1, { timeout: 45_000 }); + await expect(backImage).toHaveAttribute( + "src", + `https://lh4.googleusercontent.com/d/${orphanBackId}=h800` + ); - await expect(page.getByText("Review Invalid Cards")).not.toBeVisible(); - await expect(frontSlot.getByRole("status")).not.toBeVisible({ - timeout: 15_000, - }); await page.waitForTimeout(500); await page.screenshot({ - path: "test-results/orphan-xml-import-desktop.png", + path: "test-results/orphan-xml-import-backs-desktop.png", }); }); });