diff --git a/boneset-api/data/bones/axis.json b/boneset-api/data/bones/axis.json index 650641bd..3a3ab7a1 100644 --- a/boneset-api/data/bones/axis.json +++ b/boneset-api/data/bones/axis.json @@ -3,7 +3,6 @@ "id": "axis", "subBones": [ "axis_body", - "axis_body_(dens)", "axis_transverse_process", "axis_transverse_foramen", "axis_pedicle", diff --git a/boneset-api/data/subbones/axis_body_(dens).json b/boneset-api/data/subbones/axis_body_(dens).json deleted file mode 100644 index fd6b6925..00000000 --- a/boneset-api/data/subbones/axis_body_(dens).json +++ /dev/null @@ -1 +0,0 @@ -{"id": "axis_body_(dens)", "name": "Axis Body (Dens)"} \ No newline at end of file diff --git a/boneset-api/images.js b/boneset-api/images.js index 4aab5b2f..b6a21818 100644 --- a/boneset-api/images.js +++ b/boneset-api/images.js @@ -13,7 +13,7 @@ const path = require("path"); const IMAGE_EXTENSIONS = new Set([".jpg", ".jpeg", ".png"]); const DESCRIPTION_SUFFIX = /_description\.json$/i; -const BONE_ID_PATTERN = /^[a-z0-9_]+$/i; +const BONE_ID_PATTERN = /^[a-z0-9_-]+$/i; // hyphens allowed for 2nd-5th_metatarsals (Issue #469) const HEADER_BYTES = 256 * 1024; function imageUrl(filename) { diff --git a/boneset-api/legacyImport.js b/boneset-api/legacyImport.js index d9aec8d7..20e88b90 100644 --- a/boneset-api/legacyImport.js +++ b/boneset-api/legacyImport.js @@ -10,7 +10,7 @@ const fs = require("fs").promises; const path = require("path"); const { getImageDimensions, imageUrl } = require("./images"); -const BONE_ID_PATTERN = /^[a-z0-9_]+$/i; +const BONE_ID_PATTERN = /^[a-z0-9_-]+$/i; // hyphens allowed for 2nd-5th_metatarsals (Issue #469) const MAX_IMAGE_SIZE = 480; // longest side of an imported image, in scene units const IMAGE_GAP = 40; // space between imported images, and between import rows diff --git a/boneset-api/migrationValidation.test.js b/boneset-api/migrationValidation.test.js index 682f4ea5..1a25a744 100644 --- a/boneset-api/migrationValidation.test.js +++ b/boneset-api/migrationValidation.test.js @@ -9,6 +9,7 @@ const { buildLegacyAnnotations } = require("./legacyAnnotations"); const { listItems, migrate } = require("./scripts/migration-report"); const DATA_DIR = path.join(__dirname, "data"); +const LONG_TEST = 30000; // going through every item can take longer than Jest's default 5 seconds const items = listItems(); function readJson(file) { @@ -37,7 +38,7 @@ describe("Migrated scene validation - Issue 468", () => { const row = await migrate(item); if (row.status !== "imported") expect(row.note.length).toBeGreaterThan(0); } - }); + }, LONG_TEST); describe("every imported item", () => { let imported; @@ -47,7 +48,7 @@ describe("Migrated scene validation - Issue 468", () => { const result = await importItem(item.id).catch(() => null); if (result) imported.push({ id: item.id, ...result }); } - }); + }, LONG_TEST); it("keeps the viewer's image order", () => { for (const { id, images } of imported) { @@ -120,4 +121,4 @@ describe("Migrated scene validation - Issue 468", () => { } }); }); -}); \ No newline at end of file +}); diff --git a/boneset-api/sceneStatus.test.js b/boneset-api/sceneStatus.test.js new file mode 100644 index 00000000..d5416057 --- /dev/null +++ b/boneset-api/sceneStatus.test.js @@ -0,0 +1,186 @@ +// Issues #458 (Define Draft Scenes) and #459 (Publish and Unpublish Scenes), +// part of #457 (Control Scene and Annotation Visibility). +const fs = require("fs"); +const os = require("os"); +const path = require("path"); +const express = require("express"); +const request = require("supertest"); +const { createScenesRouter, createFileSceneStore } = require("./scenes"); + +function buildApp(store) { + const app = express(); + app.use(express.json({ limit: "6mb" })); + app.use("/api/scenes", createScenesRouter(store)); + return app; +} + +const content = () => ({ + images: [{ id: "img-1", src: "/api/images/ilium_image1.jpg", x: 10, y: 20, width: 235, height: 371, rotation: 15 }], + annotations: [{ id: "ann-1", type: "text", text: "Iliac crest", x: 40, y: 50, fontSize: 16, color: "#1d3557" }], +}); + +describe("Scene drafts and publishing - Issues 458, 459", () => { + let dir; + let store; + let app; + + const create = (body = {}) => request(app).post("/api/scenes").send(body); + const setStatus = (id, status) => request(app).patch(`/api/scenes/${id}/status`).send({ status }); + const list = (query = "") => request(app).get(`/api/scenes${query}`); + const names = (response) => response.body.scenes.map((scene) => scene.name); + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "bonebox-scene-status-")); + store = createFileSceneStore(dir); + app = buildApp(store); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + describe("drafts (#458)", () => { + it("starts every new scene as a draft", async () => { + const created = await create({ name: "Pelvis Study" }); + expect(created.statusCode).toBe(201); + expect(created.body.status).toBe("draft"); + + const listed = await list(); + expect(listed.body.scenes[0]).toMatchObject({ name: "Pelvis Study", status: "draft", publishedAt: null }); + }); + + it("ignores a status sent when creating, so nothing is published by accident", async () => { + const created = await create({ name: "Sneaky", status: "published" }); + expect(created.body.status).toBe("draft"); + }); + + it("treats scenes saved before statuses existed as drafts", async () => { + await store.save({ + id: "11111111-1111-4111-8111-111111111111", name: "Old Scene", images: [], annotations: [], + createdAt: "2026-09-01T00:00:00.000Z", updatedAt: "2026-09-01T00:00:00.000Z", + }); + + const listed = await list(); + expect(listed.body.scenes[0]).toMatchObject({ name: "Old Scene", status: "draft" }); + expect(names(await list("?status=published"))).toEqual([]); + }); + + it("leaves drafts out of the published list the general viewer uses", async () => { + const draft = await create({ name: "Draft Scene" }); + const done = await create({ name: "Finished Scene" }); + await setStatus(done.body.id, "published"); + + expect(names(await list("?status=published"))).toEqual(["Finished Scene"]); + expect(names(await list("?status=draft"))).toEqual(["Draft Scene"]); + expect(names(await list()).sort()).toEqual(["Draft Scene", "Finished Scene"]); // the editor sees both + expect(draft.statusCode).toBe(201); + }); + + it("lets editors still open and save a draft", async () => { + const created = await create({ name: "Work in Progress" }); + + const opened = await request(app).get(`/api/scenes/${created.body.id}`); + expect(opened.statusCode).toBe(200); + + const saved = await request(app).put(`/api/scenes/${created.body.id}`) + .send({ ...content(), baseUpdatedAt: created.body.updatedAt }); + expect(saved.statusCode).toBe(200); + expect(saved.body.status).toBe("draft"); + }); + + it("makes a duplicate of a published scene a draft", async () => { + const original = await create({ name: "Published Original" }); + await setStatus(original.body.id, "published"); + + const copy = await request(app).post(`/api/scenes/${original.body.id}/duplicate`).send({}); + expect(copy.statusCode).toBe(201); + expect(copy.body.status).toBe("draft"); + expect(copy.body.publishedAt).toBeUndefined(); + expect(names(await list("?status=published"))).toEqual(["Published Original"]); + }); + + it("rejects an unknown status filter", async () => { + const response = await list("?status=secret"); + expect(response.statusCode).toBe(400); + }); + }); + + describe("publish and unpublish (#459)", () => { + it("publishes a scene so it appears in the published list", async () => { + const created = await create({ name: "Ilium Study" }); + + const response = await setStatus(created.body.id, "published"); + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ id: created.body.id, name: "Ilium Study", status: "published" }); + expect(Date.parse(response.body.publishedAt)).not.toBeNaN(); + + expect(names(await list("?status=published"))).toEqual(["Ilium Study"]); + const opened = await request(app).get(`/api/scenes/${created.body.id}`); + expect(opened.body.status).toBe("published"); + }); + + it("unpublishes a scene so it's no longer listed as published", async () => { + const created = await create({ name: "Ilium Study" }); + await setStatus(created.body.id, "published"); + + const response = await setStatus(created.body.id, "draft"); + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ status: "draft", publishedAt: null }); + + expect(names(await list("?status=published"))).toEqual([]); + const opened = await request(app).get(`/api/scenes/${created.body.id}`); + expect(opened.body.status).toBe("draft"); + expect(opened.body.publishedAt).toBeUndefined(); + }); + + it("doesn't change the scene's content or version when publishing", async () => { + const created = await create({ name: "Content Check" }); + const saved = await request(app).put(`/api/scenes/${created.body.id}`) + .send({ ...content(), baseUpdatedAt: created.body.updatedAt }); + + await setStatus(created.body.id, "published"); + const opened = await request(app).get(`/api/scenes/${created.body.id}`); + + expect(opened.body.status).toBe("published"); + for (const field of ["name", "images", "annotations", "createdAt", "updatedAt"]) { + expect(opened.body[field]).toEqual(saved.body[field]); + } + }); + + it("still lets an open copy be saved after publishing, and it stays published", async () => { + const created = await create({ name: "Keep Editing" }); + await setStatus(created.body.id, "published"); + + // The editor loaded the scene before publishing; its version is still valid. + const saved = await request(app).put(`/api/scenes/${created.body.id}`) + .send({ ...content(), baseUpdatedAt: created.body.updatedAt }); + expect(saved.statusCode).toBe(200); + expect(saved.body.status).toBe("published"); + expect(names(await list("?status=published"))).toEqual(["Keep Editing"]); + }); + + it("can publish and unpublish the same scene repeatedly", async () => { + const created = await create({ name: "Back and Forth" }); + for (const status of ["published", "draft", "published"]) { + const response = await setStatus(created.body.id, status); + expect(response.body.status).toBe(status); + } + expect(names(await list("?status=published"))).toEqual(["Back and Forth"]); + }); + + it("rejects a status that isn't draft or published", async () => { + const created = await create({ name: "Bad Status" }); + for (const body of [{ status: "archived" }, { status: "" }, {}]) { + const response = await request(app).patch(`/api/scenes/${created.body.id}/status`).send(body); + expect(response.statusCode).toBe(400); + } + const opened = await request(app).get(`/api/scenes/${created.body.id}`); + expect(opened.body.status).toBe("draft"); + }); + + it("returns 404 for a scene that doesn't exist", async () => { + const response = await setStatus("00000000-0000-4000-8000-000000000000", "published"); + expect(response.statusCode).toBe(404); + }); + }); +}); \ No newline at end of file diff --git a/boneset-api/scenes.js b/boneset-api/scenes.js index 12e96175..8b209f69 100644 --- a/boneset-api/scenes.js +++ b/boneset-api/scenes.js @@ -167,6 +167,16 @@ function applyImagePatch(existingImage, fields) { return { image: merged }; } +// The bone viewer items a scene's images were imported from (Issue #469), so the +// viewer can link an item to the scenes made from it. +function legacySources(scene) { + const boneIds = (scene.images || []) + .filter((image) => image && image.source && image.source.type === "legacy-image") + .map((image) => image.source.boneId) + .filter((boneId) => typeof boneId === "string"); + return [...new Set(boneIds)]; +} + function isPlainObject(value) { return value !== null && typeof value === "object" && !Array.isArray(value); } @@ -237,12 +247,29 @@ function nextUpdatedAt(previous) { return new Date(Number.isFinite(previousMs) && previousMs >= now ? previousMs + 1 : now).toISOString(); } +// Issues #458/#459: a scene is a draft (unfinished, only shown in the Scene +// Editor) until someone publishes it for the general bone viewer. +const SCENE_STATUSES = ["draft", "published"]; + +/** + * A scene's publication status. Scenes saved before statuses existed count as + * drafts, so nothing unfinished is ever shown as final content by accident. + * @param {object} scene + * @returns {"draft"|"published"} + */ +function sceneStatus(scene) { + return scene && scene.status === "published" ? "published" : "draft"; +} + function toSummary(scene) { return { id: scene.id, name: scene.name, + status: sceneStatus(scene), + publishedAt: sceneStatus(scene) === "published" ? scene.publishedAt || null : null, imageCount: (scene.images || []).length, annotationCount: (scene.annotations || []).length, + sources: legacySources(scene), createdAt: scene.createdAt, updatedAt: scene.updatedAt, }; @@ -519,9 +546,14 @@ function createScenesRouter(store = resolveSceneStore(), options = {}) { /** * Lists all scenes (summaries only) for the scene picker. Issues #422, #423, #424. */ - router.get("/", async (_req, res) => { + router.get("/", async (req, res) => { + const { status } = req.query; + if (status !== undefined && !SCENE_STATUSES.includes(status)) { + return res.status(400).json({ error: "status must be \"draft\" or \"published\"" }); + } try { - const scenes = await store.list(); + let scenes = await store.list(); + if (status) scenes = scenes.filter((scene) => sceneStatus(scene) === status); scenes.sort((a, b) => String(b.updatedAt).localeCompare(String(a.updatedAt))); res.json({ scenes: scenes.map(toSummary) }); } catch (error) { @@ -565,6 +597,8 @@ function createScenesRouter(store = resolveSceneStore(), options = {}) { images: content.images, annotations: content.annotations, ...(content.importLog ? { importLog: content.importLog } : {}), + // New scenes always start unpublished (Issue #458). + status: "draft", createdAt: now, updatedAt: now, }; @@ -806,6 +840,42 @@ function createScenesRouter(store = resolveSceneStore(), options = {}) { } }); + + + /** + * Publishes or unpublishes a scene (Issues #458, #459). + * Body: { status: "draft" | "published" }. Responds with the scene's summary. + * Only the status changes: the images, annotations, and `updatedAt` version + * (which Save uses to detect conflicts) stay exactly the same, so publishing + * never changes a scene's content and an open working copy can still be + * saved normally afterwards. + */ + router.patch("/:sceneId/status", async (req, res) => { + const { sceneId } = req.params; + const status = isPlainObject(req.body) ? req.body.status : undefined; + if (!SCENE_STATUSES.includes(status)) { + return res.status(400).json({ error: "status must be \"draft\" or \"published\"" }); + } + try { + await store.withLock(sceneId, async () => { + const scene = await store.get(sceneId); + if (!scene) { + res.status(404).json({ error: "Scene not found" }); + return; + } + const updated = { ...scene, status }; + if (status === "published") { + updated.publishedAt = new Date().toISOString(); + } else { + delete updated.publishedAt; + } + await store.save(updated); + res.json(toSummary(updated)); + }); + } catch (error) { + sendStoreError(res, error, "Failed to change the scene's status"); + } + }); /** * Creates an independent copy of a saved scene with its own id and a * " copy" name, numbered if taken (Issue #455). @@ -824,9 +894,12 @@ function createScenesRouter(store = resolveSceneStore(), options = {}) { ...JSON.parse(JSON.stringify(source)), id: crypto.randomUUID(), name: nextCopyName(await store.list(), source.name), + // A copy is new, unfinished work even if the original is published. + status: "draft", createdAt: now, updatedAt: now, }; + delete copy.publishedAt; await store.save(copy); res.status(201).location(`${req.baseUrl}/${copy.id}`).json(copy); }); diff --git a/boneset-api/server.js b/boneset-api/server.js index aa83b0aa..9acd5e33 100644 --- a/boneset-api/server.js +++ b/boneset-api/server.js @@ -60,9 +60,10 @@ function isValidBoneId(boneId) { return false; } - // Only allow alphanumeric characters and underscores + // Only allow letters, numbers, underscores, and hyphens. // This prevents path traversal and URL injection attacks - const validBoneIdPattern = /^[a-z0-9_]+$/i; + // (hyphens are needed for ids like 2nd-5th_metatarsals, Issue #469). + const validBoneIdPattern = /^[a-z0-9_-]+$/i; return validBoneIdPattern.test(boneId) && boneId.length > 0 && boneId.length <= 100; } @@ -304,7 +305,7 @@ app.get("/api/bone-data/", async (req, res) => { } if (!isValidBoneId(boneId)) { - return res.status(400).json({ error: "Invalid boneId format. Only alphanumeric characters and underscores are allowed." }); + return res.status(400).json({ error: "Invalid boneId format. Only letters, numbers, underscores, and hyphens are allowed." }); } const descriptionPath = path.join(DESCRIPTIONS_DIR, `${boneId}_description.json`); diff --git a/boneset-api/viewerTransition.test.js b/boneset-api/viewerTransition.test.js new file mode 100644 index 00000000..7f59fa78 --- /dev/null +++ b/boneset-api/viewerTransition.test.js @@ -0,0 +1,176 @@ +// Tests for keeping the bone viewer usable while content is moved into scenes. +// Issue #469 (parent #463). +const fs = require("fs"); +const os = require("os"); +const path = require("path"); +const request = require("supertest"); + +// Keep scenes made by these tests out of boneset-api/data/scenes. +// This has to be set before server.js is loaded. +const scenesDir = fs.mkdtempSync(path.join(os.tmpdir(), "bonebox-transition-test-")); +process.env.SCENES_DIR = scenesDir; + +const { app, serverReady } = require("./server"); + +const IMAGES_DIR = path.join(__dirname, "data", "images"); +const LONG_TEST = 30000; // tests that go through every item need more than Jest's default 5 seconds + +// Every boneset, bone, and bone part in the viewer's dropdowns. +let items = []; + +beforeAll(async () => { + await serverReady; + const response = await request(app).get("/combined-data"); + items = [ + ...response.body.bonesets.map((set) => set.id), + ...response.body.bones.map((bone) => bone.id), + ...response.body.subbones.map((subbone) => subbone.id), + ]; +}); + +afterAll(() => { + fs.rmSync(scenesDir, { recursive: true, force: true }); +}); + +const createScene = async (name) => (await request(app).post("/api/scenes").send({ name })).body; + // Import only builds the content now (#466); save it like the editor's Save does (#451). + async function importInto(sceneId, boneId) { + const imported = await request(app) + .post(`/api/scenes/${sceneId}/import-legacy`) + .send({ boneId, annotations: { labels: true, regions: true } }); + if (imported.statusCode !== 200) return imported; + + const scene = (await request(app).get(`/api/scenes/${sceneId}`)).body; + return request(app).put(`/api/scenes/${sceneId}`).send({ + images: [...scene.images, ...imported.body.images], + annotations: [...scene.annotations, ...imported.body.annotations], + importLog: [...(scene.importLog || []), imported.body.importLogEntry], + baseUpdatedAt: scene.updatedAt, + }); + } +// Everything the viewer loads for one item, so we can check it doesn't change. +async function viewerContent(boneId) { + const boneData = await request(app).get(`/api/bone-data/?boneId=${boneId}`); + const description = await request(app).get(`/api/description/?boneId=${boneId}`); + const regions = await request(app).get(`/api/colored-regions?boneId=${boneId}`); + return { + boneData: [boneData.statusCode, boneData.body], + description: [description.statusCode, description.text], + regions: [regions.statusCode, regions.body], + }; +} + +describe("Non-migrated content stays viewable - Issue 469", () => { + it("checks all the items in the dropdowns", () => { + expect(items.length).toBeGreaterThan(150); + }); + + it("gives every item an id the API accepts", async () => { + const rejected = []; + for (const id of items) { + const response = await request(app).get(`/api/bone-data/?boneId=${encodeURIComponent(id)}`); + if (response.statusCode === 400) rejected.push(id); + } + expect(rejected).toEqual([]); + }, LONG_TEST); + + it("never gives a server error for any item", async () => { + for (const id of items) { + const boneData = await request(app).get(`/api/bone-data/?boneId=${encodeURIComponent(id)}`); + // 200 = has content, 404 = no content yet (the viewer shows a message for that) + expect([200, 404]).toContain(boneData.statusCode); + + const description = await request(app).get(`/api/description/?boneId=${encodeURIComponent(id)}`); + expect(description.statusCode).toBe(200); + expect(description.text).not.toContain("Invalid bone ID"); + } + }, LONG_TEST); + + it("has a file for every image the viewer lists", async () => { + const missing = []; + for (const id of items) { + const response = await request(app).get(`/api/bone-data/?boneId=${encodeURIComponent(id)}`); + if (response.statusCode !== 200) continue; + for (const image of response.body.images) { + if (!fs.existsSync(path.join(IMAGES_DIR, image.filename))) missing.push(`${id}: ${image.filename}`); + } + } + expect(missing).toEqual([]); + }, LONG_TEST); + + it("no longer lists the leftover axis_body_(dens) item", () => { + expect(items).not.toContain("axis_body_(dens)"); + expect(items).toContain("axis_body"); + }); + + it("can open 2nd-5th metatarsals now that ids can have hyphens", async () => { + const boneData = await request(app).get("/api/bone-data/?boneId=2nd-5th_metatarsals"); + expect(boneData.statusCode).toBe(200); + expect(boneData.body.images.length).toBeGreaterThan(0); + + const regions = await request(app).get("/api/colored-regions?boneId=2nd-5th_metatarsals"); + expect(regions.statusCode).toBe(200); + }); + + it("still rejects ids that could reach other folders", async () => { + expect((await request(app).get("/api/bone-data/?boneId=../server")).statusCode).toBe(400); + expect((await request(app).get("/api/bone-data/?boneId=a/b")).statusCode).toBe(400); + expect((await request(app).get("/api/colored-regions?boneId=..%2Fserver")).statusCode).toBe(400); + }); +}); + +describe("Migrated and non-migrated content side by side - Issue 469", () => { + it("lists which viewer items each scene was made from", async () => { + const scene = await createScene("Pelvis and Femur"); + expect((await importInto(scene.id, "ilium")).statusCode).toBe(200); + expect((await importInto(scene.id, "femur")).statusCode).toBe(200); + + const list = await request(app).get("/api/scenes"); + const summary = list.body.scenes.find((s) => s.id === scene.id); + expect(summary.sources).toEqual(["ilium", "femur"]); + }); + + it("lists no sources for a scene without imported bone images", async () => { + const scene = await createScene("Empty Scene"); + const list = await request(app).get("/api/scenes"); + expect(list.body.scenes.find((s) => s.id === scene.id).sources).toEqual([]); + }); + + it("doesn't change what the viewer shows for an item after it is migrated", async () => { + const before = await viewerContent("iliac_crest"); + + const scene = await createScene("Iliac Crest Scene"); + expect((await importInto(scene.id, "iliac_crest")).statusCode).toBe(200); + + expect(await viewerContent("iliac_crest")).toEqual(before); + }); + + it("works when moving between migrated and non-migrated items in one session", async () => { + const scene = await createScene("Atlas Scene"); + expect((await importInto(scene.id, "atlas")).statusCode).toBe(200); + const atlasBefore = await viewerContent("atlas"); + + // 1. A migrated item in the viewer + expect(atlasBefore.boneData[0]).toBe(200); + + // 2. The scene made from it, with every image loading + const opened = await request(app).get(`/api/scenes/${scene.id}`); + expect(opened.statusCode).toBe(200); + expect(opened.body.images.length).toBeGreaterThan(0); + for (const image of opened.body.images) { + expect(image.source.boneId).toBe("atlas"); + const file = await request(app).get(image.src); + expect(file.statusCode).toBe(200); + } + + // 3. An item nobody has migrated + const scenes = (await request(app).get("/api/scenes")).body.scenes; + expect(scenes.some((s) => s.sources.includes("humerus_head"))).toBe(false); + const humerusHead = await viewerContent("humerus_head"); + expect(humerusHead.boneData[0]).toBe(200); + expect(humerusHead.boneData[1].images.length).toBeGreaterThan(0); + + // 4. Back to the migrated item, which looks the same as before + expect(await viewerContent("atlas")).toEqual(atlasBefore); + }); +}); diff --git a/templates/boneset.html b/templates/boneset.html index 3367c5e2..010c9ec1 100644 --- a/templates/boneset.html +++ b/templates/boneset.html @@ -140,6 +140,7 @@

Visual Reference

+ @@ -207,7 +208,12 @@

Workspace

+

+ Draft + +

+

@@ -248,6 +254,7 @@

+ diff --git a/templates/js/api.js b/templates/js/api.js index 5fe4ae9e..fd7c02b1 100644 --- a/templates/js/api.js +++ b/templates/js/api.js @@ -33,26 +33,58 @@ export async function fetchCombinedData() { } /** - * Fetch full bone data (description + images) for a single bone from the backend API. + * Possible outcomes of a bone data request (Issue #469). + * - OK: the request worked and `data` holds the bone data. + * - MISSING: the server confirmed there is no content for this item (404). + * - ERROR: the request failed (network error, 5xx, other bad status, or a + * response that couldn't be read), so we don't know whether content exists. + */ +export const BONE_DATA_STATUS = Object.freeze({ + OK: "ok", + MISSING: "missing", + ERROR: "error", +}); + +/** + * Fetch full bone data for a single bone and report what happened, so callers + * can tell content that isn't there yet apart from a failed request. * @param {string} boneId - * @returns {Object|null} bone data or null on error + * @returns {Promise<{status: string, data?: Object, httpStatus?: number, error?: Error}>} */ -export async function fetchBoneData(boneId) { - if (!boneId) return null; +export async function fetchBoneDataResult(boneId) { + if (!boneId) return { status: BONE_DATA_STATUS.MISSING }; const url = `${API_CONFIG.BASE_URL}${API_CONFIG.ENDPOINTS.BONE_DATA}/?boneId=${encodeURIComponent(boneId)}`; try { const resp = await fetch(url); + if (resp.status === 404) { + return { status: BONE_DATA_STATUS.MISSING, httpStatus: 404 }; + } if (!resp.ok) { - throw new Error(`HTTP error! status: ${resp.status}`); + const error = new Error(`HTTP error! status: ${resp.status}`); + console.error(`Error fetching bone data for ${boneId}:`, error); + return { status: BONE_DATA_STATUS.ERROR, httpStatus: resp.status, error }; } - return await resp.json(); - } catch (err) { - console.error(`Error fetching bone data for ${boneId}:`, err); - return null; + const data = await resp.json(); + return { status: BONE_DATA_STATUS.OK, httpStatus: resp.status, data }; + } catch (error) { + console.error(`Error fetching bone data for ${boneId}:`, error); + return { status: BONE_DATA_STATUS.ERROR, error }; } } +/** + * Fetch full bone data (description + images) for a single bone from the backend API. + * Kept for callers that only need the data; use `fetchBoneDataResult` when the + * reason for a missing result matters. + * @param {string} boneId + * @returns {Object|null} bone data or null on error + */ +export async function fetchBoneData(boneId) { + const result = await fetchBoneDataResult(boneId); + return result.status === BONE_DATA_STATUS.OK ? result.data : null; +} + /** * Fetch colored region data for a specific bone from the API server * @param {string} boneId - The bone identifier (e.g., "pubis", "ilium") diff --git a/templates/js/description.js b/templates/js/description.js index b2f1dc59..4bc2b2ba 100644 --- a/templates/js/description.js +++ b/templates/js/description.js @@ -8,14 +8,21 @@ import { fetchDescription } from "./api.js"; * used to construct the filename `{id}_description.json`. * @returns {Promise} */ +// Counts requests so a slow answer for an item the user already left can't +// replace the description they're reading now (Issue #469). +let latestRequest = 0; + export async function loadDescription(id) { const container = document.getElementById("description-Container"); container.innerHTML = ""; + const requestId = ++latestRequest; try { const html = await fetchDescription(id); + if (requestId !== latestRequest) return; container.innerHTML = html; } catch (error) { + if (requestId !== latestRequest) return; container.innerHTML = "
  • Error loading description.
  • "; console.error("Failed to fetch description:", error); } diff --git a/templates/js/dropdowns.js b/templates/js/dropdowns.js index ff75bc17..78fd07ca 100644 --- a/templates/js/dropdowns.js +++ b/templates/js/dropdowns.js @@ -1,8 +1,9 @@ import { loadDescription } from "./description.js"; import {displayBoneImages, showPlaceholder} from "./imageDisplay.js"; import {clearAnnotations} from "./annotationOverlay.js"; -import {fetchBoneData} from "./api.js"; +import { fetchBoneDataResult, BONE_DATA_STATUS } from "./api.js"; import { imageAnnotationToDropdownMap } from "./imageAnnotationToDropdownMap.js"; +import { showSceneLinks, SHOW_IN_VIEWER_EVENT } from "./sceneLinks.js"; document.addEventListener("DOMContentLoaded", () => { showPlaceholder(); @@ -18,6 +19,52 @@ function getImageStage() { } +// Counts image requests so a slow answer for an item the user already left +// can't replace the item they're looking at now (Issue #469). +let latestImageRequest = 0; + +/** + * Shows a message in the image area when the selected item has no images yet, + * instead of the "please select a bone" prompt (Issue #469). Some items in the + * dropdowns (like the Skull views) don't have migrated content yet. + * @returns {void} + */ +function showNoImagesMessage() { + showPlaceholder(); + const message = document.querySelector("#bone-image-container .images-placeholder p"); + if (message) { + message.textContent = "There are no images for this item yet. Try one of the bones or bone parts under it."; + } +} + +/** + * Shows an error in the image area when the item's data couldn't be loaded + * (network problem or server error), with a button to try again. + * This is kept separate from the "no images yet" message so an outage doesn't + * make real content look like it hasn't been migrated. + * @param {Function} onRetry - Called when the user clicks "Try again". + * @returns {void} + */ +function showLoadErrorMessage(onRetry) { + showPlaceholder(); + const placeholder = document.querySelector("#bone-image-container .images-placeholder"); + if (!placeholder) return; + placeholder.classList.add("images-load-error"); + placeholder.setAttribute("role", "alert"); + + const message = placeholder.querySelector("p"); + if (message) { + message.textContent = "We couldn't load the images for this item. Check your connection and try again."; + } + + const retryButton = document.createElement("button"); + retryButton.type = "button"; + retryButton.className = "images-retry-button"; + retryButton.textContent = "Try again"; + retryButton.addEventListener("click", () => onRetry()); + placeholder.appendChild(retryButton); +} + /** Helper: fetch images for a bone/sub-bone and render them. * @param {string} boneId - The bone or subbone ID to load images for. * @param {Object} [options={}] - Options forwarded to `displayBoneImages`. @@ -25,31 +72,50 @@ function getImageStage() { */ async function loadBoneImages(boneId, options = {}) { const stage = getImageStage(); + const requestId = ++latestImageRequest; + const resetStage = () => { + if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } + }; - if (stage) { - clearAnnotations(stage); - stage.classList.remove("with-annotations"); - } + resetStage(); + + // Links to any scenes made from this item (hidden when there are none). + showSceneLinks(boneId || null); if (!boneId) { showPlaceholder(); - if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } + resetStage(); return; } + + const showError = () => { + showLoadErrorMessage(() => loadBoneImages(boneId, options)); + resetStage(); + }; + try { - const data = await fetchBoneData(boneId); - const images = Array.isArray(data.images) ? data.images : []; + const result = await fetchBoneDataResult(boneId); + if (requestId !== latestImageRequest) return; + + if (!result || result.status === BONE_DATA_STATUS.ERROR) { + showError(); + return; + } + + const data = result.status === BONE_DATA_STATUS.OK ? result.data : null; + const images = data && Array.isArray(data.images) ? data.images : []; if (images.length === 0) { - showPlaceholder(); - if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } + // Confirmed missing (404) or the item exists but has no images yet. + showNoImagesMessage(); + resetStage(); } else { // Pass both boneId (for colored regions) and options (for text annotations) displayBoneImages(images, { ...options, boneId }); } } catch (err) { + if (requestId !== latestImageRequest) return; console.error("Failed to load bone images:", err); - showPlaceholder(); - if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } + showError(); } } @@ -96,6 +162,13 @@ export function setupDropdownListeners(combinedData) { if (!combinedData) return; + // The Scene Editor asks the viewer to show the item a scene came from (Issue #469). + document.addEventListener(SHOW_IN_VIEWER_EVENT, (e) => { + // Skip if this page was re-rendered and these dropdowns are no longer on it. + if (!bonesetSelect || !bonesetSelect.isConnected) return; + showItemInViewer(combinedData, e.detail && e.detail.boneId); + }); + // Boneset change bonesetSelect.addEventListener("change", (e) => { const selectedBonesetId = e.target.value; @@ -116,7 +189,7 @@ bonesetSelect.addEventListener("change", (e) => { if (!selectedBonesetId || relatedBones.length === 0) { const stage = getImageStage(); if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } - showPlaceholder(); + loadBoneImages(null); return; } @@ -168,7 +241,7 @@ boneSelect.addEventListener("change", (e) => { loadDescription(selectedBonesetId); loadBoneImages(selectedBonesetId, { boneId: selectedBonesetId }); } else { - showPlaceholder(); + loadBoneImages(null); } } }); @@ -200,7 +273,7 @@ subboneSelect.addEventListener("change", (e) => { loadDescription(selectedBoneId); loadBoneImages(selectedBoneId, { boneId: selectedBoneId }); } else { - showPlaceholder(); + loadBoneImages(null); } } }); @@ -266,4 +339,38 @@ subboneSelect.addEventListener("change", (e) => { } }); -} \ No newline at end of file +} + +/** + * Selects an item in the boneset, bone, and bone part dropdowns, the same as + * if the user picked it, so its description and images load (Issue #469). + * @param {Object} combinedData - The full application data set. + * @param {string} itemId - A boneset, bone, or bone part id. + * @returns {boolean} false if the item isn't in the dropdowns. + */ +export function showItemInViewer(combinedData, itemId) { + const bonesetSelect = document.getElementById("boneset-select"); + const boneSelect = document.getElementById("bone-select"); + const subboneSelect = document.getElementById("subbone-select"); + if (!combinedData || !itemId || !bonesetSelect || !boneSelect || !subboneSelect) return false; + + const subbone = combinedData.subbones.find((sb) => sb.id === itemId); + const bone = combinedData.bones.find((b) => b.id === (subbone ? subbone.bone : itemId)); + let bonesetId = null; + if (bone) { + bonesetId = bone.boneset; + } else if (combinedData.bonesets.some((set) => set.id === itemId)) { + bonesetId = itemId; + } + if (!bonesetId) return false; + + const choose = (select, value) => { + select.disabled = false; + select.value = value; + select.dispatchEvent(new Event("change")); + }; + choose(bonesetSelect, bonesetId); + if (bone) choose(boneSelect, bone.id); + if (subbone) choose(subboneSelect, subbone.id); + return true; +} diff --git a/templates/js/sceneLinks.js b/templates/js/sceneLinks.js new file mode 100644 index 00000000..ae042dbd --- /dev/null +++ b/templates/js/sceneLinks.js @@ -0,0 +1,98 @@ +// Links between the bone viewer and the Scene Editor while content is being +// migrated (Issue #469). Old bone viewer content and new scenes are both kept, +// so the viewer shows which scenes were made from the item that's open, and the +// Scene Editor can send the user back to the item a scene came from. + +// Events the two views use to talk to each other. +export const OPEN_SCENE_EVENT = "bonebox:open-scene"; +export const SHOW_IN_VIEWER_EVENT = "bonebox:show-in-viewer"; + +let latestRequest = 0; + +/** + * Turns an id like "iliac_crest" into a readable name ("Iliac crest"). + * @param {string} boneId + * @returns {string} + */ +export function readableName(boneId) { + const text = String(boneId || "").replace(/_/g, " ").trim(); + return text.charAt(0).toUpperCase() + text.slice(1); +} + +/** + * Gets the published scenes that have images imported from this item. Drafts + * are left out, so unfinished work isn't shown as final content (Issue #458). + * Returns an empty list if scenes can't be loaded, so the viewer still works + * when the scene store is down. + * @param {string} boneId + * @returns {Promise>} + */ +export async function fetchScenesForItem(boneId) { + try { + const response = await fetch("/api/scenes?status=published"); + if (!response.ok) return []; + const body = await response.json(); + const scenes = Array.isArray(body.scenes) ? body.scenes : []; + return scenes.filter((scene) => Array.isArray(scene.sources) && scene.sources.includes(boneId)); + } catch (error) { + console.warn("Could not load scenes for the bone viewer:", error); + return []; + } +} + +function hideLinks(box) { + box.hidden = true; + box.replaceChildren(); +} + +/** + * Shows buttons under the viewer's images for every scene made from this item. + * Hides the box when there are none, so items that haven't been migrated look + * exactly the same as before. + * @param {string|null} boneId + * @returns {Promise} + */ +export async function showSceneLinks(boneId) { + const box = document.getElementById("viewer-scene-links"); + if (!box) return; + + const requestId = ++latestRequest; + hideLinks(box); + if (!boneId) return; + + const scenes = await fetchScenesForItem(boneId); + // Ignore the answer if the user already picked a different item. + if (requestId !== latestRequest || scenes.length === 0) return; + + const label = document.createElement("p"); + label.className = "viewer-scene-links-label"; + label.textContent = scenes.length === 1 + ? "There is also a scene made from this item:" + : `There are also ${scenes.length} scenes made from this item:`; + box.append(label); + + for (const scene of scenes) { + const button = document.createElement("button"); + button.type = "button"; + button.className = "viewer-scene-link"; + button.textContent = `Open "${scene.name}" in the Scene Editor`; + button.addEventListener("click", () => { + document.dispatchEvent(new CustomEvent(OPEN_SCENE_EVENT, { detail: { sceneId: scene.id } })); + }); + box.append(button); + } + box.hidden = false; +} + +/** + * The viewer items a scene's images were imported from, in order, without repeats. + * @param {object} scene + * @returns {string[]} + */ +export function sceneSources(scene) { + const boneIds = ((scene && scene.images) || []) + .filter((image) => image && image.source && image.source.type === "legacy-image") + .map((image) => image.source.boneId) + .filter((boneId) => typeof boneId === "string"); + return [...new Set(boneIds)]; +} diff --git a/templates/js/scenes.js b/templates/js/scenes.js index 71cf48bf..9f1b450b 100644 --- a/templates/js/scenes.js +++ b/templates/js/scenes.js @@ -1,4 +1,5 @@ import { renderScene } from "./sceneCanvas.js"; +import { OPEN_SCENE_EVENT, SHOW_IN_VIEWER_EVENT, sceneSources, readableName } from "./sceneLinks.js"; const SCENES_URL = "/api/scenes"; @@ -82,6 +83,27 @@ export function duplicateScene(sceneId) { return request(`${scenePath(sceneId)}/duplicate`, { method: "POST", body: "{}" }); } +/** + * Publishes a scene for the general bone viewer, or makes it a draft again + * (Issues #458, #459). Only the status changes, never the scene's content. + * @param {string} sceneId + * @param {"draft"|"published"} status + * @returns {Promise} The scene's updated summary. + */ +export function setSceneStatus(sceneId, status) { + return request(`${scenePath(sceneId)}/status`, { method: "PATCH", body: JSON.stringify({ status }) }); +} + +/** + * A scene's publication status. Scenes without one (saved before statuses + * existed) are drafts, matching the server. + * @param {object} scene + * @returns {"draft"|"published"} + */ +export function sceneStatusOf(scene) { + return scene && scene.status === "published" ? "published" : "draft"; +} + export function deleteScene(sceneId) { return request(scenePath(sceneId), { method: "DELETE" }); } @@ -324,7 +346,9 @@ function updateControls() { dom.emptyNewButton.disabled = locked; dom.renameButton.disabled = locked || !state.active; dom.deleteButton.disabled = locked || !state.active; + dom.duplicateButton.disabled = locked || !state.active; + updatePublishControls(locked); dom.saveButton.disabled = locked || !hasUnsavedChanges(); if (dom.importToggle) { dom.importToggle.disabled = state.busy || !state.active; @@ -338,6 +362,34 @@ function updateControls() { syncUnloadGuard(); } +/** + * Shows the open scene's Draft/Published badge, a note on who can see it, and + * the matching Publish/Unpublish button (Issues #458, #459). A scene can only + * be published once its changes are saved, so what goes live is exactly what + * the editor sees. + * @param {boolean} locked - Another action is in progress. + */ +function updatePublishControls(locked) { + if (!dom.publishButton) return; + const scene = state.active; + const status = sceneStatusOf(scene); + const published = status === "published"; + const needsSave = !published && hasUnsavedChanges(); + + dom.publishButton.textContent = published ? "Unpublish" : "Publish"; + dom.publishButton.disabled = locked || !scene || needsSave; + dom.statusBadge.textContent = published ? "Published" : "Draft"; + dom.statusBadge.dataset.status = status; + if (needsSave) { + dom.statusNote.textContent = "Save your changes before publishing."; + } else if (published) { + dom.statusNote.textContent = "Visible to everyone in the bone viewer."; + } else { + dom.statusNote.textContent = "Only visible in the Scene Editor until published."; + } +} + + /** Records a change to the working copy (Issue #451). Callers re-render. */ function markChanged() { if (!state.active) return; @@ -441,7 +493,12 @@ function renderLibrary() { meta.className = "scene-list-meta"; meta.textContent = `${plural(scene.imageCount, "image")} · ${plural(scene.annotationCount, "annotation")}`; - button.append(name, meta); + const status = document.createElement("span"); + status.className = "scene-status-badge"; + status.dataset.status = sceneStatusOf(scene); + status.textContent = sceneStatusOf(scene) === "published" ? "Published" : "Draft"; + + button.append(name, meta, status); if (isActive) { const badge = document.createElement("span"); badge.className = "scene-list-badge"; @@ -528,9 +585,48 @@ function renderWorkspace() { dom.sceneTitle.textContent = scene.name; dom.sceneMeta.textContent = `${plural(scene.images.length, "image")} · ${plural(scene.annotations.length, "annotation")}`; + renderSceneSources(scene); renderCanvas(); } +/** + * Lists the bone viewer items this scene's images were imported from, each + * with a button that opens it in the bone viewer (Issue #469). The original + * viewer content is kept, so the user can always go back and compare. + */ +function renderSceneSources(scene) { + if (!dom.sceneSources) return; + dom.sceneSources.replaceChildren(); + const sources = sceneSources(scene); + dom.sceneSources.hidden = sources.length === 0; + if (sources.length === 0) return; + + dom.sceneSources.append("Made from: "); + sources.forEach((boneId, index) => { + if (index > 0) dom.sceneSources.append(", "); + const button = document.createElement("button"); + button.type = "button"; + button.className = "scene-source-link"; + button.textContent = readableName(boneId); + button.setAttribute("aria-label", `View ${readableName(boneId)} in the bone viewer`); + button.addEventListener("click", () => showInViewer(boneId)); + dom.sceneSources.append(button); + }); +} + +function showInViewer(boneId) { + leaveEditor(); + document.dispatchEvent(new CustomEvent(SHOW_IN_VIEWER_EVENT, { detail: { boneId } })); +} + +/** + * Opens a scene from the bone viewer's "Open in the Scene Editor" link (Issue #469). + */ +function openSceneFromViewer(sceneId) { + if (dom.editorView.hidden) enterEditor(); + openScene(sceneId); +} + function clearActiveScene() { setActiveScene(null); state.openRequest += 1; @@ -638,6 +734,55 @@ async function openScene(sceneId, { force = false } = {}) { } } +/** + * Publishes the open scene, or makes it a draft again (Issues #458, #459). + * Publishing doesn't touch the working copy, so any unsaved edits (only + * possible when unpublishing) stay unsaved. + */ +async function handlePublishToggle() { + if (!state.active || state.busy || state.saving) return; + const { id, name } = state.active; + const publishing = sceneStatusOf(state.active) === "draft"; + if (publishing && hasUnsavedChanges()) return; + + setBusy(true); + hideWorkspaceError(); + try { + const summary = await setSceneStatus(id, publishing ? "published" : "draft"); + if (state.active && state.active.id === id) { + for (const scene of [state.active, state.saved]) { + if (!scene) continue; + scene.status = summary.status; + if (summary.publishedAt) scene.publishedAt = summary.publishedAt; + else delete scene.publishedAt; + } + } + const listed = state.scenes.find((s) => s.id === id); + if (listed) { + listed.status = summary.status; + listed.publishedAt = summary.publishedAt; + } + setBusy(false); + renderWorkspace(); + renderLibrary(); + announce(publishing + ? `Published ${name}. It's now visible in the bone viewer.` + : `${name} is a draft again and is hidden from the bone viewer.`); + dom.publishButton.focus(); + } catch (error) { + setBusy(false); + const message = describeError(error, "publish"); + if (error.status === 404) { + handleActiveSceneMissing(message); + await loadLibrary(); + } else { + showWorkspaceError(message, { retry: handlePublishToggle }); + } + announce(message); + } +} + + async function handleCreate() { if (state.busy || state.saving) return; if (!confirmDiscard()) return; @@ -1574,6 +1719,7 @@ export function initializeSceneEditor(root = document) { titleRow: $("scene-title-row"), sceneTitle: $("scene-title"), sceneMeta: $("scene-meta"), + sceneSources: $("scene-sources"), saveStatus: $("scene-save-status"), saveButton: $("scene-save"), duplicateButton: $("scene-duplicate"), @@ -1585,6 +1731,9 @@ export function initializeSceneEditor(root = document) { renameCancel: $("scene-rename-cancel"), renameError: $("scene-rename-error"), fitToggle: $("scene-fit-toggle"), + publishButton: $("scene-publish"), + statusBadge: $("scene-status-badge"), + statusNote: $("scene-status-note"), canvas: $("scene-canvas"), canvasEmpty: $("scene-canvas-empty"), canvasNote: $("scene-canvas-note"), @@ -1624,6 +1773,12 @@ export function initializeSceneEditor(root = document) { else leaveEditor(); }); dom.backButton.addEventListener("click", leaveEditor); + const editorView = dom.editorView; + document.addEventListener(OPEN_SCENE_EVENT, (event) => { + // Skip if this page was re-rendered and this editor is no longer on it. + if (!editorView.isConnected || !event.detail || !event.detail.sceneId) return; + openSceneFromViewer(event.detail.sceneId); + }); dom.newButton.addEventListener("click", handleCreate); dom.emptyNewButton.addEventListener("click", handleCreate); dom.libraryRetry.addEventListener("click", loadLibrary); @@ -1643,6 +1798,7 @@ export function initializeSceneEditor(root = document) { dom.deleteButton.addEventListener("click", handleDelete); if (dom.saveButton) dom.saveButton.addEventListener("click", () => handleSave()); if (dom.duplicateButton) dom.duplicateButton.addEventListener("click", handleDuplicate); + if (dom.publishButton) dom.publishButton.addEventListener("click", handlePublishToggle); dom.fitToggle.addEventListener("click", () => setFitToWidth(!state.fitToWidth)); if (dom.importToggle) { dom.importToggle.addEventListener("click", () => { diff --git a/templates/style.css b/templates/style.css index 659e2c44..df983f9d 100644 --- a/templates/style.css +++ b/templates/style.css @@ -773,6 +773,47 @@ select:disabled { box-shadow: 0 4px 16px rgba(0, 0, 0, 0.12); } +/* Links from the viewer to scenes made from the open item (Issue #469) */ +.viewer-scene-links { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: var(--space-2); + padding: var(--space-3) var(--space-4); + background: #f8fafc; + border-top: 3px solid var(--slu-gold); +} + +.viewer-scene-links[hidden] { + display: none; +} + +.viewer-scene-links-label { + margin: 0; + width: 100%; + font-size: 0.9rem; + color: var(--slu-navy); +} + +.viewer-scene-link { + padding: var(--space-1) var(--space-3); + border: 2px solid var(--slu-navy); + border-radius: var(--radius-md, 6px); + background: white; + color: var(--slu-navy); + font: inherit; + font-size: 0.9rem; + text-transform: none; + letter-spacing: normal; + cursor: pointer; +} + +.viewer-scene-link:hover, +.viewer-scene-link:focus-visible { + background: var(--slu-navy); + color: white; +} + .images-panel:hover { transform: translateY(-2px); box-shadow: none; @@ -1534,6 +1575,30 @@ button:disabled { color: #d1d5db; } +/* Error state when bone data fails to load, with a retry button (Issue #469) */ +.images-placeholder.images-load-error { + flex-direction: column; + gap: var(--space-3); +} + +.images-retry-button { + padding: 8px 18px; + border: none; + border-radius: 6px; + background: var(--slu-navy); + color: #ffffff; + font-weight: 600; + cursor: pointer; +} + +.images-retry-button:hover, +.images-retry-button:focus-visible { + background: var(--slu-gold); + color: var(--slu-navy); + outline: 2px solid var(--slu-navy); + outline-offset: 2px; +} + .images-content .images-placeholder:hover { border-color: var(--slu-gold); background: linear-gradient(135deg, #374151 0%, #4b5563 100%); @@ -2778,6 +2843,75 @@ button:disabled { color: var(--text-secondary); } +/* Draft / Published status (Issues #458, #459) */ +.scene-status-row { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: var(--space-2); + margin: var(--space-1) 0 0; +} + +.scene-status-badge { + display: inline-block; + font-size: 0.75rem; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.03em; + padding: 2px var(--space-2); + border-radius: 999px; + border: 1px solid transparent; +} + +.scene-status-badge[data-status="draft"] { + background: #fef3c7; + color: #92400e; + border-color: #fcd34d; +} + +.scene-status-badge[data-status="published"] { + background: #dcfce7; + color: #166534; + border-color: #86efac; +} + +.scene-status-note { + font-size: 0.85rem; + color: var(--text-secondary); +} + +.scene-list-item .scene-status-badge { + grid-column: 1; + justify-self: start; + margin-top: var(--space-1); +} + +/* "Made from" links back to the bone viewer (Issue #469) */ +.scene-sources { + margin: var(--space-1) 0 0; + font-size: 0.9rem; + color: var(--text-secondary); +} + +.scene-source-link { + padding: 0; + border: none; + background: none; + color: var(--primary); + font: inherit; + text-transform: none; + letter-spacing: normal; + box-shadow: none; + text-decoration: underline; + cursor: pointer; +} + +.scene-source-link:hover, +.scene-source-link:focus-visible { + color: var(--slu-gold); + background: var(--primary); +} + .scene-save-status { margin: var(--space-1) 0 0; font-size: 0.85rem; diff --git a/templates/tests/dropdowns.test.js b/templates/tests/dropdowns.test.js index 46572df2..9ab6856d 100644 --- a/templates/tests/dropdowns.test.js +++ b/templates/tests/dropdowns.test.js @@ -11,12 +11,16 @@ jest.mock("../js/annotationOverlay.js", () => ({ jest.mock("../js/api.js", () => ({ // Images are keyed by boneId so tests can confirm the correct parent's // images (not just any images) are displayed after a deselect. - fetchBoneData: jest.fn((boneId) => Promise.resolve({ images: [{ url: `${boneId}.jpg` }] })), + fetchBoneDataResult: jest.fn((boneId) => Promise.resolve({ + status: "ok", + data: { images: [{ url: `${boneId}.jpg` }] }, + })), + BONE_DATA_STATUS: { OK: "ok", MISSING: "missing", ERROR: "error" }, })); const { loadDescription } = require("../js/description.js"); const { showPlaceholder, displayBoneImages } = require("../js/imageDisplay.js"); -const { fetchBoneData } = require("../js/api.js"); +const { fetchBoneDataResult } = require("../js/api.js"); const { setupDropdownListeners, populateBonesetDropdown } = require("../js/dropdowns.js"); const combinedData = { @@ -62,7 +66,7 @@ describe("Deselecting a bone/sub-bone reverts to parent info - Issue 248", () => await new Promise(process.nextTick); loadDescription.mockClear(); showPlaceholder.mockClear(); - fetchBoneData.mockClear(); + fetchBoneDataResult.mockClear(); displayBoneImages.mockClear(); selectValue(boneSelect, ""); @@ -73,7 +77,7 @@ describe("Deselecting a bone/sub-bone reverts to parent info - Issue 248", () => // Confirm the boneset's own images were fetched and rendered, not the // bone's images left over from before the deselect (nor a placeholder). - expect(fetchBoneData).toHaveBeenCalledWith("bony_pelvis"); + expect(fetchBoneDataResult).toHaveBeenCalledWith("bony_pelvis"); expect(displayBoneImages).toHaveBeenCalledWith( [{ url: "bony_pelvis.jpg" }], expect.objectContaining({ boneId: "bony_pelvis" }) @@ -87,7 +91,7 @@ describe("Deselecting a bone/sub-bone reverts to parent info - Issue 248", () => await new Promise(process.nextTick); loadDescription.mockClear(); showPlaceholder.mockClear(); - fetchBoneData.mockClear(); + fetchBoneDataResult.mockClear(); displayBoneImages.mockClear(); selectValue(subboneSelect, ""); @@ -98,7 +102,7 @@ describe("Deselecting a bone/sub-bone reverts to parent info - Issue 248", () => // Confirm the parent bone's own images were fetched and rendered, not the // sub-bone's images left over from before the deselect (nor a placeholder). - expect(fetchBoneData).toHaveBeenCalledWith("ilium"); + expect(fetchBoneDataResult).toHaveBeenCalledWith("ilium"); expect(displayBoneImages).toHaveBeenCalledWith( [{ url: "ilium.jpg" }], expect.objectContaining({ boneId: "ilium" }) diff --git a/templates/tests/scenePublish.test.js b/templates/tests/scenePublish.test.js new file mode 100644 index 00000000..2c035fca --- /dev/null +++ b/templates/tests/scenePublish.test.js @@ -0,0 +1,285 @@ +// Draft and published scenes in the Scene Editor and bone viewer. +// Issues #458 (Define Draft Scenes) and #459 (Publish and Unpublish Scenes). +const fs = require("fs"); +const path = require("path"); + +const pageHtml = fs.readFileSync(path.join(__dirname, "..", "boneset.html"), "utf8"); +const bodyHtml = pageHtml.match(/]*>([\s\S]*)<\/body>/i)[1].replace(//gi, ""); + +// Small in-memory stand-in for /api/scenes with draft/published statuses. +function createFakeBackend() { + const scenes = new Map(); + const calls = []; + let counter = 0; + let failNext = null; + const respond = (status, body) => ({ + ok: status >= 200 && status < 300, + status, + json: async () => JSON.parse(JSON.stringify(body)), + }); + const statusOf = (s) => (s.status === "published" ? "published" : "draft"); + const summary = (s) => ({ + id: s.id, name: s.name, status: statusOf(s), publishedAt: s.publishedAt || null, + imageCount: s.images.length, annotationCount: s.annotations.length, + sources: s.sources || [], createdAt: s.createdAt, updatedAt: s.updatedAt, + }); + + const fetch = jest.fn(async (url, options = {}) => { + const method = (options.method || "GET").toUpperCase(); + const body = options.body ? JSON.parse(options.body) : {}; + calls.push({ method, url, body }); + if (failNext && failNext === method) { + failNext = null; + return respond(500, { error: "Server error" }); + } + const [pathPart, query = ""] = url.split("?"); + const match = pathPart.match(/^\/api\/scenes\/?([^/]*)(?:\/(.+))?$/); + if (!match) return respond(404, { error: "Not found" }); + const id = decodeURIComponent(match[1]) || null; + const action = match[2] || null; + const now = new Date(Date.UTC(2026, 9, 1, 0, 0, counter)).toISOString(); + + if (!id && method === "GET") { + const wanted = new URLSearchParams(query).get("status"); + const listed = [...scenes.values()].filter((s) => !wanted || statusOf(s) === wanted); + return respond(200, { scenes: listed.map(summary) }); + } + if (!id && method === "POST") { + counter += 1; + const scene = { + id: `scene-${counter}`, name: body.name || `Untitled Scene ${counter}`, status: "draft", + images: [], annotations: [], createdAt: now, updatedAt: now, + }; + scenes.set(scene.id, scene); + return respond(201, scene); + } + const scene = scenes.get(id); + if (!scene) return respond(404, { error: "Scene not found" }); + if (action === "status" && method === "PATCH") { + scene.status = body.status; + if (body.status === "published") scene.publishedAt = now; + else delete scene.publishedAt; + return respond(200, summary(scene)); + } + if (action) return respond(404, { error: "Not found" }); + if (method === "GET") return respond(200, scene); + if (method === "PUT") { + counter += 1; + Object.assign(scene, { images: body.images, annotations: body.annotations, updatedAt: now }); + return respond(200, scene); + } + return respond(405, { error: "Method not allowed" }); + }); + + return { + fetch, + calls, + scenes, + seed(scene) { + counter += 1; + const full = { + id: `scene-${counter}`, images: [], annotations: [], + createdAt: "2026-09-01T00:00:00.000Z", updatedAt: "2026-09-01T00:00:00.000Z", ...scene, + }; + scenes.set(full.id, full); + return full; + }, + failNext(method) { + failNext = method; + }, + }; +} + +async function waitFor(check, timeout = 1000) { + const start = Date.now(); + for (;;) { + try { + return check(); + } catch (error) { + if (Date.now() - start > timeout) throw error; + } + await new Promise((r) => setTimeout(r, 5)); + } +} + +const $ = (id) => document.getElementById(id); +const click = (id) => $(id).click(); +const listBadge = (name) => { + const item = [...document.querySelectorAll(".scene-list-item")] + .find((b) => b.querySelector(".scene-list-name").textContent === name); + return item.querySelector(".scene-status-badge"); +}; +const statusCalls = () => backend.calls.filter((c) => c.method === "PATCH" && c.url.endsWith("/status")); + +let backend; +let scenesModule; + +beforeEach(() => { + document.body.innerHTML = bodyHtml; + backend = createFakeBackend(); + global.fetch = backend.fetch; + jest.resetModules(); + scenesModule = require("../js/scenes.js"); + scenesModule.initializeSceneEditor(); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +async function enterEditor() { + click("text-button-SceneEditor"); + await waitFor(() => expect($("scene-library-message").hidden).toBe(true)); +} + +async function openNewScene() { + await enterEditor(); + click("scene-new"); + await waitFor(() => expect($("scene-workspace-scene").hidden).toBe(false)); +} + +async function publish() { + click("scene-publish"); + await waitFor(() => expect($("scene-status-badge").textContent).toBe("Published")); +} + +function makeUnsavedChange() { + scenesModule.updateActiveScene((scene) => { + scene.annotations.push({ id: "note", type: "text", text: "New label", x: 10, y: 10 }); + }); +} + +describe("Scene editor: drafts (Issue 458)", () => { + it("marks a new scene as a draft in the title and the scene list", async () => { + await openNewScene(); + + expect($("scene-status-badge").textContent).toBe("Draft"); + expect($("scene-status-badge").dataset.status).toBe("draft"); + expect($("scene-status-note").textContent).toMatch(/only visible in the scene editor/i); + expect(listBadge("Untitled Scene 1").textContent).toBe("Draft"); + expect($("scene-publish").textContent).toBe("Publish"); + expect($("scene-publish").disabled).toBe(false); + }); + + it("shows a scene saved before statuses existed as a draft", async () => { + backend.seed({ name: "Old Scene" }); // no status field + await enterEditor(); + + expect(listBadge("Old Scene").textContent).toBe("Draft"); + }); + + it("shows which scenes in the list are published", async () => { + backend.seed({ name: "Finished", status: "published" }); + backend.seed({ name: "In Progress", status: "draft" }); + await enterEditor(); + + expect(listBadge("Finished").textContent).toBe("Published"); + expect(listBadge("Finished").dataset.status).toBe("published"); + expect(listBadge("In Progress").textContent).toBe("Draft"); + }); +}); + +describe("Scene editor: publish and unpublish (Issue 459)", () => { + it("publishes the open scene and updates the badge, note, list, and button", async () => { + await openNewScene(); + + await publish(); + + expect(statusCalls()).toEqual([ + { method: "PATCH", url: "/api/scenes/scene-1/status", body: { status: "published" } }, + ]); + expect($("scene-status-badge").dataset.status).toBe("published"); + expect($("scene-status-note").textContent).toMatch(/visible to everyone in the bone viewer/i); + expect(listBadge("Untitled Scene 1").textContent).toBe("Published"); + expect($("scene-publish").textContent).toBe("Unpublish"); + }); + + it("unpublishes a published scene, making it a draft again", async () => { + await openNewScene(); + await publish(); + + click("scene-publish"); + await waitFor(() => expect($("scene-status-badge").textContent).toBe("Draft")); + + expect(statusCalls()[1].body).toEqual({ status: "draft" }); + expect(listBadge("Untitled Scene 1").textContent).toBe("Draft"); + expect($("scene-publish").textContent).toBe("Publish"); + }); + + it("doesn't mark the scene as having unsaved changes when publishing", async () => { + await openNewScene(); + await publish(); + + expect($("scene-save-status").textContent).toBe("Saved"); + expect($("scene-save").disabled).toBe(true); + }); + + it("asks the user to save before publishing, and allows it once saved", async () => { + await openNewScene(); + makeUnsavedChange(); + + expect($("scene-publish").disabled).toBe(true); + expect($("scene-status-note").textContent).toBe("Save your changes before publishing."); + + click("scene-save"); + await waitFor(() => expect($("scene-save-status").textContent).toBe("Saved")); + expect($("scene-publish").disabled).toBe(false); + + await publish(); + expect(statusCalls()).toHaveLength(1); + }); + + it("can unpublish while there are unsaved changes, and keeps those changes", async () => { + await openNewScene(); + await publish(); + makeUnsavedChange(); + expect($("scene-publish").disabled).toBe(false); // taking a scene down is always allowed + + click("scene-publish"); + await waitFor(() => expect($("scene-status-badge").textContent).toBe("Draft")); + + expect($("scene-save-status").textContent).toBe("Unsaved changes"); + expect(scenesModule.getActiveScene().annotations.map((a) => a.id)).toEqual(["note"]); + }); + + it("shows an error with a retry when publishing fails, and leaves the scene a draft", async () => { + await openNewScene(); + backend.failNext("PATCH"); + + click("scene-publish"); + await waitFor(() => expect($("scene-workspace-error").hidden).toBe(false)); + expect($("scene-workspace-error-text").textContent).toMatch(/something went wrong/i); + expect($("scene-status-badge").textContent).toBe("Draft"); + + click("scene-workspace-retry"); + await waitFor(() => expect($("scene-status-badge").textContent).toBe("Published")); + expect($("scene-workspace-error").hidden).toBe(true); + }); + + it("keeps the status when the scene is closed and opened again", async () => { + await openNewScene(); + await publish(); + click("scene-new"); + await waitFor(() => expect($("scene-title").textContent).toBe("Untitled Scene 2")); + expect($("scene-status-badge").textContent).toBe("Draft"); + + [...document.querySelectorAll(".scene-list-item")] + .find((b) => b.querySelector(".scene-list-name").textContent === "Untitled Scene 1").click(); + await waitFor(() => expect($("scene-title").textContent).toBe("Untitled Scene 1")); + expect($("scene-status-badge").textContent).toBe("Published"); + }); +}); + +describe("Bone viewer shows only published scenes (Issues 458, 459)", () => { + it("asks for published scenes only, so drafts never appear as viewer links", async () => { + backend.seed({ name: "Published Ilium", status: "published", sources: ["ilium"] }); + backend.seed({ name: "Draft Ilium", status: "draft", sources: ["ilium"] }); + backend.seed({ name: "Published Ischium", status: "published", sources: ["ischium"] }); + const { fetchScenesForItem } = require("../js/sceneLinks.js"); + + const scenes = await fetchScenesForItem("ilium"); + + expect(backend.calls.at(-1).url).toBe("/api/scenes?status=published"); + expect(scenes.map((s) => s.name)).toEqual(["Published Ilium"]); + }); +}); \ No newline at end of file diff --git a/templates/tests/viewerTransition.test.js b/templates/tests/viewerTransition.test.js new file mode 100644 index 00000000..fea852c1 --- /dev/null +++ b/templates/tests/viewerTransition.test.js @@ -0,0 +1,357 @@ +// Tests for moving between the bone viewer and migrated scenes in one session. +// Issue #469 (parent #463). +jest.mock("../js/description.js", () => ({ + loadDescription: jest.fn(), +})); +jest.mock("../js/imageDisplay.js", () => ({ + displayBoneImages: jest.fn(), + showPlaceholder: jest.fn(), +})); +jest.mock("../js/annotationOverlay.js", () => ({ + clearAnnotations: jest.fn(), +})); +jest.mock("../js/api.js", () => ({ + fetchBoneData: jest.fn(), + fetchBoneDataResult: jest.fn(), + BONE_DATA_STATUS: { OK: "ok", MISSING: "missing", ERROR: "error" }, +})); + +const fs = require("fs"); +const path = require("path"); + +const pageHtml = fs.readFileSync(path.join(__dirname, "..", "boneset.html"), "utf8"); +const bodyHtml = pageHtml.match(/]*>([\s\S]*)<\/body>/i)[1].replace(//gi, ""); + +const combinedData = { + bonesets: [ + { id: "bony_pelvis", name: "Bony Pelvis" }, + { id: "skull", name: "Skull" }, + ], + bones: [ + { id: "ilium", name: "Ilium", boneset: "bony_pelvis" }, + { id: "ischium", name: "Ischium", boneset: "bony_pelvis" }, + { id: "anterior", name: "Anterior", boneset: "skull" }, + ], + subbones: [{ id: "iliac_crest", name: "iliac crest", bone: "ilium" }], +}; + +// A scene made from Ilium and Iliac crest. Nothing has been migrated from Ischium. +const scene = { + id: "scene-1", + name: "Ilium Study", + images: [ + { id: "img-1", src: "/api/images/ilium_image1.jpg", x: 0, y: 0, width: 100, height: 120, + source: { type: "legacy-image", boneId: "ilium", filename: "ilium_image1.jpg" } }, + { id: "img-2", src: "/api/images/ilium_image2.jpg", x: 140, y: 0, width: 100, height: 120, + source: { type: "legacy-image", boneId: "iliac_crest", filename: "ilium_image2.jpg" } }, + ], + annotations: [], + createdAt: "2026-10-01T00:00:00.000Z", + updatedAt: "2026-10-01T00:00:00.000Z", +}; +const summary = { + id: scene.id, name: scene.name, status: "published", imageCount: 2, annotationCount: 0, + sources: ["ilium", "iliac_crest"], createdAt: scene.createdAt, updatedAt: scene.updatedAt, +}; + +let scenesStatus; +const respond = (status, body) => ({ ok: status >= 200 && status < 300, status, json: async () => body }); +async function fakeFetch(url) { + if (url === "/api/scenes" || url === "/api/scenes?status=published") { + return scenesStatus === 200 ? respond(200, { scenes: [summary] }) : respond(scenesStatus, { error: "Down" }); + } + if (url === `/api/scenes/${scene.id}`) return respond(200, JSON.parse(JSON.stringify(scene))); + return respond(404, { error: "Not found" }); +} + +// Bone data the viewer gets for each item. The Skull items have no content yet, +// so the server answers 404 for them. +const okResult = (data) => ({ status: "ok", httpStatus: 200, data }); +const missingResult = () => ({ status: "missing", httpStatus: 404 }); +const errorResult = (httpStatus) => ({ status: "error", httpStatus, error: new Error("Request failed") }); +function fakeBoneData(boneId) { + if (boneId === "skull" || boneId === "anterior") return Promise.resolve(missingResult()); + return Promise.resolve(okResult({ images: [{ url: `${boneId}.jpg` }] })); +} + +async function waitFor(check, timeout = 1000) { + const start = Date.now(); + for (;;) { + try { + check(); + return; + } catch (error) { + if (Date.now() - start > timeout) throw error; + await new Promise((r) => setTimeout(r, 5)); + } + } +} + +const $ = (id) => document.getElementById(id); +const settle = () => new Promise((r) => setTimeout(r, 20)); +function choose(id, value) { + $(id).value = value; + $(id).dispatchEvent(new Event("change")); +} +const linkTexts = () => [...document.querySelectorAll(".viewer-scene-link")].map((b) => b.textContent); + +let api; +let imageDisplay; + +beforeEach(() => { + document.body.innerHTML = bodyHtml; + scenesStatus = 200; + global.fetch = jest.fn(fakeFetch); + jest.resetModules(); + + api = require("../js/api.js"); + api.fetchBoneDataResult.mockImplementation(fakeBoneData); + imageDisplay = require("../js/imageDisplay.js"); + // Same markup as the real placeholder, so the "no images" message can be checked. + imageDisplay.showPlaceholder.mockImplementation(() => { + document.getElementById("bone-image-container").innerHTML = + "

    Please select a bone from the dropdown to view its image.

    "; + }); + const dropdowns = require("../js/dropdowns.js"); + const scenes = require("../js/scenes.js"); + + dropdowns.populateBonesetDropdown(combinedData.bonesets); + dropdowns.setupDropdownListeners(combinedData); + scenes.initializeSceneEditor(); +}); + +afterEach(async () => { + // Let any requests still running finish before the next test starts. + await settle(); +}); + +describe("Bone viewer links to migrated scenes - Issue 469", () => { + it("shows a link for a migrated item and nothing for an item that isn't migrated", async () => { + choose("boneset-select", "bony_pelvis"); + choose("bone-select", "ilium"); + await waitFor(() => expect($("viewer-scene-links").hidden).toBe(false)); + expect(linkTexts()).toEqual(["Open \"Ilium Study\" in the Scene Editor"]); + + choose("bone-select", "ischium"); + await settle(); + expect($("viewer-scene-links").hidden).toBe(true); + expect(linkTexts()).toEqual([]); + // The non-migrated item still shows its normal viewer content. + expect(imageDisplay.displayBoneImages).toHaveBeenLastCalledWith( + [{ url: "ischium.jpg" }], expect.objectContaining({ boneId: "ischium" })); + }); + + it("opens the scene in the Scene Editor from the link", async () => { + choose("boneset-select", "bony_pelvis"); + choose("bone-select", "ilium"); + await waitFor(() => expect(linkTexts()).toHaveLength(1)); + + document.querySelector(".viewer-scene-link").click(); + + await waitFor(() => expect($("scene-title").textContent).toBe("Ilium Study")); + expect($("scene-editor-view").hidden).toBe(false); + expect($("editor-view").hidden).toBe(true); + }); + + it("still shows the viewer content when scenes can't be loaded", async () => { + scenesStatus = 503; + choose("boneset-select", "bony_pelvis"); + choose("bone-select", "ilium"); + await settle(); + + expect($("viewer-scene-links").hidden).toBe(true); + expect(imageDisplay.displayBoneImages).toHaveBeenLastCalledWith( + [{ url: "ilium.jpg" }], expect.objectContaining({ boneId: "ilium" })); + }); +}); + +describe("Scene Editor links back to the bone viewer - Issue 469", () => { + async function openSceneInEditor() { + $("text-button-SceneEditor").click(); + await waitFor(() => expect(document.querySelectorAll(".scene-list-item").length).toBe(1)); + document.querySelector(".scene-list-item").click(); + await waitFor(() => expect($("scene-title").textContent).toBe("Ilium Study")); + } + + it("lists the viewer items the scene was made from", async () => { + await openSceneInEditor(); + expect($("scene-sources").hidden).toBe(false); + const buttons = [...document.querySelectorAll(".scene-source-link")].map((b) => b.textContent); + expect(buttons).toEqual(["Ilium", "Iliac crest"]); + }); + + it("goes back to the bone viewer and opens the item the scene came from", async () => { + await openSceneInEditor(); + const iliacCrest = [...document.querySelectorAll(".scene-source-link")].find((b) => b.textContent === "Iliac crest"); + iliacCrest.click(); + + expect($("editor-view").hidden).toBe(false); + expect($("scene-editor-view").hidden).toBe(true); + expect($("boneset-select").value).toBe("bony_pelvis"); + expect($("bone-select").value).toBe("ilium"); + expect($("subbone-select").value).toBe("iliac_crest"); + await waitFor(() => expect(imageDisplay.displayBoneImages).toHaveBeenLastCalledWith( + [{ url: "iliac_crest.jpg" }], expect.objectContaining({ boneId: "iliac_crest" }))); + // And the viewer links back to the scene from there. + await waitFor(() => expect(linkTexts()).toEqual(["Open \"Ilium Study\" in the Scene Editor"])); + }); + + it("moves between migrated and non-migrated items in one session", async () => { + await openSceneInEditor(); + [...document.querySelectorAll(".scene-source-link")].find((b) => b.textContent === "Ilium").click(); + await waitFor(() => expect(linkTexts()).toHaveLength(1)); + + // A non-migrated item + choose("bone-select", "ischium"); + await settle(); + expect(linkTexts()).toEqual([]); + expect(imageDisplay.displayBoneImages).toHaveBeenLastCalledWith( + [{ url: "ischium.jpg" }], expect.objectContaining({ boneId: "ischium" })); + + // Back to the scene from the migrated item + choose("bone-select", "ilium"); + await waitFor(() => expect(linkTexts()).toHaveLength(1)); + document.querySelector(".viewer-scene-link").click(); + await waitFor(() => expect($("scene-editor-view").hidden).toBe(false)); + expect($("scene-title").textContent).toBe("Ilium Study"); + }); +}); + +describe("No broken or empty content - Issue 469", () => { + it("says an item has no images yet instead of asking the user to pick a bone", async () => { + choose("boneset-select", "skull"); + await settle(); + const message = document.querySelector("#bone-image-container .images-placeholder p").textContent; + expect(message).toMatch(/no images for this item yet/i); + expect(imageDisplay.displayBoneImages).not.toHaveBeenCalled(); + }); + + it("still asks the user to pick a bone when nothing is selected", async () => { + choose("boneset-select", "bony_pelvis"); + choose("boneset-select", ""); + await settle(); + const message = document.querySelector("#bone-image-container .images-placeholder p").textContent; + expect(message).toMatch(/please select a bone/i); + }); + + it("doesn't let a slow answer for an earlier item replace the current one", async () => { + api.fetchBoneDataResult.mockImplementation((boneId) => (boneId === "ilium" + ? new Promise((r) => setTimeout(() => r(okResult({ images: [{ url: "ilium.jpg" }] })), 50)) + : fakeBoneData(boneId))); + + choose("boneset-select", "bony_pelvis"); + choose("bone-select", "ilium"); + choose("bone-select", "ischium"); + await new Promise((r) => setTimeout(r, 100)); + + const shown = imageDisplay.displayBoneImages.mock.calls.map(([images]) => images[0].url); + expect(shown).toEqual(["ischium.jpg"]); + }); + }); + + describe("Missing content vs. failed requests - Issue 469", () => { + const placeholderText = () => + document.querySelector("#bone-image-container .images-placeholder p").textContent; + const retryButton = () => document.querySelector("#bone-image-container .images-retry-button"); + + it("shows an error with a retry button, not \"no images yet\", when the server fails", async () => { + api.fetchBoneDataResult.mockImplementation((boneId) => (boneId === "ilium" + ? Promise.resolve(errorResult(500)) + : fakeBoneData(boneId))); + + choose("boneset-select", "bony_pelvis"); + choose("bone-select", "ilium"); + await settle(); + + expect(placeholderText()).not.toMatch(/no images for this item yet/i); + expect(placeholderText()).toMatch(/couldn't load the images/i); + expect(retryButton()).not.toBeNull(); + expect(imageDisplay.displayBoneImages).not.toHaveBeenCalled(); + }); + + it("treats a network failure as an error, not missing content", async () => { + api.fetchBoneDataResult.mockImplementation((boneId) => (boneId === "ilium" + ? Promise.resolve({ status: "error", error: new TypeError("Failed to fetch") }) + : fakeBoneData(boneId))); + + choose("boneset-select", "bony_pelvis"); + choose("bone-select", "ilium"); + await settle(); + + expect(placeholderText()).not.toMatch(/no images for this item yet/i); + expect(retryButton()).not.toBeNull(); + }); + + it("loads the images when the user retries after a failure", async () => { + let attempts = 0; + api.fetchBoneDataResult.mockImplementation((boneId) => { + if (boneId !== "ilium") return fakeBoneData(boneId); + attempts += 1; + return Promise.resolve(attempts === 1 ? errorResult(503) : okResult({ images: [{ url: "ilium.jpg" }] })); + }); + + choose("boneset-select", "bony_pelvis"); + choose("bone-select", "ilium"); + await waitFor(() => expect(retryButton()).not.toBeNull()); + + retryButton().click(); + + await waitFor(() => expect(imageDisplay.displayBoneImages).toHaveBeenLastCalledWith( + [{ url: "ilium.jpg" }], expect.objectContaining({ boneId: "ilium" }))); + expect(attempts).toBe(2); + }); + + it("still shows \"no images yet\" (and no retry button) for content that is really missing", async () => { + choose("boneset-select", "skull"); + await settle(); + + expect(placeholderText()).toMatch(/no images for this item yet/i); + expect(retryButton()).toBeNull(); + }); +}); + +describe("fetchBoneDataResult classifies responses - Issue 469", () => { + let realApi; + let consoleSpy; + + beforeEach(() => { + realApi = jest.requireActual("../js/api.js"); + consoleSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleSpy.mockRestore(); + }); + + it("returns ok with the data for a successful response", async () => { + global.fetch = jest.fn(async () => respond(200, { images: [{ url: "ilium.jpg" }] })); + const result = await realApi.fetchBoneDataResult("ilium"); + expect(result.status).toBe(realApi.BONE_DATA_STATUS.OK); + expect(result.data).toEqual({ images: [{ url: "ilium.jpg" }] }); + }); + + it("returns missing for a 404", async () => { + global.fetch = jest.fn(async () => respond(404, { error: "Not found" })); + const result = await realApi.fetchBoneDataResult("anterior"); + expect(result.status).toBe(realApi.BONE_DATA_STATUS.MISSING); + }); + + it("returns error for a 500", async () => { + global.fetch = jest.fn(async () => respond(500, { error: "Server error" })); + const result = await realApi.fetchBoneDataResult("ilium"); + expect(result.status).toBe(realApi.BONE_DATA_STATUS.ERROR); + expect(result.httpStatus).toBe(500); + }); + + it("returns error when the network request fails", async () => { + global.fetch = jest.fn(async () => { throw new TypeError("Failed to fetch"); }); + const result = await realApi.fetchBoneDataResult("ilium"); + expect(result.status).toBe(realApi.BONE_DATA_STATUS.ERROR); + }); + + it("keeps fetchBoneData returning null on failure for its other callers", async () => { + global.fetch = jest.fn(async () => respond(500, { error: "Server error" })); + await expect(realApi.fetchBoneData("ilium")).resolves.toBeNull(); + }); +}); +