From 96d235db944cc1a2eabfa2ede827ec28846aa313 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 15:18:38 +0100 Subject: [PATCH 01/13] Add e2e journeys for technical recall, deferral and second-reader comparison Safety net before moving reading data onto the episode. Between them the reading journeys now cover all four ways a reader can leave a case. --- docs/testing.md | 20 ++++- tests/e2e/helpers/modals.js | 22 +++++ tests/e2e/reading.spec.js | 168 ++++++++++++++++++++++++++++++++++-- 3 files changed, 201 insertions(+), 9 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 534f8c7c..995a2954 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -38,13 +38,28 @@ name, and the skip list is printed on every run so it stays visible. **Journeys** (`tests/e2e/`) drive real flows in Chromium via Playwright. This is the only layer that exercises POST handlers, session state and the client-side -JavaScript. Three journeys: +JavaScript. Six journeys: - a screening appointment recording medical history and a symptom, from check-in to completion - a screening appointment straight through with nothing to record - an image reading session: normal opinions, then a recall for assessment with an annotation +- a technical recall, through its views-to-retake form and the review step +- deferring a case, then unflagging it from the deferred cases page +- a second reader reaching the comparison page and keeping their opinion + +Between them the reading journeys cover all four ways a reader can leave a case +— normal, recall for assessment, technical recall and deferral — which is what +makes them useful as a net under changes to how reading data is stored. + +**Some pages render inside the shared modal, some break out of it.** With +`modalForms` on, the reading details forms and the review step load into the +modal, so ids there carry a `modal-` prefix and assertions must be scoped to +the modal — the page underneath has its own copy of the opinion buttons, and an +unscoped locator will match those instead. Recall for assessment and the +second-reader comparison deliberately break out to a full page. Which of the +two a step does is worth checking before writing selectors for it. ## Things worth knowing @@ -85,5 +100,6 @@ npx playwright show-trace test-results//trace.zip - The image-marking annotation modes in image reading (`with-images-simple` and friends), which need pixel-accurate clicks on the mammogram views. The reading spec pins `without-images`, where the location is typed instead. -- Arbitration and second-reader comparison flows +- Arbitration, and the second reader adopting the first reader's opinion + (the comparison journey covers keeping their own) - CI — the suite runs on demand, not on every push diff --git a/tests/e2e/helpers/modals.js b/tests/e2e/helpers/modals.js index ac253e4c..b91fafb9 100644 --- a/tests/e2e/helpers/modals.js +++ b/tests/e2e/helpers/modals.js @@ -58,6 +58,27 @@ const clickToOpenModal = async (page, name, options = {}) => { return openFormModal(page) } +/** + * Click a link that opens its target in the shared modal, and wait for it + * + * The link variant of clickToOpenModal - openInModal rewires links with + * data-load-modal-url rather than the data-modal-submit it puts on buttons, + * but both end up in the same shared modal. + * + * @param {import('@playwright/test').Page} page - Playwright page + * @param {string} name - Accessible name of the link + * @param {object} [options] - Extra options passed to getByRole + * @returns {import('@playwright/test').Locator} The open modal container + */ +const clickLinkToOpenModal = async (page, name, options = {}) => { + await waitForModalsReady(page) + await page + .getByRole('link', { name, ...options }) + .first() + .click() + return openFormModal(page) +} + /** * Wait for a modal to close again after submitting * @@ -82,6 +103,7 @@ module.exports = { waitForModalsReady, openFormModal, clickToOpenModal, + clickLinkToOpenModal, expectModalClosed, openSection } diff --git a/tests/e2e/reading.spec.js b/tests/e2e/reading.spec.js index 00674089..0d044df8 100644 --- a/tests/e2e/reading.spec.js +++ b/tests/e2e/reading.spec.js @@ -1,19 +1,23 @@ // tests/e2e/reading.spec.js // -// One journey through image reading: create a small session, record a normal -// opinion, then a recall for assessment with an annotation, and reach the end -// of the session. +// Journeys through image reading. Between them they cover the four ways a +// reader can leave a case - normal, recall for assessment, technical recall, +// and deferral - plus the second reader's comparison step. // -// The session is created with an explicit limit so the test reads a known, +// Sessions are created with an explicit limit so each test reads a known, // small number of cases rather than working through a default-sized session. // // Not covered: the image-marking annotation modes ('with-images-simple' and -// friends), which need pixel-accurate clicks on the mammogram views. This spec -// pins 'without-images', where the abnormality location is typed instead. +// friends), which need pixel-accurate clicks on the mammogram views. These +// specs pin 'without-images', where the abnormality location is typed instead. const { test, expect } = require('./helpers/fixtures') const { pinSettings, readingSettings } = require('./helpers/settings') -const { clickToOpenModal, expectModalClosed } = require('./helpers/modals') +const { + clickToOpenModal, + clickLinkToOpenModal, + expectModalClosed +} = require('./helpers/modals') // Cases in the session: two read as normal, the last recalled for assessment const sessionSize = 3 @@ -30,6 +34,36 @@ const recordNormal = async (page) => { await page.getByRole('button', { name: 'Normal (N)' }).first().click() } +/** + * Fill in the technical recall details for one view and continue. + * + * The technical recall form opens in the shared modal - unlike recall for + * assessment, its route doesn't break out to a full page - so ids inside it + * carry the modal's 'modal-' prefix. Field names are left alone, which makes + * them the more stable thing to target. + * + * @param {import('@playwright/test').Locator} modal - The open modal container + * @param {string} view - View code to mark for retaking, eg 'RMLO' + * @param {string} reason - Reason to select from the dropdown + */ +const recordTechnicalRecallDetails = async (modal, view, reason) => { + await modal + .locator( + `input[name="imageReadingTemp[technicalRecall][selectedViews]"][value="${view}"]` + ) + .check() + + // The reason select sits in the checkbox's conditional reveal, so it only + // becomes reachable once the view above is checked + await modal + .locator( + `select[name="imageReadingTemp[technicalRecall][views][${view}][reason]"]` + ) + .selectOption(reason) + + await modal.getByRole('button', { name: 'Continue' }).first().click() +} + test.describe('Image reading', () => { test('reads a session of cases as normal and recall', async ({ page }) => { await pinSettings(page, readingSettings) @@ -95,4 +129,124 @@ test.describe('Image reading', () => { page.getByRole('heading', { name: 'Session complete' }) ).toBeVisible() }) + + test('records a technical recall', async ({ page }) => { + // The review page is opt-in for technical recall, and it's the step that + // proves the selected views survived to the point of saving + await pinSettings(page, { + ...readingSettings, + 'settings[reading][confirmTechnicalRecall]': 'true' + }) + + await page.goto('/reading/create-session?type=all_reads&limit=1&lazy=false') + await expect(page).toHaveURL(/\/reading\/session\/[^/]+\/appointments\//) + + await expect( + page.getByRole('heading', { name: 'What is your opinion of these images?' }) + ).toBeVisible() + + const recallModal = await clickToOpenModal(page, 'Technical recall (T)') + await expect( + recallModal.getByRole('heading', { name: 'Technical recall' }) + ).toBeVisible() + + await recordTechnicalRecallDetails(recallModal, 'RMLO', 'Image blurred') + + // The review step stays in the modal, and the opinion page underneath still + // has its own "Technical recall" button - so assert against the modal, not + // the page, or the hidden button below matches first + await expect( + recallModal.getByRole('heading', { name: 'Confirm your opinion' }) + ).toBeVisible() + await expect(recallModal.getByText('Technical recall').first()).toBeVisible() + await expect(recallModal.getByText('RMLO').first()).toBeVisible() + await expect(recallModal.getByText('Image blurred')).toBeVisible() + await recallModal.getByRole('button', { name: 'Confirm and save' }).click() + + await expect( + page.getByRole('heading', { name: 'Session complete' }) + ).toBeVisible() + }) + + test('defers a case and returns it to the reading queue', async ({ page }) => { + await pinSettings(page, readingSettings) + + // The reason is the thing that identifies this deferral on the deferred + // cases page, so make it distinctive + const deferralReason = 'Prior images needed before an opinion can be given' + + await page.goto('/reading/create-session?type=all_reads&limit=1&lazy=false') + await expect(page).toHaveURL(/\/reading\/session\/[^/]+\/appointments\//) + + const deferModal = await clickLinkToOpenModal(page, 'Defer this case') + await deferModal.locator('#modal-deferralReason').fill(deferralReason) + await deferModal + .getByRole('button', { name: 'Confirm deferral' }) + .first() + .click() + + // Deferral takes the only case out of the session, so there is nothing + // left to read + await expect(page).toHaveURL(/\/no-more-cases/) + + // The case now sits on the deferred list, waiting for manual review + await page.goto('/reading/deferred') + await expect( + page.getByRole('heading', { name: 'Deferred cases' }) + ).toBeVisible() + await expect(page.getByText(deferralReason)).toBeVisible() + + // Unflagging returns it to the queue, keeping a record of why it was held + await page.getByRole('button', { name: 'Unflag case' }).first().click() + + await expect( + page.getByRole('heading', { name: 'Recently resolved' }) + ).toBeVisible() + await expect(page.getByText('No deferred cases.')).toBeVisible() + await expect(page.getByText(deferralReason)).toBeVisible() + }) + + test('shows the second reader the first read before saving', async ({ + page + }) => { + // 'early' shows the comparison as soon as an opinion is chosen, before any + // details are entered - the shortest path to the page under test. Combined + // with 'non_normal', any non-normal second opinion reaches it whatever the + // first reader said, so the test doesn't depend on the seeded read. + await pinSettings(page, { + ...readingSettings, + 'settings[reading][secondReaderComparison]': 'early', + 'settings[reading][compareWhen]': 'non_normal', + 'settings[reading][confirmTechnicalRecall]': 'true' + }) + + await page.goto( + '/reading/create-session?type=second_reads&limit=1&lazy=false' + ) + // An empty candidate list would redirect to /reading instead + await expect(page).toHaveURL(/\/reading\/session\/[^/]+\/appointments\//) + + await expect( + page.getByRole('heading', { name: 'What is your opinion of these images?' }) + ).toBeVisible() + + // Unlike the details pages, the comparison breaks out of the modal and + // takes over the page, so this is a plain click rather than clickToOpenModal + await page.getByRole('button', { name: 'Technical recall (T)' }).first().click() + + await expect(page).toHaveURL(/\/compare/) + // Exact, because the page heading ("The first reader had a different + // opinion") would otherwise match too + await expect( + page.getByRole('heading', { name: 'First read', exact: true }) + ).toBeVisible() + await expect( + page.getByRole('heading', { name: 'Your read', exact: true }) + ).toBeVisible() + + // Standing by the second opinion continues to its details page + await page.getByRole('button', { name: 'Keep your opinion' }).first().click() + + await expect(page).toHaveURL(/\/technical-recall/) + }) }) From 0ee3230662b7a184639b0da87174a9857a1be8e3 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 15:44:29 +0100 Subject: [PATCH 02/13] Move image reading onto the episode as reading cases Reads lived on appointment.imageReading, keyed by user, with the arbitration read implied by being third. They now live on episode.readingCases[] - one case per set of images, keyed to the appointment that produced it. - reads are an ordered array, each carrying readType (first/second/arbitration) settled when the read is written, since a withdrawn read shifts positions - deferral moves to the case; priors stay on the appointment - getOutcome splits into getReadingCaseState (where the case has got to) and getReadingCaseOutcome (what it found, null while reading is under way). arbitration_required and in_arbitration are separate states, ready for the manual gate that releases a case into arbitration - episode stage derives from the latest case, fixing the technical-recall snag - reading-cases.js is pure and takes a case; episodes.js resolves and writes them; reading.js is the appointment/session layer above checkEpisodes gains assertions for the new shape. Docs updated. --- app/lib/generate-seed-data.js | 79 +- app/lib/generators/episode-generator.js | 106 +- app/lib/generators/reading-generator.js | 478 +++---- app/lib/utils/appointment-status.js | 5 +- app/lib/utils/episodes.js | 151 ++- app/lib/utils/reading-cases.js | 644 +++++++++ app/lib/utils/reading.js | 1151 +++++------------ app/routes/participants.js | 4 +- app/routes/reading.js | 162 +-- app/views/_includes/reading/opinion-ui.njk | 4 +- .../_includes/reading/workflow-navigation.njk | 2 +- app/views/reading/deferred.html | 34 +- app/views/reading/history.html | 11 +- app/views/reading/index-complex.html | 12 +- app/views/reading/index-simple.html | 23 +- app/views/reading/session.html | 45 +- app/views/reading/workflow/compare.html | 2 +- app/views/reading/workflow/defer-case.html | 2 +- app/views/reading/workflow/existing-read.html | 10 +- app/views/reading/workflow/opinion.html | 4 +- docs/DATA-GENERATOR-REFERENCE.md | 10 +- docs/IMAGE-READING-TECHNICAL-SUMMARY.md | 96 +- docs/data-conventions.md | 59 +- docs/utils-filter-reference.md | 220 ++-- scripts/generate-utils-reference.js | 8 +- 25 files changed, 1890 insertions(+), 1432 deletions(-) create mode 100644 app/lib/utils/reading-cases.js diff --git a/app/lib/generate-seed-data.js b/app/lib/generate-seed-data.js index 55d8100b..55d1a8b4 100644 --- a/app/lib/generate-seed-data.js +++ b/app/lib/generate-seed-data.js @@ -15,6 +15,7 @@ const { generateAppointment } = require('./generators/appointment-generator') const { generateEpisode, generateHistoricEpisodes, + syncEpisodeReadingCases, finaliseEpisodeStage, checkEpisodes } = require('./generators/episode-generator') @@ -24,7 +25,11 @@ const { generateSingleRead } = require('./generators/reading-generator') const { getSeedDataProfile } = require('./generators/seed-profiles') -const { getOutcome } = require('./utils/reading') +const { + buildRead, + getLatestReadingCase, + getReadingCaseOutcome +} = require('./utils/reading-cases') const riskLevels = require('../data/risk-levels') @@ -398,7 +403,12 @@ const seedTechnicalRecallRescreen = ({ if (episode.appointmentIds.length !== 1) return false const appointment = appointmentsById.get(episode.appointmentIds[0]) - return Boolean(appointment) && getOutcome(appointment, {}) === 'technical_recall' + if (!appointment) return false + + return ( + getReadingCaseOutcome(getLatestReadingCase(episode), {}) === + 'technical_recall' + ) } const episode = @@ -424,12 +434,23 @@ const seedTechnicalRecallRescreen = ({ ) const [firstReader, secondReader] = users - const firstRead = generateSingleRead( + // Reads live on the episode's case for this set of images + const readingCase = getLatestReadingCase(episode) + if (!readingCase) return null + + const generatedRead = generateSingleRead( firstAppointment, firstReader.id, firstReader.role, readAt.add(1, 'day').toISOString(), - { forceOpinion: 'technical_recall', readNumber: 1 } + { forceOpinion: 'technical_recall' } + ) + const firstRead = buildRead( + readingCase, + firstReader.id, + firstReader.role, + generatedRead, + { timestamp: readAt.add(1, 'day').toISOString() } ) // The second read has to agree with the first, down to which views need @@ -439,17 +460,12 @@ const seedTechnicalRecallRescreen = ({ ...structuredClone(firstRead), readerId: secondReader.id, readerType: secondReader.role, + readType: 'second', readNumber: 2, timestamp: readAt.add(2, 'day').toISOString() } - firstAppointment.imageReading = { - ...firstAppointment.imageReading, - reads: { - [firstReader.id]: firstRead, - [secondReader.id]: secondRead - } - } + readingCase.reads = [firstRead, secondRead] finaliseEpisodeStage(episode, [firstAppointment], clinicsById) if (!owedRescreen(episode)) return null @@ -575,10 +591,27 @@ const generateData = async (options = {}) => { return new Date(a.timing.startTime) - new Date(b.timing.startTime) }) + const appointmentsById = new Map( + sortedAppointments.map((appointment) => [appointment.id, appointment]) + ) + const clinicsById = new Map(allClinics.map((clinic) => [clinic.id, clinic])) + + // Every set of images taken is a case to read, so the cases have to exist + // before there is anywhere to write reads to + const episodeAppointmentsFor = (episode) => + episode.appointmentIds + .map((appointmentId) => appointmentsById.get(appointmentId)) + .filter(Boolean) + + allEpisodes.forEach((episode) => { + syncEpisodeReadingCases(episode, episodeAppointmentsFor(episode)) + }) + console.log('Generating sample reading data...') - const appointmentsWithReadingData = generateReadingData( + generateReadingData( sortedAppointments, users, + allEpisodes, selectedSeedDataProfile ) @@ -586,28 +619,20 @@ const generateData = async (options = {}) => { // multi-appointment (technical recall) case, then the summary-level past rounds console.log('Finalising episodes...') - const appointmentsById = new Map( - appointmentsWithReadingData.map((appointment) => [appointment.id, appointment]) - ) - const clinicsById = new Map(allClinics.map((clinic) => [clinic.id, clinic])) - allEpisodes.forEach((episode) => { - const episodeAppointments = episode.appointmentIds - .map((appointmentId) => appointmentsById.get(appointmentId)) - .filter(Boolean) - finaliseEpisodeStage(episode, episodeAppointments, clinicsById) + finaliseEpisodeStage(episode, episodeAppointmentsFor(episode), clinicsById) }) const rescreenAppointment = seedTechnicalRecallRescreen({ episodes: allEpisodes, - appointments: appointmentsWithReadingData, + appointments: sortedAppointments, clinics: allClinics, participants: finalParticipants, users, seedDataProfile: selectedSeedDataProfile }) if (rescreenAppointment) { - appointmentsWithReadingData.push(rescreenAppointment) + sortedAppointments.push(rescreenAppointment) } const historicEpisodes = generateHistoricEpisodesForParticipants( @@ -628,7 +653,7 @@ const generateData = async (options = {}) => { ) // The re-screen appointment was added after the appointments map was built - appointmentsWithReadingData.forEach((appointment) => appointmentsById.set(appointment.id, appointment)) + sortedAppointments.forEach((appointment) => appointmentsById.set(appointment.id, appointment)) const episodeProblems = checkEpisodes(episodesWithHistory, appointmentsById) if (episodeProblems.length) { @@ -659,7 +684,7 @@ const generateData = async (options = {}) => { ) })) }) - writeData('appointments.json', { appointments: appointmentsWithReadingData }) + writeData('appointments.json', { appointments: sortedAppointments }) writeData('episodes.json', { episodes: episodesWithHistory }) writeData('generation-info.json', { generatedAt: new Date().toISOString(), @@ -667,7 +692,7 @@ const generateData = async (options = {}) => { stats: { participants: finalParticipants.length, clinics: allClinics.length, - appointments: appointmentsWithReadingData.length, + appointments: sortedAppointments.length, episodes: episodesWithHistory.length } }) @@ -676,7 +701,7 @@ const generateData = async (options = {}) => { console.log('Generated:') console.log(`- ${finalParticipants.length} participants`) console.log(`- ${allClinics.length} clinics`) - console.log(`- ${appointmentsWithReadingData.length} appointments`) + console.log(`- ${sortedAppointments.length} appointments`) console.log( `- ${episodesWithHistory.length} episodes ` + `(${allEpisodes.length} current, ${historicEpisodes.length} historic)` diff --git a/app/lib/generators/episode-generator.js b/app/lib/generators/episode-generator.js index ba993984..b3e3f534 100644 --- a/app/lib/generators/episode-generator.js +++ b/app/lib/generators/episode-generator.js @@ -15,7 +15,12 @@ const weighted = require('weighted') const { faker } = require('@faker-js/faker') const generateId = require('../utils/id-generator') const riskLevels = require('../../data/risk-levels') -const { getOutcome } = require('../utils/reading') +const { + buildReadingCase, + getLatestReadingCase, + getReadingCaseOutcome, + getReadsAsArray +} = require('../utils/reading-cases') const { eligibleForReading, isCompleted } = require('../utils/status') const { EPISODE_OUTCOMES, @@ -90,10 +95,42 @@ const generateEpisode = ({ participant, type, appointmentDate }) => { closedDate: null, appointmentIds: [], mammograms: [], + readingCases: [], isHistoric: false } } +/** + * Open a reading case for each set of images the episode's appointments + * produced, keeping any case that already exists. + * + * Runs before reading data is generated, because reads are written onto cases - + * there has to be a case to write to. Existing cases are kept rather than + * rebuilt so re-running never throws away reads. + * + * @param {object} episode - Episode to sync (mutated - generation only) + * @param {Array} appointments - The episode's appointments, oldest first + * @returns {object} The same episode + */ +const syncEpisodeReadingCases = (episode, appointments) => { + const existingByAppointmentId = new Map( + (episode.readingCases || []).map((readingCase) => [ + readingCase.appointmentId, + readingCase + ]) + ) + + episode.readingCases = appointments + .filter(appointmentProducedImages) + .map( + (appointment) => + existingByAppointmentId.get(appointment.id) || + buildReadingCase(appointment) + ) + + return episode +} + /** * Work out an episode's stage and outcome from the state of its appointments. * @@ -117,12 +154,14 @@ const finaliseEpisodeStage = (episode, appointments, clinicsById = new Map()) => episode.closedDate = null // The round's record of images taken is derived from the appointments alongside - // the stage, so a re-run refreshes both together + // the stage, so a re-run refreshes both together. Its reading cases follow the + // same image sets, and keep any reads already written to them episode.mammograms = appointments .filter(appointmentProducedImages) .map((appointment) => buildMammogramEntry(appointment, clinicsById.get(appointment.clinicId)) ) + syncEpisodeReadingCases(episode, appointments) const latestAppointment = appointments[appointments.length - 1] if (!latestAppointment) return episode @@ -153,10 +192,12 @@ const finaliseEpisodeStage = (episode, appointments, clinicsById = new Map()) => moveTo(destination, appointmentEnded) // Once the images are taken, it's the reading that decides what happens - // next - close the episode, or send it back for a technical recall + // next - close the episode, or send it back for a technical recall. The + // *latest* case decides: a technical recall's re-screen supersedes the + // reading that asked for it if (episode.stage === 'reading') { - const reads = Object.values(latestAppointment.imageReading?.reads || {}) - const lastReadAt = reads + const latestCase = getLatestReadingCase(episode) + const lastReadAt = getReadsAsArray(latestCase) .map((read) => read.timestamp) .sort() .pop() @@ -169,7 +210,7 @@ const finaliseEpisodeStage = (episode, appointments, clinicsById = new Map()) => : appointmentEnded const destinationAfterReading = - EPISODE_STAGE_BY_READING_OUTCOME[getOutcome(latestAppointment, {})] + EPISODE_STAGE_BY_READING_OUTCOME[getReadingCaseOutcome(latestCase, {})] if (destinationAfterReading) { moveTo(destinationAfterReading, concludedAt) @@ -305,6 +346,7 @@ const generateHistoricEpisodes = ({ openedDate: openedDate.toISOString(), closedDate: closedDate.toISOString(), appointmentIds: [], + readingCases: [], isHistoric: true, // Enough to list this round as a prior without holding a full image @@ -378,6 +420,11 @@ const checkEpisodes = (episodes, appointmentsById) => { `historic episode ${episode.id} outcome and mammograms disagree` ) } + // A summary round records that it was read, not how - it has no + // appointment for a case to hang off + if (episode.readingCases?.length) { + problems.push(`historic episode ${episode.id} has reading cases`) + } return } @@ -403,6 +450,52 @@ const checkEpisodes = (episodes, appointmentsById) => { ) } + // One reading case per image set, and none without images to read: the two + // records answer the same question from different sides, so they must agree + const caseAppointmentIds = (episode.readingCases || []).map( + (readingCase) => readingCase.appointmentId + ) + if ( + screenedAppointmentIds.length !== caseAppointmentIds.length || + screenedAppointmentIds.some( + (appointmentId) => !caseAppointmentIds.includes(appointmentId) + ) + ) { + problems.push( + `episode ${episode.id} reading cases don't match its screened appointments` + ) + } + + // A case is read at most twice until arbitration, and an arbitration read + // only makes sense once two reads have disagreed + ;(episode.readingCases || []).forEach((readingCase) => { + const reads = getReadsAsArray(readingCase) + const arbitrationReads = reads.filter( + (read) => read.readType === 'arbitration' + ) + + if (reads.length > 3) { + problems.push( + `reading case ${readingCase.id} has ${reads.length} reads` + ) + } + if (arbitrationReads.length > 1) { + problems.push( + `reading case ${readingCase.id} has more than one arbitration read` + ) + } + if (arbitrationReads.length && reads.length < 3) { + problems.push( + `reading case ${readingCase.id} has an arbitration read without two prior reads` + ) + } + if (new Set(reads.map((read) => read.readerId)).size !== reads.length) { + problems.push( + `reading case ${readingCase.id} has the same reader twice` + ) + } + }) + // Reading needs images: an episode can only be in reading off the back of // a completed mammogram appointment that is still within the reading window if (episode.stage === 'reading') { @@ -434,6 +527,7 @@ const checkEpisodes = (episodes, appointmentsById) => { module.exports = { generateEpisode, generateHistoricEpisodes, + syncEpisodeReadingCases, finaliseEpisodeStage, checkEpisodes } diff --git a/app/lib/generators/reading-generator.js b/app/lib/generators/reading-generator.js index d29cf7ca..119a3192 100644 --- a/app/lib/generators/reading-generator.js +++ b/app/lib/generators/reading-generator.js @@ -4,6 +4,7 @@ const dayjs = require('dayjs') const weighted = require('weighted') const { faker } = require('@faker-js/faker') const { eligibleForReading } = require('../utils/status') +const { buildRead } = require('../utils/reading-cases') const { getSetById, getResolvedAnnotations @@ -71,7 +72,6 @@ const TAG_TO_RESULT = { * @param {object} [options] - Generation options * @param {boolean} [options.forceAlignment] - Force alignment with set (ignore probability) * @param {string} [options.forceOpinion] - Force a specific opinion type - * @param {number} [options.readNumber] - The read number (1 = first, 2 = second) * @returns {object} The generated read object */ const generateSingleRead = ( @@ -111,13 +111,13 @@ const generateSingleRead = ( opinion = weighted.select(DEFAULT_READ_WEIGHTS) } - // Build base read object + // Build base read object. readNumber and readType are settled by buildRead, + // from where the case had got to when the read was made const read = { opinion, readerId, readerType, - timestamp, - readNumber: options.readNumber || 1 + timestamp } // Add opinion-specific data @@ -336,24 +336,80 @@ const generatePlaceholderAnnotation = (side, breastData) => { } } +/** + * Add a generated read to a case. + * + * Mutates the case: episodes are ordinary objects during generation, and only + * become shared read-only data once they're loaded into the store. + * + * @param {object} readingCase - The case being read + * @param {object} appointment - The appointment whose images these are + * @param {object} reader - The reader (a user record) + * @param {string} timestamp - ISO timestamp for the read + * @param {object} [options] - Options passed through to generateSingleRead + * @returns {object} The read that was added + */ +const addRead = (readingCase, appointment, reader, timestamp, options = {}) => { + const generated = generateSingleRead( + appointment, + reader.id, + reader.role, + timestamp, + options + ) + + // buildRead settles readNumber and readType from where the case had got to + const read = buildRead(readingCase, reader.id, reader.role, generated, { + timestamp + }) + + const existingIndex = readingCase.reads.findIndex( + (candidate) => candidate.readerId === reader.id + ) + if (existingIndex >= 0) { + readingCase.reads[existingIndex] = read + } else { + readingCase.reads.push(read) + } + + return read +} + +/** + * Pick an opinion for a second read: usually agreeing with the first, sometimes + * not, so some cases land in arbitration. + * + * @param {object} firstRead - The first read + * @param {number} [agreementProbability] - Chance of agreeing + * @returns {string} The opinion to force + */ +const pickSecondOpinion = (firstRead, agreementProbability = 0.8) => { + if (Math.random() <= agreementProbability) return firstRead.opinion + + const otherOpinions = [...new Set(Object.values(TAG_TO_RESULT))].filter( + (opinion) => opinion !== firstRead.opinion + ) + + return faker.helpers.arrayElement(otherOpinions) +} + /** * Apply reads when a backlogLimit is set. * - * All eligible appointments except the last `backlogLimit` are fully read (2 reads - * by non-current users). Of the remaining `backlogLimit` appointments: - * - The first `floor(backlogLimit × partialReadRatio)` get 1 read by a - * non-current user (second read still available to current user) - * - The rest get no reads (first read available to current user) + * All eligible cases except the last `backlogLimit` are fully read (2 reads + * by non-current users). Of the remaining `backlogLimit` cases: + * - The first `floor(backlogLimit × partialReadRatio)` get 1 read by the + * current user (so the second read is still available to someone else) + * - The rest get no reads (first read available to the current user) * - * @param {Array} allAppointments - Full appointments array * @param {Array} eligibleAppointments - Appointments eligible for reading + * @param {Map} casesByAppointmentId - Reading cases, keyed by appointment id * @param {object} readers - { firstReader, secondReader, thirdReader } * @param {object} options - { backlogLimit, backlogPartialReadRatio, alignmentProbability } - * @returns {Array} Updated appointments array */ const generateReadingDataWithBacklogLimit = ( - allAppointments, eligibleAppointments, + casesByAppointmentId, { firstReader, secondReader, thirdReader }, { backlogLimit, backlogPartialReadRatio, alignmentProbability } ) => { @@ -392,36 +448,30 @@ const generateReadingDataWithBacklogLimit = ( `${blockedByPriorsAppointments.length} unread (awaiting priors)` ) - const updatedAppointments = [...allAppointments] - let baseTime = dayjs().subtract(72, 'hours') // Fully read: 2 reads by secondReader and thirdReader fullyReadAppointments.forEach((appointment) => { - const index = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (index === -1) return - if (!updatedAppointments[index].imageReading) { - updatedAppointments[index].imageReading = { reads: {} } - } + const readingCase = casesByAppointmentId.get(appointment.id) + if (!readingCase) return baseTime = baseTime.add(1, 'minute') - const firstRead = generateSingleRead( - updatedAppointments[index], - secondReader.id, - secondReader.role, + + const firstRead = addRead( + readingCase, + appointment, + secondReader, baseTime.toISOString(), - { readNumber: 1, alignmentProbability } + { alignmentProbability } ) - updatedAppointments[index].imageReading.reads[secondReader.id] = firstRead - const secondRead = generateSingleRead( - updatedAppointments[index], - thirdReader.id, - thirdReader.role, + addRead( + readingCase, + appointment, + thirdReader, baseTime.add(15, 'minutes').toISOString(), - { forceOpinion: firstRead.opinion, readNumber: 2, alignmentProbability } + { forceOpinion: firstRead.opinion, alignmentProbability } ) - updatedAppointments[index].imageReading.reads[thirdReader.id] = secondRead }) baseTime = dayjs().subtract(24, 'hours') @@ -429,44 +479,46 @@ const generateReadingDataWithBacklogLimit = ( // Partially read: 1 read by firstReader (the current user) — so the current // user has already read these and cannot read them again partialAppointments.forEach((appointment) => { - const index = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (index === -1) return - if (!updatedAppointments[index].imageReading) { - updatedAppointments[index].imageReading = { reads: {} } - } + const readingCase = casesByAppointmentId.get(appointment.id) + if (!readingCase) return baseTime = baseTime.add(1, 'minute') - const firstRead = generateSingleRead( - updatedAppointments[index], - firstReader.id, - firstReader.role, - baseTime.toISOString(), - { readNumber: 1, alignmentProbability } - ) - updatedAppointments[index].imageReading.reads[firstReader.id] = firstRead + addRead(readingCase, appointment, firstReader, baseTime.toISOString(), { + alignmentProbability + }) }) // Unread appointments: no reads added - - return updatedAppointments } /** - * Generate sample image reading data to simulate first and second reads + * Generate sample reading data to simulate first and second reads. + * + * Reads are written onto the episode's reading cases, so the cases must already + * exist — syncEpisodeReadingCases runs over the episodes before this does. * * @param {Array} appointments - Array of screening appointments * @param {Array} users - Array of system users - * @returns {Array} Updated appointments with reading data + * @param {Array} episodes - Array of episodes, holding the reading cases + * @param {object} [seedProfile] - Active seed profile */ -const generateReadingData = (appointments, users, seedProfile = {}) => { +const generateReadingData = (appointments, users, episodes, seedProfile = {}) => { const alignmentProbability = seedProfile?.imageReading?.probabilityFirstReaderOpinionMatchesImages ?? DEFAULT_ALIGNMENT_PROBABILITY if (!appointments || !appointments.length || !users || users.length < 2) { console.log('No appointments or not enough users to generate reading data') - return appointments + return } + // Every case, keyed by the appointment whose images it covers + const casesByAppointmentId = new Map() + episodes.forEach((episode) => { + ;(episode.readingCases || []).forEach((readingCase) => { + casesByAppointmentId.set(readingCase.appointmentId, readingCase) + }) + }) + // Use the first, second, and third users as our readers const firstReader = users[0] const secondReader = users[1] @@ -482,9 +534,9 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { // default clinic-by-clinic pattern. backlogLimit=0 means empty backlog. const backlogLimit = seedProfile?.reading?.backlogLimit ?? null if (backlogLimit !== null) { - return generateReadingDataWithBacklogLimit( - appointments, + generateReadingDataWithBacklogLimit( recentAppointments, + casesByAppointmentId, { firstReader, secondReader, thirdReader }, { backlogLimit, @@ -493,6 +545,7 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { alignmentProbability } ) + return } // Sort by date (oldest first) @@ -502,7 +555,7 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { if (sortedAppointments.length === 0) { console.log('No recent completed appointments to add reading data to') - return appointments + return } // Group appointments by clinic @@ -528,22 +581,56 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { `Found ${clinics.length} clinics with completed appointments in the last 30 days` ) - // Clone the appointments array to avoid modifying the original - const updatedAppointments = [...appointments] - - // Track which appointments are updated for efficient lookup later - const updatedAppointmentIds = new Set() + // Track which appointments have been dealt with, so later passes skip them + const readAppointmentIds = new Set() // Function to generate a recent timestamp (within past 7 days) - const generateRecentTimestamp = (baseDate, minHours = 2, maxHours = 36) => { + const generateRecentTimestamp = (minHours = 2, maxHours = 36) => { const hoursAgo = Math.floor(Math.random() * (maxHours - minHours)) + minHours return dayjs().subtract(hoursAgo, 'hours').toISOString() } - // TWO OLDEST CLINICS: Complete first and second reads + /** + * Walk a clinic's appointments, giving each one a read, and note them as done + * + * @param {object} clinic - Clinic with its appointments + * @param {object} reader - Who is reading + * @param {object} [options] - Options + * @param {dayjs.Dayjs} options.startTime - Time of the first read + * @param {Array} [options.only] - Restrict to these appointments + * @param {boolean} [options.skipAlreadyRead] - Leave dealt-with appointments alone + * @returns {number} How many reads were added + */ + const readClinic = (clinic, reader, options = {}) => { + const { startTime, only = null, skipAlreadyRead = true } = options + let baseReadTime = startTime + let count = 0 + + const candidates = only || clinic.appointments + + candidates.forEach((appointment) => { + if (skipAlreadyRead && readAppointmentIds.has(appointment.id)) return + + const readingCase = casesByAppointmentId.get(appointment.id) + if (!readingCase) return + + baseReadTime = baseReadTime.add(1, 'minute') + addRead(readingCase, appointment, reader, baseReadTime.toISOString(), { + alignmentProbability + }) + + readAppointmentIds.add(appointment.id) + count++ + }) + + return count + } + + // TWO OLDEST CLINICS: complete first and second reads if (clinics.length >= 2) { let count = 0 + for (let i = 0; i < 2 && i < clinics.length; i++) { const clinic = clinics[i] console.log( @@ -551,207 +638,93 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { ) // Use the same base time for all reads in this clinic, then advance by 1 minute for each appointment - let baseReadTime = dayjs(generateRecentTimestamp(clinic.date, 48, 72)) + let baseReadTime = dayjs(generateRecentTimestamp(48, 72)) clinic.appointments.forEach((appointment) => { - // Find the appointment in our array - const appointmentIndex = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (appointmentIndex === -1) return + const readingCase = casesByAppointmentId.get(appointment.id) + if (!readingCase) return - // Ensure the imageReading structure exists - if (!updatedAppointments[appointmentIndex].imageReading) { - updatedAppointments[appointmentIndex].imageReading = { reads: {} } - } - - // Advance time by 1 minute for each read baseReadTime = baseReadTime.add(1, 'minute') - const firstReadTime = baseReadTime.toISOString() - - // First read (by second user) - aligned with image set - const firstRead = generateSingleRead( - updatedAppointments[appointmentIndex], - secondReader.id, - secondReader.role, - firstReadTime, - { readNumber: 1, alignmentProbability } + + // First read by the second user, aligned with the image set + const firstRead = addRead( + readingCase, + appointment, + secondReader, + baseReadTime.toISOString(), + { alignmentProbability } ) - updatedAppointments[appointmentIndex].imageReading.reads[secondReader.id] = - firstRead - // Second read (by first user) - 80% chance of agreement with first read + // Second read by the current user, usually agreeing with the first const secondReadTime = baseReadTime .add(Math.floor(Math.random() * 16) + 15, 'minutes') .toISOString() - // Determine second read opinion - 80% agree with first, 20% different - const forceSecondOpinion = - Math.random() > 0.8 - ? Object.keys(TAG_TO_RESULT) - .map((t) => TAG_TO_RESULT[t]) - .filter((r) => r !== firstRead.opinion)[ - Math.floor(Math.random() * 2) - ] - : firstRead.opinion - - const secondRead = generateSingleRead( - updatedAppointments[appointmentIndex], - firstReader.id, - firstReader.role, - secondReadTime, - { - forceOpinion: forceSecondOpinion, - readNumber: 2, - alignmentProbability - } - ) - updatedAppointments[appointmentIndex].imageReading.reads[firstReader.id] = - secondRead + addRead(readingCase, appointment, firstReader, secondReadTime, { + forceOpinion: pickSecondOpinion(firstRead), + alignmentProbability + }) - updatedAppointmentIds.add(appointment.id) + readAppointmentIds.add(appointment.id) count++ }) } + console.log( `Added first and second reads to ${count} appointments in the 2 oldest clinics` ) } - // NEW: Add clinic where both reads are completed, but neither by the current user + // NEXT CLINIC: both reads completed, but neither by the current user if (clinics.length >= 3) { - let count = 0 - // Use the next clinic for this scenario const clinic = clinics[2] console.log( `Adding a clinic with both reads completed by users other than current user to clinic ${clinic.id}` ) - // Use the same base time for all reads in this clinic, then advance by 1 minute for each appointment - let baseReadTime = dayjs(generateRecentTimestamp(clinic.date, 30, 48)) - - // Add full first reads by third user - clinic.appointments.forEach((appointment) => { - // Skip if already updated - if (updatedAppointmentIds.has(appointment.id)) return - - // Find the appointment in our array - const appointmentIndex = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (appointmentIndex === -1) return - - // Ensure the imageReading structure exists - if (!updatedAppointments[appointmentIndex].imageReading) { - updatedAppointments[appointmentIndex].imageReading = { reads: {} } - } - - // Advance time by 1 minute for each read - baseReadTime = baseReadTime.add(1, 'minute') - const firstReadTime = baseReadTime.toISOString() - - // First read (by third user) - aligned with image set - const firstRead = generateSingleRead( - updatedAppointments[appointmentIndex], - thirdReader.id, - thirdReader.role, - firstReadTime, - { readNumber: 1, alignmentProbability } - ) - updatedAppointments[appointmentIndex].imageReading.reads[thirdReader.id] = firstRead - - updatedAppointmentIds.add(appointment.id) - count++ + const count = readClinic(clinic, thirdReader, { + startTime: dayjs(generateRecentTimestamp(30, 48)) }) - // Add second reads by second user to 60% of appointments + // Second reads by the second user on 60% of them const appointmentsForSecondRead = clinic.appointments - .filter((appointment) => updatedAppointmentIds.has(appointment.id)) - .slice(0, Math.ceil(clinic.appointments.length * 0.6)) // Take 60% of appointments for second read + .filter((appointment) => readAppointmentIds.has(appointment.id)) + .slice(0, Math.ceil(clinic.appointments.length * 0.6)) - baseReadTime = dayjs(generateRecentTimestamp(clinic.date, 12, 24)) // More recent timestamp for second reads + let baseReadTime = dayjs(generateRecentTimestamp(12, 24)) // More recent than the first reads appointmentsForSecondRead.forEach((appointment) => { - const appointmentIndex = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (appointmentIndex === -1) return - - // Get the first read - const firstRead = - updatedAppointments[appointmentIndex].imageReading.reads[thirdReader.id] + const readingCase = casesByAppointmentId.get(appointment.id) + const firstRead = readingCase?.reads?.[0] if (!firstRead) return - // Second read (by second user) - 80% chance of agreement with first read - // Determine second read opinion - 80% agree with first, 20% different - const forceSecondOpinion = - Math.random() > 0.8 - ? Object.keys(TAG_TO_RESULT) - .map((t) => TAG_TO_RESULT[t]) - .filter((r) => r !== firstRead.opinion)[ - Math.floor(Math.random() * 2) - ] - : firstRead.opinion - - // Advance time by 1-2 minutes for each read baseReadTime = baseReadTime.add( 1 + Math.floor(Math.random() * 2), 'minute' ) - const secondReadTime = baseReadTime.toISOString() - - const secondRead = generateSingleRead( - updatedAppointments[appointmentIndex], - secondReader.id, - secondReader.role, - secondReadTime, - { - forceOpinion: forceSecondOpinion, - readNumber: 2, - alignmentProbability - } + + addRead( + readingCase, + appointment, + secondReader, + baseReadTime.toISOString(), + { forceOpinion: pickSecondOpinion(firstRead), alignmentProbability } ) - updatedAppointments[appointmentIndex].imageReading.reads[secondReader.id] = secondRead }) console.log( - `Added a clinic with ${clinic.appointments.length} first reads and ${appointmentsForSecondRead.length} second reads, both done by users other than current user` + `Added a clinic with ${count} first reads and ${appointmentsForSecondRead.length} second reads, both done by users other than current user` ) } - // NEXT TWO CLINICS: First user (current user) reads all first, but no second reads + // NEXT TWO CLINICS: current user reads all first, but no second reads if (clinics.length >= 5) { let count = 0 for (let i = 3; i < 5 && i < clinics.length; i++) { const clinic = clinics[i] console.log(`Adding first reads by current user to clinic ${clinic.id}`) - - // Use the same base time for all reads in this clinic, then advance by 1 minute for each appointment - let baseReadTime = dayjs(generateRecentTimestamp(clinic.date, 12, 36)) - - clinic.appointments.forEach((appointment) => { - // Skip if already updated - if (updatedAppointmentIds.has(appointment.id)) return - - // Find the appointment in our array - const appointmentIndex = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (appointmentIndex === -1) return - - // Ensure the imageReading structure exists - if (!updatedAppointments[appointmentIndex].imageReading) { - updatedAppointments[appointmentIndex].imageReading = { reads: {} } - } - - // Advance time by 1 minute for each read - baseReadTime = baseReadTime.add(1, 'minute') - const firstReadTime = baseReadTime.toISOString() - - // First read (by first user/current user) - aligned with image set - const firstRead = generateSingleRead( - updatedAppointments[appointmentIndex], - firstReader.id, - firstReader.role, - firstReadTime, - { readNumber: 1, alignmentProbability } - ) - updatedAppointments[appointmentIndex].imageReading.reads[firstReader.id] = firstRead - - updatedAppointmentIds.add(appointment.id) - count++ + count += readClinic(clinic, firstReader, { + startTime: dayjs(generateRecentTimestamp(12, 36)) }) } console.log( @@ -759,46 +732,14 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { ) } - // NEXT TWO CLINICS: Second user reads all first, waiting for first user (current user) to do second reads + // NEXT TWO CLINICS: second user reads all first, waiting on the current user if (clinics.length >= 7) { let count = 0 for (let i = 5; i < 7 && i < clinics.length; i++) { const clinic = clinics[i] console.log(`Adding first reads by second user to clinic ${clinic.id}`) - - // Use the same base time for all reads in this clinic, then advance by 1 minute for each appointment - let baseReadTime = dayjs(generateRecentTimestamp(clinic.date, 4, 24)) - - clinic.appointments.forEach((appointment) => { - // Skip if already updated - if (updatedAppointmentIds.has(appointment.id)) return - - // Find the appointment in our array - const appointmentIndex = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (appointmentIndex === -1) return - - // Ensure the imageReading structure exists - if (!updatedAppointments[appointmentIndex].imageReading) { - updatedAppointments[appointmentIndex].imageReading = { reads: {} } - } - - // Advance time by 1 minute for each read - baseReadTime = baseReadTime.add(1, 'minute') - const firstReadTime = baseReadTime.toISOString() - - // First read (by second user) - aligned with image set - const firstRead = generateSingleRead( - updatedAppointments[appointmentIndex], - secondReader.id, - secondReader.role, - firstReadTime, - { readNumber: 1, alignmentProbability } - ) - updatedAppointments[appointmentIndex].imageReading.reads[secondReader.id] = - firstRead - - updatedAppointmentIds.add(appointment.id) - count++ + count += readClinic(clinic, secondReader, { + startTime: dayjs(generateRecentTimestamp(4, 24)) }) } console.log( @@ -806,47 +747,21 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { ) } - // NEXT TWO OLDEST CLINICS: 75% first read by third user + // NEXT TWO CLINICS: 75% first read by the third user if (clinics.length >= 9) { let count = 0 for (let i = 7; i < 9 && i < clinics.length; i++) { const clinic = clinics[i] console.log(`Adding partial first reads to clinic ${clinic.id}`) - // Use the same base time for all reads in this clinic, then advance by 1 minute for each appointment - let baseReadTime = dayjs(generateRecentTimestamp(clinic.date, 1, 12)) - - // Only read 75% of appointments in these clinics + // Only read 75% of the appointments in these clinics const appointmentsToRead = clinic.appointments - .filter((appointment) => !updatedAppointmentIds.has(appointment.id)) - .slice(0, Math.ceil(clinic.appointments.length * 0.75)) // Take first 75% - - appointmentsToRead.forEach((appointment) => { - // Find the appointment in our array - const appointmentIndex = updatedAppointments.findIndex((e) => e.id === appointment.id) - if (appointmentIndex === -1) return + .filter((appointment) => !readAppointmentIds.has(appointment.id)) + .slice(0, Math.ceil(clinic.appointments.length * 0.75)) - // Ensure the imageReading structure exists - if (!updatedAppointments[appointmentIndex].imageReading) { - updatedAppointments[appointmentIndex].imageReading = { reads: {} } - } - - // Advance time by 1 minute for each read - baseReadTime = baseReadTime.add(1, 'minute') - const firstReadTime = baseReadTime.toISOString() - - // First read (by third user) - aligned with image set - const firstRead = generateSingleRead( - updatedAppointments[appointmentIndex], - thirdReader.id, - thirdReader.role, - firstReadTime, - { readNumber: 1, alignmentProbability } - ) - updatedAppointments[appointmentIndex].imageReading.reads[thirdReader.id] = firstRead - - updatedAppointmentIds.add(appointment.id) - count++ + count += readClinic(clinic, thirdReader, { + startTime: dayjs(generateRecentTimestamp(1, 12)), + only: appointmentsToRead }) } console.log( @@ -854,8 +769,7 @@ const generateReadingData = (appointments, users, seedProfile = {}) => { ) } - console.log(`Total appointments with reading data: ${updatedAppointmentIds.size}`) - return updatedAppointments + console.log(`Total appointments with reading data: ${readAppointmentIds.size}`) } module.exports = { diff --git a/app/lib/utils/appointment-status.js b/app/lib/utils/appointment-status.js index 6352f776..3e2cf5ff 100644 --- a/app/lib/utils/appointment-status.js +++ b/app/lib/utils/appointment-status.js @@ -17,6 +17,7 @@ const { } = require('./appointment-data') const { syncEpisodeMammogramsForAppointment, + syncReadingCasesForAppointment, advanceEpisodeForAppointmentStatus } = require('./episodes') @@ -51,8 +52,10 @@ const updateAppointmentStatus = (data, appointmentId, newStatus) => { if (!updatedAppointment) return null // Keep the appointment's episode in step, and record on the episode that - // images were taken (or weren't after all, on an undo) + // images were taken (or weren't after all, on an undo). A new set of images + // is both a mammogram entry and a new case to read, so the two sync together syncEpisodeMammogramsForAppointment(data, updatedAppointment) + syncReadingCasesForAppointment(data, updatedAppointment) advanceEpisodeForAppointmentStatus(data, updatedAppointment) return updatedAppointment diff --git a/app/lib/utils/episodes.js b/app/lib/utils/episodes.js index 15efa81d..fb1b3fcc 100644 --- a/app/lib/utils/episodes.js +++ b/app/lib/utils/episodes.js @@ -1,18 +1,25 @@ // app/lib/utils/episodes.js // // An episode is one screening round for a participant - the container its -// appointment(s) sit in. See docs/data-conventions.md. +// appointment(s) and its reading sit in. See docs/data-conventions.md. // -// Reading data still lives on appointments, not on the episode; the reading -// accessors here derive an episode's state from its appointments rather than -// holding a copy. That move happens with the work that needs it. +// Reading lives on the episode as `episode.readingCases[]`, one case per set of +// images. The case logic itself is in reading-cases.js, which is deliberately +// pure - this file owns getting cases out of session data and writing them +// back, because that means going through the episode. const dataStore = require('../data-store') const { getAppointment } = require('./appointment-data.js') -const { getReadingStatusForAppointments } = require('./reading.js') const { getClinic } = require('./clinics.js') const { formatMonthYear } = require('./dates.js') const { isActive } = require('./status.js') +const { + buildReadingCase, + getReadingCases, + getLatestReadingCase, + getReadingCaseForAppointment, + getReadingCaseOutcome +} = require('./reading-cases.js') // An episode is open until it closes. While open, the stage says where in the // process it has got to; `closed` means it has an outcome and is done. @@ -74,9 +81,9 @@ const EPISODE_STAGE_BY_APPOINTMENT_STATUS = { attended_not_screened: { stage: 'closed', outcome: 'no_result' } } -// Where a concluded reading leaves its episode. Reading outcomes that mean -// reading is still under way (not_read, pending_second_read, -// arbitration_pending) are absent - the episode stays in `reading`. +// Where a concluded reading leaves its episode. A reading still under way has +// no outcome at all (getReadingCaseOutcome returns null), and an episode with +// no reading outcome stays in `reading`. // // Note a clear reading is the only one that ends the episode. Recall for // assessment is an interim routing decision, not a result: it moves the @@ -226,18 +233,123 @@ const getEpisodeAppointments = (data, episode) => { } /** - * Get the reading status of an episode, derived from its appointments. + * Get the reading case covering an appointment's images. * - * Reading data lives on appointments, so this rescopes the existing group-level - * reading helper over just this episode's appointments. + * The single bridge from an appointment to its case. Everything that works in + * appointment terms - the reading session, the workflow pages, the backlog - + * comes through here once, at the edge, and passes the case onwards. * * @param {object} data - Session data + * @param {object} appointment - Appointment object + * @returns {object | null} The case, or null if the appointment produced no images + */ +const getReadingCase = (data, appointment) => { + if (!appointment?.episodeId) return null + + const episode = getEpisode(data, appointment.episodeId) + if (!episode) return null + + return getReadingCaseForAppointment(episode, appointment.id) +} + +/** + * Get an episode's reading cases, oldest first + * + * @param {object} episode - Episode object + * @returns {Array} The cases + */ +const getEpisodeReadingCases = (episode) => { + return getReadingCases(episode) +} + +/** + * Get the case that says where an episode's reading has got to - its latest. + * + * @param {object} episode - Episode object + * @returns {object | null} The latest case, or null if the round has no images + */ +const getEpisodeReadingCase = (episode) => { + return getLatestReadingCase(episode) +} + +/** + * Get an episode's reading outcome, from its latest case. + * + * The *latest* case, so a technical recall's re-screen supersedes the reading + * that asked for it rather than the episode being stuck on the older answer. + * * @param {object} episode - Episode object - * @param {string} [userId] - Optional user, for per-user reading counts - * @returns {object} Reading status and metrics for the episode + * @param {object} [settings] - Site settings object (data.settings) + * @returns {string | null} The outcome, or null while reading is under way */ -const getEpisodeReadingStatus = (data, episode, userId = null) => { - return getReadingStatusForAppointments(getEpisodeAppointments(data, episode), userId) +const getEpisodeReadingOutcome = (episode, settings = {}) => { + const latestCase = getLatestReadingCase(episode) + return latestCase ? getReadingCaseOutcome(latestCase, settings) : null +} + +/** + * Save a changed reading case back to its episode. + * + * Build a whole replacement case and pass it in - the one you read came out of + * shared read-only data (see docs/data-conventions.md). reading-cases.js has + * `withRead` / `withoutRead` for building those replacements. + * + * @param {object} data - Session data + * @param {string} episodeId - Episode ID + * @param {object} updatedCase - Whole replacement case record + * @returns {object | null} The updated episode, or null if not found + */ +const updateReadingCase = (data, episodeId, updatedCase) => { + const episode = getEpisode(data, episodeId) + if (!episode) { + console.warn(`updateReadingCase: no episode with id ${episodeId}`) + return null + } + + const readingCases = getReadingCases(episode).map((existing) => + existing.id === updatedCase.id ? updatedCase : existing + ) + + return updateEpisode(data, episodeId, { readingCases }) +} + +/** + * Keep an episode's reading cases in step with one of its appointments. + * + * Called alongside syncEpisodeMammogramsForAppointment, because they answer the + * same question from two sides: a new set of images is both a mammogram entry + * and a new case to read. Undoing a screened status removes both again. + * + * @param {object} data - Session data + * @param {object} appointment - The appointment whose status just changed + * @returns {object | null} The updated episode, or null if nothing to do + */ +const syncReadingCasesForAppointment = (data, appointment) => { + if (!appointment?.episodeId) return null + + const episode = getEpisode(data, appointment.episodeId) + if (!episode) return null + + const existing = getReadingCases(episode) + const otherCases = existing.filter( + (readingCase) => readingCase.appointmentId !== appointment.id + ) + + if (!appointmentProducedImages(appointment)) { + // No images from this appointment any more - drop its case. Any reads on it + // go too, which is right: they were reads of images that didn't happen. + if (otherCases.length === existing.length) return null + return updateEpisode(data, episode.id, { readingCases: otherCases }) + } + + // Already has a case - leave it and its reads alone + if (otherCases.length !== existing.length) return null + + const readingCases = [...existing, buildReadingCase(appointment)].sort( + (a, b) => new Date(a.openedDate) - new Date(b.openedDate) + ) + + return updateEpisode(data, episode.id, { readingCases }) } /** @@ -573,7 +685,7 @@ const advanceEpisodeForAppointmentStatus = (data, appointment) => { * * @param {object} data - Session data * @param {object} appointment - The appointment that was read - * @param {string} readingOutcome - Outcome from getOutcome + * @param {string} readingOutcome - Outcome from getReadingCaseOutcome * @returns {object | null} The updated episode, or null if nothing to do */ const advanceEpisodeForReadingOutcome = (data, appointment, readingOutcome) => { @@ -598,7 +710,12 @@ module.exports = { getEpisodesForParticipant, getCurrentEpisode, getEpisodeAppointments, - getEpisodeReadingStatus, + getReadingCase, + getEpisodeReadingCases, + getEpisodeReadingCase, + getEpisodeReadingOutcome, + updateReadingCase, + syncReadingCasesForAppointment, getEpisodeMammogramDate, getLastMammogram, getNextAppointment, diff --git a/app/lib/utils/reading-cases.js b/app/lib/utils/reading-cases.js new file mode 100644 index 00000000..b1ad25cd --- /dev/null +++ b/app/lib/utils/reading-cases.js @@ -0,0 +1,644 @@ +// app/lib/utils/reading-cases.js +// +// A reading case is one set of mammograms being read. Taking images triggers a +// case; the reads - first, second, and later an arbitration read - belong to +// the case rather than to the episode or the appointment directly. Cases live +// on the episode as `episode.readingCases[]`, one per image set, oldest first. +// +// Most episodes have one case. A technical recall produces a second set of +// images and therefore a second case, which is why the episode's reading state +// comes from its *latest* case rather than from any appointment. +// +// The trigger rule is: new image set -> new case. `episode.mammograms[]` is +// already one entry per image set, so cases are opened alongside those entries +// (see syncReadingCasesForAppointment). +// +// Everything here takes a case, not an appointment. Resolving the case from an +// appointment happens once, at the edges - route handlers, the reading workflow +// middleware, and the list-building helpers in reading.js - via +// getReadingCase. + +const generateId = require('./id-generator') + +// What kind of read this was, recorded when the read is written rather than +// worked out later from its position. Once arbitration has a manual gate in +// front of it, "was this an arbitration read" depends on where the case had got +// to at the time, which nothing can recover afterwards. +const READ_TYPES = ['first', 'second', 'arbitration'] + +// Where a case has got to. Derived from its reads plus the acts recorded on it +// - never stored, so it can't drift from the reads it describes. +// +// `arbitration_required` and `in_arbitration` are deliberately different +// states: reads disagreeing is what makes arbitration *necessary*, but a case +// only becomes arbitratable once someone releases it. Nothing performs that +// release yet, so no case reaches `in_arbitration` today. +const READING_CASE_STATES = [ + 'awaiting_first_read', + 'awaiting_second_read', + 'arbitration_required', + 'in_arbitration', + 'concluded' +] + +// What a concluded case found. Note this is the *reading* outcome, not the +// episode outcome - a recall for assessment concludes the reading but leaves +// the episode open, because the result comes from the assessment. +// +// Deferral and outstanding priors are deliberately not outcomes: both are +// temporary states that hold a case up, and a case held up still owes an +// outcome once it is released. +const READING_CASE_OUTCOMES = [ + 'normal', + 'technical_recall', + 'recall_for_assessment' +] + +/** + * Build a new reading case for one set of images. + * + * Shared by the seed generator and the runtime sync so the two can't drift. + * + * @param {object} appointment - The appointment that produced the images + * @param {string} [openedDate] - When the case opened; defaults to when the + * images were taken + * @returns {object} Reading case record + */ +const buildReadingCase = (appointment, openedDate = null) => { + return { + id: generateId(), + appointmentId: appointment.id, + openedDate: + openedDate || + appointment.timing?.actualStartTime || + appointment.timing?.startTime || + null, + reads: [] + } +} + +/** + * All of an episode's reading cases, oldest first + * + * @param {object} episode - Episode object + * @returns {Array} Reading cases + */ +const getReadingCases = (episode) => { + return episode?.readingCases || [] +} + +/** + * An episode's most recent reading case - the one that decides where the + * episode has got to. + * + * A technical recall leaves an older, superseded case behind; that one is + * history, and must not drag the episode backwards. + * + * @param {object} episode - Episode object + * @returns {object | null} The latest case, or null if the round has no images yet + */ +const getLatestReadingCase = (episode) => { + const cases = getReadingCases(episode) + return cases[cases.length - 1] || null +} + +/** + * Find the reading case covering a given appointment's images + * + * @param {object} episode - Episode object + * @param {string} appointmentId - Appointment ID + * @returns {object | null} The case, or null if that appointment produced no images + */ +const getReadingCaseForAppointment = (episode, appointmentId) => { + return ( + getReadingCases(episode).find( + (readingCase) => readingCase.appointmentId === appointmentId + ) || null + ) +} + +/** + * A case's reads in order, oldest first. + * + * Reads are stored in order, so this mainly exists to give callers a safe empty + * array and a single place to change if ordering ever needs deriving again. + * + * @param {object} readingCase - Reading case + * @returns {Array} The reads + */ +const getReadsAsArray = (readingCase) => { + return readingCase?.reads || [] +} + +/** + * Get one user's read on a case + * + * @param {object} readingCase - Reading case + * @param {string} userId - User ID + * @returns {object | null} Their read, or null if they haven't read it + */ +const getReadForUser = (readingCase, userId) => { + if (!userId) return null + + return ( + getReadsAsArray(readingCase).find((read) => read.readerId === userId) || null + ) +} + +/** + * Get the reads on a case made by anyone other than the given user + * + * @param {object} readingCase - Reading case + * @param {string} userId - User ID to exclude + * @returns {Array} The other reads, in order + */ +const getOtherReads = (readingCase, userId) => { + return getReadsAsArray(readingCase).filter((read) => read.readerId !== userId) +} + +/** + * Get the arbitration read on a case, if one has been made + * + * @param {object} readingCase - Reading case + * @returns {object | null} The arbitration read, or null + */ +const getArbitrationRead = (readingCase) => { + return ( + getReadsAsArray(readingCase).find((read) => read.readType === 'arbitration') || + null + ) +} + +/** + * Whether a user has read a case + * + * @param {object} readingCase - Reading case + * @param {string} userId - User ID + * @returns {boolean} + */ +const userHasReadCase = (readingCase, userId) => { + return Boolean(getReadForUser(readingCase, userId)) +} + +/** + * Whether a case has any reads + * + * @param {object} readingCase - Reading case + * @returns {boolean} + */ +const caseHasReads = (readingCase) => { + return getReadsAsArray(readingCase).length > 0 +} + +/** + * Whether a case has been deferred from reading. + * + * Deferral is recorded as an act (who deferred it and when), and the state is + * read back from its presence - the same shape arbitration release takes. + * + * @param {object} readingCase - Reading case + * @returns {boolean} + */ +const isCaseDeferred = (readingCase) => { + return Boolean(readingCase?.deferral?.deferredAt) +} + +/** + * Whether a case has been released into arbitration. + * + * Nothing performs the release yet - the flow that does is future work - so + * this is false for every case today. It exists so the state vocabulary is + * whole, rather than growing a new value later across every call site. + * + * @param {object} readingCase - Reading case + * @returns {boolean} + */ +const isCaseInArbitration = (readingCase) => { + return Boolean(readingCase?.arbitration?.releasedAt) +} + +/** + * Whether the reads on a case disagree in a clinically meaningful way. + * + * Rules: + * - Different top-level opinions -> always discordant + * - Both technical recall: discordant if the set of selected views differs + * (reasons are ignored - same views = concordant even with different reasons) + * - Both recall for assessment: discordant if either per-breast assessment + * differs (annotations and comments are ignored) + * - Both normal -> concordant + * + * Handles partial data gracefully: if TR views or RFA breast assessments are + * not yet filled in, falls back to comparing only what's available. + * + * @param {object} readA - First read (saved read or imageReadingTemp) + * @param {object} readB - Second read (saved read or imageReadingTemp) + * @returns {boolean} Whether the reads are discordant + */ +const areReadsDiscordant = (readA, readB) => { + if (!readA?.opinion || !readB?.opinion) return false + + // Different top-level opinions -> always discordant + if (readA.opinion !== readB.opinion) return true + + const opinion = readA.opinion + + // Both TR: compare the set of selected view keys + if (opinion === 'technical_recall') { + const viewsA = readA.technicalRecall?.views + const viewsB = readB.technicalRecall?.views + // If either side has no view data yet, can't compare further + if (!viewsA || !viewsB) return false + const keysA = new Set(Object.keys(viewsA)) + const keysB = new Set(Object.keys(viewsB)) + if (keysA.size !== keysB.size) return true + for (const view of keysA) { + if (!keysB.has(view)) return true + } + return false + } + + // Both RFA: compare per-breast assessments + if (opinion === 'recall_for_assessment') { + const leftA = readA.left?.breastAssessment + const leftB = readB.left?.breastAssessment + const rightA = readA.right?.breastAssessment + const rightB = readB.right?.breastAssessment + // If no breast data on either side yet, can't compare further + if (!leftA && !leftB && !rightA && !rightB) return false + if ((leftA || leftB) && leftA !== leftB) return true + if ((rightA || rightB) && rightA !== rightB) return true + return false + } + + return false +} + +/** + * Whether two reads mean the case needs arbitrating, taking the site's + * arbitration policy into account. + * + * Policies (from settings.reading.arbitrationPolicy): + * - 'discordant_only' (default): only discordant reads need arbitration + * - 'all_non_normal': any concordant non-normal outcome does too + * + * @param {object} readA - First read + * @param {object} readB - Second read + * @param {object} [settings] - Site settings object (data.settings) + * @returns {boolean} + */ +const willGoToArbitration = (readA, readB, settings = {}) => { + if (!readA || !readB) return false + + // Discordant reads always go to arbitration + if (areReadsDiscordant(readA, readB)) return true + + // Concordant but non-normal: depends on policy + const policy = settings?.reading?.arbitrationPolicy || 'discordant_only' + if (policy === 'all_non_normal') { + return readA.opinion !== 'normal' + } + + return false +} + +/** + * Where a case has got to. + * + * Deferral and outstanding priors are not states here - both hold a case up + * without changing how far through reading it is, so they belong alongside the + * state as flags rather than replacing it. + * + * @param {object} readingCase - Reading case + * @param {object} [settings] - Site settings object (data.settings) + * @returns {string} One of READING_CASE_STATES + */ +const getReadingCaseState = (readingCase, settings = {}) => { + const reads = getReadsAsArray(readingCase) + + if (reads.length === 0) return 'awaiting_first_read' + if (reads.length === 1) return 'awaiting_second_read' + + // An arbitration read settles the case whatever the first two said + if (getArbitrationRead(readingCase)) return 'concluded' + + const [firstRead, secondRead] = reads + + if (willGoToArbitration(firstRead, secondRead, settings)) { + return isCaseInArbitration(readingCase) + ? 'in_arbitration' + : 'arbitration_required' + } + + return 'concluded' +} + +/** + * What a case found, or null while reading is still under way. + * + * Null is meaningful: it says the reading hasn't produced an answer yet, which + * is what leaves an episode sitting in `reading`. + * + * @param {object} readingCase - Reading case + * @param {object} [settings] - Site settings object (data.settings) + * @returns {string | null} One of READING_CASE_OUTCOMES, or null + */ +const getReadingCaseOutcome = (readingCase, settings = {}) => { + if (getReadingCaseState(readingCase, settings) !== 'concluded') return null + + // Arbitration, where it happened, is the deciding read + const arbitrationRead = getArbitrationRead(readingCase) + if (arbitrationRead) return arbitrationRead.opinion || null + + // Otherwise the two reads agreed, so either one gives the answer + return getReadsAsArray(readingCase)[0]?.opinion || null +} + +/** + * Summary counts and flags for a case, for lists and progress displays + * + * @param {object} readingCase - Reading case + * @param {object} [settings] - Site settings object (data.settings) + * @returns {object} Reading metadata + */ +const getReadingMetadata = (readingCase, settings = {}) => { + const reads = getReadsAsArray(readingCase) + const uniqueReaderCount = new Set(reads.map((read) => read.readerId)).size + const opinions = [...new Set(reads.map((read) => read.opinion))].filter( + Boolean + ) + + // Discordance is richer than comparing opinion strings - it also checks TR + // views and RFA breast assessments + const isDiscordant = + reads.length >= 2 ? areReadsDiscordant(reads[0], reads[1]) : false + + return { + readCount: reads.length, + uniqueReaderCount, + firstReadComplete: reads.length >= 1, + secondReadComplete: reads.length >= 2, + isDiscordant, + opinions, + state: getReadingCaseState(readingCase, settings), + outcome: getReadingCaseOutcome(readingCase, settings) + } +} + +/** + * Whether a case still needs a first read + * + * @param {object} readingCase - Reading case + * @returns {boolean} + */ +const caseNeedsFirstRead = (readingCase) => { + return !caseHasReads(readingCase) +} + +/** + * Whether a case has a first read and still needs a second + * + * @param {object} readingCase - Reading case + * @returns {boolean} + */ +const caseNeedsSecondRead = (readingCase) => { + return getReadsAsArray(readingCase).length === 1 +} + +/** + * Whether a case needs arbitrating but hasn't been released into it + * + * @param {object} readingCase - Reading case + * @param {object} [settings] - Site settings object (data.settings) + * @returns {boolean} + */ +const caseNeedsArbitration = (readingCase, settings = {}) => { + return getReadingCaseState(readingCase, settings) === 'arbitration_required' +} + +/** + * Whether a user can read a case. + * + * Only covers what the case itself knows. Outstanding priors also block + * reading, but those live on the appointment, so callers working from an + * appointment combine the two (see canUserReadAppointment in reading.js). + * + * @param {object} readingCase - Reading case + * @param {string} userId - User ID + * @param {object} [options] - Options + * @param {number} [options.maxReadsPerCase] - Reads needed before it's complete + * @returns {boolean} + */ +const canUserReadCase = (readingCase, userId, options = {}) => { + const { maxReadsPerCase = 2 } = options + + if (!userId) return false + + // A deferred case is out of the queue until someone reviews it + if (isCaseDeferred(readingCase)) return false + + // Enough readers have had it already + if (getReadsAsArray(readingCase).length >= maxReadsPerCase) return false + + // Nobody reads the same case twice + return !userHasReadCase(readingCase, userId) +} + +/** + * Work out what the second reader should be shown about the first read. + * + * Returns false when there is nothing to compare - the user is the first + * reader, or both opinions are normal. + * + * @param {object} readingCase - The case being read + * @param {object | string} secondReadData - The second reader's working data + * (imageReadingTemp or a read object); a bare opinion string also works + * @param {string} userId - Current user ID + * @param {object} [settings] - Site settings object (data.settings) + * @returns {false | object} False if no comparison is needed, else the details + */ +const getComparisonInfo = ( + readingCase, + secondReadData, + userId, + settings = {} +) => { + // Support passing just an opinion string + const secondRead = + typeof secondReadData === 'string' + ? { opinion: secondReadData } + : secondReadData + + // Reads are held in order, so the earliest by anyone else is the first read + const firstRead = getOtherReads(readingCase, userId)[0] + + // No first read exists - the user is the first reader + if (!firstRead) return false + + const firstOpinion = firstRead.opinion + const secondOpinion = secondRead?.opinion + + // Both normal - nothing worth comparing + if (firstOpinion === 'normal' && secondOpinion === 'normal') return false + + const discordant = areReadsDiscordant(firstRead, secondRead) + + return { + type: discordant ? 'discordant' : 'agreeing', + discordant, + goesToArbitration: willGoToArbitration(firstRead, secondRead, settings), + firstRead, + firstOpinion, + secondOpinion + } +} + +/** + * Whether the compare page should be shown to the second reader. + * + * Combines whether there is anything to compare with the show-when setting + * (settings.reading.compareWhen): + * - 'non_normal' (default): show whenever either opinion is non-normal + * - 'discordant_only': only show when the two reads actually disagree + * + * The separate timing setting (secondReaderComparison) decides *when* in the + * flow to ask this, and is handled by the routes. + * + * @param {object} readingCase - The case being read + * @param {object} secondReadData - The second reader's working data + * @param {string} userId - Current user ID + * @param {object} [settings] - Site settings object (data.settings) + * @returns {boolean} + */ +const shouldShowComparePage = ( + readingCase, + secondReadData, + userId, + settings = {} +) => { + const comparisonInfo = getComparisonInfo( + readingCase, + secondReadData, + userId, + settings + ) + + if (!comparisonInfo) return false + + const compareWhen = settings?.reading?.compareWhen || 'non_normal' + + if (compareWhen === 'discordant_only') return comparisonInfo.discordant + + return true +} + +/** + * Build the read record for a user's opinion on a case. + * + * The read type is settled here, from where the case had got to when the read + * was made - so it stays true even if reads are later withdrawn. + * + * @param {object} readingCase - The case being read + * @param {string} userId - Reader's user ID + * @param {string} readerType - Reader's role + * @param {object} reading - The opinion and its details + * @param {object} [options] - Options + * @param {string} [options.timestamp] - When the read was made + * @returns {object} The read record + */ +const buildRead = (readingCase, userId, readerType, reading, options = {}) => { + const existingRead = getReadForUser(readingCase, userId) + const otherReads = getOtherReads(readingCase, userId) + + // Amending a read keeps its place in the order; a new one takes the next + const readNumber = existingRead?.readNumber || otherReads.length + 1 + + // A case already in arbitration is being settled, whoever is reading it + const readType = isCaseInArbitration(readingCase) + ? 'arbitration' + : READ_TYPES[Math.min(readNumber, READ_TYPES.length) - 1] + + return { + ...reading, + readerId: userId, + readerType, + readType, + readNumber, + timestamp: options.timestamp || new Date().toISOString() + } +} + +/** + * Add or replace a user's read on a case, returning a new case record. + * + * Returns a replacement rather than mutating: cases live inside episodes, which + * are shared read-only data (see docs/data-conventions.md). + * + * @param {object} readingCase - The case being read + * @param {object} read - The read record, from buildRead + * @returns {object} A new case record with the read applied + */ +const withRead = (readingCase, read) => { + const reads = getReadsAsArray(readingCase) + const existingIndex = reads.findIndex( + (candidate) => candidate.readerId === read.readerId + ) + + const updatedReads = + existingIndex >= 0 + ? reads.map((candidate, index) => (index === existingIndex ? read : candidate)) + : [...reads, read] + + return { ...readingCase, reads: updatedReads } +} + +/** + * Remove a user's read from a case, returning a new case record. + * + * Deferring after giving an opinion withdraws that opinion - the reader is + * saying they can't judge this case after all. + * + * @param {object} readingCase - The case + * @param {string} userId - Whose read to remove + * @returns {object} A new case record without that read + */ +const withoutRead = (readingCase, userId) => { + return { + ...readingCase, + reads: getReadsAsArray(readingCase).filter( + (read) => read.readerId !== userId + ) + } +} + +module.exports = { + READ_TYPES, + READING_CASE_STATES, + READING_CASE_OUTCOMES, + buildReadingCase, + getReadingCases, + getLatestReadingCase, + getReadingCaseForAppointment, + getReadsAsArray, + getReadForUser, + getOtherReads, + getArbitrationRead, + userHasReadCase, + caseHasReads, + isCaseDeferred, + isCaseInArbitration, + areReadsDiscordant, + willGoToArbitration, + getComparisonInfo, + shouldShowComparePage, + getReadingCaseState, + getReadingCaseOutcome, + getReadingMetadata, + caseNeedsFirstRead, + caseNeedsSecondRead, + caseNeedsArbitration, + canUserReadCase, + buildRead, + withRead, + withoutRead +} diff --git a/app/lib/utils/reading.js b/app/lib/utils/reading.js index 16040df5..d555289c 100644 --- a/app/lib/utils/reading.js +++ b/app/lib/utils/reading.js @@ -1,125 +1,86 @@ // app/lib/utils/reading.js +// +// The appointment- and session-shaped layer over image reading: reading +// sessions, backlogs, clinic lists, progress and navigation, all of which work +// in terms of the appointments a reader is working through. +// +// The reading data itself lives on the episode, as `episode.readingCases[]`. +// Case-level logic is in reading-cases.js and takes a case; getting the case +// for an appointment goes through getReadingCase in episodes.js. That is why +// most helpers here take `data` - it's what resolving a case needs. const dayjs = require('dayjs') const { getClinic } = require('./clinics') const { eligibleForReading, getStatusTagColour } = require('./status') const { isWithinDayRange } = require('./dates') const { awaitingPriors, userRequestedPriors } = require('./prior-mammograms') -const { updateAppointmentData } = require('./appointment-data') - -// /** -// * Get first unread appointment in a clinic -// */ -// const getFirstUnreadAppointment = (data, clinicId) => { -// return data.appointments.find(appointment => -// appointment.clinicId === clinicId && -// eligibleForReading(appointment) && -// !appointment.reads?.length -// ) || null -// } - -// /** -// * Get first unread appointment from first available clinic -// */ -// const getFirstUnreadAppointmentOverall = (data) => { -// const firstClinic = getFirstAvailableClinic(data) -// if (!firstClinic) return null - -// return getFirstUnreadAppointment(data, firstClinic.id) -// } +const { + getReadingCase, + getEpisodeAppointments, + updateReadingCase +} = require('./episodes') +const { + getReadsAsArray, + getReadForUser, + getReadingMetadata, + getReadingCaseState, + isCaseDeferred, + caseHasReads, + caseNeedsFirstRead, + caseNeedsSecondRead, + canUserReadCase, + userHasReadCase, + buildRead, + withRead +} = require('./reading-cases') /************************************************************************ // Single appointment -//*********************************************************************** - -/** - * Get reading metadata for an appointment - * @param {Object} appointment - The appointment to check - * @returns {Object} Object with reading metadata - */ -const getReadingMetadata = (appointment) => { - // Get all reads from the imageReading structure - const reads = appointment.imageReading?.reads - ? Object.values(appointment.imageReading.reads) - : [] - const readerIds = reads.map((read) => read.readerId) - const uniqueReaderCount = new Set(readerIds).size - - // Get all unique opinions from reads - const opinions = reads.map((read) => read.opinion) - const uniqueOpinions = [...new Set(opinions)].filter(Boolean) // Filter out undefined - - // Discordance: richer than just comparing opinion strings — also checks TR views and RFA breast assessments - const isDiscordant = - reads.length >= 2 ? areReadsDiscordant(reads[0], reads[1]) : false - - return { - readCount: reads.length, - uniqueReaderCount, - firstReadComplete: reads.length >= 1, - secondReadComplete: reads.length >= 2, - isDiscordant, - opinions: uniqueOpinions - } -} +// +// Thin bridges from an appointment to its case, for the places that only have +// an appointment to hand. Anything doing real work with reads should take the +// case instead. +//***********************************************************************/ /** - * Get all reads for an appointment as an ordered array - * Sorted by readNumber if available, otherwise by timestamp - * @param {Object} appointment - The appointment to get reads for - * @returns {Array} Array of read objects sorted by read order + * Get the reading metadata for an appointment's case + * + * @param {object} data - Session data + * @param {object} appointment - The appointment + * @returns {object} Reading metadata, all zeros if there is no case yet */ -const getReadsAsArray = function (appointment) { - if (!appointment?.imageReading?.reads) { - return [] - } - - return Object.values(appointment.imageReading.reads).sort((a, b) => { - // Sort by readNumber if both have it - if (a.readNumber && b.readNumber) { - return a.readNumber - b.readNumber - } - // Fall back to timestamp - return new Date(a.timestamp) - new Date(b.timestamp) - }) +const getAppointmentReadingMetadata = (data, appointment) => { + return getReadingMetadata(resolveCase(data, appointment), data?.settings || {}) } /** - * Save a user's reading for an appointment, and remove the appointment from the reading - * session's skipped list if present + * Save a user's read of an appointment's images, and take the appointment off + * the reading session's skipped list if it was on it. * - * Builds a new imageReading object and saves it through updateAppointmentData - * rather than mutating the appointment - appointment records are shared read-only data. + * The read goes onto the episode's reading case, not the appointment - a read + * is a read of one set of images, and the case is what holds those. * - * @param {object} appointment - The appointment to update - * @param {string} userId - User ID - * @param {object} reading - Reading data to save * @param {object} data - Session data + * @param {object} appointment - The appointment whose images were read + * @param {string} userId - User ID + * @param {object} reading - The opinion and its details * @param {string | null} [sessionId] - Reading session ID (if in session context) + * @returns {object | null} The updated case, or null if there was none to write to */ -const writeReading = (appointment, userId, reading, data, sessionId = null) => { - // Work on a clone so the shared record is never touched in place - const imageReading = structuredClone(appointment.imageReading || {}) - if (!imageReading.reads) { - imageReading.reads = {} +const writeReading = (data, appointment, userId, reading, sessionId = null) => { + const readingCase = getReadingCase(data, appointment) + if (!readingCase) { + console.warn( + `writeReading: no reading case for appointment ${appointment?.id}` + ) + return null } - // Calculate readNumber based on existing reads - const existingReadCount = Object.keys(imageReading.reads).length - // If this user already has a read, keep their readNumber; otherwise assign next number - const existingRead = imageReading.reads[userId] - const readNumber = existingRead?.readNumber || existingReadCount + 1 - - // Add the reading with timestamp and readNumber - imageReading.reads[userId] = { - ...reading, - readerId: userId, // Ensure the reader ID is saved - readNumber, - timestamp: new Date().toISOString() - } + const readerType = data.users?.find((user) => user.id === userId)?.role + const read = buildRead(readingCase, userId, readerType, reading) + const updatedCase = withRead(readingCase, read) - // Saves to the appointment and mirrors into data.appointment if it matches - updateAppointmentData(data, appointment.id, { imageReading }) + updateReadingCase(data, appointment.episodeId, updatedCase) // Note the episode deliberately stays in `reading`. Two opinions and a // computed outcome is not a confirmed result, and there is no step in the @@ -137,6 +98,103 @@ const writeReading = (appointment, userId, reading, data, sessionId = null) => { session.skippedAppointments.splice(skippedIndex, 1) } } + + return updatedCase +} + +/** + * Get the reading status of an episode. + * + * Lives here rather than in episodes.js so the requires stay one-directional - + * episodes.js owns getting cases out of session data, and this is the layer + * above that summarises them. + * + * Scoped by the episode's appointments rather than its cases directly, because + * the summary mixes case facts (reads) with appointment ones (outstanding + * priors, how long ago the images were taken). + * + * @param {object} data - Session data + * @param {object} episode - Episode object + * @param {string} [userId] - Optional user, for per-user reading counts + * @returns {object} Reading status and metrics for the episode + */ +const getEpisodeReadingStatus = (data, episode, userId = null) => { + return getReadingStatusForAppointments( + data, + getEpisodeAppointments(data, episode), + userId + ) +} + +/** + * Every case currently deferred from reading, most recently deferred first. + * + * Deferral is a case-level fact, so this walks the reading backlog and pairs + * each deferred case with the appointment and participant it belongs to - what + * a list of deferred cases needs to show a row. + * + * @param {object} data - Session data + * @returns {Array} `{ appointment, participant, readingCase, deferral }`, newest first + */ +const getDeferredCases = (data) => { + return data.appointments + .map((appointment) => ({ + appointment, + participant: data.participants.find( + (participant) => participant.id === appointment.participantId + ), + readingCase: getReadingCase(data, appointment) + })) + .filter((row) => isCaseDeferred(row.readingCase)) + .map((row) => ({ ...row, deferral: row.readingCase.deferral })) + .sort( + (a, b) => new Date(b.deferral.deferredAt) - new Date(a.deferral.deferredAt) + ) +} + +/** + * Every deferral that has since been resolved, most recently resolved first. + * + * A case can have been deferred and returned to the queue more than once, so + * this is a list of deferrals rather than of cases. + * + * @param {object} data - Session data + * @returns {Array} `{ appointment, participant, readingCase, deferral }`, newest first + */ +const getResolvedDeferrals = (data) => { + return data.appointments + .flatMap((appointment) => { + const readingCase = getReadingCase(data, appointment) + + return (readingCase?.deferralHistory || []).map((deferral) => ({ + appointment, + participant: data.participants.find( + (participant) => participant.id === appointment.participantId + ), + readingCase, + deferral + })) + }) + .sort( + (a, b) => new Date(b.deferral.resolvedAt) - new Date(a.deferral.resolvedAt) + ) +} + +/** + * Get an appointment's reading case, preferring one already attached. + * + * List-building code enriches appointments with their case up front so a long + * list resolves each one once; anything working from a raw appointment record + * falls through to resolving it here. + * + * @param {object} data - Session data + * @param {object} appointment - The appointment + * @returns {object | null} The case, or null if the appointment produced no images + */ +const resolveCase = (data, appointment) => { + return appointment?.readingCase !== undefined + ? appointment.readingCase + : getReadingCase(data, appointment) } /************************************************************************ @@ -144,23 +202,35 @@ const writeReading = (appointment, userId, reading, data, sessionId = null) => { //*********************************************************************** /** - * Enhance appointments with pre-calculated reading metadata + * Enhance appointments with their reading case and pre-calculated metadata. + * + * Attaching the case here is what lets the rest of the list-handling code stay + * cheap - everything downstream reads `appointment.readingCase` instead of + * resolving the episode again per row. + * + * @param {object} data - Session data * @param {Array} appointments - Array of appointments to enhance * @param {Array} participants - Array of participants for lookups * @param {string} userId - Current user ID - * @returns {Array} Enhanced appointments with pre-calculated metadata + * @returns {Array} Enhanced appointments with their case and metadata */ -const enhanceAppointmentsWithReadingData = (appointments, participants, userId) => { +const enhanceAppointmentsWithReadingData = ( + data, + appointments, + participants, + userId +) => { // Create a lookup map for participants const participantMap = new Map(participants.map((p) => [p.id, p])) - // Enhanced appointments with pre-calculated metadata + // Enhanced appointments with their case and pre-calculated metadata return appointments.map((appointment) => { - // Calculate metadata once - const metadata = getReadingMetadata(appointment) + const readingCase = getReadingCase(data, appointment) + const enhanced = { ...appointment, readingCase } + const metadata = getReadingMetadata(readingCase, data?.settings || {}) return { - ...appointment, + ...enhanced, participant: participantMap.get(appointment.participantId), readStatus: metadata.readCount > 0 ? `Read (${metadata.readCount})` : 'Not read', @@ -168,7 +238,7 @@ const enhanceAppointmentsWithReadingData = (appointments, participants, userId) metadata.readCount > 0 ? 'read' : 'not_read' ), readingMetadata: metadata, - canUserRead: canUserReadAppointment(appointment, userId) + canUserRead: canUserReadAppointment(data, enhanced, userId) } }) } @@ -176,19 +246,21 @@ const enhanceAppointmentsWithReadingData = (appointments, participants, userId) /** * Calculate core reading metrics used for both status and progress tracking * + * @param {object} data - Session data * @param {Array} appointments - Array of appointments to analyze * @param {string | null} userId - User ID for user-specific metrics * @param {Array} [skippedAppointments] - Array of skipped appointment IDs * @returns {object} Core metrics object */ const calculateReadingMetrics = function ( + data, appointments, userId = null, skippedAppointments = [] ) { // Get user ID and settings from context if not provided and we're in a template context const currentUserId = userId || this?.ctx?.data?.currentUser?.id - const settings = this?.ctx?.data?.settings || {} + const settings = data?.settings || this?.ctx?.data?.settings || {} if (!appointments || appointments.length === 0) { return { @@ -214,25 +286,26 @@ const calculateReadingMetrics = function ( } } - // Count first reads (appointments with at least one read) - const firstReadCount = appointments.filter(hasReads).length + // Resolve each appointment's case once - every count below is about the case + const cases = appointments.map((appointment) => resolveCase(data, appointment)) + + // Count first reads (cases with at least one read) + const firstReadCount = cases.filter(caseHasReads).length const completedCount = firstReadCount // For compatibility with current usage - // Count second reads (appointments with at least two different readers) - const secondReadCount = appointments.filter((appointment) => { - const metadata = getReadingMetadata(appointment) - return metadata.uniqueReaderCount >= 2 - }).length + // Count second reads (cases with at least two different readers) + const secondReadCount = cases.filter( + (readingCase) => + getReadingMetadata(readingCase, settings).uniqueReaderCount >= 2 + ).length - // Count appointments that are ready for second read (have first read but not second) - const secondReadReady = appointments.filter((appointment) => { - const metadata = getReadingMetadata(appointment) - return metadata.readCount === 1 // Exactly one read means ready for second - }).length + // Count cases that are ready for second read (have first read but not second) + const secondReadReady = cases.filter(caseNeedsSecondRead).length - // Count appointments needing arbitration (policy-aware via getOutcome) - const arbitrationCount = appointments.filter( - (appointment) => getOutcome(appointment, settings) === 'arbitration_pending' + // Count cases needing arbitration (policy-aware via the case state) + const arbitrationCount = cases.filter( + (readingCase) => + getReadingCaseState(readingCase, settings) === 'arbitration_required' ).length // Global awaiting priors count (appointments with any outstanding prior request) @@ -250,54 +323,40 @@ const calculateReadingMetrics = function ( let userSecondReadableCount = 0 if (currentUserId) { - // Appointments this user has read - userReadCount = appointments.filter((appointment) => - userHasReadAppointment(appointment, currentUserId) + // Cases this user has read + userReadCount = cases.filter((readingCase) => + userHasReadCase(readingCase, currentUserId) ).length - // Count first/second reads by this user - appointments.forEach((appointment) => { - const metadata = getReadingMetadata(appointment) - const reads = appointment.imageReading?.reads - ? Object.values(appointment.imageReading.reads) - : [] - - // Find reads by this user - const userReads = reads.filter((read) => read.readerId === currentUserId) - - // Count based on read position (first or second) - if (userReads.length > 0) { - // Check if this user did the first read - if (reads[0]?.readerId === currentUserId) { - userFirstReadCount++ - } - - // Check if this user did the second read - if (reads.length > 1 && reads[1]?.readerId === currentUserId) { - userSecondReadCount++ - } - } - }) + // Count first/second reads by this user, by the read's own recorded type + // rather than its position - a withdrawn read must not shuffle the rest + cases.forEach((readingCase) => { + const userRead = getReadForUser(readingCase, currentUserId) + if (!userRead) return - // Appointments where this user has an outstanding priors request - userAwaitingPriorsCount = appointments.filter( - (appointment) => - awaitingPriors(appointment) && userRequestedPriors(appointment, currentUserId) - ).length + if (userRead.readType === 'first') userFirstReadCount++ + if (userRead.readType === 'second') userSecondReadCount++ + }) // Appointments this user can read userReadableCount = appointments.filter((appointment) => - canUserReadAppointment(appointment, currentUserId) + canUserReadAppointment(data, appointment, currentUserId) ).length // Appointments needing first read that this user can read - userFirstReadableCount = filterAppointmentsByNeedsFirstRead(appointments).filter( - (appointment) => canUserReadAppointment(appointment, currentUserId) + userFirstReadableCount = filterAppointmentsByNeedsFirstRead( + data, + appointments + ).filter((appointment) => + canUserReadAppointment(data, appointment, currentUserId) ).length // Appointments needing second read that this user can read - userSecondReadableCount = filterAppointmentsByNeedsSecondRead(appointments).filter( - (appointment) => canUserReadAppointment(appointment, currentUserId) + userSecondReadableCount = filterAppointmentsByNeedsSecondRead( + data, + appointments + ).filter((appointment) => + canUserReadAppointment(data, appointment, currentUserId) ).length // Appointments where this user has an outstanding prior request @@ -339,13 +398,14 @@ const calculateReadingMetrics = function ( /** * Get detailed reading status for a group of appointments * + * @param {object} data - Session data * @param {Array} appointments - Array of appointments to analyze * @param {string | null} [userId] - Optional user ID (defaults to current user if available) * @returns {object} Detailed reading status */ -const getReadingStatusForAppointments = function (appointments, userId = null) { +const getReadingStatusForAppointments = function (data, appointments, userId = null) { // Get metrics from base calculation function - const metrics = calculateReadingMetrics(appointments, userId) + const metrics = calculateReadingMetrics.call(this, data, appointments, userId) // If no appointments, return basic metrics with default status if (!appointments || appointments.length === 0) { @@ -386,6 +446,7 @@ const getReadingStatusForAppointments = function (appointments, userId = null) { * Get progress through reading a set of appointments * Enhanced to include user-specific navigation * + * @param {object} data - Session data * @param {Array} appointments - Array of appointments to track progress through * @param {string} currentAppointmentId - ID of current appointment * @param {Array} skippedAppointments - Array of appointment IDs that have been skipped @@ -393,13 +454,20 @@ const getReadingStatusForAppointments = function (appointments, userId = null) { * @returns {object} Progress information */ const getReadingProgress = function ( + data, appointments, currentAppointmentId, skippedAppointments = [], userId = null ) { // Get base metrics - const metrics = calculateReadingMetrics(appointments, userId, skippedAppointments) + const metrics = calculateReadingMetrics.call( + this, + data, + appointments, + userId, + skippedAppointments + ) // Get user ID from context if not provided and we're in a template context const currentUserId = userId || this?.ctx?.data?.currentUser?.id @@ -412,7 +480,7 @@ const getReadingProgress = function ( const previousAppointment = getPreviousAppointmentInList(appointments, currentAppointmentId, false) // Get appointments needing any reads (first or second) - const readableAppointments = filterAppointmentsByNeedsAnyRead(appointments) + const readableAppointments = filterAppointmentsByNeedsAnyRead(data, appointments) // Find next/previous of each type const nextReadableAppointment = @@ -428,6 +496,7 @@ const getReadingProgress = function ( let userNavigableAppointments = appointments if (currentUserId) { userNavigableAppointments = filterAppointmentsByUserCanReadOrHasRead( + data, appointments, currentUserId ) @@ -469,10 +538,10 @@ const getReadingProgress = function ( previousUserReadableId: previousUserReadableAppointment?.id || null, // Whether user has already read the previous/next appointment (for review page links) previousUserHasRead: previousUserReadableAppointment - ? userHasReadAppointment(previousUserReadableAppointment, currentUserId) + ? userHasReadAppointment(data, previousUserReadableAppointment, currentUserId) : false, nextUserHasRead: nextUserReadableAppointment - ? userHasReadAppointment(nextUserReadableAppointment, currentUserId) + ? userHasReadAppointment(data, nextUserReadableAppointment, currentUserId) : false, // Skipped appointments skippedAppointments, @@ -484,221 +553,6 @@ const getReadingProgress = function ( } } -// /** -// * Get progress through reading a set of appointments -// * Enhanced to include user-specific navigation -// * @param {Array} appointments - Array of appointments to track progress through -// * @param {string} currentAppointmentId - ID of current appointment -// * @param {Array} skippedAppointments - Array of appointment IDs that have been skipped -// * @param {string} [userId=null] - Optional user ID (defaults to current user if available) -// * @returns {Object} Progress information -// */ -// const getReadingProgress = function(appointments, currentAppointmentId, skippedAppointments = [], userId = null) { -// // Get user ID from context if not provided and we're in a template context -// const currentUserId = userId || (this?.ctx?.data?.currentUser?.id); - -// const currentIndex = appointments.findIndex(e => e.id === currentAppointmentId); - -// // Get complete appointments count -// const completedCount = appointments.filter(hasReads).length; - -// // Basic sequential navigation -// const nextAppointment = getNextAppointmentInList(appointments, currentAppointmentId, false); -// const previousAppointment = getPreviousAppointmentInList(appointments, currentAppointmentId, false); - -// // Get appointments needing any reads (first or second) -// const readableAppointments = filterAppointmentsByNeedsAnyRead(appointments); - -// // Find next/previous of each type -// const nextReadableAppointment = currentIndex !== -1 ? -// getNextAppointmentInList(readableAppointments, currentAppointmentId, true) : null; -// const previousReadableAppointment = currentIndex !== -1 ? -// getPreviousAppointmentInList(readableAppointments, currentAppointmentId, true) : null; - -// // For user-specific navigation, get appointments this user can read or has read -// let userNavigableAppointments = appointments; -// if (currentUserId) { -// userNavigableAppointments = filterAppointmentsByUserCanReadOrHasRead(appointments, currentUserId); -// } - -// // Find next/previous user-readable appointments if userId provided -// let nextUserReadableAppointment = null; -// let previousUserReadableAppointment = null; - -// if (currentUserId && currentIndex !== -1) { -// nextUserReadableAppointment = getNextAppointmentInList(userNavigableAppointments, currentAppointmentId, true); -// previousUserReadableAppointment = getPreviousAppointmentInList(userNavigableAppointments, currentAppointmentId, true); -// } - -// return { -// current: currentIndex + 1, -// total: appointments.length, -// completed: completedCount, -// // Appointment navigation -// hasNext: !!nextAppointment, -// hasPrevious: !!previousAppointment, -// nextAppointmentId: nextAppointment?.id || null, -// previousAppointmentId: previousAppointment?.id || null, -// hasNextReadableAppointment: !!nextReadableAppointment, -// hasPreviousReadableAppointment: !!previousReadableAppointment, -// nextReadableAppointmentId: nextReadableAppointment?.id || null, -// previousReadableAppointmentId: previousReadableAppointment?.id || null, -// // User-specific navigation -// hasNextUserReadable: !!nextUserReadableAppointment, -// hasPreviousUserReadable: !!previousUserReadableAppointment, -// nextUserReadableId: nextUserReadableAppointment?.id || null, -// previousUserReadableId: previousUserReadableAppointment?.id || null, -// // Skipped appointments -// skippedCount: skippedAppointments.length, -// skippedAppointments, -// isCurrentSkipped: skippedAppointments.includes(currentAppointmentId), -// nextAppointmentSkipped: nextAppointment ? skippedAppointments.includes(nextAppointment.id) : false, -// previousAppointmentSkipped: previousAppointment ? skippedAppointments.includes(previousAppointment.id) : false -// }; -// }; - -// /** -// * Get detailed reading status for a group of appointments -// * @param {Array} appointments - Array of appointments to analyze -// * @param {string} [userId=null] - Optional user ID (defaults to current user if available) -// * @returns {Object} Detailed reading status -// */ -// const getReadingStatusForAppointments = function(appointments, userId = null) { -// // Get user ID from context if not provided and we're in a template context -// const currentUserId = userId || (this?.ctx?.data?.currentUser?.id); - -// if (!appointments || appointments.length === 0) { -// return { -// total: 0, -// firstReadCount: 0, -// firstReadRemaining: 0, -// secondReadCount: 0, -// secondReadRemaining: 0, -// secondReadReady: 0, -// arbitrationCount: 0, -// status: 'no_appointments', -// statusColor: 'grey', -// // User-specific counts -// userReadCount: 0, -// userFirstReadCount: 0, -// userSecondReadCount: 0, -// userReadableCount: 0, -// userFirstReadableCount: 0, -// userSecondReadableCount: 0 -// }; -// } - -// // Count first reads (appointments with at least one read) -// const firstReadCount = appointments.filter(hasReads).length; - -// // Count second reads (appointments with at least two different readers) -// const secondReadCount = appointments.filter(appointment => { -// const metadata = getReadingMetadata(appointment); -// return metadata.uniqueReaderCount >= 2; -// }).length; - -// // Count appointments that are ready for second read (have first read but not second) -// const secondReadReady = appointments.filter(appointment => { -// const metadata = getReadingMetadata(appointment); -// return metadata.readCount === 1; // Exactly one read means ready for second -// }).length; - -// // Count appointments needing arbitration (still track this for informational purposes) -// const arbitrationCount = appointments.filter(appointment => { -// const metadata = getReadingMetadata(appointment); -// return metadata.needsArbitration; -// }).length; - -// // User-specific counts if userId provided -// let userReadCount = 0; -// let userFirstReadCount = 0; -// let userSecondReadCount = 0; -// let userReadableCount = 0; -// let userFirstReadableCount = 0; -// let userSecondReadableCount = 0; - -// if (currentUserId) { -// // Appointments this user has read -// userReadCount = appointments.filter(appointment => userHasReadAppointment(appointment, currentUserId)).length; - -// // Count first/second reads by this user -// appointments.forEach(appointment => { -// const metadata = getReadingMetadata(appointment); -// const reads = appointment.imageReading?.reads ? Object.values(appointment.imageReading.reads) : []; - -// // Find reads by this user -// const userReads = reads.filter(read => read.readerId === currentUserId); - -// // Count based on read position (first or second) -// if (userReads.length > 0) { -// // Check if this user did the first read -// if (reads[0]?.readerId === currentUserId) { -// userFirstReadCount++; -// } - -// // Check if this user did the second read -// if (reads.length > 1 && reads[1]?.readerId === currentUserId) { -// userSecondReadCount++; -// } -// } -// }); - -// // Appointments this user can read -// userReadableCount = appointments.filter(appointment => canUserReadAppointment(appointment, currentUserId)).length; - -// // Appointments needing first read that this user can read -// userFirstReadableCount = filterAppointmentsByNeedsFirstRead(appointments) -// .filter(appointment => canUserReadAppointment(appointment, currentUserId)).length; - -// // Appointments needing second read that this user can read -// userSecondReadableCount = filterAppointmentsByNeedsSecondRead(appointments) -// .filter(appointment => canUserReadAppointment(appointment, currentUserId)).length; -// } - -// // Determine detailed status based on read counts -// let status; - -// if (firstReadCount === 0) { -// status = 'not_started'; -// } else if (firstReadCount < appointments.length) { -// if (secondReadCount > 0) { -// status = 'mixed_reads'; -// } else { -// status = 'partial_first_read'; -// } -// } else if (secondReadCount === 0) { -// status = 'first_read_complete'; -// } else if (secondReadCount < appointments.length) { -// status = 'partial_second_read'; -// } else { -// status = 'complete'; -// } - -// return { -// total: appointments.length, -// firstReadCount, -// firstReadRemaining: appointments.length - firstReadCount, -// secondReadCount, -// secondReadRemaining: appointments.length - secondReadCount, -// secondReadReady, // Appointments ready for immediate second read -// arbitrationCount, -// status, -// statusColor: getStatusTagColour(status), -// daysSinceScreening: appointments[0] ? -// dayjs().startOf('day').diff(dayjs(appointments[0].timing.startTime).startOf('day'), 'days') : 0, -// // User-specific counts -// userReadCount, -// userFirstReadCount, -// userSecondReadCount, -// userReadableCount, -// userFirstReadableCount, -// userSecondReadableCount, -// userCanRead: userReadableCount > 0 -// }; -// }; - -// Add this to app/lib/utils/reading.js - /** * Sort appointments by screening date (oldest first) * @@ -755,7 +609,11 @@ const getReadingClinics = (data, options = {}) => { ...clinic, unit, location, - readingStatus: getReadingStatusForAppointments(appointments, data.currentUser.id) + readingStatus: getReadingStatusForAppointments( + data, + appointments, + data.currentUser.id + ) } }) .sort((a, b) => new Date(a.id) - new Date(b.id)) // Some clinics share the same date so sort first by a unique ID to keep consistent sort @@ -777,6 +635,7 @@ const getReadableAppointmentsForClinic = (data, clinicId) => { // Enhance the appointments with reading metadata const enhancedAppointments = enhanceAppointmentsWithReadingData( + data, eligibleAppointments, data.participants, data.currentUser?.id @@ -805,89 +664,102 @@ const filterAppointmentsByEligibleForReading = (appointments) => { /** * Filter appointments that need any read (first or second) * + * @param {object} data - Session data * @param {Array} appointments - Appointments to filter - * @param {number} maxReadsPerAppointment - Number of reads required to be complete (default: 2) + * @param {number} maxReadsPerCase - Number of reads required to be complete (default: 2) * @returns {Array} Appointments needing any read */ -const filterAppointmentsByNeedsAnyRead = (appointments, maxReadsPerAppointment = 2) => { - return appointments.filter((appointment) => { - const metadata = getReadingMetadata(appointment) - return metadata.uniqueReaderCount < maxReadsPerAppointment - }) +const filterAppointmentsByNeedsAnyRead = (data, appointments, maxReadsPerCase = 2) => { + return appointments.filter( + (appointment) => + getReadsAsArray(resolveCase(data, appointment)).length < maxReadsPerCase + ) } /** * Filter appointments that need a first read * + * @param {object} data - Session data * @param {Array} appointments - Appointments to filter * @returns {Array} Appointments needing first read */ -const filterAppointmentsByNeedsFirstRead = (appointments) => { - return appointments.filter((appointment) => needsFirstRead(appointment)) +const filterAppointmentsByNeedsFirstRead = (data, appointments) => { + return appointments.filter((appointment) => + caseNeedsFirstRead(resolveCase(data, appointment)) + ) } /** * Filter appointments that need a second read * + * @param {object} data - Session data * @param {Array} appointments - Appointments to filter * @returns {Array} Appointments needing second read */ -const filterAppointmentsByNeedsSecondRead = (appointments) => { - return appointments.filter((appointment) => needsSecondRead(appointment)) +const filterAppointmentsByNeedsSecondRead = (data, appointments) => { + return appointments.filter((appointment) => + caseNeedsSecondRead(resolveCase(data, appointment)) + ) } /** * Filter appointments that are fully read (have all required reads) * + * @param {object} data - Session data * @param {Array} appointments - Appointments to filter * @param {number} requiredReads - Number of required reads (default: 2) * @returns {Array} Fully read appointments */ -const filterAppointmentsByFullyRead = (appointments, requiredReads = 2) => { - return appointments.filter((appointment) => { - const metadata = getReadingMetadata(appointment) - return metadata.uniqueReaderCount >= requiredReads - }) +const filterAppointmentsByFullyRead = (data, appointments, requiredReads = 2) => { + return appointments.filter( + (appointment) => + getReadsAsArray(resolveCase(data, appointment)).length >= requiredReads + ) } /** * Filter appointments that a specific user can read * + * @param {object} data - Session data * @param {Array} appointments - Appointments to filter * @param {string} userId - User ID * @returns {Array} Appointments user can read */ -const filterAppointmentsByUserCanRead = (appointments, userId) => { - return appointments.filter((appointment) => canUserReadAppointment(appointment, userId)) +const filterAppointmentsByUserCanRead = (data, appointments, userId) => { + return appointments.filter((appointment) => + canUserReadAppointment(data, appointment, userId) + ) } /** * Filter appointments that user can read or has already read * + * @param {object} data - Session data * @param {Array} appointments - Array of appointments to filter * @param {string} userId - User ID to check * @param {object} [options] - Options for determining eligibility * @returns {Array} Appointments user can read or has read * - * Priarily to support navigating backwards through appointments + * Primarily to support navigating backwards through appointments */ -const filterAppointmentsByUserCanReadOrHasRead = (appointments, userId, options = {}) => { - const { maxReadsPerAppointment = 2 } = options +const filterAppointmentsByUserCanReadOrHasRead = ( + data, + appointments, + userId, + options = {} +) => { + const { maxReadsPerCase = 2 } = options return appointments.filter((appointment) => { - const metadata = getReadingMetadata(appointment) + const readingCase = resolveCase(data, appointment) - // Include if user has already read this appointment - if (userHasReadAppointment(appointment, userId)) { - return true - } + // Include if the user has already read this case + if (userHasReadCase(readingCase, userId)) return true - // Include if appointment isn't fully read and user can read it - if (metadata.uniqueReaderCount < maxReadsPerAppointment) { - return true - } + // Include if the case isn't fully read yet, so they could still read it + if (getReadsAsArray(readingCase).length < maxReadsPerCase) return true - // Exclude appointments that are fully read by other users + // Exclude cases that are fully read by other users return false }) } @@ -978,47 +850,37 @@ const getPreviousAppointmentInList = (appointments, currentAppointmentId, wrap = / User functions /***********************************************************************/ -/** - * Get the read object for a specific user on an appointment - * - * @param {object} appointment - The appointment to check - * @param {string | null} [userId] - User ID (falls back to current user from context) - * @returns {object | null} The read object, or null if not found - */ -const getReadForUser = function (appointment, userId = null) { - const currentUserId = userId || this?.ctx?.data?.currentUser?.id - - if (!currentUserId) { - return null - } - - return appointment.imageReading?.reads?.[currentUserId] || null -} - /** * Get first appointment from an array that a user can read * + * @param {object} data - Session data * @param {Array} appointments - Array of appointments to search * @param {string | null} userId - User ID to check for * @returns {object | null} First appointment user can read or null if none */ -const getFirstUserReadableAppointment = function (appointments, userId = null) { +const getFirstUserReadableAppointment = function (data, appointments, userId = null) { // Get user ID from context if not provided and we're in a template context const currentUserId = userId || this?.ctx?.data?.currentUser?.id - const readableAppointments = filterAppointmentsByUserCanRead(appointments, currentUserId) + const readableAppointments = filterAppointmentsByUserCanRead( + data, + appointments, + currentUserId + ) return readableAppointments.length > 0 ? readableAppointments[0] : null } /** * Get the next appointment the user can read after the current appointment, wrapping to start if needed * + * @param {object} data - Session data * @param {Array} appointments - Array of all appointments * @param {string} currentAppointmentId - ID of the current appointment * @param {string | null} [userId] - User ID (falls back to current user from context) * @returns {object | null} Next readable appointment, or null if none */ const getNextUserReadableAppointment = function ( + data, appointments, currentAppointmentId, userId = null, @@ -1030,7 +892,7 @@ const getNextUserReadableAppointment = function ( const appointmentsFromNext = wrap ? [...appointments.slice(currentIndex + 1), ...appointments.slice(0, currentIndex)] : appointments.slice(currentIndex + 1) - return getFirstUserReadableAppointment(appointmentsFromNext, currentUserId) + return getFirstUserReadableAppointment(data, appointmentsFromNext, currentUserId) } /** @@ -1045,12 +907,14 @@ const getNextUserReadableAppointment = function ( * * Falls back to getFirstUserReadableAppointment if the user has no reads or skips yet. * + * @param {object} data - Session data * @param {Array} appointments - Array of all appointments in the session, in session order * @param {string | null} [userId] - User ID (falls back to current user from context) * @param {Array} [skippedAppointments] - Array of skipped appointment IDs from the session * @returns {object | null} The appointment to resume from, or null if nothing to read */ const getResumeAppointmentForUser = function ( + data, appointments, userId = null, skippedAppointments = [] @@ -1061,7 +925,10 @@ const getResumeAppointmentForUser = function ( let lastActedIndex = -1 appointments.forEach((appointment, index) => { - const wasReadByUser = !!appointment.imageReading?.reads?.[currentUserId] + const wasReadByUser = userHasReadCase( + resolveCase(data, appointment), + currentUserId + ) const wasSkipped = skippedAppointments.includes(appointment.id) if (wasReadByUser || wasSkipped) { lastActedIndex = index @@ -1070,7 +937,7 @@ const getResumeAppointmentForUser = function ( // Nothing acted on yet — fall back to first readable if (lastActedIndex === -1) { - return getFirstUserReadableAppointment(appointments, currentUserId) + return getFirstUserReadableAppointment(data, appointments, currentUserId) } // Search for the first readable appointment after lastActedIndex, wrapping around @@ -1078,20 +945,26 @@ const getResumeAppointmentForUser = function ( ...appointments.slice(lastActedIndex + 1), ...appointments.slice(0, lastActedIndex + 1) ] - return getFirstUserReadableAppointment(appointmentsFromNext, currentUserId) + return getFirstUserReadableAppointment(data, appointmentsFromNext, currentUserId) } /************************************************************************ // Booleans -//*********************************************************************** +// +// Only the predicates that need more than the case: reading is blocked by +// outstanding priors, which live on the appointment. Everything else about a +// read is in reading-cases.js and takes a case. +//***********************************************************************/ /** - * Check if a user has already read an appointment - * @param {Object} appointment - The appointment to check - * @param {string} userId - User ID to check + * Check if a user has already read an appointment's images + * + * @param {object} data - Session data + * @param {object} appointment - The appointment to check + * @param {string} [userId] - User ID (falls back to current user from context) * @returns {boolean} Whether the user has read this appointment */ -const userHasReadAppointment = function (appointment, userId) { +const userHasReadAppointment = function (data, appointment, userId = null) { const currentUserId = userId || this?.ctx?.data?.currentUser?.id if (!currentUserId) { @@ -1101,290 +974,28 @@ const userHasReadAppointment = function (appointment, userId) { return false } - return !!getReadForUser(appointment, currentUserId) -} - -/** - * Get reads from other users (not the current user) - * @param {Object} appointment - The appointment to check - * @param {string} userId - Current user ID to exclude - * @returns {Array} Array of read objects from other users - */ -const getOtherReads = function (appointment, userId = null) { - const currentUserId = userId || this?.ctx?.data?.currentUser?.id - - if (!appointment?.imageReading?.reads) { - return [] - } - - return Object.entries(appointment.imageReading.reads) - .filter(([readerId]) => readerId !== currentUserId) - .map(([readerId, read]) => ({ - ...read, - readerId - })) -} - -/** - * Determine if two reads are discordant (disagree in a clinically meaningful way). - * - * Rules: - * - Different top-level opinions → always discordant - * - Both technical recall: discordant if the set of selected views differs - * (reasons are ignored — same views = concordant even with different reasons) - * - Both recall for assessment: discordant if either per-breast assessment differs - * (annotations and comments are ignored) - * - Both normal → concordant - * - * Handles partial data gracefully: if TR views or RFA breast assessments are not - * yet filled in, falls back to comparing only what's available. - * - * @param {object} readA - First read (saved read or imageReadingTemp) - * @param {object} readB - Second read (saved read or imageReadingTemp) - * @returns {boolean} Whether the reads are discordant - */ -const areReadsDiscordant = (readA, readB) => { - if (!readA?.opinion || !readB?.opinion) return false - - // Different top-level opinions → always discordant - if (readA.opinion !== readB.opinion) return true - - const opinion = readA.opinion - - // Both TR: compare the set of selected view keys - if (opinion === 'technical_recall') { - const viewsA = readA.technicalRecall?.views - const viewsB = readB.technicalRecall?.views - // If either side has no view data yet, can't compare further - if (!viewsA || !viewsB) return false - const keysA = new Set(Object.keys(viewsA)) - const keysB = new Set(Object.keys(viewsB)) - if (keysA.size !== keysB.size) return true - for (const view of keysA) { - if (!keysB.has(view)) return true - } - return false - } - - // Both RFA: compare per-breast assessments - if (opinion === 'recall_for_assessment') { - const leftA = readA.left?.breastAssessment - const leftB = readB.left?.breastAssessment - const rightA = readA.right?.breastAssessment - const rightB = readB.right?.breastAssessment - // If no breast data on either side yet, can't compare further - if (!leftA && !leftB && !rightA && !rightB) return false - if ((leftA || leftB) && leftA !== leftB) return true - if ((rightA || rightB) && rightA !== rightB) return true - return false - } - - return false -} - -/** - * Determine whether two reads will result in arbitration, taking the site's - * arbitration policy into account. - * - * Policies (from settings.reading.arbitrationPolicy): - * - 'discordant_only' (default): only discordant reads go to arbitration - * - 'all_non_normal': any concordant non-normal outcome also goes to arbitration - * - * @param {object} readA - First read - * @param {object} readB - Second read - * @param {object} [settings] - Site settings object (data.settings) - * @returns {boolean} - */ -const willGoToArbitration = (readA, readB, settings = {}) => { - if (!readA || !readB) return false - - // Discordant reads always go to arbitration - if (areReadsDiscordant(readA, readB)) return true - - // Concordant but non-normal: depends on policy - const policy = settings?.reading?.arbitrationPolicy || 'discordant_only' - if (policy === 'all_non_normal') { - return readA.opinion !== 'normal' - } - - return false + return userHasReadCase(resolveCase(data, appointment), currentUserId) } /** - * Compute the overall outcome for an appointment based on its reads and site policy. - * - * Outcomes: - * - 'not_read' — no reads yet - * - 'pending_second_read' — one read, awaiting second - * - 'arbitration_pending' — two reads that are discordant (or policy requires arbitration) - * - 'normal' / 'technical_recall' / 'recall_for_assessment' - * — concordant outcome (or resolved by an arbitration read) - * - * Note: outcome is computed on demand, not persisted. If you need to filter or - * report by outcome at scale, consider writing it to appointment.imageReading.outcome at - * save-opinion time. - * - * @param {object} appointment - The appointment - * @param {object} [settings] - Site settings object (data.settings) - * @returns {string} Outcome key - */ -const getOutcome = function (appointment, settings = null) { - const resolvedSettings = settings || this?.ctx?.data?.settings || {} - const reads = getReadsAsArray(appointment) - - if (reads.length === 0) return 'not_read' - if (reads.length === 1) return 'pending_second_read' - - // Third read = arbitration read; its opinion resolves the case - if (reads.length >= 3) { - return reads[2].opinion - } - - const [firstRead, secondRead] = reads - - if (willGoToArbitration(firstRead, secondRead, resolvedSettings)) { - return 'arbitration_pending' - } - - // Concordant reads — outcome is the shared opinion - return firstRead.opinion -} - -/** - * Determine if a comparison page should be shown to the second reader. - * Returns false if user is first reader, or if both opinions are normal. - * Otherwise returns comparison info including discordance and arbitration flags. - * - * @param {object} appointment - The appointment being read - * @param {object} secondReadData - The second reader's data (imageReadingTemp or a read object) - * @param {string} [userId] - Current user ID (optional, falls back to context) - * @param {object} [settings] - Site settings (optional, falls back to context) - * @returns {false | object} False if no comparison needed, else comparison info - */ -const getComparisonInfo = function ( - appointment, - secondReadData, - userId = null, - settings = null -) { - const currentUserId = userId || this?.ctx?.data?.currentUser?.id - const resolvedSettings = settings || this?.ctx?.data?.settings || {} - - // Support passing just an opinion string for backwards compatibility - const secondRead = - typeof secondReadData === 'string' - ? { opinion: secondReadData } - : secondReadData - - // Get the first read (from other users) - const otherReads = getOtherReads.call(this, appointment, currentUserId) - - // No first read exists - user is first reader - if (otherReads.length === 0) { - return false - } - - // Get the first reader's opinion (sorted by readNumber) - const firstRead = otherReads.sort((a, b) => { - if (a.readNumber && b.readNumber) return a.readNumber - b.readNumber - return new Date(a.timestamp) - new Date(b.timestamp) - })[0] - - const firstOpinion = firstRead.opinion - const secondOpinion = secondRead.opinion - - // Both normal - no comparison needed - if (firstOpinion === 'normal' && secondOpinion === 'normal') { - return false - } - - const discordant = areReadsDiscordant(firstRead, secondRead) - const type = discordant ? 'discordant' : 'agreeing' - - return { - type, - discordant, - goesToArbitration: willGoToArbitration( - firstRead, - secondRead, - resolvedSettings - ), - firstRead, - firstOpinion, - secondOpinion - } -} - -/** - * Decide whether the compare page should be shown to the second reader. - * - * Combines the timing setting (secondReaderComparison) and the new show-when - * setting (compareWhen) to give a single boolean answer. + * Check if a user can read an appointment's images. * - * compareWhen values (settings.reading.compareWhen): - * - 'non_normal' (default): show whenever either opinion is non-normal - * (i.e. whenever getComparisonInfo returns a result — current behaviour) - * - 'discordant_only': only show when the two reads are discordant + * Combines what the case knows (deferral, who has read it, how many reads it + * has) with the one blocker that lives on the appointment: a prior mammogram + * that has been requested but not yet arrived. * - * @param {object} appointment - The appointment being read - * @param {object} secondReadData - The second reader's data (imageReadingTemp or read object) - * @param {string} [userId] - Current user ID (optional, falls back to context) - * @param {object} [settings] - Site settings (optional, falls back to context) - * @returns {boolean} + * @param {object} data - Session data + * @param {object} appointment - The appointment to check + * @param {string | null} [userId] - User ID (falls back to current user from context) + * @param {object} [options] - Options for determining eligibility + * @returns {boolean} Whether the user can read this appointment */ -const shouldShowComparePage = function ( +const canUserReadAppointment = function ( + data, appointment, - secondReadData, userId = null, - settings = null + options = {} ) { - const resolvedSettings = settings || this?.ctx?.data?.settings || {} - const currentUserId = userId || this?.ctx?.data?.currentUser?.id - - const comparisonInfo = getComparisonInfo.call( - this, - appointment, - secondReadData, - currentUserId, - resolvedSettings - ) - - // getComparisonInfo returns false when no comparison needed - // (user is first reader, or both opinions are normal) - if (!comparisonInfo) return false - - const compareWhen = resolvedSettings?.reading?.compareWhen || 'non_normal' - - // 'non_normal': show for any non-normal combination — current behaviour - if (compareWhen === 'non_normal') return true - - // 'discordant_only': only show when reads disagree in a clinically meaningful way - if (compareWhen === 'discordant_only') return comparisonInfo.discordant - - return true -} - -/** - * Check if current user can read an appointment - * - * @param {object} appointment - The appointment to check - * @param {string | null} userId - Current user ID - * @param {object} [options] - Options for determining eligibility - * @returns {boolean} Whether the current user can read this appointment - */ -/** - * Check if an appointment has been deferred from reading - * - * @param {object} appointment - The appointment to check - * @returns {boolean} Whether the appointment has been deferred - */ -const isDeferred = (appointment) => { - return !!appointment?.imageReading?.deferral?.deferredAt -} - -const canUserReadAppointment = function (appointment, userId = null, options = {}) { - const { maxReadsPerAppointment = 2 } = options - const currentUserId = userId || this?.ctx?.data?.currentUser?.id if (!currentUserId) { @@ -1394,70 +1005,12 @@ const canUserReadAppointment = function (appointment, userId = null, options = { return false } - // Can't read if appointment is awaiting priors - if (awaitingPriors(appointment)) { - return false - } - - // Can't read if appointment has been deferred - if (isDeferred(appointment)) { - return false - } - - const metadata = getReadingMetadata(appointment) + // Can't read while a requested prior is still outstanding + if (awaitingPriors(appointment)) return false - // If we already have enough unique readers, no more reads needed - if (metadata.uniqueReaderCount >= maxReadsPerAppointment) { - return false - } - - // User can't read if they've already read it - if (userHasReadAppointment(appointment, currentUserId)) { - return false - } - - return true + return canUserReadCase(resolveCase(data, appointment), currentUserId, options) } -/** - * Check if an appointment has any reads - * - * @param {object} appointment - The appointment to check - * @returns {boolean} Whether the appointment has any reads - */ -const hasReads = (appointment) => { - return ( - appointment.imageReading?.reads && - Object.keys(appointment.imageReading.reads).length > 0 - ) -} - -/** - * Check if an appointment needs a first read - * - * @param {object} appointment - The appointment to check - * @returns {boolean} Whether a first read is needed - */ -const needsFirstRead = (appointment) => { - return !hasReads(appointment) -} - -/** - * Check if an appointment needs a second read - */ -const needsSecondRead = (appointment) => { - const metadata = getReadingMetadata(appointment) - return metadata.firstReadComplete && !metadata.secondReadComplete -} - -/** - * Check if an appointment needs arbitration. - * Policy-aware: reads arbitrationPolicy from Nunjucks context if available. - */ -const needsArbitration = function (appointment) { - const settings = this?.ctx?.data?.settings || {} - return getOutcome(appointment, settings) === 'arbitration_pending' -} /************************************************************************ // Sessions @@ -1511,7 +1064,7 @@ const getEligibleCandidatesForSession = (data, sessionOptions) => { } else { // 1. Filter to appointments the user can read (unless overridden) if (filters.userCanRead !== false) { - appointments = filterAppointmentsByUserCanRead(appointments, currentUserId) + appointments = filterAppointmentsByUserCanRead(data, appointments, currentUserId) } // 2. Apply awaiting priors filter @@ -1537,14 +1090,14 @@ const getEligibleCandidatesForSession = (data, sessionOptions) => { // Apply read type filters switch (type) { case 'first_reads': - appointments = filterAppointmentsByNeedsFirstRead(appointments) + appointments = filterAppointmentsByNeedsFirstRead(data, appointments) break case 'second_reads': - appointments = filterAppointmentsByNeedsSecondRead(appointments) + appointments = filterAppointmentsByNeedsSecondRead(data, appointments) break case 'all_reads': case 'awaiting_priors': - appointments = filterAppointmentsByNeedsAnyRead(appointments) + appointments = filterAppointmentsByNeedsAnyRead(data, appointments) break } @@ -1745,7 +1298,9 @@ const getFirstReadableAppointmentInSession = (data, sessionId, userId = null) => // Find the first one the user can read return ( - sessionAppointments.find((appointment) => canUserReadAppointment(appointment, currentUserId)) || + sessionAppointments.find((appointment) => + canUserReadAppointment(data, appointment, currentUserId) + ) || null ) } @@ -1797,9 +1352,9 @@ const topUpSession = (data, sessionId) => { const appointment = data.appointments.find((e) => e.id === appointmentId) if (!appointment) return false return ( - userHasReadAppointment(appointment, currentUserId) || - canUserReadAppointment(appointment, currentUserId) || - isDeferred(appointment) || + userHasReadAppointment(data, appointment, currentUserId) || + canUserReadAppointment(data, appointment, currentUserId) || + isCaseDeferred(getReadingCase(data, appointment)) || awaitingPriors(appointment) ) }).length @@ -1848,6 +1403,7 @@ const getSessionReadingProgress = ( // Use existing function for progress tracking, then add session-level size info const progress = getReadingProgress( + data, sessionAppointments, currentAppointmentId, session.skippedAppointments, @@ -1873,9 +1429,9 @@ const getSessionReadingProgress = ( // toward reachable size. topUpSession will replace them when appointments are read. const deadCount = sessionAppointments.filter((appointment) => { return ( - !userHasReadAppointment(appointment, resolvedUserId) && - !canUserReadAppointment(appointment, resolvedUserId) && - !isDeferred(appointment) && + !userHasReadAppointment(data, appointment, resolvedUserId) && + !canUserReadAppointment(data, appointment, resolvedUserId) && + !isCaseDeferred(getReadingCase(data, appointment)) && !awaitingPriors(appointment) ) }).length @@ -1885,7 +1441,9 @@ const getSessionReadingProgress = ( const effectiveTargetSize = Math.min(resolvedTargetSize, reachableSessionSize) // Count deferred appointments so they count toward the session target - const deferredCount = sessionAppointments.filter(isDeferred).length + const deferredCount = sessionAppointments.filter((appointment) => + isCaseDeferred(getReadingCase(data, appointment)) + ).length return { ...progress, @@ -1907,15 +1465,12 @@ const getSessionReadingProgress = ( } module.exports = { - // getFirstUnreadAppointment, - // getFirstUnreadAppointmentOverall, - // Single appointment - getReadingMetadata, - areReadsDiscordant, - willGoToArbitration, - getOutcome, + getAppointmentReadingMetadata, writeReading, + getEpisodeReadingStatus, + getDeferredCases, + getResolvedDeferrals, // Multiple appointments enhanceAppointmentsWithReadingData, @@ -1943,22 +1498,12 @@ module.exports = { getNextAppointmentInList, getPreviousAppointmentInList, // User functions - getReadForUser, - getOtherReads, - getComparisonInfo, - shouldShowComparePage, - getReadsAsArray, getFirstUserReadableAppointment, getNextUserReadableAppointment, getResumeAppointmentForUser, // Booleans userHasReadAppointment, canUserReadAppointment, - isDeferred, - hasReads, - needsArbitration, - needsFirstRead, - needsSecondRead, // Sessions getEligibleCandidatesForSession, diff --git a/app/routes/participants.js b/app/routes/participants.js index 78fda502..cc11cb9b 100644 --- a/app/routes/participants.js +++ b/app/routes/participants.js @@ -10,9 +10,9 @@ const { getEpisodesForParticipant, getCurrentEpisode, getEpisodeAppointments, - getEpisodeMammogramDate, - getEpisodeReadingStatus + getEpisodeMammogramDate } = require('../lib/utils/episodes') +const { getEpisodeReadingStatus } = require('../lib/utils/reading') const { getClinic } = require('../lib/utils/clinics') const { findById } = require('../lib/utils/arrays') const { createDynamicTemplateRoute } = require('../lib/utils/dynamic-routing') diff --git a/app/routes/reading.js b/app/routes/reading.js index 3628e6c7..6ca32d64 100644 --- a/app/routes/reading.js +++ b/app/routes/reading.js @@ -13,7 +13,6 @@ const { getReadingStatusForAppointments, getReadingClinics, getReadingProgress, - hasReads, canUserReadAppointment, userHasReadAppointment, writeReading, @@ -25,13 +24,21 @@ const { getSessionReadingProgress, skipAppointmentInSession, topUpSession, - getReadingMetadata, - getComparisonInfo, - shouldShowComparePage, + getAppointmentReadingMetadata, filterAppointmentsByEligibleForReading, filterAppointmentsByNeedsAnyRead, filterAppointmentsByUserCanRead } = require('../lib/utils/reading') +const { getReadingCase, updateReadingCase } = require('../lib/utils/episodes') +const { + getComparisonInfo, + shouldShowComparePage, + getReadingMetadata, + getReadsAsArray, + getReadForUser, + isCaseDeferred, + withoutRead +} = require('../lib/utils/reading-cases') const { getParticipant, getShortName } = require('../lib/utils/participants') const { PRIOR_REQUEST_STATUSES, @@ -377,6 +384,7 @@ module.exports = (router) => { .filter(Boolean) const resumeAppointment = getResumeAppointmentForUser( + data, sessionAppointments, data.currentUser.id, session.skippedAppointments || [] @@ -390,6 +398,7 @@ module.exports = (router) => { // Check if there are any readable cases left in the session const firstReadable = getFirstUserReadableAppointment( + data, sessionAppointments, data.currentUser.id ) @@ -453,21 +462,23 @@ module.exports = (router) => { ) .filter(Boolean) .map((appointment) => { - // Add participant data and reading metadata + // Add participant data, the reading case, and its metadata const participant = data.participants.find( (p) => p.id === appointment.participantId ) - const metadata = getReadingMetadata(appointment) + const readingCase = getReadingCase(data, appointment) return { ...appointment, participant, - readingMetadata: metadata + readingCase, + readingMetadata: getReadingMetadata(readingCase, data.settings) } }) // Get reading status for the session const readingStatus = getReadingStatusForAppointments( + data, enhancedAppointments, data.currentUser.id ) @@ -482,6 +493,7 @@ module.exports = (router) => { // Find where the user should resume — first readable after the furthest // point they've reached (reads or skips), falling back to first readable const resumeAppointment = getResumeAppointmentForUser( + data, enhancedAppointments, data.currentUser.id, session.skippedAppointments || [] @@ -491,7 +503,8 @@ module.exports = (router) => { const firstUserReadTimestamp = enhancedAppointments .map( (appointment) => - appointment.imageReading?.reads?.[data.currentUser.id]?.timestamp + getReadForUser(appointment.readingCase, data.currentUser.id) + ?.timestamp ) .filter(Boolean) .sort((a, b) => new Date(a) - new Date(b))[0] @@ -515,6 +528,7 @@ module.exports = (router) => { // Checks only cases the current user can actually read (not already read by them, // not fully read by others, not deferred or awaiting priors). const backlogTotal = filterAppointmentsByUserCanRead( + data, filterAppointmentsByEligibleForReading(data.appointments), data.currentUser.id ).length @@ -583,7 +597,10 @@ module.exports = (router) => { !data.imageReadingTemp || data.imageReadingTemp.appointmentId !== appointmentId ) { - const existingRead = appointment.imageReading?.reads?.[currentUserId] + const existingRead = getReadForUser( + getReadingCase(data, appointment), + currentUserId + ) if (existingRead) { // User has already read this appointment - populate temp from saved read console.log( @@ -617,8 +634,11 @@ module.exports = (router) => { } } - // Set up locals for templates + // Set up locals for templates. The case is resolved once here so the + // workflow templates can work in reading-case terms without each of them + // walking back to the episode res.locals.isReadingWorkflow = true + res.locals.readingCase = getReadingCase(data, appointment) res.locals.session = session res.locals.appointmentData = { clinic, @@ -656,7 +676,7 @@ module.exports = (router) => { } // Check if user has already read this appointment - if (userHasReadAppointment(appointment, currentUserId)) { + if (userHasReadAppointment(data, appointment, currentUserId)) { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/existing-read` ) @@ -671,8 +691,7 @@ module.exports = (router) => { } // Check if appointment has been deferred from reading - const { isDeferred } = require('../lib/utils/reading') - if (isDeferred(appointment)) { + if (isCaseDeferred(getReadingCase(data, appointment))) { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/existing-read` ) @@ -707,6 +726,7 @@ module.exports = (router) => { .map((id) => data.appointments.find((e) => e.id === id)) .filter(Boolean) const nextUnreadAppointment = getNextUserReadableAppointment( + data, sessionAppointments, appointmentId, currentUserId, @@ -722,6 +742,7 @@ module.exports = (router) => { } else { // Check if there are any readable cases left in the session const firstReadable = getFirstUserReadableAppointment( + data, sessionAppointments, currentUserId ) @@ -808,6 +829,7 @@ module.exports = (router) => { .map((id) => data.appointments.find((e) => e.id === id)) .filter(Boolean) const nextUnreadAppointment = getNextUserReadableAppointment( + data, sessionAppointments, appointmentId, currentUserId, @@ -837,6 +859,7 @@ module.exports = (router) => { } else { // Check if there are any readable cases left in the session const firstReadable = getFirstUserReadableAppointment( + data, sessionAppointments, currentUserId ) @@ -909,22 +932,20 @@ module.exports = (router) => { // mutating in place - appointment records are shared read-only data; writes // go through the update helpers const appointment = data.appointments.find((e) => e.id === appointmentId) - if (appointment) { - const imageReading = structuredClone(appointment.imageReading || {}) - - // Remove any existing read by this user — deferral replaces a prior opinion - if (imageReading.reads?.[currentUserId]) { - delete imageReading.reads[currentUserId] - } - - imageReading.deferral = { - deferredAt: new Date().toISOString(), - deferredBy: currentUserId, - reason: reason || null + const readingCase = getReadingCase(data, appointment) + if (readingCase) { + // Deferring withdraws any opinion this user had already given - they're + // saying they can't judge this case after all + const updatedCase = { + ...withoutRead(readingCase, currentUserId), + deferral: { + deferredAt: new Date().toISOString(), + deferredBy: currentUserId, + reason: reason || null + } } - // Saves to the appointment and mirrors into data.appointment if it matches - updateAppointmentData(data, appointmentId, { imageReading }) + updateReadingCase(data, appointment.episodeId, updatedCase) } // If submitted from an existing-read page (e.g. editing reason), return there @@ -947,6 +968,7 @@ module.exports = (router) => { .map((id) => data.appointments.find((e) => e.id === id)) .filter(Boolean) const nextUnreadAppointment = getNextUserReadableAppointment( + data, sessionAppointments, appointmentId, currentUserId, @@ -976,6 +998,7 @@ module.exports = (router) => { } else { // Check if there are any readable cases left in the session const firstReadable = getFirstUserReadableAppointment( + data, sessionAppointments, currentUserId ) @@ -998,13 +1021,10 @@ module.exports = (router) => { const { sessionId, appointmentId } = req.params const appointment = data.appointments.find((e) => e.id === appointmentId) - if (appointment?.imageReading?.deferral) { - // Clone rather than mutate - appointment records are shared read-only data - const imageReading = structuredClone(appointment.imageReading) - delete imageReading.deferral - - // Saves to the appointment and mirrors into data.appointment if it matches - updateAppointmentData(data, appointmentId, { imageReading }) + const readingCase = getReadingCase(data, appointment) + if (isCaseDeferred(readingCase)) { + const { deferral, ...withoutDeferral } = readingCase + updateReadingCase(data, appointment.episodeId, withoutDeferral) } res.redirect( @@ -1025,21 +1045,21 @@ module.exports = (router) => { const { appointmentId } = req.body const appointment = data.appointments.find((e) => e.id === appointmentId) - if (appointment?.imageReading?.deferral) { - // Clone rather than mutate - appointment records are shared read-only data - const imageReading = structuredClone(appointment.imageReading) - imageReading.deferralHistory = [ - ...(imageReading.deferralHistory || []), - { - ...imageReading.deferral, - resolvedAt: new Date().toISOString(), - resolvedBy: data.currentUser?.id - } - ] - delete imageReading.deferral - - // Saves to the appointment and mirrors into data.appointment if it matches - updateAppointmentData(data, appointmentId, { imageReading }) + const readingCase = getReadingCase(data, appointment) + if (isCaseDeferred(readingCase)) { + const { deferral, ...withoutDeferral } = readingCase + + updateReadingCase(data, appointment.episodeId, { + ...withoutDeferral, + deferralHistory: [ + ...(readingCase.deferralHistory || []), + { + ...deferral, + resolvedAt: new Date().toISOString(), + resolvedBy: data.currentUser?.id + } + ] + }) const participant = data.participants.find( (p) => p.id === appointment.participantId @@ -1804,7 +1824,7 @@ module.exports = (router) => { if (comparisonSetting === 'late' && !formData?.comparisonComplete) { if ( shouldShowComparePage( - appointment, + getReadingCase(data, appointment), formData, currentUserId, data.settings @@ -1905,7 +1925,7 @@ module.exports = (router) => { } // Write the reading (passing session context to handle skipped appointments) - writeReading(appointment, currentUserId, readResult, data, sessionId) + writeReading(data, appointment, currentUserId, readResult, sessionId) // Top up the session with the next eligible appointment if under target size topUpSession(data, sessionId) @@ -1916,6 +1936,7 @@ module.exports = (router) => { .map((id) => data.appointments.find((e) => e.id === id)) .filter(Boolean) const nextUnreadAppointment = getNextUserReadableAppointment( + data, sessionAppointments, appointmentId, currentUserId, @@ -1970,6 +1991,7 @@ module.exports = (router) => { } else { // Check if there are any readable cases left in the session const firstReadable = getFirstUserReadableAppointment( + data, sessionAppointments, currentUserId ) @@ -2048,7 +2070,7 @@ module.exports = (router) => { const currentUserId = data.currentUser?.id if ( shouldShowComparePage( - appointment, + getReadingCase(data, appointment), data.imageReadingTemp, currentUserId, data.settings @@ -2069,7 +2091,7 @@ module.exports = (router) => { if (comparisonSetting === 'late') { if ( shouldShowComparePage( - appointment, + getReadingCase(data, appointment), data.imageReadingTemp, data.currentUser?.id, data.settings @@ -2129,9 +2151,10 @@ module.exports = (router) => { const opinion = data.imageReadingTemp?.opinion const comparisonInfo = getComparisonInfo( - appointment, + getReadingCase(data, appointment), data.imageReadingTemp, - currentUserId + currentUserId, + data.settings ) const firstOpinion = comparisonInfo?.firstOpinion const forceNormalDetailsForDiscordantNormal = @@ -2250,29 +2273,18 @@ module.exports = (router) => { // Get all recent readings across all appointments - last 30 days const thirtyDaysAgo = dayjs().subtract(30, 'days').toISOString() - // Collect all readings from appointments + // Collect all reads across the reading cases const allReadings = [] data.appointments.forEach((appointment) => { - if (!appointment.imageReading?.reads) return - - const appointmentReadings = Object.entries( - appointment.imageReading.reads - ).map(([readerId, reading]) => { - // Determine if this is a first or second read - const readingsForAppointment = Object.values( - appointment.imageReading.reads - ) - const sortedReadings = [...readingsForAppointment].sort( - (a, b) => new Date(a.timestamp) - new Date(b.timestamp) - ) - - const readOrder = sortedReadings.findIndex( - (r) => r.readerId === readerId - ) + const readingCase = getReadingCase(data, appointment) + const reads = getReadsAsArray(readingCase) + if (!reads.length) return - const readType = - readOrder === 0 ? 'first' : readOrder === 1 ? 'second' : 'arbitration' + const appointmentReadings = reads.map((reading) => { + // Each read records what kind it was when it was made, so history + // doesn't have to infer it from ordering + const readType = reading.readType // Get participant info const participant = data.participants.find( @@ -2337,7 +2349,7 @@ module.exports = (router) => { .filter(Boolean) const userCompletedCount = sessionAppointments.filter( (appointment) => - userHasReadAppointment(appointment, currentUserId) || + userHasReadAppointment(data, appointment, currentUserId) || userRequestedPriors(appointment, currentUserId) ).length diff --git a/app/views/_includes/reading/opinion-ui.njk b/app/views/_includes/reading/opinion-ui.njk index 03117e9b..9abaa27b 100644 --- a/app/views/_includes/reading/opinion-ui.njk +++ b/app/views/_includes/reading/opinion-ui.njk @@ -1,4 +1,4 @@ -{% set existingRead = appointment | getReadForUser %} +{% set existingRead = readingCase | getReadForUser(data.currentUser.id) %} {{ existingRead | log("Existing Read")}} {% set existingOpinion = existingRead.opinion %} {% set hasSymptoms = appointment.medicalInformation.symptoms | length > 0 %} @@ -36,7 +36,7 @@ {# Work out if *this* is a first or second read. Have to compare the read counts and whether the current user has read #} - {% set readCount = (appointment | getReadingMetadata).readCount %} + {% set readCount = (readingCase | getReadingMetadata).readCount %} {% if existingResult %} {% if readCount == 1 %} diff --git a/app/views/_includes/reading/workflow-navigation.njk b/app/views/_includes/reading/workflow-navigation.njk index a62323fe..997305d1 100644 --- a/app/views/_includes/reading/workflow-navigation.njk +++ b/app/views/_includes/reading/workflow-navigation.njk @@ -18,7 +18,7 @@ {% if progress.hasNextUserReadable %}
- {% if appointment | canUserReadAppointment %} + {% if data | canUserReadAppointment(appointment) %} {{ appForwardLink({ href: "/reading/session/" ~ sessionId ~ "/appointments/" ~ appointmentId ~ "/skip", text: "Skip to next" diff --git a/app/views/reading/deferred.html b/app/views/reading/deferred.html index d638ca2f..2618210e 100644 --- a/app/views/reading/deferred.html +++ b/app/views/reading/deferred.html @@ -14,38 +14,24 @@ {% block pageContent %} - {# Get all appointments that have been deferred #} - {% set deferredAppointments = [] %} - {% for thisAppointment in data.appointments %} - {% if thisAppointment | isDeferred %} - {% set deferredAppointments = deferredAppointments | push(thisAppointment) %} - {% endif %} - {% endfor %} - - {# Sort by deferral date, most recent first #} - {% set deferredAppointments = deferredAppointments | sort(true, false, 'imageReading.deferral.deferredAt') %} - - {# Collect resolved deferrals across all appointments, most recently resolved first #} - {% set resolvedDeferrals = [] %} - {% for thisAppointment in data.appointments %} - {% for pastDeferral in thisAppointment.imageReading.deferralHistory %} - {% set resolvedDeferrals = resolvedDeferrals | push({ appointment: thisAppointment, deferral: pastDeferral }) %} - {% endfor %} - {% endfor %} - {% set resolvedDeferrals = resolvedDeferrals | sort(true, false, 'deferral.resolvedAt') %} + {# Deferral lives on the reading case, so both lists come from there, + already paired with their appointment and participant #} + {% set deferredCases = data | getDeferredCases %} + {% set resolvedDeferrals = data | getResolvedDeferrals %} Image reading

{{ pageHeading }}

Cases deferred from reading require manual review before they can be returned to the reading queue.

- {% if deferredAppointments | length == 0 %} + {% if deferredCases | length == 0 %}

No deferred cases.

{% else %} - {% for thisAppointment in deferredAppointments %} - {% set thisParticipant = data | getParticipant(thisAppointment.participantId) %} - {% set deferral = thisAppointment.imageReading.deferral %} + {% for row in deferredCases %} + {% set thisAppointment = row.appointment %} + {% set thisParticipant = row.participant %} + {% set deferral = row.deferral %} {% set screenedHtml %} {% set daysSinceScreening = thisAppointment.timing.startTime | daysSince %} @@ -114,8 +100,8 @@

Recently resolved

{% for row in resolvedDeferrals %} {% set thisAppointment = row.appointment %} + {% set thisParticipant = row.participant %} {% set deferral = row.deferral %} - {% set thisParticipant = data | getParticipant(thisAppointment.participantId) %} {% set deferredHtml %} {{ deferral.deferredAt | formatDate("D MMM YYYY") }} diff --git a/app/views/reading/history.html b/app/views/reading/history.html index c9b22aff..7afc12e5 100644 --- a/app/views/reading/history.html +++ b/app/views/reading/history.html @@ -169,15 +169,16 @@

{{ pageHeading }}

{% set appointment = data.appointments | find('id', reading.appointmentId) %} - {% set outcome = appointment | getOutcome %} - {% if outcome == 'not_read' %} + {% set thisCase = data | getReadingCase(appointment) %} + {% set caseState = thisCase | getReadingCaseState(data.settings) %} + {% if caseState == 'awaiting_first_read' %} {{ "Waiting for 1st read" | toTag }} - {% elseif outcome == 'pending_second_read' %} + {% elseif caseState == 'awaiting_second_read' %} {{ "Waiting for 2nd read" | toTag }} - {% elseif outcome == 'arbitration_pending' %} + {% elseif caseState == 'arbitration_required' or caseState == 'in_arbitration' %} {{ "Arbitration" | toTag }} {% else %} - {{ outcome | toTag }} + {{ (thisCase | getReadingCaseOutcome(data.settings)) | toTag }} {% endif %} diff --git a/app/views/reading/index-complex.html b/app/views/reading/index-complex.html index 1b570faa..adbf98ab 100644 --- a/app/views/reading/index-complex.html +++ b/app/views/reading/index-complex.html @@ -38,14 +38,14 @@

Start new reading session

-{% set firstReadsAppointments = allReadsAppointments | filterAppointmentsByNeedsFirstRead %} -{% set secondReadsAppointments = allReadsAppointments | filterAppointmentsByNeedsSecondRead %} +{% set firstReadsAppointments = filterAppointmentsByNeedsFirstRead(data, allReadsAppointments) %} +{% set secondReadsAppointments = filterAppointmentsByNeedsSecondRead(data, allReadsAppointments) %} {# Filter for current user - currently not used #} -{% set allReadsForUser = allReadsAppointments | filterAppointmentsByUserCanRead(data.currentUser.id) %} -{% set firstReadsForUser = firstReadsAppointments | filterAppointmentsByUserCanRead(data.currentUser.id) %} -{% set secondReadsForUser = secondReadsAppointments | filterAppointmentsByUserCanRead(data.currentUser.id) %} -{% set awaitingPriorsForUser = awaitingPriorsAppointments | filterAppointmentsByUserCanRead(data.currentUser.id) %} +{% set allReadsForUser = filterAppointmentsByUserCanRead(data, allReadsAppointments, data.currentUser.id) %} +{% set firstReadsForUser = filterAppointmentsByUserCanRead(data, firstReadsAppointments, data.currentUser.id) %} +{% set secondReadsForUser = filterAppointmentsByUserCanRead(data, secondReadsAppointments, data.currentUser.id) %} +{% set awaitingPriorsForUser = filterAppointmentsByUserCanRead(data, awaitingPriorsAppointments, data.currentUser.id) %} {# Get oldest #} {% set oldestAllRead = allReadsAppointments[0].timing.startTime if allReadsAppointments.length > 0 %} diff --git a/app/views/reading/index-simple.html b/app/views/reading/index-simple.html index 5fd863de..e728e5bd 100644 --- a/app/views/reading/index-simple.html +++ b/app/views/reading/index-simple.html @@ -20,7 +20,7 @@ {% set userReadCount = 0 %} {% for appointmentId in session.appointmentIds %} {% set thisAppointment = data.appointments | find('id', appointmentId) %} - {% if thisAppointment and (thisAppointment | userHasReadAppointment(currentUserId)) %} + {% if thisAppointment and (data | userHasReadAppointment(thisAppointment, currentUserId)) %} {% set userReadCount = userReadCount + 1 %} {% endif %} {% endfor %} @@ -58,8 +58,8 @@ {# Inset counts. - arbitrationCount is derived from real data — appointments whose computed outcome is - arbitration_pending. + arbitrationCount is derived from real data — cases whose derived state is + arbitration_required. newlyArrivedPriorsCount is faked for the prototype. In a real implementation this would be the count of cases where priors the reader requested or was @@ -68,14 +68,14 @@ #} {% set arbitrationCount = 0 %} {% for thisAppointment in data.appointments %} - {% if (thisAppointment | getOutcome) == 'arbitration_pending' %} + {% if ((data | getReadingCase(thisAppointment)) | getReadingCaseState(data.settings)) == 'arbitration_required' %} {% set arbitrationCount = arbitrationCount + 1 %} {% endif %} {% endfor %} {% set deferredCount = 0 %} {% for thisAppointment in data.appointments %} - {% if thisAppointment | isDeferred %} + {% if (data | getReadingCase(thisAppointment)) | isCaseDeferred %} {% set deferredCount = deferredCount + 1 %} {% endif %} {% endfor %} @@ -95,18 +95,17 @@ Backlog counts — all eligible cases that still need reading, banded by age relative to the configured priority/urgent thresholds. #} -{% set backlogAppointments = data.appointments - | filterAppointmentsByEligibleForReading - | filterAppointmentsByNeedsAnyRead %} +{% set backlogAppointments = filterAppointmentsByNeedsAnyRead( + data, data.appointments | filterAppointmentsByEligibleForReading +) %} {% set backlogTotal = backlogAppointments | length %} {% set urgentBacklog = backlogAppointments | filterAppointmentsByDayRange(data.config.reading.urgentThreshold) | length %} {% set priorityBacklog = backlogAppointments | filterAppointmentsByDayRange(data.config.reading.priorityThreshold, data.config.reading.urgentThreshold - 1) | length %} {# Cases the current user can actually read — used to gate the start button #} -{% set userReadableTotal = data.appointments - | filterAppointmentsByEligibleForReading - | filterAppointmentsByUserCanRead(currentUserId) - | length %} +{% set userReadableTotal = filterAppointmentsByUserCanRead( + data, data.appointments | filterAppointmentsByEligibleForReading, currentUserId +) | length %} {% block pageContent %}
diff --git a/app/views/reading/session.html b/app/views/reading/session.html index 070718c0..a3cb7197 100644 --- a/app/views/reading/session.html +++ b/app/views/reading/session.html @@ -107,7 +107,7 @@

{# YOUR READS VIEW - Shows cases from user's perspective #} {% set userReadableAppointments = [] %} {% for appointment in appointments %} - {% if appointment | canUserReadAppointment or appointment | userHasReadAppointment or appointment | userRequestedPriors(data.currentUser.id) or appointment | isDeferred %} + {% if (data | canUserReadAppointment(appointment)) or (data | userHasReadAppointment(appointment)) or (appointment | userRequestedPriors(data.currentUser.id)) or (appointment.readingCase | isCaseDeferred) %} {% set userReadableAppointments = userReadableAppointments | push(appointment) %} {% endif %} {% endfor %} @@ -124,8 +124,8 @@

{% set deferredAppointments = [] %} {% for appointment in userReadableAppointments %} - {% if appointment | userHasReadAppointment %} - {% set read = appointment | getReadForUser %} + {% if data | userHasReadAppointment(appointment) %} + {% set read = appointment.readingCase | getReadForUser(data.currentUser.id) %} {% if read.opinion == 'normal' %} {% set normalAppointments = normalAppointments | push(appointment) %} {% elseif read.opinion == 'technical_recall' %} @@ -135,7 +135,7 @@

{% endif %} {% elseif appointment | userRequestedPriors(data.currentUser.id) %} {% set priorsRequestedAppointments = priorsRequestedAppointments | push(appointment) %} - {% elseif appointment | isDeferred %} + {% elseif appointment.readingCase | isCaseDeferred %} {% set deferredAppointments = deferredAppointments | push(appointment) %} {% endif %} {% endfor %} @@ -238,8 +238,8 @@

Reading session cases

{% for appointment in userReadableAppointments %} - {% set metadata = appointment | getReadingMetadata %} - + {% set metadata = appointment.readingCase | getReadingMetadata(data.settings) %} + {{ loop.index }}. @@ -255,7 +255,7 @@

Reading session cases


{% set readCount = metadata.readCount %} - {% if appointment | userHasReadAppointment %} + {% if data | userHasReadAppointment(appointment) %} {{ "1st read" if readCount == 1 else "2nd read" }} {% else %} {{ "1st read" if readCount == 0 else "2nd read" }} @@ -290,12 +290,12 @@

Reading session cases

 - {% if appointment | userHasReadAppointment %} - {% set read = appointment | getReadForUser %} + {% if data | userHasReadAppointment(appointment) %} + {% set read = appointment.readingCase | getReadForUser(data.currentUser.id) %} {% if read.opinion %} {{ read.opinion | toTag }} {% endif %} - {% elseif appointment | isDeferred %} + {% elseif appointment.readingCase | isCaseDeferred %} {{ "Deferred" | toTag }} {% elseif appointment | userRequestedPriors(data.currentUser.id) %} {{ "Priors requested" | toTag }} @@ -306,7 +306,7 @@

Reading session cases

{% endif %} - {% if resumeAppointment and appointment.id == resumeAppointment.id and not (appointment | userHasReadAppointment) %} + {% if resumeAppointment and appointment.id == resumeAppointment.id and not (data | userHasReadAppointment(appointment)) %} {{ readingActionText }} {% endif %} @@ -345,8 +345,9 @@

Reading session cases

{% set allViewArbitrationAppointments = [] %} {% for appointment in appointments %} - {% set appointmentOutcome = appointment | getOutcome %} - {% if appointmentOutcome == 'pending_second_read' %} + {% set appointmentState = appointment.readingCase | getReadingCaseState(data.settings) %} + {% set appointmentOutcome = appointment.readingCase | getReadingCaseOutcome(data.settings) %} + {% if appointmentState == 'awaiting_second_read' %} {% set awaitingSecondReadAppointments = awaitingSecondReadAppointments | push(appointment) %} {% elseif appointmentOutcome == 'normal' %} {% set allViewNormalAppointments = allViewNormalAppointments | push(appointment) %} @@ -354,7 +355,7 @@

Reading session cases

{% set allViewTechnicalRecallAppointments = allViewTechnicalRecallAppointments | push(appointment) %} {% elseif appointmentOutcome == 'recall_for_assessment' %} {% set allViewRecallForAssessmentAppointments = allViewRecallForAssessmentAppointments | push(appointment) %} - {% elseif appointmentOutcome == 'arbitration_pending' %} + {% elseif appointmentState == 'arbitration_required' or appointmentState == 'in_arbitration' %} {% set allViewArbitrationAppointments = allViewArbitrationAppointments | push(appointment) %} {% endif %} {% endfor %} @@ -434,7 +435,7 @@

Reading session cases

{% for appointment in appointments %} - {% set metadata = appointment | getReadingMetadata %} + {% set metadata = appointment.readingCase | getReadingMetadata(data.settings) %} {{ loop.index }}. @@ -485,7 +486,7 @@

Reading session cases

{% set isBlind = data.settings.reading.blindReading | falsify %} {% set isCurrentUserRead = false %} - {% set allReads = appointment | getReadsAsArray %} + {% set allReads = appointment.readingCase | getReadsAsArray %} {% if metadata.readCount >= 1 %} {% set firstRead = allReads[0] %} @@ -534,17 +535,17 @@

Reading session cases

{{ "Waiting for 1st read" | toTag }} {% endif %} - {# Outcome column — uses getOutcome which respects the site's arbitration policy #} + {# Outcome column — the case state respects the site's arbitration policy #} - {% set outcome = appointment | getOutcome %} - {% if outcome == 'not_read' %} + {% set caseState = appointment.readingCase | getReadingCaseState(data.settings) %} + {% if caseState == 'awaiting_first_read' %} {{ "Waiting for 1st read" | toTag }} - {% elseif outcome == 'pending_second_read' %} + {% elseif caseState == 'awaiting_second_read' %} {{ "Waiting for 2nd read" | toTag }} - {% elseif outcome == 'arbitration_pending' %} + {% elseif caseState == 'arbitration_required' or caseState == 'in_arbitration' %} {{ "Arbitration" | toTag }} {% else %} - {{ outcome | toTag }} + {{ (appointment.readingCase | getReadingCaseOutcome(data.settings)) | toTag }} {% endif %} diff --git a/app/views/reading/workflow/compare.html b/app/views/reading/workflow/compare.html index 00d84182..3788ed54 100644 --- a/app/views/reading/workflow/compare.html +++ b/app/views/reading/workflow/compare.html @@ -21,7 +21,7 @@ {# Get comparison info — pass full imageReadingTemp so discordance can check TR views and RFA breast assessments #} {% set secondOpinion = data.imageReadingTemp.opinion %} - {% set comparisonInfo = appointment | getComparisonInfo(data.imageReadingTemp) %} + {% set comparisonInfo = readingCase | getComparisonInfo(data.imageReadingTemp, data.currentUser.id, data.settings) %} {% set firstRead = comparisonInfo.firstRead %} {% set firstOpinion = comparisonInfo.firstOpinion %} {% set comparisonType = comparisonInfo.type %} diff --git a/app/views/reading/workflow/defer-case.html b/app/views/reading/workflow/defer-case.html index 82de25e0..afe3add1 100644 --- a/app/views/reading/workflow/defer-case.html +++ b/app/views/reading/workflow/defer-case.html @@ -38,7 +38,7 @@

text: "Explain why you are unable to give an opinion on this case" }, rows: 4, - value: data.deferralReason if data.deferralReason else appointment.imageReading.deferral.reason + value: data.deferralReason if data.deferralReason else readingCase.deferral.reason }) }} {{ button({ diff --git a/app/views/reading/workflow/existing-read.html b/app/views/reading/workflow/existing-read.html index 1493bcbd..c0ca7912 100644 --- a/app/views/reading/workflow/existing-read.html +++ b/app/views/reading/workflow/existing-read.html @@ -9,8 +9,8 @@ {% block pageContent %} {% set isAwaitingPriors = appointment | awaitingPriors %} - {% set hasUserRead = appointment | userHasReadAppointment %} - {% set isDeferredCase = appointment | isDeferred %} + {% set hasUserRead = readingCase | userHasReadCase(data.currentUser.id) %} + {% set isDeferredCase = readingCase | isCaseDeferred %} {{ participant | getFullName }} @@ -20,7 +20,7 @@ {# Appointment has been deferred - show deferral summary #}

Your opinion

- {% set deferral = appointment.imageReading.deferral %} + {% set deferral = readingCase.deferral %} {% set canUndo = deferral.deferredBy == data.currentUser.id %} {% set deferralSummaryHtml %} @@ -140,7 +140,7 @@

Your opinion

{% endif %} {% set readSummaryHtml %} - {% set read = appointment | getReadForUser %} + {% set read = readingCase | getReadForUser(data.currentUser.id) %} {% set allowEdits = true %} {% set changeOpinionUrl = "./opinion" %} {% set showAnnotationImages = withImages %} @@ -156,7 +156,7 @@

Your opinion

}) }} {# Other readers' assessments (for testing/debugging) #} - {% set otherReads = appointment | getOtherReads %} + {% set otherReads = readingCase | getOtherReads(data.currentUser.id) %} {% if otherReads | length > 0 %} {% for otherRead in otherReads %} {% set readerName = otherRead.readerId | getUsername %} diff --git a/app/views/reading/workflow/opinion.html b/app/views/reading/workflow/opinion.html index 94586d64..6b9d2f42 100644 --- a/app/views/reading/workflow/opinion.html +++ b/app/views/reading/workflow/opinion.html @@ -21,7 +21,7 @@ ========================================================= #} {# Get existing read data for this user (saved read) #} - {% set existingRead = appointment | getReadForUser %} + {% set existingRead = readingCase | getReadForUser(data.currentUser.id) %} {% set existingOpinion = existingRead.opinion %} {# Also check for in-progress read in temp data #} @@ -367,7 +367,7 @@

{% endif %} {# First/second read tag #} - {% set readCount = (appointment | getReadingMetadata).readCount %} + {% set readCount = (readingCase | getReadingMetadata).readCount %} {% if existingOpinion %} {% if readCount == 1 %} {{ "First read" | toTag }} diff --git a/docs/DATA-GENERATOR-REFERENCE.md b/docs/DATA-GENERATOR-REFERENCE.md index a1ac1e95..0208f3ee 100644 --- a/docs/DATA-GENERATOR-REFERENCE.md +++ b/docs/DATA-GENERATOR-REFERENCE.md @@ -69,15 +69,21 @@ generate-seed-data.js (main orchestrator) **Episode level:** - The screening round: stage (`scheduled` | `mammograms` | `reading` | `assessment` | `closed`), stage history, final outcome - Links to its appointments (`appointmentIds`) +- Reading cases (`readingCases`), one per set of images taken — the reads live here - Historic episodes: summary-level record of a past round (dates, outcome, `mammogramSummary`) with no appointments **Appointment level:** - Medical information collected during appointments - Session details (who, when, where) -- Mammogram images and reading data (`imageReading`) +- Mammogram images (`mammogramData`) and prior mammograms - Appointment-specific data -**Key principle:** Medical information is stored at the **appointment level**, representing data collected during specific appointments. Round-level state (stage, outcome) lives on the **episode**. +**Key principle:** Medical information is stored at the **appointment level**, representing data collected during specific appointments. Round-level state (stage, outcome) and the reading of each image set live on the **episode**. + +Because reads are written onto cases, generation runs in that order: episodes +open a case per image set (`syncEpisodeReadingCases`), then `generateReadingData` +writes reads into them, then `finaliseEpisodeStage` settles each episode's stage +from its latest case. ## Generator Pattern diff --git a/docs/IMAGE-READING-TECHNICAL-SUMMARY.md b/docs/IMAGE-READING-TECHNICAL-SUMMARY.md index f066a3b1..4e22c2c9 100644 --- a/docs/IMAGE-READING-TECHNICAL-SUMMARY.md +++ b/docs/IMAGE-READING-TECHNICAL-SUMMARY.md @@ -100,34 +100,49 @@ app/ ## Data Storage Locations -### 1. Permanent Storage: `appointment.imageReading.reads` +### 1. Permanent Storage: `episode.readingCases[]` -Final reading opinions are stored on the appointment object: +A **reading case** is one set of mammograms being read. Cases live on the +episode, one per image set, and the reads belong to the case: ```javascript -appointment.imageReading = { - reads: { - [userId]: { - opinion: 'normal' | 'technical_recall' | 'recall_for_assessment', - readerId: userId, - readerType: 'radiologist', - readNumber: 1, // 1 = first read, 2 = second read - timestamp: '2025-01-15T10:30:00.000Z', - // For abnormal opinions, includes per-breast data: - left: { - breastAssessment: 'normal' | 'clinical' | 'abnormal', - comment: 'optional text', - annotations: [...] - }, - right: { ... } - } +episode.readingCases = [ + { + id: 'abc12345', + appointmentId: 'def67890', // whose images this case covers + openedDate: '2026-01-15T09:00:00.000Z', + reads: [ + { + opinion: 'normal' | 'technical_recall' | 'recall_for_assessment', + readerId: userId, + readerType: 'radiologist', + readType: 'first' | 'second' | 'arbitration', + readNumber: 1, + timestamp: '2026-01-15T10:30:00.000Z', + // For abnormal opinions, includes per-breast data: + left: { + breastAssessment: 'normal' | 'clinical' | 'abnormal', + comment: 'optional text', + annotations: [...] + }, + right: { ... } + } + ], + deferral, deferralHistory } -} +] ``` -- Keyed by userId - each user can have one reading per appointment -- Written via `writeReading()` utility function -- `readNumber` indicates order (1 = first, 2 = second read, 3 = arbitration read) +- One case per image set; a technical recall's re-screen opens a second case, + and the episode's reading state comes from the latest one +- Reads are ordered, at most one per reader, and each records its own `readType` + — settled when the read is written rather than inferred from position later +- Written via `writeReading()` (appointments) or the case helpers in + `reading-cases.js` +- Resolve an appointment's case with `getReadingCase(data, appointment)` from + `episodes.js` + +See [data-conventions.md](data-conventions.md) for the state and outcome model. ### 2. Temporary Storage: `data.imageReadingTemp` @@ -401,14 +416,15 @@ Templates receive via `res.locals`: ### reading.js — Single Appointment -- `getReadingMetadata(appointment)` - Returns `{ readCount, uniqueReaderCount, firstReadComplete, secondReadComplete, isDiscordant, opinions }` (computed on demand) +- `getReadingMetadata(readingCase, settings)` - Returns `{ readCount, uniqueReaderCount, firstReadComplete, secondReadComplete, isDiscordant, opinions, state, outcome }` (computed on demand). `getAppointmentReadingMetadata(data, appointment)` is the appointment-shaped wrapper. - `getReadsAsArray(appointment)` - Returns reads sorted by readNumber (or timestamp fallback) - `getReadForUser(appointment, userId)` - Get this user's read object - `getOtherReads(appointment, userId)` - Get reads from other users (for comparison) -- `writeReading(appointment, userId, reading, data, sessionId)` - Saves a reading, assigns readNumber, removes from skipped list +- `writeReading(data, appointment, userId, reading, sessionId)` - Saves a read onto the appointment's case, settles readNumber and readType, removes from skipped list - `areReadsDiscordant(readA, readB)` - Compares opinions, TR views, and RFA breast assessments - `willGoToArbitration(readA, readB, settings)` - Policy-aware: always true if discordant; may be true for concordant non-normal depending on `arbitrationPolicy` -- `getOutcome(appointment, settings)` - Computes outcome: `not_read` | `pending_second_read` | `arbitration_pending` | `normal` | `technical_recall` | `recall_for_assessment`. Third read (readNumber 3) is the arbitration read and its opinion resolves the case. +- `getReadingCaseState(readingCase, settings)` - Where the case has got to: `awaiting_first_read` | `awaiting_second_read` | `arbitration_required` | `in_arbitration` | `concluded` +- `getReadingCaseOutcome(readingCase, settings)` - What it found: `normal` | `technical_recall` | `recall_for_assessment`, or `null` while reading is still under way. The arbitration read, where there is one, is the deciding read. - `getComparisonInfo(appointment, secondReadData, userId, settings)` - Returns comparison data for second reader, or `false` if not applicable - `shouldShowComparePage(appointment, secondReadData, userId, settings)` - Boolean: whether to show compare page given timing/filter settings @@ -451,11 +467,20 @@ Templates receive via `res.locals`: ### reading.js — Boolean Checks -- `hasReads(appointment)` - Has any reads -- `canUserReadAppointment(appointment, userId)` - User can read (not already read, not awaiting priors, under max reads) -- `userHasReadAppointment(appointment, userId)` - User has already read -- `isDeferred(appointment)` - Case has an active deferral (`imageReading.deferral`) -- `needsFirstRead(appointment)`, `needsSecondRead(appointment)`, `needsArbitration(appointment)` (policy-aware context function) +Appointment-shaped, so they take `data` to resolve the case: + +- `canUserReadAppointment(data, appointment, userId)` - User can read (not already read, not awaiting priors, not deferred, under max reads) +- `userHasReadAppointment(data, appointment, userId)` - User has already read + +### reading-cases.js — Boolean Checks + +Case-shaped, and pure: + +- `caseHasReads(readingCase)` - Has any reads +- `isCaseDeferred(readingCase)` - Case has an active deferral +- `isCaseInArbitration(readingCase)` - Released into arbitration (nothing does this yet) +- `caseNeedsFirstRead(readingCase)`, `caseNeedsSecondRead(readingCase)`, `caseNeedsArbitration(readingCase, settings)` +- `canUserReadCase(readingCase, userId)`, `userHasReadCase(readingCase, userId)` ### prior-mammograms.js @@ -489,7 +514,7 @@ Each appointment needs two independent reads: ### Reading Metadata -`getReadingMetadata(appointment)` calculates (computed on demand, not stored): +`getReadingMetadata(readingCase, settings)` calculates (computed on demand, not stored): - `readCount` - Total reads - `uniqueReaderCount` - Different readers @@ -497,7 +522,7 @@ Each appointment needs two independent reads: - `isDiscordant` - Whether existing reads disagree meaningfully (not just opinion string) - `opinions` - Array of unique opinion values -For arbitration state, use `getOutcome(appointment, settings)` or the `needsArbitration` filter. +For arbitration state, use `getReadingCaseState(readingCase, settings)` or the `caseNeedsArbitration` filter. Use in templates: `{% set metadata = appointment | getReadingMetadata %}` @@ -590,9 +615,10 @@ Prior mammograms are generated at seed time in `appointment-generator.js` using A reader can defer a case out of the reading queue (for example to raise it with a colleague) rather than skip or read it. -- Deferral is stored on the appointment: `appointment.imageReading.deferral = { deferredAt, deferredBy, reason }` -- Deferring removes any existing read by that user — a deferral replaces a prior opinion -- `isDeferred(appointment)` (in `lib/utils/reading.js`) checks for an active deferral +- Deferral is stored on the reading case: `readingCase.deferral = { deferredAt, deferredBy, reason }` +- Deferring removes any existing read by that user — a deferral withdraws a prior opinion +- `isCaseDeferred(readingCase)` (in `lib/utils/reading-cases.js`) checks for an active deferral +- `getDeferredCases(data)` / `getResolvedDeferrals(data)` (in `lib/utils/reading.js`) build the lists the deferred cases page shows - Deferred cases are excluded from reading; `/reading/deferred` lists them, and a deferral can be undone (via `/reading/deferred/undo` or the per-case `/undo-defer` route), returning the case to the queue - The workflow's `defer-case.html` step collects an optional reason (`deferralReason`) diff --git a/docs/data-conventions.md b/docs/data-conventions.md index 15360552..7d55cd64 100644 --- a/docs/data-conventions.md +++ b/docs/data-conventions.md @@ -165,13 +165,60 @@ A round screened longer ago than the reading window closes rather than sitting in `reading` forever - it was read at the time, we just don't seed reads going back that far. -### What deliberately doesn't live on the episode yet +### Reading cases -The target model puts `imageReadings[]`, priors and deferral on the episode. -They are all still **on the appointment**, because moving them touches most of the -reading code. `getEpisodeReadingStatus` derives an episode's reading state -from its appointments rather than holding a copy. The physical move happens with the -work that needs it (arbitration / case views). +A **reading case** is one set of mammograms being read. Cases live on the +episode as `episode.readingCases[]`, one per image set, oldest first — the same +sets `episode.mammograms` records, from the other side: + +```js +{ + id, appointmentId, openedDate, + reads: [{ readerId, readerType, readType, readNumber, timestamp, opinion, ... }], + deferral, deferralHistory +} +``` + +The trigger rule is **new image set → new case**. `updateAppointmentStatus` +opens one the moment an appointment reaches a screened status, alongside the +mammogram entry, and removes it again if that is undone. Most episodes have one +case; a technical recall produces a second set of images and therefore a second +case, and the episode's reading state comes from the **latest** one. + +Reads are an ordered array, and each records its own `readType` (`first`, +`second`, `arbitration`). The type is settled when the read is written, from +where the case had got to at the time — which is not recoverable later, because +reads can be withdrawn (deferring after giving an opinion does exactly that). + +Two functions answer the two different questions, and the split matters: + +| | | +|---|---| +| `getReadingCaseState(case, settings)` | where the case has got to: `awaiting_first_read`, `awaiting_second_read`, `arbitration_required`, `in_arbitration`, `concluded` | +| `getReadingCaseOutcome(case, settings)` | what it found — `normal` / `technical_recall` / `recall_for_assessment`, or **null** while reading is still under way | + +`arbitration_required` and `in_arbitration` are deliberately different. Reads +disagreeing is what makes arbitration *necessary*; a case only becomes +arbitratable once someone releases it into arbitration. Nothing performs that +release yet, so no case reaches `in_arbitration` today — the state exists so the +vocabulary is whole rather than growing a value later across every call site. +Deferral works the same way already: the act is recorded +(`deferral: { deferredAt, deferredBy, reason }`) and `isCaseDeferred` reads the +state back from its presence. + +Priors are the exception that stays on the appointment +(`appointment.previousMammograms`), so `canUserReadAppointment` combines the two. +Deferral and outstanding priors are **states, not outcomes** — both hold a case +up, and a case held up still owes an outcome once it is released. + +**Where the code lives.** `reading-cases.js` holds the case logic and is +deliberately pure — everything there takes a case. `episodes.js` owns getting +cases out of session data (`getReadingCase`) and writing them back +(`updateReadingCase`), because both go through the episode. `reading.js` is the +appointment- and session-shaped layer above: sessions, backlogs, progress. That +is why most of its helpers take `data` — resolving a case is what needs it. +Resolution happens once, at the edges: the reading workflow middleware sets +`res.locals.readingCase`, and list-building attaches `readingCase` to each row. ### Historic episodes diff --git a/docs/utils-filter-reference.md b/docs/utils-filter-reference.md index 996d8f5c..d81f2dba 100644 --- a/docs/utils-filter-reference.md +++ b/docs/utils-filter-reference.md @@ -3,7 +3,7 @@ --- **Auto-generated** — do not edit manually. -- **Generated:** 2026-07-24 15:22 UTC +- **Generated:** 2026-07-28 14:40 UTC - **Source:** `app/lib/utils/` and `app/filters/` - **Regenerate:** `npm run docs` @@ -15,30 +15,31 @@ | File | Purpose | Line | |---|---|---| -| `dates.js` | Date formatting and calculation using dayjs | 49 | -| `strings.js` | String manipulation: case conversion, formatting, NHS-specific formats (NHS number, phone), pluralisation, and HTML-wrapping helpers for use in templates. | 85 | -| `status.js` | Appointment status checks and display helpers | 120 | -| `participants.js` | Participant lookups and derived data: full/short names, age, clinic history, and risk level. | 144 | -| `appointment-data.js` | Appointment lookups and mutations in session data | 164 | -| `episodes.js` | Episode lookups and stage changes | 178 | -| `clinics.js` | Clinic filtering by time period, slot formatting, and opening hours calculation. | 209 | -| `reading.js` | Image reading workflow: read state, progress tracking, batch management, per-user navigation, and filtering | 224 | -| `prior-mammograms.js` | Prior mammogram request state (awaiting, unrequested, resolved) and one-line summary helpers. | 280 | -| `medical-information.js` | Summarise medical history items, symptoms, breast features, and other clinical information into concise display strings. | 299 | -| `annotation-summary.js` | Summarise image reading annotations (abnormality type, level of concern, location) into concise display strings. | 320 | -| `arrays.js` | Array helpers: find by key/id, filter, push (immutable), remove empty | 333 | -| `objects.js` | Object utilities for extracting and flattening values. | 351 | -| `summary-list.js` | NHS summary list helpers: replace empty row values with "Enter X" links or "Not provided" text, and remove the bottom border from the last row. | 361 | -| `random.js` | Seeded random functions for stable prototype data | 372 | -| `referrers.js` | Referrer chain navigation for multi-level back links | 389 | -| `roles-and-permissions.js` | User role checks | 402 | -| `utility.js` | General-purpose type coercion (`falsify`) and limiting utilities. | 420 | +| `dates.js` | Date formatting and calculation using dayjs | 50 | +| `strings.js` | String manipulation: case conversion, formatting, NHS-specific formats (NHS number, phone), pluralisation, and HTML-wrapping helpers for use in templates. | 86 | +| `status.js` | Appointment status checks and display helpers | 121 | +| `participants.js` | Participant lookups and derived data: full/short names, age, clinic history, and risk level. | 145 | +| `appointment-data.js` | Appointment lookups and mutations in session data | 165 | +| `episodes.js` | Episode lookups and stage changes | 179 | +| `clinics.js` | Clinic filtering by time period, slot formatting, and opening hours calculation. | 215 | +| `reading-cases.js` | A reading case is one set of mammograms being read, held on the episode as episode.readingCases[] | 230 | +| `reading.js` | Image reading workflow: read state, progress tracking, batch management, per-user navigation, and filtering | 265 | +| `prior-mammograms.js` | Prior mammogram request state (awaiting, unrequested, resolved) and one-line summary helpers. | 312 | +| `medical-information.js` | Summarise medical history items, symptoms, breast features, and other clinical information into concise display strings. | 331 | +| `annotation-summary.js` | Summarise image reading annotations (abnormality type, level of concern, location) into concise display strings. | 352 | +| `arrays.js` | Array helpers: find by key/id, filter, push (immutable), remove empty | 365 | +| `objects.js` | Object utilities for extracting and flattening values. | 383 | +| `summary-list.js` | NHS summary list helpers: replace empty row values with "Enter X" links or "Not provided" text, and remove the bottom border from the last row. | 393 | +| `random.js` | Seeded random functions for stable prototype data | 404 | +| `referrers.js` | Referrer chain navigation for multi-level back links | 421 | +| `roles-and-permissions.js` | User role checks | 434 | +| `utility.js` | General-purpose type coercion (`falsify`) and limiting utilities. | 452 | | | | | -| `formatting.js` | Display formatting for yes/no answers and ordinal names. (filter only) | 436 | -| `forms.js` | Injects matching flash error messages into NHS form component configs by field name. (filter only) | 448 | -| `nunjucks.js` | Nunjucks-specific helpers: joining arrays, resolving user names from IDs, template debugging, and template literal support. (filter only) | 460 | -| `tags.js` | Convert status strings to NHS `` HTML elements. (filter only) | 474 | -| `markdown.js` | Convert markdown strings to Nunjucks-safe HTML using markdown-it (filter only) | 484 | +| `formatting.js` | Display formatting for yes/no answers and ordinal names. (filter only) | 468 | +| `forms.js` | Injects matching flash error messages into NHS form component configs by field name. (filter only) | 480 | +| `nunjucks.js` | Nunjucks-specific helpers: joining arrays, resolving user names from IDs, template debugging, and template literal support. (filter only) | 492 | +| `tags.js` | Convert status strings to NHS `` HTML elements. (filter only) | 506 | +| `markdown.js` | Convert markdown strings to Nunjucks-safe HTML using markdown-it (filter only) | 516 | --- @@ -183,28 +184,33 @@ Episode lookups and stage changes. An episode is one screening round - the conta | Function | Description | Line | |---|---|---| -| `appointmentProducedImages(appointment)` | Whether an appointment's status means mammograms were taken. | 91 | -| `buildMammogramEntry(appointment, [clinic])` | Build the episode's summary record of one set of mammograms. | 105 | -| `getEpisode(data, episodeId)` | Get an episode by ID | 146 | -| `getEpisodesForParticipant(data, participantId)` | Get all of a participant's episodes, oldest first | 167 | -| `getCurrentEpisode(data, participantId)` | Get a participant's current episode - their most recent one that hasn't | 197 | -| `getEpisodeAppointments(data, episode)` | Get an episode's appointments, oldest first | 213 | -| `getEpisodeReadingStatus(data, episode, [userId])` | Get the reading status of an episode, derived from its appointments. | 228 | -| `isEpisodeClosed(episode)` | Whether an episode has closed | 243 | -| `isEpisodeOpen(episode)` | Whether an episode is still open - anything that hasn't closed, whatever | 253 | -| `getEpisodeMammogramDate(episode)` | When this round's mammograms were taken, from the episode's own record. | 264 | -| `getLastMammogram(data, participantId)` | The participant's last mammogram on record, before today. | 279 | -| `getNextAppointment(data, participantId)` | The participant's next booked appointment, if they have one. | 326 | -| `getEpisodeLabel(episode)` | Human name for an episode. Episodes are named by date, not number - | 352 | -| `getEpisodeStageText(stage)` | Display text for an episode's stage | 368 | -| `getEpisodeStageTagColour(stage)` | Tag colour for an episode's stage | 378 | -| `getEpisodeOutcomeText(outcome)` | Display text for an episode's outcome | 388 | -| `getEpisodeOutcomeTagColour(outcome)` | Tag colour for an episode's outcome | 398 | -| `updateEpisode(data, episodeId, updates)` | Update an episode, persisting the change for this session. | 408 | -| `updateEpisodeStage(data, episodeId, stage, [options])` | Advance an episode to a new stage, appending to its stageHistory. | 441 | -| `syncEpisodeMammogramsForAppointment(data, appointment)` | Keep an episode's mammograms record in step with one of its appointments. | 490 | -| `advanceEpisodeForAppointmentStatus(data, appointment)` | Move an appointment's episode to wherever the appointment's status leaves it. | 530 | -| `advanceEpisodeForReadingOutcome(data, appointment, readingOutcome)` | Move an appointment's episode to wherever its reading outcome leaves it. | 563 | +| `appointmentProducedImages(appointment)` | Whether an appointment's status means mammograms were taken. | 98 | +| `buildMammogramEntry(appointment, [clinic])` | Build the episode's summary record of one set of mammograms. | 112 | +| `getEpisode(data, episodeId)` | Get an episode by ID | 153 | +| `getEpisodesForParticipant(data, participantId)` | Get all of a participant's episodes, oldest first | 174 | +| `getCurrentEpisode(data, participantId)` | Get a participant's current episode - their most recent one that hasn't | 204 | +| `getEpisodeAppointments(data, episode)` | Get an episode's appointments, oldest first | 220 | +| `getReadingCase(data, appointment)` | Get the reading case covering an appointment's images. | 235 | +| `getEpisodeReadingCases(episode)` | Get an episode's reading cases, oldest first | 255 | +| `getEpisodeReadingCase(episode)` | Get the case that says where an episode's reading has got to - its latest. | 265 | +| `getEpisodeReadingOutcome(episode, [settings])` | Get an episode's reading outcome, from its latest case. | 275 | +| `updateReadingCase(data, episodeId, updatedCase)` | Save a changed reading case back to its episode. | 290 | +| `syncReadingCasesForAppointment(data, appointment)` | Keep an episode's reading cases in step with one of its appointments. | 316 | +| `isEpisodeClosed(episode)` | Whether an episode has closed | 355 | +| `isEpisodeOpen(episode)` | Whether an episode is still open - anything that hasn't closed, whatever | 365 | +| `getEpisodeMammogramDate(episode)` | When this round's mammograms were taken, from the episode's own record. | 376 | +| `getLastMammogram(data, participantId)` | The participant's last mammogram on record, before today. | 391 | +| `getNextAppointment(data, participantId)` | The participant's next booked appointment, if they have one. | 438 | +| `getEpisodeLabel(episode)` | Human name for an episode. Episodes are named by date, not number - | 464 | +| `getEpisodeStageText(stage)` | Display text for an episode's stage | 480 | +| `getEpisodeStageTagColour(stage)` | Tag colour for an episode's stage | 490 | +| `getEpisodeOutcomeText(outcome)` | Display text for an episode's outcome | 500 | +| `getEpisodeOutcomeTagColour(outcome)` | Tag colour for an episode's outcome | 510 | +| `updateEpisode(data, episodeId, updates)` | Update an episode, persisting the change for this session. | 520 | +| `updateEpisodeStage(data, episodeId, stage, [options])` | Advance an episode to a new stage, appending to its stageHistory. | 553 | +| `syncEpisodeMammogramsForAppointment(data, appointment)` | Keep an episode's mammograms record in step with one of its appointments. | 602 | +| `advanceEpisodeForAppointmentStatus(data, appointment)` | Move an appointment's episode to wherever the appointment's status leaves it. | 642 | +| `advanceEpisodeForReadingOutcome(data, appointment, readingOutcome)` | Move an appointment's episode to wherever its reading outcome leaves it. | 675 | ### clinics.js @@ -221,61 +227,87 @@ Clinic filtering by time period, slot formatting, and opening hours calculation. | `getClinicHours(clinic)` | Get clinic opening hours | 79 | | `getFilteredClinics(clinics, [filter])` | Get clinics filtered by time period | 97 | +### reading-cases.js + +`app/lib/utils/reading-cases.js` + +A reading case is one set of mammograms being read, held on the episode as episode.readingCases[]. Pure case logic: reads, read types, case state and outcome, discordance and arbitration, deferral. Everything here takes a case — resolve one from an appointment with getReadingCase in episodes.js. + +| Function | Description | Line | +|---|---|---| +| `buildReadingCase(appointment, [openedDate])` | Build a new reading case for one set of images. | 57 | +| `getReadingCases(episode)` | All of an episode's reading cases, oldest first | 80 | +| `getLatestReadingCase(episode)` | An episode's most recent reading case - the one that decides where the | 90 | +| `getReadingCaseForAppointment(episode, appointmentId)` | Find the reading case covering a given appointment's images | 105 | +| `getReadsAsArray(readingCase)` | A case's reads in order, oldest first. | 120 | +| `getReadForUser(readingCase, userId)` | Get one user's read on a case | 133 | +| `getOtherReads(readingCase, userId)` | Get the reads on a case made by anyone other than the given user | 148 | +| `getArbitrationRead(readingCase)` | Get the arbitration read on a case, if one has been made | 159 | +| `userHasReadCase(readingCase, userId)` | Whether a user has read a case | 172 | +| `caseHasReads(readingCase)` | Whether a case has any reads | 183 | +| `isCaseDeferred(readingCase)` | Whether a case has been deferred from reading. | 193 | +| `isCaseInArbitration(readingCase)` | Whether a case has been released into arbitration. | 206 | +| `areReadsDiscordant(readA, readB)` | Whether the reads on a case disagree in a clinically meaningful way. | 220 | +| `willGoToArbitration(readA, readB, [settings])` | Whether two reads mean the case needs arbitrating, taking the site's | 277 | +| `getReadingCaseState(readingCase, [settings])` | Where a case has got to. | 305 | +| `getReadingCaseOutcome(readingCase, [settings])` | What a case found, or null while reading is still under way. | 336 | +| `getReadingMetadata(readingCase, [settings])` | Summary counts and flags for a case, for lists and progress displays | 357 | +| `caseNeedsFirstRead(readingCase)` | Whether a case still needs a first read | 388 | +| `caseNeedsSecondRead(readingCase)` | Whether a case has a first read and still needs a second | 398 | +| `caseNeedsArbitration(readingCase, [settings])` | Whether a case needs arbitrating but hasn't been released into it | 408 | +| `canUserReadCase(readingCase, userId, [options], [options.maxReadsPerCase])` | Whether a user can read a case. | 419 | +| `getComparisonInfo(readingCase, secondReadData, userId, [settings])` | Work out what the second reader should be shown about the first read. | 447 | +| `shouldShowComparePage(readingCase, secondReadData, userId, [settings])` | Whether the compare page should be shown to the second reader. | 496 | +| `buildRead(readingCase, userId, readerType, reading, [options], [options.timestamp])` | Build the read record for a user's opinion on a case. | 535 | +| `withRead(readingCase, read)` | Add or replace a user's read on a case, returning a new case record. | 571 | +| `withoutRead(readingCase, userId)` | Remove a user's read from a case, returning a new case record. | 595 | + ### reading.js `app/lib/utils/reading.js` -Image reading workflow: read state, progress tracking, batch management, per-user navigation, and filtering. The main module for anything related to image reading. +Image reading workflow: read state, progress tracking, batch management, per-user navigation, and filtering. The appointment- and session-shaped layer over reading cases. | Function | Description | Line | |---|---|---| -| `getReadingMetadata(appointment)` | Get reading metadata for an appointment | 35 | -| `getReadsAsArray(appointment)` | Get all reads for an appointment as an ordered array | 66 | -| `writeReading(appointment, userId, reading, data, [sessionId])` | Save a user's reading for an appointment, and remove the appointment from the reading | 87 | -| `enhanceAppointmentsWithReadingData(appointments, participants, userId)` | Enhance appointments with pre-calculated reading metadata | 146 | -| `getReadingStatusForAppointments(appointments, [userId])` | Get detailed reading status for a group of appointments | 341 | -| `getReadingProgress(appointments, currentAppointmentId, skippedAppointments, [userId])` | Get progress through reading a set of appointments | 387 | -| `sortAppointmentsByScreeningDate(appointments)` | Sort appointments by screening date (oldest first) | 704 | -| `getFirstAvailableClinic(data)` | Get the first clinic that still has appointments needing reads | 724 | -| `getReadingClinics(data, [options])` | Get all clinics available for reading, enriched with unit, location, and reading status | 735 | -| `getReadableAppointmentsForClinic(data, clinicId)` | Get readable appointments for a clinic with pre-calculated metadata | 767 | -| `filterAppointmentsByEligibleForReading(appointments)` | Filter appointments that are eligible for reading | 798 | -| `filterAppointmentsByNeedsAnyRead(appointments, maxReadsPerAppointment)` | Filter appointments that need any read (first or second) | 807 | -| `filterAppointmentsByNeedsFirstRead(appointments)` | Filter appointments that need a first read | 821 | -| `filterAppointmentsByNeedsSecondRead(appointments)` | Filter appointments that need a second read | 831 | -| `filterAppointmentsByFullyRead(appointments, requiredReads)` | Filter appointments that are fully read (have all required reads) | 841 | -| `filterAppointmentsByUserCanRead(appointments, userId)` | Filter appointments that a specific user can read | 855 | -| `filterAppointmentsByUserCanReadOrHasRead(appointments, userId, [options])` | Filter appointments that user can read or has already read | 866 | -| `filterAppointmentsByClinic(appointments, clinicId)` | Filter appointments for a specific clinic | 897 | -| `filterAppointmentsByDayRange(appointments, minDays, [maxDays])` | Filter appointments that are within a specific day range | 908 | -| `getFirstAppointmentInList(appointments)` | Get the first appointment from an array | 928 | -| `getNextAppointmentInList(appointments, currentAppointmentId, wrap)` | Get the next appointment after a specific appointment | 937 | -| `getPreviousAppointmentInList(appointments, currentAppointmentId, wrap)` | Get the previous appointment before a specific appointment | 958 | -| `getReadForUser(appointment, [userId])` | Get the read object for a specific user on an appointment | 983 | -| `getFirstUserReadableAppointment(appointments, userId)` | Get first appointment from an array that a user can read | 1000 | -| `getNextUserReadableAppointment(appointments, currentAppointmentId, [userId])` | Get the next appointment the user can read after the current appointment, wrapping to start if needed | 1015 | -| `getResumeAppointmentForUser(appointments, [userId], [skippedAppointments])` | Get the appointment the user should resume reading from. | 1038 | -| `userHasReadAppointment(appointment, userId)` | Check if a user has already read an appointment | 1090 | -| `getOtherReads(appointment, userId)` | Get reads from other users (not the current user) | 1109 | -| `areReadsDiscordant(readA, readB)` | Determine if two reads are discordant (disagree in a clinically meaningful way). | 1130 | -| `willGoToArbitration(readA, readB, [settings])` | Determine whether two reads will result in arbitration, taking the site's | 1187 | -| `getOutcome(appointment, [settings])` | Compute the overall outcome for an appointment based on its reads and site policy. | 1215 | -| `getComparisonInfo(appointment, secondReadData, [userId], [settings])` | Determine if a comparison page should be shown to the second reader. | 1255 | -| `shouldShowComparePage(appointment, secondReadData, [userId], [settings])` | Decide whether the compare page should be shown to the second reader. | 1320 | -| `isDeferred(appointment)` | Check if an appointment has been deferred from reading | 1377 | -| `hasReads(appointment)` | Check if an appointment has any reads | 1424 | -| `needsFirstRead(appointment)` | Check if an appointment needs a first read | 1437 | -| `needsSecondRead(appointment)` | Check if an appointment needs a second read | 1447 | -| `needsArbitration()` | Check if an appointment needs arbitration. | 1455 | -| `getEligibleCandidatesForSession(data, sessionOptions)` | Get eligible appointment candidates for a session based on its type and filters | 1495 | -| `createReadingSession(data, options, options.type, [options.name], [options.clinicId], [options.sessionId], [options.limit], [options.filters])` | Create a session of appointments for reading based on specified criteria | 1559 | -| `getDefaultSessionName(type, clinicId, data)` | Generate a default name for a session based on its type | 1648 | -| `generateSessionId()` | Generate a unique ID for a session | 1683 | -| `getReadingSession(data, sessionId)` | Get a reading session by ID | 1692 | -| `getFirstReadableAppointmentInSession(data, sessionId, [userId])` | Get the first appointment in a session that a user can read | 1729 | -| `skipAppointmentInSession(data, sessionId, appointmentId)` | Mark an appointment as skipped in a session | 1755 | -| `topUpSession(data, sessionId)` | Add the next eligible appointment to a session if it is under its target size | 1778 | -| `getSessionReadingProgress(data, sessionId, currentAppointmentId, [userId])` | Get reading progress for a session | 1828 | +| `getAppointmentReadingMetadata(data, appointment)` | Get the reading metadata for an appointment's case | 45 | +| `writeReading(data, appointment, userId, reading, [sessionId])` | Save a user's read of an appointment's images, and take the appointment off | 56 | +| `getEpisodeReadingStatus(data, episode, [userId])` | Get the reading status of an episode. | 105 | +| `getDeferredCases(data)` | Every case currently deferred from reading, most recently deferred first. | 129 | +| `getResolvedDeferrals(data)` | Every deferral that has since been resolved, most recently resolved first. | 155 | +| `enhanceAppointmentsWithReadingData(data, appointments, participants, userId)` | Enhance appointments with their reading case and pre-calculated metadata. | 204 | +| `getReadingStatusForAppointments(data, appointments, [userId])` | Get detailed reading status for a group of appointments | 398 | +| `getReadingProgress(data, appointments, currentAppointmentId, skippedAppointments, [userId])` | Get progress through reading a set of appointments | 445 | +| `sortAppointmentsByScreeningDate(appointments)` | Sort appointments by screening date (oldest first) | 556 | +| `getFirstAvailableClinic(data)` | Get the first clinic that still has appointments needing reads | 576 | +| `getReadingClinics(data, [options])` | Get all clinics available for reading, enriched with unit, location, and reading status | 587 | +| `getReadableAppointmentsForClinic(data, clinicId)` | Get readable appointments for a clinic with pre-calculated metadata | 623 | +| `filterAppointmentsByEligibleForReading(appointments)` | Filter appointments that are eligible for reading | 655 | +| `filterAppointmentsByNeedsAnyRead(data, appointments, maxReadsPerCase)` | Filter appointments that need any read (first or second) | 664 | +| `filterAppointmentsByNeedsFirstRead(data, appointments)` | Filter appointments that need a first read | 679 | +| `filterAppointmentsByNeedsSecondRead(data, appointments)` | Filter appointments that need a second read | 692 | +| `filterAppointmentsByFullyRead(data, appointments, requiredReads)` | Filter appointments that are fully read (have all required reads) | 705 | +| `filterAppointmentsByUserCanRead(data, appointments, userId)` | Filter appointments that a specific user can read | 720 | +| `filterAppointmentsByUserCanReadOrHasRead(data, appointments, userId, [options])` | Filter appointments that user can read or has already read | 734 | +| `filterAppointmentsByClinic(appointments, clinicId)` | Filter appointments for a specific clinic | 767 | +| `filterAppointmentsByDayRange(appointments, minDays, [maxDays])` | Filter appointments that are within a specific day range | 778 | +| `getFirstAppointmentInList(appointments)` | Get the first appointment from an array | 798 | +| `getNextAppointmentInList(appointments, currentAppointmentId, wrap)` | Get the next appointment after a specific appointment | 807 | +| `getPreviousAppointmentInList(appointments, currentAppointmentId, wrap)` | Get the previous appointment before a specific appointment | 828 | +| `getFirstUserReadableAppointment(data, appointments, userId)` | Get first appointment from an array that a user can read | 853 | +| `getNextUserReadableAppointment(data, appointments, currentAppointmentId, [userId])` | Get the next appointment the user can read after the current appointment, wrapping to start if needed | 873 | +| `getResumeAppointmentForUser(data, appointments, [userId], [skippedAppointments])` | Get the appointment the user should resume reading from. | 898 | +| `userHasReadAppointment(data, appointment, [userId])` | Check if a user has already read an appointment's images | 959 | +| `canUserReadAppointment(data, appointment, [userId], [options])` | Check if a user can read an appointment's images. | 980 | +| `getEligibleCandidatesForSession(data, sessionOptions)` | Get eligible appointment candidates for a session based on its type and filters | 1046 | +| `createReadingSession(data, options, options.type, [options.name], [options.clinicId], [options.sessionId], [options.limit], [options.filters])` | Create a session of appointments for reading based on specified criteria | 1110 | +| `getDefaultSessionName(type, clinicId, data)` | Generate a default name for a session based on its type | 1199 | +| `generateSessionId()` | Generate a unique ID for a session | 1234 | +| `getReadingSession(data, sessionId)` | Get a reading session by ID | 1243 | +| `getFirstReadableAppointmentInSession(data, sessionId, [userId])` | Get the first appointment in a session that a user can read | 1280 | +| `skipAppointmentInSession(data, sessionId, appointmentId)` | Mark an appointment as skipped in a session | 1308 | +| `topUpSession(data, sessionId)` | Add the next eligible appointment to a session if it is under its target size | 1331 | +| `getSessionReadingProgress(data, sessionId, currentAppointmentId, [userId])` | Get reading progress for a session | 1381 | ### prior-mammograms.js diff --git a/scripts/generate-utils-reference.js b/scripts/generate-utils-reference.js index 6eb1f959..01f77883 100644 --- a/scripts/generate-utils-reference.js +++ b/scripts/generate-utils-reference.js @@ -45,9 +45,14 @@ const FILE_META = { label: 'clinics.js', description: 'Clinic filtering by time period, slot formatting, and opening hours calculation.' }, + 'app/lib/utils/reading-cases.js': { + label: 'reading-cases.js', + description: + 'A reading case is one set of mammograms being read, held on the episode as episode.readingCases[]. Pure case logic: reads, read types, case state and outcome, discordance and arbitration, deferral. Everything here takes a case — resolve one from an appointment with getReadingCase in episodes.js.' + }, 'app/lib/utils/reading.js': { label: 'reading.js', - description: 'Image reading workflow: read state, progress tracking, batch management, per-user navigation, and filtering. The main module for anything related to image reading.' + description: 'Image reading workflow: read state, progress tracking, batch management, per-user navigation, and filtering. The appointment- and session-shaped layer over reading cases.' }, 'app/lib/utils/prior-mammograms.js': { label: 'prior-mammograms.js', @@ -120,6 +125,7 @@ const UTILS_FILES = [ 'app/lib/utils/appointment-data.js', 'app/lib/utils/episodes.js', 'app/lib/utils/clinics.js', + 'app/lib/utils/reading-cases.js', 'app/lib/utils/reading.js', 'app/lib/utils/prior-mammograms.js', 'app/lib/utils/medical-information.js', From 1f3334bd0932800539947b01499d60ef68ffff2e Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 16:07:52 +0100 Subject: [PATCH 03/13] Stop resume filling a lazy session to its target size /resume looped topUpSession until it stopped adding, to replace dead slots - cases fully read by other readers, which can leave a session with nothing readable while the backlog still has cases. But topUpSession grows a session towards its target size, so running it to exhaustion loaded every remaining case at once and defeated lazy sessions entirely. Top up only until a readable case appears. Normally there already is one, so nothing is added. Pre-existing, not from the reading cases move. --- app/routes/reading.js | 23 ++++++++++++++++++++--- docs/testing.md | 3 ++- tests/e2e/reading.spec.js | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/app/routes/reading.js b/app/routes/reading.js index 6ca32d64..aee307d8 100644 --- a/app/routes/reading.js +++ b/app/routes/reading.js @@ -368,11 +368,28 @@ module.exports = (router) => { return res.redirect('/reading') } - // Fill any dead slots (appointments fully read by others) before looking for the - // next readable case. topUpSession adds one appointment at a time so loop until - // it can no longer add anything. + // A session can end up with nothing readable in it — every loaded case + // taken by other readers — while the backlog still has cases waiting. Top + // up until one readable case appears, and no further. + // + // Not "until topUpSession stops adding": it grows the session towards its + // target size, so running it to exhaustion here would load every remaining + // case at once and defeat lazy sessions. The bound is a backstop only. const maxTopUps = session.targetSize || 25 for (let count = 0; count < maxTopUps; count++) { + const loadedAppointments = session.appointmentIds + .map((appointmentId) => + data.appointments.find((e) => e.id === appointmentId) + ) + .filter(Boolean) + + const hasReadableCase = getFirstUserReadableAppointment( + data, + loadedAppointments, + data.currentUser.id + ) + if (hasReadableCase) break + if (!topUpSession(data, sessionId)) break } diff --git a/docs/testing.md b/docs/testing.md index 995a2954..51e1c1ab 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -38,7 +38,7 @@ name, and the skip list is printed on every run so it stays visible. **Journeys** (`tests/e2e/`) drive real flows in Chromium via Playwright. This is the only layer that exercises POST handlers, session state and the client-side -JavaScript. Six journeys: +JavaScript. Seven journeys: - a screening appointment recording medical history and a symptom, from check-in to completion @@ -48,6 +48,7 @@ JavaScript. Six journeys: - a technical recall, through its views-to-retake form and the review step - deferring a case, then unflagging it from the deferred cases page - a second reader reaching the comparison page and keeping their opinion +- a lazy session staying lazy across leaving and resuming it Between them the reading journeys cover all four ways a reader can leave a case — normal, recall for assessment, technical recall and deferral — which is what diff --git a/tests/e2e/reading.spec.js b/tests/e2e/reading.spec.js index 0d044df8..3afdc377 100644 --- a/tests/e2e/reading.spec.js +++ b/tests/e2e/reading.spec.js @@ -206,6 +206,38 @@ test.describe('Image reading', () => { await expect(page.getByText(deferralReason)).toBeVisible() }) + test('keeps a lazy session lazy across a resume', async ({ page }) => { + // Lazy sessions load one case at a time, so the overview should only ever + // show what has actually been reached. Leaving and resuming used to load + // every remaining case at once, which made the whole backlog look claimed. + await pinSettings(page, readingSettings) + + await page.goto( + '/reading/create-session?type=all_reads&limit=10&lazy=true' + ) + await expect(page).toHaveURL(/\/reading\/session\/[^/]+\/appointments\//) + + const sessionId = page.url().split('/session/')[1].split('/')[0] + + // Rows for cases the session hasn't reached yet are rendered as + // placeholders, so count the ones that link to a real case + const caseRows = page.locator( + '.app-reading-session-table tbody tr:not(.app-placeholder-row) a[href*="/appointments/"]' + ) + + await page.goto(`/reading/session/${sessionId}/all-reads`) + const rowsBefore = await caseRows.count() + expect(rowsBefore).toBeLessThanOrEqual(2) + + // Leave, come back through the dashboard, and resume + await page.goto('/reading') + await page.goto(`/reading/session/${sessionId}/resume`) + await expect(page).toHaveURL(/\/appointments\//) + + await page.goto(`/reading/session/${sessionId}/all-reads`) + await expect(caseRows).toHaveCount(rowsBefore) + }) + test('shows the second reader the first read before saving', async ({ page }) => { From ffbad9df979c42cf8030d48345491202e2edc811 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 16:21:52 +0100 Subject: [PATCH 04/13] Seed a summary reading case for each past round Past rounds recorded an outcome but nothing about how they got there, so an episode page for a previous round had no reading to show. Each screened historic episode now carries one reading case in the same shape as a live one, with appointmentId null - there is no appointment record behind a past round. Reads are conditioned on the outcome the round was already seeded with: a round ending in treatment was recalled for assessment, a clear round was usually read as normal, and a minority were recalled and then cleared at assessment. checkEpisodes enforces the coherence. Also fixes the e2e seed helper building full names without middle names, which made the appointment journey fail whenever it happened to pick a participant who had one. --- app/lib/generate-seed-data.js | 16 ++- app/lib/generators/episode-generator.js | 139 ++++++++++++++++++++++-- app/views/episodes/show.html | 36 +++++- docs/data-conventions.md | 16 ++- tests/e2e/helpers/seed-data.js | 7 +- 5 files changed, 193 insertions(+), 21 deletions(-) diff --git a/app/lib/generate-seed-data.js b/app/lib/generate-seed-data.js index 55d1a8b4..0c42b4ae 100644 --- a/app/lib/generate-seed-data.js +++ b/app/lib/generate-seed-data.js @@ -290,12 +290,14 @@ const generateSnapshotPeriod = (startDate, numberOfDays) => { * @param {Array} episodes - The generated (real) episodes * @param {Array} participants - All participants * @param {object} seedDataProfile - Active seed profile + * @param {Array} readers - Users who could have read these past rounds * @returns {Array} Historic episodes */ const generateHistoricEpisodesForParticipants = ( episodes, participants, - seedDataProfile + seedDataProfile, + readers ) => { const max = config.generation.maxHistoricEpisodesPerParticipant const outcomeWeights = seedDataProfile?.episodes?.historicOutcomeWeights @@ -324,7 +326,8 @@ const generateHistoricEpisodesForParticipants = ( type: earliest.type, earliestOpenedDate: earliest.openedDate, max, - outcomeWeights + outcomeWeights, + readers }) ) }) @@ -445,8 +448,12 @@ const seedTechnicalRecallRescreen = ({ readAt.add(1, 'day').toISOString(), { forceOpinion: 'technical_recall' } ) + // Built against an empty case, because this replaces whatever reads it had + // rather than adding to them - buildRead types a read from where the case + // had got to, and against the existing reads this would come out as an + // arbitration read const firstRead = buildRead( - readingCase, + { ...readingCase, reads: [] }, firstReader.id, firstReader.role, generatedRead, @@ -638,7 +645,8 @@ const generateData = async (options = {}) => { const historicEpisodes = generateHistoricEpisodesForParticipants( allEpisodes, finalParticipants, - selectedSeedDataProfile + selectedSeedDataProfile, + users ) const episodesWithHistory = [...allEpisodes, ...historicEpisodes] diff --git a/app/lib/generators/episode-generator.js b/app/lib/generators/episode-generator.js index b3e3f534..24f064ef 100644 --- a/app/lib/generators/episode-generator.js +++ b/app/lib/generators/episode-generator.js @@ -55,6 +55,87 @@ const ASSESSMENT_OUTCOME_WEIGHTS = { refer_for_treatment: 0.2 } +// How often a past round that ended in routine recall got there via assessment +// rather than straight from a clear reading. Most clear rounds were simply read +// as normal; a minority were recalled and then found to be clear. Without this +// every historic recall would have ended in treatment, which is the opposite of +// how screening actually goes. +const HISTORIC_RECALLED_THEN_CLEAR_PROBABILITY = 0.06 + +/** + * Build the summary reading case for a past round. + * + * Past rounds are seeded outcome-first, so the reads are chosen to be + * consistent with the outcome already picked rather than the other way round: + * a round that ended in treatment must have been recalled for assessment, and a + * clear round was usually - though not always - read as normal. + * + * Deliberately lean. There is no appointment behind a past round, so no image + * set for annotations to point at; what a past round can honestly say is who + * read it, when, and what they concluded. + * + * @param {object} options + * @param {string} options.outcome - The round's episode outcome + * @param {object} options.screenedDate - When the images were taken (dayjs) + * @param {object} options.closedDate - When the round closed (dayjs) + * @param {Array} options.readers - Users available to have read it + * @returns {object | null} The reading case, or null if the round wasn't screened + */ +const buildHistoricReadingCase = ({ + outcome, + screenedDate, + closedDate, + readers +}) => { + if (outcome === 'no_result' || readers.length < 2) return null + + // A round ending in treatment was recalled for assessment; a clear round + // usually wasn't, but sometimes was and the assessment found nothing + const wasRecalled = + outcome === 'refer_for_treatment' || + Math.random() < HISTORIC_RECALLED_THEN_CLEAR_PROBABILITY + + const opinion = wasRecalled ? 'recall_for_assessment' : 'normal' + + // Two readers who agreed - a past round that needed arbitration is a story we + // have no way to tell honestly at summary fidelity + const [firstReader, secondReader] = faker.helpers.arrayElements(readers, 2) + + // Read in the window between the images being taken and the round closing + const firstReadAt = screenedDate.add( + faker.number.int({ min: 1, max: 4 }), + 'day' + ) + const secondReadAt = firstReadAt.add( + faker.number.int({ min: 0, max: 2 }), + 'day' + ) + const cappedSecondReadAt = secondReadAt.isAfter(closedDate) + ? closedDate + : secondReadAt + + const buildSummaryRead = (reader, readType, readNumber, timestamp) => ({ + opinion, + readerId: reader.id, + readerType: reader.role, + readType, + readNumber, + timestamp: timestamp.toISOString() + }) + + return { + id: generateId(), + // A past round has no appointment record for the case to hang off - the + // same reason its mammogram entry carries no appointmentId + appointmentId: null, + openedDate: screenedDate.toISOString(), + reads: [ + buildSummaryRead(firstReader, 'first', 1, firstReadAt), + buildSummaryRead(secondReader, 'second', 2, cappedSecondReadAt) + ] + } +} + /** * Record a stage change, keeping stageHistory in step * @@ -276,10 +357,11 @@ const countHistoricEpisodes = ( /** * Generate summary-level episodes for a participant's past screening rounds. * - * Outcome-first: each round says what it found, and we don't model how it got - * there - no appointments, no reads, no assessment detail. That is enough for - * every "what happened before" view, and cheap to hold. If we later model the - * steps, the outcome can be computed from them instead. + * Outcome-first: each round says what it found, and the detail beneath it is + * chosen to be consistent with that - not the other way round. A past round + * holds no appointment and no assessment detail, but it does carry a summary + * reading case, so "who read this and what did they say" is answerable for + * every round rather than only the current one. * * Spacing follows the risk level's own screening interval (routine every 3 * years, family history / high risk yearly). @@ -290,6 +372,7 @@ const countHistoricEpisodes = ( * @param {string|Date} options.earliestOpenedDate - Opened date of their oldest real episode * @param {number} options.max - Cap on how many to generate * @param {object} [options.outcomeWeights] - Override the default outcome mix + * @param {Array} [options.readers] - Users who could have read these rounds * @returns {Array} Historic episodes, oldest first */ const generateHistoricEpisodes = ({ @@ -297,7 +380,8 @@ const generateHistoricEpisodes = ({ type, earliestOpenedDate, max, - outcomeWeights + outcomeWeights, + readers = [] }) => { const riskLevel = riskLevels[type] || riskLevels.routine const weights = outcomeWeights || HISTORIC_OUTCOME_WEIGHTS @@ -329,6 +413,13 @@ const generateHistoricEpisodes = ({ // A round with no result never produced images either const wasScreened = outcome !== 'no_result' + const readingCase = buildHistoricReadingCase({ + outcome, + screenedDate, + closedDate, + readers + }) + episodes.push({ id: generateId(), participantId: participant.id, @@ -346,7 +437,7 @@ const generateHistoricEpisodes = ({ openedDate: openedDate.toISOString(), closedDate: closedDate.toISOString(), appointmentIds: [], - readingCases: [], + readingCases: readingCase ? [readingCase] : [], isHistoric: true, // Enough to list this round as a prior without holding a full image @@ -420,11 +511,39 @@ const checkEpisodes = (episodes, appointmentsById) => { `historic episode ${episode.id} outcome and mammograms disagree` ) } - // A summary round records that it was read, not how - it has no - // appointment for a case to hang off - if (episode.readingCases?.length) { - problems.push(`historic episode ${episode.id} has reading cases`) + // A screened past round carries exactly one summary reading case, and a + // round that produced no images has nothing to read + const historicCases = episode.readingCases || [] + if (wasScreened !== (historicCases.length === 1)) { + problems.push( + `historic episode ${episode.id} should have exactly one reading case when screened` + ) } + + historicCases.forEach((readingCase) => { + // No appointment record exists behind a past round + if (readingCase.appointmentId) { + problems.push( + `historic reading case ${readingCase.id} references an appointment` + ) + } + // The reads have to agree with the outcome the round was seeded with: + // a round ending in treatment must have been recalled for assessment + const readingOutcome = getReadingCaseOutcome(readingCase, {}) + if ( + episode.outcome === 'refer_for_treatment' && + readingOutcome !== 'recall_for_assessment' + ) { + problems.push( + `historic episode ${episode.id} ended in treatment but was read as "${readingOutcome}"` + ) + } + if (getReadsAsArray(readingCase).length !== 2) { + problems.push( + `historic reading case ${readingCase.id} does not have two reads` + ) + } + }) return } diff --git a/app/views/episodes/show.html b/app/views/episodes/show.html index 7a5f47ce..57cdc1c2 100644 --- a/app/views/episodes/show.html +++ b/app/views/episodes/show.html @@ -41,7 +41,8 @@

{% if episode.isHistoric %} {# Past rounds are held as summaries - what the round concluded, not how - it got there. There are no appointment or reading records to show. #} + it got there. There is no appointment record, but the round does carry a + summary of its reading. #}
Information: @@ -99,6 +100,39 @@

{{ summaryList({ rows: rows }) }} + {# What the round's reading concluded. A summary case records who read it + and what they said, but no annotations - there is no image set behind a + past round for them to point at. #} + {% set historicCase = episode.readingCases | first %} + {% if historicCase %} + +

Image reading

+ + {% set readingRows = [ + { + key: { text: "Reading outcome" }, + value: { html: (historicCase | getReadingCaseOutcome(data.settings)) | toTag } + } + ] %} + + {% for read in historicCase | getReadsAsArray %} + {% set readValueHtml %} + {{ read.opinion | toTag }} + + {{ read.readerId | getUsername({ format: "short", identifyCurrentUser: true }) }}, + {{ read.timestamp | formatDate }} + + {% endset %} + {% set readingRows = readingRows | push({ + key: { text: read.readType | sentenceCase + " read" }, + value: { html: readValueHtml } + }) %} + {% endfor %} + + {{ summaryList({ rows: readingRows }) }} + + {% endif %} + {% else %} {{ summaryList({ diff --git a/docs/data-conventions.md b/docs/data-conventions.md index 7d55cd64..0d4286d1 100644 --- a/docs/data-conventions.md +++ b/docs/data-conventions.md @@ -226,10 +226,18 @@ Participants who have a real episode also get their past rounds as **historic** episodes (`isHistoric: true`): summary-level records with dates and an outcome, but no appointments, no reads and no assessment detail. -They are seeded **outcome-first** - we say what the round found and don't model -how it got there. That's enough for any "what happened before" view, and cheap -to hold. If we later model the steps, the outcome can be computed from them -instead, without the record changing shape. +They are seeded **outcome-first**: the round's outcome is picked, and the detail +beneath it is chosen to be consistent with that rather than the other way round. +A past round carries one **summary reading case** - who read it, when, and what +they concluded - with `appointmentId: null`, since there is no appointment +record behind it for the case to hang off (the same reason its mammogram entry +has no `appointmentId`). + +The reads have to agree with the outcome: a round that ended in +`refer_for_treatment` was recalled for assessment, and a clear round was usually +read as normal - though a minority were recalled and then found clear at +assessment, which is what actually happens in screening. `checkEpisodes` +enforces the first of those. How many a participant gets follows from their **age** and their screening interval, since screening starts at the risk level's lower age bound: a routine diff --git a/tests/e2e/helpers/seed-data.js b/tests/e2e/helpers/seed-data.js index 90672b87..8b502e5d 100644 --- a/tests/e2e/helpers/seed-data.js +++ b/tests/e2e/helpers/seed-data.js @@ -63,13 +63,16 @@ const findTodayAppointment = ({ status = 'scheduled', index = 0 } = {}) => { const participant = participants.find( (item) => item.id === appointment.participantId ) - const { firstName, lastName } = participant.demographicInformation + const { firstName, middleName, lastName } = participant.demographicInformation return { clinic, appointment, participant, - fullName: `${firstName} ${lastName}` + // Built the same way getFullName does, middle name included - some + // participants have one, and which participant a run picks depends on the + // seed data, so a first+last name only matches some of the time + fullName: [firstName, middleName, lastName].filter(Boolean).join(' ') } } From 8dd1af44500165e3063196d6ae92435cab051a9a Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 16:36:15 +0100 Subject: [PATCH 05/13] Give past rounds a stand-in appointment, and let seed profiles reach them A past round recorded an outcome and, since the last commit, its reading - but nothing about what was recorded on the day. Each screened historic episode now carries episode.summaryAppointment: the round's date and the medical information taken at it. It sits on the episode rather than in data.appointments on purpose. Nothing links to it yet, and keeping it out of the appointments collection means it can't drift into a clinic list, a reading queue, or a route expecting a real appointment record. Recorded dates are stamped back to the round's screening date. The medical information generators date everything "now", which is right for a live appointment and wrong for a past one. Seed profiles now also shape history: episodes.historicOutcomeWeights was a hook nothing set, so "all normals" still produced past referrals for treatment. It now defaults explicitly and is overridden by allNormals and highAbnormalities. Historic medical information follows the profile's probabilities too. --- app/lib/generate-seed-data.js | 3 +- app/lib/generators/episode-generator.js | 77 ++++++++++++++++++++++++- app/lib/generators/seed-profiles.js | 27 +++++++++ app/views/episodes/show.html | 14 +++++ docs/data-conventions.md | 20 +++++++ 5 files changed, 139 insertions(+), 2 deletions(-) diff --git a/app/lib/generate-seed-data.js b/app/lib/generate-seed-data.js index 0c42b4ae..7cc668be 100644 --- a/app/lib/generate-seed-data.js +++ b/app/lib/generate-seed-data.js @@ -327,7 +327,8 @@ const generateHistoricEpisodesForParticipants = ( earliestOpenedDate: earliest.openedDate, max, outcomeWeights, - readers + readers, + seedProfile: seedDataProfile }) ) }) diff --git a/app/lib/generators/episode-generator.js b/app/lib/generators/episode-generator.js index 24f064ef..7809fcc4 100644 --- a/app/lib/generators/episode-generator.js +++ b/app/lib/generators/episode-generator.js @@ -15,6 +15,9 @@ const weighted = require('weighted') const { faker } = require('@faker-js/faker') const generateId = require('../utils/id-generator') const riskLevels = require('../../data/risk-levels') +const { + generateMedicalInformation +} = require('./medical-information-generator') const { buildReadingCase, getLatestReadingCase, @@ -62,6 +65,69 @@ const ASSESSMENT_OUTCOME_WEIGHTS = { // how screening actually goes. const HISTORIC_RECALLED_THEN_CLEAR_PROBABILITY = 0.06 +/** + * Build the stand-in appointment for a past round. + * + * A past round has no appointment record - reviving one in full is what the + * episodes work deliberately dropped, because a whole historic clinic snapshot + * cost far more than it was worth. This is the fidelity tier in between: enough + * to show what an appointment that day looked like, held on the episode rather + * than in `data.appointments`, so it can never drift into a clinic list, a + * reading queue or a route that expects a real appointment. + * + * Where and when the round was screened already lives in `episode.mammograms`; + * what this adds is the medical information recorded at the time. + * + * @param {object} options + * @param {object} options.screenedDate - When the images were taken (dayjs) + * @param {object} [options.seedProfile] - Active seed profile + * @returns {object} The summary appointment + */ +const buildHistoricSummaryAppointment = ({ screenedDate, seedProfile }) => { + const medicalInformation = generateMedicalInformation({ + ...(seedProfile?.medicalInformation || {}) + }) + + return { + startTime: screenedDate.toISOString(), + medicalInformation: stampRecordedDates(medicalInformation, screenedDate) + } +} + +/** + * Re-date everything in a generated medical information record to when it was + * actually recorded. + * + * The medical information generators stamp `dateAdded` with the current time, + * which is right for a live appointment and wrong for a past round - it would + * show a symptom from 2023 as having been recorded today. Rewriting them here + * keeps that fix to historic generation, rather than threading a date through + * every generator that the live flow also uses. + * + * Walks the whole structure rather than naming fields, so it keeps working as + * the medical information shape grows. + * + * @param {*} value - Generated medical information, or part of it + * @param {object} recordedDate - When the round was screened (dayjs) + * @returns {*} The same shape, with recorded dates moved back + */ +const stampRecordedDates = (value, recordedDate) => { + if (Array.isArray(value)) { + return value.map((item) => stampRecordedDates(item, recordedDate)) + } + + if (!value || typeof value !== 'object') return value + + return Object.fromEntries( + Object.entries(value).map(([key, item]) => [ + key, + key === 'dateAdded' + ? recordedDate.toISOString() + : stampRecordedDates(item, recordedDate) + ]) + ) +} + /** * Build the summary reading case for a past round. * @@ -373,6 +439,7 @@ const countHistoricEpisodes = ( * @param {number} options.max - Cap on how many to generate * @param {object} [options.outcomeWeights] - Override the default outcome mix * @param {Array} [options.readers] - Users who could have read these rounds + * @param {object} [options.seedProfile] - Active seed profile * @returns {Array} Historic episodes, oldest first */ const generateHistoricEpisodes = ({ @@ -381,7 +448,8 @@ const generateHistoricEpisodes = ({ earliestOpenedDate, max, outcomeWeights, - readers = [] + readers = [], + seedProfile }) => { const riskLevel = riskLevels[type] || riskLevels.routine const weights = outcomeWeights || HISTORIC_OUTCOME_WEIGHTS @@ -420,6 +488,12 @@ const generateHistoricEpisodes = ({ readers }) + // A round that produced no images had no screening appointment worth + // standing in for either + const summaryAppointment = wasScreened + ? buildHistoricSummaryAppointment({ screenedDate, seedProfile }) + : null + episodes.push({ id: generateId(), participantId: participant.id, @@ -438,6 +512,7 @@ const generateHistoricEpisodes = ({ closedDate: closedDate.toISOString(), appointmentIds: [], readingCases: readingCase ? [readingCase] : [], + summaryAppointment, isHistoric: true, // Enough to list this round as a prior without holding a full image diff --git a/app/lib/generators/seed-profiles.js b/app/lib/generators/seed-profiles.js index f9f531db..0f7ed145 100644 --- a/app/lib/generators/seed-profiles.js +++ b/app/lib/generators/seed-profiles.js @@ -33,6 +33,16 @@ const SEED_DATA_PROFILE_DEFAULTS = { imageReading: { probabilityFirstReaderOpinionMatchesImages: 0.95 }, + episodes: { + // How past rounds turned out. Overriding this is what stops a profile's + // claim about the current round being contradicted by a participant's + // history - 'all normals' shouldn't show past referrals for treatment. + historicOutcomeWeights: { + routine_recall: 0.9, + refer_for_treatment: 0.03, + no_result: 0.07 + } + }, reading: { // null = no limit (use default clinic-pattern behaviour) // 0 = empty backlog, n = limit backlog to n cases @@ -216,6 +226,14 @@ const SEED_DATA_PROFILE_DEFINITIONS = [ previousMammograms: { rate: 0 }, + // Nothing was ever found, in this round or any before it + episodes: { + historicOutcomeWeights: { + routine_recall: 0.93, + refer_for_treatment: 0, + no_result: 0.07 + } + }, imageSetSelection: { contextualTagWeights: { default: { @@ -233,6 +251,15 @@ const SEED_DATA_PROFILE_DEFINITIONS = [ label: 'High abnormalities', description: 'High chance of abnormalities in selected images', settings: { + // A population with abnormal current rounds plausibly has a history of + // them too, so past rounds find more as well + episodes: { + historicOutcomeWeights: { + routine_recall: 0.78, + refer_for_treatment: 0.15, + no_result: 0.07 + } + }, imageSetSelection: { contextualTagWeights: { default: { diff --git a/app/views/episodes/show.html b/app/views/episodes/show.html index 57cdc1c2..c3400fc3 100644 --- a/app/views/episodes/show.html +++ b/app/views/episodes/show.html @@ -100,6 +100,20 @@

{{ summaryList({ rows: rows }) }} + {# What was recorded on the day. A past round holds no appointment record, + so this stands in for one - enough to show what an appointment then + looked like, without pretending to be a real one you can open. #} + {% if episode.summaryAppointment %} + +

Medical information recorded

+ + {# The shared summary reads `appointment.medicalInformation`, and the + stand-in carries the same shape, so it can be reused as-is #} + {% set appointment = episode.summaryAppointment %} + {% include "_includes/summary-lists/medical-info-summary.njk" %} + + {% endif %} + {# What the round's reading concluded. A summary case records who read it and what they said, but no annotations - there is no image set behind a past round for them to point at. #} diff --git a/docs/data-conventions.md b/docs/data-conventions.md index 0d4286d1..03101f75 100644 --- a/docs/data-conventions.md +++ b/docs/data-conventions.md @@ -239,6 +239,26 @@ read as normal - though a minority were recalled and then found clear at assessment, which is what actually happens in screening. `checkEpisodes` enforces the first of those. +A screened past round also carries `episode.summaryAppointment` - a stand-in for +the appointment we don't model, holding the round's date and the medical +information recorded at it: + +```js +{ startTime, medicalInformation } +``` + +It lives on the episode rather than in `data.appointments` deliberately, so it +can't drift into a clinic list, a reading queue or a route expecting a real +appointment. Its recorded dates are stamped back to the round's screening date - +the medical information generators date everything "now", which is right for a +live appointment and wrong for a past one. + +**Seed profiles reach history too.** `episodes.historicOutcomeWeights` shapes how +past rounds turned out, so a profile's claim about the current round isn't +contradicted by the participant's past - "all normals" shows no past referrals, +"high abnormalities" shows more. Medical information for past rounds uses the +profile's `medicalInformation` probabilities, the same as a live appointment. + How many a participant gets follows from their **age** and their screening interval, since screening starts at the risk level's lower age bound: a routine participant aged 51 has none, at 54 has one, at 69 has six. From 7eee74d4bfc0f6cbdc9467ae8f17c1b0f0474115 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 16:51:47 +0100 Subject: [PATCH 06/13] Show an appointments list on past rounds, and medical info on open ones Past rounds held their stand-in appointment as a single object, and showed it only as a block of medical information. It is now episode.summaryAppointments[] - a list, like the episode's other per-appointment records - and the episode page renders it as an appointments table the same way a live round does, with "Summary only" where a live round has a link. An episode can hold more than one appointment, and a past one should have somewhere to show that. Medical information was only surfaced on historic episodes, which left the two kinds of episode page inconsistent. Open episodes now show it too, from the latest appointment that recorded any. Also fixes a flake in the reading spec: the annotation modal scrolls via .app-modal__content and the dialog itself is overflow: clip, so Playwright could not bring Save into view on a long form. The test now scrolls the content area the way a user would. --- app/lib/generators/episode-generator.js | 30 ++++++++-- app/views/episodes/show.html | 76 +++++++++++++++++++++++-- docs/data-conventions.md | 22 ++++--- tests/e2e/reading.spec.js | 7 +++ 4 files changed, 116 insertions(+), 19 deletions(-) diff --git a/app/lib/generators/episode-generator.js b/app/lib/generators/episode-generator.js index 7809fcc4..095ab806 100644 --- a/app/lib/generators/episode-generator.js +++ b/app/lib/generators/episode-generator.js @@ -76,7 +76,12 @@ const HISTORIC_RECALLED_THEN_CLEAR_PROBABILITY = 0.06 * reading queue or a route that expects a real appointment. * * Where and when the round was screened already lives in `episode.mammograms`; - * what this adds is the medical information recorded at the time. + * what this adds is the appointment's own detail - its status, and the medical + * information recorded at it. + * + * Held as a list, like the episode's other per-appointment records, so a past + * round is shaped the same as a live one. Only one is generated today; a past + * technical recall would have had two. * * @param {object} options * @param {object} options.screenedDate - When the images were taken (dayjs) @@ -89,6 +94,12 @@ const buildHistoricSummaryAppointment = ({ screenedDate, seedProfile }) => { }) return { + id: generateId(), + // A past round was screened - that is why it has images and a reading. The + // status is carried explicitly so the episode page can show an appointments + // list the same way a live round does. + status: 'complete', + type: 'screening', startTime: screenedDate.toISOString(), medicalInformation: stampRecordedDates(medicalInformation, screenedDate) } @@ -490,9 +501,9 @@ const generateHistoricEpisodes = ({ // A round that produced no images had no screening appointment worth // standing in for either - const summaryAppointment = wasScreened - ? buildHistoricSummaryAppointment({ screenedDate, seedProfile }) - : null + const summaryAppointments = wasScreened + ? [buildHistoricSummaryAppointment({ screenedDate, seedProfile })] + : [] episodes.push({ id: generateId(), @@ -512,7 +523,7 @@ const generateHistoricEpisodes = ({ closedDate: closedDate.toISOString(), appointmentIds: [], readingCases: readingCase ? [readingCase] : [], - summaryAppointment, + summaryAppointments, isHistoric: true, // Enough to list this round as a prior without holding a full image @@ -586,6 +597,15 @@ const checkEpisodes = (episodes, appointmentsById) => { `historic episode ${episode.id} outcome and mammograms disagree` ) } + // A screened past round has a stand-in appointment; one that produced no + // images was never screened, so has none + const summaryAppointments = episode.summaryAppointments || [] + if (wasScreened !== (summaryAppointments.length > 0)) { + problems.push( + `historic episode ${episode.id} summary appointments and outcome disagree` + ) + } + // A screened past round carries exactly one summary reading case, and a // round that produced no images has nothing to read const historicCases = episode.readingCases || [] diff --git a/app/views/episodes/show.html b/app/views/episodes/show.html index c3400fc3..768bbd32 100644 --- a/app/views/episodes/show.html +++ b/app/views/episodes/show.html @@ -100,16 +100,57 @@

{{ summaryList({ rows: rows }) }} - {# What was recorded on the day. A past round holds no appointment record, - so this stands in for one - enough to show what an appointment then - looked like, without pretending to be a real one you can open. #} - {% if episode.summaryAppointment %} + {# A past round holds stand-ins rather than appointment records, so the list + is shaped the same as a live round's but has nothing to open. Showing it + as a list rather than a single block is deliberate: an episode can hold + more than one appointment, and a past one is no different. #} +

Appointments

+ + {% if episode.summaryAppointments.length %} + + + + + + + + + + + {% for summaryAppointment in episode.summaryAppointments %} + + + + + + + {% endfor %} + +
DateTypeStatusActions
+ {{ summaryAppointment.startTime | formatDate }} + + {{ summaryAppointment.type | formatWords | sentenceCase }} + + {{ tag({ + text: summaryAppointment.status | getStatusText('appointment'), + colour: summaryAppointment.status | getStatusTagColour('appointment') + }) }} + + Summary only +
+ {% else %} +

This round was not screened, so it has no appointments.

+ {% endif %} + + {# What was recorded on the day #} + {% set latestSummaryAppointment = episode.summaryAppointments | last %} + {% if latestSummaryAppointment %}

Medical information recorded

{# The shared summary reads `appointment.medicalInformation`, and the stand-in carries the same shape, so it can be reused as-is #} - {% set appointment = episode.summaryAppointment %} + {% set appointment = latestSummaryAppointment %} {% include "_includes/summary-lists/medical-info-summary.njk" %} {% endif %} @@ -217,6 +258,31 @@

Appointments

+ {# What was recorded at the appointment, the same as a past round shows. + The latest appointment that got as far as recording anything: an + episode can hold more than one, and a booked-but-not-yet-attended one + has nothing to show. #} + {% set appointmentsWithInfo = [] %} + {% for row in episodeAppointments %} + {% if row.appointment.medicalInformation %} + {% set appointmentsWithInfo = appointmentsWithInfo | push(row.appointment) %} + {% endif %} + {% endfor %} + {% set latestWithInfo = appointmentsWithInfo | last %} + + {% if latestWithInfo %} +

Medical information recorded

+ + {% if appointmentsWithInfo | length > 1 %} +

+ As recorded at the {{ latestWithInfo.timing.startTime | formatDate }} appointment. +

+ {% endif %} + + {% set appointment = latestWithInfo %} + {% include "_includes/summary-lists/medical-info-summary.njk" %} + {% endif %} +

Image reading

Reading state across this episode’s appointments: diff --git a/docs/data-conventions.md b/docs/data-conventions.md index 03101f75..00a06d1b 100644 --- a/docs/data-conventions.md +++ b/docs/data-conventions.md @@ -239,19 +239,23 @@ read as normal - though a minority were recalled and then found clear at assessment, which is what actually happens in screening. `checkEpisodes` enforces the first of those. -A screened past round also carries `episode.summaryAppointment` - a stand-in for -the appointment we don't model, holding the round's date and the medical -information recorded at it: +A screened past round also carries `episode.summaryAppointments[]` - stand-ins +for the appointment records we don't model: ```js -{ startTime, medicalInformation } +{ id, status, type, startTime, medicalInformation } ``` -It lives on the episode rather than in `data.appointments` deliberately, so it -can't drift into a clinic list, a reading queue or a route expecting a real -appointment. Its recorded dates are stamped back to the round's screening date - -the medical information generators date everything "now", which is right for a -live appointment and wrong for a past one. +A list, like the episode's other per-appointment records, so a past round is +shaped the same as a live one and the episode page can show an appointments +table either way. Only one is generated today; a past technical recall would +have had two. + +They live on the episode rather than in `data.appointments` deliberately, so +they can't drift into a clinic list, a reading queue or a route expecting a real +appointment - there is nothing to open. Their recorded dates are stamped back to +the round's screening date; the medical information generators date everything +"now", which is right for a live appointment and wrong for a past one. **Seed profiles reach history too.** `episodes.historicOutcomeWeights` shapes how past rounds turned out, so a profile's claim about the current round isn't diff --git a/tests/e2e/reading.spec.js b/tests/e2e/reading.spec.js index 3afdc377..e5ad7614 100644 --- a/tests/e2e/reading.spec.js +++ b/tests/e2e/reading.spec.js @@ -113,6 +113,13 @@ test.describe('Image reading', () => { 'label[for="modal-imageReadingTemp[annotationTemp][levelOfConcern]-4"]' ) .click() + // The dialog is `overflow: clip` and scrolls via .app-modal__content, so + // Playwright's auto-scroll can't bring Save into view on a long form - it + // reports the button as visible but outside the viewport. Scroll the + // content area the way a user would, then click. + await annotationModal + .locator('.app-modal__content') + .evaluate((element) => element.scrollTo(0, element.scrollHeight)) await annotationModal.getByRole('button', { name: 'Save' }).first().click() await expectModalClosed(annotationModal) From dd005b4af10b38cd47106818e54a12c07472b05c Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 17:10:39 +0100 Subject: [PATCH 07/13] Seed historic arbitration, link episode rows, clear snags A few percent of past rounds now went to arbitration - the two readers disagreed and a third settled it - so the arbitration work has seeded examples rather than only cases made by hand. The deciding read carries the round's conclusion, and checkEpisodes allows the third read. Episode pages show reading as a table of cases, each linking to the case view the reading backlog work will serve, rather than listing individual reads. The reads belong on the case view. Past appointments link too; both are dead links alluding to pages that don't exist yet. Snags cleared: deleted the unused reading/batch.html, renamed the seed profile's `imageReading` group to `reads` (nothing in the data model is called imageReading any more), and removed the arbitration and newly-arrived-priors counts in index-simple.html that were computed and never displayed. Two e2e fragilities fixed, both intermittent and both pre-existing: the normal opinion helper only handled cases without symptoms, and controls low in a modal could not be scrolled into view - now a shared revealInModal helper. --- app/lib/generators/episode-generator.js | 75 ++- app/lib/generators/reading-generator.js | 2 +- app/lib/generators/seed-profiles.js | 5 +- app/routes/settings.js | 4 +- .../episodes/reading-cases-table.njk | 49 ++ app/views/episodes/show.html | 60 +- app/views/reading/batch.html | 564 ------------------ app/views/reading/index-simple.html | 25 +- app/views/settings/seed-profiles/custom.html | 4 +- docs/data-conventions.md | 5 + tests/e2e/appointment.spec.js | 6 + tests/e2e/helpers/modals.js | 22 + tests/e2e/reading.spec.js | 45 +- 13 files changed, 214 insertions(+), 652 deletions(-) create mode 100644 app/views/_includes/episodes/reading-cases-table.njk delete mode 100644 app/views/reading/batch.html diff --git a/app/lib/generators/episode-generator.js b/app/lib/generators/episode-generator.js index 095ab806..bc4f4fb3 100644 --- a/app/lib/generators/episode-generator.js +++ b/app/lib/generators/episode-generator.js @@ -65,6 +65,12 @@ const ASSESSMENT_OUTCOME_WEIGHTS = { // how screening actually goes. const HISTORIC_RECALLED_THEN_CLEAR_PROBABILITY = 0.06 +// How often a past round's two readers disagreed and it went to arbitration. +// A few percent of cases is the right order for real screening - the point of +// seeding any is that arbitration work has past examples to look at, not only +// cases created by hand. +const HISTORIC_ARBITRATION_PROBABILITY = 0.04 + /** * Build the stand-in appointment for a past round. * @@ -172,11 +178,17 @@ const buildHistoricReadingCase = ({ outcome === 'refer_for_treatment' || Math.random() < HISTORIC_RECALLED_THEN_CLEAR_PROBABILITY + // What the round concluded at reading - the arbitration read where there was + // one, otherwise what both readers agreed const opinion = wasRecalled ? 'recall_for_assessment' : 'normal' - // Two readers who agreed - a past round that needed arbitration is a story we - // have no way to tell honestly at summary fidelity - const [firstReader, secondReader] = faker.helpers.arrayElements(readers, 2) + // Sometimes the two readers disagreed and a third settled it. Needs three + // readers: nobody reads the same case twice, arbitration included + const wentToArbitration = + readers.length >= 3 && Math.random() < HISTORIC_ARBITRATION_PROBABILITY + + const [firstReader, secondReader, arbitrationReader] = + faker.helpers.arrayElements(readers, wentToArbitration ? 3 : 2) // Read in the window between the images being taken and the round closing const firstReadAt = screenedDate.add( @@ -191,8 +203,8 @@ const buildHistoricReadingCase = ({ ? closedDate : secondReadAt - const buildSummaryRead = (reader, readType, readNumber, timestamp) => ({ - opinion, + const buildSummaryRead = (reader, readType, readNumber, timestamp, readOpinion) => ({ + opinion: readOpinion, readerId: reader.id, readerType: reader.role, readType, @@ -200,16 +212,55 @@ const buildHistoricReadingCase = ({ timestamp: timestamp.toISOString() }) + // When it went to arbitration the two readers disagreed, so one of them said + // the opposite of what the round concluded. Which one is arbitrary - either + // reader could have been the one who saw something + const disagreeingOpinion = + opinion === 'normal' ? 'recall_for_assessment' : 'normal' + const firstReaderDisagreed = wentToArbitration && Math.random() < 0.5 + + const reads = [ + buildSummaryRead( + firstReader, + 'first', + 1, + firstReadAt, + firstReaderDisagreed ? disagreeingOpinion : opinion + ), + buildSummaryRead( + secondReader, + 'second', + 2, + cappedSecondReadAt, + wentToArbitration && !firstReaderDisagreed ? disagreeingOpinion : opinion + ) + ] + + if (wentToArbitration) { + // The arbitration read is what settled it, so it carries the round's + // conclusion - and dates after the two it was called in to resolve + const arbitratedAt = cappedSecondReadAt.add( + faker.number.int({ min: 1, max: 3 }), + 'day' + ) + reads.push( + buildSummaryRead( + arbitrationReader, + 'arbitration', + 3, + arbitratedAt.isAfter(closedDate) ? closedDate : arbitratedAt, + opinion + ) + ) + } + return { id: generateId(), // A past round has no appointment record for the case to hang off - the // same reason its mammogram entry carries no appointmentId appointmentId: null, openedDate: screenedDate.toISOString(), - reads: [ - buildSummaryRead(firstReader, 'first', 1, firstReadAt), - buildSummaryRead(secondReader, 'second', 2, cappedSecondReadAt) - ] + reads } } @@ -633,9 +684,11 @@ const checkEpisodes = (episodes, appointmentsById) => { `historic episode ${episode.id} ended in treatment but was read as "${readingOutcome}"` ) } - if (getReadsAsArray(readingCase).length !== 2) { + // Two readers, or three where they disagreed and one arbitrated + const historicReadCount = getReadsAsArray(readingCase).length + if (historicReadCount !== 2 && historicReadCount !== 3) { problems.push( - `historic reading case ${readingCase.id} does not have two reads` + `historic reading case ${readingCase.id} has ${historicReadCount} reads` ) } }) diff --git a/app/lib/generators/reading-generator.js b/app/lib/generators/reading-generator.js index 119a3192..8ec7948b 100644 --- a/app/lib/generators/reading-generator.js +++ b/app/lib/generators/reading-generator.js @@ -504,7 +504,7 @@ const generateReadingDataWithBacklogLimit = ( */ const generateReadingData = (appointments, users, episodes, seedProfile = {}) => { const alignmentProbability = - seedProfile?.imageReading?.probabilityFirstReaderOpinionMatchesImages ?? + seedProfile?.reads?.probabilityFirstReaderOpinionMatchesImages ?? DEFAULT_ALIGNMENT_PROBABILITY if (!appointments || !appointments.length || !users || users.length < 2) { console.log('No appointments or not enough users to generate reading data') diff --git a/app/lib/generators/seed-profiles.js b/app/lib/generators/seed-profiles.js index 0f7ed145..c9f45527 100644 --- a/app/lib/generators/seed-profiles.js +++ b/app/lib/generators/seed-profiles.js @@ -30,7 +30,10 @@ const mergeDeep = (base, override) => { } const SEED_DATA_PROFILE_DEFAULTS = { - imageReading: { + // How generated reads behave. Named `reads` rather than `imageReading` - + // nothing in the data model is called imageReading any more, and the reads + // themselves are what this shapes. + reads: { probabilityFirstReaderOpinionMatchesImages: 0.95 }, episodes: { diff --git a/app/routes/settings.js b/app/routes/settings.js index dd151a24..791c3c5c 100644 --- a/app/routes/settings.js +++ b/app/routes/settings.js @@ -45,7 +45,7 @@ const convertPercentageOverrides = (overrides, fallbackProfile) => { const getCustomOverridesFromBody = (body = {}, fallbackProfile = {}) => { const rawOverrides = { - imageReading: body.imageReading, + reads: body.reads, medicalInformation: body.medicalInformation, specialAppointment: body.specialAppointment, previousMammograms: body.previousMammograms, @@ -54,7 +54,7 @@ const getCustomOverridesFromBody = (body = {}, fallbackProfile = {}) => { } const converted = convertPercentageOverrides(rawOverrides, { - imageReading: fallbackProfile.imageReading, + reads: fallbackProfile.reads, medicalInformation: fallbackProfile.medicalInformation, specialAppointment: fallbackProfile.specialAppointment, previousMammograms: fallbackProfile.previousMammograms, diff --git a/app/views/_includes/episodes/reading-cases-table.njk b/app/views/_includes/episodes/reading-cases-table.njk new file mode 100644 index 00000000..cf8a4722 --- /dev/null +++ b/app/views/_includes/episodes/reading-cases-table.njk @@ -0,0 +1,49 @@ +{# app/views/_includes/episodes/reading-cases-table.njk + + One row per reading case - one set of images read. Shared by past and live + rounds, because a case is the same thing either way; only the fidelity of + what sits behind it differs. + + The individual reads deliberately aren't here. They belong on the case view, + which is where a reader goes to see who said what. #} + + + + + + + + + + + + + {% for readingCase in episode.readingCases %} + {% set caseState = readingCase | getReadingCaseState(data.settings) %} + {% set caseOutcome = readingCase | getReadingCaseOutcome(data.settings) %} + + + + + + + + {% endfor %} + +
Images takenReadsStateOutcomeActions
+ {{ readingCase.openedDate | formatDate }} + + {{ (readingCase | getReadsAsArray) | length }} + + {{ caseState | formatWords | sentenceCase }} + + {% if caseOutcome %} + {{ caseOutcome | toTag }} + {% else %} + Not yet + {% endif %} + + {# Alludes to the reading case view - the URL the reading backlog + work will serve. Nothing answers it yet. #} + View case +
diff --git a/app/views/episodes/show.html b/app/views/episodes/show.html index 768bbd32..7bb682fa 100644 --- a/app/views/episodes/show.html +++ b/app/views/episodes/show.html @@ -132,7 +132,11 @@

Appointments

}) }} - Summary only + {# Alludes to the page a past appointment would have. Nothing + serves it yet - a past round has no appointment record. #} + + View appointment + {% endfor %} @@ -155,37 +159,11 @@

Medical information recorded

{% endif %} - {# What the round's reading concluded. A summary case records who read it - and what they said, but no annotations - there is no image set behind a - past round for them to point at. #} - {% set historicCase = episode.readingCases | first %} - {% if historicCase %} - -

Image reading

- - {% set readingRows = [ - { - key: { text: "Reading outcome" }, - value: { html: (historicCase | getReadingCaseOutcome(data.settings)) | toTag } - } - ] %} - - {% for read in historicCase | getReadsAsArray %} - {% set readValueHtml %} - {{ read.opinion | toTag }} - - {{ read.readerId | getUsername({ format: "short", identifyCurrentUser: true }) }}, - {{ read.timestamp | formatDate }} - - {% endset %} - {% set readingRows = readingRows | push({ - key: { text: read.readType | sentenceCase + " read" }, - value: { html: readValueHtml } - }) %} - {% endfor %} - - {{ summaryList({ rows: readingRows }) }} - + {# One row per set of images read. The individual reads belong on the case + view rather than here - this is the round's shape, not its detail. #} + {% if episode.readingCases.length %} +

Image reading

+ {% include "_includes/episodes/reading-cases-table.njk" %} {% endif %} {% else %} @@ -284,13 +262,17 @@

Medical information recorded

{% endif %}

Image reading

-

- Reading state across this episode’s appointments: - {{ tag({ - text: readingStatus.status | formatWords | sentenceCase, - colour: readingStatus.statusColor - }) }} -

+ {% if episode.readingCases.length %} + {% include "_includes/episodes/reading-cases-table.njk" %} + {% else %} +

+ Reading state across this episode’s appointments: + {{ tag({ + text: readingStatus.status | formatWords | sentenceCase, + colour: readingStatus.statusColor + }) }} +

+ {% endif %} {% else %}

This episode has no appointments.

diff --git a/app/views/reading/batch.html b/app/views/reading/batch.html deleted file mode 100644 index 9bbb83a5..00000000 --- a/app/views/reading/batch.html +++ /dev/null @@ -1,564 +0,0 @@ -{# /app/views/reading/batch.html #} - -{% extends 'layout-app.html' %} - -{% set isClinicContext = batch.clinicId %} - -{% if isClinicContext %} - {% set back = { - href: "/reading/clinics", - text: "Back to clinics" - } %} -{% endif %} - -{% set pageHeading %} - {{ batch.name }} -{% endset %} - -{% set gridColumn = "none" %} - -{% block pageContent %} - -{{ batch | log("Batch data") }} -{{ readingStatus | log("Reading status") }} -{{ appointments | log("Appointments in batch") }} - -
-
-
- Image reading -

- {{ pageHeading }} -

- {% set isClinicContext = batch.clinicId %} - {% if isClinicContext %} -
- {{ readingStatus.status | formatWords | sentenceCase | toTag }} -
- {% endif %} -
-
-
-
-
- - {% set currentUserHasRead = readingStatus.userReadCount > 0 %} - {% set readingActionText = "Resume reading" if currentUserHasRead else "Start reading" %} - - {% if resumeAppointment %} - - {{ readingActionText }} - - {% else %} - {% set sessionCompletePanelHtml %} -

An opinion has been provided for {{ readingStatus.userReadCount }} {{ "case" | pluralise(readingStatus.userReadCount) }}. The first opinion will be automatically finalised in {{ autoFinaliseMinutesRemaining }} {{ "minute" | pluralise(autoFinaliseMinutesRemaining) }}. - Finalise opinions now

-
- {{ button({ - text: "Start a new session", - href: "/reading", - variant: "reverse", - classes: "nhsuk-u-margin-bottom-0" - }) }} - Return to reading dashboard -
- {% endset %} - - {{ panel({ - titleText: "Session complete", - html: sessionCompletePanelHtml - }) }} - {% endif %} - - {# Secondary navigation for view tabs #} - {% set secondaryNavItems = [] %} - - {% for item in [ - { id: 'your-reads', label: 'Your opinion' }, - { id: 'all-reads', label: 'All reads and outcomes' } - ] %} - {% set href -%} - /reading/batch/{{ batch.id }}/{{ item.id }} - {% endset %} - {% set secondaryNavItems = secondaryNavItems | push({ - text: item.label, - href: href | trim, - current: true if item.id == view - }) %} - {% endfor %} - - {{ appSecondaryNavigation({ - visuallyHiddenTitle: "Batch view modes", - items: secondaryNavItems - }) }} - - {# Number of slots not yet populated in a lazy batch #} - {% set pendingCount = (batch.targetSize - batch.appointmentIds | length) if batch.targetSize else 0 %} - {% set pendingCount = 0 if pendingCount < 0 else pendingCount %} - - {% if view == 'your-reads' %} - {# YOUR READS VIEW - Shows cases from user's perspective #} - {% set userReadableAppointments = [] %} - {% for appointment in appointments %} - {% if appointment | canUserReadAppointment or appointment | userHasReadAppointment or appointment | userRequestedPriors(data.currentUser.id) %} - {% set userReadableAppointments = userReadableAppointments | push(appointment) %} - {% endif %} - {% endfor %} - - {% if userReadableAppointments | length == 0 %} -

You don't have any cases you can read in this batch.

- {% else %} - - {# Build opinion counts from appointments the user has read #} - {% set normalAppointments = [] %} - {% set technicalRecallAppointments = [] %} - {% set recallForAssessmentAppointments = [] %} - {% set priorsRequestedAppointments = [] %} - - {% for appointment in userReadableAppointments %} - {% if appointment | userHasReadAppointment %} - {% set read = appointment | getReadForUser %} - {% if read.opinion == 'normal' %} - {% set normalAppointments = normalAppointments | push(appointment) %} - {% elseif read.opinion == 'technical_recall' %} - {% set technicalRecallAppointments = technicalRecallAppointments | push(appointment) %} - {% elseif read.opinion == 'recall_for_assessment' %} - {% set recallForAssessmentAppointments = recallForAssessmentAppointments | push(appointment) %} - {% endif %} - {% elseif appointment | userRequestedPriors(data.currentUser.id) %} - {% set priorsRequestedAppointments = priorsRequestedAppointments | push(appointment) %} - {% endif %} - {% endfor %} - - {% set totalCount = userReadableAppointments | length %} - {% set normalCount = normalAppointments | length %} - {% set technicalRecallCount = technicalRecallAppointments | length %} - {% set recallForAssessmentCount = recallForAssessmentAppointments | length %} - {% set priorsRequestedCount = priorsRequestedAppointments | length %} - - {% set normalPercent = (normalCount / totalCount * 100) | round %} - {% set technicalRecallPercent = (technicalRecallCount / totalCount * 100) | round %} - {% set recallForAssessmentPercent = (recallForAssessmentCount / totalCount * 100) | round %} - {% set priorsRequestedPercent = (priorsRequestedCount / totalCount * 100) | round %} - - {% set normalCardContent %} -

- - Normal: {{ normalPercent }}% - -

-

Normal

-

{{ normalCount }} cases

- {% endset %} - - {% set technicalRecallCardContent %} -

- - Technical recall: {{ technicalRecallPercent }}% - -

-

Technical recall

-

{{ technicalRecallCount }} cases

- {% endset %} - - {% set recallForAssessmentCardContent %} -

- - Recall for assessment: {{ recallForAssessmentPercent }}% - -

-

Recall for assessment

-

{{ recallForAssessmentCount }} cases

- {% endset %} - - {% set priorsRequestedCardContent %} -

- - Priors requested: {{ priorsRequestedPercent }}% - -

-

Priors requested

-

{{ priorsRequestedCount }} cases

- {% endset %} - - {% if not resumeAppointment %} -

Opinion summary

- -
    -
  • - {{ card({ - descriptionHtml: normalCardContent - }) }} -
    • -
  • - {{ card({ - descriptionHtml: technicalRecallCardContent - }) }} -
    • -
  • - {{ card({ - descriptionHtml: recallForAssessmentCardContent - }) }} -
    • -
  • - {{ card({ - descriptionHtml: priorsRequestedCardContent - }) }} -
    • -
- {% endif %} - -

Reading session cases

- {% set userSessionRemainingCount = batch.targetSize - readingStatus.userReadCount - readingStatus.userAwaitingPriorsCount %} - {% set userSessionRemainingCount = 0 if userSessionRemainingCount < 0 else userSessionRemainingCount %} - {% if userSessionRemainingCount > 0 or readingStatus.userAwaitingPriorsCount > 0 %} -

Progress: {{ readingStatus.userReadCount }} read{%- if readingStatus.userAwaitingPriorsCount > 0 -%}, {{ readingStatus.userAwaitingPriorsCount }} awaiting priors{%- endif -%}, {{ userSessionRemainingCount }} remaining

- {% endif %} - - - - - - - - - - - - - {% for appointment in userReadableAppointments %} - {% set metadata = appointment | getReadingMetadata %} - - - - - - - - {% endfor %} - - {# Placeholder rows for pending lazy-batch slots #} - {% set placeholderRowsShown = 3 if pendingCount > 3 else pendingCount %} - {% for i in range(0, placeholderRowsShown) %} - - - - - - - - {% endfor %} - -
No.CaseScreening dateYour opinionAction
- {{ loop.index }}. - - {% set readingHref -%} - /reading/batch/{{ batch.id }}/appointments/{{ appointment.id }} - {%- endset %} - {% if appointment.medicalInformation.symptoms | length %} - {{ "Has symptoms" | toTag }} - {% endif %} - {{ appointment.participant | getFullName }} - -
- - {% set readCount = metadata.readCount %} - {% if appointment | userHasReadAppointment %} - {{ "1st read" if readCount == 1 else "2nd read" }} - {% else %} - {{ "1st read" if readCount == 0 else "2nd read" }} - {% endif %} - - {% if (data.settings.debugMode | falsify) %} - {% if appointment | hasRecordedMammograms %} -
{{ "Has priors" | toTag }} - {% endif %} - {% if appointment.mammogramData.metadata.hasAdditionalImages %} -
{{ "Has additional" | toTag }} - {% endif %} - {% if appointment.mammogramData.isImperfectButBestPossible | includes("yes") %} -
{{ "Imperfect" | toTag }} - {% endif %} - {% if appointment.mammogramData.isIncompleteMammography | includes("yes") %} -
{{ "Incomplete" | toTag }} - {% endif %} - {% endif %} - 		- {% set daysSinceScreening = appointment.timing.startTime | daysSince %} - {% if daysSinceScreening >= data.config.reading.urgentThreshold %} - {{ "Urgent" | toTag }}
- {% elseif daysSinceScreening >= data.config.reading.priorityThreshold %} - {{ "Due soon" | toTag }} -
- {% endif %} - {{ appointment.timing.startTime | formatDate }}
- - {{ appointment.timing.startTime | formatRelativeDate }} - - 		- {% if appointment | userHasReadAppointment %} - {% set read = appointment | getReadForUser %} - {% if read.opinion %} - {{ read.opinion | toTag }} - {% endif %} - {% elseif appointment | userRequestedPriors(data.currentUser.id) %} - {{ "Priors requested" | toTag }} - {% elseif batch.skippedAppointments | includes(appointment.id) %} - {{ "Skipped" | toTag }} - {% else %} - {{ "Not read" | toTag }} - {% endif %} - - {% if resumeAppointment and appointment.id == resumeAppointment.id and not (appointment | userHasReadAppointment) %} - {{ readingActionText }} - {% endif %} -
- {{ (userReadableAppointments | length) + loop.index }}. - - - {% if pendingCount > 3 and loop.last %} - and {{ pendingCount - 3 }} more in this batch - {% endif %} -
- {% endif %} - {% else %} - {# ALL READS VIEW - Shows outcome stats and all cases #} - - {# Build outcome counts for stats cards #} - {% set awaitingSecondReadAppointments = [] %} - {% set allViewNormalAppointments = [] %} - {% set allViewTechnicalRecallAppointments = [] %} - {% set allViewRecallForAssessmentAppointments = [] %} - {% set allViewArbitrationAppointments = [] %} - - {% for appointment in appointments %} - {% set appointmentOutcome = appointment | getOutcome %} - {% if appointmentOutcome == 'pending_second_read' %} - {% set awaitingSecondReadAppointments = awaitingSecondReadAppointments | push(appointment) %} - {% elseif appointmentOutcome == 'normal' %} - {% set allViewNormalAppointments = allViewNormalAppointments | push(appointment) %} - {% elseif appointmentOutcome == 'technical_recall' %} - {% set allViewTechnicalRecallAppointments = allViewTechnicalRecallAppointments | push(appointment) %} - {% elseif appointmentOutcome == 'recall_for_assessment' %} - {% set allViewRecallForAssessmentAppointments = allViewRecallForAssessmentAppointments | push(appointment) %} - {% elseif appointmentOutcome == 'arbitration_pending' %} - {% set allViewArbitrationAppointments = allViewArbitrationAppointments | push(appointment) %} - {% endif %} - {% endfor %} - - {% set allTotal = appointments | length %} - {% set allTotalDenominator = allTotal if allTotal > 0 else 1 %} - {% set awaitingCount = awaitingSecondReadAppointments | length %} - {% set allViewNormalCount = allViewNormalAppointments | length %} - {% set allViewTechnicalRecallCount = allViewTechnicalRecallAppointments | length %} - {% set allViewRecallForAssessmentCount = allViewRecallForAssessmentAppointments | length %} - {% set allViewArbitrationCount = allViewArbitrationAppointments | length %} - {% set twiceReadTotal = allViewNormalCount + allViewTechnicalRecallCount + allViewRecallForAssessmentCount + allViewArbitrationCount %} - {% set twiceReadDenominator = twiceReadTotal if twiceReadTotal > 0 else 1 %} - - {% set awaitingCardContent %} -

- - Awaiting 2nd read: {{ (awaitingCount / allTotalDenominator * 100) | round }}% - -

-

Awaiting 2nd read

-

{{ awaitingCount }} cases

- {% endset %} - - {% set allViewNormalCardContent %} -

- - Normal: {{ (allViewNormalCount / twiceReadDenominator * 100) | round }}% - -

-

Normal

-

{{ allViewNormalCount }} cases

- {% endset %} - - {% set allViewTechnicalRecallCardContent %} -

- - Technical recall: {{ (allViewTechnicalRecallCount / twiceReadDenominator * 100) | round }}% - -

-

Technical recall

-

{{ allViewTechnicalRecallCount }} cases

- {% endset %} - - {% set allViewRecallForAssessmentCardContent %} -

- - Recall for assessment: {{ (allViewRecallForAssessmentCount / twiceReadDenominator * 100) | round }}% - -

-

Recall for assessment

-

{{ allViewRecallForAssessmentCount }} cases

- {% endset %} - - {% set allViewArbitrationCardContent %} -

- - Require arbitration: {{ (allViewArbitrationCount / twiceReadDenominator * 100) | round }}% - -

-

Require arbitration

-

{{ allViewArbitrationCount }} cases

- {% endset %} - -

Reading session cases

- - - - - - - - - - - - - - {% for appointment in appointments %} - {% set metadata = appointment | getReadingMetadata %} - - - - - {# 1st read column #} - - {# 2nd read column #} - - {# Outcome column — uses getOutcome which respects the site's arbitration policy #} - - - {% endfor %} - - {# Placeholder rows for pending lazy-batch slots #} - {% set placeholderRowsShown = 3 if pendingCount > 3 else pendingCount %} - {% for i in range(0, placeholderRowsShown) %} - - - - - - - - - {% endfor %} - -
No.CaseScreening date1st read2nd readOutcome
- {{ loop.index }}. - - {% set readingHref -%} - /reading/batch/{{ batch.id }}/appointments/{{ appointment.id }} - {%- endset %} - {% if appointment.medicalInformation.symptoms | length %} - {{ "Has symptoms" | toTag }} - {% endif %} - {{ appointment.participant | getFullName }} - - {% if appointment | awaitingPriors %} -
- {{ "Awaiting priors" | toTag }} - {% endif %} - {% if (data.settings.debugMode | falsify) %} - {% if appointment | hasRecordedMammograms %} -
{{ "Has priors" | toTag }} - {% endif %} - {% if appointment.mammogramData.metadata.hasAdditionalImages %} -
{{ "Has additional" | toTag }} - {% endif %} - {% if appointment.mammogramData.isImperfectButBestPossible | includes("yes") %} -
{{ "Imperfect" | toTag }} - {% endif %} - {% if appointment.mammogramData.isIncompleteMammography | includes("yes") %} -
{{ "Incomplete" | toTag }} - {% endif %} - {% endif %} - 		- {% set daysSinceScreening = appointment.timing.startTime | daysSince %} - {% if daysSinceScreening >= data.config.reading.urgentThreshold %} - {{ "Urgent" | toTag }} -
- {% elseif daysSinceScreening >= data.config.reading.priorityThreshold %} - {{ "Due soon" | toTag }} -
- {% endif %} - {{ appointment.timing.startTime | formatDate }}
- - {{ appointment.timing.startTime | formatRelativeDate }} - - 		- {% set isBlind = data.settings.reading.blindReading | falsify %} - {% set isCurrentUserRead = false %} - {% set allReads = appointment | getReadsAsArray %} - - {% if metadata.readCount >= 1 %} - {% set firstRead = allReads[0] %} - {% set isCurrentUserRead = firstRead.readerId === data.currentUser.id %} - - {% if isBlind and not isCurrentUserRead and metadata.readCount < 2 %} - {{ "Completed (blind)" | toTag }} - {% else %} - {# Override default colour and make sure it's always grey #} - {{ firstRead.opinion | toTag({colour: "grey"}) }} - {% endif %} - -
- by {{ firstRead.readerId | getUsername({ - format: 'short', - identifyCurrentUser: true - }) }} - {% elif batch.skippedAppointments | includes(appointment.id) %} - {{ "Skipped" | toTag }} - {% else %} - {{ "Not read" | toTag }} - {% endif %} - 		- {% set isBlind = data.settings.reading.blindReading | falsify %} - {% set isCurrentUserRead = false %} - - {% if metadata.readCount >= 2 %} - {% set secondRead = allReads[1] %} - {% set isCurrentUserRead = secondRead.readerId === data.currentUser.id %} - {# Force tags to grey #} - {{ secondRead.opinion | toTag({colour: "grey"}) }} -
- by {{ secondRead.readerId | getUsername({ - format: 'short', - identifyCurrentUser: true - }) }} - {% elif metadata.readCount == 1 %} - {% if batch.skippedAppointments | includes(appointment.id) %} - {{ "Skipped" | toTag }} - {% else %} - {{ "Not read" | toTag }} - {% endif %} - {% else %} - {{ "Waiting for 1st read" | toTag }} - {% endif %} - 		- {% set outcome = appointment | getOutcome %} - {% if outcome == 'not_read' %} - {{ "Waiting for 1st read" | toTag }} - {% elseif outcome == 'pending_second_read' %} - {{ "Waiting for 2nd read" | toTag }} - {% elseif outcome == 'arbitration_pending' %} - {{ "Arbitration" | toTag }} - {% else %} - {{ outcome | toTag }} - {% endif %} -
- {{ (appointments | length) + loop.index }}. - - - {% if pendingCount > 3 and loop.last %} - and {{ pendingCount - 3 }} more in this batch - {% endif %} -
- {% endif %} -
-
-{% endblock %} diff --git a/app/views/reading/index-simple.html b/app/views/reading/index-simple.html index e728e5bd..665ef5f8 100644 --- a/app/views/reading/index-simple.html +++ b/app/views/reading/index-simple.html @@ -55,24 +55,9 @@ {% endif %} {% endfor %} -{# - Inset counts. - - arbitrationCount is derived from real data — cases whose derived state is - arbitration_required. - - newlyArrivedPriorsCount is faked for the prototype. In a real implementation - this would be the count of cases where priors the reader requested or was - waiting on have since arrived. Edit the value below (or set it to 0) to see - the empty state of the inset. -#} -{% set arbitrationCount = 0 %} -{% for thisAppointment in data.appointments %} - {% if ((data | getReadingCase(thisAppointment)) | getReadingCaseState(data.settings)) == 'arbitration_required' %} - {% set arbitrationCount = arbitrationCount + 1 %} - {% endif %} -{% endfor %} - +{# Inset counts. Only the ones an inset actually shows are worked out here - + an arbitration count and a faked newly-arrived-priors count used to be + calculated and then never rendered. #} {% set deferredCount = 0 %} {% for thisAppointment in data.appointments %} {% if (data | getReadingCase(thisAppointment)) | isCaseDeferred %} @@ -87,10 +72,6 @@ {% endif %} {% endfor %} -{% set newlyArrivedPriorsCount = 2 %} - -{% set actionTotal = arbitrationCount + newlyArrivedPriorsCount %} - {# Backlog counts — all eligible cases that still need reading, banded by age relative to the configured priority/urgent thresholds. diff --git a/app/views/settings/seed-profiles/custom.html b/app/views/settings/seed-profiles/custom.html index 28c16d38..0b0674e3 100644 --- a/app/views/settings/seed-profiles/custom.html +++ b/app/views/settings/seed-profiles/custom.html @@ -46,9 +46,9 @@

Image reading

{{ seedProfilePercentageInput({ id: "probabilityFirstReaderOpinionMatchesImages", - name: "settings[seedProfiles][customForm][imageReading][probabilityFirstReaderOpinionMatchesImages]", + name: "settings[seedProfiles][customForm][reads][probabilityFirstReaderOpinionMatchesImages]", label: "First reader opinion matches images", - value: formProfile.imageReading.probabilityFirstReaderOpinionMatchesImages * 100 + value: formProfile.reads.probabilityFirstReaderOpinionMatchesImages * 100 }) }}

Reading backlog

diff --git a/docs/data-conventions.md b/docs/data-conventions.md index 00a06d1b..57859b3c 100644 --- a/docs/data-conventions.md +++ b/docs/data-conventions.md @@ -239,6 +239,11 @@ read as normal - though a minority were recalled and then found clear at assessment, which is what actually happens in screening. `checkEpisodes` enforces the first of those. +A few percent of past rounds went to **arbitration**: the two readers disagreed +and a third read settled it, carrying the round's conclusion. Those are seeded +so arbitration work has past examples to look at rather than only cases made by +hand. + A screened past round also carries `episode.summaryAppointments[]` - stand-ins for the appointment records we don't model: diff --git a/tests/e2e/appointment.spec.js b/tests/e2e/appointment.spec.js index d3f3b25a..c0e11224 100644 --- a/tests/e2e/appointment.spec.js +++ b/tests/e2e/appointment.spec.js @@ -10,6 +10,7 @@ const { pinSettings, appointmentSettings } = require('./helpers/settings') const { findTodayAppointment } = require('./helpers/seed-data') const { clickToOpenModal, + revealInModal, expectModalClosed, openSection, waitForModalsReady @@ -106,6 +107,11 @@ test.describe('Screening appointment', () => { await openSection(page, 'Symptoms') const symptomModal = await clickToOpenModal(page, 'Lump', { exact: true }) + await revealInModal( + symptomModal.locator( + 'input[name="appointment[symptomTemp][location]"][value="right breast"]' + ) + ) await symptomModal .locator( 'input[name="appointment[symptomTemp][location]"][value="right breast"]' diff --git a/tests/e2e/helpers/modals.js b/tests/e2e/helpers/modals.js index b91fafb9..f7fed294 100644 --- a/tests/e2e/helpers/modals.js +++ b/tests/e2e/helpers/modals.js @@ -79,6 +79,27 @@ const clickLinkToOpenModal = async (page, name, options = {}) => { return openFormModal(page) } +/** + * Bring a control inside a modal into view before interacting with it. + * + * The dialog is `overflow: clip` and scrolls via `.app-modal__content`, and it + * is positioned with a transform. Between them, Playwright's automatic + * scroll-into-view can report a control as visible but "outside of the + * viewport" and never manage to click it - intermittently, because whether a + * control sits below the fold depends on how much the form happens to hold. + * + * Scrolling it in explicitly is also what a real user does. + * + * @param {import('@playwright/test').Locator} target - Control inside the modal + * @returns {import('@playwright/test').Locator} The same locator, for chaining + */ +const revealInModal = async (target) => { + await target.evaluate((element) => + element.scrollIntoView({ block: 'center' }) + ) + return target +} + /** * Wait for a modal to close again after submitting * @@ -104,6 +125,7 @@ module.exports = { openFormModal, clickToOpenModal, clickLinkToOpenModal, + revealInModal, expectModalClosed, openSection } diff --git a/tests/e2e/reading.spec.js b/tests/e2e/reading.spec.js index e5ad7614..d26aef91 100644 --- a/tests/e2e/reading.spec.js +++ b/tests/e2e/reading.spec.js @@ -16,6 +16,7 @@ const { pinSettings, readingSettings } = require('./helpers/settings') const { clickToOpenModal, clickLinkToOpenModal, + revealInModal, expectModalClosed } = require('./helpers/modals') @@ -23,7 +24,12 @@ const { const sessionSize = 3 /** - * Record a normal opinion on the case currently on screen + * Record a normal opinion on the case currently on screen. + * + * A case whose participant disclosed significant symptoms offers "Normal, and + * add details" instead of a plain "Normal", and then asks the reader to + * acknowledge the symptoms. Which cases a session picks up depends on the seed + * data, so both paths have to work. * * @param {import('@playwright/test').Page} page - Playwright page */ @@ -31,7 +37,29 @@ const recordNormal = async (page) => { await expect( page.getByRole('heading', { name: 'What is your opinion of these images?' }) ).toBeVisible() - await page.getByRole('button', { name: 'Normal (N)' }).first().click() + + const plainNormal = page.getByRole('button', { name: 'Normal (N)' }).first() + + if (await plainNormal.isVisible()) { + await plainNormal.click() + return + } + + // The details route loads into the shared modal, so this is a modal flow + const detailsModal = await clickToOpenModal( + page, + 'Normal, and add details (N)' + ) + + // Symptoms have to be acknowledged before a normal opinion can be saved + await detailsModal + .locator('input[name="imageReadingTemp[symptomsAcknowledged]"][value="true"]') + .check() + await detailsModal.getByRole('button', { name: 'Continue' }).first().click() + + // The modal has to finish closing before the next case's opinion page is + // clickable - its overlay swallows pointer events while it is still open + await expectModalClosed(detailsModal) } /** @@ -113,14 +141,11 @@ test.describe('Image reading', () => { 'label[for="modal-imageReadingTemp[annotationTemp][levelOfConcern]-4"]' ) .click() - // The dialog is `overflow: clip` and scrolls via .app-modal__content, so - // Playwright's auto-scroll can't bring Save into view on a long form - it - // reports the button as visible but outside the viewport. Scroll the - // content area the way a user would, then click. - await annotationModal - .locator('.app-modal__content') - .evaluate((element) => element.scrollTo(0, element.scrollHeight)) - await annotationModal.getByRole('button', { name: 'Save' }).first().click() + const saveAnnotation = annotationModal + .getByRole('button', { name: 'Save' }) + .first() + await revealInModal(saveAnnotation) + await saveAnnotation.click() await expectModalClosed(annotationModal) await expect(page.getByText('Level 4 (suspicious)')).toBeVisible() From d43d6190834908cb8573fc5e668e6e3a24390ac2 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Tue, 28 Jul 2026 17:14:06 +0100 Subject: [PATCH 08/13] Point the placeholder episode links at nothing They allude to pages that don't exist yet, so an anchor is more honest than a speculative URL that would 404. --- app/views/_includes/episodes/reading-cases-table.njk | 6 +++--- app/views/episodes/show.html | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/views/_includes/episodes/reading-cases-table.njk b/app/views/_includes/episodes/reading-cases-table.njk index cf8a4722..97936451 100644 --- a/app/views/_includes/episodes/reading-cases-table.njk +++ b/app/views/_includes/episodes/reading-cases-table.njk @@ -39,9 +39,9 @@ {% endif %} - {# Alludes to the reading case view - the URL the reading backlog - work will serve. Nothing answers it yet. #} - View case + {# Alludes to the reading case view, which the reading backlog work + will build. Nothing serves it yet, so it goes nowhere. #} + View case {% endfor %} diff --git a/app/views/episodes/show.html b/app/views/episodes/show.html index 7bb682fa..25ecd7c0 100644 --- a/app/views/episodes/show.html +++ b/app/views/episodes/show.html @@ -133,10 +133,8 @@

Appointments

{# Alludes to the page a past appointment would have. Nothing - serves it yet - a past round has no appointment record. #} - - View appointment - + serves it yet, so it deliberately goes nowhere. #} + View appointment {% endfor %} From 1070ec00077845ba8e02089b2b3c903326624318 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Wed, 29 Jul 2026 10:18:41 +0100 Subject: [PATCH 09/13] Improve summary epsiode display --- app/routes/participants.js | 7 ++---- .../screening-episode-history.njk | 24 +++++++++---------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/app/routes/participants.js b/app/routes/participants.js index cc11cb9b..485bb0d7 100644 --- a/app/routes/participants.js +++ b/app/routes/participants.js @@ -148,11 +148,8 @@ module.exports = (router) => { return { episode, - date: - mammogramDate || - latestAppointment?.timing?.startTime || - episode.closedDate || - episode.openedDate, + screenedDate: mammogramDate || null, + closedDate: episode.closedDate || null, wasScreened: Boolean(mammogramDate) } }) diff --git a/app/views/_includes/summary-lists/screening-episode-history.njk b/app/views/_includes/summary-lists/screening-episode-history.njk index 5dd4ca32..b29affd9 100644 --- a/app/views/_includes/summary-lists/screening-episode-history.njk +++ b/app/views/_includes/summary-lists/screening-episode-history.njk @@ -10,9 +10,9 @@ - + + - @@ -22,23 +22,21 @@ {% set episode = row.episode %} - {# Mammogram location and date #} + {# Mammogram location and date, plus the reader's reason for the request #} - {# Status #} + {# Status. Pending was flagged by a reader, so shows when they + flagged it; every later status was actioned by admin staff #} @@ -235,7 +215,7 @@

{{ pageHeading }}

// Mirrors the check-in pattern used on clinic show page document.addEventListener('DOMContentLoaded', () => { const statusLabels = { - pending: 'Needs requesting', + pending: 'Priors required', requested: 'Requested', received: 'Received', not_available: 'Not available', @@ -292,14 +272,11 @@

{{ pageHeading }}

return response.json() }) .then(() => { - // Update the status tag in-place, preserving reason text if present + // Update the status tag in-place const statusCell = document.querySelector( '[data-mammogram-status="' + mammogramId + '"]' ) if (statusCell) { - const reasonEl = statusCell.querySelector('.js-prior-reason') - const existingReason = reasonEl ? reasonEl.textContent.trim() : '' - const colourClass = statusColours[newStatus] || '' const today = new Date().toLocaleDateString('en-GB', { day: 'numeric', month: 'short', year: 'numeric' @@ -314,11 +291,6 @@

{{ pageHeading }}

statusHtml += '
' + today + ' by ' + username + '' } - // Preserve reason when moving from pending to requested - if (existingReason && newStatus === 'requested') { - statusHtml += '
Reason: ' + existingReason + '' - } - statusCell.innerHTML = statusHtml } diff --git a/app/views/reading/session.html b/app/views/reading/session.html index a3cb7197..d1164e2e 100644 --- a/app/views/reading/session.html +++ b/app/views/reading/session.html @@ -298,7 +298,7 @@

Reading session cases

{% elseif appointment.readingCase | isCaseDeferred %} {{ "Deferred" | toTag }} {% elseif appointment | userRequestedPriors(data.currentUser.id) %} - {{ "Priors requested" | toTag }} + {{ "Priors required" | toTag }} {% elseif session.skippedAppointments | includes(appointment.id) %} {{ "Skipped" | toTag }} {% else %} diff --git a/app/views/reading/workflow/existing-read.html b/app/views/reading/workflow/existing-read.html index c0ca7912..c85d29ba 100644 --- a/app/views/reading/workflow/existing-read.html +++ b/app/views/reading/workflow/existing-read.html @@ -90,7 +90,7 @@

Your opinion

text: "Opinion" }, value: { - html: "Priors requested" | toTag + html: "Priors required" | toTag }, actions: { items: [{ From 55436be01b2f85a6da493a9b61a05f5e94e940dd Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Thu, 30 Jul 2026 17:02:15 +0100 Subject: [PATCH 12/13] Keep the priors request story in one column Who flagged or actioned a prior, when, and the reader's reason now read as one block under the mammogram, separated from it by a line break. The status column carries just the tag, so it stays scannable. --- app/views/reading/priors.html | 95 +++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 32 deletions(-) diff --git a/app/views/reading/priors.html b/app/views/reading/priors.html index 000f56e8..64027c00 100644 --- a/app/views/reading/priors.html +++ b/app/views/reading/priors.html @@ -126,37 +126,49 @@

{{ pageHeading }}

- {# Mammogram location and date, plus the reader's reason for the request #} - - - {# Status. Pending was flagged by a reader, so shows when they - flagged it; every later status was actioned by admin staff #} - + {# Status #} + + {# Actions #} {# Mammogram location and date, then the request story - who - flagged or actioned it, when, and the reader's reason. - Pending was flagged by a reader, so uses the request date; - every later status was actioned by admin staff #} + requested or actioned it, when, and the reader's reason #}
DateScreenedClosed TypeStage Outcome Actions
- {% if row.date %} - {{ row.date | formatDate }} - {% if not row.wasScreened %} -
{{ "Not screened" | asHint }} - {% endif %} + {% if row.screenedDate %} + {{ row.screenedDate | formatDate }} {% else %} - {{ "Not booked" | asHint }} + {{ "Not screened" | asHint }} {% endif %} 		- {{ episode.type | sentenceCase }} + {% if row.closedDate %} + {{ row.closedDate | formatDate }} + {% else %} + {{ "—" | asHint }} + {% endif %} - {{ tag({ - text: episode.stage | getEpisodeStageText, - colour: episode.stage | getEpisodeStageTagColour - }) }} + {{ episode.type | sentenceCase }} {% if episode.outcome %} From ca1db5047cfd55fab9e0c00f45a24c4e0bf213a9 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Thu, 30 Jul 2026 10:11:48 +0100 Subject: [PATCH 10/13] Return to existing-read page when making edits --- app/routes/reading.js | 83 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/app/routes/reading.js b/app/routes/reading.js index aee307d8..c70be246 100644 --- a/app/routes/reading.js +++ b/app/routes/reading.js @@ -1836,6 +1836,15 @@ module.exports = (router) => { const appointment = data.appointments.find((e) => e.id === appointmentId) if (!appointment) return res.redirect(`/reading/session/${sessionId}`) + // Editing a read the user has already saved. The existing-read page they + // came from is itself a summary of the read, so the confirmation step is + // redundant — save straight away regardless of the confirmation settings. + const isEditingExistingRead = userHasReadAppointment( + data, + appointment, + currentUserId + ) + // Check for late comparison if not already done const comparisonSetting = data.settings?.reading?.secondReaderComparison if (comparisonSetting === 'late' && !formData?.comparisonComplete) { @@ -1858,7 +1867,10 @@ module.exports = (router) => { case 'normal': // opinion-details-complete is only reached for normal when the user // went through the normal-details page, so use confirmNormalWithDetails - if (data.settings?.reading?.confirmNormalWithDetails === 'true') { + if ( + !isEditingExistingRead && + data.settings?.reading?.confirmNormalWithDetails === 'true' + ) { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/confirm-normal` ) @@ -1872,7 +1884,10 @@ module.exports = (router) => { const trChainParam = trReferrer ? `?referrerChain=${encodeURIComponent(trReferrer)}` : '' - if (data.settings?.reading?.confirmTechnicalRecall !== 'false') { + if ( + !isEditingExistingRead && + data.settings?.reading?.confirmTechnicalRecall !== 'false' + ) { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/review${trChainParam}` ) @@ -1887,7 +1902,10 @@ module.exports = (router) => { const rfaChainParam = rfaReferrer ? `?referrerChain=${encodeURIComponent(rfaReferrer)}` : '' - if (data.settings?.reading?.confirmRecallForAssessment !== 'false') { + if ( + !isEditingExistingRead && + data.settings?.reading?.confirmRecallForAssessment !== 'false' + ) { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/review${rfaChainParam}` ) @@ -1930,6 +1948,17 @@ module.exports = (router) => { return res.redirect(`/reading/session/${sessionId}`) } + // Whether this save is an edit of a read the user had already made. + // The case URL only routes already-read users via existing-read, so any + // journey reaching here with a read in place started from that page — + // and should return to it rather than moving on to the next case. + // Must be checked before the read is written. + const isEditingExistingRead = userHasReadAppointment( + data, + appointment, + currentUserId + ) + delete data.imageReadingTemp delete res.locals.data?.imageReadingTemp @@ -1960,10 +1989,11 @@ module.exports = (router) => { { wrap: false } ) - // Store banner message for the next case, but only if there is one + // Store banner message for the next case, but only if there is one. + // Edits stay on the current case, so there's nowhere to show it. // Bypassing req.flash as we couldn't get it to work - possibly due to redirect loops // Todo: can we get this working with req.flash? - if (nextUnreadAppointment) { + if (nextUnreadAppointment && !isEditingExistingRead) { const participant = data.participants.find( (person) => person.id === appointment.participantId ) @@ -1994,6 +2024,17 @@ module.exports = (router) => { return } + // An edit returns to the read it was made from, so the reader can see the + // change they've just made rather than being pushed on to the next case + if (isEditingExistingRead) { + res.redirect( + modalBreakout( + `/reading/session/${sessionId}/appointments/${appointmentId}/existing-read` + ) + ) + return + } + // Redirect to next unread appointment or end-of-session page if (nextUnreadAppointment) { res.redirect( @@ -2047,6 +2088,14 @@ module.exports = (router) => { const appointment = data.appointments.find((e) => e.id === appointmentId) if (!appointment) return res.redirect(`/reading/session/${sessionId}`) + // Editing a read the user has already saved — skip the confirmation step, + // the existing-read page they return to already summarises the read + const isEditingExistingRead = userHasReadAppointment( + data, + appointment, + data.currentUser?.id + ) + // Ensure appointmentId is set for tracking if (!data.imageReadingTemp) { data.imageReadingTemp = { appointmentId: appointmentId } @@ -2119,7 +2168,10 @@ module.exports = (router) => { ) } } - if (data.settings.reading.confirmNormal === 'true') { + if ( + !isEditingExistingRead && + data.settings.reading.confirmNormal === 'true' + ) { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/confirm-normal` ) @@ -2166,6 +2218,14 @@ module.exports = (router) => { const appointment = data.appointments.find((e) => e.id === appointmentId) if (!appointment) return res.redirect(`/reading/session/${sessionId}`) + // Editing a read the user has already saved — skip the confirmation step, + // the existing-read page they return to already summarises the read + const isEditingExistingRead = userHasReadAppointment( + data, + appointment, + currentUserId + ) + const opinion = data.imageReadingTemp?.opinion const comparisonInfo = getComparisonInfo( getReadingCase(data, appointment), @@ -2216,6 +2276,12 @@ module.exports = (router) => { console.log('Adopted first reader opinion:', firstRead.opinion) } + if (isEditingExistingRead) { + return res.redirect( + `/reading/session/${sessionId}/appointments/${appointmentId}/save-opinion` + ) + } + // Go straight to review since we have complete data return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/review` @@ -2250,7 +2316,10 @@ module.exports = (router) => { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/normal-details` ) - } else if (data.settings.reading.confirmNormal === 'true') { + } else if ( + !isEditingExistingRead && + data.settings.reading.confirmNormal === 'true' + ) { return res.redirect( `/reading/session/${sessionId}/appointments/${appointmentId}/confirm-normal` ) From 09ee8dbf78c611b9dff8214b59e6fdedd74d2d64 Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Thu, 30 Jul 2026 16:52:00 +0100 Subject: [PATCH 11/13] Use one 'Priors required' status, and move the request reason Readers saw 'Priors requested' while the priors dashboard called the same thing 'Needs requesting', which bled back into the reader's medical summary. Both now read 'Priors required' in orange. On the dashboard, the reason moves out of the cramped status column into the prior mammogram column as body text. --- app/lib/utils/status.js | 6 +- app/views/reading/priors.html | 78 ++++++------------- app/views/reading/session.html | 2 +- app/views/reading/workflow/existing-read.html | 2 +- 4 files changed, 31 insertions(+), 57 deletions(-) diff --git a/app/lib/utils/status.js b/app/lib/utils/status.js index 213c17c0..db120a17 100644 --- a/app/lib/utils/status.js +++ b/app/lib/utils/status.js @@ -249,7 +249,7 @@ const STATUS_TAGS = { // External prior mammogram request tracking (episode.priors) priorsRequest: { not_requested: { label: 'Not requested', colour: 'white' }, - pending: { label: 'Needs requesting', colour: 'orange' }, + pending: { label: 'Priors required', colour: 'orange' }, requested: { label: 'Requested', colour: 'yellow' }, received: { label: 'Received', colour: 'green' }, not_available: { label: 'Not available', colour: 'grey' }, @@ -267,7 +267,9 @@ const STATUS_TAGS = { incomplete: { colour: 'orange' }, urgent: { colour: 'red' }, due_soon: { colour: 'orange' }, - priors_requested: { colour: 'yellow' }, + // Case-level equivalent of priorsRequest.pending - same words and colour, + // so readers and admin staff see one status rather than two + priors_required: { colour: 'orange' }, deferred: { colour: 'orange' } } } diff --git a/app/views/reading/priors.html b/app/views/reading/priors.html index 6a24c8a9..000f56e8 100644 --- a/app/views/reading/priors.html +++ b/app/views/reading/priors.html @@ -64,7 +64,7 @@

{{ pageHeading }}

{% set tabItems = [ { id: "all", label: "All", count: allRows | length }, { id: "not-requested", label: "Not requested", count: notRequestedRows | length }, - { id: "pending", label: "Needs requesting", count: pendingRows | length }, + { id: "pending", label: "Priors required", count: pendingRows | length }, { id: "requested", label: "Requested", count: requestedRows | length }, { id: "resolved", label: "Recently resolved", count: resolvedRows | length } ] %} @@ -126,54 +126,34 @@

{{ pageHeading }}

 {{ mammogram | summarisePriorMammogram }} + {% if mammogram.requestReason %} +
Reason: {{ mammogram.requestReason }} + {% endif %} 		- {{ mammogram.requestStatus | toTag({ vocabulary: "priorsRequest" }) }} {% if mammogram.requestStatus == "pending" %} - {# Flagged by reader — show who requested and reason #} - {% if mammogram.requestedDate %} -
- - {{ mammogram.requestedDate | formatDate("D MMM YYYY") }} - {% if mammogram.requestedBy %} - by {{ mammogram.requestedBy | getUsername({ format: "short", identifyCurrentUser: true }) }} - {% endif %} - - {% endif %} - {% if mammogram.requestReason %} -
- Reason: {{ mammogram.requestReason }} - {% endif %} - {% elseif mammogram.requestStatus == "requested" %} - {# Admin has sent IEP request — show when admin actioned it #} - {% if mammogram.statusChangedDate %} -
- - {{ mammogram.statusChangedDate | formatDate("D MMM YYYY") }} - {% if mammogram.statusChangedBy %} - by {{ mammogram.statusChangedBy | getUsername({ format: "short", identifyCurrentUser: true }) }} - {% endif %} - - {% endif %} - {% if mammogram.requestReason %} -
- Reason: {{ mammogram.requestReason }} - {% endif %} - {% elseif mammogram.requestStatus == "received" or mammogram.requestStatus == "not_available" or mammogram.requestStatus == "not_needed" %} - {% if mammogram.statusChangedDate %} -
- - {{ mammogram.statusChangedDate | formatDate("D MMM YYYY") }} - {% if mammogram.statusChangedBy %} - by {{ mammogram.statusChangedBy | getUsername({ format: "short", identifyCurrentUser: true }) }} - {% endif %} - - {% endif %} + {% set changedDate = mammogram.requestedDate %} + {% set changedBy = mammogram.requestedBy %} + {% else %} + {% set changedDate = mammogram.statusChangedDate %} + {% set changedBy = mammogram.statusChangedBy %} + {% endif %} + + {{ mammogram.requestStatus | toTag({ vocabulary: "priorsRequest" }) }} + {% if changedDate and mammogram.requestStatus != "not_requested" %} +
+ + {{ changedDate | formatDate("D MMM YYYY") }} + {% if changedBy %} + by {{ changedBy | getUsername({ format: "short", identifyCurrentUser: true }) }} + {% endif %} + {% endif %} 		- {{ mammogram | summarisePriorMammogram }} - {% if mammogram.requestReason %} -
Reason: {{ mammogram.requestReason }} - {% endif %} - 		+ {# Mammogram location and date, then the request story - who + flagged or actioned it, when, and the reader's reason. + Pending was flagged by a reader, so uses the request date; + every later status was actioned by admin staff #} + {% if mammogram.requestStatus == "pending" %} + {% set changedLabel = "Flagged" %} {% set changedDate = mammogram.requestedDate %} {% set changedBy = mammogram.requestedBy %} - {% else %} + {% elseif mammogram.requestStatus == "requested" %} + {% set changedLabel = "Requested" %} + {% set changedDate = mammogram.statusChangedDate %} + {% set changedBy = mammogram.statusChangedBy %} + {% elseif mammogram.requestStatus != "not_requested" %} + {% set changedLabel = "Updated" %} {% set changedDate = mammogram.statusChangedDate %} {% set changedBy = mammogram.statusChangedBy %} {% endif %} - {{ mammogram.requestStatus | toTag({ vocabulary: "priorsRequest" }) }} - {% if changedDate and mammogram.requestStatus != "not_requested" %} -
- - {{ changedDate | formatDate("D MMM YYYY") }} - {% if changedBy %} - by {{ changedBy | getUsername({ format: "short", identifyCurrentUser: true }) }} +

{{ mammogram | summarisePriorMammogram }}

+ + {% if changedDate or mammogram.requestReason %} +

+ {% if changedDate %} + + {{- changedLabel }} + {%- if changedBy %} by {{ changedBy | getUsername({ format: "short", identifyCurrentUser: true }) }}{% endif %} + on {{ changedDate | formatDate("D MMMM YYYY") -}} + + {% endif %} + {% if mammogram.requestReason %} + {% if changedDate %}
{% endif %} + Reason: {{ mammogram.requestReason }} {% endif %} - +

{% endif %} 		+ {{ mammogram.requestStatus | toTag({ vocabulary: "priorsRequest" }) }} + {% if mammogram.requestStatus == "not_requested" or mammogram.requestStatus == "pending" %} @@ -250,6 +262,14 @@

{{ pageHeading }}

not_needed: 'Not needed' } + // How the request line describes each admin action + const requestLabels = { + requested: 'Requested', + received: 'Updated', + not_available: 'Updated', + not_needed: 'Updated' + } + function handleFormSubmit(e) { e.preventDefault() @@ -272,26 +292,37 @@

{{ pageHeading }}

return response.json() }) .then(() => { + const today = new Date().toLocaleDateString('en-GB', { + day: 'numeric', month: 'long', year: 'numeric' + }) + const table = document.querySelector('[data-current-username]') + const username = table ? table.getAttribute('data-current-username') : 'you' + // Update the status tag in-place const statusCell = document.querySelector( '[data-mammogram-status="' + mammogramId + '"]' ) if (statusCell) { const colourClass = statusColours[newStatus] || '' - const today = new Date().toLocaleDateString('en-GB', { - day: 'numeric', month: 'short', year: 'numeric' - }) - const table = document.querySelector('[data-current-username]') - const username = table ? table.getAttribute('data-current-username') : 'you' - - let statusHtml = '' + statusLabels[newStatus] + '' + statusCell.innerHTML = '' + + statusLabels[newStatus] + '' + } - // Show date and attribution after admin actions - if (['requested', 'received', 'not_available', 'not_needed'].includes(newStatus)) { - statusHtml += '
' + today + ' by ' + username + '' + // Re-attribute the request line, leaving the mammogram summary and + // the reader's reason alone. Rows never requested have no line yet + const detailsCell = document.querySelector( + '[data-mammogram-details="' + mammogramId + '"]' + ) + if (detailsCell) { + let requestLine = detailsCell.querySelector('.js-prior-request') + if (!requestLine) { + const paragraph = document.createElement('p') + paragraph.className = 'nhsuk-u-margin-top-3 nhsuk-u-margin-bottom-0' + paragraph.innerHTML = '' + detailsCell.appendChild(paragraph) + requestLine = paragraph.querySelector('.js-prior-request') } - - statusCell.innerHTML = statusHtml + requestLine.textContent = requestLabels[newStatus] + ' by ' + username + ' on ' + today } // Update the actions cell with new available actions From 5531cd7e6d3c8d9d8fbf4af1af9a690ad47c1bbc Mon Sep 17 00:00:00 2001 From: Ed Horsford Date: Thu, 30 Jul 2026 17:04:18 +0100 Subject: [PATCH 13/13] Say requested, not flagged, on the priors dashboard --- app/views/reading/priors.html | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/app/views/reading/priors.html b/app/views/reading/priors.html index 64027c00..fa344132 100644 --- a/app/views/reading/priors.html +++ b/app/views/reading/priors.html @@ -127,20 +127,16 @@

{{ pageHeading }}

 {% if mammogram.requestStatus == "pending" %} - {% set changedLabel = "Flagged" %} + {# Requested by a reader, so the request date is theirs #} + {% set changedLabel = "Requested" %} {% set changedDate = mammogram.requestedDate %} {% set changedBy = mammogram.requestedBy %} - {% elseif mammogram.requestStatus == "requested" %} - {% set changedLabel = "Requested" %} - {% set changedDate = mammogram.statusChangedDate %} - {% set changedBy = mammogram.statusChangedBy %} {% elseif mammogram.requestStatus != "not_requested" %} - {% set changedLabel = "Updated" %} + {# Actioned by admin staff #} + {% set changedLabel = "Requested" if mammogram.requestStatus == "requested" else "Updated" %} {% set changedDate = mammogram.statusChangedDate %} {% set changedBy = mammogram.statusChangedBy %} {% endif %}