From c917bba54a4105bc1b46b0a4db4fa0f6f09b112f Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:27:53 +0100 Subject: [PATCH] fix(pokedex): open detail cards without a per-card round trip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the grid transport change, opening a Pokémon fetched /api/pokedexes/[id]/entries/[entryId]. That request re-scanned the whole dex to authorise one entry, on top of an auth call and an ownership query, and production runs its functions in a different region from the database — so each card took seconds to fill in. The page's 60-second reconcile also wiped the detail cache, making cards refetch about once a minute. Details are catalog text plus personal notes, so the dex is now read once per account/dex in the background at the interactive/idle boundary and fills that cache; a card opens straight from memory. That read reuses the existing combined-data endpoint, which gains an opt-out for its count query since a caller taking every row already knows the total. The per-entry endpoint stays as the fallback for a card opened before the background read lands, and now checks membership for the single entry instead of materialising the dex. Catch status still comes from the live grid row and any queued write, so a cached detail cannot show stale progress. The packed grid transport and virtualised rendering are unchanged. --- docs/performance/pokedex-implementation.md | 5 +- scripts/performance/verify-behavior.mjs | 6 ++ scripts/performance/verify.mjs | 23 ++++- .../repositories/CombinedDataRepository.ts | 45 +++++++++ src/lib/services/CombinedDataService.ts | 27 +++++- src/lib/services/PokedexGridService.ts | 9 +- src/lib/utils/pokemonDetail.ts | 36 +++++++ .../pokedexes/[id]/combined-data/+server.ts | 5 +- src/routes/pokedex/[id]/+page.svelte | 92 +++++++++++++----- tests/bdd/features/performance.feature | 6 ++ tests/bdd/steps/performance.steps.ts | 75 ++++++++++++--- tests/unit/combinedDataRepository.test.ts | 51 ++++++++++ tests/unit/combinedDataService.test.ts | 19 ++++ tests/unit/pokemonDetail.test.ts | 93 +++++++++++++++++++ 14 files changed, 442 insertions(+), 50 deletions(-) create mode 100644 src/lib/utils/pokemonDetail.ts create mode 100644 tests/unit/pokemonDetail.test.ts diff --git a/docs/performance/pokedex-implementation.md b/docs/performance/pokedex-implementation.md index 4d16beb..a295372 100644 --- a/docs/performance/pokedex-implementation.md +++ b/docs/performance/pokedex-implementation.md @@ -7,7 +7,8 @@ Status: application changes implemented; hosting comparison remains gated. Measu - Ownership and saved scope links are read together. Scoped entry retrieval is shared by rows/count consumers; the full grid performs no count query. Catch joins use ID maps. Scope deduplication, named default forms, supplements and ordering remain covered by repository tests. - The page awaits the entire compact grid and renders initial boxes on the server. Authentication state reaches SSR through the validated layout user. Successful navigation needs no grid API request; failed grid loads expose retry. - `PokedexGridRow` contains identity, sprite resolution fields and catch flags. The page and authenticated `/api/pokedexes/[id]/grid` endpoint transport named tuples defined in `PokedexGridRow.ts`; `packGrid`/`unpackGrid` keep that wire format out of components. Instructions, notes, origin games and repeated owner/dex IDs are absent. Existing full combined-data consumers retain their contracts. -- The detail endpoint verifies ownership and membership before returning one full `CombinedData` row. The modal opens immediately with identity and full artwork, then loads editable details. Its account/dex/entry cache, abort/sequence checks and pending-patch merge protect rapid selection changes and optimistic edits. +- Details for the whole dex are read once per account/dex in the background, at the same interactive/idle boundary as the other deferred startup work, and fill the modal's cache. Opening a card therefore renders from memory with no request of its own: a per-card round trip cost seconds on the deployed cross-region setup, which local timings did not show. That read reuses `/api/pokedexes/[id]/combined-data`, which now skips its count query when a caller asks for every row at once. +- The per-entry detail endpoint remains as the fallback for a card opened before the background read lands, and for a failed read. It verifies ownership and membership before returning one full `CombinedData` row, checking membership for the single entry rather than materialising the dex. The modal opens immediately with identity and full artwork, then fills in editable details. Its account/dex/entry cache, abort/sequence checks and pending-patch merge protect rapid selection changes and optimistic edits; catch status always comes from the live grid row, so a cached detail can never show stale progress. - Status writes send changed fields. Bulk writes group records by supplied columns, preserving omitted notes and flags; explicit empty notes clear them. New records use database defaults. Full-record callers and exports remain supported. Bulk box actions still target all original 30 slots, including dimmed entries. - Grid placeholders preserve geometry; visible boxes and one row of overscan mount populated cells. Focused boxes remain mounted, keyboard navigation crosses boundaries, modal close restores focus, and an accessible render-all option exposes the complete document. Each Pokémon uses one button with identity/status and a noninteractive tooltip. - Density is persisted in a cookie for stable SSR geometry and in local storage. Resize/density changes preserve the current box anchor. Existing local-storage-only preferences are replaced by the cookie after choosing a density. @@ -26,7 +27,7 @@ Fixtures: national (1,025 entries), Scarlet/Paldea forms (439 entries), mixed ca The packed grid is approximately 81 KB national and 36 KB scoped, versus the investigation's approximately 593 KB and 255 KB full-data JSON. The same-fixture integration assertion separately verifies at least 60% reduction against full combined rows. These are serialized row sizes, not compressed HTML document sizes. -The local browser checks verify at most 180 populated mounted cells, fewer than 2,500 DOM elements and CLS at most 0.1; observed initial population is 120 cells. Density/mobile checks include all three densities at 1,350 and 390 pixels. Detail artwork is asserted to load at 512 pixels. No redundant grid request is allowed; intentional detail requests are allowed. +The local browser checks verify at most 180 populated mounted cells, fewer than 2,500 DOM elements and CLS at most 0.1; observed initial population is 120 cells. Density/mobile checks include all three densities at 1,350 and 390 pixels. Detail artwork is asserted to load at 512 pixels. No redundant grid request is allowed; the dex is read for details exactly once, and opening a card afterwards must make no request at all. The offline-isolation checks block that background read so the snapshot path, not the primed cache, answers those clicks. Warm results from the corrected harness (30 samples per row; [sanitized summary](pokedex-local-results.json)): diff --git a/scripts/performance/verify-behavior.mjs b/scripts/performance/verify-behavior.mjs index 1b476df..c417e41 100644 --- a/scripts/performance/verify-behavior.mjs +++ b/scripts/performance/verify-behavior.mjs @@ -71,6 +71,9 @@ try { `Scroll anchor moved: ${anchorTop} to ${resizedTop}` ); await page.setViewportSize({ width: 1350, height: 940 }); + // Let the resize settle first: restoring the box anchor scrolls the page, and a late restore + // would undo the scroll below. + await page.waitForTimeout(200); await page.evaluate(() => window.scrollTo(0, document.body.scrollHeight)); await page.waitForSelector('[data-entry-index="1024"]'); assert.ok((await page.locator('[data-entry-index]').count()) < 300); @@ -117,6 +120,9 @@ try { await page.waitForSelector('[data-grid-interactive]'); assert.ok((await page.locator('body').innerText()).includes('Showing 439 of 439')); await page.goBack(); + // Block the background detail read so the cache stays empty: these checks are about the offline + // snapshot answering a click, not about details the browser already holds. + await page.route(`**/api/pokedexes/${national.id}/combined-data*`, (route) => route.abort()); await page .locator('.card') .filter({ hasText: national.name }) diff --git a/scripts/performance/verify.mjs b/scripts/performance/verify.mjs index f5261f8..b008f3b 100644 --- a/scripts/performance/verify.mjs +++ b/scripts/performance/verify.mjs @@ -41,6 +41,14 @@ const errors = []; page.on('pageerror', (error) => errors.push(error.message)); const requests = []; page.on('request', (request) => requests.push(new URL(request.url()).pathname)); +const primedDetails = () => + requests.filter((path) => /\/api\/pokedexes\/[^/]+\/combined-data$/.test(path)).length; +// Details are read once per dex at the idle boundary, so allow for a slow idle callback. +async function waitForDetailPrime() { + const deadline = Date.now() + 15_000; + while (primedDetails() === 0 && Date.now() < deadline) await page.waitForTimeout(100); + assert.equal(primedDetails(), 1, 'Details must be read exactly once for the dex'); +} const results = []; try { for (const dex of fixture.dexes) { @@ -61,18 +69,25 @@ try { assert.ok(stats.cells <= 180, `Mounted cells: ${stats.cells}`); assert.ok(stats.dom < 2500, `DOM elements: ${stats.dom}`); assert.ok(stats.cls <= 0.1, `CLS: ${stats.cls}`); - assert.equal( - requests.filter((path) => /\/api\/pokedexes\/[^/]+\/(grid|combined-data)$/.test(path)).length, - 0 - ); + // The grid ships with the page, so re-fetching it would be redundant. The details read is + // deliberate: it is what lets a card open without a round trip of its own. + assert.equal(requests.filter((path) => /\/api\/pokedexes\/[^/]+\/grid$/.test(path)).length, 0); + await waitForDetailPrime(); assert.equal(await page.locator('button button').count(), 0); await page.screenshot({ path: `${directory}/${dex.gameScope ? 'scoped' : 'national'}.png` }); const first = page.locator('[data-entry-index="0"]'); const entryId = await first.getAttribute('data-entry-id'); const notes = await readJson(`/api/pokedexes/${dex.id}/entries/${entryId}`); + const openedWith = requests.filter((path) => path.endsWith(`/entries/${entryId}`)).length; await first.click(); const modal = page.getByRole('dialog', { name: 'Pokémon details' }); await modal.getByLabel('Notes:', { exact: true }).waitFor(); + // Primed details mean opening a card costs no request of its own. + assert.equal( + requests.filter((path) => path.endsWith(`/entries/${entryId}`)).length, + openedWith, + 'Opening a card must not fetch its details separately' + ); assert.equal( await modal.getByLabel('Notes:', { exact: true }).inputValue(), notes.catchRecord.personalNotes diff --git a/src/lib/repositories/CombinedDataRepository.ts b/src/lib/repositories/CombinedDataRepository.ts index 64f5a2e..80fe9a8 100644 --- a/src/lib/repositories/CombinedDataRepository.ts +++ b/src/lib/repositories/CombinedDataRepository.ts @@ -397,6 +397,51 @@ class CombinedDataRepository { return entries; } + /** + * Authorises one entry without materialising the dex. Mirrors `findGridEntries`, which reads the + * scoped dex tables and — for game-scoped form dexes — supplements named forms from + * `pokedex_entries`. Region is not a grid filter, so it is not applied here either. + */ + async isEntryInDex( + entryId: number, + enableForms: boolean, + game: string, + dexScopes: string[] + ): Promise { + if (dexScopes.length) { + let scoped = this.supabase + .from('game_pokedex_entry_details') + .select('id') + .in('dexId', dexScopes) + .eq('id', entryId); + if (!enableForms) scoped = scoped.eq('isDefaultForm', true); + + const { data, error } = await scoped.limit(1); + if (error) throw new Error('Unable to load dex entries'); + if (data && data.length > 0) return true; + if (!enableForms || !game) return false; + + // Named forms are absent from the game dex tables and are supplemented from pokedex_entries. + const { data: forms, error: formsError } = await this.supabase + .from('pokedex_entries') + .select('id') + .eq('id', entryId) + .not('form', 'is', null) + .contains('gamesToCatchIn', [game]) + .limit(1); + if (formsError) throw new Error('Unable to load form entries'); + return !!forms && forms.length > 0; + } + + let query = this.supabase.from('pokedex_entries').select('id').eq('id', entryId); + if (!enableForms) query = query.eq('isDefaultForm', true); + if (game) query = query.contains('gamesToCatchIn', [game]); + + const { data, error } = await query.limit(1); + if (error) throw new Error('Unable to load dex entries'); + return !!data && data.length > 0; + } + async joinGridCatches(entries: PokedexEntryDB[]): Promise { const catches = new Map( ( diff --git a/src/lib/services/CombinedDataService.ts b/src/lib/services/CombinedDataService.ts index 00791f9..7c2b682 100644 --- a/src/lib/services/CombinedDataService.ts +++ b/src/lib/services/CombinedDataService.ts @@ -9,6 +9,8 @@ export type CombinedDataQuery = { enableForms: boolean; region?: string; game?: string; + /** Counting repeats the whole scoped read; callers fetching every row in one page can skip it. */ + includeCount?: boolean; }; /** @@ -20,16 +22,37 @@ export async function loadCombinedDataPage( supabase: SupabaseClient, userId: string, pokedex: Pokedex, - { page, limit, enableForms, region = '', game = '' }: CombinedDataQuery + { page, limit, enableForms, region = '', game = '', includeCount = true }: CombinedDataQuery ) { // Use the pokédex's gameScope as the default filter if no manual game filter is set. const effectiveGame = game || pokedex.gameScope || ''; const dexScopes = await resolveDexScopes(supabase, pokedex); const repo = new CombinedDataRepository(supabase, userId, pokedex._id); + const rows = repo.findCombinedData( + userId, + page, + limit, + enableForms, + region, + effectiveGame, + dexScopes + ); + + if (!includeCount) { + // Single-page callers already hold every row, so counting would repeat the same scoped read. + const combinedData = await rows; + return { + combinedData, + totalPages: 1, + currentPage: page, + totalCount: combinedData.length + }; + } + // The rows and the count are independent queries, so run them together. const [combinedData, totalCount] = await Promise.all([ - repo.findCombinedData(userId, page, limit, enableForms, region, effectiveGame, dexScopes), + rows, repo.countCombinedData(enableForms, region, effectiveGame, dexScopes) ]); diff --git a/src/lib/services/PokedexGridService.ts b/src/lib/services/PokedexGridService.ts index c271d4c..a0b7705 100644 --- a/src/lib/services/PokedexGridService.ts +++ b/src/lib/services/PokedexGridService.ts @@ -27,12 +27,13 @@ export async function loadPokedexEntryDetail( entryId: number ) { const scopes = await resolveDexScopes(supabase, pokedex); - const membership = new CombinedDataRepository(supabase, userId, pokedex._id, true); - const entries = await membership.findGridEntries( + const repo = new CombinedDataRepository(supabase, userId, pokedex._id); + const member = await repo.isEntryInDex( + entryId, pokedex.isFormDex, pokedex.gameScope || '', scopes ); - if (!entries.some((entry) => entry.id === entryId)) return null; - return new CombinedDataRepository(supabase, userId, pokedex._id).findEntryDetail(entryId); + if (!member) return null; + return repo.findEntryDetail(entryId); } diff --git a/src/lib/utils/pokemonDetail.ts b/src/lib/utils/pokemonDetail.ts new file mode 100644 index 0000000..7f8d9ec --- /dev/null +++ b/src/lib/utils/pokemonDetail.ts @@ -0,0 +1,36 @@ +import type { CatchRecord } from '$lib/models/CatchRecord'; +import type { CombinedData } from '$lib/models/CombinedData'; +import type { PokedexGridRow } from '$lib/models/PokedexGridRow'; + +/** + * Builds the Pokémon shown in the detail modal. The detail row supplies catalog text and personal + * notes; the live grid row and any queued write supply catch status, so a detail response can never + * undo a status change made while it was being loaded. + */ +export function mergeEntryDetail( + detail: CombinedData, + gridRecord: PokedexGridRow['catchRecord'] | null | undefined, + pending: Partial | null | undefined, + owner: string, + pokedexId: string, + entryId: string +): CombinedData { + if (!detail.catchRecord && !gridRecord && !pending) return { ...detail, catchRecord: null }; + return { + ...detail, + catchRecord: { + _id: '', + userId: owner, + pokedexId, + pokemonId: entryId, + caught: false, + haveToEvolve: false, + inHome: false, + hasGigantamaxed: false, + personalNotes: '', + ...detail.catchRecord, + ...gridRecord, + ...pending + } + }; +} diff --git a/src/routes/api/pokedexes/[id]/combined-data/+server.ts b/src/routes/api/pokedexes/[id]/combined-data/+server.ts index 89b6cce..490d724 100644 --- a/src/routes/api/pokedexes/[id]/combined-data/+server.ts +++ b/src/routes/api/pokedexes/[id]/combined-data/+server.ts @@ -21,6 +21,8 @@ export const GET = async (event: RequestEvent) => { const enableForms = url.searchParams.get('enableForms') === 'true'; const region = url.searchParams.get('region') || ''; const game = url.searchParams.get('game') || ''; + // Callers that fetch the whole dex in one page opt out of the duplicate count query. + const includeCount = url.searchParams.get('includeCount') !== 'false'; if (!userId) { // Anonymous users cannot view pokédexes @@ -42,7 +44,8 @@ export const GET = async (event: RequestEvent) => { limit, enableForms, region, - game + game, + includeCount }) ); } catch (err) { diff --git a/src/routes/pokedex/[id]/+page.svelte b/src/routes/pokedex/[id]/+page.svelte index 9b17845..c297261 100644 --- a/src/routes/pokedex/[id]/+page.svelte +++ b/src/routes/pokedex/[id]/+page.svelte @@ -31,6 +31,8 @@ type CatchRecordPatch } from '$lib/models/PokedexGridRow'; import PokemonSprite from '$lib/components/PokemonSprite.svelte'; + import { afterCriticalPageWork } from '$lib/utils/criticalPageWork'; + import { mergeEntryDetail } from '$lib/utils/pokemonDetail'; export let data: PageData; @@ -180,6 +182,10 @@ onDestroy(() => { detailRequest++; detailAbort?.abort(); + detailPrimeRequest++; + detailPrimeAbort?.abort(); + cancelDetailPrime?.(); + cancelDetailPrime = null; }); onDestroy(() => { catchWriteQueueUnsubscribe?.(); @@ -196,12 +202,62 @@ let returnFocus: HTMLElement | null = null; const detailCache = new Map(); let detailOwner = ''; + // Details are catalog text plus personal notes, so one background read serves every card in the + // dex. Without it each card open costs an authenticated round trip before anything can render. + let detailPrimeKey = ''; + let detailPrimeRequest = 0; + let detailPrimeAbort: AbortController | null = null; + let cancelDetailPrime: (() => void) | null = null; + $: if (localUser?.id !== detailOwner) { detailOwner = localUser?.id ?? ''; detailCache.clear(); + detailPrimeKey = ''; closePokemonModal(); } + $: if (browser && pokedexId && localUser?.id) scheduleDetailPrime(); + + function scheduleDetailPrime(force = false) { + if (!browser) return; + const owner = localUser?.id ?? ''; + const id = pokedexId; + if (!owner || !id) return; + const key = `${owner}:${id}`; + if (!force && key === detailPrimeKey) return; + detailPrimeKey = key; + cancelDetailPrime?.(); + cancelDetailPrime = afterCriticalPageWork(() => { + cancelDetailPrime = null; + void primeDetailCache(owner, id); + }); + } + + async function primeDetailCache(owner: string, id: string) { + if (!browser || !navigator.onLine) return; + const request = ++detailPrimeRequest; + detailPrimeAbort?.abort(); + detailPrimeAbort = new AbortController(); + try { + const enableForms = pokedex?.isFormDex ?? false; + const response = await fetch( + `/api/pokedexes/${id}/combined-data?page=1&limit=9999&enableForms=${enableForms}&includeCount=false`, + { signal: detailPrimeAbort.signal } + ); + if (!response.ok) throw new Error('Unable to load details'); + const result: { combinedData: CombinedData[] } = await response.json(); + if (request !== detailPrimeRequest || id !== pokedexId || owner !== localUser?.id) return; + for (const row of result.combinedData) { + detailCache.set(`${owner}:${id}:${row.pokedexEntry._id}`, row); + } + // A card opened before this landed is still waiting on its own request; serve it now. + if (showModal && !selectedPokemon && selectedSummary) void openPokemonModal(selectedSummary); + } catch { + // Cards fall back to fetching their own details; allow a later attempt to prime again. + if (request === detailPrimeRequest) detailPrimeKey = ''; + } + } + async function openPokemonModal(pokemon: PokedexGridRow) { if (!showModal) returnFocus = document.activeElement as HTMLElement; selectedSummary = pokemon; @@ -230,29 +286,14 @@ if (request !== detailRequest || id !== pokedexId || owner !== localUser?.id) return; if (!detail) throw new Error('These details are not saved for offline use.'); detailCache.set(key, detail); - // A detail response must not undo status changes made while it was in flight. - const current = combinedData?.find((row) => row.pokedexEntry._id === entryId)?.catchRecord; - const pending = catchWriteQueue?.getPendingPatch(entryId); - selectedPokemon = { - ...detail, - catchRecord: - detail.catchRecord || current || pending - ? { - _id: '', - userId: owner, - pokedexId: id, - pokemonId: entryId, - caught: false, - haveToEvolve: false, - inHome: false, - hasGigantamaxed: false, - personalNotes: '', - ...detail.catchRecord, - ...current, - ...pending - } - : null - }; + selectedPokemon = mergeEntryDetail( + detail, + combinedData?.find((row) => row.pokedexEntry._id === entryId)?.catchRecord, + catchWriteQueue?.getPendingPatch(entryId), + owner, + id, + entryId + ); } catch (error) { if (request !== detailRequest || id !== pokedexId || owner !== localUser?.id) return; detailError = error instanceof Error ? error.message : 'Unable to load details.'; @@ -430,7 +471,7 @@ ) return; combinedData = unpackGrid(result.grid); - detailCache.clear(); + // Cached details keep their catalog text; catch status is taken from the grid on open. } catch { if (request === gridRequest && id === pokedexId && owner === localUser?.id) failedToLoad = true; @@ -609,6 +650,7 @@ gridRequest++; closePokemonModal(); detailCache.clear(); + detailPrimeKey = ''; combinedData = data.grid ? unpackGrid(data.grid) : null; failedToLoad = data.grid === null; } @@ -636,6 +678,8 @@ const onOnline = () => { online = true; void catchWriteQueue?.flushNow(); + // Priming is skipped while offline, so ask for it again now the network is back. + scheduleDetailPrime(true); }; window.addEventListener('offline', onOffline); diff --git a/tests/bdd/features/performance.feature b/tests/bdd/features/performance.feature index e8f4a10..8941cb0 100644 --- a/tests/bdd/features/performance.feature +++ b/tests/bdd/features/performance.feature @@ -14,6 +14,12 @@ Feature: Signed-in page speed Then its entries appear within 5 seconds And the browser did not request the grid separately + Scenario: A Pokémon's card opens from data the browser already has + When I load the Pokédex page directly + And the Pokédex has finished loading its details in the background + And I open the first Pokémon + Then its details were already in the browser + Scenario: Moving between my Pokédex list and a Pokédex is quick When I switch between my Pokédex list and the Pokédex Then each switch finishes within 3 seconds diff --git a/tests/bdd/steps/performance.steps.ts b/tests/bdd/steps/performance.steps.ts index 5802467..70f4de1 100644 --- a/tests/bdd/steps/performance.steps.ts +++ b/tests/bdd/steps/performance.steps.ts @@ -5,10 +5,14 @@ import { test, expect } from '../fixtures'; const { When, Then } = createBdd(test); // Per-page scratch values; scenarios run one at a time (workers: 1). -const timings = new WeakMap< - Page, - { entriesMs?: number; switchMs: number[]; entryRequests: number } ->(); +type Timing = { + entriesMs?: number; + switchMs: number[]; + gridRequests: number; + detailRequests: number; + detailsPrimed: boolean; +}; +const timings = new WeakMap(); function entriesVisible(page: Page) { return expect(page.getByRole('button', { name: /^View details for / }).first()).toBeVisible({ @@ -18,21 +22,61 @@ function entriesVisible(page: Page) { When('I load the Pokédex page directly', async ({ page, state }) => { if (!state.pokedexId) throw new Error('A Pokédex must exist before it can be opened'); - const record = { switchMs: [], entryRequests: 0 }; + const record: Timing = { + switchMs: [], + gridRequests: 0, + detailRequests: 0, + detailsPrimed: false + }; timings.set(page, record); // Only requests made while the page first loads matter; the page's 60s reconciliation refetch // can't fire within this window. - const countEntryRequests = (request: { url(): string }) => { - if (/\/api\/pokedexes\/[^/]+\/(?:grid|combined-data)/.test(request.url())) - record.entryRequests++; + const countGridRequests = (request: { url(): string }) => { + if (/\/api\/pokedexes\/[^/]+\/grid/.test(request.url())) record.gridRequests++; }; - page.on('request', countEntryRequests); + // The background detail read is deliberately allowed: it is what keeps cards from fetching + // themselves one at a time. Remember when it lands so a later step can open a card. + const notePrimedDetails = (response: { url(): string; status(): number }) => { + if (/\/api\/pokedexes\/[^/]+\/combined-data/.test(response.url()) && response.status() === 200) + record.detailsPrimed = true; + }; + page.on('request', countGridRequests); + page.on('response', notePrimedDetails); const started = Date.now(); await page.goto(`/pokedex/${state.pokedexId}`); await entriesVisible(page); - (record as { entriesMs?: number }).entriesMs = Date.now() - started; - page.off('request', countEntryRequests); + record.entriesMs = Date.now() - started; + page.off('request', countGridRequests); +}); + +When('the Pokédex has finished loading its details in the background', async ({ page }) => { + const record = timings.get(page); + expect(record, 'the Pokédex page was never loaded').toBeDefined(); + await expect.poll(() => record!.detailsPrimed, { timeout: 30_000 }).toBe(true); +}); + +When('I open the first Pokémon', async ({ page }) => { + const record = timings.get(page); + expect(record, 'the Pokédex page was never loaded').toBeDefined(); + const countDetailRequests = (request: { url(): string }) => { + if (/\/api\/pokedexes\/[^/]+\/entries\//.test(request.url())) record!.detailRequests++; + }; + page.on('request', countDetailRequests); + await page + .getByRole('button', { name: /^View details for / }) + .first() + .click(); + // "Where to catch" comes from the detail row, so it only renders once details are present. + await expect(page.getByText('Where to catch:')).toBeVisible({ timeout: 10_000 }); + // Give any stray per-entry request time to be made before asserting none was. + await page.waitForTimeout(500); + page.off('request', countDetailRequests); +}); + +Then('its details were already in the browser', async ({ page }) => { + await expect(page.getByRole('status')).toHaveCount(0); + expect(timings.get(page)?.detailRequests).toBe(0); }); Then('its entries appear within {int} seconds', async ({ page }, seconds: number) => { @@ -44,12 +88,17 @@ Then('its entries appear within {int} seconds', async ({ page }, seconds: number Then('the browser did not request the grid separately', async ({ page }) => { // The server load includes the compact grid with the HTML, so the page must not make // the old hydrate-then-fetch round trip. - expect(timings.get(page)?.entryRequests).toBe(0); + expect(timings.get(page)?.gridRequests).toBe(0); }); When('I switch between my Pokédex list and the Pokédex', async ({ page, state }) => { if (!state.pokedexName) throw new Error('A Pokédex must exist before switching to it'); - const record = { switchMs: [] as number[], entryRequests: 0 }; + const record: Timing = { + switchMs: [], + gridRequests: 0, + detailRequests: 0, + detailsPrimed: false + }; timings.set(page, record); await page.goto('/my-pokedexes'); const card = page.locator('.card').filter({ hasText: state.pokedexName }).first(); diff --git a/tests/unit/combinedDataRepository.test.ts b/tests/unit/combinedDataRepository.test.ts index ce7f914..b8c52aa 100644 --- a/tests/unit/combinedDataRepository.test.ts +++ b/tests/unit/combinedDataRepository.test.ts @@ -234,3 +234,54 @@ describe('compact grid reads', () => { expect(selection?.args).toHaveLength(1); }); }); + +describe('CombinedDataRepository.isEntryInDex', () => { + it('accepts an entry the scoped dex lists, without reading the whole dex', async () => { + const { supabase, queries } = createSupabaseStub((table) => + table === 'game_pokedex_entry_details' + ? { data: [{ id: 25 }], error: null } + : { data: [], error: null } + ); + const repo = new CombinedDataRepository(supabase, 'user-1', 'dex-1'); + + await expect(repo.isEntryInDex(25, false, 'Black', ['black-unova'])).resolves.toBe(true); + + const [dexEntries] = queryFor(queries, 'game_pokedex_entry_details'); + expect(hasCall(dexEntries, 'eq', ['id', 25])).toBe(true); + expect(hasCall(dexEntries, 'eq', ['isDefaultForm', true])).toBe(true); + expect(hasCall(dexEntries, 'limit', [1])).toBe(true); + // A membership check must not page the dex the way the grid read does. + expect(dexEntries.calls.some((c) => c.method === 'range')).toBe(false); + }); + + it('accepts a named form a game-scoped form dex supplements from pokedex_entries', async () => { + const { supabase, queries } = createSupabaseStub((table) => + table === 'pokedex_entries' ? { data: [{ id: 99 }], error: null } : { data: [], error: null } + ); + const repo = new CombinedDataRepository(supabase, 'user-1', 'dex-1'); + + await expect(repo.isEntryInDex(99, true, 'Scarlet', ['scarlet-paldea'])).resolves.toBe(true); + + const [forms] = queryFor(queries, 'pokedex_entries'); + expect(hasCall(forms, 'not', ['form', 'is', null])).toBe(true); + expect(hasCall(forms, 'contains', ['gamesToCatchIn', ['Scarlet']])).toBe(true); + }); + + it('rejects an entry that is in neither the scoped dex nor its form supplement', async () => { + const { supabase } = createSupabaseStub(); + const repo = new CombinedDataRepository(supabase, 'user-1', 'dex-1'); + + await expect(repo.isEntryInDex(1, true, 'Scarlet', ['scarlet-paldea'])).resolves.toBe(false); + }); + + it('checks an unscoped dex against the game filter alone', async () => { + const { supabase, queries } = createSupabaseStub(() => ({ data: [{ id: 25 }], error: null })); + const repo = new CombinedDataRepository(supabase, 'user-1', 'dex-1'); + + await expect(repo.isEntryInDex(25, true, 'Red', [])).resolves.toBe(true); + + const [entries] = queryFor(queries, 'pokedex_entries'); + expect(hasCall(entries, 'contains', ['gamesToCatchIn', ['Red']])).toBe(true); + expect(mentionsIsDefaultForm(entries)).toBe(false); + }); +}); diff --git a/tests/unit/combinedDataService.test.ts b/tests/unit/combinedDataService.test.ts index d8a4110..267a486 100644 --- a/tests/unit/combinedDataService.test.ts +++ b/tests/unit/combinedDataService.test.ts @@ -74,4 +74,23 @@ describe('loadCombinedDataPage', () => { releaseRows([]); await expect(pending).resolves.toMatchObject({ totalCount: 45, totalPages: 5 }); }); + + it('skips the count query when the caller asks for every row at once', async () => { + findCombinedData.mockResolvedValue([{ id: 'a' }, { id: 'b' }]); + + const result = await loadCombinedDataPage(supabase, 'user-1', pokedex, { + page: 1, + limit: 9999, + enableForms: true, + includeCount: false + }); + + expect(countCombinedData).not.toHaveBeenCalled(); + expect(result).toEqual({ + combinedData: [{ id: 'a' }, { id: 'b' }], + totalPages: 1, + currentPage: 1, + totalCount: 2 + }); + }); }); diff --git a/tests/unit/pokemonDetail.test.ts b/tests/unit/pokemonDetail.test.ts new file mode 100644 index 0000000..3e6d4dc --- /dev/null +++ b/tests/unit/pokemonDetail.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest'; +import { mergeEntryDetail } from '$lib/utils/pokemonDetail'; +import type { CombinedData } from '$lib/models/CombinedData'; + +const detail = { + pokedexEntry: { + _id: '25', + pokedexNumber: 25, + pokemon: 'Pikachu', + form: '', + spriteKey: '25', + canGigantamax: true, + regionToCatchIn: 'Kanto', + gamesToCatchIn: ['Red'], + regionToEvolveIn: '', + evolutionInformation: 'Use a Thunder Stone', + catchInformation: ['Viridian Forest'], + notes: 'Dex note' + }, + catchRecord: { + _id: 'catch-1', + userId: 'user-1', + pokedexId: 'dex-1', + pokemonId: '25', + caught: false, + haveToEvolve: false, + inHome: false, + hasGigantamaxed: false, + personalNotes: 'My note' + } +} satisfies CombinedData; + +describe('mergeEntryDetail', () => { + it('keeps catalog text and personal notes while taking status from the grid row', () => { + const merged = mergeEntryDetail( + detail, + { _id: 'catch-1', caught: true, haveToEvolve: false, inHome: true, hasGigantamaxed: false }, + undefined, + 'user-1', + 'dex-1', + '25' + ); + + expect(merged.pokedexEntry.evolutionInformation).toBe('Use a Thunder Stone'); + expect(merged.catchRecord).toMatchObject({ + caught: true, + inHome: true, + personalNotes: 'My note' + }); + }); + + it('lets a queued write win over both the detail row and the grid row', () => { + const merged = mergeEntryDetail( + detail, + { _id: 'catch-1', caught: false, haveToEvolve: false, inHome: false, hasGigantamaxed: false }, + { userId: 'user-1', pokedexId: 'dex-1', pokemonId: '25', caught: true }, + 'user-1', + 'dex-1', + '25' + ); + + expect(merged.catchRecord?.caught).toBe(true); + }); + + it('builds a record from a queued write when nothing has been saved yet', () => { + const merged = mergeEntryDetail( + { ...detail, catchRecord: null }, + null, + { userId: 'user-1', pokedexId: 'dex-1', pokemonId: '25', haveToEvolve: true }, + 'user-1', + 'dex-1', + '25' + ); + + expect(merged.catchRecord).toEqual({ + _id: '', + userId: 'user-1', + pokedexId: 'dex-1', + pokemonId: '25', + caught: false, + haveToEvolve: true, + inHome: false, + hasGigantamaxed: false, + personalNotes: '' + }); + }); + + it('leaves an uncaught Pokémon without a catch record', () => { + const merged = mergeEntryDetail({ ...detail, catchRecord: null }, null, null, 'u', 'd', '25'); + + expect(merged.catchRecord).toBeNull(); + }); +});