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/scenes.js b/boneset-api/scenes.js index 12e96175..814163a6 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); } @@ -243,6 +253,7 @@ function toSummary(scene) { name: scene.name, imageCount: (scene.images || []).length, annotationCount: (scene.annotations || []).length, + sources: legacySources(scene), createdAt: scene.createdAt, updatedAt: scene.updatedAt, }; 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..0bae95d1 100644 --- a/templates/boneset.html +++ b/templates/boneset.html @@ -140,6 +140,7 @@

Visual Reference

+ @@ -208,6 +209,7 @@

Workspace

+

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..8ca4c602 100644 --- a/templates/js/dropdowns.js +++ b/templates/js/dropdowns.js @@ -3,6 +3,7 @@ import {displayBoneImages, showPlaceholder} from "./imageDisplay.js"; import {clearAnnotations} from "./annotationOverlay.js"; import {fetchBoneData} from "./api.js"; import { imageAnnotationToDropdownMap } from "./imageAnnotationToDropdownMap.js"; +import { showSceneLinks, SHOW_IN_VIEWER_EVENT } from "./sceneLinks.js"; document.addEventListener("DOMContentLoaded", () => { showPlaceholder(); @@ -18,6 +19,24 @@ 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."; + } +} + /** 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,12 +44,16 @@ function getImageStage() { */ async function loadBoneImages(boneId, options = {}) { const stage = getImageStage(); + const requestId = ++latestImageRequest; if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } + // 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"); } @@ -38,17 +61,19 @@ async function loadBoneImages(boneId, options = {}) { } try { const data = await fetchBoneData(boneId); - const images = Array.isArray(data.images) ? data.images : []; + if (requestId !== latestImageRequest) return; + const images = data && Array.isArray(data.images) ? data.images : []; if (images.length === 0) { - showPlaceholder(); + showNoImagesMessage(); if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } } 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(); + showNoImagesMessage(); if (stage) { clearAnnotations(stage); stage.classList.remove("with-annotations"); } } } @@ -96,6 +121,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 +148,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 +200,7 @@ boneSelect.addEventListener("change", (e) => { loadDescription(selectedBonesetId); loadBoneImages(selectedBonesetId, { boneId: selectedBonesetId }); } else { - showPlaceholder(); + loadBoneImages(null); } } }); @@ -200,7 +232,7 @@ subboneSelect.addEventListener("change", (e) => { loadDescription(selectedBoneId); loadBoneImages(selectedBoneId, { boneId: selectedBoneId }); } else { - showPlaceholder(); + loadBoneImages(null); } } }); @@ -266,4 +298,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..e2e79a53 --- /dev/null +++ b/templates/js/sceneLinks.js @@ -0,0 +1,97 @@ +// 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 saved scenes that have images imported from this item. + * 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"); + 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..a3f6f595 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"; @@ -528,9 +529,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; @@ -1574,6 +1614,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"), @@ -1624,6 +1665,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); diff --git a/templates/style.css b/templates/style.css index 659e2c44..f0381486 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; @@ -2778,6 +2819,32 @@ button:disabled { color: var(--text-secondary); } +/* "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/viewerTransition.test.js b/templates/tests/viewerTransition.test.js new file mode 100644 index 00000000..95ea49b6 --- /dev/null +++ b/templates/tests/viewerTransition.test.js @@ -0,0 +1,244 @@ +// 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(), +})); + +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, 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") { + 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. +function fakeBoneData(boneId) { + if (boneId === "skull" || boneId === "anterior") return Promise.resolve(null); + return Promise.resolve({ 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.fetchBoneData.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.fetchBoneData.mockImplementation((boneId) => (boneId === "ilium" + ? new Promise((r) => setTimeout(() => r({ 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"]); + }); +});