From da1aa271eaa7506e02c83ebe8a9a09c195c45c91 Mon Sep 17 00:00:00 2001 From: Developer Date: Tue, 6 Oct 2026 09:00:21 -0500 Subject: [PATCH] feat(system-builder): list problem counts and install names ahead (4a1b1) listSystems gives each system's `problemCount` (store.problemsOf, shared with getSystem: a draft that can't be read counts as one), so SYSTEMS.EXE can badge problems without fetching every system. previewInstall gives `installsAs`, the name each install mode would use (store.installNames: null where the mode doesn't apply). installSystem now takes its name from the same function, so the preview and the install can't disagree. matchesFor also returns the systems in use. citysys.summarize counts currencies, for the preview's INSIDE badges. --- CHANGELOG.md | 6 ++ .../__tests__/system_builder_citysys.test.js | 26 +++++- .../__tests__/system_builder_store.test.js | 12 ++- backend/systemBuilder/citysys.js | 1 + backend/systemBuilder/store.js | 88 +++++++++++++------ 5 files changed, 102 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ffdface..cfb7438b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Under the hood +- **The system list counts each system's problems, and an install says its name first.** The list + of a GM's systems now says how many problems each draft has, so SYSTEMS.EXE can mark them without + opening each one. Looking at a file before installing it now says the name it will go in under + ("Vault Knights copy" when that name is taken), for each way of installing it, and how many + currencies it has. + - **A system can be duplicated, and an install is never stopped by a name.** A duplicate is a new, unpublished system made from the original's draft, unpublished changes included, and named "Hearth copy", "Hearth copy 02" and so on. Installing a file whose system has a name already diff --git a/backend/__tests__/system_builder_citysys.test.js b/backend/__tests__/system_builder_citysys.test.js index acc4303e..dd4a289b 100644 --- a/backend/__tests__/system_builder_citysys.test.js +++ b/backend/__tests__/system_builder_citysys.test.js @@ -130,9 +130,11 @@ describe('previewing an install', () => { expect(res.body).toMatchObject({ name: 'Vault Knights', manifest: { author: 'Cody', version: 3, origin: 'org_vault' }, - inside: { words: 1, derived: 1, healthModel: 'wounds' }, + inside: { words: 1, derived: 1, currencies: 0, healthModel: 'wounds' }, problems: [], installed: [], restores: null, + installsAs: { new: 'Vault Knights', update: null, keep_both: 'Vault Knights' }, }); + expect((await preview(fileOf({ ...VAULT, currencies: [{ id: 'gold', name: 'Gold' }, { id: 'favor', name: 'Favor' }] }))).body.inside.currencies).toBe(2); expect(await rows()).toEqual(before); }); @@ -279,6 +281,28 @@ describe('installing under a name already here', () => { expect(await nameOf(res.body.id)).toMatchObject({ name: 'Vault Knights copy', draft: 'Vault Knights copy', published: null }); }); + it('says beforehand the name each way of installing would give, and installing keeps to it', async () => { + const mine = await create(); + const asNew = (await preview(fileOf(VAULT))).body.installsAs; + expect(asNew).toEqual({ new: 'Vault Knights copy', update: null, keep_both: 'Vault Knights copy' }); + const id = (await install(fileOf(VAULT), 'new')).body.id; + // Installed: an update keeps its name while the other is here; keeping both counts on. + expect((await preview(fileOf(VAULT))).body.installsAs).toEqual({ new: null, update: 'Vault Knights copy', keep_both: 'Vault Knights copy 02' }); + expect((await install(fileOf(VAULT), 'keep_both')).body.name).toBe('Vault Knights copy 02'); + // With the other gone, an update takes the file's own name back. + await request(app).delete(`/api/systems/${mine}`).set(gm); + expect((await preview(fileOf(VAULT))).body.installsAs.update).toBe('Vault Knights'); + expect((await install(fileOf({ ...VAULT, description: 'v2' }, 'org_vault', 4), 'update')).body).toMatchObject({ id, name: 'Vault Knights' }); + }); + + it('names one brought back from deletion as it will be', async () => { + const id = (await install(fileOf(VAULT), 'new')).body.id; + await request(app).delete(`/api/systems/${id}`).set(gm); + await create(); + expect((await preview(fileOf(VAULT))).body).toMatchObject({ restores: { id }, installsAs: { new: 'Vault Knights copy' } }); + expect((await install(fileOf(VAULT), 'new')).body).toMatchObject({ id, restored: true, name: 'Vault Knights copy' }); + }); + it('ignores a deleted system\'s name', async () => { const mine = await create(); await request(app).delete(`/api/systems/${mine}`).set(gm); diff --git a/backend/__tests__/system_builder_store.test.js b/backend/__tests__/system_builder_store.test.js index 2fa203fa..8f14ff6b 100644 --- a/backend/__tests__/system_builder_store.test.js +++ b/backend/__tests__/system_builder_store.test.js @@ -138,7 +138,14 @@ describe('the systems routes', () => { expect(res.body.id).toMatch(/^sys_[0-9a-f]{16}$/); expect(res.body.problems).toEqual([]); const [only] = await list(); - expect(only).toMatchObject({ id: res.body.id, name: 'Vault Knights', version: 0, published: false, unpublishedChanges: true, installed: false }); + expect(only).toMatchObject({ id: res.body.id, name: 'Vault Knights', version: 0, published: false, unpublishedChanges: true, installed: false, problemCount: 0 }); + }); + + it('counts a draft that cannot be read at all as one problem, as publishing would', async () => { + const { id } = (await create({ name: 'Broken' })).body; + await run(db, 'UPDATE custom_systems SET draft = ? WHERE id = ?', ['not json', id]); + expect((await list())[0].problemCount).toBe(1); + expect((await request(app).get(`/api/systems/${id}`).set(bearer(GM))).body.problems).toEqual([{ where: 'definition', message: expect.any(String) }]); }); it('never collides with a built-in system id', () => { @@ -166,6 +173,8 @@ describe('the systems routes', () => { const saved = await request(app).put(`/api/systems/${id}/draft`).set(bearer(GM)).send({ definition: broken }); expect(saved.status).toBe(200); expect(saved.body.problems).toEqual([{ where: 'derived hp', message: 'Depends on itself: hp → hp' }]); + // The list counts them, so SYSTEMS.EXE can badge it without fetching the system. + expect((await list())[0].problemCount).toBe(1); const refused = await request(app).post(`/api/systems/${id}/publish`).set(bearer(GM)); expect(refused.status).toBe(409); @@ -174,6 +183,7 @@ describe('the systems routes', () => { const fixed = { format: 1, name: 'Draft', derived: [{ id: 'hp', formula: '@con + 10' }] }; await request(app).put(`/api/systems/${id}/draft`).set(bearer(GM)).send({ definition: fixed }); + expect((await list())[0].problemCount).toBe(0); const published = await request(app).post(`/api/systems/${id}/publish`).set(bearer(GM)); expect(published.status).toBe(200); expect(published.body).toEqual({ version: 1 }); diff --git a/backend/systemBuilder/citysys.js b/backend/systemBuilder/citysys.js index 3e18ebfe..f6877bf6 100644 --- a/backend/systemBuilder/citysys.js +++ b/backend/systemBuilder/citysys.js @@ -100,6 +100,7 @@ const summarize = (definition) => { return { words: isPlainObject(definition.words) ? Object.keys(definition.words).length : 0, partsOff: isPlainObject(definition.parts) ? Object.values(definition.parts).filter((p) => isPlainObject(p) && p.on === false).length : 0, + currencies: count(definition.currencies), derived: count(definition.derived), lookups: isPlainObject(definition.lookups) ? Object.keys(definition.lookups).length : 0, sheetFields: fields, diff --git a/backend/systemBuilder/store.js b/backend/systemBuilder/store.js index 0cef346a..661813b5 100644 --- a/backend/systemBuilder/store.js +++ b/backend/systemBuilder/store.js @@ -33,7 +33,16 @@ const parse = (text) => { /** A stored error: `status` is what a route answers with. */ const fail = (status, message, extra) => Object.assign(new Error(message), { status, ...(extra || {}) }); -/** Every system, newest change first, without their definitions. */ +/** A draft's problems, as publishing would list them. */ +const problemsOf = (draft) => { + const checked = checkDefinition(draft); + return checked.fatal ? [{ where: 'definition', message: checked.fatal }] : checked.problems; +}; + +/** + * Every system, newest change first, without their definitions. Each says how many problems its + * draft has, so SYSTEMS.EXE can badge them without fetching every system. + */ const listSystems = (db, cb) => { db.all( `SELECT id, name, version, draft, published, source_hash, updated_at, published_at @@ -52,6 +61,7 @@ const listSystems = (db, cb) => { unpublishedChanges: r.published == null || r.draft !== r.published, // Installed from a file, rather than made here: only an install records the file's hash. installed: r.source_hash != null, + problemCount: problemsOf(parse(r.draft)).length, }))); }, ); @@ -64,7 +74,6 @@ const getSystem = (db, id, cb) => { if (err) return cb(err); if (!row) return cb(fail(404, 'No such system')); const draft = parse(row.draft); - const checked = checkDefinition(draft); cb(null, { id: row.id, name: row.name, @@ -73,7 +82,7 @@ const getSystem = (db, id, cb) => { publishedAt: row.published_at, draft, published: parse(row.published), - problems: checked.fatal ? [{ where: 'definition', message: checked.fatal }] : checked.problems, + problems: problemsOf(draft), }); }); }; @@ -262,7 +271,10 @@ const exportSystem = (db, id, cb) => { const editedSinceInstall = (row) => citysys.hashOf(row.draft) !== row.source_hash || (row.published != null && citysys.hashOf(row.published) !== row.source_hash); -/** The systems here with a file's origin: the ones in use, and the most recently deleted. */ +/** + * The systems here with a file's origin: the ones in use, and the most recently deleted; and + * every system in use, whose names a new one must not take. + */ const matchesFor = (db, origin, cb) => { db.all( `SELECT id, name, version, draft, published, source_hash, deleted_at FROM custom_systems @@ -270,17 +282,38 @@ const matchesFor = (db, origin, cb) => { [origin], (err, rows) => { if (err) return cb(err); - cb(null, { - installed: rows.filter((r) => r.deleted_at == null), - deleted: rows.find((r) => r.deleted_at != null) || null, + db.all('SELECT id, name FROM custom_systems WHERE deleted_at IS NULL', [], (err2, inUse) => { + if (err2) return cb(err2); + cb(null, { + installed: rows.filter((r) => r.deleted_at == null), + deleted: rows.find((r) => r.deleted_at != null) || null, + inUse, + }); }); }, ); }; +/** + * The name a file's system goes in under, for each way of installing it: its own, or + * " copy"... when another system has it (decided with the user, 2026-10-06). An update + * replaces the copy here, so that copy's name is not in its way. Null where the way does not + * apply: `new` once it is installed, `update` until it is. + */ +const installNames = (name, found) => { + const here = found.installed[0]; + const taken = (rows) => rows.map((r) => r.name); + return { + new: here ? null : uniqueName(name, taken(found.inUse)), + update: here ? uniqueName(name, taken(found.inUse.filter((r) => r.id !== here.id))) : null, + keep_both: uniqueName(name, taken(found.inUse)), + }; +}; + /** * What installing a file would do, changing nothing: its cover, what is inside, its problems, - * the copies already here, and whether it would bring a deleted system back. + * the copies already here, whether it would bring a deleted system back, and the name it would + * go in under for each way of installing it (installNames). */ const previewInstall = (db, text, cb) => { const read = citysys.readFile(text); @@ -301,6 +334,7 @@ const previewInstall = (db, text, cb) => { name: found.deleted.name, replacesChanges: found.deleted.draft !== JSON.stringify(definition) && editedSinceInstall(found.deleted), } : null, + installsAs: installNames(definition.name, found), }); }); }; @@ -322,9 +356,8 @@ const installSystem = (db, text, mode, cb) => { const { manifest, definition } = read.file; const publishable = read.problems.length === 0; - /** The file's definition as stored, under a name none of `others` has. */ - const named = (others) => { - const name = uniqueName(definition.name, others.map((r) => r.name)); + /** The file's definition as stored, under the name this way of installing gives it. */ + const named = (name) => { const stored = JSON.stringify({ ...definition, name }); return { name, stored, hash: citysys.hashOf(stored) }; }; @@ -347,24 +380,21 @@ const installSystem = (db, text, mode, cb) => { matchesFor(db, manifest.origin, (err, found) => { if (err) return cb(err); - db.all('SELECT id, name FROM custom_systems WHERE deleted_at IS NULL', [], (err2, systems) => { - if (err2) return cb(err2); - const here = found.installed[0]; - if (mode === 'keep_both') { - const id = newId(); - return insert(id, id, named(systems)); - } - if (mode === 'update') { - if (!here) return cb(fail(404, 'This system is not installed here; install it as new')); - if (editedSinceInstall(here)) return cb(fail(409, 'This system has been changed here since it was installed. Keep both instead.')); - if (!publishable) return cb(fail(409, 'The file has problems; install it as a second copy to fix them', { problems: read.problems })); - // Its own name is not in the way of itself. - return replace(here, named(systems.filter((s) => s.id !== here.id))); - } - if (here) return cb(fail(409, 'Already installed. Update it or keep both.', { installed: found.installed.map((r) => ({ id: r.id, name: r.name })) })); - if (found.deleted) return replace(found.deleted, named(systems), { restored: true }); - return insert(newId(), manifest.origin, named(systems)); - }); + const here = found.installed[0]; + const names = installNames(definition.name, found); + if (mode === 'keep_both') { + const id = newId(); + return insert(id, id, named(names.keep_both)); + } + if (mode === 'update') { + if (!here) return cb(fail(404, 'This system is not installed here; install it as new')); + if (editedSinceInstall(here)) return cb(fail(409, 'This system has been changed here since it was installed. Keep both instead.')); + if (!publishable) return cb(fail(409, 'The file has problems; install it as a second copy to fix them', { problems: read.problems })); + return replace(here, named(names.update)); + } + if (here) return cb(fail(409, 'Already installed. Update it or keep both.', { installed: found.installed.map((r) => ({ id: r.id, name: r.name })) })); + if (found.deleted) return replace(found.deleted, named(names.new), { restored: true }); + return insert(newId(), manifest.origin, named(names.new)); }); };