diff --git a/app/games/[id]/load-game.ts b/app/games/[id]/load-game.ts index 7dc1bdaf..8abeb9ba 100644 --- a/app/games/[id]/load-game.ts +++ b/app/games/[id]/load-game.ts @@ -18,15 +18,29 @@ export type LoadGameResult = * A game IGDB returns with `hidden: true` (DLC, a bundle, an edition, the * Erotic theme) is reported as not found: the catalog filter exists to keep * those out of browsing, the way an adult movie is (spec 0025, rationale). + */ +export async function loadGame(id: number): Promise { + const result = await readGameIncludingHidden(id); + return result.kind === "found" && result.game.hidden + ? { kind: "not_found" } + : result; +} + +/** + * `loadGame` without the hidden rule: a hidden game comes back as found, + * with `hidden: true`. The game status actions need the difference, because + * Want to play accepts a hidden game, so a listed game stays movable, while + * Playing and Finished refuse it (spec 0026, AC-9). * * Only an `IgdbError` becomes `failed`. Anything else, `IgdbValidationError` * included, is a bug rather than an IGDB outcome, so it is rethrown instead - * of being dressed up as "Couldn't reach IGDB" (AC-8). + * of being dressed up as "Couldn't reach IGDB" (spec 0025, AC-8). */ -export async function loadGame(id: number): Promise { +export async function readGameIncludingHidden( + id: number, +): Promise { try { - const game = await getGame(id); - return game.hidden ? { kind: "not_found" } : { kind: "found", game }; + return { kind: "found", game: await getGame(id) }; } catch (error) { if (isIgdbNotFound(error)) return { kind: "not_found" }; if (error instanceof IgdbError) return { kind: "failed" }; diff --git a/app/games/[id]/page.test.ts b/app/games/[id]/page.test.ts index e06596f6..003fd641 100644 --- a/app/games/[id]/page.test.ts +++ b/app/games/[id]/page.test.ts @@ -4,6 +4,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { GameDetailSkeleton } from "@/components/game/game-detail-skeleton"; import { GameHero } from "@/components/game/game-hero"; +import { GameTrackingSlot } from "@/components/tracking/game-tracking-slot"; /** * covers: spec 0025, AC-1, AC-6, AC-7, AC-8, AC-9, AC-10 @@ -148,7 +149,8 @@ describe("the streamed body", () => { expect(loadGame).toHaveBeenCalledWith(1942); expect(hero.type).toBe(GameHero); - expect(hero.props).toEqual({ + const { tracking, ...facts } = hero.props as { tracking: Element }; + expect(facts).toEqual({ name: FOUND.name, coverUrl: FOUND.coverUrl, releaseDate: FOUND.releaseDate, @@ -160,6 +162,24 @@ describe("the streamed body", () => { }); }); + it("puts the status control in the hero's tracking slot, in its own null fallback boundary (spec 0026, AC-1)", async () => { + loadGame.mockResolvedValue({ kind: "found", game: FOUND }); + + const article = await bodyFor("1942"); + const hero = article.props.children as Element; + const tracking = hero.props.tracking as Element; + const slot = tracking.props.children as Element; + + expect(tracking.type).toBe(Suspense); + expect(tracking.props.fallback).toBeNull(); + expect(slot.type).toBe(GameTrackingSlot); + expect(slot.props).toEqual({ + gameId: 1942, + name: FOUND.name, + releaseDate: FOUND.releaseDate, + }); + }); + it("calls notFound for a game IGDB does not know or hides (AC-7)", async () => { loadGame.mockResolvedValue({ kind: "not_found" }); diff --git a/app/games/[id]/page.tsx b/app/games/[id]/page.tsx index 95a8c93e..ac3b3955 100644 --- a/app/games/[id]/page.tsx +++ b/app/games/[id]/page.tsx @@ -6,6 +6,7 @@ import { GameDetailSkeleton } from "@/components/game/game-detail-skeleton"; import { GameHero } from "@/components/game/game-hero"; import { RetryLink } from "@/components/retry-link"; import { StatePanel } from "@/components/state-panel"; +import { GameTrackingSlot } from "@/components/tracking/game-tracking-slot"; import { parseGameId } from "@/lib/catalog/ids"; import { truncateAtWord } from "@/lib/format"; @@ -60,8 +61,9 @@ export async function generateMetadata({ * the boundary. * * Nothing here reads a cookie, a header, the session or the visitor's region - * (AC-9). G4 adds the status control in its own boundary through the hero's - * `tracking` slot, and G9 adds the sections under the hero. + * (AC-9). The status control reads the session in its own Suspense boundary + * through the hero's `tracking` slot (spec 0026, AC-1), and G9 adds the + * sections under the hero. */ export default function GamePage({ params }: PageProps<"/games/[id]">) { return ( @@ -110,6 +112,15 @@ async function GameDetail({ platforms={game.platforms} communityRating={game.communityRating} communityRatingCount={game.communityRatingCount} + tracking={ + + + + } /> ); diff --git a/app/games/actions.test.ts b/app/games/actions.test.ts new file mode 100644 index 00000000..1058f748 --- /dev/null +++ b/app/games/actions.test.ts @@ -0,0 +1,486 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * covers: spec 0026, AC-3 to AC-6, AC-8 to AC-10, AC-15 + * + * The session, IGDB and the database are the boundaries, so those are the + * three things replaced, as in `app/movies/actions.test.ts`. The fake client + * records every call, so each test asserts what would reach PostgREST, and + * that nothing is written on a refusal. + */ +const getOptionalUser = vi.fn(); +vi.mock("@/lib/auth/user", () => ({ getOptionalUser })); + +const readGameIncludingHidden = vi.fn(); +vi.mock("./[id]/load-game", () => ({ readGameIncludingHidden })); + +const refresh = vi.fn(); +vi.mock("next/cache", () => ({ refresh })); + +const calls: { method: string; args: unknown[] }[] = []; +let result: { data: unknown; error: unknown } = { data: null, error: null }; + +function builder() { + const chain: Record = {}; + for (const method of ["from", "update", "eq", "not", "rpc"]) { + chain[method] = (...args: unknown[]) => { + calls.push({ method, args }); + return chain; + }; + } + // biome-ignore lint/suspicious/noThenProperty: stands in for a thenable PostgREST builder. + chain.then = (resolve: (value: unknown) => unknown) => resolve(result); + return chain; +} + +const createClient = vi.fn(async () => ({ + from: (...args: unknown[]) => + (builder().from as (...a: unknown[]) => unknown)(...args), + rpc: (...args: unknown[]) => + (builder().rpc as (...a: unknown[]) => unknown)(...args), +})); +vi.mock("@/lib/supabase/server", () => ({ createClient })); + +const { + removeGame, + restoreGameStatus, + restoreRemovedGame, + setGameRating, + setGameStatus, +} = await import("./actions"); + +const USER = { id: "user-a", email: "a@example.test" }; +const GAME = 1942; +const writes = () => calls.filter((call) => call.method !== "eq"); + +const PREVIOUS = { + status: "finished", + rating: 9, + addedAt: "2026-09-01T10:00:00+00:00", + startedAt: null, + finishedAt: "2026-09-20T10:00:00+00:00", +} as const; + +function released(overrides: Record = {}) { + return { + kind: "found", + game: { id: GAME, releaseDate: "2015-05-19", hidden: false, ...overrides }, + }; +} + +let warn: ReturnType; + +beforeEach(() => { + getOptionalUser.mockResolvedValue(USER); + readGameIncludingHidden.mockResolvedValue(released()); + result = { data: null, error: null }; + calls.length = 0; + warn = vi.spyOn(console, "warn").mockImplementation(() => {}); +}); + +afterEach(() => { + vi.clearAllMocks(); + warn.mockRestore(); +}); + +describe("setGameStatus", () => { + const NO_ROW = { + prev_status: null, + prev_rating: null, + prev_added_at: null, + prev_started_at: null, + prev_finished_at: null, + }; + + it("sets a status through set_game_status, then refreshes (AC-3, AC-10)", async () => { + result = { data: [NO_ROW], error: null }; + expect(await setGameStatus(GAME, "playing")).toEqual({ ok: true }); + expect(writes()).toEqual([ + { + method: "rpc", + args: ["set_game_status", { p_game_id: GAME, p_status: "playing" }], + }, + ]); + expect(refresh).toHaveBeenCalledOnce(); + }); + + it("returns the status Undo when a move to Want to play cleared a score (AC-5)", async () => { + result = { + data: [ + { + prev_status: "finished", + prev_rating: 9, + prev_added_at: PREVIOUS.addedAt, + prev_started_at: null, + prev_finished_at: PREVIOUS.finishedAt, + }, + ], + error: null, + }; + expect(await setGameStatus(GAME, "want_to_play")).toEqual({ + ok: true, + undo: { kind: "status", previous: PREVIOUS }, + }); + }); + + it("offers no Undo when no score was cleared (AC-5)", async () => { + result = { + data: [ + { + prev_status: "playing", + prev_rating: null, + prev_added_at: PREVIOUS.addedAt, + prev_started_at: PREVIOUS.addedAt, + prev_finished_at: null, + }, + ], + error: null, + }; + expect(await setGameStatus(GAME, "want_to_play")).toEqual({ ok: true }); + }); + + it("offers no Undo on a move between Playing and Finished (AC-6)", async () => { + result = { + data: [{ ...NO_ROW, prev_status: "playing", prev_rating: 8 }], + error: null, + }; + expect(await setGameStatus(GAME, "finished")).toEqual({ ok: true }); + }); + + it.each([ + ["a future date", "2999-01-01"], + ["no date", null], + ])( + "refuses Playing and Finished before release, with %s (AC-9)", + async (_, releaseDate) => { + readGameIncludingHidden.mockResolvedValue(released({ releaseDate })); + for (const status of ["playing", "finished"] as const) { + expect(await setGameStatus(GAME, status)).toEqual({ + ok: false, + error: "not_released", + }); + } + expect(createClient).not.toHaveBeenCalled(); + }, + ); + + it("accepts Want to play before release (AC-8)", async () => { + readGameIncludingHidden.mockResolvedValue(released({ releaseDate: null })); + result = { data: [NO_ROW], error: null }; + expect(await setGameStatus(GAME, "want_to_play")).toEqual({ ok: true }); + }); + + it("refuses a hidden game on Playing and Finished, but accepts Want to play (AC-9)", async () => { + readGameIncludingHidden.mockResolvedValue(released({ hidden: true })); + expect(await setGameStatus(GAME, "playing")).toEqual({ + ok: false, + error: "not_found", + }); + expect(await setGameStatus(GAME, "finished")).toEqual({ + ok: false, + error: "not_found", + }); + result = { data: [NO_ROW], error: null }; + expect(await setGameStatus(GAME, "want_to_play")).toEqual({ ok: true }); + }); + + it("refuses an id IGDB does not know (AC-9)", async () => { + readGameIncludingHidden.mockResolvedValue({ kind: "not_found" }); + expect(await setGameStatus(GAME, "want_to_play")).toEqual({ + ok: false, + error: "not_found", + }); + }); + + it("refuses with igdb_unavailable when IGDB fails, Want to play too (AC-9)", async () => { + readGameIncludingHidden.mockResolvedValue({ kind: "failed" }); + expect(await setGameStatus(GAME, "want_to_play")).toEqual({ + ok: false, + error: "igdb_unavailable", + }); + expect(createClient).not.toHaveBeenCalled(); + }); + + it("refuses a status outside the three as invalid_input (AC-10)", async () => { + expect( + await setGameStatus(GAME, "dropped" as unknown as "playing"), + ).toEqual({ ok: false, error: "invalid_input" }); + expect(getOptionalUser).not.toHaveBeenCalled(); + }); + + it("logs a refusal with its event and outcome only (AC-15)", async () => { + readGameIncludingHidden.mockResolvedValue(released({ releaseDate: null })); + await setGameStatus(GAME, "playing"); + expect(warn).toHaveBeenCalledWith( + "game_tracking.status refused not_released", + ); + }); +}); + +describe("setGameRating", () => { + it("scores through rate_game (AC-6)", async () => { + expect(await setGameRating(GAME, 8)).toEqual({ ok: true }); + expect(writes()).toEqual([ + { method: "rpc", args: ["rate_game", { p_game_id: GAME, p_rating: 8 }] }, + ]); + }); + + it("clears with a plain update that never calls IGDB (AC-6, AC-9)", async () => { + readGameIncludingHidden.mockResolvedValue({ kind: "failed" }); + expect(await setGameRating(GAME, null)).toEqual({ ok: true }); + expect(writes()).toEqual([ + { method: "from", args: ["user_game_state"] }, + { method: "update", args: [{ rating: null }] }, + // Only a scored row, so an unscored one keeps its Undo window. + { method: "not", args: ["rating", "is", null] }, + ]); + expect(readGameIncludingHidden).not.toHaveBeenCalled(); + }); + + it("refuses a score before release (AC-9)", async () => { + readGameIncludingHidden.mockResolvedValue( + released({ releaseDate: "2999-01-01" }), + ); + expect(await setGameRating(GAME, 8)).toEqual({ + ok: false, + error: "not_released", + }); + }); + + it("maps BS004 to not_started (AC-6, AC-15)", async () => { + result = { data: null, error: { code: "BS004" } }; + expect(await setGameRating(GAME, 8)).toEqual({ + ok: false, + error: "not_started", + }); + expect(refresh).not.toHaveBeenCalled(); + }); + + it("maps BS003 to write_failed, logged as identity_locked (AC-15)", async () => { + result = { data: null, error: { code: "BS003" } }; + expect(await setGameRating(GAME, 8)).toEqual({ + ok: false, + error: "write_failed", + }); + expect(warn).toHaveBeenCalledWith( + "game_tracking.rate refused identity_locked", + ); + }); + + it("refuses a score of 11 as invalid_input", async () => { + expect(await setGameRating(GAME, 11)).toEqual({ + ok: false, + error: "invalid_input", + }); + }); +}); + +describe("removeGame", () => { + it("returns the removal Undo when a row came back, with no IGDB call (AC-4, AC-9)", async () => { + readGameIncludingHidden.mockResolvedValue({ kind: "failed" }); + result = { + data: [ + { + removed_status: "finished", + removed_rating: 9, + removed_added_at: PREVIOUS.addedAt, + removed_started_at: null, + removed_finished_at: PREVIOUS.finishedAt, + removed_at: "2026-10-24T10:00:00+00:00", + }, + ], + error: null, + }; + expect(await removeGame(GAME)).toEqual({ + ok: true, + undo: { + kind: "removed", + previous: PREVIOUS, + removedAt: "2026-10-24T10:00:00+00:00", + }, + }); + expect(readGameIncludingHidden).not.toHaveBeenCalled(); + }); + + it("is a quiet success with no Undo when the row was already gone (AC-4)", async () => { + result = { data: [], error: null }; + expect(await removeGame(GAME)).toEqual({ ok: true }); + }); +}); + +describe("the restores", () => { + it("restores a status with the old values and no IGDB call (AC-5, AC-9)", async () => { + readGameIncludingHidden.mockResolvedValue({ kind: "failed" }); + expect(await restoreGameStatus(GAME, PREVIOUS)).toEqual({ ok: true }); + expect(writes()).toEqual([ + { + method: "rpc", + args: [ + "restore_game_status", + { + p_game_id: GAME, + p_status: "finished", + p_added_at: PREVIOUS.addedAt, + p_rating: 9, + p_started_at: undefined, + p_finished_at: PREVIOUS.finishedAt, + }, + ], + }, + ]); + expect(readGameIncludingHidden).not.toHaveBeenCalled(); + }); + + it("restores a removal with its time of removal (AC-4)", async () => { + expect( + await restoreRemovedGame(GAME, PREVIOUS, "2026-10-24T10:00:00+00:00"), + ).toEqual({ ok: true }); + expect(writes()[0]).toMatchObject({ + method: "rpc", + args: [ + "restore_removed_game", + { p_game_id: GAME, p_removed_at: "2026-10-24T10:00:00+00:00" }, + ], + }); + }); + + it("maps P0002 to undo_expired (AC-14)", async () => { + result = { data: null, error: { code: "P0002" } }; + expect(await restoreGameStatus(GAME, PREVIOUS)).toEqual({ + ok: false, + error: "undo_expired", + }); + }); + + it.each([ + ["Playing with no start", { status: "playing", startedAt: null }], + ["Finished with no finish", { status: "finished", finishedAt: null }], + ["Want to play with a score", { status: "want_to_play", rating: 7 }], + ])( + "refuses %s as invalid_input before any call (AC-14)", + async (_, change) => { + expect( + await restoreGameStatus(GAME, { + ...PREVIOUS, + ...change, + } as typeof PREVIOUS), + ).toEqual({ ok: false, error: "invalid_input" }); + expect(getOptionalUser).not.toHaveBeenCalled(); + }, + ); +}); + +describe("input and failure edges", () => { + it("refuses game id 0 as invalid_input, with nothing read or written (value source: gameIdSchema)", async () => { + expect(await setGameStatus(0, "playing")).toEqual({ + ok: false, + error: "invalid_input", + }); + expect(getOptionalUser).not.toHaveBeenCalled(); + expect(readGameIncludingHidden).not.toHaveBeenCalled(); + expect(createClient).not.toHaveBeenCalled(); + }); + + it.each([-1, 1.5, 2147483648])( + "refuses the game id %s on every action (AC-15)", + async (id) => { + const refused = { ok: false, error: "invalid_input" }; + expect(await setGameRating(id, 5)).toEqual(refused); + expect(await removeGame(id)).toEqual(refused); + expect(await restoreGameStatus(id, PREVIOUS)).toEqual(refused); + expect(createClient).not.toHaveBeenCalled(); + }, + ); + + it("returns session_expired with no IGDB read and no write when signed out (AC-10)", async () => { + getOptionalUser.mockResolvedValue(null); + expect(await setGameStatus(GAME, "playing")).toEqual({ + ok: false, + error: "session_expired", + }); + expect(readGameIncludingHidden).not.toHaveBeenCalled(); + expect(createClient).not.toHaveBeenCalled(); + expect(refresh).not.toHaveBeenCalled(); + }); + + it("turns a thrown network failure into write_failed, with no refresh and no message in the log (AC-15)", async () => { + createClient.mockRejectedValueOnce(new Error("fetch failed: token=abc")); + expect(await removeGame(GAME)).toEqual({ + ok: false, + error: "write_failed", + }); + expect(refresh).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith("game_tracking.remove refused db_error"); + expect(JSON.stringify(warn.mock.calls)).not.toContain("token=abc"); + }); + + it("does not refresh after a refused write (AC-10)", async () => { + result = { data: null, error: { code: "XX000" } }; + expect(await setGameStatus(GAME, "playing")).toEqual({ + ok: false, + error: "write_failed", + }); + expect(refresh).not.toHaveBeenCalled(); + }); +}); + +describe("the server release gate by UTC day (AC-9)", () => { + beforeEach(() => { + // Late evening UTC, when a local clock east of UTC is already on the 25th. + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date("2026-10-24T23:30:00Z")); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("accepts Playing and a score on the release day itself", async () => { + readGameIncludingHidden.mockResolvedValue( + released({ releaseDate: "2026-10-24" }), + ); + result = { data: [], error: null }; + expect(await setGameStatus(GAME, "playing")).toEqual({ ok: true }); + expect(await setGameRating(GAME, 7)).toEqual({ ok: true }); + }); + + it("refuses Playing, Finished and a score for a game out tomorrow", async () => { + readGameIncludingHidden.mockResolvedValue( + released({ releaseDate: "2026-10-25" }), + ); + const refused = { ok: false, error: "not_released" }; + expect(await setGameStatus(GAME, "playing")).toEqual(refused); + expect(await setGameStatus(GAME, "finished")).toEqual(refused); + expect(await setGameRating(GAME, 7)).toEqual(refused); + expect(createClient).not.toHaveBeenCalled(); + }); + + it("still removes, clears a score and restores before release, with no IGDB read (AC-8)", async () => { + readGameIncludingHidden.mockResolvedValue(released({ releaseDate: null })); + expect(await removeGame(GAME)).toEqual({ ok: true }); + expect(await setGameRating(GAME, null)).toEqual({ ok: true }); + expect(await restoreGameStatus(GAME, PREVIOUS)).toEqual({ ok: true }); + expect(readGameIncludingHidden).not.toHaveBeenCalled(); + }); +}); + +describe("restoreRemovedGame refusals (AC-14)", () => { + it("maps P0002, a removal older than 10 minutes, to undo_expired", async () => { + result = { data: null, error: { code: "P0002" } }; + expect( + await restoreRemovedGame(GAME, PREVIOUS, "2026-10-24T09:00:00+00:00"), + ).toEqual({ ok: false, error: "undo_expired" }); + expect(refresh).not.toHaveBeenCalled(); + }); + + it.each(["yesterday", "2026-10-24", "2026-10-24T10:00:00"])( + "refuses the removal time %j as invalid_input before any call", + async (removedAt) => { + expect(await restoreRemovedGame(GAME, PREVIOUS, removedAt)).toEqual({ + ok: false, + error: "invalid_input", + }); + expect(getOptionalUser).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/app/games/actions.ts b/app/games/actions.ts new file mode 100644 index 00000000..dc60d264 --- /dev/null +++ b/app/games/actions.ts @@ -0,0 +1,330 @@ +"use server"; + +import { isGameReleased } from "@/lib/catalog/game-page"; +import { TRACKING_EVENT, type TrackingEvent } from "@/lib/tracking/log"; +import { + gameRatingInputSchema, + gameStatusInputSchema, + removeGameInputSchema, + restoreGameStatusInputSchema, + restoreRemovedGameInputSchema, +} from "@/lib/tracking/schemas"; +import { classifyGameTrackingError } from "@/lib/tracking/supabase-error"; +import type { + GameRestoreState, + GameStatus, + GameTrackingError, + GameTrackingResult, + GameUndo, +} from "@/lib/tracking/types"; +import { + parseTrackingInput, + runTrackingWrite, + type WriteStep, +} from "@/lib/tracking/write"; +import { todayUtc } from "@/lib/tv/air-status"; + +import { readGameIncludingHidden } from "./[id]/load-game"; + +/** + * The game status and rating mutations, as Server Actions (spec 0026, API + * surface), sharing `runTrackingWrite` with `app/movies/actions.ts`. + * + * Each takes a target value, never a toggle, so the same call twice gives the + * same row (AC-3). Each runs the same order: Zod parse, then the verified + * session, then the IGDB check where a write could move a game forward, then + * the SQL call through Row Level Security, then `refresh()` on success only + * (AC-10). The release gate lives here and not in the database (spec 0024, + * security model). + * + * None of them throws or redirects: every failure comes back as a + * `GameTrackingResult`, so it reaches a toast rather than an error boundary. + * `user_id` is always `auth.uid()` inside the SQL, never an argument + * (`AGENTS.md` section 5). + */ + +/** + * What a write confirms with IGDB first (AC-9). `none` for a removal, a + * cleared score and both restores, which only ever put back or take away and + * so must work during an outage, for a hidden game and before release. + * `known` for Want to play: any id IGDB has, hidden included, so a listed + * game IGDB later hides can still be moved back. `listed` for Playing and + * Finished: known, not hidden, and released. `released` for a score: known + * and released. A hidden game can still be scored: `rate_game` needs Playing + * or Finished, which the game could only have reached while it was listed. + */ +type IgdbCheck = "none" | "known" | "listed" | "released"; + +/** + * The game side of `runTrackingWrite`: the IGDB check and release gate, and + * the Undo that comes back. + * + * @param event The log event for a refusal. + * @param gameId The already parsed game id. + * @param check What to confirm with IGDB before writing. + * @param write The Supabase call, given the request client and the user id. + * @param readUndo For a write an Undo can take back, reads it from the data. + */ +async function runGameWrite( + event: TrackingEvent, + gameId: number, + check: IgdbCheck, + write: WriteStep, + readUndo?: (data: D | null) => GameUndo | undefined, +): Promise { + const outcome = await runTrackingWrite({ + event, + check: check === "none" ? undefined : () => checkIgdb(gameId, check), + write, + classify: classifyGameTrackingError, + read: readUndo, + }); + if (!outcome.ok) return outcome; + return outcome.value ? { ok: true, undo: outcome.value } : { ok: true }; +} + +/** Confirms the game with IGDB, and where asked, that it is out. */ +async function checkIgdb( + gameId: number, + check: Exclude, +): Promise { + const result = await readGameIncludingHidden(gameId); + if (result.kind === "failed") return "igdb_unavailable"; + if ( + result.kind === "not_found" || + (check === "listed" && result.game.hidden) + ) { + return "not_found"; + } + if ( + check !== "known" && + !isGameReleased(result.game.releaseDate, todayUtc(new Date())) + ) { + return "not_released"; + } + return null; +} + +/** + * The five columns `set_game_status` (`prev_*`) and `remove_game` + * (`removed_*`) both return, under their prefix. Every column is nullable + * here although the generated row marks them not null: `prev_*` are all null + * when there was no row. Typed from the prefix rather than a hand written + * shape, so a renamed SQL column is still a compile error at the call. + */ +type RestoreColumns

= Record< + `${P}status`, + GameStatus | null +> & + Record<`${P}rating`, number | null> & + Record<`${P}added_at` | `${P}started_at` | `${P}finished_at`, string | null>; + +/** + * Reads the state an Undo puts back from a write's prefixed columns, or null + * when the row held no game (no status or no added date). + */ +function readRestoreState

( + row: RestoreColumns

, + prefix: P, +): GameRestoreState | null { + const status: GameStatus | null = row[`${prefix}status`]; + const addedAt: string | null = row[`${prefix}added_at`]; + if (status === null || addedAt === null) return null; + return { + status, + addedAt, + rating: row[`${prefix}rating`], + startedAt: row[`${prefix}started_at`], + finishedAt: row[`${prefix}finished_at`], + }; +} + +/** The restore arguments both Undo functions share. */ +function restoreArgs(previous: GameRestoreState) { + return { + p_status: previous.status, + p_added_at: previous.addedAt, + p_rating: previous.rating ?? undefined, + p_started_at: previous.startedAt ?? undefined, + p_finished_at: previous.finishedAt ?? undefined, + }; +} + +/** + * Sets a game's status, from any status or none (AC-3). A move to Want to + * play clears the score in the same statement, and when it did, the result + * carries the old status, score and dates as the `status` Undo (AC-5). + * + * Playing and Finished are forward moves, so they wait for the release date + * and refuse a hidden game; Want to play only needs IGDB to know the id + * (AC-9). `set_game_status` reads the previous values under a row lock, so + * two tabs can never both receive the same Undo (AC-13). + * + * The generated row type marks every `prev_*` column not null, but all of + * them are null when there was no row, so `readRestoreState` treats them as + * nullable. + */ +export async function setGameStatus( + gameId: number, + status: GameStatus, +): Promise { + const event = TRACKING_EVENT.gameStatus; + const input = parseTrackingInput( + gameStatusInputSchema, + { gameId, status }, + event, + ); + if (!input) return { ok: false, error: "invalid_input" }; + + return runGameWrite( + event, + input.gameId, + input.status === "want_to_play" ? "known" : "listed", + (supabase) => + supabase.rpc("set_game_status", { + p_game_id: input.gameId, + p_status: input.status, + }), + (data) => { + const row = data?.[0]; + const previous = row ? readRestoreState(row, "prev_") : null; + if ( + input.status !== "want_to_play" || + !previous || + previous.status === "want_to_play" || + previous.rating === null + ) { + return undefined; + } + return { kind: "status", previous }; + }, + ); +} + +/** + * Stores a score from 1 to 10, or clears it (AC-6). A score waits for the + * release date; clearing never does, so a score stored before release can + * always be removed (AC-8, AC-9). + * + * A score goes through `rate_game`, which writes only on Playing or Finished + * and otherwise refuses with `not_started`. Clearing is a plain update that + * touches only `rating`, so no date moves; on a row another tab removed it + * matches nothing and succeeds, and `refresh()` shows the truth. It matches + * only a scored row, because any update moves `updated_at`, and + * `restore_game_status` measures its 10 minute window from that: a stale + * tab clearing an unscored Want to play row would otherwise extend an old + * Undo. + */ +export async function setGameRating( + gameId: number, + rating: number | null, +): Promise { + const event = TRACKING_EVENT.gameRate; + const input = parseTrackingInput( + gameRatingInputSchema, + { gameId, rating }, + event, + ); + if (!input) return { ok: false, error: "invalid_input" }; + + const score = input.rating; + if (score !== null) { + return runGameWrite(event, input.gameId, "released", (supabase) => + supabase.rpc("rate_game", { p_game_id: input.gameId, p_rating: score }), + ); + } + + return runGameWrite(event, input.gameId, "none", (supabase, userId) => + supabase + .from("user_game_state") + .update({ rating: null }) + .eq("user_id", userId) + .eq("game_id", input.gameId) + .not("rating", "is", null), + ); +} + +/** + * Removes a game from the list (AC-4). The result carries what the row held + * as the `removed` Undo; a game another tab already removed comes back with + * no Undo, a quiet success. Never calls IGDB (AC-9). + * + * As with `set_game_status`, the generated row type marks the nullable + * columns not null, so `readRestoreState` widens them. + */ +export async function removeGame(gameId: number): Promise { + const event = TRACKING_EVENT.gameRemove; + const input = parseTrackingInput(removeGameInputSchema, { gameId }, event); + if (!input) return { ok: false, error: "invalid_input" }; + + return runGameWrite( + event, + input.gameId, + "none", + (supabase) => supabase.rpc("remove_game", { p_game_id: input.gameId }), + (data) => { + const row = data?.[0]; + const previous = row ? readRestoreState(row, "removed_") : null; + if (!row || !previous) return undefined; + return { kind: "removed", removedAt: row.removed_at, previous }; + }, + ); +} + +/** + * The Undo of a move to Want to play that cleared a score (AC-5): puts back + * the old status, score and all three dates. The values are the caller's own, + * which `setGameStatus` returned; `restore_game_status` accepts them only on a + * row still Want to play with no score, changed in the last 10 minutes, with + * every date in the past, and otherwise refuses as `undo_expired` (AC-14). + * + * No IGDB check: a restore only puts back a state the user already had, so + * it works during an outage, for a hidden game and before release (AC-9). + */ +export async function restoreGameStatus( + gameId: number, + previous: GameRestoreState, +): Promise { + const event = TRACKING_EVENT.gameRestoreStatus; + const input = parseTrackingInput( + restoreGameStatusInputSchema, + { gameId, previous }, + event, + ); + if (!input) return { ok: false, error: "invalid_input" }; + + return runGameWrite(event, input.gameId, "none", (supabase) => + supabase.rpc("restore_game_status", { + p_game_id: input.gameId, + ...restoreArgs(input.previous), + }), + ); +} + +/** + * The Undo of a removal (AC-4): puts the row back exactly as `remove_game` + * returned it. `restore_removed_game` accepts it only when no row exists, + * within 10 minutes of `removedAt`, with every date in the past (AC-14). No + * IGDB check, for the same reason as `restoreGameStatus`. + */ +export async function restoreRemovedGame( + gameId: number, + previous: GameRestoreState, + removedAt: string, +): Promise { + const event = TRACKING_EVENT.gameRestoreRemoved; + const input = parseTrackingInput( + restoreRemovedGameInputSchema, + { gameId, previous, removedAt }, + event, + ); + if (!input) return { ok: false, error: "invalid_input" }; + + return runGameWrite(event, input.gameId, "none", (supabase) => + supabase.rpc("restore_removed_game", { + p_game_id: input.gameId, + p_removed_at: input.removedAt, + ...restoreArgs(input.previous), + }), + ); +} diff --git a/app/movies/actions.ts b/app/movies/actions.ts index 78feffef..14944f3e 100644 --- a/app/movies/actions.ts +++ b/app/movies/actions.ts @@ -1,16 +1,7 @@ "use server"; -import { refresh } from "next/cache"; -import type { z } from "zod"; - -import { getOptionalUser } from "@/lib/auth/user"; import { isMovieReleased } from "@/lib/catalog/movie-page"; -import { createClient } from "@/lib/supabase/server"; -import { - logTrackingEvent, - TRACKING_EVENT, - type TrackingEvent, -} from "@/lib/tracking/log"; +import { TRACKING_EVENT, type TrackingEvent } from "@/lib/tracking/log"; import { ratingInputSchema, restoreWatchedInputSchema, @@ -21,8 +12,14 @@ import { import { classifyMovieTrackingError } from "@/lib/tracking/supabase-error"; import type { MovieClearedState, + MovieTrackingError, MovieTrackingResult, } from "@/lib/tracking/types"; +import { + parseTrackingInput, + runTrackingWrite, + type WriteStep, +} from "@/lib/tracking/write"; import { todayUtc } from "@/lib/tv/air-status"; import { loadMovie } from "./[id]/load-movie"; @@ -45,18 +42,6 @@ import { loadMovie } from "./[id]/load-movie"; * AC-18). */ -type SupabaseClient = Awaited>; - -/** - * What a write step returns: PostgREST's typed row, and its error or null on - * success. `D` is the generated return type of the call, so the reader that - * consumes it is checked against `database.types.ts`. - */ -type WriteStep = ( - supabase: SupabaseClient, - userId: string, -) => PromiseLike<{ data: D | null; error: { code?: string | null } | null }>; - /** * Reads what `plan_movie` or `unmark_movie_watched` cleared. Both return the * old watch mark and score, null when nothing was watched; only a removed @@ -86,7 +71,8 @@ function readCleared( type TmdbCheck = "none" | "exists" | "released"; /** - * The shared shape of every tracking action. + * The movie side of `runTrackingWrite`: the TMDB check and release gate, and + * the cleared watch mark that comes back for an Undo. * * @param event The log event for a refusal. * @param movieId The already parsed movie id. @@ -94,67 +80,39 @@ type TmdbCheck = "none" | "exists" | "released"; * @param write The Supabase call, given the request client and the user id. * @param readResult For a write whose row says what it cleared, reads it. */ -async function runTrackingWrite( +async function runMovieWrite( event: TrackingEvent, movieId: number, check: TmdbCheck, write: WriteStep, readResult?: (data: D | null) => MovieClearedState | undefined, ): Promise { - let cleared: MovieClearedState | undefined; - try { - const user = await getOptionalUser(); - if (!user) { - logTrackingEvent(event, "session_expired"); - return { ok: false, error: "session_expired" }; - } - - if (check !== "none") { - const movie = await loadMovie(movieId); - if (movie.kind !== "found") { - const error = - movie.kind === "failed" ? "tmdb_unavailable" : "not_found"; - logTrackingEvent(event, error); - return { ok: false, error }; - } - if ( - check === "released" && - !isMovieReleased(movie.movie.releaseDate, todayUtc(new Date())) - ) { - logTrackingEvent(event, "not_released"); - return { ok: false, error: "not_released" }; - } - } - - const supabase = await createClient(); - const { data, error } = await write(supabase, user.id); - if (error) { - const classified = classifyMovieTrackingError(error); - logTrackingEvent(event, classified.outcome); - return { ok: false, error: classified.error }; - } - cleared = readResult?.(data); - } catch { - // A network failure inside supabase-js or a bug surfaced by `loadMovie`. - // The error object is dropped on purpose: its message can carry request - // details that must not reach a log line (AC-21). - logTrackingEvent(event, "db_error"); - return { ok: false, error: "write_failed" }; - } - - refresh(); - return cleared ? { ok: true, cleared } : { ok: true }; + const outcome = await runTrackingWrite({ + event, + check: check === "none" ? undefined : () => checkTmdb(movieId, check), + write, + classify: classifyMovieTrackingError, + read: readResult, + }); + if (!outcome.ok) return outcome; + return outcome.value ? { ok: true, cleared: outcome.value } : { ok: true }; } -/** Parses an action's arguments, or reports `invalid_input` (AC-13). */ -function parse( - schema: z.ZodType, - input: unknown, - event: TrackingEvent, -): T | null { - const parsed = schema.safeParse(input); - if (parsed.success) return parsed.data; - logTrackingEvent(event, "invalid_input"); +/** Confirms the movie with TMDB, and for `released` its release date. */ +async function checkTmdb( + movieId: number, + check: Exclude, +): Promise { + const movie = await loadMovie(movieId); + if (movie.kind !== "found") { + return movie.kind === "failed" ? "tmdb_unavailable" : "not_found"; + } + if ( + check === "released" && + !isMovieReleased(movie.movie.releaseDate, todayUtc(new Date())) + ) { + return "not_released"; + } return null; } @@ -173,11 +131,15 @@ export async function setMovieWatchlist( inWatchlist: boolean, ): Promise { const event = TRACKING_EVENT.watchlist; - const input = parse(watchlistInputSchema, { movieId, inWatchlist }, event); + const input = parseTrackingInput( + watchlistInputSchema, + { movieId, inWatchlist }, + event, + ); if (!input) return { ok: false, error: "invalid_input" }; if (input.inWatchlist) { - return runTrackingWrite( + return runMovieWrite( event, input.movieId, "exists", @@ -187,7 +149,7 @@ export async function setMovieWatchlist( ); } - return runTrackingWrite(event, input.movieId, "none", (supabase, userId) => + return runMovieWrite(event, input.movieId, "none", (supabase, userId) => supabase .from("user_movie_state") .update({ in_watchlist: false }) @@ -212,16 +174,20 @@ export async function setMovieWatched( watched: boolean, ): Promise { const event = TRACKING_EVENT.watched; - const input = parse(watchedInputSchema, { movieId, watched }, event); + const input = parseTrackingInput( + watchedInputSchema, + { movieId, watched }, + event, + ); if (!input) return { ok: false, error: "invalid_input" }; if (input.watched) { - return runTrackingWrite(event, input.movieId, "released", (supabase) => + return runMovieWrite(event, input.movieId, "released", (supabase) => supabase.rpc("mark_movie_watched", { p_movie_id: input.movieId }), ); } - return runTrackingWrite( + return runMovieWrite( event, input.movieId, "none", @@ -248,12 +214,16 @@ export async function setMovieRating( rating: number | null, ): Promise { const event = TRACKING_EVENT.rate; - const input = parse(ratingInputSchema, { movieId, rating }, event); + const input = parseTrackingInput( + ratingInputSchema, + { movieId, rating }, + event, + ); if (!input) return { ok: false, error: "invalid_input" }; const score = input.rating; if (score !== null) { - return runTrackingWrite(event, input.movieId, "released", (supabase) => + return runMovieWrite(event, input.movieId, "released", (supabase) => supabase.rpc("rate_movie", { p_movie_id: input.movieId, p_rating: score, @@ -261,7 +231,7 @@ export async function setMovieRating( ); } - return runTrackingWrite(event, input.movieId, "none", (supabase, userId) => + return runMovieWrite(event, input.movieId, "none", (supabase, userId) => supabase .from("user_movie_state") .update({ rating: null }) @@ -284,10 +254,14 @@ export async function restoreMovieWatchlist( movieId: number, ): Promise { const event = TRACKING_EVENT.restoreWatchlist; - const input = parse(restoreWatchlistInputSchema, { movieId }, event); + const input = parseTrackingInput( + restoreWatchlistInputSchema, + { movieId }, + event, + ); if (!input) return { ok: false, error: "invalid_input" }; - return runTrackingWrite(event, input.movieId, "none", (supabase) => + return runMovieWrite(event, input.movieId, "none", (supabase) => supabase.rpc("restore_movie_watchlist", { p_movie_id: input.movieId }), ); } @@ -314,14 +288,14 @@ export async function restoreMovieWatched( rating: number | null, ): Promise { const event = TRACKING_EVENT.restoreWatched; - const input = parse( + const input = parseTrackingInput( restoreWatchedInputSchema, { movieId, watchedAt, rating }, event, ); if (!input) return { ok: false, error: "invalid_input" }; - return runTrackingWrite(event, input.movieId, "released", (supabase) => + return runMovieWrite(event, input.movieId, "released", (supabase) => supabase.rpc("restore_movie_watched", { p_movie_id: input.movieId, p_watched_at: input.watchedAt, diff --git a/components/tracking/game-status-control.test.tsx b/components/tracking/game-status-control.test.tsx new file mode 100644 index 00000000..4819dee0 --- /dev/null +++ b/components/tracking/game-status-control.test.tsx @@ -0,0 +1,346 @@ +import { act, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { GameTrackingState } from "@/lib/tracking/types"; + +/** + * covers: spec 0026, AC-2 to AC-8, AC-11 + * + * The Server Actions, the router and Sonner are the boundaries. Each action + * is a deferred promise the test settles by hand, so the optimistic state is + * asserted while the write is in flight and again once it settles. + */ +type Deferred = { resolve: (value: unknown) => void }; +const pending: Deferred[] = []; +const deferredAction = vi.fn( + (..._args: unknown[]) => + new Promise((resolve) => { + pending.push({ resolve }); + }), +); + +vi.mock("@/app/games/actions", () => ({ + setGameStatus: (...args: unknown[]) => deferredAction("status", ...args), + setGameRating: (...args: unknown[]) => deferredAction("rating", ...args), + removeGame: (...args: unknown[]) => deferredAction("remove", ...args), + restoreGameStatus: (...args: unknown[]) => + deferredAction("restoreStatus", ...args), + restoreRemovedGame: (...args: unknown[]) => + deferredAction("restoreRemoved", ...args), +})); + +const push = vi.fn(); +vi.mock("next/navigation", () => ({ useRouter: () => ({ push }) })); + +const toast = vi.fn(); +const dismiss = vi.fn(); +vi.mock("sonner", () => ({ + toast: Object.assign((...args: unknown[]) => toast(...args), { dismiss }), +})); + +const { GameStatusControl } = await import("./game-status-control"); + +const NONE: GameTrackingState = { status: null, rating: null }; +const UNRELEASED = "Releases Oct 24, 2027"; + +function renderControl( + state: GameTrackingState = NONE, + releaseNote: string | null = null, +) { + return render( + , + ); +} + +const pill = (label: string) => + screen.getByRole("button", { name: `${label}: The Witcher 3` }); +const score = () => + screen.queryByRole("button", { name: /^Your score for The Witcher 3/ }); + +async function settle(index: number, value: unknown) { + await act(async () => pending[index].resolve(value)); +} + +beforeEach(() => { + pending.length = 0; +}); + +afterEach(async () => { + await act(async () => { + for (const write of pending) write.resolve({ ok: true }); + }); + vi.clearAllMocks(); +}); + +describe("GameStatusControl", () => { + it("renders three toggles in a labelled group, the stored one pressed (AC-2)", () => { + renderControl({ status: "playing", rating: 8 }); + expect( + screen.getByRole("group", { name: "Status for The Witcher 3" }), + ).toBeInTheDocument(); + expect(pill("Want to play")).toHaveAttribute("aria-pressed", "false"); + expect(pill("Playing")).toHaveAttribute("aria-pressed", "true"); + expect(pill("Finished")).toHaveAttribute("aria-pressed", "false"); + expect(score()).toHaveAccessibleName("Your score for The Witcher 3: 8"); + }); + + it("shows no score pill with no status or on Want to play (AC-2)", () => { + renderControl({ status: "want_to_play", rating: null }); + expect(score()).toBeNull(); + }); + + it("reads Rate with no score, with the Not rated name (AC-6)", () => { + renderControl({ status: "playing", rating: null }); + expect(score()).toHaveTextContent("Rate"); + expect(score()).toHaveAccessibleName( + "Your score for The Witcher 3: Not rated", + ); + }); + + it("sets a status at once and sends the target (AC-3, AC-11)", async () => { + const user = userEvent.setup(); + renderControl(); + await user.click(pill("Playing")); + expect(pill("Playing")).toHaveAttribute("aria-pressed", "true"); + expect(deferredAction).toHaveBeenCalledWith("status", 1942, "playing"); + }); + + it("removes through the pressed pill and offers Undo (AC-4)", async () => { + const user = userEvent.setup(); + renderControl({ status: "want_to_play", rating: null }); + await user.click(pill("Want to play")); + expect(deferredAction).toHaveBeenCalledWith("remove", 1942); + expect(pill("Want to play")).toHaveAttribute("aria-pressed", "false"); + await settle(0, { + ok: true, + undo: { + kind: "removed", + removedAt: "2026-10-24T10:00:00+00:00", + previous: { + status: "want_to_play", + rating: null, + addedAt: "2026-09-01T10:00:00+00:00", + startedAt: null, + finishedAt: null, + }, + }, + }); + expect(toast).toHaveBeenCalledWith( + "Removed from your games.", + expect.objectContaining({ id: "game-undo-1942" }), + ); + }); + + it("shows no toast for a removal that found nothing (AC-4)", async () => { + const user = userEvent.setup(); + renderControl({ status: "playing", rating: null }); + await user.click(pill("Playing")); + await settle(0, { ok: true }); + expect(toast).not.toHaveBeenCalled(); + }); + + it("toasts the score cleared Undo on a move to Want to play (AC-5)", async () => { + const user = userEvent.setup(); + renderControl({ status: "finished", rating: 9 }); + await user.click(pill("Want to play")); + expect(score()).toBeNull(); + await settle(0, { + ok: true, + undo: { + kind: "status", + previous: { + status: "finished", + rating: 9, + addedAt: "2026-09-01T10:00:00+00:00", + startedAt: null, + finishedAt: "2026-09-20T10:00:00+00:00", + }, + }, + }); + expect(toast).toHaveBeenCalledWith( + "Moved to Want to play. Your score was removed.", + expect.objectContaining({ id: "game-undo-1942" }), + ); + }); + + it("opens the picker on a move into unscored Finished, not into Playing (AC-7)", async () => { + const user = userEvent.setup(); + renderControl({ status: "want_to_play", rating: null }); + await user.click(pill("Playing")); + expect(screen.queryByRole("radiogroup")).toBeNull(); + await settle(0, { ok: true }); + await user.click(pill("Finished")); + expect( + await screen.findByRole("radiogroup", { name: "Your score" }), + ).toBeInTheDocument(); + }); + + it("does not open the picker on a move into Finished with a score (AC-7)", async () => { + const user = userEvent.setup(); + renderControl({ status: "playing", rating: 8 }); + await user.click(pill("Finished")); + expect(screen.queryByRole("radiogroup")).toBeNull(); + }); + + it("closes the picker and rolls back when the Finished write fails (AC-7, AC-11)", async () => { + const user = userEvent.setup(); + renderControl(); + await user.click(pill("Finished")); + expect(await screen.findByRole("radiogroup")).toBeInTheDocument(); + await settle(0, { ok: false, error: "write_failed" }); + expect(screen.queryByRole("radiogroup")).toBeNull(); + expect(pill("Finished")).toHaveAttribute("aria-pressed", "false"); + expect(toast).toHaveBeenCalledWith( + "Couldn't save that change. Try again.", + expect.objectContaining({ id: "game-tracking-1942-status" }), + ); + }); + + it("toasts only the status error when a queued score fails behind it (AC-11)", async () => { + const user = userEvent.setup(); + renderControl(); + await user.click(pill("Finished")); + await user.click(await screen.findByRole("radio", { name: "8" })); + expect(deferredAction).toHaveBeenCalledWith("rating", 1942, 8); + await settle(0, { ok: false, error: "igdb_unavailable" }); + await settle(1, { ok: false, error: "not_started" }); + expect(toast).toHaveBeenCalledTimes(1); + expect(toast).toHaveBeenCalledWith( + "Couldn't reach IGDB. Try again.", + expect.anything(), + ); + expect(pill("Finished")).toHaveAttribute("aria-pressed", "false"); + }); + + it("holds back Playing and Finished before release, with the line (AC-8)", async () => { + const user = userEvent.setup(); + renderControl(NONE, UNRELEASED); + expect(screen.getByText(UNRELEASED)).toBeInTheDocument(); + expect(pill("Playing")).toHaveAttribute("aria-disabled", "true"); + expect(pill("Finished")).toHaveAttribute("aria-disabled", "true"); + expect(pill("Want to play")).not.toHaveAttribute("aria-disabled"); + await user.click(pill("Playing")); + expect(deferredAction).not.toHaveBeenCalled(); + }); + + it("lets a stored Playing game be removed or moved back before release (AC-8)", async () => { + const user = userEvent.setup(); + renderControl({ status: "playing", rating: 6 }, UNRELEASED); + expect(pill("Playing")).not.toHaveAttribute("aria-disabled"); + expect(pill("Finished")).toHaveAttribute("aria-disabled", "true"); + expect(score()).not.toBeNull(); + await user.click(score() as HTMLElement); + expect( + await screen.findByText(UNRELEASED, { selector: "p[id]" }), + ).toBeInTheDocument(); + expect(screen.getByRole("radio", { name: "8" })).toHaveAttribute( + "aria-disabled", + "true", + ); + }); + + it("carries Sign in on a session expired toast (AC-10)", async () => { + const user = userEvent.setup(); + renderControl(); + await user.click(pill("Want to play")); + await settle(0, { ok: false, error: "session_expired" }); + const options = toast.mock.calls[0][1] as { + action: { label: string; onClick: () => void }; + }; + expect(options.action.label).toBe("Sign in"); + options.action.onClick(); + expect(push).toHaveBeenCalledWith("/sign-in?next=%2Fgames%2F1942"); + }); + describe("the Undo in the toast (AC-4, AC-5, AC-14)", () => { + const REMOVED = { + kind: "removed", + removedAt: "2026-10-24T10:00:00+00:00", + previous: { + status: "finished", + rating: 9, + addedAt: "2026-09-01T10:00:00+00:00", + startedAt: "2026-09-02T10:00:00+00:00", + finishedAt: "2026-09-20T10:00:00+00:00", + }, + } as const; + + async function removeThenUndo() { + const user = userEvent.setup(); + const { rerender } = renderControl({ status: "finished", rating: 9 }); + await user.click(pill("Finished")); + await settle(0, { ok: true, undo: REMOVED }); + // What refresh() delivers after the removal: the row is gone. + rerender( + , + ); + const options = toast.mock.calls[0][1] as { + action: { onClick: (event: { preventDefault: () => void }) => void }; + }; + await act(async () => + options.action.onClick({ preventDefault: () => {} }), + ); + } + + it("restores a removal with the exact values and time it returned, showing them at once", async () => { + await removeThenUndo(); + expect(deferredAction).toHaveBeenLastCalledWith( + "restoreRemoved", + 1942, + REMOVED.previous, + REMOVED.removedAt, + ); + expect(pill("Finished")).toHaveAttribute("aria-pressed", "true"); + expect(score()).toHaveAccessibleName("Your score for The Witcher 3: 9"); + }); + + it("dismisses the toast once the restore lands", async () => { + await removeThenUndo(); + await settle(1, { ok: true }); + expect(dismiss).toHaveBeenCalledWith("game-undo-1942"); + }); + + it("says the Undo expired when the restore is refused, and rolls back", async () => { + await removeThenUndo(); + await settle(1, { ok: false, error: "undo_expired" }); + expect(toast).toHaveBeenLastCalledWith( + "Couldn't undo. Set the status again on this page.", + expect.objectContaining({ id: "game-undo-1942" }), + ); + expect(pill("Finished")).toHaveAttribute("aria-pressed", "false"); + }); + + it("restores a cleared score through restoreGameStatus", async () => { + const user = userEvent.setup(); + renderControl({ status: "finished", rating: 9 }); + await user.click(pill("Want to play")); + await settle(0, { + ok: true, + undo: { kind: "status", previous: REMOVED.previous }, + }); + const options = toast.mock.calls[0][1] as { + action: { onClick: (event: { preventDefault: () => void }) => void }; + }; + await act(async () => + options.action.onClick({ preventDefault: () => {} }), + ); + expect(deferredAction).toHaveBeenLastCalledWith( + "restoreStatus", + 1942, + REMOVED.previous, + ); + }); + }); +}); diff --git a/components/tracking/game-status-control.tsx b/components/tracking/game-status-control.tsx new file mode 100644 index 00000000..f9d4d0d5 --- /dev/null +++ b/components/tracking/game-status-control.tsx @@ -0,0 +1,246 @@ +"use client"; + +import { cn } from "cn"; +import { StarIcon } from "lucide-react"; +import { useRouter } from "next/navigation"; +import { startTransition, useOptimistic, useRef, useState } from "react"; + +import { removeGame, setGameRating, setGameStatus } from "@/app/games/actions"; +import { glassPillClassName } from "@/components/glass-pill"; +import { + Popover, + PopoverContent, + PopoverTitle, + PopoverTrigger, +} from "@/components/ui/popover"; +import { formatPersonalScore } from "@/lib/format"; +import { + applyGameIntent, + canHoldScore, + type GameIntent, +} from "@/lib/tracking/game-intent"; +import { GAME_STATUS_LABELS } from "@/lib/tracking/messages"; +import { + GAME_STATUSES, + type GameStatus, + type GameTrackingResult, + type GameTrackingState, + type GameUndo, +} from "@/lib/tracking/types"; + +import { ScorePicker } from "./score-picker"; +import { WatchedIcon } from "./tracking-icons"; +import { TRACKING_PILL, TRACKING_PILL_UNAVAILABLE } from "./tracking-pill"; +import { + type GameControl, + gameTrackingToastId, + settleResultCall, + showGameTrackingError, +} from "./tracking-toast"; +import { useGameUndo } from "./use-game-undo"; + +/** + * The game page's status pills and score (spec 0026, AC-2 to AC-8, AC-11). + * + * Three toggles in one group: a click on an unpressed pill sets that status, + * a click on the pressed one removes the game. One `useOptimistic` holds the + * whole state, reduced by `applyGameIntent`, which the tests pin against the + * SQL, so a click shows its result at once; the optimistic value lasts only + * while the action is in flight, and the confirmed state is always the + * server prop `refresh()` delivers. Every target is read from the optimistic + * value, so a double click sends two different targets. + * + * The score pill follows the three only on Playing or Finished. A move into + * Finished with no score opens its picker at once; a move into Playing never + * does (AC-7). + * + * Before release, Playing and Finished are `aria-disabled` unless pressed + * (the pressed one can still remove), and the picker can clear but not score + * (`AGENTS.md` "Games", release gate; AC-8). The server checks the same gate + * again. + */ +function GameStatusControl({ + gameId, + name, + state, + returnPath, + releaseNote, +}: { + gameId: number; + name: string; + state: GameTrackingState; + /** Where the session expired toast's Sign in action comes back to. */ + returnPath: string; + /** + * Before release, the line under the pills: "Releases Oct 24, 2026" or + * "Release date to be announced". Null once the game is out (UTC day). + */ + releaseNote: string | null; +}) { + const router = useRouter(); + const [optimistic, addIntent] = useOptimistic(state, applyGameIntent); + const [pickerOpen, setPickerOpen] = useState(false); + const initialFocusRef = useRef(null); + // Set while a status failure is the latest word, so a score queued behind + // it (the auto open case) does not add its own `not_started` toast (AC-11). + const statusFailedRef = useRef(false); + + const showUndo = useGameUndo({ gameId, returnPath }); + + /** Shows the restored status and score at once while an Undo runs. */ + function showRestored(undo: GameUndo) { + addIntent({ kind: "status", value: undo.previous.status }); + addIntent({ kind: "rating", value: undo.previous.rating }); + } + + function run( + control: GameControl, + intent: GameIntent, + call: () => Promise, + onError?: () => void, + ) { + if (control === "status") statusFailedRef.current = false; + startTransition(async () => { + addIntent(intent); + const result = await settleResultCall(call); + if (!result.ok) { + onError?.(); + if (control === "status") statusFailedRef.current = true; + if ( + control === "rating" && + result.error === "not_started" && + statusFailedRef.current + ) { + return; + } + showGameTrackingError(result.error, { + id: gameTrackingToastId(gameId, control), + returnPath, + navigate: router.push, + }); + return; + } + if ("undo" in result && result.undo) { + const undo = result.undo; + showUndo(undo, () => showRestored(undo)); + } + }); + } + + function isHeldBack(status: GameStatus): boolean { + return ( + releaseNote !== null && + status !== "want_to_play" && + optimistic.status !== status + ); + } + + function choose(status: GameStatus) { + if (isHeldBack(status)) return; + if (optimistic.status === status) { + run("status", { kind: "status", value: null }, () => removeGame(gameId)); + return; + } + const opensPicker = + status === "finished" && + optimistic.status !== "finished" && + optimistic.rating === null; + if (opensPicker) setPickerOpen(true); + run( + "status", + { kind: "status", value: status }, + () => setGameStatus(gameId, status), + opensPicker ? () => setPickerOpen(false) : undefined, + ); + } + + function rate(value: number | null) { + setPickerOpen(false); + run("rating", { kind: "rating", value }, () => + setGameRating(gameId, value), + ); + } + + const canScore = canHoldScore(optimistic.status); + const scoreText = formatPersonalScore(optimistic.rating); + + return ( +

+
+
+ {GAME_STATUSES.map((status) => { + const pressed = optimistic.status === status; + const heldBack = isHeldBack(status); + const label = GAME_STATUS_LABELS[status]; + return ( + + ); + })} +
+ + {canScore && ( + + + + + + Your score + + rate(null)} + initialFocusRef={initialFocusRef} + unavailableNote={releaseNote ?? undefined} + /> + + + )} +
+ + {releaseNote !== null && ( +

+ {releaseNote} +

+ )} +
+ ); +} + +export { GameStatusControl }; diff --git a/components/tracking/game-tracking-slot.test.tsx b/components/tracking/game-tracking-slot.test.tsx new file mode 100644 index 00000000..aa71b6e5 --- /dev/null +++ b/components/tracking/game-tracking-slot.test.tsx @@ -0,0 +1,102 @@ +import { render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +/** + * covers: spec 0026, AC-1, AC-8, AC-12 + * + * The server slot decides between nothing, a failure line and the control, + * and decides the release gate from the request's UTC day. The read and the + * day are the boundaries, so they are what is replaced. + */ +const getGameTracking = vi.fn(); +vi.mock("@/lib/tracking/game-state", () => ({ + getGameTracking: (...args: unknown[]) => getGameTracking(...args), +})); +vi.mock("@/lib/tracking/episode-state", () => ({ + requestTodayUtc: () => "2026-10-24", +})); +vi.mock("@/app/games/actions", () => ({})); +vi.mock("next/navigation", () => ({ useRouter: () => ({ push: vi.fn() }) })); +vi.mock("sonner", () => ({ toast: vi.fn() })); + +const { GameTrackingSlot } = await import("./game-tracking-slot"); + +afterEach(() => { + vi.clearAllMocks(); +}); + +const WITCHER = { + gameId: 1942, + name: "The Witcher 3", + releaseDate: "2015-05-19", +}; + +const pill = (label: string, name = "The Witcher 3") => + screen.getByRole("button", { name: `${label}: ${name}` }); + +describe("GameTrackingSlot", () => { + it("renders nothing for a visitor, not even the release line (AC-1)", async () => { + getGameTracking.mockResolvedValue({ kind: "signed_out" }); + const { container } = render( + await GameTrackingSlot({ ...WITCHER, releaseDate: null }), + ); + expect(container).toBeEmptyDOMElement(); + }); + + it("shows the failure line with a full page retry to the same game", async () => { + getGameTracking.mockResolvedValue({ kind: "failed" }); + render(await GameTrackingSlot(WITCHER)); + expect( + screen.getByText("Couldn't load your tracking."), + ).toBeInTheDocument(); + expect(screen.getByRole("link", { name: "Try again" })).toHaveAttribute( + "href", + "/games/1942", + ); + }); + + it("renders the control with the stored state and asks for the right game (AC-12)", async () => { + getGameTracking.mockResolvedValue({ + kind: "ok", + state: { status: "playing", rating: 8 }, + }); + render(await GameTrackingSlot(WITCHER)); + expect(getGameTracking).toHaveBeenCalledWith(1942); + expect(pill("Playing")).toHaveAttribute("aria-pressed", "true"); + expect( + screen.getByRole("button", { name: "Your score for The Witcher 3: 8" }), + ).toBeInTheDocument(); + }); + + it("treats the release day itself as released, so no line shows (AC-8)", async () => { + getGameTracking.mockResolvedValue({ + kind: "ok", + state: { status: null, rating: null }, + }); + render(await GameTrackingSlot({ ...WITCHER, releaseDate: "2026-10-24" })); + expect(screen.queryByText(/^Releases/)).toBeNull(); + expect(pill("Playing")).not.toHaveAttribute("aria-disabled"); + }); + + it("gates a game out tomorrow by the UTC day, with its date (AC-8)", async () => { + getGameTracking.mockResolvedValue({ + kind: "ok", + state: { status: null, rating: null }, + }); + render(await GameTrackingSlot({ ...WITCHER, releaseDate: "2026-10-25" })); + expect(screen.getByText("Releases Oct 25, 2026")).toBeInTheDocument(); + expect(pill("Playing")).toHaveAttribute("aria-disabled", "true"); + expect(pill("Finished")).toHaveAttribute("aria-disabled", "true"); + }); + + it("says the date is to be announced when IGDB has none (AC-8)", async () => { + getGameTracking.mockResolvedValue({ + kind: "ok", + state: { status: null, rating: null }, + }); + render(await GameTrackingSlot({ ...WITCHER, releaseDate: null })); + expect( + screen.getByText("Release date to be announced"), + ).toBeInTheDocument(); + }); +}); diff --git a/components/tracking/game-tracking-slot.tsx b/components/tracking/game-tracking-slot.tsx new file mode 100644 index 00000000..8198d01c --- /dev/null +++ b/components/tracking/game-tracking-slot.tsx @@ -0,0 +1,69 @@ +import { RetryLink } from "@/components/retry-link"; +import { gameReleaseNote, isGameReleased } from "@/lib/catalog/game-page"; +import { requestTodayUtc } from "@/lib/tracking/episode-state"; +import { getGameTracking } from "@/lib/tracking/game-state"; +import { TRACKING_READ_FAILED } from "@/lib/tracking/messages"; + +import { GameStatusControl } from "./game-status-control"; + +/** + * The game page's tracking slot: nothing for a visitor, the retry line when + * the read fails, otherwise the status control with the stored state (spec + * 0026, AC-1, AC-12), the movie slot's shape. + * + * It reads the session, so the page renders it inside its own Suspense + * boundary with a `null` fallback: a visitor never sees a skeleton for a + * control they will not get, and a signed in user never sees a default that + * then corrects itself. + * + * The release gate is decided here, from the request's UTC day, so the + * control only receives the line (AC-8). The line lives in the slot, so only + * a signed in user sees it. + * + * @param gameId The game, already confirmed by `loadGame`. + * @param name The IGDB name, for every accessible name. + * @param releaseDate `Game.releaseDate`, the UTC day of IGDB's + * `first_release_date`. + */ +async function GameTrackingSlot({ + gameId, + name, + releaseDate, +}: { + gameId: number; + name: string; + releaseDate: string | null; +}) { + const result = await getGameTracking(gameId); + const returnPath = `/games/${gameId}`; + + if (result.kind === "signed_out") return null; + + if (result.kind === "failed") { + return ( +
+

{TRACKING_READ_FAILED}

+ +
+ ); + } + + return ( + + ); +} + +export { GameTrackingSlot }; diff --git a/components/tracking/tracking-toast.ts b/components/tracking/tracking-toast.ts index 9ae3caf6..c18bc48a 100644 --- a/components/tracking/tracking-toast.ts +++ b/components/tracking/tracking-toast.ts @@ -5,12 +5,14 @@ import { toast } from "sonner"; import { EPISODE_TRACKING_MESSAGES, + GAME_TRACKING_MESSAGES, SHOW_TRACKING_MESSAGES, SIGN_IN_ACTION_LABEL, TRACKING_MESSAGES, } from "@/lib/tracking/messages"; import type { EpisodeTrackingError, + GameTrackingError, MovieTrackingError, ShowTrackingError, } from "@/lib/tracking/types"; @@ -205,9 +207,52 @@ export function showShowTrackingError( }); } +/** Which game control failed, so each keeps one toast of its own (AC-11). */ +export type GameControl = "status" | "rating"; + +/** + * The id of a game's failure toast, one per game and control, so repeated + * failures replace each other (spec 0026, AC-11). The status control and the + * Undo hook both build it here, so a failed Undo replaces a failed click. + */ +export function gameTrackingToastId( + gameId: number, + control: GameControl, +): string { + return `game-tracking-${gameId}-${control}`; +} + +/** + * Shows the toast for a failed game write (spec 0026, AC-11). The caller + * names the toast id with `gameTrackingToastId`. + * + * @param error The error class the action returned. + * @param options.id The toast id. + * @param options.returnPath The page to come back to after signing in. + * @param options.navigate The router push, for the Sign in action. + */ +export function showGameTrackingError( + error: GameTrackingError, + { + id, + returnPath, + navigate, + }: { id: string; returnPath: string; navigate: (href: string) => void }, +): void { + toast(GAME_TRACKING_MESSAGES[error], { + id, + description: undefined, + action: signInAction(error, returnPath, navigate), + }); +} + /** The Sign in action a session expired toast carries, and no other. */ function signInAction( - error: MovieTrackingError | EpisodeTrackingError | ShowTrackingError, + error: + | MovieTrackingError + | EpisodeTrackingError + | ShowTrackingError + | GameTrackingError, returnPath: string, navigate: (href: string) => void, ) { diff --git a/components/tracking/use-game-undo.ts b/components/tracking/use-game-undo.ts new file mode 100644 index 00000000..c1588e99 --- /dev/null +++ b/components/tracking/use-game-undo.ts @@ -0,0 +1,74 @@ +"use client"; + +import { useRouter } from "next/navigation"; +import { toast } from "sonner"; + +import { restoreGameStatus, restoreRemovedGame } from "@/app/games/actions"; +import { GAME_UNDO_MESSAGES, UNDO_ACTION_LABEL } from "@/lib/tracking/messages"; +import type { GameUndo } from "@/lib/tracking/types"; + +import { + gameTrackingToastId, + runUndoInToast, + showGameTrackingError, + UNDO_TOAST_MS, +} from "./tracking-toast"; + +/** + * The two game toasts that carry an Undo (spec 0026, AC-4, AC-5): a removal, + * and a move to Want to play that cleared a score. Both share the id + * `game-undo-{id}`, so a newer one replaces an older one, and both run + * through `runUndoInToast`, which keeps the toast while the restore runs. + * + * @param gameId The game. + * @param returnPath Where the session expired toast's Sign in comes back to. + */ +export function useGameUndo({ + gameId, + returnPath, +}: { + gameId: number; + returnPath: string; +}) { + const router = useRouter(); + const toastId = `game-undo-${gameId}`; + + /** + * Shows the toast for what a write can take back. + * + * @param undo The action's `undo`; nothing shows when it is absent. + * @param restoring Runs inside the Undo's transition, for an optimistic + * update that shows the old state back at once. + */ + return function showUndo(undo: GameUndo | undefined, restoring?: () => void) { + if (!undo) return; + const message = + undo.kind === "removed" + ? GAME_UNDO_MESSAGES.removed + : GAME_UNDO_MESSAGES.scoreCleared; + toast(message, { + id: toastId, + duration: UNDO_TOAST_MS, + action: { + label: UNDO_ACTION_LABEL, + onClick: (event) => + runUndoInToast(event, { + id: toastId, + message, + expiredMessage: GAME_UNDO_MESSAGES.undoExpired, + restore: () => + undo.kind === "removed" + ? restoreRemovedGame(gameId, undo.previous, undo.removedAt) + : restoreGameStatus(gameId, undo.previous), + restoring, + onError: (error) => + showGameTrackingError(error, { + id: gameTrackingToastId(gameId, "status"), + returnPath, + navigate: router.push, + }), + }), + }, + }); + }; +} diff --git a/docs/reviews/2026-10-24-feat-game-status-rating.md b/docs/reviews/2026-10-24-feat-game-status-rating.md new file mode 100644 index 00000000..8c28a7d1 --- /dev/null +++ b/docs/reviews/2026-10-24-feat-game-status-rating.md @@ -0,0 +1,36 @@ +# Review, feat/game-status-rating, 2026-10-24 + +**Reviewed by**: Claude Sonnet 5.5 (author on the same model family, fresh context) +**Scope**: about 40 files, branch vs main (plus 3 untracked test files) +**Verdict**: Approve with nits + +## Summary +The change adds the game status pills and score on the game page, five security invoker SQL functions, and five Server Actions that parse, check the session, check IGDB and the release day, write under the user's JWT and refresh. I found no bugs and no security problems. The SQL, the actions and the optimistic reducer agree with each other and with the spec. The vitest run for the changed areas passes (531 tests) and typecheck passes. I did not run pgTAP or the build. + +## Minor +### 🟡 Unsafe casts on RPC rows, `app/games/actions.ts:188` and `app/games/actions.ts:273` +**Problem**: The `prev_*` and `removed_*` rows are read with an `as` cast because the generated types mark nullable columns as not null. +**Why it matters**: If a column name changes in the SQL, the cast hides it from the compiler and the Undo silently stops appearing. +**Suggested fix**: Parse the row with a small Zod schema, or add a test that fails when the cast shape and the generated type drift. + +### 🟡 Clearing a score bumps `updated_at`, `app/games/actions.ts:248` +**Problem**: The plain update that clears a score also fires when the row is Want to play or already unscored, and moves `updated_at`. +**Why it matters**: `restore_game_status` uses `updated_at` for its 10 minute window, so a stale tab clicking Clear rating can extend an old Undo. Harmless (own row only) but surprising. +**Suggested fix**: Add `.not("rating", "is", null)` to the update so it only touches a scored row. + +### 🟡 Spec and code differ on parameter order, `supabase/migrations/20261024120000_game_tracking_functions.sql:152` +**Problem**: The spec table lists `restore_game_status(game, status, rating, added_at, ...)`, but the function takes `p_added_at` before `p_rating`. The spec also says "four" functions in places and lists five. +**Why it matters**: A later reader following the spec could call it positionally and swap values. The app uses named arguments, so nothing breaks today. +**Suggested fix**: Update the spec text to match the migration. + +## Nits +- ⚪ `components/tracking/game-status-control.tsx:121`, the picker is opened with `setPickerOpen(true)` outside the transition; fine today, but a comment on why it is safe would help. +- ⚪ `app/games/actions.ts:96`, the catch drops all error detail, including non IGDB bugs rethrown by `readGame`; they log as `db_error`, which can mislead. Consider a separate outcome. + +## Strengths +- Row lock plus read plus write in one SQL function gives a correct Undo payload under two tabs, and the escape setting is turned off on every path, including before a raise. +- Release gate, hidden game rules and "restores never call IGDB" are exactly as the spec says, and the boundary test registers every new action. +- Grants are written by hand and asserted in pgTAP 020; the migration and schema file bodies are identical. + +## Test coverage +Good. Actions, reducer, schemas, error classification, the control, the slot, the release helpers and the SQL functions each have tests, including failure and release gate cases. Only gap: nothing pins the `as` casts to the generated types (see Minor). diff --git a/docs/scope/games.md b/docs/scope/games.md index 306cecb1..bea6b14e 100644 --- a/docs/scope/games.md +++ b/docs/scope/games.md @@ -27,7 +27,7 @@ _This is the second scope for BeStats. The movie and TV scope lives in [scope.md | G1 | IGDB integration module | Foundation | done | | G2 | Game data model and security policies | Foundation | done | | G3 | Game page (tracer) | Slice G1 | done | -| G4 | Game status and rating | Slice G1 | planned | +| G4 | Game status and rating | Slice G1 | in-progress | | G5 | Games tab and library pages | Slice G2 | planned | | G6 | Games home and list pages | Slice G3 | planned | | G7 | Game search and filters | Slice G4 | planned | @@ -82,10 +82,20 @@ The thinnest real `/games/{id}`: hero only (cover, title, release date, platform - [x] Test it: `/test game page` spec [0025](../specs/0025-game-page/index.md) · code in [app/games/](../../app/games/) -### G4. Game status and rating · needs a decision · GA +### G4. Game status and rating · in-progress · GA The segmented control (Want to play · Playing · Finished) and the score on the game page, saved through Server Actions. **Done when:** a signed in user can set any status, remove the game by clicking the active option, and rate it 1 to 10 on Playing or Finished; every change survives reload and another session; the release gate disables Playing, Finished and rating before release, also in the Server Action; moving back to Want to play clears the score and Undo restores it; a signed out user is sent to sign in; failed writes never show false success. -- [ ] Design it (spec): `/architect game status and rating` +- [x] Design it (spec): `/architect game status and rating` +- [x] Build it: `/develop game status and rating` + - [x] Thin thread: `set_game_status`, `getGameTracking`, `setGameStatus` and the three pills in the hero slot, surviving reload, AC-1 to AC-3, AC-10, AC-12, AC-13 + - [x] Removal and score: `remove_game`, `rate_game`, both restores and their Undo toasts, the score pill with auto open on Finished, AC-4 to AC-7, AC-14, AC-15 + - [x] Release gate: `isGameReleased`, the server checks, the release line and disabled pills, AC-8, AC-9 + - [x] Optimistic, states and checks: `applyGameIntent`, 375px and 1280px, grants and sweep tests, drift and type checks, then `supabase db push` before merge, AC-11, AC-16, AC-17 +- [x] Verify it: `/check verify game status and rating` +- [x] Test it: `/test game status and rating` +- [x] Review it (fresh model): `/check review game status and rating` +- [x] Document it: `/document game status and rating` +spec [0026](../specs/0026-game-status-rating/index.md) · code in [app/games/](../../app/games/) ## Slice G2: The games library diff --git a/docs/specs/0026-game-status-rating/index.md b/docs/specs/0026-game-status-rating/index.md new file mode 100644 index 00000000..c9fbed68 --- /dev/null +++ b/docs/specs/0026-game-status-rating/index.md @@ -0,0 +1,211 @@ +# 0026. Game status and rating: the status pills, the score and their Undo on the game page + +**Date**: 2026-10-24 +**Status**: In Progress + +Scope feature: [G4. Game status and rating](../../scope/games.md) · GA tier + +Builds on: [0024](../0024-game-data-model-security/index.md) (the `user_game_state` table, its trigger owned dates and the `bestats.restore_game` escape) and [0025](../0025-game-page/index.md) (the game page, `loadGame` and the hero's `tracking` slot). Settled in an `/architect` session on 2026-10-24, on top of the games rules in [docs/scope/games.md](../../scope/games.md). + +## Summary + +This puts three status pills (Want to play, Playing, Finished) and a personal score on `/games/{id}` for a signed in user. A click on a pill sets that status, and a click on the pill that is already on removes the game, with Undo. The score (1 to 10) shows only on Playing or Finished. Moving back to Want to play clears it, and Undo brings it back. Every write goes through a Server Action that checks the session and IGDB (the release date gates Playing, Finished and scoring), then calls one of five small SQL functions that run as the user, so row level security (the database rule that a row is reachable only by its owner) still applies. No new table: the functions sit on top of spec 0024's table. + +## Requirements + +**User stories**: +- As a signed in user, I want to mark a game Want to play, Playing or Finished in one click so that my list reflects where I am with it. +- As a signed in user, I want to score a game I am playing or have finished so that I remember what I thought of it. +- As a signed in user, I want an Undo when a click removed a game or a score so that a slip never costs me data. +- As a signed in user, I want Playing, Finished and scoring held back until a game is out so that my list stays honest. + +**Acceptance criteria**: +- **AC-1**: On `/games/{id}`, the status control renders in the hero's `tracking` slot, under the IGDB rating block, inside its own Suspense boundary with a `null` fallback. A visitor sees nothing there (the same as the movie and show pages). A signed in user sees the control already showing the stored state, never a default that then corrects itself. When the read fails, the slot shows `TRACKING_READ_FAILED` with a Retry link to `/games/{id}`. The page shell stays prerendered: nothing outside that boundary reads the session (spec 0025, AC-9). +- **AC-2**: The control is three glass pills in a `
` (implicit `role="group"`) labelled `Status for {name}`, in the order Want to play, Playing, Finished, using `glassPillClassName()` and `TRACKING_PILL` like the movie pills. Each pill is a toggle with `aria-pressed` and a fixed accessible name (`Want to play: {name}`, `Playing: {name}`, `Finished: {name}`), so a screen reader hears one control change state. At most one pill is pressed. A score pill follows them only while the status is Playing or Finished. +- **AC-3**: Clicking a pill that is not pressed sets that status through `setGameStatus(gameId, status)`, from any status (or none) to any other, in one click. The action takes a target value, never a toggle: the same call twice leaves one row and moves no date (spec 0024, AC-5, AC-8). +- **AC-4**: Clicking the pressed pill removes the game through `removeGame(gameId)`, which deletes the row. On success a toast reads `Removed from your games.` with an Undo action for `UNDO_TOAST_MS` (10 seconds). Undo calls `restoreRemovedGame` with what `remove_game` returned, and the game comes back with its old status, score, `added_at`, `started_at` and `finished_at`. A removal that finds no row (another tab already removed it) is a quiet success with no toast. +- **AC-5**: Moving a game from Playing or Finished to Want to play clears its score in the same statement (spec 0024, AC-2). When a score was cleared, a toast reads `Moved to Want to play. Your score was removed.` with Undo, which calls `restoreGameStatus` and puts back the old status, score and all three dates exactly. When there was no score, no toast shows. +- **AC-6**: Moving between Playing and Finished keeps the score. The score pill reads the score (`formatPersonalScore`) or `Rate` when there is none, with the accessible name `Your score for {name}: {score or Not rated}` (the movie wording), and opens the existing `ScorePicker` in a popover with the current score checked. A pick saves through `setGameRating(gameId, rating)` and closes the popover. Clear rating saves `null`, closes it and keeps the status. A score on a game with no row or on Want to play is refused by the database with `BS004` (`game_not_started`), reported as `not_started`. +- **AC-7**: A click that moves a game into Finished from any other status (or none) opens the score picker at once, but only when the game has no score yet. A move into Playing never does. Closing the picker leaves the game Finished and unscored. If the write fails, the picker closes with the rollback. +- **AC-8**: Before release (IGDB `releaseDate` null, or a UTC day after today's UTC day), the slot shows the release line under the pills: `Releases {date}` or `Release date to be announced`, the movie page's copy. The line lives in the slot, so only a signed in user sees it. The Playing and Finished pills are `aria-disabled` and do nothing, except the one that is pressed, which can still be clicked to remove the game. Want to play is never held back by the release gate (it can still fail with `not_found` or `igdb_unavailable`, AC-9). The score picker gets `unavailableNote` set to the release line, so its ten choices do nothing while Clear rating still works. A game stored as Playing or Finished before release keeps its pressed pill and score pill. +- **AC-9**: The Server Actions check IGDB through `readGameIncludingHidden` (`loadGame` without its hidden rule, in `app/games/[id]/load-game.ts`) before writing, and only where a write could move a game forward: `setGameStatus` to `playing` or `finished` refuses an id IGDB does not know or hides with `not_found`, and refuses before release with `not_released`; `setGameStatus` to `want_to_play` refuses only an id IGDB does not know (`not_found`), and accepts a hidden game; `setGameRating` with a score refuses an unknown id with `not_found` and refuses before release with `not_released`, and accepts a hidden game, since `rate_game` needs Playing or Finished, which the game could only reach while listed. An IGDB failure on any of these refuses with `igdb_unavailable`, which is accepted for Want to play too. `removeGame`, clearing a score, `restoreGameStatus` and `restoreRemovedGame` never call IGDB: a restore only puts back a state the user already had, so it never advances, and it works during an outage, for a hidden game, and before release. The slot decides the release line from `requestTodayUtc()` and the action from `todayUtc(new Date())`; across UTC midnight they can disagree, so an enabled pill can still return `not_released`, which is accepted. +- **AC-10**: Every action runs Zod parse, then `getOptionalUser()`, then the IGDB check, then the SQL call through the request's Supabase client under the user's JWT, then `refresh()` on success only. None throws or redirects; each returns `GameTrackingResult`. `user_id` is always `auth.uid()` inside the SQL, never an argument. A missing session returns `session_expired`, whose toast carries Sign in to `/sign-in?next=/games/{id}`. Each new export is registered in `private-actions-boundary.test.ts`, which proves it refuses a missing session before it opens a database client. +- **AC-11**: The control is optimistic through `useOptimistic` and a pure reducer `applyGameIntent` (moving to Want to play or removing nulls the score), so a click shows its result at once. A `rating` intent on no status or on Want to play changes nothing. On failure the control falls back to the server prop and shows the matching toast; when a score pick is queued behind a status write that fails (the auto open case), both roll back together and only the status toast shows (the score's `not_started` is not toasted while a status error is showing). Toast ids: `game-tracking-{id}-status` (status and removal failures), `game-tracking-{id}-rating` (score failures), `game-undo-{id}` (both Undo toasts). Every target is read from the optimistic value, so a double click sends two different targets, not the same one twice. +- **AC-12**: Every change survives a reload and shows in another signed in session: the slot reads the row per request with `getGameTracking(gameId)`, which never enters a shared or `use cache` scope (AGENTS.md section 11). +- **AC-13**: A migration adds `set_game_status`, `rate_game`, `remove_game`, `restore_game_status` and `restore_removed_game` exactly as in the API surface below, each `security invoker` with `set search_path = ''`, its execute revoked from `public`, `anon` and `authenticated` and granted back to `authenticated` only. `set_game_status` locks the existing row (`for update`) before it reads the previous values and writes, so two tabs cannot both receive the same Undo payload. +- **AC-14**: The restores refuse with `P0002`, reported as `undo_expired` with the existing Undo expired copy, unless: `p_added_at <= now()`, `(p_started_at is null or p_started_at <= now())` and `(p_finished_at is null or p_finished_at <= now())`; for `restore_game_status`, the row exists, is `want_to_play` with a null score, `updated_at > now() - interval '10 minutes'`, and the restored status is `playing` or `finished`; for `restore_removed_game`, no row exists (`insert … on conflict do nothing`, then `if not found` raises), `p_removed_at <= now()` and `p_removed_at > now() - interval '10 minutes'`. pgTAP pins both window boundaries. A payload that passes these guards but breaks a table check (a Playing state with no start) fails with `23514` as `write_failed`; the Zod schema refuses that shape first (API surface). Both set `bestats.restore_game` to `on` only around their one statement and back to `off` before returning or raising. The table checks (spec 0024, AC-1 to AC-3) still apply to what they write. +- **AC-15**: Errors map through `classifyGameTrackingError`, reading only `error.code`: `BS004` is `not_started`, `BS003` (the identity lock, unreachable through these functions) is `write_failed` logged as `identity_locked`, and every other code follows `classifyTrackingError`. Each refusal, `invalid_input` and `session_expired` included, is logged with `logTrackingEvent` under the action's own `game_tracking.*` event and an outcome class only, never an id, a date or a raw error. +- **AC-16**: `supabase/tests/190-game-tracking-functions.test.sql` covers every function, guard and refusal above; `020-security-configuration` asserts the new function grants; `140-security-sweep` passes unchanged (no `security definer`). `supabase db diff` after `supabase db reset` reports no drift, `pnpm db:types` regenerates the function signatures, and `pnpm db:types:check`, `pnpm typecheck`, `pnpm lint`, `pnpm test`, `pnpm test:db` and `pnpm build` pass. The migration is pushed with `supabase db push` before the PR merges (docs/deploy.md). +- **AC-17**: At 375px and 1280px the pills wrap without horizontal scroll, keep a 44px tap target through the `hit-area` utilities, show visible focus, and the score popover fits the screen (`max-w-[calc(100vw-2rem)]`). Keyboard alone can set every status, remove, open the picker, score and use Undo from the toast. + +## Decision + +**Chosen option**: Option 1: Server Actions over five `security invoker` SQL functions plus a plain PATCH to clear the score (see [rationale.md](rationale.md)). + +The game page gets a `GameTrackingSlot` and a client `GameStatusControl` shaped like the movie ones, and `app/games/actions.ts` mirrors `app/movies/actions.ts`: one shared `runGameWrite` runs the parse, session, IGDB and release check, write and `refresh()` order, and every write whose result an Undo needs goes through a SQL function that locks, reads and writes in one transaction. + +**Implementation skills**: `supabase-postgres-best-practices` (`supabase/agent-skills`, `.agents/skills/supabase-postgres-best-practices/`) · `supabase` (`supabase/agent-skills`, `.agents/skills/supabase/`) · `next-dev-loop` (`vercel/next.js`, `.agents/skills/next-dev-loop/`) + +## Rationale + +Reasoning and options: see [rationale.md](rationale.md). + +## Feature design + +**Data model sketch**: + +No new table, column, index or enum value. Everything sits on `public.user_game_state` from spec 0024 (one row per user and game: `status`, `rating`, `added_at`, `started_at`, `finished_at`, `created_at`, `updated_at`), whose trigger owns every date and whose checks refuse a score on Want to play. + +New in `supabase/schemas/05-functions.sql`, after the show tracking block, and in one hand written migration `supabase/migrations/20261024120000_game_tracking_functions.sql` (grown across the build tasks while unpushed, then pushed once): + +| Function | Language | Does | Returns | +|---|---|---|---| +| `set_game_status(p_game_id integer, p_status public.game_status)` | plpgsql | `select … for update` the caller's row into the previous values (a missing row locks nothing and leaves them null); `insert into public.user_game_state as s … on conflict (user_id, game_id) do update set status = excluded.status, rating = case when excluded.status = 'want_to_play' then null else s.rating end`; `return query select` the previous values | always exactly one row: `table (prev_status public.game_status, prev_rating smallint, prev_added_at timestamptz, prev_started_at timestamptz, prev_finished_at timestamptz)`, all null when there was no row; the action reads `data[0]` | +| `rate_game(p_game_id integer, p_rating smallint)` | plpgsql | `update … set rating = p_rating where user_id = auth.uid() and game_id = p_game_id and status in ('playing', 'finished') and p_rating between 1 and 10`; when not found, `raise exception 'game_not_started' using errcode = 'BS004'` (a null `p_rating` from a direct call is refused the same way, so only the PATCH clears) | `void` | +| `remove_game(p_game_id integer)` | plpgsql | `delete from public.user_game_state as s where s.user_id = auth.uid() and s.game_id = p_game_id returning` the five values `into` locals; `if found then return query select` them with `now()` | `table (removed_status, removed_rating, removed_added_at, removed_started_at, removed_finished_at, removed_at timestamptz)`; zero rows when there was nothing to delete; the action reads `data?.[0]` | +| `restore_game_status(p_game_id integer, p_status public.game_status, p_added_at timestamptz, p_rating smallint default null, p_started_at timestamptz default null, p_finished_at timestamptz default null)` | plpgsql | Escape `on`; `update … set status, rating, added_at, started_at, finished_at` to the given values `where` the AC-14 guards hold; `if not found`, escape `off` then raise `P0002`; otherwise escape `off` | `void` | +| `restore_removed_game(p_game_id integer, p_status public.game_status, p_added_at timestamptz, p_removed_at timestamptz, p_rating smallint default null, p_started_at timestamptz default null, p_finished_at timestamptz default null)` | plpgsql | If the date and window guards of AC-14 fail, raise `P0002`; escape `on`; `insert … on conflict (user_id, game_id) do nothing`; `if not found` (a row appeared meanwhile), escape `off` then raise `P0002`; otherwise escape `off` | `void` | + +`p_rating`, `p_started_at` and `p_finished_at` default to null. Clearing a score is not a function: the action sends `update user_game_state set rating = null where user_id = $user and game_id = $id and rating is not null`, which moves no date (spec 0024, AC-5). On a row another tab already removed it matches nothing and returns ok on purpose; `refresh()` then shows the true state. The `rating is not null` filter keeps a stale tab clearing an unscored row from moving `updated_at`, which would stretch the 10 minute window of `restore_game_status`. `set_game_status` does not check the release gate; only the Server Action does. + +SQLSTATEs: `BS004` (`game_not_started`) is new, beside `BS001` to `BS003`. `P0002` is the shared Undo refusal. + +**State transitions**: + +The status machine and its dates are spec 0024's transition table, unchanged. What G4 adds is which click reaches which transition, and what it offers back: + +| Current | Click | Write | Undo offered | +|---|---|---|---| +| none | Want to play, Playing or Finished | `set_game_status` (insert) | none | +| any status | a different, unpressed pill | `set_game_status` (update) | only on a move to Want to play that cleared a score (AC-5) | +| any status | the pressed pill | `remove_game` | always, when a row was deleted (AC-4) | +| Playing or Finished | a score, or Clear rating | `rate_game`, or the PATCH | none | +| unreleased, none or Want to play | Playing or Finished | none: the pill is disabled (AC-8) | | +| unreleased, Playing or Finished | the other of the two | none: disabled | | +| unreleased, Playing or Finished | Want to play, or the pressed pill | allowed, as above | as above | + +**API surface**: + +Server Actions in `app/games/actions.ts` (`"use server"`), each returning `GameTrackingResult`: + +| Action | Key inputs | IGDB check | SQL | Key outputs | Key errors | +|---|---|---|---|---|---| +| `setGameStatus` | `gameId: number`, `status: "want_to_play" \| "playing" \| "finished"` | `want_to_play`: known to IGDB (hidden accepted); `playing` or `finished`: known, not hidden and released | `rpc("set_game_status")` | `{ ok: true, undo? }`, `undo` of kind `status` only when the previous status was `playing` or `finished`, the target is `want_to_play` and `prev_rating` was not null | `invalid_input`, `session_expired`, `not_found`, `not_released`, `igdb_unavailable`, `write_failed` | +| `setGameRating` | `gameId`, `rating: 1..10 \| null` | released, only when `rating` is not null | `rpc("rate_game")`, or the PATCH for `null` | `{ ok: true }` | `not_started`, `not_released`, plus the shared ones | +| `removeGame` | `gameId` | none | `rpc("remove_game")` | `{ ok: true, undo? }`, `undo` of kind `removed` when a row came back | shared ones | +| `restoreGameStatus` | `gameId`, `previous: GameRestoreState` | none | `rpc("restore_game_status")` | `{ ok: true }` | `undo_expired`, plus the shared ones | +| `restoreRemovedGame` | `gameId`, `previous: GameRestoreState`, `removedAt: string` | none | `rpc("restore_removed_game")` | `{ ok: true }` | `undo_expired`, plus the shared ones | + +Shapes in `lib/tracking/types.ts`: +- `GameStatus = Database["public"]["Enums"]["game_status"]`. +- `GameTrackingState = { status: GameStatus | null; rating: number | null }`, `EMPTY_GAME_TRACKING = { status: null, rating: null }`. +- `GameRestoreState = { status: GameStatus; rating: number | null; addedAt: string; startedAt: string | null; finishedAt: string | null }`. +- `GameUndo = { kind: "status"; previous: GameRestoreState } | { kind: "removed"; previous: GameRestoreState; removedAt: string }`. +- `GameTrackingError = Exclude | "igdb_unavailable" | "not_released" | "not_started"`. +- `GameTrackingResult = { ok: true; undo?: GameUndo } | { ok: false; error: GameTrackingError }`. + +Zod in `lib/tracking/schemas.ts`: `gameIdSchema` with the bounds `parseGameId` uses, `gameStatusSchema = z.enum(["want_to_play", "playing", "finished"])`, `ratingSchema` reused, and a `gameRestoreStateSchema` whose dates are `z.iso.datetime({ offset: true })` (nullable for `startedAt` and `finishedAt`), as `restoreWatchedInputSchema` does, refined so `playing` needs `startedAt`, `finished` needs `finishedAt`, and `want_to_play` needs a null `rating` (the table's checks, refused early as `invalid_input`). + +Read in `lib/tracking/game-state.ts` (`server-only`): `getGameTracking(gameId): Promise>`, using `getOptionalUser()` then `select status, rating … eq(game_id).maybeSingle()` under RLS; a failure is logged and returned as `failed`. + +Pure helpers: `applyGameIntent` in `lib/tracking/game-intent.ts` (intents `{ kind: "status"; value: GameStatus | null }` and `{ kind: "rating"; value: number | null }`); `isGameReleased(releaseDate, today)` and `gameReleaseNote(releaseDate)` in `lib/catalog/game-page.ts`, delegating to `airStatus` and `UPCOMING_MESSAGES` so the copy and the UTC day boundary match `isMovieReleased` and `movieReleaseNote`, and so G5 and G8 reuse them. + +UI: `components/tracking/game-tracking-slot.tsx` (server, like `MovieTrackingSlot`, computes the release line from `requestTodayUtc()`), `components/tracking/game-status-control.tsx` (client), and `components/tracking/use-game-undo.ts` (the two Undo toasts through `runUndoInToast`). The page passes `` to `GameHero`'s `tracking` prop. + +Copy in `lib/tracking/messages.ts`: `GAME_STATUS_LABELS` (`Want to play`, `Playing`, `Finished`), `GAME_TRACKING_MESSAGES: Record` (reusing the movie wording for the shared classes, `Couldn't reach IGDB. Try again.` for `igdb_unavailable`, `This game isn't out yet.` for `not_released`, `Set Playing or Finished to score this game.` for `not_started`), and `GAME_UNDO_MESSAGES` (`removed: "Removed from your games."`, `scoreCleared: "Moved to Want to play. Your score was removed."`, `undoExpired` reusing the existing Undo expired copy). + +Log events in `lib/tracking/log.ts`: `game_tracking.status`, `game_tracking.rate`, `game_tracking.remove`, `game_tracking.restore_status`, `game_tracking.restore_removed`, `game_tracking.read`; `TrackingOutcome` gains `GameTrackingError` and `identity_locked`. + +**Value sourcing**: + +| Action | Value produced / displayed | Source | +|---|---|---| +| any write | `user_id` | `auth.uid()` inside the SQL, from the verified JWT | +| any write | `game_id` | the action's `gameId`, parsed by `gameIdSchema`; on the page, the `id` already parsed by `parseGameId` | +| `setGameStatus` | `status` | the clicked pill, read as a target from the optimistic state | +| `setGameStatus` | the cleared score's Undo payload | `prev_*` columns returned by `set_game_status`, read under the row lock | +| `setGameRating` | `rating` | the picked score from `ScorePicker`, or `null` from Clear rating | +| `removeGame` | the removal's Undo payload and `removedAt` | `remove_game`'s `returning` and its `now()` | +| both restores | the dates, status and score to put back | the Undo payload the earlier write returned, carried in the toast closure, never shown or edited | +| both restores | whether the Undo is still allowed | `updated_at` (status) or `p_removed_at` (removal) against `now()` in the SQL; the toast's 10 seconds is the visible limit | +| release gate (server) | released or not | `Game.releaseDate` from `readGameIncludingHidden` (UTC day of IGDB `first_release_date`, spec 0023) against `todayUtc(new Date())` | +| release gate (page) | the release line and the disabled pills | `releaseDate` passed from the page to the slot, `requestTodayUtc()` in the slot, `gameReleaseNote` for the text | +| slot | stored status and score | `getGameTracking` per request | +| control | accessible names | the IGDB `name` the page already has | +| score pill | the shown score | `formatPersonalScore(optimistic.rating)` | +| `session_expired` toast | where Sign in returns | `returnPath = /games/{id}`, built in the slot | + +**Key invariants**: +- One row per user and game; a repeated click writes the same row (spec 0024). +- A score exists only on Playing or Finished: the table check, `rate_game`'s `BS004`, and `set_game_status` nulling it on Want to play all hold it. +- No client chooses a date except through the two restores, which accept only past dates, only inside a 10 minute window, and only for the caller's own row. A direct call with a forged `p_removed_at` can therefore insert the caller's own row with any past dates, even Playing or Finished on an unreleased game; it touches only their own private row and is accepted (Security model). +- No new forward move the app makes (a status into Playing or Finished, a score) happens before release; removing, clearing, moving to Want to play and Undo are always possible. +- The confirmed state on screen is always the server's after `refresh()`; the optimistic value lives only while an action is in flight. +- No page writes when it loads (AGENTS.md, spec 0020 AC-21): the slot only reads. + +**Security model**: +- `user_game_state` stays owner only under forced RLS (spec 0024, AC-9). Every new function is `security invoker`, so the four policies apply inside it, and writes `auth.uid()` itself. A null `auth.uid()` fails the `not null` on `user_id`, so no unauthenticated call writes. +- Execute on every new function is revoked from `public`, `anon` and `authenticated`, then granted to `authenticated` only, written into the migration by hand (the declarative diff does not track function privileges). No `security definer`; `username_available` stays the only one. +- The Server Actions are the only app write path and they re-check the session (`getOptionalUser`, never `getSession`) on every call; the proxy is not the boundary (`lib/auth/AGENTS.md`). +- The release gate is enforced in the Server Action, not the database (spec 0024 security model). A signed in user calling the functions directly can set their own unreleased game Playing; it touches only their own private row and is accepted, as for movies. +- The restores take client supplied dates. They are bounded to the past, to the caller's own row, to a state the matching write produced, and to 10 minutes; a forged `p_removed_at` lets a direct call insert the caller's own row with any past dates and any status, skipping the release gate, which is the same accepted own row exposure as calling `set_game_status` directly. The window is a staleness limit, not a security boundary (the same note as `restore_movie_watchlist`). The restores skip IGDB on purpose (AC-9), so this path never depends on IGDB. +- `bestats.restore_game` is set only inside the two restore functions and set back to `off` on every path, including before a raise. +- Logs carry an event and an outcome class only. No processor is added, so `/privacy` and `PROCESSORS` stay unchanged. + +**Configuration required**: +- None. No new environment variable or credential; the IGDB credentials from spec 0023 already serve `loadGame`. + +**Critical test scenarios**: +- Happy path (pgTAP and the dev loop): as user A, Want to play, then Playing, score 8, then Finished; the score stays and the dates follow spec 0024; reload and a second session show the same, verifies **AC-3**, **AC-6**, **AC-12**. +- Auto open: a move into Finished opens the picker, a move into Playing does not; a failed Finished write closes it, verifies **AC-7**. +- Removal and Undo: clicking the pressed pill deletes the row, `remove_game` returns the values and `removed_at`; `restore_removed_game` puts back status, score and all three dates exactly; a second `remove_game` returns zero rows and the action returns no `undo`, verifies **AC-4**, **AC-14**. +- Score cleared: Finished with score 9 to Want to play returns the previous values; `restore_game_status` restores Finished, 9 and the old `added_at`, `started_at`, `finished_at`; a move from unscored Playing returns no `undo`, verifies **AC-5**, **AC-14**. +- Undo refusals: a restore with a future date, after 10 minutes (`updated_at` or `p_removed_at` moved back in the test), on a row changed meanwhile to Playing, or a removal restore when the row already exists, each raise `P0002`, and the escape reads `off` afterwards, verifies **AC-14**. +- Score refusals: `rate_game` with no row and on Want to play raise `BS004`; the action maps it to `not_started`, verifies **AC-6**, **AC-15**. +- Release gate (Vitest on the actions, with `readGameIncludingHidden` mocked): Playing, Finished and a score refuse with `not_released` for a future date and a null date; Want to play, removal, Clear rating and both restores (of a Finished, scored state) succeed before release; a hidden game refuses `not_found` on Playing and Finished but is accepted on Want to play; an IGDB failure refuses `igdb_unavailable` on status and score while removal, clearing and both restores still work and never call `readGameIncludingHidden`, verifies **AC-8**, **AC-9**. +- Reducer edge cases: a `rating` intent on no status or Want to play is a no op; a score queued behind a failing status write rolls back with only the status toast, verifies **AC-11**. +- Score pill: `Rate` when unscored, the accessible name, the checked current score, closing after a pick; a move into Finished with an existing score does not open the picker, verifies **AC-6**, **AC-7**. +- Restore payload shapes: Zod refuses Playing with no `startedAt`, Finished with no `finishedAt` and Want to play with a score as `invalid_input`; `rate_game` with a null score raises `BS004`; `remove_game` on no row returns zero rows and `set_game_status` always one, verifies **AC-6**, **AC-13**, **AC-14**. +- Release gate (component test): an unreleased game shows the line, disabled Playing and Finished, and the picker's `unavailableNote`; a stored Playing unreleased game can still be removed and moved to Want to play, verifies **AC-8**. +- Concurrency: two `set_game_status` calls racing on one row both succeed, and only the first that moves Finished to Want to play receives a non null `prev_rating` (two sessions in pgTAP, or `for update` asserted through the function body), verifies **AC-13**. +- Optimistic: `applyGameIntent` pinned against the SQL outcomes; a double click sends two different targets; a failure rolls back and shows the toast, verifies **AC-11**. +- Auth and permission: every action without a session returns `session_expired` before any client opens (`private-actions-boundary.test.ts`); in pgTAP user A's functions never touch user B's row and `anon` cannot execute any of them; the visitor slot renders nothing, verifies **AC-1**, **AC-10**, **AC-13**. +- Read failure: `getGameTracking` returning `failed` shows the retry line, verifies **AC-1**. +- States: 375px and 1280px, keyboard only flow, focus and tap targets, verifies **AC-2**, **AC-17**. + +## Build plan + +Tracer Bullet: first one real thread from the pill to the database and back (one status write, read on reload), then thicken it with removal, the score, the release gate and the polish, each end to end. One migration, grown across tasks while unpushed, pushed once before merge. + +1. Thin thread: `set_game_status` in `05-functions.sql` and the migration, with its grants and first cases in `190-game-tracking-functions.test.sql`; `pnpm db:types`; `GameStatus`, `GameTrackingState`, `GameTrackingResult` and the Zod schemas; `getGameTracking`; `setGameStatus` with the parse, session, `readGameIncludingHidden` existence check and `refresh()` (gate added in task 4); `GameTrackingSlot` and a first `GameStatusControl` with the three pills (set only), wired into the hero slot in its own Suspense; register the action in `private-actions-boundary.test.ts`; check in the running app that a click survives reload, satisfies **AC-1**, **AC-2**, **AC-3**, **AC-10**, **AC-12**, **AC-13**. +2. Removal and its Undo: `remove_game` and `restore_removed_game` with their guards and tests; `removeGame` and `restoreRemovedGame`; clicking the pressed pill removes; `use-game-undo.ts` with the removal toast through `runUndoInToast`, satisfies **AC-4**, **AC-14**. +3. Score: `rate_game` with `BS004`; `classifyGameTrackingError`; `setGameRating` with the PATCH for `null`; the score pill and popover; auto open on Finished; `set_game_status`'s previous values turned into the `status` Undo, `restore_game_status` and `restoreGameStatus` with the score cleared toast, satisfies **AC-5**, **AC-6**, **AC-7**, **AC-14**, **AC-15**. +4. Release gate: `isGameReleased` and `gameReleaseNote`; the server checks in every action that needs them, with `not_released` and `igdb_unavailable`; the release line, disabled pills and `unavailableNote` in the control, satisfies **AC-8**, **AC-9**. +5. Optimistic and failure handling: `applyGameIntent` with tests pinned to the SQL; `useOptimistic` in the control, the toast ids, the log events and messages, satisfies **AC-11**, **AC-15**. +6. States and checks: 375px and 1280px, keyboard and focus, `020` grant assertions, `140` untouched, `supabase db diff`, `pnpm db:types:check`, `typecheck`, `lint`, `test`, `test:db`, `build`; once the PR is approved, `supabase db push` per docs/deploy.md, then merge, satisfies **AC-16**, **AC-17**. + +## Consequences + +**Positive**: +- The game page now does the job the scope promised for one game, end to end, and G5 and G8 can reuse the actions, the read, the release helpers and the reducer as they are. +- Every Undo payload is read under a row lock in the same transaction as the write, so two tabs can never hand out a wrong Undo. +- The shape copies the movie tracking stack closely, so reviewers, tests and the boundary suites already know it. + +**Negative / tradeoffs**: +- Five more SQL functions to keep in step with `applyGameIntent` and the trigger; a change to the games rules now touches SQL, TypeScript and tests together. +- Changing a status needs IGDB to answer; during an IGDB outage a user can remove a game, clear a score and use Undo, but not set a status, Want to play included. +- The release gate lives only in the Server Action, so a hand made API call can still set one's own unreleased game Playing. +- A move back to Want to play without a score offers no Undo, so its old `added_at` (its place on Want to play) is lost; one click restores the status but not that place. +- Saved games are invisible in any library until G5 ships, since the GAMES tab stays hidden. + +**Neutral**: +- The control is live in production as soon as this merges, reachable only by people who already have a `/games/{id}` URL (the pages are `noindex` until G10). +- A new app SQLSTATE, `BS004`, joins `BS001` to `BS003`. +- `database.types.ts` gains the five function signatures. + +## Follow-up + +- [ ] `/sync` after merge: a repo fact line in `AGENTS.md` naming the five game functions, `BS004`, and that the game release gate lives in `app/games/actions.ts`; and the `lib/tracking/` reuse note for G5 and G8. +- [ ] G5 (`/architect games tab and library pages`): reuse `getGameTracking`'s shape for the list read, `isGameReleased` for the Want to play release badge, and the removal Undo for the library cards. +- [ ] G8 (`/architect game cards everywhere`): the card control reuses `setGameStatus`, `removeGame` and `applyGameIntent`; decide there whether the card offers the score. +- [ ] Check during `/develop` whether `/showcase` lists the movie tracking pills; if it does, add the game pills' pressed, disabled and release line states beside them. diff --git a/docs/specs/0026-game-status-rating/rationale.md b/docs/specs/0026-game-status-rating/rationale.md new file mode 100644 index 00000000..1c443408 --- /dev/null +++ b/docs/specs/0026-game-status-rating/rationale.md @@ -0,0 +1,80 @@ +# 0026. Game status and rating: rationale + +The decision record behind [index.md](index.md). `/develop` builds from the index; this file holds the why. + +## Context + +G2 (spec 0024) shipped `user_game_state` with its rules in the database: one row per user and game, a score only on Playing or Finished, every date owned by a trigger, and a `bestats.restore_game` escape that only a server function may set, so an Undo can put a game back exactly as it was. G3 (spec 0025) shipped a public `/games/{id}` whose hero already has an empty `tracking` slot. Nothing writes to the table yet, so a user cannot record anything about a game. + +The games rules were settled on 2026-10-17: three statuses set by hand, any status to any other in one click, removal deletes the row with Undo, a score from 1 to 10 only on Playing or Finished, moving back to Want to play clears the score with Undo, and a release gate (UTC day of IGDB `first_release_date`) that holds Playing, Finished and scoring back and that the database cannot check, because it never sees IGDB data. The project already has a mature movie tracking stack (Server Actions that never throw, `security invoker` SQL functions, optimistic pills, Undo through toasts, boundary tests) that reviewers and tests know well. + +The forces: two of the writes must hand back what they cleared (the score on a move to Want to play, the whole row on a removal), and two tabs open on the same game must not both receive an Undo for one change. The release gate needs IGDB at write time, which can fail. The page shell must stay prerendered (spec 0025, AC-9), so anything that reads the session must stream inside its own boundary. And the feature is GA tier: it holds private data and enforces the status and score rules, so every write path must fail visibly, never with a false success. + +Not deciding leaves G5 and G8 with no write path to build on, and leaves the restore escape that spec 0024 built unused and unguarded. + +## Options considered + +### Option 1: Server Actions over `security invoker` SQL functions + +Each Server Action parses, checks the session and IGDB, then calls one SQL function (`set_game_status`, `rate_game`, `remove_game`, two restores) that locks the row, reads what it is about to change and writes it, in one statement or one transaction. Clearing a score is a plain PATCH, as for movies. + +**Pros**: +- The previous values for Undo are read under the same row lock as the write, so two tabs never both get an Undo for one change. +- The restore guards (past dates, a 10 minute window, the expected current state) and the escape's on and off live next to the write they guard, inside the database. +- The same shape as `plan_movie`, `unmark_movie_watched` and the movie restores; the sweep tests and review habits already cover it. + +**Cons**: +- Five functions to write, grant by hand, test in pgTAP and keep in step with the TypeScript reducer. +- A rules change touches SQL, generated types and TypeScript together. + +### Option 2: Direct PostgREST writes from the Server Actions + +The actions upsert, update and delete `user_game_state` directly, reading the row first when an Undo needs it. + +**Pros**: +- No new SQL; the table's checks and trigger already refuse every invalid state. +- Fewer moving parts to review. + +**Cons**: +- The read for the Undo and the write are two requests, so a second tab can change the row between them and the Undo restores the wrong thing. +- The restores still need the escape, which PostgREST cannot set, so at least the restore functions are needed anyway; the result is a mix of both patterns. + +### Option 3: One generic `save_game(p_game_id, p_status, p_rating)` function + +A single function handles set, rate and remove (a null status meaning remove), returning the previous row every time. + +**Pros**: +- One function to grant and test; one RPC call shape in TypeScript. + +**Cons**: +- Every call carries both status and score, so a score change races a status change from another tab and can overwrite it. +- Branching on null to mean removal hides a destructive write behind an ordinary save, which is harder to review and to log. +- It still needs separate restore functions for the escape. + +## Rationale + +Option 1 is chosen because the two Undo payloads are the load bearing part of this feature, and only a function can read them under the row lock in the same transaction as the write. Option 2 leaves a real race between two tabs (both act on one stale read) and still needs functions for the restores, so it saves nothing. Option 3 trades that race for another (status and score overwriting each other) and makes removal look like a save. Copying the movie stack also means the boundary suites (`private-actions-boundary.test.ts`, `140-security-sweep`) and the reviewers' checklist apply with no new pattern to learn, which matters for a GA feature. + +The calls made in the session, each with its runner up: + +- **Visitors see nothing in the slot.** Matches the movie and show pages, and keeps "sent to sign in" to the session expired toast. Runner up: show the pills and send a click to sign in, which would make the game page the only title page that does this. +- **Live as soon as it ships, no flag.** Only people who already have a `/games/{id}` URL reach it (the page is `noindex`), and their games show up when G5 ships. Runner up: a `NEXT_PUBLIC_` flag until G5, which adds a variable and a redeploy for little gain. +- **Score picker opens by itself only on a move into Finished.** Finishing is when people rate; starting a game is not. Runner up: open on Playing too, which would mean dismissing it every time. +- **Undo on a move to Want to play only when a score was cleared.** Mirrors unmarking a movie: a toast only when data is lost. The cost is that an unscored move loses its old place on Want to play. Runner up: always offer Undo. +- **Stored early, only undo, never advance.** Before release a game can go back to Want to play, be removed, or lose its score, but cannot move to or between Playing and Finished or gain or change a score. Same as movies. Runner up: no gate once a row is past Want to play, which skips the scope's rule. +- **IGDB checked on status and score, never on removal, clearing or restore.** Removal must work during an IGDB outage and for a game IGDB later hides (AGENTS.md, Games, catalog filter). _Amended after the cross check on 2026-10-24:_ the restores first carried the gate too, which made the Undo of removing an unreleased Playing game always fail; a restore only puts back a state the user had, so it never advances and needs no IGDB. Want to play refuses only an unknown id and accepts a hidden one. Runner up: check only creating writes, which would let a move between existing statuses skip the gate. +- **Movie release wording.** "Releases {date}" or "Release date to be announced", through helpers that share `airStatus` and `UPCOMING_MESSAGES`, so both title pages read alike. Runner up: the shorter "TBA" from the scope note. +- **Removal Undo limited by a removal time in the payload.** The deleted row leaves no `updated_at` to measure from, so `remove_game` returns `removed_at = now()` and the restore accepts it only within 10 minutes and not in the future. A forged time can only bring back the caller's own game. Runner up: no window, which lets a stale tab restore a game removed hours ago. +- **No dates on the game page.** "Finished {month year}" arrives with the G5 cards; G4 stays status and score. Runner up: one date line under the pills. +- **Three glass pills in a group, toggles with `aria-pressed`.** One look across the title pages, and toggle semantics fit "click the pressed one to remove", which a radio group does not. Runner up: a joined segmented bar, a new glass component plus a `/showcase` entry. + +Decided while writing (no engineer input needed): + +- **Two restore functions, not one.** Undoing a status move is an update guarded on `updated_at`; undoing a removal is an insert guarded on the row's absence. One function branching on which case applies would hide two different guards behind one name. Runner up: one `restore_game` with an optional `p_removed_at`. +- **`BS004` for a score without Playing or Finished.** The app's own SQLSTATE, so the action recognises it without reading a message, like `BS001` for movies. Runner up: reuse `BS001`, which would mix a movie refusal into games. +- **`BS003` mapped to `write_failed` with its own `identity_locked` log outcome.** The functions never send a different id, so reaching it means a bug, and a distinct outcome keeps it from hiding among network failures. Runner up: fold it into `db_error`. +- **`igdb_unavailable` instead of reusing `tmdb_unavailable`.** The toast names the right source, and the movie and game error unions stay separate. Runner up: a shared `catalog_unavailable`, which would mean renaming the movie code. +- **`restore_removed_game` uses `on conflict do nothing` plus `if not found` → `P0002`.** A row that reappeared meanwhile (another tab added the game again) means the Undo is stale, not that it should overwrite. Chosen over catching `unique_violation` in an exception block after the cross check: both raise the same `P0002`, and this form needs no subtransaction, so the escape's `off` is easy to place on every path. Runner up: the exception block. +- **Keep `rate_game` and `BS004`** rather than a filtered PATCH that reads zero rows as `not_started` (the cross check's simpler option), for parity with `rate_movie`, plus a `p_rating between 1 and 10` guard so a direct call cannot clear through it. +- **Score picker opens on a move into Finished only when unscored**, and an unscored pill reads `Rate` (decided after the cross check): a game that already has a score does not need asking again. +- **Release helpers in `lib/catalog/game-page.ts`, delegating to the movie ones' building blocks.** Pure and free of `server-only`, so the slot, the actions and the G5 and G8 cards share one day boundary. Runner up: call `isMovieReleased` directly, whose name would mislead. diff --git a/docs/specs/0026-game-status-rating/verify.md b/docs/specs/0026-game-status-rating/verify.md new file mode 100644 index 00000000..772edf1f --- /dev/null +++ b/docs/specs/0026-game-status-rating/verify.md @@ -0,0 +1,44 @@ +# Verify: game status and rating · spec 0026 · updated 2026-10-24 +_Steps derived from spec 0026 acceptance criteria. `/check verify` runs these; `/test` locks the durable ones._ + +Run against the local stack (`pnpm dev:docker`), signed in as user A (`user-a@example.test` / `password-a`) unless a step says otherwise. Run `supabase db reset` first so the seed is clean (user A has 1942 as Playing with score 8). + +## UI / manual +- [x] Signed out, open `/games/1942` → the hero shows, and there are no pills and no release line under the IGDB rating → AC-1 +- [x] Signed in, open `/games/1942` → the pills appear already showing Playing pressed and a score pill reading `8`, with no flash of a default state → AC-1, AC-12 +- [x] With the screen reader tree (or devtools), check the group is labelled `Status for The Witcher 3: Wild Hunt`, the pills are named `Want to play: …`, `Playing: …`, `Finished: …` with `aria-pressed`, and the score pill is named `Your score for …: 8` → AC-2, AC-6 +- [x] Click Finished → it is pressed at once, the score stays 8, and no picker opens (the game already has a score) → AC-3, AC-6, AC-7 +- [x] Click Want to play → the score pill disappears and a toast reads `Moved to Want to play. Your score was removed.` with Undo; click Undo within 10 seconds → Finished and 8 come back, and in the database `added_at`, `started_at` and `finished_at` equal their values before the move → AC-5, AC-14 +- [x] Click Playing on a game with no score (or after clearing it) → no picker opens; then click Finished → the picker opens at once; press Escape → the game stays Finished and unscored → AC-7 +- [x] Open the score pill, pick 9 → the popover closes and the pill reads 9; open it again and press Clear rating → the pill reads `Rate`, the status stays → AC-6 +- [x] Click the pressed pill → the game is removed (no pill pressed) and a toast reads `Removed from your games.` with Undo; Undo → status, score and all three dates come back exactly → AC-4, AC-14 +- [x] Remove the game in a second tab, then click the pressed pill in the first tab → no toast, and the page settles on no status → AC-4 +- [x] Reload after each change, and open the page in a second browser signed in as user A → the same state shows → AC-12 +- [x] Open an unreleased game (IGDB release date after today, or none) → under the pills: `Releases {date}` or `Release date to be announced`; Playing and Finished are `aria-disabled` and do nothing; Want to play works → AC-8 +- [x] On an unreleased game stored as Playing (set it directly in the database), the pressed Playing pill still removes, Want to play still works, and the picker shows the release line with its ten choices disabled while Clear rating works → AC-8, AC-9 +- [x] Sign out in another tab, then click a pill → the change rolls back and a toast says the session expired with Sign in, which goes to `/sign-in?next=%2Fgames%2F1942` → AC-10, AC-11 +- [x] At 375px and 1280px: the pills wrap with no horizontal scroll, each tap target is at least 44px on mobile, focus is visible, and the score popover fits the screen → AC-17 +- [x] Keyboard only: Tab to each pill and press Enter or Space to set, remove, open the picker, move with the arrows and commit a score, and reach Undo in the toast → AC-17 + +## Commands +- [x] `pnpm test:db` (after `supabase db reset`) → all files pass, including `190-game-tracking-functions` and `020-security-configuration`, with `140-security-sweep` unchanged → AC-13, AC-14, AC-16 +- [x] `supabase db diff` after `supabase db reset` → `No schema changes found` → AC-16 +- [x] `pnpm db:types:check` → the types match the schema → AC-16 +- [x] `pnpm typecheck`, `pnpm lint`, `pnpm test`, `pnpm build` → all pass, and `/games/[id]` stays a partial prerender → AC-1, AC-16 +- [x] `pnpm vitest run app/games/actions.test.ts private-actions-boundary.test.ts` → the release gate, hidden game, IGDB failure and missing session cases pass → AC-9, AC-10, AC-15 +- [x] `supabase db push` before the PR merges (docs/deploy.md) → the five functions exist in the cloud project → AC-16 + +## Value sourcing checks +- [x] `user_id`: call `set_game_status` as user A for game 1942 → user B's row for 1942 stays Want to play with its seed date → value source `auth.uid()` +- [x] `game_id`: call `setGameStatus(0, "playing")` → `invalid_input`, nothing written → `gameIdSchema` +- [x] status target: double click a pill quickly → two different targets are sent (set, then remove), never the same one twice → optimistic state +- [x] cleared score Undo payload: in two tabs, move Finished (scored) to Want to play in both → only the first gets an Undo with the score → `prev_*` under the row lock +- [x] removal Undo payload and `removedAt`: remove, wait over 10 minutes (or move `removed_at` back in a direct call), then Undo → `Couldn't undo. Set the status again on this page.` → `remove_game` and its `now()` +- [x] restore window: change `updated_at` to 10 minutes ago and call `restore_game_status` → `P0002` → `updated_at` against `now()` +- [x] release gate server side: set a game with today's UTC date and one with tomorrow's → today's accepts Playing, tomorrow's returns `not_released` → `Game.releaseDate` against `todayUtc(new Date())` +- [x] release gate on the page: the same two games show no line and the `Releases {date}` line, deciding by the UTC day, not the local day → `requestTodayUtc()` +- [x] score display: a score of 7 shows `7`, none shows `Rate` with the name `… Not rated` → `formatPersonalScore` +- [x] Sign in return: the session expired toast's Sign in returns to `/games/{id}` → `returnPath` + +## Acceptance-criteria coverage +- AC-1: visitor and signed in steps, page test · AC-2: names and group step · AC-3: Finished click · AC-4: removal and Undo steps · AC-5: Want to play toast step · AC-6: score pick and clear · AC-7: auto open steps · AC-8: unreleased steps · AC-9: actions tests, unreleased stored step · AC-10: session step, boundary test · AC-11: rollback steps, component tests · AC-12: reload and second session · AC-13: pgTAP grants and lock · AC-14: Undo window checks · AC-15: actions tests (BS004, BS003, logs) · AC-16: commands · AC-17: 375px, 1280px and keyboard steps diff --git a/lib/catalog/game-page.test.ts b/lib/catalog/game-page.test.ts new file mode 100644 index 00000000..ef20706b --- /dev/null +++ b/lib/catalog/game-page.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; + +import { gameReleaseNote, isGameReleased } from "./game-page"; +import { isMovieReleased, movieReleaseNote } from "./movie-page"; + +/** + * covers: spec 0026, AC-8, AC-9 + * + * The game release gate is the movie rule by design, so a game and a movie + * open on the same UTC day. These cases pin the day boundary and that the two + * never drift. + */ +const today = "2026-10-24"; + +describe("isGameReleased", () => { + it("is true on the release day itself and after it (AC-9)", () => { + expect(isGameReleased(today, today)).toBe(true); + expect(isGameReleased("2015-05-19", today)).toBe(true); + }); + + it("is false the day before release, with no date, or a malformed date (AC-8)", () => { + expect(isGameReleased("2026-10-25", today)).toBe(false); + expect(isGameReleased(null, today)).toBe(false); + expect(isGameReleased("2026-13-40", today)).toBe(false); + }); + + it("agrees with the movie gate for every date (AGENTS.md Games)", () => { + for (const date of [today, "2026-10-23", "2026-10-25", null, "bad"]) { + expect(isGameReleased(date, today)).toBe(isMovieReleased(date, today)); + } + }); +}); + +describe("gameReleaseNote", () => { + it("prints the real date, and never invents one (AC-8)", () => { + expect(gameReleaseNote("2027-10-24")).toBe("Releases Oct 24, 2027"); + expect(gameReleaseNote(null)).toBe("Release date to be announced"); + expect(gameReleaseNote("2026-13-40")).toBe("Release date to be announced"); + }); + + it("reads exactly like the movie line", () => { + expect(gameReleaseNote("2027-01-02")).toBe(movieReleaseNote("2027-01-02")); + }); +}); diff --git a/lib/catalog/game-page.ts b/lib/catalog/game-page.ts new file mode 100644 index 00000000..19b5ba5b --- /dev/null +++ b/lib/catalog/game-page.ts @@ -0,0 +1,33 @@ +import { isMovieReleased, movieReleaseNote } from "@/lib/catalog/movie-page"; + +/** + * Whether a game can be Playing, Finished or scored yet (`AGENTS.md` + * "Games", spec 0026, AC-8, AC-9). + * + * The movie rule exactly, so a game and a movie flip on the same UTC day: out + * from the UTC day of IGDB's `first_release_date`, and a missing date counts + * as not released. It delegates rather than copies, so the two can never + * drift, and G5 and G8 reuse it for the library and the cards. + * + * @param releaseDate `Game.releaseDate`, the UTC day of IGDB's + * `first_release_date` (spec 0023). + * @param today The UTC day, read once: `requestTodayUtc()` in a render, + * `todayUtc(new Date())` in a Server Action. + */ +export function isGameReleased( + releaseDate: string | null, + today: string, +): boolean { + return isMovieReleased(releaseDate, today); +} + +/** + * The line an unreleased game shows under its status pills: "Releases Oct + * 24, 2026", or "Release date to be announced" with no date. The movie + * page's copy, so the two read alike. + * + * @param releaseDate `Game.releaseDate`. + */ +export function gameReleaseNote(releaseDate: string | null): string { + return movieReleaseNote(releaseDate); +} diff --git a/lib/supabase/database.types.ts b/lib/supabase/database.types.ts index 838e94be..ba88d58b 100644 --- a/lib/supabase/database.types.ts +++ b/lib/supabase/database.types.ts @@ -257,6 +257,10 @@ export type Database = { watched_at: string }[] } + rate_game: { + Args: { p_game_id: number; p_rating: number } + Returns: undefined + } rate_movie: { Args: { p_movie_id: number; p_rating: number } Returns: { @@ -276,10 +280,32 @@ export type Database = { isSetofReturn: false } } + remove_game: { + Args: { p_game_id: number } + Returns: { + removed_added_at: string + removed_at: string + removed_finished_at: string + removed_rating: number + removed_started_at: string + removed_status: Database["public"]["Enums"]["game_status"] + }[] + } restore_episodes_watched: { Args: { p_entries: Json; p_show_id: number } Returns: number } + restore_game_status: { + Args: { + p_added_at: string + p_finished_at?: string + p_game_id: number + p_rating?: number + p_started_at?: string + p_status: Database["public"]["Enums"]["game_status"] + } + Returns: undefined + } restore_movie_watched: { Args: { p_movie_id: number; p_rating?: number; p_watched_at: string } Returns: undefined @@ -288,6 +314,18 @@ export type Database = { Args: { p_movie_id: number } Returns: undefined } + restore_removed_game: { + Args: { + p_added_at: string + p_finished_at?: string + p_game_id: number + p_rating?: number + p_removed_at: string + p_started_at?: string + p_status: Database["public"]["Enums"]["game_status"] + } + Returns: undefined + } restore_show_tracking: { Args: { p_show_id: number; p_tracked_at: string } Returns: { @@ -304,6 +342,19 @@ export type Database = { isSetofReturn: false } } + set_game_status: { + Args: { + p_game_id: number + p_status: Database["public"]["Enums"]["game_status"] + } + Returns: { + prev_added_at: string + prev_finished_at: string + prev_rating: number + prev_started_at: string + prev_status: Database["public"]["Enums"]["game_status"] + }[] + } track_show: { Args: { p_show_id: number }; Returns: boolean } track_show_after_watch: { Args: { diff --git a/lib/tracking/game-intent.test.ts b/lib/tracking/game-intent.test.ts new file mode 100644 index 00000000..733eaef3 --- /dev/null +++ b/lib/tracking/game-intent.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "vitest"; + +import { applyGameIntent } from "./game-intent"; +import type { GameTrackingState } from "./types"; + +/** + * covers: spec 0026, AC-5, AC-6, AC-11 + * + * The reducer must give exactly what the SQL does, or the control flips + * twice. Each case names the function it mirrors. + */ +const NONE: GameTrackingState = { status: null, rating: null }; +const PLAYING_8: GameTrackingState = { status: "playing", rating: 8 }; + +describe("applyGameIntent", () => { + it("sets a first status with no score (set_game_status insert)", () => { + expect( + applyGameIntent(NONE, { kind: "status", value: "finished" }), + ).toEqual({ status: "finished", rating: null }); + }); + + it("keeps the score between Playing and Finished (set_game_status)", () => { + expect( + applyGameIntent(PLAYING_8, { kind: "status", value: "finished" }), + ).toEqual({ status: "finished", rating: 8 }); + }); + + it("nulls the score on Want to play (set_game_status)", () => { + expect( + applyGameIntent(PLAYING_8, { kind: "status", value: "want_to_play" }), + ).toEqual({ status: "want_to_play", rating: null }); + }); + + it("empties the state on removal (remove_game)", () => { + expect(applyGameIntent(PLAYING_8, { kind: "status", value: null })).toEqual( + NONE, + ); + }); + + it("scores Playing or Finished (rate_game)", () => { + expect( + applyGameIntent( + { status: "finished", rating: null }, + { kind: "rating", value: 9 }, + ), + ).toEqual({ status: "finished", rating: 9 }); + }); + + it("changes nothing for a score on no status or Want to play (rate_game BS004)", () => { + expect(applyGameIntent(NONE, { kind: "rating", value: 9 })).toEqual(NONE); + const want = { status: "want_to_play", rating: null } as const; + expect(applyGameIntent(want, { kind: "rating", value: 9 })).toEqual(want); + }); + + it("clears a score and keeps the status (the plain update)", () => { + expect(applyGameIntent(PLAYING_8, { kind: "rating", value: null })).toEqual( + { status: "playing", rating: null }, + ); + }); + + it("settles a queued Finished then score on the scored state (AC-7)", () => { + const queued = applyGameIntent( + applyGameIntent(NONE, { kind: "status", value: "finished" }), + { kind: "rating", value: 7 }, + ); + expect(queued).toEqual({ status: "finished", rating: 7 }); + }); +}); diff --git a/lib/tracking/game-intent.ts b/lib/tracking/game-intent.ts new file mode 100644 index 00000000..5db4c5fd --- /dev/null +++ b/lib/tracking/game-intent.ts @@ -0,0 +1,49 @@ +import type { GameStatus, GameTrackingState } from "./types"; + +/** + * Whether a status can carry a score: Playing or Finished, never Want to + * play or no status (`AGENTS.md` "Games", spec 0026, AC-6). The one copy the + * TypeScript side reads; `rate_game` and the table's check hold it in SQL. + */ +export function canHoldScore(status: GameStatus | null): boolean { + return status === "playing" || status === "finished"; +} + +/** + * One click on the game status control, as the target it asks for (spec + * 0026, AC-11). A null status is a removal. Targets rather than toggles, so + * replaying an intent gives the same state. + */ +export type GameIntent = + | { kind: "status"; value: GameStatus | null } + | { kind: "rating"; value: number | null }; + +/** + * The optimistic state a click produces, on top of the last state the server + * confirmed. + * + * It mirrors the SQL in `supabase/schemas/05-functions.sql`, so the control + * never flips twice: `set_game_status` keeps the score between Playing and + * Finished and nulls it on Want to play, `remove_game` deletes the row, and + * `rate_game` refuses a score on no status or Want to play, so that changes + * nothing here either. Clearing a score always lands. + * + * @param state The state to build on: the server prop, or an earlier guess. + * @param intent The click. + * @returns The state the database will hold once the write lands. + */ +export function applyGameIntent( + state: GameTrackingState, + intent: GameIntent, +): GameTrackingState { + if (intent.kind === "status") { + if (intent.value === null || intent.value === "want_to_play") { + return { status: intent.value, rating: null }; + } + return { status: intent.value, rating: state.rating }; + } + if (intent.value === null) return { ...state, rating: null }; + return canHoldScore(state.status) + ? { ...state, rating: intent.value } + : state; +} diff --git a/lib/tracking/game-state.test.ts b/lib/tracking/game-state.test.ts new file mode 100644 index 00000000..0364b699 --- /dev/null +++ b/lib/tracking/game-state.test.ts @@ -0,0 +1,99 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * covers: spec 0026, AC-1, AC-12 + * + * The read behind the game page's tracking slot. The session and the database + * are the boundaries, so those are what is replaced; the query builder + * records what it was asked, as in `movie-state.test.ts`. + */ +const getOptionalUser = vi.fn(); +vi.mock("@/lib/auth/user", () => ({ getOptionalUser })); + +const calls: { method: string; args: unknown[] }[] = []; +let response: { data: unknown; error: unknown } = { data: null, error: null }; + +function builder() { + const chain: Record = {}; + for (const method of ["from", "select", "eq"]) { + chain[method] = (...args: unknown[]) => { + calls.push({ method, args }); + return chain; + }; + } + chain.maybeSingle = () => Promise.resolve(response); + return chain; +} + +// The client itself must not be thenable, or awaiting `createClient()` would +// resolve straight to the query result. +const createClient = vi.fn(async () => ({ + from: (...args: unknown[]) => + (builder().from as (...a: unknown[]) => unknown)(...args), +})); +vi.mock("@/lib/supabase/server", () => ({ createClient })); + +const { getGameTracking } = await import("./game-state"); + +let warn: ReturnType; + +beforeEach(() => { + vi.stubEnv("NEXT_PUBLIC_SUPABASE_URL", "http://127.0.0.1:54321"); + vi.stubEnv("NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY", "sb_publishable_test"); + vi.stubEnv("NEXT_PUBLIC_SITE_URL", "http://localhost:3000"); + getOptionalUser.mockResolvedValue({ id: "user-a", email: "a@example.test" }); + response = { data: null, error: null }; + calls.length = 0; + warn = vi.spyOn(console, "warn").mockImplementation(() => {}); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + vi.clearAllMocks(); + warn.mockRestore(); +}); + +describe("getGameTracking", () => { + it("is signed out, with no session read, when the public env is missing (AC-1)", async () => { + vi.stubEnv("NEXT_PUBLIC_SUPABASE_URL", undefined); + expect(await getGameTracking(1942)).toEqual({ kind: "signed_out" }); + expect(getOptionalUser).not.toHaveBeenCalled(); + }); + + it("is signed out, with no query, for a visitor (AC-1)", async () => { + getOptionalUser.mockResolvedValue(null); + expect(await getGameTracking(1942)).toEqual({ kind: "signed_out" }); + expect(createClient).not.toHaveBeenCalled(); + }); + + it("returns no status and no score when the game is not in the list", async () => { + expect(await getGameTracking(1942)).toEqual({ + kind: "ok", + state: { status: null, rating: null }, + }); + }); + + it("maps the row and scopes the query to the session user and game (AC-12)", async () => { + response = { data: { status: "playing", rating: 8 }, error: null }; + expect(await getGameTracking(1942)).toEqual({ + kind: "ok", + state: { status: "playing", rating: 8 }, + }); + expect(calls).toEqual([ + { method: "from", args: ["user_game_state"] }, + { method: "select", args: ["status, rating"] }, + { method: "eq", args: ["user_id", "user-a"] }, + { method: "eq", args: ["game_id", 1942] }, + ]); + }); + + it("reports failed and logs only the event and outcome on a database error", async () => { + response = { + data: null, + error: { code: "XX000", message: "secret detail" }, + }; + expect(await getGameTracking(1942)).toEqual({ kind: "failed" }); + expect(warn).toHaveBeenCalledWith("game_tracking.read refused db_error"); + expect(JSON.stringify(warn.mock.calls)).not.toContain("secret detail"); + }); +}); diff --git a/lib/tracking/game-state.ts b/lib/tracking/game-state.ts new file mode 100644 index 00000000..f452d01a --- /dev/null +++ b/lib/tracking/game-state.ts @@ -0,0 +1,50 @@ +import "server-only"; + +import { getOptionalUser } from "@/lib/auth/user"; +import { publicEnvProblems } from "@/lib/env"; +import { createClient } from "@/lib/supabase/server"; + +import { logTrackingEvent, TRACKING_EVENT } from "./log"; +import { + EMPTY_GAME_TRACKING, + type GameTrackingState, + type TrackingRead, +} from "./types"; + +/** + * The signed in user's status and score for one game, or `signed_out`, or + * `failed` (spec 0026, AC-1, AC-12). + * + * Read per request and never inside `use cache`: the row belongs to one + * person, and a shared entry would hand it to the next visitor (`AGENTS.md` + * section 11). Only the game page's tracking slot calls it, inside its own + * Suspense boundary, so the page shell stays prerendered. It checks + * `publicEnvProblems()` first, as `getMovieTracking` does, so a clone with no + * Supabase configuration still serves the page with no control. + * + * @param gameId An id the page has already parsed and IGDB has confirmed. + */ +export async function getGameTracking( + gameId: number, +): Promise> { + if (publicEnvProblems()) return { kind: "signed_out" }; + + const user = await getOptionalUser(); + if (!user) return { kind: "signed_out" }; + + const supabase = await createClient(); + const { data, error } = await supabase + .from("user_game_state") + .select("status, rating") + .eq("user_id", user.id) + .eq("game_id", gameId) + .maybeSingle(); + + if (error) { + logTrackingEvent(TRACKING_EVENT.gameRead, "db_error"); + return { kind: "failed" }; + } + if (!data) return { kind: "ok", state: EMPTY_GAME_TRACKING }; + + return { kind: "ok", state: { status: data.status, rating: data.rating } }; +} diff --git a/lib/tracking/log.ts b/lib/tracking/log.ts index 9bdd6aeb..f89c9ca4 100644 --- a/lib/tracking/log.ts +++ b/lib/tracking/log.ts @@ -2,6 +2,7 @@ import "server-only"; import type { EpisodeTrackingError, + GameTrackingError, MovieTrackingError, ShowTrackingError, } from "./types"; @@ -35,6 +36,12 @@ export const TRACKING_EVENT = { showTrackingRead: "show_tracking.read", watchedEpisodesRead: "show_tracking.watched_read", showGridRead: "show_tracking.grid_read", + gameStatus: "game_tracking.status", + gameRate: "game_tracking.rate", + gameRemove: "game_tracking.remove", + gameRestoreStatus: "game_tracking.restore_status", + gameRestoreRemoved: "game_tracking.restore_removed", + gameRead: "game_tracking.read", } as const; export type TrackingEvent = @@ -44,12 +51,16 @@ export type TrackingEvent = * The classified reason. `forbidden` is a `42501` (a missing grant or a policy * refusal, which means a bug) and `db_error` is anything else from Postgres; * both reach the client as `write_failed` but stay apart here, so a broken - * grant is not mistaken for a flaky network. + * grant is not mistaken for a flaky network. `identity_locked` is a game + * write that tried to move a row (`BS003`), which no app path can reach + * (spec 0026, AC-15). */ export type TrackingOutcome = | MovieTrackingError | EpisodeTrackingError | ShowTrackingError + | GameTrackingError + | "identity_locked" | "forbidden" | "db_error"; diff --git a/lib/tracking/messages.test.ts b/lib/tracking/messages.test.ts index 3d32516e..037f9645 100644 --- a/lib/tracking/messages.test.ts +++ b/lib/tracking/messages.test.ts @@ -2,6 +2,9 @@ import { describe, expect, it } from "vitest"; import { EPISODE_TRACKING_MESSAGES, + GAME_STATUS_LABELS, + GAME_TRACKING_MESSAGES, + GAME_UNDO_MESSAGES, LIBRARY_COPY, LIBRARY_NOTES, MARK_NEXT_MESSAGES, @@ -222,3 +225,38 @@ describe("LIBRARY_NOTES, labels and the held section (spec 0020, AC-10, AC-12, A }); }); }); + +describe("game copy (spec 0026, AC-2, AC-4, AC-5)", () => { + it("labels the three pills in their order", () => { + expect(Object.values(GAME_STATUS_LABELS)).toEqual([ + "Want to play", + "Playing", + "Finished", + ]); + }); + + it("pins the two Undo toasts and the refused Undo line", () => { + expect(GAME_UNDO_MESSAGES).toEqual({ + removed: "Removed from your games.", + scoreCleared: "Moved to Want to play. Your score was removed.", + undoExpired: "Couldn't undo. Set the status again on this page.", + }); + expect(GAME_TRACKING_MESSAGES.undo_expired).toBe( + GAME_UNDO_MESSAGES.undoExpired, + ); + }); + + it("names IGDB, never TMDB or a movie, on every failure line", () => { + for (const line of Object.values(GAME_TRACKING_MESSAGES)) { + expect(line.length).toBeGreaterThan(0); + expect(line).not.toMatch(/TMDB|movie/i); + } + expect(GAME_TRACKING_MESSAGES.igdb_unavailable).toMatch(/IGDB/); + }); + + it("keeps the shared session copy, so the Sign in toast reads the same (AC-10)", () => { + expect(GAME_TRACKING_MESSAGES.session_expired).toBe( + TRACKING_MESSAGES.session_expired, + ); + }); +}); diff --git a/lib/tracking/messages.ts b/lib/tracking/messages.ts index b9367780..91c17de0 100644 --- a/lib/tracking/messages.ts +++ b/lib/tracking/messages.ts @@ -2,6 +2,8 @@ import type { MediaType } from "@/lib/catalog/media-type"; import type { EpisodeTrackingError, + GameStatus, + GameTrackingError, MovieTrackingError, ShowTrackingError, TrackingError, @@ -296,3 +298,41 @@ export const MISSING_SHOW_COPY = { /** The Stop tracking button's accessible name. */ stopLabel: "Stop tracking missing title", } as const; + +/** The game page's three status pills, in their order (spec 0026, AC-2). */ +export const GAME_STATUS_LABELS: Record = { + want_to_play: "Want to play", + playing: "Playing", + finished: "Finished", +}; + +/** What a refused game Undo says: change it again on the game page. */ +const GAME_UNDO_EXPIRED = "Couldn't undo. Set the status again on this page."; + +/** + * The game status control's failure copy (spec 0026, API surface): the movie + * copy for the shared classes, with the lines that name a game, IGDB in place + * of TMDB, and the game's own refusals. + */ +export const GAME_TRACKING_MESSAGES: Record = { + session_expired: SHARED_TRACKING_MESSAGES.session_expired, + write_failed: SHARED_TRACKING_MESSAGES.write_failed, + invalid_input: SHARED_TRACKING_MESSAGES.invalid_input, + not_found: "This game isn't available to track.", + undo_expired: GAME_UNDO_EXPIRED, + igdb_unavailable: "Couldn't reach IGDB. Try again.", + not_released: "This game isn't out yet.", + // Only a stale tab or a crafted call reaches it: the score pill shows only + // on Playing or Finished. + not_started: "Set Playing or Finished to score this game.", +}; + +/** + * The two game toasts that carry an Undo (spec 0026, AC-4, AC-5). A status + * change that removed nothing confirms itself through the pill alone. + */ +export const GAME_UNDO_MESSAGES = { + removed: "Removed from your games.", + scoreCleared: "Moved to Want to play. Your score was removed.", + undoExpired: GAME_UNDO_EXPIRED, +} as const; diff --git a/lib/tracking/schemas.test.ts b/lib/tracking/schemas.test.ts index 6cc21050..23c66d75 100644 --- a/lib/tracking/schemas.test.ts +++ b/lib/tracking/schemas.test.ts @@ -4,9 +4,12 @@ import { episodeIdListSchema, episodeRatingInputSchema, episodeWatchedInputSchema, + gameRestoreStateSchema, + gameStatusInputSchema, movieIdSchema, ratingInputSchema, ratingSchema, + restoreRemovedGameInputSchema, restoreShowTrackingInputSchema, restoreWatchedInputSchema, restoreWatchlistInputSchema, @@ -311,3 +314,86 @@ describe("restoreShowTrackingInputSchema (spec 0020, AC-3)", () => { expect(parse({ trackedAt: "2026-09-26T10:00:00Z" })).toBe(true); }); }); + +describe("game input schemas (spec 0026)", () => { + const PREVIOUS = { + status: "finished", + rating: 9, + addedAt: "2026-09-01T10:00:00+00:00", + startedAt: "2026-09-02T10:00:00+00:00", + finishedAt: "2026-09-20T10:00:00Z", + }; + + it("takes the three statuses and nothing else (AGENTS.md Games)", () => { + for (const status of ["want_to_play", "playing", "finished"]) { + expect( + gameStatusInputSchema.safeParse({ gameId: 1942, status }).success, + ).toBe(true); + } + for (const status of ["dropped", "paused", "", null]) { + expect( + gameStatusInputSchema.safeParse({ gameId: 1942, status }).success, + ).toBe(false); + } + }); + + it("refuses game id 0 (value source: gameIdSchema)", () => { + expect( + gameStatusInputSchema.safeParse({ gameId: 0, status: "playing" }).success, + ).toBe(false); + }); + + it("drops a client supplied user_id (AGENTS.md section 5)", () => { + expect( + gameStatusInputSchema.parse({ + gameId: 1942, + status: "playing", + user_id: "user-b", + }), + ).toEqual({ gameId: 1942, status: "playing" }); + }); + + it("accepts a full restore state with a Z or an offset (AC-14)", () => { + expect(gameRestoreStateSchema.safeParse(PREVIOUS).success).toBe(true); + }); + + it.each([ + ["Playing with no start", { status: "playing", startedAt: null }], + ["Finished with no finish", { status: "finished", finishedAt: null }], + ["Want to play with a score", { status: "want_to_play", rating: 7 }], + ["a date with no offset", { addedAt: "2026-09-01T10:00:00" }], + ["a score of 0", { rating: 0 }], + ])("refuses %s", (_, change) => { + expect( + gameRestoreStateSchema.safeParse({ ...PREVIOUS, ...change }).success, + ).toBe(false); + }); + + it("accepts Want to play with no score and no dates beyond added", () => { + expect( + gameRestoreStateSchema.safeParse({ + status: "want_to_play", + rating: null, + addedAt: PREVIOUS.addedAt, + startedAt: null, + finishedAt: null, + }).success, + ).toBe(true); + }); + + it("requires the removal time on a removal Undo (AC-4, AC-14)", () => { + expect( + restoreRemovedGameInputSchema.safeParse({ + gameId: 1942, + previous: PREVIOUS, + }).success, + ).toBe(false); + expect( + restoreRemovedGameInputSchema.safeParse({ + gameId: 1942, + previous: PREVIOUS, + removedAt: "2026-10-24T10:00:00+00:00", + }).success, + ).toBe(true); + }); +}); diff --git a/lib/tracking/schemas.ts b/lib/tracking/schemas.ts index 0899d8ef..141b9401 100644 --- a/lib/tracking/schemas.ts +++ b/lib/tracking/schemas.ts @@ -1,5 +1,8 @@ import { z } from "zod"; +import { canHoldScore } from "./game-intent"; +import { GAME_STATUSES } from "./types"; + /** * The input rules every movie tracking action applies before any Supabase or * TMDB call (spec 0007, AC-13). @@ -148,3 +151,61 @@ export const restoreShowTrackingInputSchema = z.object({ showId: tmdbIdSchema, undo: z.object({ trackedAt: pastInstantSchema }), }); + +/** An IGDB game id: the bounds `parseGameId` applies (spec 0026). */ +export const gameIdSchema = movieIdSchema; + +/** The three statuses, set by hand (`AGENTS.md` "Games"). */ +export const gameStatusSchema = z.enum(GAME_STATUSES); + +/** `setGameStatus`: a target status, never a toggle (spec 0026, AC-3). */ +export const gameStatusInputSchema = z.object({ + gameId: gameIdSchema, + status: gameStatusSchema, +}); + +/** `setGameRating`: a score from 1 to 10, or null to clear it (AC-6). */ +export const gameRatingInputSchema = z.object({ + gameId: gameIdSchema, + rating: ratingSchema.nullable(), +}); + +/** `removeGame`: only the id, since removal works from any status (AC-4). */ +export const removeGameInputSchema = z.object({ gameId: gameIdSchema }); + +const instantSchema = z.iso.datetime({ offset: true }); + +/** + * What a game Undo puts back (spec 0026, API surface). The dates are full ISO + * timestamps with an offset, as PostgREST returned them, and the refinement + * refuses the shapes the table's checks would (Playing with no start, + * Finished with no finish, a score on Want to play) as `invalid_input` + * before a round trip. The restores bound every date to the past again. + */ +export const gameRestoreStateSchema = z + .object({ + status: gameStatusSchema, + rating: ratingSchema.nullable(), + addedAt: instantSchema, + startedAt: instantSchema.nullable(), + finishedAt: instantSchema.nullable(), + }) + .refine( + (state) => + (state.status !== "playing" || state.startedAt !== null) && + (state.status !== "finished" || state.finishedAt !== null) && + (canHoldScore(state.status) || state.rating === null), + ); + +/** `restoreGameStatus`: the state `setGameStatus` returned as its Undo (AC-5). */ +export const restoreGameStatusInputSchema = z.object({ + gameId: gameIdSchema, + previous: gameRestoreStateSchema, +}); + +/** `restoreRemovedGame`: the row `removeGame` returned, and when (AC-4). */ +export const restoreRemovedGameInputSchema = z.object({ + gameId: gameIdSchema, + previous: gameRestoreStateSchema, + removedAt: instantSchema, +}); diff --git a/lib/tracking/supabase-error.test.ts b/lib/tracking/supabase-error.test.ts index 7c500b74..15df9689 100644 --- a/lib/tracking/supabase-error.test.ts +++ b/lib/tracking/supabase-error.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { + classifyGameTrackingError, classifyMovieTrackingError, classifyTrackingError, } from "./supabase-error"; @@ -88,3 +89,36 @@ describe("classifyMovieTrackingError", () => { }, ); }); + +describe("classifyGameTrackingError (spec 0026, AC-15)", () => { + it("reports rate_game's BS004 as not_started", () => { + expect(classifyGameTrackingError({ code: "BS004" })).toEqual({ + error: "not_started", + outcome: "not_started", + }); + }); + + it("shows the identity lock BS003 as a failed save, logged apart as identity_locked", () => { + expect(classifyGameTrackingError({ code: "BS003" })).toEqual({ + error: "write_failed", + outcome: "identity_locked", + }); + }); + + it("falls back to the shared classes for every other code", () => { + expect(classifyGameTrackingError({ code: "P0002" })).toEqual( + classifyTrackingError({ code: "P0002" }), + ); + expect(classifyGameTrackingError({ code: "42501" })).toEqual( + classifyTrackingError({ code: "42501" }), + ); + expect(classifyGameTrackingError({})).toEqual(classifyTrackingError({})); + }); + + it("leaves BS004 and BS003 out of the shared classifier", () => { + expect(classifyTrackingError({ code: "BS004" }).error).toBe("write_failed"); + expect(classifyTrackingError({ code: "BS003" }).outcome).not.toBe( + "identity_locked", + ); + }); +}); diff --git a/lib/tracking/supabase-error.ts b/lib/tracking/supabase-error.ts index ab830bbe..f98e576f 100644 --- a/lib/tracking/supabase-error.ts +++ b/lib/tracking/supabase-error.ts @@ -1,7 +1,11 @@ import "server-only"; import type { TrackingOutcome } from "./log"; -import type { MovieTrackingError, TrackingError } from "./types"; +import type { + GameTrackingError, + MovieTrackingError, + TrackingError, +} from "./types"; /** * Reduces a PostgREST error to the class the client sees and the outcome the @@ -28,7 +32,8 @@ import type { MovieTrackingError, TrackingError } from "./types"; * @param error Anything with an optional string `code`, as PostgREST returns. */ export function classifyTrackingError(error: { code?: string | null }): { - error: TrackingError; + /** Never `tmdb_unavailable`: no database error means TMDB failed. */ + error: Exclude; outcome: TrackingOutcome; } { switch (error.code) { @@ -65,3 +70,24 @@ export function classifyMovieTrackingError(error: { code?: string | null }): { } return classifyTrackingError(error); } + +/** + * `classifyTrackingError` plus the SQLSTATEs only a game write raises (spec + * 0026, AC-15): `BS004`, which `rate_game` raises for a score on a game that + * is not Playing or Finished, and `BS003`, the identity lock of spec 0024, + * which these functions cannot reach and so is logged apart as a bug. + * + * @param error Anything with an optional string `code`, as PostgREST returns. + */ +export function classifyGameTrackingError(error: { code?: string | null }): { + error: GameTrackingError; + outcome: TrackingOutcome; +} { + if (error.code === "BS004") { + return { error: "not_started", outcome: "not_started" }; + } + if (error.code === "BS003") { + return { error: "write_failed", outcome: "identity_locked" }; + } + return classifyTrackingError(error); +} diff --git a/lib/tracking/types.ts b/lib/tracking/types.ts index f90808c7..7ac4f0bd 100644 --- a/lib/tracking/types.ts +++ b/lib/tracking/types.ts @@ -5,6 +5,8 @@ * `MovieTrackingState` and branch on a `MovieTrackingError` to pick a toast. */ +import { Constants, type Database } from "@/lib/supabase/database.types"; + /** What the reads return and what the controls hold between clicks. */ export type MovieTrackingState = { inWatchlist: boolean; @@ -167,3 +169,75 @@ export type ShowTrackingResult = export type UntrackShowResult = | { ok: true; undo: ShowTrackingUndo | null } | { ok: false; error: ShowTrackingError }; + +/** + * A game's place in a person's list, set by hand (spec 0024, `AGENTS.md` + * "Games"). Taken from the generated enum, so a status added in SQL fails + * the type check everywhere it is not handled. + */ +export type GameStatus = Database["public"]["Enums"]["game_status"]; + +/** + * Every status, from the generated enum in its SQL order, which is also the + * pill order (spec 0026, AC-2). The Zod schema and the control both read it, + * so a status added in SQL reaches them with `pnpm db:types` alone. + */ +export const GAME_STATUSES = Constants.public.Enums.game_status; + +/** + * One game's stored state (spec 0026, API surface): no status means the game + * is not in the list. A score lives only on Playing or Finished. + */ +export type GameTrackingState = { + status: GameStatus | null; + /** The personal score, an integer from 1 to 10, or null when unrated. */ + rating: number | null; +}; + +/** No row: the game is not in the list. */ +export const EMPTY_GAME_TRACKING: GameTrackingState = { + status: null, + rating: null, +}; + +/** + * Everything an Undo puts back exactly (spec 0026, AC-4, AC-5): the status, + * the score and the three list dates, as the write that removed them + * returned them. Carried in the toast, never shown or edited. + */ +export type GameRestoreState = { + status: GameStatus; + rating: number | null; + addedAt: string; + startedAt: string | null; + finishedAt: string | null; +}; + +/** + * How to take back a game write: a move to Want to play that cleared a + * score (`status`), or a removal (`removed`), which also carries when it + * happened, since the 10 minute window is measured from it (AC-14). + */ +export type GameUndo = + | { kind: "status"; previous: GameRestoreState } + | { kind: "removed"; previous: GameRestoreState; removedAt: string }; + +/** + * The shared refusals with IGDB in place of TMDB, plus the game's own: a + * forward move before the release date (AC-9), and a score on a game that + * is not Playing or Finished (AC-6). + */ +export type GameTrackingError = + | Exclude + | "igdb_unavailable" + | "not_released" + | "not_started"; + +/** + * What every game action returns. As for movies, no state comes back: the + * control converges on the server prop `refresh()` delivers. `undo` is the + * one thing only the write knows. + */ +export type GameTrackingResult = + | { ok: true; undo?: GameUndo } + | { ok: false; error: GameTrackingError }; diff --git a/lib/tracking/write.ts b/lib/tracking/write.ts new file mode 100644 index 00000000..88ddcb95 --- /dev/null +++ b/lib/tracking/write.ts @@ -0,0 +1,119 @@ +import "server-only"; + +import { refresh } from "next/cache"; +import type { z } from "zod"; + +import { getOptionalUser } from "@/lib/auth/user"; +import { createClient } from "@/lib/supabase/server"; + +import { + logTrackingEvent, + type TrackingEvent, + type TrackingOutcome, +} from "./log"; + +type SupabaseClient = Awaited>; + +/** + * What a write step returns: PostgREST's typed data, and its error or null on + * success. `D` is the generated return type of the call, so the reader that + * consumes it is checked against `database.types.ts`. + */ +export type WriteStep = ( + supabase: SupabaseClient, + userId: string, +) => PromiseLike<{ data: D | null; error: { code?: string | null } | null }>; + +/** + * The shared order of every movie and game tracking action once its input is + * parsed (spec 0007, spec 0026, AC-10): the verified session, then the + * catalog check, then the write through Row Level Security, then `refresh()` + * on success only. One copy, so the two media types can never drift apart on + * the order or on what reaches a log line. + * + * It never throws: every failure is returned, so it reaches a toast rather + * than an error boundary. `user_id` is always the session's, never an + * argument (`AGENTS.md` section 5). The show and episode actions keep their + * own runner, because their bodies interleave TMDB reads with several writes. + * + * @param options.event The log event for a refusal. + * @param options.check The catalog check, run after the session: a refusal, + * or null to go on. Absent for a write that must work whatever the catalog + * says (a removal, a cleared score, a restore). + * @param options.write The Supabase call, given the request client and the + * user id. + * @param options.classify Maps a PostgREST error to the class the client sees + * and the outcome the log records. + * @param options.read For a write whose row carries something back (what it + * cleared, an Undo), reads it from the data. + * @returns `value` is what `read` found, or undefined. + */ +export async function runTrackingWrite({ + event, + check, + write, + classify, + read, +}: { + event: TrackingEvent; + check?: () => Promise; + write: WriteStep; + classify: (error: { code?: string | null }) => { + error: E; + outcome: TrackingOutcome; + }; + read?: (data: D | null) => R | undefined; +}): Promise< + | { ok: true; value: R | undefined } + | { ok: false; error: E | "session_expired" | "write_failed" } +> { + let value: R | undefined; + try { + const user = await getOptionalUser(); + if (!user) { + logTrackingEvent(event, "session_expired"); + return { ok: false, error: "session_expired" }; + } + + const refusal = check ? await check() : null; + if (refusal) { + logTrackingEvent(event, refusal); + return { ok: false, error: refusal }; + } + + const supabase = await createClient(); + const { data, error } = await write(supabase, user.id); + if (error) { + const classified = classify(error); + logTrackingEvent(event, classified.outcome); + return { ok: false, error: classified.error }; + } + value = read?.(data); + } catch { + // A network failure inside supabase-js, or a bug the catalog check + // rethrew. The error object is dropped on purpose: its message can carry + // request details that must not reach a log line (spec 0007, AC-21; + // spec 0026, AC-15). + logTrackingEvent(event, "db_error"); + return { ok: false, error: "write_failed" }; + } + + refresh(); + return { ok: true, value }; +} + +/** + * Parses a tracking action's arguments, or logs `invalid_input` and returns + * null (spec 0007, AC-13; spec 0026, AC-15). A Server Action is a public + * endpoint, so its arguments are external input (`AGENTS.md` section 11). + */ +export function parseTrackingInput( + schema: z.ZodType, + input: unknown, + event: TrackingEvent, +): T | null { + const parsed = schema.safeParse(input); + if (parsed.success) return parsed.data; + logTrackingEvent(event, "invalid_input"); + return null; +} diff --git a/private-actions-boundary.test.ts b/private-actions-boundary.test.ts index 30be96e8..93da71a0 100644 --- a/private-actions-boundary.test.ts +++ b/private-actions-boundary.test.ts @@ -37,16 +37,27 @@ const loadSeason = vi.fn(); vi.mock("@/app/shows/[id]/season/[number]/load-season", () => ({ loadSeason, })); +const readGameIncludingHidden = vi.fn(); +vi.mock("@/app/games/[id]/load-game", () => ({ readGameIncludingHidden })); const applyAutoCompletion = vi.fn(); vi.mock("@/lib/tracking/auto-completion", () => ({ applyAutoCompletion })); const movieActions = await import("@/app/movies/actions"); const showActions = await import("@/app/shows/actions"); const accountActions = await import("@/app/account/actions"); +const gameActions = await import("@/app/games/actions"); /** Valid arguments for each action, so the session check is what refuses. */ const SHOW = 1396; const MOVIE = 550; +const GAME = 1942; +const GAME_PREVIOUS = { + status: "finished", + rating: 8, + addedAt: "2026-09-01T10:00:00+00:00", + startedAt: null, + finishedAt: "2026-09-20T10:00:00+00:00", +} as const; function passwordForm(): FormData { const form = new FormData(); @@ -120,6 +131,23 @@ const PRIVATE_ACTIONS: Record< }), }, }, + "app/games/actions.ts": { + module: gameActions, + refusal: TRACKING_REFUSAL, + calls: { + setGameStatus: () => gameActions.setGameStatus(GAME, "playing"), + setGameRating: () => gameActions.setGameRating(GAME, 8), + removeGame: () => gameActions.removeGame(GAME), + restoreGameStatus: () => + gameActions.restoreGameStatus(GAME, GAME_PREVIOUS), + restoreRemovedGame: () => + gameActions.restoreRemovedGame( + GAME, + GAME_PREVIOUS, + "2026-10-24T10:00:00+00:00", + ), + }, + }, "app/account/actions.ts": { module: accountActions, refusal: failure(AUTH_OUTCOME.sessionExpired), @@ -174,6 +202,10 @@ beforeEach(() => { episodes: [{ id: 62085, episodeNumber: 1, airDate: "2020-01-01" }], }, }); + readGameIncludingHidden.mockResolvedValue({ + kind: "found", + game: { id: GAME, releaseDate: "2015-05-19", hidden: false }, + }); applyAutoCompletion.mockResolvedValue({ changed: null }); warn = vi.spyOn(console, "warn").mockImplementation(() => {}); }); diff --git a/supabase/migrations/20261024120000_game_tracking_functions.sql b/supabase/migrations/20261024120000_game_tracking_functions.sql new file mode 100644 index 00000000..11104f60 --- /dev/null +++ b/supabase/migrations/20261024120000_game_tracking_functions.sql @@ -0,0 +1,256 @@ +-- Game status and rating functions (spec 0026): `set_game_status`, +-- `rate_game`, `remove_game`, `restore_game_status` and +-- `restore_removed_game`, the writes behind the game page's status pills and +-- score. +-- +-- Written by hand from `supabase/schemas/05-functions.sql`, which stays the +-- source of truth. The revokes and grants are written out because the +-- declarative diff does not track function privileges. It only adds +-- functions, so the app deployed before it keeps working; push it before +-- merging its PR (docs/deploy.md). + +-- Game status and rating writes (spec 0026, API surface). +-- +-- The status machine and its dates are spec 0024's: `set_game_timestamps` +-- owns every date, and the table's checks keep a score on Playing or +-- Finished only. These functions are how the game page moves between the +-- states, and every write whose result an Undo needs reads the old values +-- and writes in one transaction, under a row lock, so two tabs can never +-- both receive the same Undo payload (AC-13). +-- +-- The same shape as every function in this file: SECURITY INVOKER with an +-- empty `search_path`, so the four row level security policies apply inside, +-- and `user_id` always `auth.uid()`, never an argument. The release gate is +-- not here; only the Server Action in `app/games/actions.ts` checks it +-- (spec 0024, security model). `lib/tracking/game-intent.ts` mirrors these +-- rules for the optimistic UI, and +-- `supabase/tests/190-game-tracking-functions.test.sql` pins them. + +-- Sets a status from any status, or none, in one click (AC-3). A move back to +-- Want to play nulls the score in the same statement, since a score lives +-- only on Playing or Finished (spec 0024, AC-2). The previous values come +-- back for the score cleared Undo (AC-5), read under the row lock; a missing +-- row locks nothing and leaves them all null, so the caller always gets +-- exactly one row. +create or replace function public.set_game_status( + p_game_id integer, + p_status public.game_status +) +returns table ( + prev_status public.game_status, + prev_rating smallint, + prev_added_at timestamptz, + prev_started_at timestamptz, + prev_finished_at timestamptz +) +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +declare + v_status public.game_status; + v_rating smallint; + v_added_at timestamptz; + v_started_at timestamptz; + v_finished_at timestamptz; +begin + select s.status, s.rating, s.added_at, s.started_at, s.finished_at + into v_status, v_rating, v_added_at, v_started_at, v_finished_at + from public.user_game_state s + where s.user_id = auth.uid() + and s.game_id = p_game_id + for update; + + insert into public.user_game_state as s (user_id, game_id, status) + values (auth.uid(), p_game_id, p_status) + on conflict (user_id, game_id) do update + set status = excluded.status, + rating = case when excluded.status = 'want_to_play' then null else s.rating end; + + return query select v_status, v_rating, v_added_at, v_started_at, v_finished_at; +end; +$$; + +-- Update only: a score never creates a row, and only a game being played or +-- finished takes one (AC-6). Anything else raises `BS004`, the app's own code +-- beside `BS001` to `BS003`, which the Server Action reports as +-- `not_started`. A null score is refused the same way, so clearing goes only +-- through the action's plain update, which moves no date. +create or replace function public.rate_game(p_game_id integer, p_rating smallint) +returns void +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +begin + update public.user_game_state + set rating = p_rating + where user_id = auth.uid() + and game_id = p_game_id + and status in ('playing', 'finished') + and p_rating between 1 and 10; + + if not found then + raise exception 'game_not_started' using errcode = 'BS004'; + end if; +end; +$$; + +-- Removes a game from the list (AC-4): the row goes, and what it held comes +-- back with the time of removal, for the Undo. A game that is already gone +-- (removed in another tab) deletes nothing and returns no row, which the +-- client reads as done with no Undo. +create or replace function public.remove_game(p_game_id integer) +returns table ( + removed_status public.game_status, + removed_rating smallint, + removed_added_at timestamptz, + removed_started_at timestamptz, + removed_finished_at timestamptz, + removed_at timestamptz +) +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +declare + v_status public.game_status; + v_rating smallint; + v_added_at timestamptz; + v_started_at timestamptz; + v_finished_at timestamptz; +begin + delete from public.user_game_state as s + where s.user_id = auth.uid() + and s.game_id = p_game_id + returning s.status, s.rating, s.added_at, s.started_at, s.finished_at + into v_status, v_rating, v_added_at, v_started_at, v_finished_at; + + if found then + return query select v_status, v_rating, v_added_at, v_started_at, v_finished_at, now(); + end if; +end; +$$; + +-- The two Undo restores (AC-14). Both store client supplied dates, so both +-- are bounded: every date in the past, only the caller's own row, only the +-- state the matching write produced, and only for 10 minutes. The window is +-- a staleness limit, not a security boundary (the same note as +-- `restore_movie_watchlist`): a forged call can only write the caller's own +-- private row. A refusal raises `P0002`, which reaches the user as +-- `undo_expired`. +-- +-- `bestats.restore_game` is what tells `set_game_timestamps` to keep the +-- given dates, so it is `on` only around the one statement and `off` again +-- before returning or raising. A statement the table's checks refuse rolls +-- the setting back with it. +-- +-- `p_added_at` (and `p_removed_at`) come before the defaulted parameters +-- because Postgres needs every parameter after a default to have one. + +-- Undoes a move to Want to play that cleared a score (AC-5): puts back the +-- old status, score and all three dates, on a row that is still Want to play +-- with no score and was changed in the last 10 minutes. +create or replace function public.restore_game_status( + p_game_id integer, + p_status public.game_status, + p_added_at timestamptz, + p_rating smallint default null, + p_started_at timestamptz default null, + p_finished_at timestamptz default null +) +returns void +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +begin + perform set_config('bestats.restore_game', 'on', true); + + update public.user_game_state + set status = p_status, + rating = p_rating, + added_at = p_added_at, + started_at = p_started_at, + finished_at = p_finished_at + where user_id = auth.uid() + and game_id = p_game_id + and status = 'want_to_play' + and rating is null + and updated_at > now() - interval '10 minutes' + and p_status in ('playing', 'finished') + and p_added_at <= now() + and (p_started_at is null or p_started_at <= now()) + and (p_finished_at is null or p_finished_at <= now()); + + if not found then + perform set_config('bestats.restore_game', 'off', true); + raise exception 'undo_expired' using errcode = 'P0002'; + end if; + + perform set_config('bestats.restore_game', 'off', true); +end; +$$; + +-- Undoes a removal (AC-4): puts the row back exactly as `remove_game` +-- returned it, within 10 minutes of `p_removed_at`. A row that is already +-- back (set again since, or a second Undo) inserts nothing and raises. +create or replace function public.restore_removed_game( + p_game_id integer, + p_status public.game_status, + p_added_at timestamptz, + p_removed_at timestamptz, + p_rating smallint default null, + p_started_at timestamptz default null, + p_finished_at timestamptz default null +) +returns void +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +begin + if ( + p_added_at <= now() + and (p_started_at is null or p_started_at <= now()) + and (p_finished_at is null or p_finished_at <= now()) + and p_removed_at <= now() + and p_removed_at > now() - interval '10 minutes' + ) is not true then + raise exception 'undo_expired' using errcode = 'P0002'; + end if; + + perform set_config('bestats.restore_game', 'on', true); + + insert into public.user_game_state ( + user_id, game_id, status, rating, added_at, started_at, finished_at + ) + values ( + auth.uid(), p_game_id, p_status, p_rating, p_added_at, p_started_at, p_finished_at + ) + on conflict (user_id, game_id) do nothing; + + if not found then + perform set_config('bestats.restore_game', 'off', true); + raise exception 'undo_expired' using errcode = 'P0002'; + end if; + + perform set_config('bestats.restore_game', 'off', true); +end; +$$; + +revoke all on function public.set_game_status(integer, public.game_status) from public, anon, authenticated; +revoke all on function public.rate_game(integer, smallint) from public, anon, authenticated; +revoke all on function public.remove_game(integer) from public, anon, authenticated; +revoke all on function public.restore_game_status(integer, public.game_status, timestamptz, smallint, timestamptz, timestamptz) from public, anon, authenticated; +revoke all on function public.restore_removed_game(integer, public.game_status, timestamptz, timestamptz, smallint, timestamptz, timestamptz) from public, anon, authenticated; +grant execute on function public.set_game_status(integer, public.game_status) to authenticated; +grant execute on function public.rate_game(integer, smallint) to authenticated; +grant execute on function public.remove_game(integer) to authenticated; +grant execute on function public.restore_game_status(integer, public.game_status, timestamptz, smallint, timestamptz, timestamptz) to authenticated; +grant execute on function public.restore_removed_game(integer, public.game_status, timestamptz, timestamptz, smallint, timestamptz, timestamptz) to authenticated; diff --git a/supabase/schemas/05-functions.sql b/supabase/schemas/05-functions.sql index c4c9ec2e..1335b8b0 100644 --- a/supabase/schemas/05-functions.sql +++ b/supabase/schemas/05-functions.sql @@ -371,6 +371,252 @@ grant execute on function public.track_show_after_watch(integer, smallint, boole grant execute on function public.untrack_show(integer) to authenticated; grant execute on function public.restore_show_tracking(integer, timestamptz) to authenticated; +-- Game status and rating writes (spec 0026, API surface). +-- +-- The status machine and its dates are spec 0024's: `set_game_timestamps` +-- owns every date, and the table's checks keep a score on Playing or +-- Finished only. These functions are how the game page moves between the +-- states, and every write whose result an Undo needs reads the old values +-- and writes in one transaction, under a row lock, so two tabs can never +-- both receive the same Undo payload (AC-13). +-- +-- The same shape as every function in this file: SECURITY INVOKER with an +-- empty `search_path`, so the four row level security policies apply inside, +-- and `user_id` always `auth.uid()`, never an argument. The release gate is +-- not here; only the Server Action in `app/games/actions.ts` checks it +-- (spec 0024, security model). `lib/tracking/game-intent.ts` mirrors these +-- rules for the optimistic UI, and +-- `supabase/tests/190-game-tracking-functions.test.sql` pins them. + +-- Sets a status from any status, or none, in one click (AC-3). A move back to +-- Want to play nulls the score in the same statement, since a score lives +-- only on Playing or Finished (spec 0024, AC-2). The previous values come +-- back for the score cleared Undo (AC-5), read under the row lock; a missing +-- row locks nothing and leaves them all null, so the caller always gets +-- exactly one row. +create or replace function public.set_game_status( + p_game_id integer, + p_status public.game_status +) +returns table ( + prev_status public.game_status, + prev_rating smallint, + prev_added_at timestamptz, + prev_started_at timestamptz, + prev_finished_at timestamptz +) +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +declare + v_status public.game_status; + v_rating smallint; + v_added_at timestamptz; + v_started_at timestamptz; + v_finished_at timestamptz; +begin + select s.status, s.rating, s.added_at, s.started_at, s.finished_at + into v_status, v_rating, v_added_at, v_started_at, v_finished_at + from public.user_game_state s + where s.user_id = auth.uid() + and s.game_id = p_game_id + for update; + + insert into public.user_game_state as s (user_id, game_id, status) + values (auth.uid(), p_game_id, p_status) + on conflict (user_id, game_id) do update + set status = excluded.status, + rating = case when excluded.status = 'want_to_play' then null else s.rating end; + + return query select v_status, v_rating, v_added_at, v_started_at, v_finished_at; +end; +$$; + +-- Update only: a score never creates a row, and only a game being played or +-- finished takes one (AC-6). Anything else raises `BS004`, the app's own code +-- beside `BS001` to `BS003`, which the Server Action reports as +-- `not_started`. A null score is refused the same way, so clearing goes only +-- through the action's plain update, which moves no date. +create or replace function public.rate_game(p_game_id integer, p_rating smallint) +returns void +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +begin + update public.user_game_state + set rating = p_rating + where user_id = auth.uid() + and game_id = p_game_id + and status in ('playing', 'finished') + and p_rating between 1 and 10; + + if not found then + raise exception 'game_not_started' using errcode = 'BS004'; + end if; +end; +$$; + +-- Removes a game from the list (AC-4): the row goes, and what it held comes +-- back with the time of removal, for the Undo. A game that is already gone +-- (removed in another tab) deletes nothing and returns no row, which the +-- client reads as done with no Undo. +create or replace function public.remove_game(p_game_id integer) +returns table ( + removed_status public.game_status, + removed_rating smallint, + removed_added_at timestamptz, + removed_started_at timestamptz, + removed_finished_at timestamptz, + removed_at timestamptz +) +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +declare + v_status public.game_status; + v_rating smallint; + v_added_at timestamptz; + v_started_at timestamptz; + v_finished_at timestamptz; +begin + delete from public.user_game_state as s + where s.user_id = auth.uid() + and s.game_id = p_game_id + returning s.status, s.rating, s.added_at, s.started_at, s.finished_at + into v_status, v_rating, v_added_at, v_started_at, v_finished_at; + + if found then + return query select v_status, v_rating, v_added_at, v_started_at, v_finished_at, now(); + end if; +end; +$$; + +-- The two Undo restores (AC-14). Both store client supplied dates, so both +-- are bounded: every date in the past, only the caller's own row, only the +-- state the matching write produced, and only for 10 minutes. The window is +-- a staleness limit, not a security boundary (the same note as +-- `restore_movie_watchlist`): a forged call can only write the caller's own +-- private row. A refusal raises `P0002`, which reaches the user as +-- `undo_expired`. +-- +-- `bestats.restore_game` is what tells `set_game_timestamps` to keep the +-- given dates, so it is `on` only around the one statement and `off` again +-- before returning or raising. A statement the table's checks refuse rolls +-- the setting back with it. +-- +-- `p_added_at` (and `p_removed_at`) come before the defaulted parameters +-- because Postgres needs every parameter after a default to have one. + +-- Undoes a move to Want to play that cleared a score (AC-5): puts back the +-- old status, score and all three dates, on a row that is still Want to play +-- with no score and was changed in the last 10 minutes. +create or replace function public.restore_game_status( + p_game_id integer, + p_status public.game_status, + p_added_at timestamptz, + p_rating smallint default null, + p_started_at timestamptz default null, + p_finished_at timestamptz default null +) +returns void +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +begin + perform set_config('bestats.restore_game', 'on', true); + + update public.user_game_state + set status = p_status, + rating = p_rating, + added_at = p_added_at, + started_at = p_started_at, + finished_at = p_finished_at + where user_id = auth.uid() + and game_id = p_game_id + and status = 'want_to_play' + and rating is null + and updated_at > now() - interval '10 minutes' + and p_status in ('playing', 'finished') + and p_added_at <= now() + and (p_started_at is null or p_started_at <= now()) + and (p_finished_at is null or p_finished_at <= now()); + + if not found then + perform set_config('bestats.restore_game', 'off', true); + raise exception 'undo_expired' using errcode = 'P0002'; + end if; + + perform set_config('bestats.restore_game', 'off', true); +end; +$$; + +-- Undoes a removal (AC-4): puts the row back exactly as `remove_game` +-- returned it, within 10 minutes of `p_removed_at`. A row that is already +-- back (set again since, or a second Undo) inserts nothing and raises. +create or replace function public.restore_removed_game( + p_game_id integer, + p_status public.game_status, + p_added_at timestamptz, + p_removed_at timestamptz, + p_rating smallint default null, + p_started_at timestamptz default null, + p_finished_at timestamptz default null +) +returns void +language plpgsql +volatile +security invoker +set search_path = '' +as $$ +begin + if ( + p_added_at <= now() + and (p_started_at is null or p_started_at <= now()) + and (p_finished_at is null or p_finished_at <= now()) + and p_removed_at <= now() + and p_removed_at > now() - interval '10 minutes' + ) is not true then + raise exception 'undo_expired' using errcode = 'P0002'; + end if; + + perform set_config('bestats.restore_game', 'on', true); + + insert into public.user_game_state ( + user_id, game_id, status, rating, added_at, started_at, finished_at + ) + values ( + auth.uid(), p_game_id, p_status, p_rating, p_added_at, p_started_at, p_finished_at + ) + on conflict (user_id, game_id) do nothing; + + if not found then + perform set_config('bestats.restore_game', 'off', true); + raise exception 'undo_expired' using errcode = 'P0002'; + end if; + + perform set_config('bestats.restore_game', 'off', true); +end; +$$; + +revoke all on function public.set_game_status(integer, public.game_status) from public, anon, authenticated; +revoke all on function public.rate_game(integer, smallint) from public, anon, authenticated; +revoke all on function public.remove_game(integer) from public, anon, authenticated; +revoke all on function public.restore_game_status(integer, public.game_status, timestamptz, smallint, timestamptz, timestamptz) from public, anon, authenticated; +revoke all on function public.restore_removed_game(integer, public.game_status, timestamptz, timestamptz, smallint, timestamptz, timestamptz) from public, anon, authenticated; +grant execute on function public.set_game_status(integer, public.game_status) to authenticated; +grant execute on function public.rate_game(integer, smallint) to authenticated; +grant execute on function public.remove_game(integer) to authenticated; +grant execute on function public.restore_game_status(integer, public.game_status, timestamptz, smallint, timestamptz, timestamptz) to authenticated; +grant execute on function public.restore_removed_game(integer, public.game_status, timestamptz, timestamptz, smallint, timestamptz, timestamptz) to authenticated; + -- Marks one episode watched. Marking an already watched episode again changes -- nothing, so a stale second tab cannot move the first watched date (AC-5). -- It never touches `rating`. `prior` reads the row as it was before the diff --git a/supabase/tests/020-security-configuration.test.sql b/supabase/tests/020-security-configuration.test.sql index d99c503e..4c37f5ae 100644 --- a/supabase/tests/020-security-configuration.test.sql +++ b/supabase/tests/020-security-configuration.test.sql @@ -1,5 +1,6 @@ -- Spec 0001 AC-2 and AC-4, the configuration half of the security boundary, --- and spec 0024 AC-9 for `user_game_state`, which carries the same lock. +-- spec 0024 AC-9 for `user_game_state`, which carries the same lock, and +-- spec 0026 AC-13 for the game functions that write it. -- -- These assert the shape of the lock rather than trying the door. The -- behavioural half is in 030. Both are needed: `supabase db advisors` checks @@ -8,7 +9,7 @@ -- only place the full requirement is actually proven. begin; -select plan(12); +select plan(14); -- Row level security enabled AND forced. Forced is the half nothing else -- checks: without it the rules do not apply to the table owner. @@ -168,5 +169,34 @@ select is( 'authenticated holds all four of select, insert, update and delete on every table' ); +-- The game writes (spec 0026, AC-13): execute is revoked from `public` and +-- `anon` and granted back to `authenticated` only. The declarative diff does +-- not track function privileges, so only the migration's own lines hold this. +select is( + ( + select count(*) from pg_proc p + where p.pronamespace = 'public'::regnamespace + and p.proname in ('set_game_status', 'rate_game', 'remove_game', + 'restore_game_status', 'restore_removed_game') + and (has_function_privilege('anon', p.oid, 'execute') + or exists ( + select 1 from aclexplode(coalesce(p.proacl, acldefault('f', p.proowner))) a + where a.grantee = 0)) + ), + 0::bigint, + 'anon and public may execute none of the five game functions' +); +select is( + ( + select count(*) from pg_proc p + where p.pronamespace = 'public'::regnamespace + and p.proname in ('set_game_status', 'rate_game', 'remove_game', + 'restore_game_status', 'restore_removed_game') + and has_function_privilege('authenticated', p.oid, 'execute') + ), + 5::bigint, + 'authenticated may execute all five game functions' +); + select * from finish(); rollback; diff --git a/supabase/tests/190-game-tracking-functions.test.sql b/supabase/tests/190-game-tracking-functions.test.sql new file mode 100644 index 00000000..b149d488 --- /dev/null +++ b/supabase/tests/190-game-tracking-functions.test.sql @@ -0,0 +1,347 @@ +-- Game status and rating functions (spec 0026, AC-3 to AC-6, AC-13, AC-14): +-- `set_game_status`, `rate_game`, `remove_game`, `restore_game_status` and +-- `restore_removed_game`, their guards and refusals, and their grants. +-- +-- Runs as user A from supabase/seed.sql, who owns games 1942 (Playing, 8) and +-- 119133 (Finished); user B owns 1942 (Want to play). New rows use ids the +-- seed does not, and the whole file is rolled back. `now()` is fixed for the +-- transaction, so a date equal to it was stamped here. Rows whose +-- `updated_at` must sit at a window boundary are pinned as `postgres` with +-- `session_replication_role = replica`, which skips the trigger that would +-- stamp it now, as 070 does. + +begin; +select plan(49); + +-- Shape (AC-13) + +select is( + (select count(*) from pg_proc + where pronamespace = 'public'::regnamespace + and proname in ('set_game_status', 'rate_game', 'remove_game', + 'restore_game_status', 'restore_removed_game') + and not prosecdef + and proconfig = array['search_path=""']), + 5::bigint, + 'all five functions are security invoker with an empty search_path' +); +select is( + (select count(*) from pg_proc p + where p.pronamespace = 'public'::regnamespace + and p.proname in ('set_game_status', 'rate_game', 'remove_game', + 'restore_game_status', 'restore_removed_game') + and (has_function_privilege('anon', p.oid, 'execute') + or exists ( + select 1 from aclexplode(coalesce(p.proacl, acldefault('f', p.proowner))) a + where a.grantee = 0))), + 0::bigint, + 'neither anon nor public may execute any of them' +); +select is( + (select count(*) from pg_proc + where pronamespace = 'public'::regnamespace + and proname in ('set_game_status', 'rate_game', 'remove_game', + 'restore_game_status', 'restore_removed_game') + and has_function_privilege('authenticated', oid, 'execute')), + 5::bigint, + 'authenticated may execute all five' +); + +set local request.jwt.claims = '{"role":"anon"}'; +set local role anon; +select throws_ok( + $$select * from public.set_game_status(5201, 'want_to_play')$$, + '42501', null, + 'anon cannot set a status' +); +select throws_ok( + $$select * from public.remove_game(1942)$$, + '42501', null, + 'anon cannot remove a game' +); +reset role; + +set local request.jwt.claims = '{"sub":"11111111-1111-1111-1111-111111111111","role":"authenticated"}'; +set local role authenticated; + +-- set_game_status (AC-3, AC-5) + +select results_eq( + $$select prev_status::text, prev_rating, prev_added_at from public.set_game_status(5201, 'want_to_play')$$, + $$values (null::text, null::smallint, null::timestamptz)$$, + 'a first status returns exactly one row of nulls' +); +select ok( + (select status = 'want_to_play' and rating is null and added_at = now() + from public.user_game_state where game_id = 5201), + 'a first status inserts the row with the trigger''s dates' +); +select lives_ok( + $$select * from public.set_game_status(5201, 'want_to_play')$$, + 'setting the same status again succeeds' +); +select is( + (select count(*) from public.user_game_state where game_id = 5201), + 1::bigint, + 'the same status twice leaves one row' +); + +select results_eq( + $$select prev_status::text, prev_rating from public.set_game_status(1942, 'finished')$$, + $$values ('playing'::text, 8::smallint)$$, + 'a move returns the previous status and score' +); +select ok( + (select status = 'finished' and rating = 8 + and started_at = '2026-09-05T10:00:00Z' and finished_at = now() + from public.user_game_state where game_id = 1942), + 'Playing to Finished keeps the score and the start, and stamps the finish' +); + +select results_eq( + $$select prev_status::text, prev_rating, prev_added_at, prev_started_at, prev_finished_at + from public.set_game_status(1942, 'want_to_play')$$, + $$values ('finished'::text, 8::smallint, '2026-09-01T10:00:00Z'::timestamptz, + '2026-09-05T10:00:00Z'::timestamptz, now())$$, + 'a move to Want to play returns every previous value, the Undo payload' +); +select ok( + (select status = 'want_to_play' and rating is null and added_at = now() + from public.user_game_state where game_id = 1942), + 'a move to Want to play clears the score in the same statement' +); + +select results_eq( + $$select prev_status::text, prev_rating from public.set_game_status(119133, 'want_to_play')$$, + $$values ('finished'::text, null::smallint)$$, + 'a move from unscored Finished returns a null previous score' +); + +select ok( + (select prosrc like '%for update%' from pg_proc + where oid = 'public.set_game_status(integer, public.game_status)'::regprocedure), + 'set_game_status locks the row before it reads the previous values' +); + +-- rate_game (AC-6) + +select * from public.set_game_status(5202, 'playing'); +select lives_ok( + $$select public.rate_game(5202, 7::smallint)$$, + 'a Playing game takes a score' +); +select is( + (select rating from public.user_game_state where game_id = 5202), + 7::smallint, + 'the score is stored' +); +select ok( + (select started_at = now() from public.user_game_state where game_id = 5202), + 'a score moves no date' +); +select throws_ok( + $$select public.rate_game(5299, 7::smallint)$$, + 'BS004', 'game_not_started', + 'a score on a game with no row is refused with BS004' +); +select throws_ok( + $$select public.rate_game(5201, 7::smallint)$$, + 'BS004', 'game_not_started', + 'a score on a Want to play game is refused with BS004' +); +select throws_ok( + $$select public.rate_game(5202, null::smallint)$$, + 'BS004', 'game_not_started', + 'a null score is refused, so only the plain update clears' +); +select throws_ok( + $$select public.rate_game(5202, 11::smallint)$$, + 'BS004', 'game_not_started', + 'a score out of range is refused' +); + +-- The plain update the action sends to clear a score. +update public.user_game_state set rating = null where game_id = 5202; +select ok( + (select rating is null and status = 'playing' and started_at = now() + from public.user_game_state where game_id = 5202), + 'clearing a score keeps the status and moves no date' +); + +-- remove_game (AC-4) + +select * from public.set_game_status(5203, 'finished'); +select public.rate_game(5203, 9::smallint); +select results_eq( + $$select removed_status::text, removed_rating, removed_added_at, removed_started_at, + removed_finished_at, removed_at + from public.remove_game(5203)$$, + $$values ('finished'::text, 9::smallint, now(), null::timestamptz, now(), now())$$, + 'removing returns what the row held and the time of removal' +); +select is_empty( + $$select 1 from public.user_game_state where game_id = 5203$$, + 'removing deletes the row' +); +select is_empty( + $$select * from public.remove_game(5203)$$, + 'removing a game that is already gone returns no row' +); + +-- restore_removed_game (AC-4, AC-14) + +select lives_ok( + $$select public.restore_removed_game(5203, 'finished', '2026-07-01T10:00:00Z', + now(), 9::smallint, null, '2026-07-10T10:00:00Z')$$, + 'a removal Undo inside the window succeeds' +); +select ok( + (select status = 'finished' and rating = 9 + and added_at = '2026-07-01T10:00:00Z' and started_at is null + and finished_at = '2026-07-10T10:00:00Z' + from public.user_game_state where game_id = 5203), + 'a removal Undo puts back the status, the score and all three dates exactly' +); +select is( + current_setting('bestats.restore_game', true), + 'off', + 'the restore escape reads off after a removal Undo' +); +select throws_ok( + $$select public.restore_removed_game(5203, 'finished', '2026-07-01T10:00:00Z', + now(), 9::smallint, null, '2026-07-10T10:00:00Z')$$, + 'P0002', 'undo_expired', + 'a removal Undo when the row already exists is refused' +); +select is( + current_setting('bestats.restore_game', true), + 'off', + 'the restore escape reads off after a refused removal Undo' +); +select throws_ok( + $$select public.restore_removed_game(5204, 'want_to_play', '2026-07-01T10:00:00Z', + now() - interval '10 minutes')$$, + 'P0002', 'undo_expired', + 'a removal Undo exactly 10 minutes later is refused' +); +select lives_ok( + $$select public.restore_removed_game(5204, 'want_to_play', '2026-07-01T10:00:00Z', + now() - interval '9 minutes 59 seconds')$$, + 'a removal Undo just inside 10 minutes succeeds' +); +select throws_ok( + $$select public.restore_removed_game(5205, 'want_to_play', '2026-07-01T10:00:00Z', + now() + interval '1 second')$$, + 'P0002', 'undo_expired', + 'a removal time in the future is refused' +); +select throws_ok( + $$select public.restore_removed_game(5205, 'want_to_play', now() + interval '1 day', now())$$, + 'P0002', 'undo_expired', + 'a future added date is refused' +); +select throws_ok( + $$select public.restore_removed_game(5205, 'playing', '2026-07-01T10:00:00Z', now(), + null, now() + interval '1 day')$$, + 'P0002', 'undo_expired', + 'a future start is refused' +); +select throws_ok( + $$select public.restore_removed_game(5205, 'playing', '2026-07-01T10:00:00Z', now())$$, + '23514', null, + 'a Playing state with no start still breaks the table check' +); +select is( + current_setting('bestats.restore_game', true), + 'off', + 'the restore escape is off again after a refused statement' +); + +-- restore_game_status (AC-5, AC-14) + +select * from public.set_game_status(5206, 'playing'); +update public.user_game_state set rating = 6 where game_id = 5206; +select * from public.set_game_status(5206, 'want_to_play'); + +select throws_ok( + $$select public.restore_game_status(5206, 'want_to_play', '2026-07-01T10:00:00Z')$$, + 'P0002', 'undo_expired', + 'a status Undo to Want to play is refused' +); +select throws_ok( + $$select public.restore_game_status(5206, 'playing', now() + interval '1 day', + 6::smallint, '2026-07-02T10:00:00Z')$$, + 'P0002', 'undo_expired', + 'a status Undo with a future date is refused' +); +select lives_ok( + $$select public.restore_game_status(5206, 'playing', '2026-07-01T10:00:00Z', + 6::smallint, '2026-07-02T10:00:00Z')$$, + 'a status Undo inside the window succeeds' +); +select ok( + (select status = 'playing' and rating = 6 + and added_at = '2026-07-01T10:00:00Z' and started_at = '2026-07-02T10:00:00Z' + and finished_at is null + from public.user_game_state where game_id = 5206), + 'a status Undo puts back the status, the score and all three dates exactly' +); +select is( + current_setting('bestats.restore_game', true), + 'off', + 'the restore escape reads off after a status Undo' +); +select throws_ok( + $$select public.restore_game_status(5206, 'playing', '2026-07-01T10:00:00Z', + 6::smallint, '2026-07-02T10:00:00Z')$$, + 'P0002', 'undo_expired', + 'a second status Undo, on a row no longer Want to play, is refused' +); +select is( + current_setting('bestats.restore_game', true), + 'off', + 'the restore escape reads off after a refused status Undo' +); + +-- The window boundary, measured from `updated_at`. +select * from public.set_game_status(5207, 'want_to_play'); +select * from public.set_game_status(5208, 'want_to_play'); +reset role; +set local session_replication_role = replica; +update public.user_game_state set updated_at = now() - interval '10 minutes' +where user_id = '11111111-1111-1111-1111-111111111111' and game_id = 5207; +update public.user_game_state set updated_at = now() - interval '9 minutes 59 seconds' +where user_id = '11111111-1111-1111-1111-111111111111' and game_id = 5208; +set local session_replication_role = origin; +set local role authenticated; + +select throws_ok( + $$select public.restore_game_status(5207, 'finished', '2026-07-01T10:00:00Z', + null, null, '2026-07-03T10:00:00Z')$$, + 'P0002', 'undo_expired', + 'a status Undo on a row changed exactly 10 minutes ago is refused' +); +select lives_ok( + $$select public.restore_game_status(5208, 'finished', '2026-07-01T10:00:00Z', + null, null, '2026-07-03T10:00:00Z')$$, + 'a status Undo on a row changed just inside 10 minutes succeeds' +); + +-- Isolation: user A's calls never reach user B's row for the same game. + +select * from public.remove_game(1942); +select throws_ok( + $$select public.rate_game(1942, 5::smallint)$$, + 'BS004', 'game_not_started', + 'user A cannot score a game only user B has' +); + +set local request.jwt.claims = '{"sub":"22222222-2222-2222-2222-222222222222","role":"authenticated"}'; +select ok( + (select status = 'want_to_play' and added_at = '2026-09-10T10:00:00Z' + from public.user_game_state where game_id = 1942), + 'user B''s row is untouched by every call user A made' +); + +select * from finish(); +rollback;