diff --git a/backend/prisma/migrations/20260804231500_spell_class_class_name_index/migration.sql b/backend/prisma/migrations/20260804231500_spell_class_class_name_index/migration.sql new file mode 100644 index 00000000..5d12a410 --- /dev/null +++ b/backend/prisma/migrations/20260804231500_spell_class_class_name_index/migration.sql @@ -0,0 +1,16 @@ +-- #1712 (folded in from #1711's review): `SpellClass`'s only existing index +-- is the UNIQUE (spellId, className) composite from #1711's migration — +-- className is not the LEADING column there, so a `?class=` lookup +-- (`classMemberships: { some: { className } }`, spells.ts) can't use it and +-- falls back to a sequential scan. Harmless at today's ~600 membership rows; +-- the 2014 content slices (#1713-#1721) are about to multiply that row +-- count, so the index earns its keep before they land rather than after. +-- +-- Hand-written, not `prisma migrate dev`-generated: a pure additive index +-- needs no destructive-DDL workaround, but this repo's migrations for this +-- table are already hand-written (#1711's join migration) and Prisma 7.8's +-- `migrate dev` would otherwise want to regenerate the whole diff against +-- the drifted (already-hand-migrated) shadow database. + +-- CreateIndex +CREATE INDEX "SpellClass_className_idx" ON "SpellClass"("className"); diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index bb3334ca..5969f8be 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -1603,6 +1603,13 @@ model SpellClass { className String // lowercase class name, e.g. "wizard" @@unique([spellId, className]) + // #1712 (folded in from #1711's review): className isn't the leading + // column of the unique(spellId, className) index above, so the ?class= + // route filter (classMemberships.some, spells.ts) can't use it — a + // dedicated index before the 2014 content slices (#1713-#1721) multiply + // the row count. See the hand-written migration's own comment for why this + // needed a manual migration.sql rather than `prisma migrate dev`. + @@index([className]) } // Baseline equipment catalog, same role as Species/CharacterClass above: seeded diff --git a/backend/src/lib/character/character-create.ts b/backend/src/lib/character/character-create.ts index 7710969a..e94547c9 100644 --- a/backend/src/lib/character/character-create.ts +++ b/backend/src/lib/character/character-create.ts @@ -31,7 +31,7 @@ import { } from "@/lib/inventory/starting-equipment-package.js"; import { creationSpellEntry } from "@/lib/spellcasting/spellcasting.js"; import { clampPreparedToLimit, type SpellEntry } from "@/lib/spellcasting/spell-state.js"; -import { classesOf, SPELL_CLASS_MEMBERSHIP_SELECT } from "@/lib/spellcasting/spell-classes.js"; +import { classesOf, rejectCrossEditionSpellForks, SPELL_CLASS_MEMBERSHIP_SELECT } from "@/lib/spellcasting/spell-classes.js"; import { subclassGateLevel } from "@/lib/leveling/effective-levels.js"; import { DEFAULT_RULES_EDITION } from "@/lib/rules/edition.js"; import { crossEditionRejection, resolveEditionRow, withEditionOrShared } from "@/lib/rules/catalog-edition.js"; @@ -1483,6 +1483,11 @@ async function resolveCreationSpells( const rows = allIds.length ? await prisma.spell.findMany({ where: { id: { in: allIds } }, include: SPELL_CLASS_MEMBERSHIP_SELECT }) : []; + // #1712: reject a submitted id that's provably the WRONG edition's fork — + // see rejectCrossEditionSpellForks's own comment for why this doesn't + // reject every 2014 pick just because today's catalog is 2024-tagged. + const forkError = await rejectCrossEditionSpellForks(rows, edition); + if (forkError) return { ok: false, status: 400, error: forkError }; const byId = new Map(rows.map((r) => [r.id, { ...r, classes: classesOf(r) }])); const maxLevel = maxSpellLevelForClass(className, 1, subclass, edition); diff --git a/backend/src/lib/leveling/level-up-transaction.ts b/backend/src/lib/leveling/level-up-transaction.ts index 4eac4959..e40b6b66 100644 --- a/backend/src/lib/leveling/level-up-transaction.ts +++ b/backend/src/lib/leveling/level-up-transaction.ts @@ -19,7 +19,7 @@ import { import { applyResourceOpInTx, type ResourceOperation } from "@/lib/classes/resources.js"; import { applySpellcastingOpInTx, type LearnSpellOperation, type SpellcastingOperation } from "@/lib/spellcasting/spellcasting.js"; import { normalizeSpellcastingMutable } from "@/lib/spellcasting/spell-state.js"; -import { classesOf, SPELL_CLASS_MEMBERSHIP_SELECT } from "@/lib/spellcasting/spell-classes.js"; +import { classesOf, rejectCrossEditionSpellForks, SPELL_CLASS_MEMBERSHIP_SELECT } from "@/lib/spellcasting/spell-classes.js"; import { advancingHitDie, applyLevelUpHpInTx, @@ -340,14 +340,22 @@ type SpellPickRow = { id: string; name: string; level: number; classes: string[] async function loadPickCatalogRows( cantripOps: LearnSpellOperation[], spellOps: LearnSpellOperation[], + edition: RulesEdition, ): Promise<{ rowById: Map; levelOf: (op: LearnSpellOperation) => number | undefined }> { const ids = [...cantripOps, ...spellOps].map((o) => o.spellId).filter((id): id is string => Boolean(id)); const rows = ids.length ? await prisma.spell.findMany({ where: { id: { in: ids } }, - select: { id: true, name: true, level: true, ...SPELL_CLASS_MEMBERSHIP_SELECT }, + select: { id: true, name: true, level: true, edition: true, ...SPELL_CLASS_MEMBERSHIP_SELECT }, }) : []; + // #1712: reject an id that's provably the WRONG edition's fork before it + // ever reaches assertOnSpellList/assertCantripEligibility below — see + // rejectCrossEditionSpellForks's own comment for why this doesn't reject + // every 2014 pick just because today's catalog is 2024-tagged (would + // regress #1729's shipped 2014 known-caster level-up). + const forkError = await rejectCrossEditionSpellForks(rows, edition); + if (forkError) throw new InvalidLevelUpError(forkError); // Flattened to SpellPickRow's `classes: string[]` here (#1711) so the // eligibility checks below (assertOnSpellList, assertCantripEligibility) // never see the join shape — one seam resolves membership, not two. @@ -508,10 +516,11 @@ async function assertPickSpellEligibility( submission: LevelUpSubmission, steps: LevelUpStep[], className: string, + edition: RulesEdition, ): Promise { const cantripOps = submission.cantripsLearned ?? []; const spellOps = submission.spellsLearned ?? []; - const { rowById, levelOf } = await loadPickCatalogRows(cantripOps, spellOps); + const { rowById, levelOf } = await loadPickCatalogRows(cantripOps, spellOps, edition); assertCantripVsLeveledPlacement(cantripOps, spellOps, levelOf); const gate = resolveNewSpellsGate(steps); @@ -543,7 +552,7 @@ export async function applyLevelUpTransaction( await resolveLevelUpContext(characterId, submission.target, submission.subclassId); const steps = validateLevelUpSubmission(planCharacter, targetEntry, chosenSubclassName, submission, pickedSubclassFeatureRows); - await assertPickSpellEligibility(submission, steps, targetEntry.name); + await assertPickSpellEligibility(submission, steps, targetEntry.name, planCharacter.edition); const ops = buildLevelUpOps(steps, submission); diff --git a/backend/src/lib/spellcasting/spell-classes.ts b/backend/src/lib/spellcasting/spell-classes.ts index 320068dd..44bd5486 100644 --- a/backend/src/lib/spellcasting/spell-classes.ts +++ b/backend/src/lib/spellcasting/spell-classes.ts @@ -1,3 +1,8 @@ +import { prisma } from "@/lib/core/prisma.js"; +import { resolveEditionRow } from "@/lib/rules/catalog-edition.js"; +import { RULES_EDITION_LABELS } from "@/lib/rules/edition.js"; +import type { RulesEdition } from "@character-sheet/shared-types"; + /** * Shared read-side shape for the Spell↔SpellClass join (#1711, F2 of epic * #1517's 2014 catalog fork). Every membership READ composes @@ -15,3 +20,101 @@ export const SPELL_CLASS_MEMBERSHIP_SELECT = { export function classesOf(spell: { classMemberships: { className: string }[] }): string[] { return spell.classMemberships.map((m) => m.className); } + +/** + * Cross-edition admission check for CLIENT-SUPPLIED spell ids, already + * resolved to rows by resolveCreationSpells / loadPickCatalogRows (#1712). + * + * Deliberately NOT crossEditionRejection's plain "row.edition must be null or + * match" (catalog-edition.ts): the spell catalog is still mid-migration + * (epic #1517) — today's ~109 rows are ALL tagged EDITION_2024 with no 2014 + * counterpart yet (the 2014 content slices, #1713-#1721, are what will + * populate spells-2014/*.ts), so a plain tag-mismatch reject would 400 every + * leveled spell a 2014 character tries to learn or scribe, regressing + * #1729's shipped 2014 known-caster level-up. This rejects a row ONLY once a + * genuine fork exists: when another row shares its name AND + * resolveEditionRow prefers THAT row for the requesting edition, the + * submitted row is provably the wrong fork. With no better candidate (the + * whole catalog today), the single existing row is admitted regardless of + * its own tag — the same "shared until proven otherwise" posture the route + * takes when it falls back through the NULL row. + * + * Batches every mismatched row's name into ONE extra query rather than one + * per row. Returns the first rejection message found (message-returning, not + * throwing — same rationale as crossEditionRejection: callers wrap it in + * their own domain error shape). + */ +export async function rejectCrossEditionSpellForks( + rows: { id: string; name: string; edition: RulesEdition | null }[], + edition: RulesEdition, +): Promise { + const mismatched = rows.filter((row) => row.edition !== null && row.edition !== edition); + if (mismatched.length === 0) return null; + + const names = [...new Set(mismatched.map((row) => row.name))]; + const candidates = await prisma.spell.findMany({ + where: { name: { in: names } }, + select: { id: true, name: true, edition: true }, + }); + const candidatesByName = new Map(); + for (const candidate of candidates) { + const group = candidatesByName.get(candidate.name); + if (group) group.push(candidate); + else candidatesByName.set(candidate.name, [candidate]); + } + + for (const row of mismatched) { + const resolved = resolveEditionRow(candidatesByName.get(row.name) ?? [], edition); + if (resolved && resolved.id !== row.id) { + return `${row.name} is ${RULES_EDITION_LABELS[row.edition!]} content, not usable by a ${RULES_EDITION_LABELS[edition]} character`; + } + } + return null; +} + +/** + * List-serving counterpart to rejectCrossEditionSpellForks, for GET + * /api/spells (#1712) — same "shared until proven otherwise" posture, applied + * to WHICH rows are offered rather than which submitted id is admitted. + * + * Groups candidates by name and prefers, in order: the exact-edition row, the + * shared (edition: null) row, and — the one place this diverges from + * resolveEditionCatalog (catalog-edition.ts) — the group's remaining row when + * neither exists. That divergence is deliberate: resolveEditionCatalog's + * plain exact-then-NULL rule is correct for Feat/Subclass/Background, whose + * catalogs already have full coverage on both editions, so "no match" always + * means a genuine edition-exclusive row. The spell catalog does NOT have that + * coverage yet (epic #1517 mid-migration) — today's ~109 rows are ALL tagged + * EDITION_2024 with no 2014 counterpart, so treating a bare tag mismatch as + * "not in this edition's catalog" would empty the creation/level-up picker + * for every 2014 caster and block character creation outright (caught by + * creation.spec.ts's 2014 Warlock e2e test, which documents "level1SpellPicks + * is edition-invariant, so a 2014 Warlock still walks it exactly like the + * 2024 case" as existing, deliberate product behavior). + * + * This is inert once a genuine fork lands: a name with BOTH a 2014 and a + * 2024 row still resolves to exactly the requesting edition's own row (the + * exact-match branch wins before the fallback ever runs) — proven by + * spells.test.ts's fork-disjointness suite. Only a name with a SINGLE, + * single-edition-tagged row (today's whole real catalog) falls through to + * the graceful branch and is served to both editions until #1713-#1721 give + * it a real 2014 sibling. + */ +export function resolveSpellCatalogForEdition( + rows: T[], + edition: RulesEdition, +): T[] { + const byName = new Map(); + for (const row of rows) { + const group = byName.get(row.name); + if (group) group.push(row); + else byName.set(row.name, [row]); + } + const resolved: T[] = []; + for (const group of byName.values()) { + const exact = group.find((row) => row.edition === edition); + const shared = group.find((row) => row.edition === null); + resolved.push(exact ?? shared ?? group[0]); + } + return resolved; +} diff --git a/backend/src/routes/catalog/__tests__/spells.test.ts b/backend/src/routes/catalog/__tests__/spells.test.ts index 84f1d6ce..18c99e12 100644 --- a/backend/src/routes/catalog/__tests__/spells.test.ts +++ b/backend/src/routes/catalog/__tests__/spells.test.ts @@ -75,8 +75,14 @@ async function seedFixtures() { await seedSpellClasses(utility.id, UTILITY_SPELL_CLASSES); } -function get(path: string) { - return supertest.agent(app).set("Cookie", COOKIE).get(path); +// `?edition=` is now REQUIRED (#1712) — every existing call in this file +// exercises `?class=`/`?maxLevel=`/membership behavior, not the edition gate +// itself, so this helper appends a default edition rather than touching every +// call site. The dedicated 400/fork describe blocks below call supertest +// directly (or pass an explicit edition) where the param IS the thing under test. +function get(path: string, edition: string = "EDITION_2024") { + const sep = path.includes("?") ? "&" : "?"; + return supertest.agent(app).set("Cookie", COOKIE).get(`${path}${sep}edition=${edition}`); } function names(body: { name: string }[]): string[] { @@ -234,3 +240,113 @@ describe("GET /api/spells — class membership served from the SpellClass join ( expect(await prisma.spellClass.findMany({ where: { spellId: damage.id } })).toEqual([]); }); }); + +// #1712: `?edition=` is now REQUIRED — reverses #1377's "no ?edition=" (the +// docstring this route carried at spells.ts:18 before this slice). Absent and +// unrecognized both 400, matching featsRouter/referenceRouter's precedent +// (#1411/#1412) exactly, including the two distinct messages. +describe("GET /api/spells — ?edition= is required (#1712)", () => { + it("400s with no ?edition= at all, rather than serving a flat cross-edition catalog", async () => { + const res = await supertest(app).get("/api/spells").set("Cookie", COOKIE); + expect(res.status).toBe(400); + expect(res.body.error).toBe("Missing required query parameter: edition"); + }); + + it("400s an unrecognized ?edition= value, with a message distinct from the missing-param one", async () => { + const res = await supertest(app).get("/api/spells?edition=bogus").set("Cookie", COOKIE); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/^Unknown edition: /); + }); + + it("400s for edition even when class/maxLevel are also present", async () => { + const res = await supertest(app).get("/api/spells?class=wizard&maxLevel=3").set("Cookie", COOKIE); + expect(res.status).toBe(400); + expect(res.body.error).toBe("Missing required query parameter: edition"); + }); +}); + +// #1712: the real plumbing proof — a GENUINE 2014/2024 fork (same name, two +// rows) must resolve to exactly ONE row per requesting edition. A lone +// single-edition-tagged row with NO sibling — today's entire real ~109-row +// catalog, all EDITION_2024 with no 2014 counterpart (2014 content slices +// #1713-#1721 haven't landed) — is graceful instead: served to BOTH editions +// until a real sibling exists (resolveSpellCatalogForEdition's own comment +// has the full reasoning; a stricter "exclude on bare tag mismatch" version +// emptied the 2014 creation picker and broke creation.spec.ts's 2014 Warlock +// e2e test, which documents spells as edition-invariant today by design). +describe("GET /api/spells — genuine edition fork resolves to one row per edition (#1712)", () => { + const FORK_NAME = "Test Fork Catalog Spell"; + const LONE_2024_NAME = "Test Lone 2024-Tagged Catalog Spell"; + + function forkRow(name: string, description: string) { + return { + name, + level: 1, + school: "evocation" as const, + castingTime: "1 action", + range: "30 feet", + duration: "Instantaneous", + description, + concentration: false, + ritual: false, + cantripScaling: false, + }; + } + + afterAll(async () => { + await prisma.spell.deleteMany({ + where: { name: { in: [FORK_NAME, LONE_2024_NAME] } }, + }); + }); + + it("a name with both a 2014 and a 2024 row resolves to exactly the requesting edition's own row", async () => { + const row2014 = forkRow(FORK_NAME, "The PHB'14 text."); + const row2024 = forkRow(FORK_NAME, "The SRD 5.2 text."); + const fork2014 = await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2014" }, { ...row2014, edition: "EDITION_2014" }, row2014); + const fork2024 = await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2024" }, { ...row2024, edition: "EDITION_2024" }, row2024); + + const res2014 = await get("/api/spells", "EDITION_2014"); + const matches2014 = res2014.body.filter((s: { name: string }) => s.name === FORK_NAME); + expect(matches2014).toHaveLength(1); + expect(matches2014[0].id).toBe(fork2014.id); + expect(matches2014[0].description).toBe("The PHB'14 text."); + + const res2024 = await get("/api/spells", "EDITION_2024"); + const matches2024 = res2024.body.filter((s: { name: string }) => s.name === FORK_NAME); + expect(matches2024).toHaveLength(1); + expect(matches2024[0].id).toBe(fork2024.id); + expect(matches2024[0].description).toBe("The SRD 5.2 text."); + }); + + it("once a 2014 sibling exists, the 2024 row STOPS leaking into the 2014 response (proof the graceful fallback yields to a real fork)", async () => { + const row2024 = forkRow(FORK_NAME, "The SRD 5.2 text."); + await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2024" }, { ...row2024, edition: "EDITION_2024" }, row2024); + + // Before the 2014 sibling exists: graceful fallback serves the lone 2024 + // row to a 2014 request too (today's real-catalog behavior). + const before = await get("/api/spells", "EDITION_2014"); + expect(names(before.body).filter((n: string) => n === FORK_NAME)).toHaveLength(1); + + // Once the sibling lands, the fork becomes genuine and exact-match wins — + // the 2024 row no longer reaches a 2014 request. + const row2014 = forkRow(FORK_NAME, "The PHB'14 text."); + const fork2014 = await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2014" }, { ...row2014, edition: "EDITION_2014" }, row2014); + const after = await get("/api/spells", "EDITION_2014"); + const matches = after.body.filter((s: { name: string }) => s.name === FORK_NAME); + expect(matches).toHaveLength(1); + expect(matches[0].id).toBe(fork2014.id); + }); + + it("a lone EDITION_2024-tagged row with no sibling is served to a 2014 request too (graceful — matches today's real catalog)", async () => { + const row2024 = forkRow(LONE_2024_NAME, "Ordinary 2024-tagged content, no 2014 fork yet."); + const lone = await upsertEditionRow(prisma.spell, { name: LONE_2024_NAME, edition: "EDITION_2024" }, { ...row2024, edition: "EDITION_2024" }, row2024); + + const res2014 = await get("/api/spells", "EDITION_2014"); + const matches2014 = res2014.body.filter((s: { name: string }) => s.name === LONE_2024_NAME); + expect(matches2014).toHaveLength(1); + expect(matches2014[0].id).toBe(lone.id); + + const res2024 = await get("/api/spells", "EDITION_2024"); + expect(names(res2024.body)).toContain(LONE_2024_NAME); + }); +}); diff --git a/backend/src/routes/catalog/spells.ts b/backend/src/routes/catalog/spells.ts index 930e16ae..94e7661b 100644 --- a/backend/src/routes/catalog/spells.ts +++ b/backend/src/routes/catalog/spells.ts @@ -2,8 +2,9 @@ import { Router } from "express"; import { parseClassFilterOr400 } from "@/lib/http/parse-class-param.js"; import { parseMaxSpellLevelOr400 } from "@/lib/http/parse-max-spell-level-param.js"; +import { requireEditionOr400 } from "@/lib/http/parse-edition-param.js"; import { prisma } from "@/lib/core/prisma.js"; -import { classesOf, SPELL_CLASS_MEMBERSHIP_SELECT } from "@/lib/spellcasting/spell-classes.js"; +import { classesOf, resolveSpellCatalogForEdition, SPELL_CLASS_MEMBERSHIP_SELECT } from "@/lib/spellcasting/spell-classes.js"; export const spellsRouter = Router(); @@ -12,13 +13,33 @@ export const spellsRouter = Router(); * as GET /api/items feeds the inventory editor. Ordered by level then name * so the UI can group by level without sorting client-side. * - * `?class=` and `?maxLevel=` are OPTIONAL, unlike `?edition=` elsewhere: the - * creation ceremony asks for one class's legal band, while the sheet's picker - * legitimately wants everything. Server-applied so the eligibility rule — on the - * class's list, inside the legal level band — has exactly one home (#1377). - * No `?edition=` yet: Spell carries an `edition` column (#1710, foundation - * for the 2014 catalog) but this route doesn't filter by it — every row - * returns regardless of edition. Wiring `?edition=` in is F2/F3's job. + * `?class=` and `?maxLevel=` stay OPTIONAL: the creation ceremony asks for + * one class's legal band, while the sheet's picker legitimately wants + * everything (within one edition). Server-applied so the eligibility rule — + * on the class's list, inside the legal level band — has exactly one home + * (#1377). + * + * `?edition=` is now REQUIRED (#1712, F3 of epic #1517 — reverses #1377's "no + * `?edition=`"): Spell has carried an `edition` column since #1710, but this + * route didn't filter by it until now. Same required-param shape as + * featsRouter/referenceRouter (#1411/#1412): absent 400s, unrecognized 400s. + * + * Resolution is NOT `withEditionOrShared` + `resolveEditionCatalog` (the + * feats.ts/reference.ts pattern) — deliberately: those catalogs already have + * full coverage on both editions, so a bare tag mismatch always means a + * genuine edition-exclusive row there. The spell catalog does not have that + * coverage yet (today's ~109 rows are ALL tagged EDITION_2024 with no 2014 + * counterpart), so that pattern would empty the picker for every 2014 caster + * and block character creation outright. `resolveSpellCatalogForEdition` + * (spell-classes.ts) is the spell-specific variant: same exact-then-shared + * preference, but falls back to a name's only candidate rather than + * excluding it — see that function's own comment for the full reasoning and + * the e2e regression (creation.spec.ts's 2014 Warlock test) that caught the + * stricter version. Still resolves a genuine 2014/2024 fork correctly (the + * exact-match branch wins before the fallback ever runs); only a + * single-edition-tagged name with no sibling — today's whole real catalog — + * takes the graceful branch, until the 2014 content slices (#1713-#1721) + * give it one. * * `?class=` filters through the SpellClass join (#1711), not a scalar * column — `classMemberships.some` finds rows with at least one matching @@ -27,12 +48,15 @@ export const spellsRouter = Router(); * spellList.ts consume the response unchanged. */ spellsRouter.get("/spells", async (req, res) => { + const edition = requireEditionOr400(req, res); + if (edition === undefined) return; + const classFilter = parseClassFilterOr400(req, res); if (!classFilter.ok) return; const levelFilter = parseMaxSpellLevelOr400(req, res); if (!levelFilter.ok) return; - const spells = await prisma.spell.findMany({ + const rows = await prisma.spell.findMany({ where: { ...(classFilter.className ? { classMemberships: { some: { className: classFilter.className } } } : {}), ...(levelFilter.maxLevel === undefined ? {} : { level: { lte: levelFilter.maxLevel } }), @@ -40,6 +64,7 @@ spellsRouter.get("/spells", async (req, res) => { include: SPELL_CLASS_MEMBERSHIP_SELECT, orderBy: [{ level: "asc" }, { name: "asc" }], }); + const spells = resolveSpellCatalogForEdition(rows, edition); res.json( spells.map((row) => ({ diff --git a/backend/src/routes/character/__tests__/character-create-spells.test.ts b/backend/src/routes/character/__tests__/character-create-spells.test.ts index ad629cb6..806b789a 100644 --- a/backend/src/routes/character/__tests__/character-create-spells.test.ts +++ b/backend/src/routes/character/__tests__/character-create-spells.test.ts @@ -6,6 +6,7 @@ import { prisma } from "@/lib/core/prisma.js"; import { ensureTestOwner } from "@/test-support/owner.js"; import { authCookie } from "@/test-support/auth.js"; import { seededSpeciesAnchor } from "@/test-support/species.js"; +import { upsertEditionRow } from "@/lib/rules/catalog-edition.js"; // #1131: the creation spell/cantrip picker. A level-1 caster (Warlock: 2 cantrips // + 2 prepared spells per SRD 5.2) finishes with a prepared spellbook; a @@ -360,3 +361,97 @@ describe("POST /api/characters — wizard spellbook vs. prepared cap (#1513)", ( expect(res.body.spellcasting.preparedSpellCount).toBe(2); }); }); + +// #1712: cross-edition admission — resolveCreationSpells rejects a submitted +// spell id that is provably the WRONG edition's fork of a name (a same-named +// row the requesting edition actually resolves to exists). Today's real +// catalog has no forks (2014 content slices haven't landed), so this proves +// the mechanism with a fixture fork rather than real catalog rows — the two +// existing describe blocks above already prove a 2014/2024 creation accepts +// today's (unforked, EDITION_2024-tagged) real catalog unchanged. +describe("POST /api/characters — cross-edition spell-fork rejection (#1712)", () => { + const FORK_NAME = "CreateSpells1712 Fork Cantrip"; + + async function seedFork() { + const row2014 = { + name: FORK_NAME, level: 0, school: "evocation" as const, castingTime: "1 action", range: "30 feet", + duration: "Instantaneous", description: "The PHB'14 text.", concentration: false, ritual: false, cantripScaling: true, + }; + const row2024 = { ...row2014, description: "The SRD 5.2 text." }; + const fork2014 = await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2014" }, { ...row2014, edition: "EDITION_2014" }, row2014); + const fork2024 = await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2024" }, { ...row2024, edition: "EDITION_2024" }, row2024); + for (const spellId of [fork2014.id, fork2024.id]) { + await prisma.spellClass.upsert({ + where: { spellId_className: { spellId, className: "warlock" } }, + create: { spellId, className: "warlock" }, + update: {}, + }); + } + return { fork2014, fork2024 }; + } + + // A second warlock cantrip id, explicitly excluding FORK_NAME — warlockPicks() + // takes an unordered `take: 2` off the live catalog, and once the fork rows + // exist as warlock-tagged level-0 spells they're eligible to be picked BY + // that query too, which would silently duplicate the fork id in a two-cantrip + // submission (a "chosen only once" 400 masking the assertion under test). + async function otherWarlockCantripId(): Promise { + const row = await prisma.spell.findFirstOrThrow({ + where: { classMemberships: { some: { className: "warlock" } }, level: 0, name: { not: FORK_NAME } }, + select: { id: true }, + }); + return row.id; + } + + afterAll(async () => { + await prisma.spell.deleteMany({ where: { name: FORK_NAME } }); + }); + + it("rejects a 2024 creation submitting the 2014 fork's id, naming the spell", async () => { + const { fork2014 } = await seedFork(); + const otherCantrip = await otherWarlockCantripId(); + const picks = await warlockPicks(); + const res = await create({ + ...BASE, + name: "CreateSpells1712 Wrong2014", + classes: [{ name: "Warlock" }], + spells: { cantripIds: [fork2014.id, otherCantrip], spellIds: picks.spellIds }, + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe(`${FORK_NAME} is 2014 rules content, not usable by a 2024 rules character`); + }); + + it("rejects a 2014 creation submitting the 2024 fork's id, naming the spell", async () => { + const { fork2024 } = await seedFork(); + const otherCantrip = await otherWarlockCantripId(); + // Warlock has no 2014-tagged cantrip catalog yet — pairing the wrong-fork id + // with the OTHER real (unforked, EDITION_2024) cantrip id it accepts + // elsewhere in this file is enough: this test targets the fork check + // specifically, not the full 2014 Warlock creation count. + const picks = await warlockPicks(); + const res = await create({ + ...BASE, + name: "CreateSpells1712 Wrong2024", + rulesEdition: "EDITION_2014", + classes: [{ name: "Warlock" }], + spells: { cantripIds: [fork2024.id, otherCantrip], spellIds: picks.spellIds }, + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe(`${FORK_NAME} is 2024 rules content, not usable by a 2014 rules character`); + }); + + it("admits the requesting edition's OWN fork — the rejection is fork-specific, not a blanket cross-edition ban", async () => { + const { fork2024 } = await seedFork(); + const otherCantrip = await otherWarlockCantripId(); + const picks = await warlockPicks(); + const res = await create({ + ...BASE, + name: "CreateSpells1712 RightFork", + classes: [{ name: "Warlock" }], + spells: { cantripIds: [fork2024.id, otherCantrip], spellIds: picks.spellIds }, + }); + expect(res.status, res.body.error ?? "").toBe(201); + const names = (res.body.spellcasting.spells as Array<{ name: string }>).map((s) => s.name); + expect(names).toContain(FORK_NAME); + }); +}); diff --git a/backend/src/routes/character/__tests__/level-up-transaction.test.ts b/backend/src/routes/character/__tests__/level-up-transaction.test.ts index e276ff7c..a077babc 100644 --- a/backend/src/routes/character/__tests__/level-up-transaction.test.ts +++ b/backend/src/routes/character/__tests__/level-up-transaction.test.ts @@ -6,6 +6,7 @@ import { Prisma } from "@/generated/prisma/client.js"; import { prisma } from "@/lib/core/prisma.js"; import { ensureTestOwner } from "@/test-support/owner.js"; import { authCookie } from "@/test-support/auth.js"; +import { upsertEditionRow } from "@/lib/rules/catalog-edition.js"; const OWNER_ID = "owner-level-up-tx"; let COOKIE: string; @@ -1694,6 +1695,95 @@ describe("POST …/level-up/transactions — Warlock 3→4 cantrip + spell (#113 }); }); +// #1712: cross-edition admission for the level-up learn path — +// loadPickCatalogRows rejects a submitted spellId that's provably the WRONG +// edition's fork of a name (a same-named row the character's OWN edition +// actually resolves to exists). Reuses the Warlock 3→4 fixture shape above. +// Today's real catalog has no forks yet (2014 content slices haven't +// landed), so a fixture fork proves the mechanism; the Bard Magical Secrets +// and #1509 known-caster describe blocks above already prove a 2014 +// character's level-up accepts today's (unforked, EDITION_2024-tagged) real +// catalog unchanged. +describe("POST …/level-up/transactions — cross-edition spell-fork rejection (#1712)", () => { + const CHAR_ID = "lvtx-1712-fork"; + const FORK_NAME = "LevelUpTx1712 Fork Cantrip"; + + beforeEach(async () => { + const warlock = await prisma.characterClass.findFirstOrThrow({ where: { name: "Warlock" } }); + const theFiend = (await prisma.subclass.findFirstOrThrow({ where: { classId: warlock.id, name: "The Fiend" } })).id; + await prisma.character.create({ + data: { + ...BASE, + ownerId: OWNER_ID, + id: CHAR_ID, + name: "LevelUpTx1712 Warlock", + experiencePoints: 2700, // level 4 threshold; hitDice.total 3 → 1 pending + hitPoints: { current: 22, max: 22, temp: 0, deathSaves: { successes: 0, failures: 0 } }, + hitDice: { total: 3, die: "d8", spent: 0 }, + abilityScores: { strength: 8, dexterity: 14, constitution: 14, intelligence: 10, wisdom: 10, charisma: 16 }, + spellcasting: { slotsUsed: {}, arcanumUsed: {}, spells: [], concentratingOn: null }, + classEntries: { create: [{ name: "warlock", subclass: "The Fiend", subclassId: theFiend, classId: warlock.id, position: 0, level: 3 }] }, + }, + }); + }); + + afterEach(async () => { + await prisma.spell.deleteMany({ where: { name: FORK_NAME } }); + }); + + async function seedFork() { + const row2014 = { + name: FORK_NAME, level: 0, school: "evocation" as const, castingTime: "1 action", range: "30 feet", + duration: "Instantaneous", description: "The PHB'14 text.", concentration: false, ritual: false, cantripScaling: true, + }; + const row2024 = { ...row2014, description: "The SRD 5.2 text." }; + const fork2014 = await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2014" }, { ...row2014, edition: "EDITION_2014" }, row2014); + const fork2024 = await upsertEditionRow(prisma.spell, { name: FORK_NAME, edition: "EDITION_2024" }, { ...row2024, edition: "EDITION_2024" }, row2024); + for (const spellId of [fork2014.id, fork2024.id]) { + await prisma.spellClass.upsert({ + where: { spellId_className: { spellId, className: "warlock" } }, + create: { spellId, className: "warlock" }, + update: {}, + }); + } + return { fork2014, fork2024 }; + } + + it("rejects a 2024 character's level-up submitting the 2014 fork's id, naming the spell", async () => { + const { fork2014 } = await seedFork(); + const entry = await prisma.characterClassEntry.findFirstOrThrow({ where: { characterId: CHAR_ID } }); + const spell = await prisma.spell.findFirstOrThrow({ where: { classMemberships: { some: { className: "warlock" } }, level: 1 }, select: { id: true } }); + + const res = await post(CHAR_ID, { + target: { kind: "existing", classEntryId: entry.id }, + hp: { method: "average" }, + advancement: { type: "takeAsi", increases: [{ ability: "charisma", amount: 2 }] }, + spellsLearned: [{ type: "learnSpell", spellId: spell.id }], + cantripsLearned: [{ type: "learnSpell", spellId: fork2014.id }], + }); + expect(res.status).toBe(400); + expect(res.body.error).toBe(`${FORK_NAME} is 2014 rules content, not usable by a 2024 rules character`); + expect(await eventCount(CHAR_ID)).toBe(0); + }); + + it("admits the character's OWN edition fork — the rejection is fork-specific, not a blanket cross-edition ban", async () => { + const { fork2024 } = await seedFork(); + const entry = await prisma.characterClassEntry.findFirstOrThrow({ where: { characterId: CHAR_ID } }); + const spell = await prisma.spell.findFirstOrThrow({ where: { classMemberships: { some: { className: "warlock" } }, level: 1 }, select: { id: true } }); + + const res = await post(CHAR_ID, { + target: { kind: "existing", classEntryId: entry.id }, + hp: { method: "average" }, + advancement: { type: "takeAsi", increases: [{ ability: "charisma", amount: 2 }] }, + spellsLearned: [{ type: "learnSpell", spellId: spell.id }], + cantripsLearned: [{ type: "learnSpell", spellId: fork2024.id }], + }); + expect(res.status, res.body.error ?? "").toBe(200); + const names = res.body.spellcasting.spells.map((s: { name: string }) => s.name); + expect(names).toContain(FORK_NAME); + }); +}); + // #1131: adding a first level in a new class routes through the SAME ceremony // (target {kind:"new"}), not a creation-only fork. A caster second class picks // its level-1 spells + cantrips; a Fighter second class commits its fighting style. diff --git a/frontend/e2e/global-setup.ts b/frontend/e2e/global-setup.ts index d72ea7a8..62965f9b 100644 --- a/frontend/e2e/global-setup.ts +++ b/frontend/e2e/global-setup.ts @@ -410,8 +410,10 @@ async function resolveSpeciesId(cookie: string, name: string): Promise { } // Resolve spell names → catalog ids via GET /api/spells (#1131 create-body picks). +// `?edition=` is REQUIRED since #1712 — EDITION_2024 always, same as +// resolveSpeciesId above: no persona this file declares needs 2014. async function resolveSpellIds(cookie: string, names: string[]): Promise { - const response = await api(cookie, "/api/spells"); + const response = await api(cookie, "/api/spells?edition=EDITION_2024"); if (!response.ok) throw new Error(`Failed to load spells: ${response.status}`); const catalog = (await response.json()) as { id: string; name: string }[]; const byName = new Map(catalog.map((s) => [s.name, s.id])); diff --git a/frontend/e2e/helpers/api.ts b/frontend/e2e/helpers/api.ts index 322d5f15..01b315d9 100644 --- a/frontend/e2e/helpers/api.ts +++ b/frontend/e2e/helpers/api.ts @@ -270,7 +270,8 @@ export async function learnSpells( characterId: string, spellNames: string[], ): Promise { - const catalogResponse = await request.get("/api/spells"); + // `?edition=` is required (#1712); every e2e persona is a default-2024 character. + const catalogResponse = await request.get("/api/spells?edition=EDITION_2024"); expect(catalogResponse.ok(), `list spells: ${catalogResponse.status()}`).toBeTruthy(); const catalog = (await catalogResponse.json()) as { id: string; name: string; level: number }[]; diff --git a/frontend/src/api/catalog.test.ts b/frontend/src/api/catalog.test.ts index 87b635bf..69bd5f73 100644 --- a/frontend/src/api/catalog.test.ts +++ b/frontend/src/api/catalog.test.ts @@ -78,13 +78,26 @@ describe("fetchSpells", () => { }) ); - await expect(fetchSpells()).resolves.toMatchObject([{ name: "Fireball", level: 3 }]); + await expect(fetchSpells("EDITION_2024")).resolves.toMatchObject([{ name: "Fireball", level: 3 }]); + }); + + // #1712: `?edition=` is now REQUIRED (the route 400s without it) — same pin + // shape as fetchFeats/fetchReference above. + it("always sends ?edition=, with class/maxLevel appended when given", async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => [] }); + vi.stubGlobal("fetch", fetchMock); + + await fetchSpells("EDITION_2014"); + await fetchSpells("EDITION_2024", { className: "wizard", maxLevel: 3 }); + + expect(fetchMock.mock.calls[0][0]).toMatch(/\/spells\?edition=EDITION_2014$/); + expect(fetchMock.mock.calls[1][0]).toMatch(/\/spells\?edition=EDITION_2024&class=wizard&maxLevel=3$/); }); it("throws on a non-ok response", async () => { vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 500 })); - await expect(fetchSpells()).rejects.toThrow(); + await expect(fetchSpells("EDITION_2024")).rejects.toThrow(); }); }); diff --git a/frontend/src/api/catalog.ts b/frontend/src/api/catalog.ts index 0f1bb1da..d510a23f 100644 --- a/frontend/src/api/catalog.ts +++ b/frontend/src/api/catalog.ts @@ -24,15 +24,18 @@ export interface SpellCatalogFilter { // Feeds the spellcasting section's "learn from catalog" picker. // Ordered by level then name server-side; no client-side re-sort needed. // -// The filter is optional, following fetchFeats' query-string pattern: the -// creation ceremony asks for one class's legal band and the server applies the -// eligibility rule (#1377), while the sheet's picker wants the whole catalog. -export async function fetchSpells(filter: SpellCatalogFilter = {}): Promise { - const params = new URLSearchParams(); +// `edition` is required and the route 400s without it (#1712, same shape as +// fetchFeats/fetchReference) — every caller passes the VIEWING character's +// edition (or the creation draft's chosen edition, before a character exists) +// so a 2014 request never sees a 2024-only row. `class`/`maxLevel` stay +// optional, following fetchFeats' asiLevel pattern: the creation ceremony +// asks for one class's legal band and the server applies the eligibility +// rule (#1377), while the sheet's picker wants the whole (one-edition) catalog. +export async function fetchSpells(edition: RulesEdition, filter: SpellCatalogFilter = {}): Promise { + const params = new URLSearchParams({ edition }); if (filter.className !== undefined) params.set("class", filter.className); if (filter.maxLevel !== undefined) params.set("maxLevel", String(filter.maxLevel)); - const query = params.toString(); - return request(query ? `/spells?${query}` : "/spells", undefined, "Failed to fetch spell catalog"); + return request(`/spells?${params.toString()}`, undefined, "Failed to fetch spell catalog"); } // Feeds the advancement section's feat picker — same role as fetchManeuvers. diff --git a/frontend/src/features/character-create/CreationCeremony.tsx b/frontend/src/features/character-create/CreationCeremony.tsx index 32a31659..95777682 100644 --- a/frontend/src/features/character-create/CreationCeremony.tsx +++ b/frontend/src/features/character-create/CreationCeremony.tsx @@ -91,6 +91,12 @@ function SkillsStepBody({ c }: StepBodyProps) { function SpellsStepBody({ c }: StepBodyProps) { const picks = c.selections.class?.level1SpellPicks; + // draft.rulesEdition is RulesEdition | null (unresolved until the entry + // gate, CreationCeremony's own early return above) — narrowed here so the + // spell-catalog fetches below can take a required RulesEdition (#1712). + // Unreachable in practice: this step never renders before the gate resolves it. + const { rulesEdition } = c.draft; + if (!rulesEdition) return null; return ( <> {picks && ( @@ -99,6 +105,7 @@ function SpellsStepBody({ c }: StepBodyProps) { counts={picks} cantripIds={c.draft.cantripIds} spellIds={c.draft.spellIds} + edition={rulesEdition} onChange={c.update} /> )} @@ -107,6 +114,7 @@ function SpellsStepBody({ c }: StepBodyProps) { is the ONLY content on the step for a non-caster High Elf. */} c.update({ speciesCantripId })} /> diff --git a/frontend/src/features/character-create/CreationSpellsStep.test.tsx b/frontend/src/features/character-create/CreationSpellsStep.test.tsx index 40ccd8a3..0b3581d8 100644 --- a/frontend/src/features/character-create/CreationSpellsStep.test.tsx +++ b/frontend/src/features/character-create/CreationSpellsStep.test.tsx @@ -45,6 +45,7 @@ function renderStep(over: Partial[0]> = {} counts={COUNTS} cantripIds={[]} spellIds={[]} + edition="EDITION_2024" onChange={onChange} {...over} />, @@ -63,7 +64,7 @@ describe("CreationSpellsStep", () => { it("asks the server for the class's legal band, passing the served maxSpellLevel", async () => { renderStep(); await screen.findByRole("button", { name: "Open Eldritch Blast" }); - expect(fetchMock).toHaveBeenCalledWith({ className: "warlock", maxLevel: 1 }); + expect(fetchMock).toHaveBeenCalledWith("EDITION_2024", { className: "warlock", maxLevel: 1 }); }); // #1510: a 2014 Cleric/Druid serves maxSpellLevel: 0 (cantrips-only — see @@ -73,7 +74,7 @@ describe("CreationSpellsStep", () => { it("passes maxSpellLevel: 0 through to the fetch for a cantrips-only class", async () => { renderStep({ className: "cleric", counts: { cantrips: 3, spells: 0, maxSpellLevel: 0 } }); await screen.findByRole("button", { name: "Open Eldritch Blast" }); - expect(fetchMock).toHaveBeenCalledWith({ className: "cleric", maxLevel: 0 }); + expect(fetchMock).toHaveBeenCalledWith("EDITION_2024", { className: "cleric", maxLevel: 0 }); }); // Each render keeps exactly one group alive, which is how the level-0 split can diff --git a/frontend/src/features/character-create/CreationSpellsStep.tsx b/frontend/src/features/character-create/CreationSpellsStep.tsx index 5aff968c..0de487e1 100644 --- a/frontend/src/features/character-create/CreationSpellsStep.tsx +++ b/frontend/src/features/character-create/CreationSpellsStep.tsx @@ -12,6 +12,7 @@ import { type CreationSpellCounts, } from "@/lib/creationSpells"; import type { CharacterDraft } from "@/hooks/useCharacterDraft"; +import type { RulesEdition } from "@character-sheet/shared-types"; // #1513: shown only for the Wizard (counts.spellbookSize present) — the // prepared number is deliberately unstated: it's ability-score-dependent @@ -63,15 +64,17 @@ export default function CreationSpellsStep({ counts, cantripIds, spellIds, + edition, onChange, }: { className: string; counts: CreationSpellCounts; cantripIds: string[]; spellIds: string[]; + edition: RulesEdition; onChange: (patch: Partial) => void; }) { - const { catalog, error, showSpinner } = useSpellCatalog({ className, maxLevel: counts.maxSpellLevel }); + const { catalog, error, showSpinner } = useSpellCatalog(edition, { className, maxLevel: counts.maxSpellLevel }); const options = splitCreationCatalog(catalog); const groups = buildSpellGroups(counts, options, cantripIds, spellIds, onChange); diff --git a/frontend/src/features/character-create/SpeciesCantripSection.test.tsx b/frontend/src/features/character-create/SpeciesCantripSection.test.tsx index 600e2663..28135aa4 100644 --- a/frontend/src/features/character-create/SpeciesCantripSection.test.tsx +++ b/frontend/src/features/character-create/SpeciesCantripSection.test.tsx @@ -43,7 +43,7 @@ function renderSection(choice: Partial = {}, onCha complete: false, ...choice, }; - render(); + render(); return { onChange }; } @@ -57,6 +57,7 @@ describe("SpeciesCantripSection (#1689)", () => { const { container } = render( , ); @@ -67,7 +68,7 @@ describe("SpeciesCantripSection (#1689)", () => { it("queries the spec's OWN class list, cantrips only (maxLevel: 0) — never the character's class", async () => { renderSection(); await screen.findByRole("button", { name: "Open Fire Bolt" }); - expect(fetchMock).toHaveBeenCalledWith({ className: "wizard", maxLevel: 0 }); + expect(fetchMock).toHaveBeenCalledWith("EDITION_2024", { className: "wizard", maxLevel: 0 }); }); it("names the spec's casting ability in the panel copy", async () => { diff --git a/frontend/src/features/character-create/SpeciesCantripSection.tsx b/frontend/src/features/character-create/SpeciesCantripSection.tsx index fa7cb696..8fc48a61 100644 --- a/frontend/src/features/character-create/SpeciesCantripSection.tsx +++ b/frontend/src/features/character-create/SpeciesCantripSection.tsx @@ -11,9 +11,11 @@ import { abilityLabel } from "@/lib/abilities"; import type { CreationSpeciesCantripChoice } from "@/lib/characterCreation"; import SpellPicker, { type SpellPickerGroup } from "@/features/spells/SpellPicker"; import { useSpellCatalog } from "@/features/spells/useSpellCatalog"; +import type { RulesEdition } from "@character-sheet/shared-types"; interface SpeciesCantripSectionProps { choice: CreationSpeciesCantripChoice; + edition: RulesEdition; onChange: (spellId: string) => void; } @@ -23,14 +25,16 @@ interface SpeciesCantripSectionProps { // `className: ""` request for a species with no cantrip choice. function SpeciesCantripPicker({ choice, + edition, onChange, }: { choice: CreationSpeciesCantripChoice; + edition: RulesEdition; onChange: (spellId: string) => void; }) { // Cantrips only (maxLevel: 0) — a species-granted cantrip choice never // reaches into leveled spells. - const { catalog, error, showSpinner } = useSpellCatalog({ className: choice.list, maxLevel: 0 }); + const { catalog, error, showSpinner } = useSpellCatalog(edition, { className: choice.list, maxLevel: 0 }); const selectedIds = choice.selectedId ? [choice.selectedId] : []; const groups: SpellPickerGroup[] = [ @@ -63,7 +67,7 @@ function SpeciesCantripPicker({ ); } -export default function SpeciesCantripSection({ choice, onChange }: SpeciesCantripSectionProps) { +export default function SpeciesCantripSection({ choice, edition, onChange }: SpeciesCantripSectionProps) { if (!choice.applicable) return null; - return ; + return ; } diff --git a/frontend/src/features/entities/CampaignItemFields.tsx b/frontend/src/features/entities/CampaignItemFields.tsx index 8141aef7..eb724d94 100644 --- a/frontend/src/features/entities/CampaignItemFields.tsx +++ b/frontend/src/features/entities/CampaignItemFields.tsx @@ -39,6 +39,7 @@ import type { ItemRarity, ItemRarityOption, } from "@/types/character"; +import type { RulesEdition } from "@character-sheet/shared-types"; const legendCls = "text-sm font-semibold text-parchment-800"; const fieldsetCls = @@ -300,7 +301,7 @@ function AttunementPrereqFields({ form, set }: { form: FormState; set: SetField ); } -export function MagicFieldset({ form, setters, rarities }: FieldsProps & { rarities: ItemRarityOption[] }) { +export function MagicFieldset({ form, setters, rarities, edition }: FieldsProps & { rarities: ItemRarityOption[]; edition: RulesEdition }) { const { set } = setters; const isMagic = form.rarity !== ""; const rarityHint = rarityValueHint(form.rarity || undefined, rarities, { @@ -351,6 +352,7 @@ export function MagicFieldset({ form, setters, rarities }: FieldsProps & { rarit capabilities={form.capabilities} onChange={(capabilities) => set("capabilities", capabilities)} spellcasterAttunable={form.requiresAttunement && form.attunementPrereqKind === "spellcaster"} + edition={edition} /> )} diff --git a/frontend/src/features/entities/CampaignItemForm.tsx b/frontend/src/features/entities/CampaignItemForm.tsx index f52f7b49..9bb5ebc9 100644 --- a/frontend/src/features/entities/CampaignItemForm.tsx +++ b/frontend/src/features/entities/CampaignItemForm.tsx @@ -11,6 +11,7 @@ import { import { buildFormSetters } from "@/features/entities/campaignItemFormSetters"; import { type FormState } from "@/lib/campaignItemForm"; import type { Item, ItemRarityOption } from "@/types/character"; +import type { RulesEdition } from "@character-sheet/shared-types"; interface CampaignItemFormProps { form: FormState; @@ -20,6 +21,8 @@ interface CampaignItemFormProps { busyId: string | null; /** Served rarity rows (#1437), passed in so the form holds no query observer. */ rarities: ItemRarityOption[]; + /** The campaign's edition (#1712) — threaded to MagicFieldset's spell picker. */ + edition: RulesEdition; onSubmit: () => void; onCancel: () => void; } @@ -31,6 +34,7 @@ export default function CampaignItemForm({ catalog, busyId, rarities, + edition, onSubmit, onCancel, }: CampaignItemFormProps) { @@ -42,7 +46,7 @@ export default function CampaignItemForm({ - + diff --git a/frontend/src/features/entities/CampaignItemsPanel.tsx b/frontend/src/features/entities/CampaignItemsPanel.tsx index 3a1b9862..6e0d2acd 100644 --- a/frontend/src/features/entities/CampaignItemsPanel.tsx +++ b/frontend/src/features/entities/CampaignItemsPanel.tsx @@ -76,6 +76,7 @@ export default function CampaignItemsPanel({ campaignId, characters, edition }: catalog={catalog} busyId={busyId} rarities={rarities} + edition={edition} onSubmit={handleSubmit} onCancel={cancelForm} /> diff --git a/frontend/src/features/entities/CapabilityEditor.test.tsx b/frontend/src/features/entities/CapabilityEditor.test.tsx index c977128c..01deedbf 100644 --- a/frontend/src/features/entities/CapabilityEditor.test.tsx +++ b/frontend/src/features/entities/CapabilityEditor.test.tsx @@ -50,6 +50,7 @@ function Harness({ onChange }: { onChange?: (caps: ItemCapability[]) => void }) onChange?.(next); }} spellcasterAttunable={false} + edition="EDITION_2024" /> ); } diff --git a/frontend/src/features/entities/CapabilityEditor.tsx b/frontend/src/features/entities/CapabilityEditor.tsx index 3ddf7b79..5fb70d49 100644 --- a/frontend/src/features/entities/CapabilityEditor.tsx +++ b/frontend/src/features/entities/CapabilityEditor.tsx @@ -5,25 +5,28 @@ import { Plus } from "@/components/ui/icons"; import CapabilityRow from "@/features/entities/CapabilityRow"; import { NEW_PASSIVE } from "@/lib/capabilityDraft"; import type { CatalogSpell, ItemCapability } from "@/types/character"; +import type { RulesEdition } from "@character-sheet/shared-types"; interface CapabilityEditorProps { capabilities: ItemCapability[]; onChange: (capabilities: ItemCapability[]) => void; /** True when the item is attunable by a spellcaster — gates wielder DC/attack (#528). */ spellcasterAttunable?: boolean; + /** The campaign's edition (#1712) — GET /api/spells now requires one. */ + edition: RulesEdition; } // DM authoring for an item's capabilities (#546). Each row is one capability of a // chosen kind (passiveBonus/castSpell/grant/charges); per-kind fields live in the // sibling *Fields subcomponents, draft normalization in capabilityDraft. -export default function CapabilityEditor({ capabilities, onChange, spellcasterAttunable = false }: CapabilityEditorProps) { +export default function CapabilityEditor({ capabilities, onChange, spellcasterAttunable = false, edition }: CapabilityEditorProps) { const [spells, setSpells] = useState([]); const needSpells = capabilities.some((c) => c.kind === "castSpell"); useEffect(() => { if (needSpells && spells.length === 0) { - fetchSpells().then(setSpells).catch(() => setSpells([])); + fetchSpells(edition).then(setSpells).catch(() => setSpells([])); } - }, [needSpells, spells.length]); + }, [needSpells, spells.length, edition]); function update(index: number, patch: Partial) { onChange(capabilities.map((c, i) => (i === index ? { ...c, ...patch } : c))); diff --git a/frontend/src/features/level-up/NewSpellsStep.tsx b/frontend/src/features/level-up/NewSpellsStep.tsx index e099e929..b9dd0e18 100644 --- a/frontend/src/features/level-up/NewSpellsStep.tsx +++ b/frontend/src/features/level-up/NewSpellsStep.tsx @@ -240,7 +240,7 @@ function LeveledSpellsSection({ export default function NewSpellsStep({ step }: { step: LevelUpStep }) { const { character } = useLevelUpStepContext(); const selection = useNewSpellsSelection(step); - const { catalog, error, showSpinner } = useSpellCatalog(); + const { catalog, error, showSpinner } = useSpellCatalog(character.rulesEdition); const [cantripSearch, setCantripSearch] = useState(""); const learnedSpellIds = character.spellcasting ? deriveSpellList(character).learnedSpellIds : NO_KNOWN; diff --git a/frontend/src/features/level-up/ReviewStep.tsx b/frontend/src/features/level-up/ReviewStep.tsx index 02ded5fa..29d2744e 100644 --- a/frontend/src/features/level-up/ReviewStep.tsx +++ b/frontend/src/features/level-up/ReviewStep.tsx @@ -47,21 +47,28 @@ function useLedgerResolvers(draft: LevelUpDraft, edition: RulesEdition): { resol ); const maneuvers = useCatalogNames(maneuverFetcher); // Cantrips share the spell catalog, so either list gates the same fetch (#1157). - const spells = useCatalogNames(draft.spellsLearned?.length || draft.cantripsLearned?.length ? fetchSpells : undefined); - // Any taken feat fetches the catalog — a custom feat resolves by its own name, - // so this needs no second (featId) guard. A Fighting Style feat (#1137) resolves - // through the same catalog. // // The edition is threaded here for the wire contract, not as an admission gate: // this site resolves an id→name for an ALREADY-COMMITTED pick, so no - // cross-edition row can be introduced through it (#1411). + // cross-edition row can be introduced through it (#1411) — same reasoning + // as the feat fetcher below. `?edition=` became REQUIRED on GET /api/spells + // in #1712, so fetchSpells joined maneuvers/feats on the memoised-fetcher + // side of the line below (it could no longer get away with a bare module ref). + const needsSpells = !!(draft.spellsLearned?.length || draft.cantripsLearned?.length); + const spellFetcher = useMemo( + () => (needsSpells ? () => fetchSpells(edition) : undefined), + [needsSpells, edition], + ); + const spells = useCatalogNames(spellFetcher); + // Any taken feat fetches the catalog — a custom feat resolves by its own name, + // so this needs no second (featId) guard. A Fighting Style feat (#1137) resolves + // through the same catalog. // // Keyed on the BOOLEAN, never on draft.fightingStyleFeat's object identity, and // never an inline arrow: useCatalogNames's effect depends on [fetcher], so a // fresh identity every render means fetch → setMap → re-render → fetch, forever. - // Only fetchSpells still gets away with a bare module ref, because it alone - // takes no argument — every edition-scoped fetcher must be memoised, and - // #1412 moved maneuvers across that line. + // Every edition-scoped fetcher must be memoised — #1412 moved maneuvers across + // that line, #1712 moved spells. const needsFeats = draft.advancement?.type === "takeFeat" || !!draft.fightingStyleFeat; const featFetcher = useMemo( () => (needsFeats ? () => fetchFeats(edition) : undefined), diff --git a/frontend/src/features/spells/AddSpellPanel.test.tsx b/frontend/src/features/spells/AddSpellPanel.test.tsx index 60f8b5ae..a3a168b7 100644 --- a/frontend/src/features/spells/AddSpellPanel.test.tsx +++ b/frontend/src/features/spells/AddSpellPanel.test.tsx @@ -23,6 +23,7 @@ describe("AddSpellPanel accessibility", () => { onClose={noop} busy={false} learnedSpellIds={new Set()} + edition="EDITION_2024" /> ); diff --git a/frontend/src/features/spells/AddSpellPanel.tsx b/frontend/src/features/spells/AddSpellPanel.tsx index 0a7ab457..3086d49d 100644 --- a/frontend/src/features/spells/AddSpellPanel.tsx +++ b/frontend/src/features/spells/AddSpellPanel.tsx @@ -6,6 +6,7 @@ import { useState } from "react"; import CustomSpellForm from "@/features/spells/CustomSpellForm"; import SpellCatalogTab from "@/features/spells/SpellCatalogTab"; import type { CatalogSpell, LearnSpellOperation } from "@/types/character"; +import type { RulesEdition } from "@character-sheet/shared-types"; interface AddSpellPanelProps { /** Called with the op to send; parent batches and fires the API. */ @@ -14,9 +15,10 @@ interface AddSpellPanelProps { busy: boolean; /** Set of spellId values already in the spellbook (to disable duplicates). */ learnedSpellIds: Set; + edition: RulesEdition; } -export default function AddSpellPanel({ onLearn, onClose, busy, learnedSpellIds }: AddSpellPanelProps) { +export default function AddSpellPanel({ onLearn, onClose, busy, learnedSpellIds, edition }: AddSpellPanelProps) { const [tab, setTab] = useState<"catalog" | "custom">("catalog"); function handleCatalogLearn(spell: CatalogSpell) { @@ -58,7 +60,7 @@ export default function AddSpellPanel({ onLearn, onClose, busy, learnedSpellIds {tab === "catalog" ? ( - + ) : ( )} diff --git a/frontend/src/features/spells/SpellCatalogTab.tsx b/frontend/src/features/spells/SpellCatalogTab.tsx index 1c0cd770..6c11a503 100644 --- a/frontend/src/features/spells/SpellCatalogTab.tsx +++ b/frontend/src/features/spells/SpellCatalogTab.tsx @@ -6,15 +6,17 @@ import SpellCatalogRow from "@/features/spells/SpellCatalogRow"; import { useSpellCatalog } from "@/features/spells/useSpellCatalog"; import { INPUT_CLS, LEVEL_OPTIONS, filterCatalog } from "@/lib/addSpell"; import type { CatalogSpell } from "@/types/character"; +import type { RulesEdition } from "@character-sheet/shared-types"; interface SpellCatalogTabProps { busy: boolean; learnedSpellIds: Set; + edition: RulesEdition; onLearn: (spell: CatalogSpell) => void; } -export default function SpellCatalogTab({ busy, learnedSpellIds, onLearn }: SpellCatalogTabProps) { - const { catalog, error, showSpinner } = useSpellCatalog(); +export default function SpellCatalogTab({ busy, learnedSpellIds, edition, onLearn }: SpellCatalogTabProps) { + const { catalog, error, showSpinner } = useSpellCatalog(edition); const [search, setSearch] = useState(""); const [levelFilter, setLevelFilter] = useState(""); diff --git a/frontend/src/features/spells/SpellsSection.tsx b/frontend/src/features/spells/SpellsSection.tsx index c7a3b0b5..4287f3a9 100644 --- a/frontend/src/features/spells/SpellsSection.tsx +++ b/frontend/src/features/spells/SpellsSection.tsx @@ -80,6 +80,7 @@ export default function SpellsSection({ onClose={() => setAddPanelOpen(false)} busy={busy} learnedSpellIds={derived.learnedSpellIds} + edition={character.rulesEdition} /> )} diff --git a/frontend/src/features/spells/useSpellCatalog.ts b/frontend/src/features/spells/useSpellCatalog.ts index 829c5225..a6fe9805 100644 --- a/frontend/src/features/spells/useSpellCatalog.ts +++ b/frontend/src/features/spells/useSpellCatalog.ts @@ -6,23 +6,28 @@ import { useEffect, useState } from "react"; import { fetchSpells, type SpellCatalogFilter } from "@/api/client"; import { useDelayedFlag } from "@/hooks/useDelayedFlag"; import type { CatalogSpell } from "@/types/character"; +import type { RulesEdition } from "@character-sheet/shared-types"; -export function useSpellCatalog(filter?: SpellCatalogFilter) { +// `edition` is required (#1712): every caller already has the viewing +// character's edition (or the creation draft's chosen one) in hand, so +// threading it through here is what keeps the picker from ever offering a +// cross-edition row. +export function useSpellCatalog(edition: RulesEdition, filter?: SpellCatalogFilter) { const [catalog, setCatalog] = useState(null); const [error, setError] = useState(null); const showSpinner = useDelayedFlag(catalog === null && !error); - // Destructured so the effect depends on the two primitives, not on a fresh + // Destructured so the effect depends on the primitives, not on a fresh // object identity every render (which would refetch in a loop). const className = filter?.className; const maxLevel = filter?.maxLevel; useEffect(() => { let mounted = true; - fetchSpells({ className, maxLevel }) + fetchSpells(edition, { className, maxLevel }) .then((spells) => { if (mounted) setCatalog(spells); }) .catch(() => { if (mounted) setError("Couldn't load spell catalog."); }); return () => { mounted = false; }; - }, [className, maxLevel]); + }, [edition, className, maxLevel]); return { catalog, error, showSpinner }; }