diff --git a/.convoys/redesign-scanner-flow/audit-redesign-scanner-flow-44.md b/.convoys/redesign-scanner-flow/audit-redesign-scanner-flow-44.md index 51a6142..3bb949e 100644 --- a/.convoys/redesign-scanner-flow/audit-redesign-scanner-flow-44.md +++ b/.convoys/redesign-scanner-flow/audit-redesign-scanner-flow-44.md @@ -25,8 +25,8 @@ Reports posted to [PR #44](https://github.com/varutasu/tcg-vault/pull/44#issueco | Slug | Priority | Source | | --- | --- | --- | | `scanner-redesign-a11y-fixes` | P1 | **RESOLVED** — PR #45 | -| `scanner-user-cards-quantity-guard` | P2 | **In progress** — PR pending | -| `test-scanner-redesign-surfaces` | P2 | Reviewer — unit tests for new components + upload route | +| `scanner-user-cards-quantity-guard` | P2 | **RESOLVED** — PR #46 | +| `test-scanner-redesign-surfaces` | P2 | **In progress** — PR pending | | `document-condition-foil-destination-semantics` | P3 | Reviewer — clarify or migrate condition/foil for collection/deck rows | | `camera-scanner-token-cleanup` | P3 | Design system — replace hardcoded hex overlay colors in CameraScanner | diff --git a/.convoys/scanner-user-cards-quantity-guard.md b/.convoys/scanner-user-cards-quantity-guard.md index 1407c57..e075264 100644 --- a/.convoys/scanner-user-cards-quantity-guard.md +++ b/.convoys/scanner-user-cards-quantity-guard.md @@ -6,7 +6,7 @@ success_metric: | the decks/[id]/cards handler contract. depends_on: - redesign-scanner-flow -status: open +status: closed created: 2026-05-27 --- diff --git a/.convoys/test-scanner-redesign-surfaces.md b/.convoys/test-scanner-redesign-surfaces.md new file mode 100644 index 0000000..09c7b9f --- /dev/null +++ b/.convoys/test-scanner-redesign-surfaces.md @@ -0,0 +1,27 @@ +--- +name: test-scanner-redesign-surfaces +classification: test +success_metric: | + Vitest covers scanner redesign API validation and key component behaviors + (destination picker, scanned card row, upload-image auth/rate-limit/MIME). +depends_on: + - redesign-scanner-flow +status: open +created: 2026-05-27 +--- + +# Convoy: test-scanner-redesign-surfaces + +P2 follow-up from `audit-redesign-scanner-flow-44` reviewer report. + +## Scope + +- `test/api/user-cards.test.js` — quantity validation +- `test/api/scan/upload-image.test.js` — auth, rate limit, MIME rejection +- `test/components/ScannerDestinationPicker.test.js` — game filter + destination toggle +- `test/components/ScannedCardItem.test.js` — ownership badge + metadata controls + +## Acceptance criteria + +1. `npm run test:run` green with new tests exercising real behavior (not implementation trivia). +2. No live DB or Blob calls in tests. diff --git a/test/api/scan/upload-image.test.js b/test/api/scan/upload-image.test.js new file mode 100644 index 0000000..c2c88fe --- /dev/null +++ b/test/api/scan/upload-image.test.js @@ -0,0 +1,104 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('crypto', () => ({ + randomUUID: vi.fn(() => 'test-uuid'), +})); + +vi.mock('@vercel/blob', () => ({ + put: vi.fn(), +})); + +vi.mock('../../../lib/permission-middleware.js', () => ({ + getUserFromRequest: vi.fn(), +})); + +vi.mock('../../../lib/rate-limit.js', () => ({ + checkUploadRateLimit: vi.fn(), +})); + +import { put } from '@vercel/blob'; +import { getUserFromRequest } from '../../../lib/permission-middleware.js'; +import { checkUploadRateLimit } from '../../../lib/rate-limit.js'; +import handler from '../../../pages/api/scan/upload-image.js'; + +const TINY_JPEG = + 'data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wCEAAEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQH/wAALCAABAAEBAREA/8QAFAABAAAAAAAAAAAAAAAAAAAAAv/EABQQAQAAAAAAAAAAAAAAAAAAAAD/2gAMAwEAAhEDEQA/AL+AAf/Z'; + +function createRes() { + const res = { + statusCode: 200, + headers: {}, + body: null, + status(code) { + res.statusCode = code; + return res; + }, + json(data) { + res.body = data; + return res; + }, + setHeader(name, value) { + res.headers[name] = value; + }, + }; + return res; +} + +describe('POST /api/scan/upload-image', () => { + beforeEach(() => { + vi.clearAllMocks(); + getUserFromRequest.mockResolvedValue({ userId: 7, email: 'a@b.c', role: 'user' }); + checkUploadRateLimit.mockResolvedValue({ allowed: true, remaining: 9, reset: Date.now() + 60_000 }); + put.mockResolvedValue({ url: 'https://blob.example/scans/7/test-uuid.jpg' }); + }); + + it('returns 405 for non-POST methods', async () => { + const res = createRes(); + await handler({ method: 'GET' }, res); + expect(res.statusCode).toBe(405); + }); + + it('returns 401 when unauthenticated', async () => { + getUserFromRequest.mockResolvedValue(null); + const res = createRes(); + await handler({ method: 'POST', body: { imageData: TINY_JPEG } }, res); + expect(res.statusCode).toBe(401); + expect(put).not.toHaveBeenCalled(); + }); + + it('returns 429 when upload rate limit is exceeded', async () => { + checkUploadRateLimit.mockResolvedValue({ + allowed: false, + remaining: 0, + reset: Date.now() + 30_000, + }); + const res = createRes(); + await handler({ method: 'POST', body: { imageData: TINY_JPEG } }, res); + expect(res.statusCode).toBe(429); + expect(res.headers['Retry-After']).toBeDefined(); + expect(put).not.toHaveBeenCalled(); + }); + + it('returns 400 for unsupported image MIME in data URL', async () => { + const res = createRes(); + await handler( + { method: 'POST', body: { imageData: 'data:image/gif;base64,AAAA' } }, + res + ); + expect(res.statusCode).toBe(400); + expect(res.body.error).toMatch(/JPEG, PNG, or WebP/); + expect(put).not.toHaveBeenCalled(); + }); + + it('uploads a valid JPEG data URL and returns the blob URL', async () => { + const res = createRes(); + await handler({ method: 'POST', body: { imageData: TINY_JPEG } }, res); + expect(res.statusCode).toBe(200); + expect(res.body.url).toBe('https://blob.example/scans/7/test-uuid.jpg'); + expect(put).toHaveBeenCalledWith( + 'scans/7/test-uuid.jpg', + expect.any(Buffer), + expect.objectContaining({ access: 'public', contentType: 'image/jpeg' }) + ); + }); +}); diff --git a/test/api/user-cards.test.js b/test/api/user-cards.test.js new file mode 100644 index 0000000..c21c84c --- /dev/null +++ b/test/api/user-cards.test.js @@ -0,0 +1,82 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@vercel/postgres', () => ({ sql: vi.fn() })); +vi.mock('../../lib/permission-middleware.js', () => ({ + getUserFromRequest: vi.fn(), +})); + +import { sql } from '@vercel/postgres'; +import { getUserFromRequest } from '../../lib/permission-middleware.js'; +import handler from '../../pages/api/user-cards.js'; + +function createRes() { + const res = { + statusCode: 200, + body: null, + status(code) { + res.statusCode = code; + return res; + }, + json(data) { + res.body = data; + return res; + }, + }; + return res; +} + +describe('POST /api/user-cards', () => { + beforeEach(() => { + vi.clearAllMocks(); + getUserFromRequest.mockResolvedValue({ userId: 1, email: 'a@b.c', role: 'user' }); + sql.mockResolvedValue({ rows: [] }); + }); + + it('returns 400 when quantity is not a number', async () => { + const req = { + method: 'POST', + body: { cardId: 42, quantity: 'abc' }, + }; + const res = createRes(); + + await handler(req, res); + + expect(res.statusCode).toBe(400); + expect(res.body.error).toBe('Quantity must be at least 1'); + expect(sql).not.toHaveBeenCalled(); + }); + + it('returns 400 when quantity is zero', async () => { + const req = { + method: 'POST', + body: { cardId: 42, quantity: 0 }, + }; + const res = createRes(); + + await handler(req, res); + + expect(res.statusCode).toBe(400); + expect(res.body.error).toBe('Quantity must be at least 1'); + expect(sql).not.toHaveBeenCalled(); + }); + + it('inserts parsed quantity for a new owned card', async () => { + sql.mockResolvedValueOnce({ rows: [] }); + + const req = { + method: 'POST', + body: { cardId: 42, quantity: '3', condition: 'LP', is_foil: true }, + }; + const res = createRes(); + + await handler(req, res); + + expect(res.statusCode).toBe(200); + expect(sql).toHaveBeenCalledTimes(2); + const insertCall = sql.mock.calls[1]; + expect(insertCall[2]).toBe(42); + expect(insertCall[3]).toBe(3); + expect(insertCall[4]).toBe('LP'); + expect(insertCall[5]).toBe(true); + }); +}); diff --git a/test/components/ScannedCardItem.test.js b/test/components/ScannedCardItem.test.js new file mode 100644 index 0000000..39192fc --- /dev/null +++ b/test/components/ScannedCardItem.test.js @@ -0,0 +1,91 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; + +import ScannedCardItem from '../../components/ScannedCardItem.js'; + +const BASE_CARD = { + id: 'scan-1', + name: 'Lightning Bolt', + set: 'Alpha', + databaseId: 99, + quantity: 1, + condition: 'NM', + isFoil: false, + image_url: null, + processed: false, +}; + +function renderItem(overrides = {}, props = {}) { + const card = { ...BASE_CARD, ...overrides }; + const defaultProps = { + card, + collections: [{ id: 1, name: 'Trade Binder' }], + decks: [{ id: 2, name: 'Burn', game: 'mtg' }], + selected: false, + onToggleSelect: vi.fn(), + onIncrement: vi.fn(), + onDecrement: vi.fn(), + onUpdateMetadata: vi.fn(), + onMarkOwned: vi.fn(), + onAddToCollection: vi.fn(), + onAddToDeck: vi.fn(), + onRemove: vi.fn(), + ...props, + }; + + return render(); +} + +describe('ScannedCardItem', () => { + beforeEach(() => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ quantity: 2 }), + }) + ); + }); + + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + }); + + it('announces ownership count with role=status', async () => { + renderItem(); + + const badge = await screen.findByRole('status', { + name: /you already own 2 copies/i, + }); + expect(badge.textContent).toContain('You own 2'); + }); + + it('calls onUpdateMetadata when condition changes', () => { + const onUpdateMetadata = vi.fn(); + renderItem({}, { onUpdateMetadata }); + + fireEvent.change(screen.getByRole('combobox', { name: /condition/i }), { + target: { value: 'LP' }, + }); + expect(onUpdateMetadata).toHaveBeenCalledWith({ condition: 'LP' }); + }); + + it('calls onUpdateMetadata when foil is toggled', () => { + const onUpdateMetadata = vi.fn(); + renderItem({}, { onUpdateMetadata }); + + fireEvent.click(screen.getByRole('checkbox', { name: /foil/i })); + expect(onUpdateMetadata).toHaveBeenCalledWith({ isFoil: true }); + }); + + it('does not fetch ownership when the card is not in the catalog', async () => { + renderItem({ databaseId: null }); + + await waitFor(() => { + expect(fetch).not.toHaveBeenCalled(); + }); + expect(screen.queryByRole('status')).toBeNull(); + }); +}); diff --git a/test/components/ScannerDestinationPicker.test.js b/test/components/ScannerDestinationPicker.test.js new file mode 100644 index 0000000..515d88b --- /dev/null +++ b/test/components/ScannerDestinationPicker.test.js @@ -0,0 +1,94 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; + +import ScannerDestinationPicker from '../../components/ScannerDestinationPicker.js'; + +const COLLECTIONS = [ + { id: 1, name: 'Modern Staples', tcg: 'MTG' }, + { id: 2, name: 'Pokémon Binder', tcg: 'Pokemon' }, +]; + +const DECKS = [ + { id: 10, name: 'Commander', game: 'mtg' }, + { id: 11, name: 'Standard', game: 'pokemon' }, +]; + +describe('ScannerDestinationPicker', () => { + afterEach(() => cleanup()); + + it('shows the active destination type with aria-pressed', () => { + render( + + ); + + expect(screen.getByRole('button', { name: /owned/i }).getAttribute('aria-pressed')).toBe('true'); + expect(screen.getByRole('button', { name: /collection/i }).getAttribute('aria-pressed')).toBe('false'); + }); + + it('calls onDestinationChange when switching to a collection', () => { + const onDestinationChange = vi.fn(); + + render( + + ); + + fireEvent.click(screen.getByRole('button', { name: /collection/i })); + + expect(onDestinationChange).toHaveBeenCalledWith({ + type: 'collection', + id: 1, + label: 'Modern Staples', + }); + }); + + it('filters decks by game and disables deck toggle when none match', () => { + render( + + ); + + expect(screen.getByRole('button', { name: /deck/i }).disabled).toBe(false); + expect(screen.queryByText(/Standard/)).toBeNull(); + }); + + it('calls onGameFilterChange when the game select changes', () => { + const onGameFilterChange = vi.fn(); + + render( + + ); + + fireEvent.change(screen.getByRole('combobox', { name: /game filter/i }), { + target: { value: 'Lorcana' }, + }); + expect(onGameFilterChange).toHaveBeenCalledWith('Lorcana'); + }); +});