diff --git a/.gitignore b/.gitignore index 5caa4c1a..9ea36f71 100644 --- a/.gitignore +++ b/.gitignore @@ -73,6 +73,7 @@ packages/e2e/sampledb_e2e.sqlite scripts/ !packages/api/scripts/ !packages/api/scripts/seed-table-view-config.ts +!packages/api/src/scripts/ !packages/docs/scripts/ test-csvs/ diff --git a/packages/api/initial_schema.sql b/packages/api/initial_schema.sql index 334ed82f..ab2c5779 100644 --- a/packages/api/initial_schema.sql +++ b/packages/api/initial_schema.sql @@ -259,6 +259,7 @@ CREATE TABLE IF NOT EXISTS sheet ( (box_id IS NULL AND bag_id IS NOT NULL) OR (box_id IS NULL AND bag_id IS NULL)) );--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS sheet_box_name_idx ON sheet(box_id, name) WHERE box_id IS NOT NULL;--> statement-breakpoint CREATE TABLE IF NOT EXISTS paper ( id INTEGER PRIMARY KEY REFERENCES storage_container(id), sheet_id INTEGER NOT NULL REFERENCES sheet(id), @@ -283,7 +284,7 @@ CREATE TABLE IF NOT EXISTS storage_type ( CREATE TABLE IF NOT EXISTS schema_version ( version INTEGER NOT NULL );--> statement-breakpoint -INSERT INTO schema_version (version) VALUES (5);--> statement-breakpoint +INSERT INTO schema_version (version) VALUES (7);--> statement-breakpoint CREATE TABLE IF NOT EXISTS settings ( key TEXT NOT NULL, user_id INTEGER REFERENCES users(id), @@ -383,5 +384,42 @@ CREATE TABLE IF NOT EXISTS qpcr_experiment_well ( UNIQUE (qpcr_experiment_id, well_position) );--> statement-breakpoint CREATE INDEX IF NOT EXISTS qpcr_experiment_well_experiment_idx ON qpcr_experiment_well(qpcr_experiment_id);--> statement-breakpoint +CREATE TABLE IF NOT EXISTS qpcr_run ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + qpcr_experiment_id INTEGER NOT NULL REFERENCES qpcr_experiment(id) ON DELETE CASCADE, + instrument_type TEXT NOT NULL, + run_started_at TEXT, + run_ended_at TEXT, + experiment_name TEXT, + file_name TEXT, + created TEXT NOT NULL DEFAULT current_timestamp, + slope REAL, + y_intercept REAL, + r_squared REAL, + efficiency REAL +);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS qpcr_run_experiment_idx ON qpcr_run(qpcr_experiment_id);--> statement-breakpoint +CREATE TABLE IF NOT EXISTS qpcr_well_result ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + qpcr_run_id INTEGER NOT NULL REFERENCES qpcr_run(id) ON DELETE CASCADE, + well_position TEXT NOT NULL, + target_name TEXT, + sample_barcode TEXT, + task TEXT, + cq REAL, + quantity REAL, + standard_quantity REAL, + amp_status TEXT, + UNIQUE (qpcr_run_id, well_position, target_name) +);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS qpcr_well_result_run_idx ON qpcr_well_result(qpcr_run_id);--> statement-breakpoint +CREATE TABLE IF NOT EXISTS qpcr_amplification_data ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + qpcr_well_result_id INTEGER NOT NULL REFERENCES qpcr_well_result(id) ON DELETE CASCADE, + cycle INTEGER NOT NULL, + rn REAL, + delta_rn REAL +);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS qpcr_amplification_data_well_result_idx ON qpcr_amplification_data(qpcr_well_result_id);--> statement-breakpoint CREATE INDEX IF NOT EXISTS qpcr_experiment_well_specimen_idx ON qpcr_experiment_well(specimen_id);--> statement-breakpoint PRAGMA foreign_keys=ON; diff --git a/packages/api/src/bun-sqlite.d.ts b/packages/api/src/bun-sqlite.d.ts index 881ba4b4..35eee7f9 100644 --- a/packages/api/src/bun-sqlite.d.ts +++ b/packages/api/src/bun-sqlite.d.ts @@ -1,6 +1,7 @@ declare module 'bun:sqlite' { export class Database { - constructor(path: string | ':memory:') + constructor(path: string | ':memory:', options?: { readonly?: boolean; create?: boolean; readwrite?: boolean; strict?: boolean }) + readonly filename: string exec(sql: string): void prepare(sql: string): Statement query(sql: string): Statement diff --git a/packages/api/src/db/__tests__/client.test.ts b/packages/api/src/db/__tests__/client.test.ts index 53b82116..c08ee650 100644 --- a/packages/api/src/db/__tests__/client.test.ts +++ b/packages/api/src/db/__tests__/client.test.ts @@ -33,6 +33,7 @@ describe('Database Client', () => { db.exec(`CREATE TABLE cryovial_tube (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, barcode TEXT, position TEXT)`) db.exec(`CREATE TABLE static_well (id INTEGER PRIMARY KEY, collection_id INTEGER NOT NULL, position TEXT)`) db.exec(`CREATE TABLE study_subject (id INTEGER PRIMARY KEY, study_id INTEGER NOT NULL, name TEXT NOT NULL)`) + db.exec(`CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT NOT NULL, box_id INTEGER, bag_id INTEGER)`) // Intentionally omit paper — migration 003 preflight creates legacy stub // Intentionally do NOT create error_logs const before = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='error_logs'").get() diff --git a/packages/api/src/db/__tests__/schema-evolution.test.ts b/packages/api/src/db/__tests__/schema-evolution.test.ts index a685b192..cd591ad3 100644 --- a/packages/api/src/db/__tests__/schema-evolution.test.ts +++ b/packages/api/src/db/__tests__/schema-evolution.test.ts @@ -36,6 +36,12 @@ function createTablesForLaterMigrations(sqlite: Database): void { study_id INTEGER NOT NULL, name TEXT NOT NULL )`) + sqlite.exec(`CREATE TABLE IF NOT EXISTS sheet ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL, + box_id INTEGER, + bag_id INTEGER + )`) } describe('schema evolution', () => { @@ -98,7 +104,7 @@ describe('schema evolution', () => { 'CREATE TABLE settings (key TEXT, user_id INTEGER, value TEXT, PRIMARY KEY (key, user_id))', ) sqlite.exec('CREATE TABLE storage_container (id INTEGER PRIMARY KEY)') - sqlite.exec('CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT)') + sqlite.exec('CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT, box_id INTEGER, bag_id INTEGER)') sqlite.exec(`CREATE TABLE paper ( id INTEGER PRIMARY KEY REFERENCES storage_container(id), sheet_id INTEGER NOT NULL REFERENCES sheet(id), @@ -123,7 +129,7 @@ describe('schema evolution', () => { sqlite.exec('INSERT INTO schema_version (version) VALUES (1)') sqlite.exec('CREATE TABLE study (id INTEGER PRIMARY KEY)') sqlite.exec('CREATE TABLE storage_container (id INTEGER PRIMARY KEY)') - sqlite.exec('CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT)') + sqlite.exec('CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT, box_id INTEGER, bag_id INTEGER)') sqlite.exec(`CREATE TABLE paper ( id INTEGER PRIMARY KEY REFERENCES storage_container(id), sheet_id INTEGER NOT NULL REFERENCES sheet(id), @@ -141,7 +147,7 @@ describe('schema evolution', () => { sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') sqlite.exec('INSERT INTO schema_version (version) VALUES (2)') sqlite.exec(`CREATE TABLE storage_container (id INTEGER PRIMARY KEY)`) - sqlite.exec(`CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT)`) + sqlite.exec(`CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT, box_id INTEGER, bag_id INTEGER)`) sqlite.exec(`CREATE TABLE paper ( id INTEGER PRIMARY KEY REFERENCES storage_container(id), sheet_id INTEGER NOT NULL REFERENCES sheet(id), @@ -181,7 +187,7 @@ describe('schema evolution', () => { sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') sqlite.exec('INSERT INTO schema_version (version) VALUES (2)') sqlite.exec(`CREATE TABLE storage_container (id INTEGER PRIMARY KEY)`) - sqlite.exec(`CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT)`) + sqlite.exec(`CREATE TABLE sheet (id INTEGER PRIMARY KEY, name TEXT, box_id INTEGER, bag_id INTEGER)`) sqlite.exec(`CREATE TABLE paper ( id INTEGER PRIMARY KEY REFERENCES storage_container(id), sheet_id INTEGER NOT NULL REFERENCES sheet(id), @@ -260,6 +266,33 @@ describe('schema evolution', () => { sqlite.close() }) + it('migration 006 makes sheet names unique per box but not per bag', () => { + const sqlite = new Database(testDbPath) + sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') + sqlite.exec('INSERT INTO schema_version (version) VALUES (5)') + createTablesForLaterMigrations(sqlite) + sqlite.exec(`INSERT INTO sheet (id, name, box_id, bag_id) VALUES (1, 'S1', 1, NULL), (2, 'S1', NULL, 7), (3, 'S1', NULL, 7)`) + + evolveOperationalSchema(sqlite) + + expect(getRecordedSchemaVersion(sqlite)).toBe(CURRENT_SCHEMA_VERSION) + expect(() => sqlite.exec(`INSERT INTO sheet (id, name, box_id) VALUES (4, 'S1', 1)`)).toThrow(/UNIQUE/) + sqlite.exec(`INSERT INTO sheet (id, name, bag_id) VALUES (5, 'S1', 7)`) + sqlite.close() + }) + + it('migration 006 aborts when a box has two sheets with the same name', () => { + const sqlite = new Database(testDbPath) + sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') + sqlite.exec('INSERT INTO schema_version (version) VALUES (5)') + createTablesForLaterMigrations(sqlite) + sqlite.exec(`INSERT INTO sheet (id, name, box_id) VALUES (1, 'S1', 1), (2, 'S1', 1)`) + + expect(() => evolveOperationalSchema(sqlite)).toThrow(/Rename one of them first/) + expect(getRecordedSchemaVersion(sqlite)).toBe(5) + sqlite.close() + }) + it('fail-hard: invalid migration leaves schema_version unchanged', () => { const sqlite = new Database(testDbPath) sqlite.exec('CREATE TABLE schema_version (version INTEGER NOT NULL)') diff --git a/packages/api/src/db/migrations/006_box_sheet_name_unique.sql b/packages/api/src/db/migrations/006_box_sheet_name_unique.sql new file mode 100644 index 00000000..139a7116 --- /dev/null +++ b/packages/api/src/db/migrations/006_box_sheet_name_unique.sql @@ -0,0 +1,15 @@ +-- Migration 006: sheet names are unique within a box. Bags are exempt: legacy DBS bags hold +-- many single-paper sheets that share one name. (UNIQUE(name, box_id, bag_id) never fired +-- because one of box_id / bag_id is always NULL.) +CREATE TEMP TABLE IF NOT EXISTS __migration_006_guard (id INTEGER PRIMARY KEY);--> statement-breakpoint +CREATE TRIGGER __migration_006_guard_tr BEFORE INSERT ON __migration_006_guard +BEGIN + SELECT RAISE(ABORT, 'Cannot add unique sheet name index: a box has two sheets with the same name. Rename one of them first.') + WHERE EXISTS ( + SELECT 1 FROM sheet WHERE box_id IS NOT NULL GROUP BY box_id, name HAVING COUNT(*) > 1 + ); +END;--> statement-breakpoint +INSERT INTO __migration_006_guard DEFAULT VALUES;--> statement-breakpoint +DROP TRIGGER __migration_006_guard_tr;--> statement-breakpoint +DROP TABLE __migration_006_guard;--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS sheet_box_name_idx ON sheet(box_id, name) WHERE box_id IS NOT NULL; diff --git a/packages/api/src/db/migrations/007_qpcr_result_tables.sql b/packages/api/src/db/migrations/007_qpcr_result_tables.sql new file mode 100644 index 00000000..d0af7a9a --- /dev/null +++ b/packages/api/src/db/migrations/007_qpcr_result_tables.sql @@ -0,0 +1,40 @@ +-- Migration 007: qPCR result tables. They exist in databases created before the SQL snapshot +-- (matching definitions below) but initial_schema.sql never created them, so result uploads failed +-- on fresh installs. IF NOT EXISTS leaves existing databases unchanged. +CREATE TABLE IF NOT EXISTS qpcr_run ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + qpcr_experiment_id INTEGER NOT NULL REFERENCES qpcr_experiment(id) ON DELETE CASCADE, + instrument_type TEXT NOT NULL, + run_started_at TEXT, + run_ended_at TEXT, + experiment_name TEXT, + file_name TEXT, + created TEXT NOT NULL DEFAULT current_timestamp, + slope REAL, + y_intercept REAL, + r_squared REAL, + efficiency REAL +);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS qpcr_run_experiment_idx ON qpcr_run(qpcr_experiment_id);--> statement-breakpoint +CREATE TABLE IF NOT EXISTS qpcr_well_result ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + qpcr_run_id INTEGER NOT NULL REFERENCES qpcr_run(id) ON DELETE CASCADE, + well_position TEXT NOT NULL, + target_name TEXT, + sample_barcode TEXT, + task TEXT, + cq REAL, + quantity REAL, + standard_quantity REAL, + amp_status TEXT, + UNIQUE (qpcr_run_id, well_position, target_name) +);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS qpcr_well_result_run_idx ON qpcr_well_result(qpcr_run_id);--> statement-breakpoint +CREATE TABLE IF NOT EXISTS qpcr_amplification_data ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + qpcr_well_result_id INTEGER NOT NULL REFERENCES qpcr_well_result(id) ON DELETE CASCADE, + cycle INTEGER NOT NULL, + rn REAL, + delta_rn REAL +);--> statement-breakpoint +CREATE INDEX IF NOT EXISTS qpcr_amplification_data_well_result_idx ON qpcr_amplification_data(qpcr_well_result_id); diff --git a/packages/api/src/db/open.ts b/packages/api/src/db/open.ts index a1e922a3..6e79c029 100644 --- a/packages/api/src/db/open.ts +++ b/packages/api/src/db/open.ts @@ -80,6 +80,13 @@ function resolveDatabaseFilePath(relativeOrAbsolute: string): string { return isAbsolute(relativeOrAbsolute) ? relativeOrAbsolute : resolve(getMonorepoRoot(), relativeOrAbsolute) } +/** Path openOperationalDatabase would use: explicit path, then DATABASE_PATH, then the dev default. */ +export function resolveOperationalDatabasePath(dbPath?: string): string { + if (dbPath) return resolveDatabaseFilePath(dbPath) + if (process.env.DATABASE_PATH) return resolveDatabaseFilePath(process.env.DATABASE_PATH) + return resolve(getMonorepoRoot(), 'sampledb_dev.sqlite') +} + export type OperationalDatabase = ReturnType> /** @@ -89,15 +96,7 @@ export function openOperationalDatabase(dbPath?: string): { db: OperationalDatabase sqlite: SQLiteDatabase } { - let resolvedPath: string - - if (dbPath) { - resolvedPath = resolveDatabaseFilePath(dbPath) - } else if (process.env.DATABASE_PATH) { - resolvedPath = resolveDatabaseFilePath(process.env.DATABASE_PATH) - } else { - resolvedPath = resolve(getMonorepoRoot(), 'sampledb_dev.sqlite') - } + const resolvedPath = resolveOperationalDatabasePath(dbPath) if (process.env.NODE_ENV !== 'production') { console.log(`📁 Monorepo root: ${getMonorepoRoot()}`) diff --git a/packages/api/src/db/schema-evolution.ts b/packages/api/src/db/schema-evolution.ts index c0eeab90..b96368bd 100644 --- a/packages/api/src/db/schema-evolution.ts +++ b/packages/api/src/db/schema-evolution.ts @@ -7,7 +7,7 @@ import { listNumberedMigrations, runSqlMigration } from './migration-runner' const PAPER_SUBLABEL_MIGRATION_VERSION = 3 /** Canonical schema level; bump when adding numbered deltas under migrations/. */ -export const CURRENT_SCHEMA_VERSION = 5 +export const CURRENT_SCHEMA_VERSION = 7 const SCHEMA_VERSION_TABLE = 'schema_version' const LEGACY_BASELINE_VERSION = 1 diff --git a/packages/api/src/db/schema.ts b/packages/api/src/db/schema.ts index 48c44717..8b339eab 100644 --- a/packages/api/src/db/schema.ts +++ b/packages/api/src/db/schema.ts @@ -356,7 +356,8 @@ export const sheet = sqliteTable('sheet', { (${t.boxId} IS NULL AND ${t.bagId} IS NOT NULL) OR (${t.boxId} IS NULL AND ${t.bagId} IS NULL) `), - unq: unique().on(t.name, t.boxId, t.bagId) + unq: unique().on(t.name, t.boxId, t.bagId), + boxNameUniq: uniqueIndex('sheet_box_name_idx').on(t.boxId, t.name).where(sql`${t.boxId} IS NOT NULL`), })) export const paper = sqliteTable('paper', { diff --git a/packages/api/src/lib/__tests__/container-move.test.ts b/packages/api/src/lib/__tests__/container-move.test.ts index 0aa75f46..02f77c28 100644 --- a/packages/api/src/lib/__tests__/container-move.test.ts +++ b/packages/api/src/lib/__tests__/container-move.test.ts @@ -17,7 +17,7 @@ import { type ContainerInfo, } from '../container-move' import { storageContainer } from '../../db/schema' -import { micronixTube, staticWell } from '../../db/schema' +import { micronixTube, staticWell, box, sheet, paper } from '../../db/schema' import type { Database } from '../../db/client' import { utcNow } from '../datetime' @@ -560,4 +560,39 @@ describe('container-move', () => { expect(info?.currentCollectionName).toBe(plate.name) }) }) + + describe('paper moves between same-named sheets', () => { + it('moves papers from Sheet-1 of one box to Sheet-1 of another using parent names', async () => { + const storageType = await createTestStorageType(testDb, { name: 'Freezer' }) + const location = await createTestLocation(testDb, { name: 'Loc', storageTypeId: String(storageType.id) }) + const specimenType = await createTestSpecimenType(testDb, { name: 'Blood' }) + const specimen = await createTestSpecimen(testDb, specimenType.id) + const unit = await createTestUnit(testDb, { symbol: 'uL', name: 'microliter', category: 'volume' }) + const now = utcNow() + const [boxA] = await testDb.insert(box).values({ name: 'BA', locationId: location.id, created: now, lastUpdated: now }).returning() + const [boxB] = await testDb.insert(box).values({ name: 'BB', locationId: location.id, created: now, lastUpdated: now }).returning() + const [sheetA] = await testDb.insert(sheet).values({ name: 'Sheet-1', boxId: boxA.id }).returning() + const [sheetB] = await testDb.insert(sheet).values({ name: 'Sheet-1', boxId: boxB.id }).returning() + const [container] = await testDb + .insert(storageContainer) + .values({ specimenId: specimen.id, unitId: unit.id, totalQuantity: 1, remainingQuantity: 1, created: now, lastUpdated: now }) + .returning() + await testDb.insert(paper).values({ id: container!.id, sheetId: sheetB.id }) + + const ambiguous = await executeMoves(testDb, { + mappings: [{ fromCollectionName: 'Sheet-1', toCollectionName: 'Sheet-1' }], + moves: [{ identifier: { type: 'container_id', containerId: container!.id }, targetPosition: '' }], + }) + expect(ambiguous.success).toBe(false) + expect(ambiguous.errors?.[0].error).toMatch(/set toParentName/) + + const result = await executeMoves(testDb, { + mappings: [{ fromCollectionName: 'Sheet-1', fromParentName: 'BB', toCollectionName: 'Sheet-1', toParentName: 'BA' }], + moves: [{ identifier: { type: 'container_id', containerId: container!.id }, targetPosition: '' }], + }) + expect(result.success).toBe(true) + const [moved] = await testDb.select().from(paper).all() + expect(moved.sheetId).toBe(sheetA.id) + }) + }) }) diff --git a/packages/api/src/lib/__tests__/datetime.test.ts b/packages/api/src/lib/__tests__/datetime.test.ts new file mode 100644 index 00000000..48eb498e --- /dev/null +++ b/packages/api/src/lib/__tests__/datetime.test.ts @@ -0,0 +1,18 @@ +import { describe, it, expect } from 'vitest' +import { shiftCalendarDate } from '../datetime' + +describe('shiftCalendarDate', () => { + it('rolls over months and years', () => { + expect(shiftCalendarDate('2024-01-31', 1)).toBe('2024-02-01') + expect(shiftCalendarDate('2024-03-01', -1)).toBe('2024-02-29') + expect(shiftCalendarDate('2023-12-31', 1)).toBe('2024-01-01') + }) + + it('reads the date part of a timestamp', () => { + expect(shiftCalendarDate('2024-05-26 22:28:26', 0)).toBe('2024-05-26') + }) + + it('returns null for text that does not start with a date', () => { + expect(shiftCalendarDate('1/5/2024', 1)).toBeNull() + }) +}) diff --git a/packages/api/src/lib/__tests__/derivations-csv.test.ts b/packages/api/src/lib/__tests__/derivations-csv.test.ts index 6fd32f6a..1c28a946 100644 --- a/packages/api/src/lib/__tests__/derivations-csv.test.ts +++ b/packages/api/src/lib/__tests__/derivations-csv.test.ts @@ -78,6 +78,11 @@ describe('derivations-csv', () => { expect(rows[0].parent_container_id).toBe('1') }) + it('matches headers case-insensitively and drops rows with no values', () => { + const rows = parseCsv('Parent_Container_ID,Plate_Name\n1,P1\n,\n,,\n') + expect(rows).toEqual([{ parent_container_id: '1', plate_name: 'P1' }]) + }) + it('handles newlines inside quoted fields', () => { const text = 'parent_container_id,notes\n1,"line one\nline two"' const rows = parseCsv(text) @@ -283,4 +288,106 @@ describe('derivations-csv', () => { expect(child!.unitId).toBe(unit.id) }) }) + + describe('row values and positions', () => { + let testDb: Database + let sqlite: Awaited>['sqlite'] + + beforeEach(async () => { + const setup = await setupTestDatabase() + testDb = setup.db + sqlite = setup.sqlite + }) + + afterEach(() => { + if (sqlite) cleanupTestDatabase(sqlite) + }) + + const settings = { + derivationType: 'Extraction', + specimenTypeName: 'DNA', + containerType: 'micronix_tube' as const, + protocol: 'Standard', + derivationDate: '2025-01-15', + } + const header = 'parent_container_id,plate_name,position,container_barcode' + + /** Parent tube at PlateDef/A01 holding 10 uL, plus a tube already at A02. */ + async function seedParent() { + const now = utcNow() + const unit = await createTestUnit(testDb, { symbol: 'uL', name: 'microliter', category: 'volume' }) + await setContainerDefaults(testDb, { micronix_tube: { totalQuantity: 1, remainingQuantity: 1, defaultUnitSymbol: 'uL' } }) + await testDb.insert(containerTypeUnit).values({ containerType: 'micronix_tube', unitId: unit.id }) + const study = await createTestStudy(testDb, { title: 'Deriv Study', shortCode: 'DERIV' }) + const subject = await createTestStudySubject(testDb, { studyId: study.id, name: 'Subj1' }) + const bloodType = await createTestSpecimenType(testDb, { name: 'Blood' }) + const dnaType = await createTestSpecimenType(testDb, { name: 'DNA' }) + await testDb.insert(specimenTypeContainerType).values([ + { specimenTypeId: bloodType.id, containerType: 'micronix_tube', created: now }, + { specimenTypeId: dnaType.id, containerType: 'micronix_tube', created: now }, + ]) + const [parentSpecimen] = await testDb + .insert(specimen) + .values({ studySubjectId: subject.id, specimenTypeId: bloodType.id, collectionDate: '2025-01-01', created: now, lastUpdated: now }) + .returning() + const storageType = await createTestStorageType(testDb, { name: 'Freezer' }) + const loc = await createTestLocation(testDb, { name: 'Loc', storageTypeId: String(storageType.id), canContainCollections: true }) + const plate = await createTestMicronixPlate(testDb, { name: 'PlateDef', locationId: loc.id }) + const addTube = async (barcode: string, position: string, quantity: number) => { + const [sc] = await testDb + .insert(storageContainer) + .values({ specimenId: parentSpecimen.id, unitId: unit.id, totalQuantity: quantity, remainingQuantity: quantity, created: now, lastUpdated: now }) + .returning() + await testDb.insert(micronixTube).values({ id: sc.id, collectionId: plate.id, position, barcode }) + return sc + } + const parent = await addTube('MT-PARENT', 'A01', 10) + await addTube('MT-TAKEN', 'A02', 1) + return { parent } + } + + it('checks non-padded positions against stored ones and against each other', async () => { + const { parent } = await seedParent() + const csv = [header, `${parent.id},PlateDef,A2,C1`, `${parent.id},PlateDef,A3,C2`, `${parent.id},PlateDef,A03,C3`].join('\n') + + const result = await validateDerivationsCsv(testDb, csv, settings) + + expect(result.rows.map((r) => r.valid)).toEqual([false, true, false]) + expect(result.rows[0].error).toMatch(/already used in that plate/) + expect(result.rows[2].error).toMatch(/more than once in your file/) + }) + + it('reports a quantity that is not a number instead of dropping it', async () => { + const { parent } = await seedParent() + const csv = [`${header},quantity_used`, `${parent.id},PlateDef,B01,C1,3x`].join('\n') + + const result = await validateDerivationsCsv(testDb, csv, settings) + + expect(result.rows[0].valid).toBe(false) + expect(result.rows[0].error).toBe("quantity_used '3x' is not a number") + }) + + it('uses the shared reduce-parent setting when the cell is blank', async () => { + const { parent } = await seedParent() + const csv = [`${header},quantity_used,reduce_parent_quantity`, `${parent.id},PlateDef,B01,C1,3,`].join('\n') + + const result = await importDerivationsFromCsv(testDb, csv, { + settings: { ...settings, reduceParentQuantity: false }, + }) + + expect(result.rows[0].success).toBe(true) + const after = await testDb.select().from(storageContainer).where(eq(storageContainer.id, parent.id)).get() + expect(after?.remainingQuantity).toBe(10) + }) + + it('rejects paper rows whose box does not exist', async () => { + const { parent } = await seedParent() + const csv = ['parent_container_id,container_type,box_name,sheet_name,sublabel', `${parent.id},paper,No Such Box,Sheet-1,S1`].join('\n') + + const result = await validateDerivationsCsv(testDb, csv, { ...settings, containerType: '' }) + + expect(result.rows[0].valid).toBe(false) + expect(result.rows[0].error).toMatch(/Box 'No Such Box' not found/) + }) + }) }) diff --git a/packages/api/src/lib/__tests__/password-timing.test.ts b/packages/api/src/lib/__tests__/password-timing.test.ts new file mode 100644 index 00000000..38ee385e --- /dev/null +++ b/packages/api/src/lib/__tests__/password-timing.test.ts @@ -0,0 +1,21 @@ +import { describe, it, expect, vi } from 'vitest' +import bcrypt from 'bcryptjs' +import { verifyPasswordConstantTime } from '../auth/auth-service' + +describe('verifyPasswordConstantTime', () => { + it('still runs a bcrypt comparison when there is no such user', async () => { + const compare = vi.spyOn(bcrypt, 'compare') + + const valid = await verifyPasswordConstantTime('guess', undefined) + + expect(valid).toBe(false) + expect(compare).toHaveBeenCalledTimes(1) + compare.mockRestore() + }) + + it('checks the real hash when the user exists', async () => { + const hash = await bcrypt.hash('right', 4) + expect(await verifyPasswordConstantTime('right', hash)).toBe(true) + expect(await verifyPasswordConstantTime('wrong', hash)).toBe(false) + }) +}) diff --git a/packages/api/src/lib/__tests__/plate-csv.test.ts b/packages/api/src/lib/__tests__/plate-csv.test.ts index 6fa8a640..cf261f9e 100644 --- a/packages/api/src/lib/__tests__/plate-csv.test.ts +++ b/packages/api/src/lib/__tests__/plate-csv.test.ts @@ -131,5 +131,14 @@ describe('plate-csv', () => { const result = parsePlateCSV(csv, singleColumnConfig) expect(result[0]).toEqual({ wellPosition: 'A01', barcode: 'MT001' }) }) + + it('reads quoted scanner exports', () => { + const csv = '"Well","Barcode"\r\n"A01","FR123"\r\n"B2","FR, 456"' + const result = parsePlateCSV(csv, singleColumnConfig) + expect(result).toEqual([ + { wellPosition: 'A01', barcode: 'FR123' }, + { wellPosition: 'B02', barcode: 'FR, 456' }, + ]) + }) }) }) diff --git a/packages/api/src/lib/__tests__/write-atomicity.test.ts b/packages/api/src/lib/__tests__/write-atomicity.test.ts index 41710f55..97a55243 100644 --- a/packages/api/src/lib/__tests__/write-atomicity.test.ts +++ b/packages/api/src/lib/__tests__/write-atomicity.test.ts @@ -10,7 +10,7 @@ import { } from '../../__tests__/helpers/factories' import { setContainerDefaults } from '../settings' import { runBulkCombinedImport } from '../bulk-combined-import' -import { moveSheetsToCollection, SheetNotFoundError } from '../collections/sheet-move' +import { moveSheetsToCollection, SheetNameConflictError, SheetNotFoundError } from '../collections/sheet-move' import { specimenTypeContainerType, containerTypeUnit, @@ -116,4 +116,15 @@ describe('multi-step writes roll back on failure', () => { const [after] = await testDb.select().from(sheet).where(eq(sheet.id, s1.id)) expect(after.boxId).toBe(boxA.id) }) + + it('sheet move refuses a name that already exists in the target box', async () => { + const { loc } = await setupMicronixPlate() + const now = utcNow() + const [boxA] = await testDb.insert(box).values({ name: 'BoxA', locationId: loc.id, created: now, lastUpdated: now }).returning() + const [boxB] = await testDb.insert(box).values({ name: 'BoxB', locationId: loc.id, created: now, lastUpdated: now }).returning() + const [moving] = await testDb.insert(sheet).values({ name: 'Sheet-1', boxId: boxA.id }).returning() + await testDb.insert(sheet).values({ name: 'Sheet-1', boxId: boxB.id }) + + await expect(moveSheetsToCollection(testDb, [moving.id], boxB.id, 'box')).rejects.toBeInstanceOf(SheetNameConflictError) + }) }) diff --git a/packages/api/src/lib/auth/auth-service.ts b/packages/api/src/lib/auth/auth-service.ts index 0b2aa810..118bc779 100644 --- a/packages/api/src/lib/auth/auth-service.ts +++ b/packages/api/src/lib/auth/auth-service.ts @@ -1,4 +1,4 @@ -import { eq, and, isNull, or } from 'drizzle-orm' +import { eq, and, isNull } from 'drizzle-orm' import bcrypt from 'bcryptjs' import { nanoid } from 'nanoid' import type { Database } from '../../db/client' @@ -29,31 +29,43 @@ export async function findActiveUserByEmailOrUsername( database: Database, emailOrUsername: string ): Promise { + // Email first: a username must never shadow another user's email address. + const byEmail = await database + .select() + .from(users) + .where(and(eq(users.email, emailOrUsername), isNull(users.deletedAt))) + .get() + if (byEmail) return byEmail return ( (await database .select() .from(users) - .where( - and( - or(eq(users.email, emailOrUsername), eq(users.username, emailOrUsername)), - isNull(users.deletedAt) - ) - ) + .where(and(eq(users.username, emailOrUsername), isNull(users.deletedAt))) .get()) ?? null ) } +let dummyPasswordHash: string | null = null + +/** + * Check a password against a user's hash, or against a throwaway hash when there is no such + * user, so a missing account takes as long to reject as a wrong password. + */ +export async function verifyPasswordConstantTime(password: string, passwordHash: string | undefined): Promise { + if (passwordHash) return bcrypt.compare(password, passwordHash) + dummyPasswordHash ??= await bcrypt.hash('no-such-user', 10) + await bcrypt.compare(password, dummyPasswordHash) + return false +} + export async function verifyLoginCredentials( database: Database, emailOrUsername: string, password: string ): Promise { const user = await findActiveUserByEmailOrUsername(database, emailOrUsername) - if (!user) { - throw new UnauthorizedError('Invalid credentials') - } - const valid = await bcrypt.compare(password, user.passwordHash) - if (!valid) { + const valid = await verifyPasswordConstantTime(password, user?.passwordHash) + if (!user || !valid) { throw new UnauthorizedError('Invalid credentials') } if (!user.approvedAt) { diff --git a/packages/api/src/lib/collections/collection-resolve.ts b/packages/api/src/lib/collections/collection-resolve.ts index 510e7512..9a796d5e 100644 --- a/packages/api/src/lib/collections/collection-resolve.ts +++ b/packages/api/src/lib/collections/collection-resolve.ts @@ -6,10 +6,16 @@ import type { CollectionType } from './types' export type { CollectionType } from './types' +/** + * Collection types whose names identify one record. Sheet names repeat across boxes and bags, + * so sheets resolve by name and parent through resolveSheetsByName instead. + */ +export type NamedCollectionType = Exclude + /** Resolve collection by name. */ export async function resolveCollectionByName( name: string, - type: CollectionType, + type: NamedCollectionType, database: Database, ): Promise { switch (type) { @@ -45,19 +51,32 @@ export async function resolveCollectionByName( .get() return bagRecord?.id ?? null } - case 'sheet': { - const sheetRecord = await database - .select({ id: sheet.id }) - .from(sheet) - .where(eq(sheet.name, name)) - .get() - return sheetRecord?.id ?? null - } default: return null } } +/** + * Sheets with this name, optionally only those in the box or bag named parentName. + * Sheet names are not unique (default names like "Sheet-1" repeat in every box, and legacy + * bags hold many same-named sheets), so callers must handle several matches. + */ +export async function resolveSheetsByName( + database: Database, + name: string, + parentName?: string, +): Promise { + const rows = await database + .select({ id: sheet.id, boxName: box.name, bagName: bag.name }) + .from(sheet) + .leftJoin(box, eq(sheet.boxId, box.id)) + .leftJoin(bag, eq(sheet.bagId, bag.id)) + .where(eq(sheet.name, name)) + return rows + .filter((row) => parentName === undefined || row.boxName === parentName || row.bagName === parentName) + .map((row) => row.id) +} + /** Resolve collection by barcode (micronix plates and cryovial boxes only). */ export async function resolveCollectionByBarcode( barcode: string, @@ -89,7 +108,7 @@ export async function resolveCollectionByBarcode( /** Resolve collection by name, then barcode when applicable. */ export async function resolveCollection( identifier: string, - type: CollectionType, + type: NamedCollectionType, database: Database, ): Promise { const byName = await resolveCollectionByName(identifier, type, database) @@ -166,7 +185,7 @@ export const checkCollectionsBodySchema = z.object({ collections: z.array( z.object({ identifier: z.string(), - type: z.enum(['micronix_plate', 'cryovial_box', 'box', 'bag', 'sheet']), + type: z.enum(['micronix_plate', 'cryovial_box', 'box', 'bag']), }), ), }) @@ -225,6 +244,9 @@ export const moveContainersBodySchema = z.object({ z.object({ fromCollectionName: z.string().min(1), toCollectionName: z.string().min(1), + /** For sheets: box or bag holding the source / target sheet (sheet names repeat). */ + fromParentName: z.string().min(1).optional(), + toParentName: z.string().min(1).optional(), }), ), moves: z.array( @@ -282,7 +304,7 @@ export const deleteWithContentsBodySchema = z.object({ export async function checkCollectionsExist( database: Database, - collections: Array<{ identifier: string; type: CollectionType | 'sheet' }>, + collections: Array<{ identifier: string; type: NamedCollectionType }>, ) { return Promise.all( collections.map(async (col) => { diff --git a/packages/api/src/lib/collections/sheet-move.ts b/packages/api/src/lib/collections/sheet-move.ts index 65efc6da..e169c94a 100644 --- a/packages/api/src/lib/collections/sheet-move.ts +++ b/packages/api/src/lib/collections/sheet-move.ts @@ -1,4 +1,4 @@ -import { eq, sql } from 'drizzle-orm' +import { eq, inArray, sql } from 'drizzle-orm' import type { Database } from '../../db/client' import { box, bag, sheet } from '../../db/schema' import { withWriteTransaction } from '../../db/write-transaction' @@ -17,6 +17,13 @@ export class SheetNotFoundError extends Error { } } +export class SheetNameConflictError extends Error { + constructor(name: string) { + super(`The target box already has a sheet named '${name}'. Rename one of them first.`) + this.name = 'SheetNameConflictError' + } +} + export async function moveSheetsToCollection( database: Database, sheetIds: number[], @@ -26,6 +33,20 @@ export async function moveSheetsToCollection( if (targetCollectionType === 'box') { const exists = await database.select().from(box).where(eq(box.id, targetCollectionId)).get() if (!exists) throw new SheetMoveTargetNotFoundError('box') + // Sheet names are unique within a box (bags still allow repeated legacy names). + const moving = sheetIds.length > 0 + ? await database.select({ id: sheet.id, name: sheet.name }).from(sheet).where(inArray(sheet.id, sheetIds)) + : [] + const staying = await database + .select({ id: sheet.id, name: sheet.name }) + .from(sheet) + .where(eq(sheet.boxId, targetCollectionId)) + const movingIds = new Set(moving.map((s) => s.id)) + const taken = new Set(staying.filter((s) => !movingIds.has(s.id)).map((s) => s.name)) + for (const s of moving) { + if (taken.has(s.name)) throw new SheetNameConflictError(s.name) + taken.add(s.name) + } } else { const exists = await database.select().from(bag).where(eq(bag.id, targetCollectionId)).get() if (!exists) throw new SheetMoveTargetNotFoundError('bag') diff --git a/packages/api/src/lib/container-move.ts b/packages/api/src/lib/container-move.ts index 410640fe..e10bbeb6 100644 --- a/packages/api/src/lib/container-move.ts +++ b/packages/api/src/lib/container-move.ts @@ -11,7 +11,7 @@ import { sheet, } from '../db/schema' import { eq, and, isNull } from 'drizzle-orm' -import { resolveCollection, type CollectionType } from './collections/collection-resolve' +import { resolveCollection, resolveSheetsByName, type CollectionType } from './collections/collection-resolve' import { checkGridPositionOccupancy } from './container-occupancy' import { normalizePosition } from './normalize-position' @@ -42,6 +42,9 @@ export interface MoveOperation { export interface CollectionMapping { fromCollectionName: string toCollectionName: string + /** For sheets: box or bag holding the source / target sheet (sheet names repeat). */ + fromParentName?: string + toParentName?: string } export interface BatchMoveRequest { @@ -62,13 +65,16 @@ export interface MoveResult { errors?: ValidationError[] } +/** Collections whose containers sit at grid positions. */ +type GridCollectionType = 'cryovial_box' | 'micronix_plate' + /** * Resolve container by position and collection */ export async function resolveContainerByPosition( database: Database, collectionName: string, - collectionType: CollectionType, + collectionType: GridCollectionType, rawPosition: string ): Promise { // Stored grid positions are normalized ("B1" -> "B01"); match that form. @@ -279,7 +285,7 @@ export async function resolveContainerByIdentifier(database: Database, identifie if (identifier.type === 'barcode' && identifier.barcode) { return resolveContainerByBarcode(database, identifier.barcode) } else if (identifier.type === 'position' && identifier.sourceCollectionName && identifier.sourcePosition) { - const types: CollectionType[] = ['cryovial_box', 'micronix_plate', 'box', 'sheet'] + const types: GridCollectionType[] = ['cryovial_box', 'micronix_plate'] for (const type of types) { const result = await resolveContainerByPosition(database, identifier.sourceCollectionName, type, identifier.sourcePosition) if (result) return result @@ -378,6 +384,23 @@ export async function executeMoves(database: Database, request: BatchMoveRequest const mappingMap = new Map() const mappingErrors: string[] = [] for (const m of mappings) { + if (collectionType === 'sheet') { + // Every sheet with the source name (in fromParentName, if given) maps to the one + // target sheet; the target must be unambiguous. + const fromIds = await resolveSheetsByName(database, m.fromCollectionName, m.fromParentName) + const toIds = await resolveSheetsByName(database, m.toCollectionName, m.toParentName) + if (fromIds.length === 0) mappingErrors.push(`Source collection "${m.fromCollectionName}" not found`) + if (toIds.length === 0) mappingErrors.push(`Target collection "${m.toCollectionName}" not found`) + if (toIds.length > 1) { + mappingErrors.push( + `Target sheet "${m.toCollectionName}" matches ${toIds.length} sheets; set toParentName to the box or bag that holds it`, + ) + } + if (fromIds.length > 0 && toIds.length === 1) { + for (const fromId of fromIds) mappingMap.set(fromId, toIds[0]) + } + continue + } const fromId = await resolveCollection(m.fromCollectionName, collectionType, database) const toId = await resolveCollection(m.toCollectionName, collectionType, database) if (fromId && toId) { diff --git a/packages/api/src/lib/controls/__tests__/batch-with-specimens.test.ts b/packages/api/src/lib/controls/__tests__/batch-with-specimens.test.ts index 82debf71..d0265e3c 100644 --- a/packages/api/src/lib/controls/__tests__/batch-with-specimens.test.ts +++ b/packages/api/src/lib/controls/__tests__/batch-with-specimens.test.ts @@ -1,3 +1,4 @@ +import { ConflictError } from '../../error-handler' import { describe, it, expect, beforeEach, afterEach } from 'vitest' import { setupTestDatabase, cleanupTestDatabase } from '../../../__tests__/helpers/db-setup' import { @@ -10,8 +11,8 @@ import { } from '../../../__tests__/helpers/factories' import { setContainerDefaults, clearSettingsCache } from '../../settings' import { clearDefaultsCache } from '../../defaults' -import { createBatchWithSpecimens } from '../batch-with-specimens' -import { specimenTypeContainerType, containerTypeUnit, storageContainer, box } from '../../../db/schema' +import { createBatchWithSpecimens, createBatchesWithSpecimens } from '../batch-with-specimens' +import { specimenTypeContainerType, containerTypeUnit, storageContainer, box, controlBatch } from '../../../db/schema' import { eq } from 'drizzle-orm' import type { Database } from '../../../db/client' import { utcNow } from '../../datetime' @@ -372,4 +373,67 @@ describe('batch-with-specimens', () => { expect(created.unitId).toBe(unit.id) }) }) + + describe('createBatchesWithSpecimens', () => { + it('creates no batches when a later batch fails', async () => { + const unit = await createTestUnit(testDb, { symbol: 'uL', name: 'microliter', category: 'volume' }) + await setContainerDefaults(testDb, { + micronix_tube: { totalQuantity: 1, remainingQuantity: 1, defaultUnitSymbol: 'uL' }, + }) + await testDb.insert(containerTypeUnit).values({ containerType: 'micronix_tube', unitId: unit.id }) + const def100 = await createTestControlDefinition(testDb, { name: 'D100' }) + const def200 = await createTestControlDefinition(testDb, { name: 'D200' }) + const specimenType = await createTestSpecimenType(testDb, { name: 'DNA' }) + await testDb.insert(specimenTypeContainerType).values({ specimenTypeId: specimenType.id, containerType: 'micronix_tube', created: utcNow() }) + const storageType = await createTestStorageType(testDb, { name: 'Freezer' }) + const location = await createTestLocation(testDb, { name: 'Loc', storageTypeId: String(storageType.id) }) + const plate = await createTestMicronixPlate(testDb, { name: 'Plate1', locationId: location.id }) + const tube = (barcode: string, position: string) => ({ + containerType: 'micronix_tube' as const, + barcode, + collection: { type: 'micronix_plate' as const, id: plate.id, position }, + }) + + await expect( + createBatchesWithSpecimens(testDb, [ + { batch: { controlDefinitionId: def100.id, name: 'B100' }, specimens: [{ specimenTypeName: 'DNA', containers: [tube('MT1', 'A01')] }] }, + { batch: { controlDefinitionId: def200.id, name: 'B200' }, specimens: [{ specimenTypeName: 'Nope', containers: [tube('MT2', 'A02')] }] }, + ]), + ).rejects.toThrow(/^Control batch 2 \('B200'\): .*\. No control batches were created\.$/) + + expect(await testDb.select().from(controlBatch).all()).toHaveLength(0) + expect(await testDb.select().from(storageContainer).all()).toHaveLength(0) + }) + + it('reports a UNIQUE violation as a conflict', async () => { + const unit = await createTestUnit(testDb, { symbol: 'uL', name: 'microliter', category: 'volume' }) + await setContainerDefaults(testDb, { + micronix_tube: { totalQuantity: 1, remainingQuantity: 1, defaultUnitSymbol: 'uL' }, + }) + await testDb.insert(containerTypeUnit).values({ containerType: 'micronix_tube', unitId: unit.id }) + const def100 = await createTestControlDefinition(testDb, { name: 'D100' }) + const def200 = await createTestControlDefinition(testDb, { name: 'D200' }) + const specimenType = await createTestSpecimenType(testDb, { name: 'DNA' }) + await testDb.insert(specimenTypeContainerType).values({ specimenTypeId: specimenType.id, containerType: 'micronix_tube', created: utcNow() }) + const storageType = await createTestStorageType(testDb, { name: 'Freezer' }) + const location = await createTestLocation(testDb, { name: 'Loc', storageTypeId: String(storageType.id) }) + const plate = await createTestMicronixPlate(testDb, { name: 'Plate1', locationId: location.id }) + const tube = (barcode: string, position: string) => ({ + containerType: 'micronix_tube' as const, + barcode, + collection: { type: 'micronix_plate' as const, id: plate.id, position }, + }) + + // Stand-in for any constraint the app doesn't check first: one specimen per type. + sqlite.exec('CREATE UNIQUE INDEX test_one_specimen_per_type ON specimen(specimen_type_id)') + + const attempt = createBatchesWithSpecimens(testDb, [ + { batch: { controlDefinitionId: def100.id, name: 'B100' }, specimens: [{ specimenTypeName: 'DNA', containers: [tube('MT1', 'A01')] }] }, + { batch: { controlDefinitionId: def200.id, name: 'B200' }, specimens: [{ specimenTypeName: 'DNA', containers: [tube('MT2', 'A02')] }] }, + ]) + await expect(attempt).rejects.toBeInstanceOf(ConflictError) + await expect(attempt).rejects.toThrow(/^Control batch 2 \('B200'\): .*No control batches were created\.$/) + expect(await testDb.select().from(controlBatch).all()).toHaveLength(0) + }) + }) }) diff --git a/packages/api/src/lib/controls/batch-schemas.ts b/packages/api/src/lib/controls/batch-schemas.ts index 11601c6c..e21f3e81 100644 --- a/packages/api/src/lib/controls/batch-schemas.ts +++ b/packages/api/src/lib/controls/batch-schemas.ts @@ -73,6 +73,11 @@ export const createBatchWithSpecimensSchema = z.object({ specimens: z.array(batchSpecimenInputSchema).min(1), }) +/** Several batches (e.g. one per density from a composition CSV), created all-or-nothing. */ +export const createBatchesWithSpecimensSchema = z.object({ + batches: z.array(createBatchWithSpecimensSchema).min(1), +}) + export const addSpecimensToBatchSchema = z.object({ specimens: z.array(batchSpecimenInputSchema).min(1), }) diff --git a/packages/api/src/lib/controls/batch-with-specimens.ts b/packages/api/src/lib/controls/batch-with-specimens.ts index e1ace72a..7b03b5f3 100644 --- a/packages/api/src/lib/controls/batch-with-specimens.ts +++ b/packages/api/src/lib/controls/batch-with-specimens.ts @@ -26,7 +26,7 @@ import { import type { BatchContainerInput } from './batch-schemas' import { withWriteTransaction } from '../../db/write-transaction' import { validateCollectionDate } from '../validation' -import { ValidationError } from '../error-handler' +import { ConflictError, ValidationError, sqliteConstraintConflict } from '../error-handler' export interface CreateBatchWithSpecimensRequest { batch: { @@ -379,3 +379,30 @@ export async function addSpecimensToBatch( } }) } + +/** + * Create several batches with their specimens in one transaction, so a failure on any + * batch leaves none of them behind (retrying would otherwise duplicate the earlier ones). + */ +export async function createBatchesWithSpecimens( + database: Database, + batches: CreateBatchWithSpecimensRequest[], +): Promise>>> { + return withWriteTransaction(database, async (tx) => { + const results: Array>> = [] + for (const [index, data] of batches.entries()) { + try { + results.push(await createBatchWithSpecimens(tx, data)) + } catch (error: unknown) { + // Keep a conflict a 409 (ConflictError or a UNIQUE/FK violation); the rest are bad input. + const conflict = error instanceof Error ? sqliteConstraintConflict(error) : null + const reason = conflict ?? (error instanceof Error ? error.message : String(error)) + const sentence = /[.!?]$/.test(reason) ? reason : `${reason}.` + const message = `Control batch ${index + 1} ('${data.batch.name}'): ${sentence} No control batches were created.` + if (conflict || error instanceof ConflictError) throw new ConflictError(message) + throw new ValidationError(message) + } + } + return results + }) +} diff --git a/packages/api/src/lib/controls/strain-composition.ts b/packages/api/src/lib/controls/strain-composition.ts index c3c6118a..69956545 100644 --- a/packages/api/src/lib/controls/strain-composition.ts +++ b/packages/api/src/lib/controls/strain-composition.ts @@ -1,5 +1,5 @@ import { z } from 'zod' -import { eq, inArray } from 'drizzle-orm' +import { and, eq, inArray, ne } from 'drizzle-orm' import type { Database } from '../../db/client' import { controlDefinition, strain } from '../../db/schema' @@ -175,12 +175,13 @@ export async function resolveStrainComposition( export async function findBloodControlDefinitionByComposition( database: Database, criteria: CompositionCriteria, - options?: CompositionMatchOptions, + options?: CompositionMatchOptions & { /** Skip this definition, e.g. the one being edited. */ excludeId?: number }, ): Promise<(typeof controlDefinition.$inferSelect) | null> { + const isBlood = eq(controlDefinition.controlType, 'blood') const definitions = await database .select() .from(controlDefinition) - .where(eq(controlDefinition.controlType, 'blood')) + .where(options?.excludeId === undefined ? isBlood : and(isBlood, ne(controlDefinition.id, options.excludeId))) for (const definition of definitions) { if (definitionCompositionMatches(definition.properties, criteria, options)) { diff --git a/packages/api/src/lib/datetime.ts b/packages/api/src/lib/datetime.ts index ae8e4d05..538aba43 100644 --- a/packages/api/src/lib/datetime.ts +++ b/packages/api/src/lib/datetime.ts @@ -2,3 +2,13 @@ export function utcNow(): string { return new Date().toISOString().replace('T', ' ').replace('Z', '').split('.')[0] } + +/** + * The calendar date `days` after a value that starts with YYYY-MM-DD (negative goes back), or + * null when it doesn't start with one. Month and year rollovers follow the calendar. + */ +export function shiftCalendarDate(date: string, days: number): string | null { + const match = /^(\d{4})-(\d{2})-(\d{2})/.exec(date) + if (!match) return null + return new Date(Date.UTC(Number(match[1]), Number(match[2]) - 1, Number(match[3]) + days)).toISOString().slice(0, 10) +} diff --git a/packages/api/src/lib/derivations-csv.ts b/packages/api/src/lib/derivations-csv.ts index c39ad74d..78ad65b2 100644 --- a/packages/api/src/lib/derivations-csv.ts +++ b/packages/api/src/lib/derivations-csv.ts @@ -1,6 +1,8 @@ import type { Database } from '../db/client' import type { DatabaseOrTransaction } from './db-types' import { + bag, + box, containerDerivation, cryovialBox, cryovialTube, @@ -24,6 +26,8 @@ import { import { createDerivation, type CreateDerivationInput } from './derivations' import { withWriteTransaction } from '../db/write-transaction' import { resolveParentContainerId } from './derivations/parent-container-resolver' +import { normalizePosition } from './normalize-position' +import { checkGridPositionOccupancy } from './container-occupancy' /** * Map technical DB/constraint errors to messages that are clear for non-technical users. @@ -157,6 +161,23 @@ function parseBoolean(value?: string): boolean | undefined { return v === 'true' || v === '1' || v === 'yes' || v === 'y' } +const BOOLEAN_WORDS = new Set(['true', 'false', '1', '0', 'yes', 'no', 'y', 'n']) + +/** First problem with a row's typed cells, so a bad value is reported instead of silently dropped. */ +function rowValueError(row: DerivationCsvRow): string | null { + for (const column of ['quantity', 'quantity_used'] as const) { + const raw = row[column] + if (raw != null && raw.trim() !== '' && parseNumber(raw) === undefined) { + return `${column} '${raw}' is not a number` + } + } + const flag = row.reduce_parent_quantity + if (flag != null && flag.trim() !== '' && !BOOLEAN_WORDS.has(flag.trim().toLowerCase())) { + return `reduce_parent_quantity '${flag}' must be true or false` + } + return null +} + function parseNumber(value?: string): number | undefined { if (value == null || value.trim() === '') return undefined const n = Number(value) @@ -203,6 +224,11 @@ function buildChildSummary(row: DerivationCsvRow): string { return 'Child' } +/** Paper rows name their parent with box_name or bag_name. */ +function paperParentTypeFor(row: DerivationCsvRow): 'box' | 'bag' { + return row.box_name ? 'box' : row.bag_name ? 'bag' : 'box' +} + function getCollectionNameForType( row: DerivationCsvRow, containerType: DerivationCsvRow['container_type'] | 'micronix_tube', @@ -248,11 +274,13 @@ function derivationCsvRowToContainerWrite( * skipped, matching the previous line-based parser. */ export function parseCsv(text: string): DerivationCsvRow[] { + // Drop rows with no values at all (blank lines, or Excel's trailing ",,,," rows). const cells = parseCsvCells(text) - .filter(row => !(row.length === 1 && row[0].trim() === '')) + .filter(row => row.some(cell => cell.trim() !== '')) if (cells.length === 0) return [] - const headers = cells[0].map(h => h.trim()) + // Headers are matched case-insensitively ("Plate_Name" -> plate_name). + const headers = cells[0].map(h => h.trim().toLowerCase()) const rows: DerivationCsvRow[] = [] for (let i = 1; i < cells.length; i++) { @@ -272,6 +300,7 @@ async function resolveCollectionId( containerType: DerivationCsvRow['container_type'], collectionName?: string, collectionBarcode?: string, + paperParentType: 'box' | 'bag' = 'box', ): Promise<{ id?: number; status: 'existing' | 'will_be_created' }> { if (!collectionName && !collectionBarcode) { return { status: 'will_be_created' } @@ -318,12 +347,16 @@ async function resolveCollectionId( } if (containerType === 'paper') { - const sheetRec = await database - .select({ id: paper.sheetId }) - .from(paper) - .limit(1) + // Paper goes on a sheet in the named box or bag; the sheet is created if missing, + // but the box or bag itself must already exist. + if (!collectionName) return { status: 'will_be_created' } + const parentTable = paperParentType === 'bag' ? bag : box + const parent = await database + .select({ id: parentTable.id }) + .from(parentTable) + .where(eq(parentTable.name, collectionName.trim())) .get() - return { id: sheetRec?.id, status: sheetRec?.id ? 'existing' : 'will_be_created' } + return { id: parent?.id, status: parent ? 'existing' : 'will_be_created' } } return { status: 'will_be_created' } @@ -388,6 +421,14 @@ export async function validateDerivationsCsv( } } + const valueError = rowValueError(row) + if (valueError) { + validationRow.error = valueError + validationRows.push(validationRow) + invalidCount++ + continue + } + const containerType = row.container_type || settings?.containerType || 'micronix_tube' const collectionName = getCollectionNameForType(row, containerType) if ((containerType === 'micronix_tube' || containerType === 'cryovial_tube') && !collectionName && !row.collection_barcode) { @@ -429,10 +470,10 @@ export async function validateDerivationsCsv( if (parentSpecimen) { currentParentSpecimenTypeId = parentSpecimen.specimenTypeId - // Store first row's specimen type for comparison - if (i === 0) { + // Compare against the first row whose parent resolved + if (firstParentSpecimenTypeId === null) { firstParentSpecimenTypeId = currentParentSpecimenTypeId - } else if (firstParentSpecimenTypeId !== null && currentParentSpecimenTypeId !== firstParentSpecimenTypeId) { + } else if (currentParentSpecimenTypeId !== firstParentSpecimenTypeId) { validationRow.warnings = validationRow.warnings || [] validationRow.warnings.push('Source specimen type does not match other rows') warningCount++ @@ -471,12 +512,22 @@ export async function validateDerivationsCsv( containerType, collectionName, row.collection_barcode, + paperParentTypeFor(row), ) validationRow.collectionStatus = collectionInfo.status + if (containerType === 'paper' && collectionInfo.status !== 'existing') { + const kind = paperParentTypeFor(row) + validationRow.error = `${kind === 'bag' ? 'Bag' : 'Box'} '${collectionName ?? ''}' not found. Create it before importing paper derivations.` + validationRows.push(validationRow) + invalidCount++ + continue + } + // For tube types, validate position and check for duplicate (existing collection or within CSV) if (containerType === 'micronix_tube' || containerType === 'cryovial_tube') { - const position = (row.position ?? '').toString().trim() + // Normalize ("A2" -> "A02") so checks match what the import stores. + const position = normalizePosition(row.position) ?? '' if (!position) { validationRow.error = 'position is required for each row when deriving to a plate or box' validationRows.push(validationRow) @@ -488,25 +539,21 @@ export async function validateDerivationsCsv( // Check if position is already used in an existing collection if (collectionInfo.id !== undefined) { + // Shared check: on a plate, tubes and static wells both occupy cells. + const occupancy = await checkGridPositionOccupancy(database as Database, { + collectionKind: containerType === 'micronix_tube' ? 'micronix_plate' : 'cryovial_box', + collectionId: collectionInfo.id, + position, + }) if (containerType === 'micronix_tube') { - const existing = await database - .select({ id: micronixTube.id }) - .from(micronixTube) - .where(and(eq(micronixTube.collectionId, collectionInfo.id), eq(micronixTube.position, position))) - .get() - if (existing) { + if (occupancy.occupied) { validationRow.error = 'That position is already used in that plate. Each position in a plate can only be used once. Use a different position or a different plate.' validationRows.push(validationRow) invalidCount++ continue } } else { - const existing = await database - .select({ id: cryovialTube.id }) - .from(cryovialTube) - .where(and(eq(cryovialTube.collectionId, collectionInfo.id), eq(cryovialTube.position, position))) - .get() - if (existing) { + if (occupancy.occupied) { validationRow.error = 'That position is already used in that box. Each position in a box can only be used once. Use a different position or a different box.' validationRows.push(validationRow) invalidCount++ @@ -649,6 +696,8 @@ export async function importDerivationsFromCsv( for (let i = 0; i < rows.length; i++) { const row = rows[i] try { + const valueError = rowValueError(row) + if (valueError) throw new Error(valueError) const parentContainerId = await resolveParentContainerId(tx, row) const childContainerType = (settings?.containerType || row.container_type!) as 'micronix_tube' | 'cryovial_tube' | 'paper' const containerParsed = derivationCsvRowToContainerWrite(row, childContainerType) @@ -660,6 +709,7 @@ export async function importDerivationsFromCsv( childContainerType, getCollectionNameForType(row, childContainerType), row.collection_barcode, + paperParentTypeFor(row), ) const input: CreateDerivationInput = { @@ -670,9 +720,8 @@ export async function importDerivationsFromCsv( quantity: row.quantity ? parseNumber(row.quantity) : settings?.quantity, unitSymbol: row.unit_symbol || settings?.unitSymbol, quantityUsed: row.quantity_used ? parseNumber(row.quantity_used) : settings?.quantityUsed, - reduceParentQuantity: row.reduce_parent_quantity !== undefined - ? parseBoolean(row.reduce_parent_quantity) - : settings?.reduceParentQuantity, + // A blank cell falls back to the shared setting rather than the library default. + reduceParentQuantity: parseBoolean(row.reduce_parent_quantity) ?? settings?.reduceParentQuantity, derivationDate: settings?.derivationDate || row.derivation_date!, protocol: settings?.protocol || row.protocol!, notes: row.notes, @@ -752,6 +801,7 @@ export async function importDerivationsFromCsv( containerType, getCollectionNameForType(row, containerType), row.collection_barcode, + paperParentTypeFor(row), ) results.push({ diff --git a/packages/api/src/lib/error-handler.ts b/packages/api/src/lib/error-handler.ts index d613a87b..6afbdd78 100644 --- a/packages/api/src/lib/error-handler.ts +++ b/packages/api/src/lib/error-handler.ts @@ -29,6 +29,25 @@ function isZodError(error: unknown): error is z.ZodError { return 'issues' in obj && Array.isArray(obj.issues) } +/** Readable message for a SQLite UNIQUE / FOREIGN KEY violation, or null for other errors. */ +export function sqliteConstraintConflict(error: Error): string | null { + const code = (error as { code?: unknown }).code + if (code === 'SQLITE_CONSTRAINT_UNIQUE' || code === 'SQLITE_CONSTRAINT_PRIMARYKEY') { + // "UNIQUE constraint failed: study.title, study.x" -> "title, x" + const columns = error.message + .replace(/^.*constraint failed:\s*/i, '') + .split(',') + .map((col) => col.trim().split('.').pop()?.replace(/_/g, ' ')) + .filter(Boolean) + .join(', ') + return columns ? `A record with this ${columns} already exists.` : 'This record already exists.' + } + if (code === 'SQLITE_CONSTRAINT_FOREIGNKEY') { + return 'This record is still referenced by other data, or refers to a record that does not exist.' + } + return null +} + export function handleRouteError(error: unknown, c: Context): Response { const database = getRequestDatabase(c) @@ -131,6 +150,18 @@ export function handleRouteError(error: unknown, c: Context): Response { }, 400) } + // A UNIQUE or FOREIGN KEY violation is a conflict with existing data, not a server fault. + // Reply 409 with a readable message instead of the raw SQLite text. + const constraint = sqliteConstraintConflict(error) + if (constraint) { + logError(database, 'backend', 'warning', error.message, error, errorContext).catch(logPersistFailure) + return c.json({ + error: constraint, + errorCode: 'CONFLICT', + ...(isDevelopment && { details: error.message }), + }, 409) + } + // Log error asynchronously (non-blocking) logBackendError(database, error, errorContext).catch(logPersistFailure) return c.json({ diff --git a/packages/api/src/lib/export/__tests__/format.test.ts b/packages/api/src/lib/export/__tests__/format.test.ts index d25f9a89..11cc384d 100644 --- a/packages/api/src/lib/export/__tests__/format.test.ts +++ b/packages/api/src/lib/export/__tests__/format.test.ts @@ -203,9 +203,15 @@ describe('formatSimpleCSV', () => { ]) }) - it('handles cells starting with equals sign as plain text', () => { + it('neutralizes cells a spreadsheet would run as a formula', () => { const result = formatSimpleCSV(['formula'], [['=SUM(A1:A10)']], lfNoBom) - expect(result).toBe('formula\n=SUM(A1:A10)') + expect(result).toBe("formula\n'=SUM(A1:A10)") + }) + + it('leaves negative numbers unchanged but neutralizes other "-" text', () => { + const result = formatSimpleCSV(['subject', 'comment'], [['-131', '-2 in SubjID']], lfNoBom) + // Only whole numbers are spared; text after a leading "-" could be a formula. + expect(result).toBe("subject,comment\n-131,'-2 in SubjID") }) }) }) diff --git a/packages/api/src/lib/export/__tests__/query.test.ts b/packages/api/src/lib/export/__tests__/query.test.ts index 680bb5a2..45425920 100644 --- a/packages/api/src/lib/export/__tests__/query.test.ts +++ b/packages/api/src/lib/export/__tests__/query.test.ts @@ -10,8 +10,14 @@ import { createTestUnit, } from '../../../__tests__/helpers/factories' import type { Database } from '../../../db/client' -import { storageContainerTag } from '../../../db/schema' -import { buildContainerQuery, resolveMicronixBarcodesToContainers, buildContainerQueryByMicronixBarcodes } from '../query' +import { storageContainerTag, specimen as specimenTable } from '../../../db/schema' +import { utcNow } from '../../datetime' +import { + buildContainerQuery, + buildMultiStudyContainerQuery, + resolveMicronixBarcodesToContainers, + buildContainerQueryByMicronixBarcodes, +} from '../query' describe('buildContainerQuery', () => { let testDb: Database @@ -170,3 +176,53 @@ describe('buildContainerQueryByMicronixBarcodes', () => { }) }) + +describe('buildMultiStudyContainerQuery subject dates', () => { + let testDb: Database + let sqlite: Awaited>['sqlite'] + + beforeEach(async () => { + const setup = await setupTestDatabase() + testDb = setup.db + sqlite = setup.sqlite + }) + + afterEach(() => { + if (sqlite) cleanupTestDatabase(sqlite) + }) + + it('keeps every listed visit per subject and includes subjects listed without a date', async () => { + const study = await createTestStudy(testDb, { title: 'Visits', shortCode: 'VIS', leadPerson: 'L' }) + const s01 = await createTestStudySubject(testDb, { studyId: study.id, name: 'S01' }) + const s02 = await createTestStudySubject(testDb, { studyId: study.id, name: 'S02' }) + const type = await createTestSpecimenType(testDb, { name: 'Blood' }) + const unit = await createTestUnit(testDb, { symbol: 'uL', name: 'microliter', category: 'volume' }) + const now = utcNow() + const addSpecimen = async (subjectId: number, collectionDate: string) => { + const [row] = await testDb + .insert(specimenTable) + .values({ studySubjectId: subjectId, specimenTypeId: type.id, collectionDate, created: now, lastUpdated: now }) + .returning() + await createTestStorageContainer(testDb, { specimenId: row.id, unitId: unit.id }) + return row + } + const jan = await addSpecimen(s01.id, '2024-01-01') + const feb = await addSpecimen(s01.id, '2024-02-01') + await addSpecimen(s01.id, '2024-03-01') + const may = await addSpecimen(s02.id, '2024-05-05') + + const result = await buildMultiStudyContainerQuery( + testDb, + [ + { study_short_code: 'VIS', subject_name: 'S01', collection_date: '2024-01-01' }, + { study_short_code: 'VIS', subject_name: 'S01', collection_date: '2024-02-01' }, + { study_short_code: 'VIS', subject_name: 'S02' }, + ], + {}, + ) + + expect(result.containers.map((c) => c.specimen_id).sort((a, b) => a - b)).toEqual( + [jan.id, feb.id, may.id].sort((a, b) => a - b), + ) + }) +}) diff --git a/packages/api/src/lib/export/format.ts b/packages/api/src/lib/export/format.ts index c0814cba..3536ccdc 100644 --- a/packages/api/src/lib/export/format.ts +++ b/packages/api/src/lib/export/format.ts @@ -22,7 +22,7 @@ export function formatSimpleCSV( } const formattedRows = rows.map((row) => formatExportRow(headers, row)) - return serializeCsv(headers, formattedRows, options) + return serializeCsv(headers, formattedRows, { ...options, neutralizeFormulas: true }) } export async function formatAsCSV( @@ -41,7 +41,7 @@ export async function formatAsCSV( columnKeys.map((header) => formatExportCellValue(header, (row as any)[header])) ) - return serializeCsv(columnKeys, rows, options) + return serializeCsv(columnKeys, rows, { ...options, neutralizeFormulas: true }) } export async function formatAsJSON( diff --git a/packages/api/src/lib/export/query.ts b/packages/api/src/lib/export/query.ts index 8b40f5e2..c7c36d90 100644 --- a/packages/api/src/lib/export/query.ts +++ b/packages/api/src/lib/export/query.ts @@ -11,6 +11,7 @@ import { resolveSubjectsByStudyGrouped } from '../identifier-resolution' import { resolveContainerIdsWithAllTags } from '../container-tag-filter' import { enrichContainerData } from './enrich' import { filterContainerIdsByType } from './filter' +import type { SubjectDateFilter } from '@sampledb/contract' import type { ContainerExportData, ExportFilters, @@ -20,6 +21,8 @@ import type { StudyRecord, } from './types' import { buildExportSummary, validateStudyCodes } from './validate' +import { chunkArray, dateToUpperBound } from '../statistics/helpers' +import { shiftCalendarDate } from '../datetime' export async function buildContainerQuery(database: Database, filters: ExportFilters) { // First, get the study and its subjects @@ -76,46 +79,35 @@ export async function buildContainerQuery(database: Database, filters: ExportFil // Handle per-subject date filtering if (filters.subject_dates && Object.keys(filters.subject_dates).length > 0) { const tolerance = filters.date_tolerance || 0 + const subjectDates = filters.subject_dates const subjectDateConditions: any[] = [] - - // Build conditions for each subject with date filters - for (const [subjectIdStr, dateFilter] of Object.entries(filters.subject_dates)) { - const subjectId = parseInt(subjectIdStr) - if (isNaN(subjectId) || !filteredSubjectIds.includes(subjectId)) continue - - const subjectConditions: any[] = [eq(specimen.studySubjectId, subjectId)] - - if ('exact' in dateFilter) { - // Convert exact date with tolerance to range - const exactDate = dateFilter.exact - const fromDate = new Date(exactDate) - fromDate.setDate(fromDate.getDate() - tolerance) - const toDate = new Date(exactDate) - toDate.setDate(toDate.getDate() + tolerance) - - subjectConditions.push( - and( - gte(specimen.collectionDate, fromDate.toISOString().split('T')[0]), - lte(specimen.collectionDate, toDate.toISOString().split('T')[0]) - ) as any - ) - } else if ('from' in dateFilter || 'to' in dateFilter) { - // Date range - if (dateFilter.from) { - subjectConditions.push(gte(specimen.collectionDate, dateFilter.from)) - } - if (dateFilter.to) { - subjectConditions.push(lte(specimen.collectionDate, dateFilter.to)) - } + const undatedSubjectIds: number[] = [] + + for (const subjectId of filteredSubjectIds) { + const entry = subjectDates[subjectId] as SubjectDateFilter | SubjectDateFilter[] | undefined + const dateConditions = (entry == null ? [] : Array.isArray(entry) ? entry : [entry]) + .map((dateFilter) => subjectDateCondition(dateFilter, tolerance)) + .filter((condition) => condition != null) + if (dateConditions.length === 0) { + // Listed without a date: every collection date for this subject. + undatedSubjectIds.push(subjectId) + continue } - - subjectDateConditions.push(and(...subjectConditions) as any) + // Several entries for one subject (one per visit) are alternatives. + subjectDateConditions.push(and(eq(specimen.studySubjectId, subjectId), or(...dateConditions))) } - - // If we have per-subject date conditions, use them (OR for different subjects) - if (subjectDateConditions.length > 0) { - specimenConditions.push(or(...subjectDateConditions) as any) + + if (undatedSubjectIds.length > 0) { + subjectDateConditions.push( + and( + inArray(specimen.studySubjectId, undatedSubjectIds), + ...(filters.date_from ? [gte(specimen.collectionDate, filters.date_from)] : []), + ...(filters.date_to ? [lte(specimen.collectionDate, filters.date_to)] : []), + ), + ) } + + specimenConditions.push(or(...subjectDateConditions) as any) } else { // Use global date filters if no per-subject dates if (filters.date_from) { @@ -151,21 +143,25 @@ export async function buildContainerQuery(database: Database, filters: ExportFil // Build conditions for container query const containerConditions: any[] = [] - containerConditions.push(inArray(storageContainer.specimenId, specimenIds)) if (filters.created_from) { containerConditions.push(gte(storageContainer.created, filters.created_from)) } if (filters.created_to) { - containerConditions.push(lte(storageContainer.created, filters.created_to)) + containerConditions.push(dateToUpperBound(storageContainer.created, filters.created_to)) } - // Get matching containers - let containers = await database - .select() - .from(storageContainer) - .where(and(...containerConditions) as any) + // Get matching containers, chunking specimen ids to stay under SQLite's parameter limit + let containers: Array = [] + for (const idChunk of chunkArray(specimenIds, 5000)) { + containers.push( + ...(await database + .select() + .from(storageContainer) + .where(and(inArray(storageContainer.specimenId, idChunk), ...containerConditions) as any)), + ) + } if (filters.tag_ids && filters.tag_ids.length > 0) { const matchingIds = new Set(await resolveContainerIdsWithAllTags(database, filters.tag_ids)) @@ -282,6 +278,24 @@ export async function buildContainerQueryByMicronixBarcodes( return { containers, specimens, studies, subjectToStudyMap } } +/** Collection date condition for one per-subject filter (exact date +/- tolerance days, or a range). */ +function subjectDateCondition(dateFilter: SubjectDateFilter, toleranceDays: number) { + if ('exact' in dateFilter) { + const from = shiftCalendarDate(dateFilter.exact, -toleranceDays) + const to = shiftCalendarDate(dateFilter.exact, toleranceDays) + if (!from || !to) return eq(specimen.collectionDate, dateFilter.exact) + return and( + gte(specimen.collectionDate, from), + lte(specimen.collectionDate, to), + ) + } + const bounds = [ + ...(dateFilter.from ? [gte(specimen.collectionDate, dateFilter.from)] : []), + ...(dateFilter.to ? [lte(specimen.collectionDate, dateFilter.to)] : []), + ] + return bounds.length > 0 ? and(...bounds) : null +} + export async function buildMultiStudyContainerQuery( database: Database, entries: MultiStudyExportEntry[], @@ -329,20 +343,22 @@ export async function buildMultiStudyContainerQuery( const subjectsByStudy = await resolveSubjectsByStudyGrouped(database, subjectResolutionEntries) // Build subject dates map (by study and subject name) - const subjectDatesByStudy = new Map>() + // A subject may appear on several rows (one per visit); keep every date. A row without + // a date means all of that subject's dates, so it wins over dated rows. + const subjectDatesByStudy = new Map>() for (const [studyId, studyEntries] of entriesByStudy.entries()) { - const datesMap = new Map() + const datesMap = new Map() for (const entry of studyEntries) { - if (entry.collection_date) { - datesMap.set(entry.subject_name, { exact: entry.collection_date }) - } else if (entry.date_from || entry.date_to) { - datesMap.set(entry.subject_name, { - from: entry.date_from, - to: entry.date_to, - }) - } + const existing = datesMap.get(entry.subject_name) + if (existing === 'all') continue + const dateFilter: SubjectDateFilter | null = entry.collection_date + ? { exact: entry.collection_date } + : entry.date_from || entry.date_to + ? { from: entry.date_from, to: entry.date_to } + : null + datesMap.set(entry.subject_name, dateFilter ? [...(existing ?? []), dateFilter] : 'all') } - if (datesMap.size > 0) { + if ([...datesMap.values()].some((v) => v !== 'all')) { subjectDatesByStudy.set(studyId, datesMap) } } @@ -383,10 +399,10 @@ export async function buildMultiStudyContainerQuery( const studySubjectDates = subjectDatesByStudy.get(studyId) if (studySubjectDates && studySubjectDates.size > 0) { studyFilters.subject_dates = {} - for (const [subjectName, dateFilter] of studySubjectDates.entries()) { + for (const [subjectName, dateFilters] of studySubjectDates.entries()) { const subjectId = subjectMap.get(subjectName) - if (subjectId) { - studyFilters.subject_dates[subjectId] = dateFilter + if (subjectId && dateFilters !== 'all') { + studyFilters.subject_dates[subjectId] = dateFilters } } } diff --git a/packages/api/src/lib/plate-csv.ts b/packages/api/src/lib/plate-csv.ts index 0c094dfb..0b46dbcb 100644 --- a/packages/api/src/lib/plate-csv.ts +++ b/packages/api/src/lib/plate-csv.ts @@ -1,3 +1,4 @@ +import { parseCsv } from '@sampledb/contract' import type { ScannerConfiguration } from './settings' /** Normalize well position to A01 style (row + 2-digit column) */ @@ -30,13 +31,13 @@ export function parsePlateCSV( csvText: string, config: ScannerConfiguration ): { wellPosition: string; barcode: string }[] { - const lines = csvText.split('\n').map((l) => l.trim()).filter(Boolean) + // Shared parser: handles quoted fields ("A01","FR123"), BOM, and CRLF. + const lines = parseCsv(csvText, { skipBlankRows: true }) if (lines.length <= config.skipRows) return [] - const headerLine = lines[config.skipRows] - const headers = headerLine.split(',').map((h) => h.trim()) + const headers = lines[config.skipRows].map((h) => h.trim()) const rows: { wellPosition: string; barcode: string }[] = [] for (let i = config.skipRows + 1; i < lines.length; i++) { - const values = lines[i].split(',').map((v) => v.trim()) + const values = lines[i].map((v) => v.trim()) const row: Record = {} headers.forEach((h, j) => { row[h] = values[j] ?? '' diff --git a/packages/api/src/lib/search/__tests__/subject-search.test.ts b/packages/api/src/lib/search/__tests__/subject-search.test.ts index b30ce07a..a93f08ce 100644 --- a/packages/api/src/lib/search/__tests__/subject-search.test.ts +++ b/packages/api/src/lib/search/__tests__/subject-search.test.ts @@ -41,4 +41,14 @@ describe('subject-search', () => { url: `/subjects/${subject.id}`, }) }) + + it('treats _ and % in the query literally', async () => { + const studyRecord = await createTestStudy(testDb, { title: 'Controls', shortCode: 'CTL' }) + await createTestStudySubject(testDb, { studyId: studyRecord.id, name: '3D7_0.05' }) + await createTestStudySubject(testDb, { studyId: studyRecord.id, name: '3D7X0.05' }) + await createTestStudySubject(testDb, { studyId: studyRecord.id, name: '100% pure' }) + + expect((await searchSubjects(testDb, '3D7_0')).map((r) => r.title)).toEqual(['3D7_0.05']) + expect((await searchSubjects(testDb, '0%')).map((r) => r.title)).toEqual(['100% pure']) + }) }) diff --git a/packages/api/src/lib/search/collection-search.ts b/packages/api/src/lib/search/collection-search.ts index cd800a56..6c4392af 100644 --- a/packages/api/src/lib/search/collection-search.ts +++ b/packages/api/src/lib/search/collection-search.ts @@ -8,9 +8,10 @@ import { controlBatch, controlDefinition, } from '../../db/schema' -import { eq, or, like } from 'drizzle-orm' +import { eq, or } from 'drizzle-orm' import { formatLocationPath } from '../container-enrichment' import type { SearchResult } from './types' +import { likeContains } from '../sql-like' /** Search collections and control batches by name, barcode, or numeric id. */ export async function searchCollections(database: Database, query: string): Promise { @@ -32,7 +33,7 @@ export async function searchCollections(database: Database, query: string): Prom .where( isNumeric ? eq(micronixPlate.id, queryNum) - : or(like(micronixPlate.name, `%${query}%`), like(micronixPlate.barcode, `%${query}%`))!, + : or(likeContains(micronixPlate.name, query), likeContains(micronixPlate.barcode, query))!, ) .limit(10) @@ -68,7 +69,7 @@ export async function searchCollections(database: Database, query: string): Prom .where( isNumeric ? eq(cryovialBox.id, queryNum) - : or(like(cryovialBox.name, `%${query}%`), like(cryovialBox.barcode, `%${query}%`))!, + : or(likeContains(cryovialBox.name, query), likeContains(cryovialBox.barcode, query))!, ) .limit(10) @@ -100,7 +101,7 @@ export async function searchCollections(database: Database, query: string): Prom }) .from(box) .leftJoin(location, eq(box.locationId, location.id)) - .where(isNumeric ? eq(box.id, queryNum) : like(box.name, `%${query}%`)) + .where(isNumeric ? eq(box.id, queryNum) : likeContains(box.name, query)) .limit(10) for (const paperBox of boxes) { @@ -126,7 +127,7 @@ export async function searchCollections(database: Database, query: string): Prom }) .from(bag) .leftJoin(location, eq(bag.locationId, location.id)) - .where(isNumeric ? eq(bag.id, queryNum) : like(bag.name, `%${query}%`)) + .where(isNumeric ? eq(bag.id, queryNum) : likeContains(bag.name, query)) .limit(10) for (const paperBag of bags) { @@ -153,7 +154,7 @@ export async function searchCollections(database: Database, query: string): Prom .where( isNumeric ? eq(controlBatch.id, queryNum) - : or(like(controlBatch.name, `%${query}%`), like(controlDefinition.name, `%${query}%`))!, + : or(likeContains(controlBatch.name, query), likeContains(controlDefinition.name, query))!, ) .limit(10) diff --git a/packages/api/src/lib/search/container-search.ts b/packages/api/src/lib/search/container-search.ts index 07041022..4323d93d 100644 --- a/packages/api/src/lib/search/container-search.ts +++ b/packages/api/src/lib/search/container-search.ts @@ -1,12 +1,13 @@ import type { Database } from '../../db/client' import { micronixTube, cryovialTube, micronixPlate, cryovialBox } from '../../db/schema' -import { eq, like, sql } from 'drizzle-orm' +import { eq, sql } from 'drizzle-orm' import { resolveContainerPlacements, type ContainerPlacement, type KnownContainerPlacement, } from '../container-placement' import type { SearchResult } from './types' +import { likeContains } from '../sql-like' type ContainerSearchRow = { id: number @@ -88,7 +89,7 @@ export async function searchContainers(database: Database, query: string): Promi }) .from(micronixTube) .leftJoin(micronixPlate, eq(micronixTube.collectionId, micronixPlate.id)) - .where(like(micronixTube.barcode, `%${query}%`)) + .where(likeContains(micronixTube.barcode, query)) .limit(10) for (const tube of micronixTubes) { @@ -115,7 +116,7 @@ export async function searchContainers(database: Database, query: string): Promi }) .from(cryovialTube) .leftJoin(cryovialBox, eq(cryovialTube.collectionId, cryovialBox.id)) - .where(like(cryovialTube.barcode, `%${query}%`)) + .where(likeContains(cryovialTube.barcode, query)) .limit(10) for (const tube of cryovialTubes) { diff --git a/packages/api/src/lib/search/study-search.ts b/packages/api/src/lib/search/study-search.ts index 5879372e..0a6a8778 100644 --- a/packages/api/src/lib/search/study-search.ts +++ b/packages/api/src/lib/search/study-search.ts @@ -1,14 +1,15 @@ import type { Database } from '../../db/client' import { study } from '../../db/schema' -import { like, or } from 'drizzle-orm' +import { or } from 'drizzle-orm' import type { SearchResult } from './types' +import { likeContains } from '../sql-like' /** Search studies by short code or title. */ export async function searchStudies(database: Database, query: string): Promise { const studies = await database .select() .from(study) - .where(or(like(study.shortCode, `%${query}%`), like(study.title, `%${query}%`))!) + .where(or(likeContains(study.shortCode, query), likeContains(study.title, query))!) .limit(10) return studies.map((studyRecord) => ({ diff --git a/packages/api/src/lib/search/subject-search.ts b/packages/api/src/lib/search/subject-search.ts index 479552f9..c488b1fc 100644 --- a/packages/api/src/lib/search/subject-search.ts +++ b/packages/api/src/lib/search/subject-search.ts @@ -1,7 +1,8 @@ import type { Database } from '../../db/client' import { study, studySubject } from '../../db/schema' -import { eq, like } from 'drizzle-orm' +import { eq } from 'drizzle-orm' import type { SearchResult } from './types' +import { likeContains } from '../sql-like' /** Search subjects by name. */ export async function searchSubjects(database: Database, query: string): Promise { @@ -14,7 +15,7 @@ export async function searchSubjects(database: Database, query: string): Promise }) .from(studySubject) .leftJoin(study, eq(studySubject.studyId, study.id)) - .where(like(studySubject.name, `%${query}%`)) + .where(likeContains(studySubject.name, query)) .limit(10) return subjects.map((subject) => ({ diff --git a/packages/api/src/lib/specimens/specimen-read.ts b/packages/api/src/lib/specimens/specimen-read.ts index 74f7ecb6..9ca373ad 100644 --- a/packages/api/src/lib/specimens/specimen-read.ts +++ b/packages/api/src/lib/specimens/specimen-read.ts @@ -1,8 +1,10 @@ import type { Database } from '../../db/client' import { specimen, storageContainer, studySubject, study, specimenType, controlBatch } from '../../db/schema' -import { eq, and, like, or, sql } from 'drizzle-orm' +import { eq, and, or, sql } from 'drizzle-orm' import { validatePage, validateLimit } from '../constants' import { resolveContainerByBarcode } from '../identifier-resolution' +import { dateToUpperBound } from '../statistics/helpers' +import { likeContains } from '../sql-like' export type ListSpecimensQuery = { sourceType?: string @@ -150,7 +152,7 @@ export async function listSpecimens(database: Database, query: ListSpecimensQuer conditions.push(sql`${specimen.created} >= ${createdFrom}`) } if (createdTo) { - conditions.push(sql`${specimen.created} <= ${createdTo}`) + conditions.push(dateToUpperBound(specimen.created, createdTo)) } if (barcode) { @@ -173,9 +175,9 @@ export async function listSpecimens(database: Database, query: ListSpecimensQuer if (search) { conditions.push(or( - like(studySubject.name, `%${search}%`), - like(controlBatch.name, `%${search}%`), - like(specimenType.name, `%${search}%`), + likeContains(studySubject.name, search), + likeContains(controlBatch.name, search), + likeContains(specimenType.name, search), )) } diff --git a/packages/api/src/lib/sql-like.ts b/packages/api/src/lib/sql-like.ts new file mode 100644 index 00000000..248517a2 --- /dev/null +++ b/packages/api/src/lib/sql-like.ts @@ -0,0 +1,12 @@ +import { sql, type SQL } from 'drizzle-orm' +import type { SQLiteColumn } from 'drizzle-orm/sqlite-core' + +/** Escape LIKE wildcards so "%" and "_" in user text match themselves. */ +export function escapeLikePattern(text: string): string { + return text.replace(/[\\%_]/g, (ch) => `\\${ch}`) +} + +/** `column LIKE '%text%'` with the text taken literally (a "_" in "3D7_0.05" is not a wildcard). */ +export function likeContains(column: SQLiteColumn | SQL, text: string): SQL { + return sql`${column} LIKE ${`%${escapeLikePattern(text)}%`} ESCAPE '\\'` +} diff --git a/packages/api/src/lib/statistics/__tests__/dashboard-stats.test.ts b/packages/api/src/lib/statistics/__tests__/dashboard-stats.test.ts index 200ed625..cd8fa067 100644 --- a/packages/api/src/lib/statistics/__tests__/dashboard-stats.test.ts +++ b/packages/api/src/lib/statistics/__tests__/dashboard-stats.test.ts @@ -7,7 +7,10 @@ import { createTestMicronixPlate, createTestStorageContainer, createTestStorageType, + createTestSpecimen, + createTestSpecimenType, } from '../../../__tests__/helpers/factories' +import { utcNow } from '../../datetime' import { micronixTube } from '../../../db/schema' import { getDashboardStatistics } from '../dashboard-stats' @@ -60,6 +63,27 @@ describe('dashboard-stats', () => { expect(data.storage._summary).toBeUndefined() }) + it('does not report whole-database totals for an unknown study', async () => { + const type = await createTestSpecimenType(testDb, { name: 'Blood' }) + await createTestSpecimen(testDb, type.id) + + const all = await getDashboardStatistics(testDb, sqlite as SQLiteDatabase, {}) + const unknown = await getDashboardStatistics(testDb, sqlite as SQLiteDatabase, { study: 'NONEXISTENT' }) + + expect(all.specimens.total).toBe(1) + expect(unknown.specimens.total).toBe(0) + }) + + it('includes records created on the created_to date', async () => { + const type = await createTestSpecimenType(testDb, { name: 'Blood' }) + await createTestSpecimen(testDb, type.id) + const today = utcNow().slice(0, 10) + + const data = await getDashboardStatistics(testDb, sqlite as SQLiteDatabase, { created_to: today }) + + expect(data.specimens.total).toBe(1) + }) + it('includes containers at a filtered location', async () => { const storageType = await createTestStorageType(testDb, { name: 'Stats Storage' }) const loc = await createTestLocation(testDb, { diff --git a/packages/api/src/lib/statistics/__tests__/location-filter.test.ts b/packages/api/src/lib/statistics/__tests__/location-filter.test.ts index 7001300c..037adde7 100644 --- a/packages/api/src/lib/statistics/__tests__/location-filter.test.ts +++ b/packages/api/src/lib/statistics/__tests__/location-filter.test.ts @@ -8,7 +8,8 @@ import { createTestStorageContainer, createTestStorageType, } from '../../../__tests__/helpers/factories' -import { micronixTube } from '../../../db/schema' +import { micronixTube, staticWell, box, bag, sheet, paper } from '../../../db/schema' +import { utcNow } from '../../datetime' import { resolveContainerIdsAtLocations, resolveStatisticsLocationFilter, @@ -93,4 +94,28 @@ describe('location-filter', () => { expect(ids).toEqual([container.id]) }) + + it('resolveContainerIdsAtLocations includes static wells and papers in boxes and bags', async () => { + const storageType = await createTestStorageType(testDb, { name: 'Freezer' }) + const loc = await createTestLocation(testDb, { name: 'Freezer A', storageTypeId: String(storageType.id) }) + const now = utcNow() + const plate = await createTestMicronixPlate(testDb, { name: 'P1', locationId: loc.id }) + const well = await createTestStorageContainer(testDb) + await testDb.insert(staticWell).values({ id: well.id, collectionId: plate.id, position: 'A01' }) + const [boxRow] = await testDb.insert(box).values({ name: 'Box1', locationId: loc.id, created: now, lastUpdated: now }).returning() + const [bagRow] = await testDb.insert(bag).values({ name: 'Bag1', locationId: loc.id, created: now, lastUpdated: now }).returning() + const [boxSheet] = await testDb.insert(sheet).values({ name: 'S1', boxId: boxRow.id }).returning() + const [bagSheet] = await testDb.insert(sheet).values({ name: 'S1', bagId: bagRow.id }).returning() + const reuse = { specimenId: well.specimenId, unitId: well.unitId! } + const paperInBox = await createTestStorageContainer(testDb, reuse) + const paperInBag = await createTestStorageContainer(testDb, reuse) + await testDb.insert(paper).values([ + { id: paperInBox.id, sheetId: boxSheet.id }, + { id: paperInBag.id, sheetId: bagSheet.id }, + ]) + + const ids = await resolveContainerIdsAtLocations(testDb, [loc.id]) + + expect(ids.sort((a, b) => a - b)).toEqual([well.id, paperInBox.id, paperInBag.id].sort((a, b) => a - b)) + }) }) diff --git a/packages/api/src/lib/statistics/dashboard-stats.ts b/packages/api/src/lib/statistics/dashboard-stats.ts index 4924e109..9f087135 100644 --- a/packages/api/src/lib/statistics/dashboard-stats.ts +++ b/packages/api/src/lib/statistics/dashboard-stats.ts @@ -22,6 +22,30 @@ import { computeStorageStatistics } from './storage-aggregates' import type { DashboardStatistics, StatisticsFilters } from './types' /** Filtered dashboard statistics for specimens, containers, and storage. */ +function emptyDashboardStatistics(): DashboardStatistics { + return { + specimens: { + total: 0, + bySourceType: {}, + bySpecimenType: {}, + byStudy: {}, + collectionTimeline: [], + creationTimeline: [], + }, + containers: { + total: 0, + byType: {}, + byTags: {}, + byStatus: {}, + averagePerSpecimen: 0, + }, + storage: { + byLocation: [], + byRootLocation: {}, + }, + } +} + export async function getDashboardStatistics( database: Database, sqliteDatabase: SQLiteDatabase, @@ -52,46 +76,28 @@ export async function getDashboardStatistics( .where(eq(study.shortCode, studyCode)) .get() - if (studyRecord) { - const subjects = await database - .select({ id: studySubject.id }) - .from(studySubject) - .where(eq(studySubject.studyId, studyRecord.id)) - subjectIds = subjects.map(s => s.id) - if (subjectIds.length > 0) { - if (subjectIds.length === 1) { - specimenConditions.push( - eq(specimen.studySubjectId, subjectIds[0]) - ) - } else { - specimenConditions.push( - inArray(specimen.studySubjectId, subjectIds) - ) - } + // An unknown study matches nothing; it must not fall through to whole-database totals. + if (!studyRecord) { + return emptyDashboardStatistics() + } + const subjects = await database + .select({ id: studySubject.id }) + .from(studySubject) + .where(eq(studySubject.studyId, studyRecord.id)) + subjectIds = subjects.map(s => s.id) + if (subjectIds.length > 0) { + if (subjectIds.length === 1) { + specimenConditions.push( + eq(specimen.studySubjectId, subjectIds[0]) + ) } else { - // No subjects in study, return empty results - return { - specimens: { - total: 0, - bySourceType: {}, - bySpecimenType: {}, - byStudy: {}, - collectionTimeline: [], - creationTimeline: [], - }, - containers: { - total: 0, - byType: {}, - byTags: {}, - byStatus: {}, - averagePerSpecimen: 0, - }, - storage: { - byLocation: [], - byRootLocation: {}, - }, - } + specimenConditions.push( + inArray(specimen.studySubjectId, subjectIds) + ) } + } else { + // No subjects in study, return empty results + return emptyDashboardStatistics() } } @@ -134,27 +140,7 @@ export async function getDashboardStatistics( let filteredLocationIds: number[] = [] if (locationFilter.kind === 'not_found') { - return { - specimens: { - total: 0, - bySourceType: {}, - bySpecimenType: {}, - byStudy: {}, - collectionTimeline: [], - creationTimeline: [], - }, - containers: { - total: 0, - byType: {}, - byTags: {}, - byStatus: {}, - averagePerSpecimen: 0, - }, - storage: { - byLocation: [], - byRootLocation: {}, - }, - } + return emptyDashboardStatistics() } if (locationFilter.kind === 'resolved') { @@ -191,27 +177,7 @@ export async function getDashboardStatistics( if (tagFilteredContainerIds.length === 0) { // No containers match all the tags, return empty results - return { - specimens: { - total: 0, - bySourceType: {}, - bySpecimenType: {}, - byStudy: {}, - collectionTimeline: [], - creationTimeline: [], - }, - containers: { - total: 0, - byType: {}, - byTags: {}, - byStatus: {}, - averagePerSpecimen: 0, - }, - storage: { - byLocation: [], - byRootLocation: {}, - }, - } + return emptyDashboardStatistics() } } @@ -230,28 +196,26 @@ export async function getDashboardStatistics( ) } + const locationFilteredIdSet = locationFilteredContainerIds ? new Set(locationFilteredContainerIds) : null + const tagFilteredIdSet = tagFilteredContainerIds ? new Set(tagFilteredContainerIds) : null + // Batch query if too many specimen IDs to avoid SQLite variable limit // Use Promise.all to parallelize queries for better performance const specimenChunks = chunkArray(specimenIds, 500) const chunkResults = await Promise.all( specimenChunks.map(async (specimenChunk, chunkIndex) => { - let containerQuery = database.select().from(storageContainer) - const containerConditions = [inArray(storageContainer.specimenId, specimenChunk)] - - // Apply location filter if provided - if (locationFilteredContainerIds) { - containerConditions.push(inArray(storageContainer.id, locationFilteredContainerIds)) - } - - // Apply tag filter if provided - if (tagFilteredContainerIds) { - containerConditions.push(inArray(storageContainer.id, tagFilteredContainerIds)) - } - - containerQuery = containerQuery.where(and(...containerConditions) as any) as any - const containers = await containerQuery - return containers + const containers = await database + .select() + .from(storageContainer) + .where(inArray(storageContainer.specimenId, specimenChunk)) + // Location and tag filters are applied in memory: as IN lists they can exceed + // SQLite's parameter limit on large locations. + return containers.filter( + (c) => + (!locationFilteredIdSet || locationFilteredIdSet.has(c.id)) && + (!tagFilteredIdSet || tagFilteredIdSet.has(c.id)), + ) }) ) @@ -269,27 +233,7 @@ export async function getDashboardStatistics( }) } else if (filteredSpecimens.length === 0) { // No specimens match, so no containers - return { - specimens: { - total: 0, - bySourceType: {}, - bySpecimenType: {}, - byStudy: {}, - collectionTimeline: [], - creationTimeline: [], - }, - containers: { - total: 0, - byType: {}, - byTags: {}, - byStatus: {}, - averagePerSpecimen: 0, - }, - storage: { - byLocation: [], - byRootLocation: {}, - }, - } + return emptyDashboardStatistics() } else { // No specimen filter, but we might have state/status/location filters if (hasLocationFilter && filteredLocationIds.length > 0) { @@ -307,34 +251,28 @@ export async function getDashboardStatistics( const containerIdChunks = chunkArray(locationFilteredContainerIds, 500) const chunkResults = await Promise.all( containerIdChunks.map(async (containerIdChunk) => { - let containerQuery = database.select().from(storageContainer) - const containerConditions = [inArray(storageContainer.id, containerIdChunk)] - - // Apply tag filter if provided - if (tagFilteredContainerIds) { - containerConditions.push(inArray(storageContainer.id, tagFilteredContainerIds)) - } - - containerQuery = containerQuery.where(and(...containerConditions) as any) as any - return await containerQuery + // Tag filter is already intersected into locationFilteredContainerIds. + return await database + .select() + .from(storageContainer) + .where(inArray(storageContainer.id, containerIdChunk)) }) ) filteredContainers = chunkResults.flat() } } else { // No location filter - let containerQuery = database.select().from(storageContainer) - const containerConditions = [] - - // Apply tag filter if provided if (tagFilteredContainerIds) { - containerConditions.push(inArray(storageContainer.id, tagFilteredContainerIds)) - } - - if (containerConditions.length > 0) { - containerQuery = containerQuery.where(and(...containerConditions) as any) as any + const tagIdChunks = chunkArray(tagFilteredContainerIds, 500) + const chunkResults = await Promise.all( + tagIdChunks.map((idChunk) => + database.select().from(storageContainer).where(inArray(storageContainer.id, idChunk)), + ), + ) + filteredContainers = chunkResults.flat() + } else { + filteredContainers = await database.select().from(storageContainer) } - filteredContainers = await containerQuery } } diff --git a/packages/api/src/lib/statistics/helpers.ts b/packages/api/src/lib/statistics/helpers.ts index 7f814bdf..464f4a8d 100644 --- a/packages/api/src/lib/statistics/helpers.ts +++ b/packages/api/src/lib/statistics/helpers.ts @@ -1,5 +1,6 @@ -import { gte, lte } from 'drizzle-orm' +import { gte, lt, lte } from 'drizzle-orm' import type { SQLiteColumn } from 'drizzle-orm/sqlite-core' +import { shiftCalendarDate } from '../datetime' /** Build date filter conditions for specimen date columns. */ export function buildDateFilter(column: SQLiteColumn, dateFrom?: string, dateTo?: string) { @@ -8,11 +9,20 @@ export function buildDateFilter(column: SQLiteColumn, dateFrom?: string, dateTo? conditions.push(gte(column, dateFrom)) } if (dateTo) { - conditions.push(lte(column, dateTo)) + conditions.push(dateToUpperBound(column, dateTo)) } return conditions } +/** + * Upper bound for a "to" date filter. A bare YYYY-MM-DD includes that whole day, so a + * timestamp column ('2026-05-26 22:28:26') is compared with "< next day", not "<= date". + */ +export function dateToUpperBound(column: SQLiteColumn, dateTo: string) { + const next = /^\d{4}-\d{2}-\d{2}$/.test(dateTo) ? shiftCalendarDate(dateTo, 1) : null + return next ? lt(column, next) : lte(column, dateTo) +} + /** Batch an array into chunks for SQLite variable limits. */ export function chunkArray(array: T[], chunkSize: number): T[][] { const chunks: T[][] = [] diff --git a/packages/api/src/lib/statistics/location-filter.ts b/packages/api/src/lib/statistics/location-filter.ts index c80e8ca3..9a83f43d 100644 --- a/packages/api/src/lib/statistics/location-filter.ts +++ b/packages/api/src/lib/statistics/location-filter.ts @@ -4,10 +4,15 @@ import { location, micronixPlate, micronixTube, + staticWell, cryovialBox, cryovialTube, + box, + bag, + sheet, + paper, } from '../../db/schema' -import { eq, inArray } from 'drizzle-orm' +import { eq, inArray, or } from 'drizzle-orm' export type StatisticsLocationFilterResult = | { kind: 'none' } @@ -43,50 +48,45 @@ export async function resolveStatisticsLocationFilter( } } -/** Resolve storage container ids at micronix plates / cryovial boxes in the given locations. */ +/** + * Resolve storage container ids stored in the given locations: tubes and static wells on + * micronix plates, tubes in cryovial boxes, and papers on sheets in boxes or bags. + * Collections are matched with subqueries so large locations never build huge IN lists. + */ export async function resolveContainerIdsAtLocations( database: Database, filteredLocationIds: number[], tagFilteredContainerIds?: number[] | null, ): Promise { - const [matchingPlates, matchingBoxes] = await Promise.all([ - database - .select({ id: micronixPlate.id }) - .from(micronixPlate) - .where(inArray(micronixPlate.locationId, filteredLocationIds)), - database - .select({ id: cryovialBox.id }) - .from(cryovialBox) - .where(inArray(cryovialBox.locationId, filteredLocationIds)), - ]) - - const plateIds = matchingPlates.map((p) => p.id) - const boxIds = matchingBoxes.map((b) => b.id) + const platesHere = database + .select({ id: micronixPlate.id }) + .from(micronixPlate) + .where(inArray(micronixPlate.locationId, filteredLocationIds)) + const cryovialBoxesHere = database + .select({ id: cryovialBox.id }) + .from(cryovialBox) + .where(inArray(cryovialBox.locationId, filteredLocationIds)) + const sheetsHere = database + .select({ id: sheet.id }) + .from(sheet) + .where( + or( + inArray(sheet.boxId, database.select({ id: box.id }).from(box).where(inArray(box.locationId, filteredLocationIds))), + inArray(sheet.bagId, database.select({ id: bag.id }).from(bag).where(inArray(bag.locationId, filteredLocationIds))), + ), + ) - const [micronixContainerIds, cryovialContainerIds] = await Promise.all([ - plateIds.length > 0 - ? database - .select({ id: micronixTube.id }) - .from(micronixTube) - .where(inArray(micronixTube.collectionId, plateIds)) - : Promise.resolve([]), - boxIds.length > 0 - ? database - .select({ id: cryovialTube.id }) - .from(cryovialTube) - .where(inArray(cryovialTube.collectionId, boxIds)) - : Promise.resolve([]), + const [micronix, wells, cryovial, papers] = await Promise.all([ + database.select({ id: micronixTube.id }).from(micronixTube).where(inArray(micronixTube.collectionId, platesHere)), + database.select({ id: staticWell.id }).from(staticWell).where(inArray(staticWell.collectionId, platesHere)), + database.select({ id: cryovialTube.id }).from(cryovialTube).where(inArray(cryovialTube.collectionId, cryovialBoxesHere)), + database.select({ id: paper.id }).from(paper).where(inArray(paper.sheetId, sheetsHere)), ]) - let locationFilteredContainerIds = [ - ...new Set([...micronixContainerIds.map((r) => r.id), ...cryovialContainerIds.map((r) => r.id)]), - ] - + const ids = new Set([...micronix, ...wells, ...cryovial, ...papers].map((r) => r.id)) if (tagFilteredContainerIds) { - locationFilteredContainerIds = locationFilteredContainerIds.filter((id) => - tagFilteredContainerIds.includes(id), - ) + const tagged = new Set(tagFilteredContainerIds) + return [...ids].filter((id) => tagged.has(id)) } - - return locationFilteredContainerIds + return [...ids] } diff --git a/packages/api/src/routes/__tests__/activity.test.ts b/packages/api/src/routes/__tests__/activity.test.ts index e6b8cdb6..ccf8b4a8 100644 --- a/packages/api/src/routes/__tests__/activity.test.ts +++ b/packages/api/src/routes/__tests__/activity.test.ts @@ -9,7 +9,14 @@ import { createTestControlBatch, createTestSpecimenType, createTestSpecimen, + createTestStudy, + createTestStudySubject, + createTestStorageContainer, + createTestStorageType, + createTestLocation, + createTestMicronixPlate, } from '../../__tests__/helpers/factories' +import { micronixTube } from '../../db/schema' import { createActivityRoutes } from '../activity' describe('Activity API', () => { @@ -75,5 +82,29 @@ describe('Activity API', () => { expect(specimenActivity!.label).toContain('HPV18 DBS Control') expect(specimenActivity!.label).toContain('HPV18-2024-001') }) + + it('labels subject specimens and their containers with subject and study', async () => { + const studyRecord = await createTestStudy(ctx.db, { title: 'Malaria Cohort', shortCode: 'MAL' }) + const subject = await createTestStudySubject(ctx.db, { studyId: studyRecord.id, name: 'P-001' }) + const type = await createTestSpecimenType(ctx.db, { name: 'Whole Blood' }) + const spec = await createTestSpecimen(ctx.db, type.id, { studySubjectId: subject.id }) + const container = await createTestStorageContainer(ctx.db, { specimenId: spec.id }) + const storageType = await createTestStorageType(ctx.db, { name: 'Freezer' }) + const loc = await createTestLocation(ctx.db, { name: 'Loc', storageTypeId: String(storageType.id) }) + const plate = await createTestMicronixPlate(ctx.db, { name: 'P1', locationId: loc.id }) + await ctx.db.insert(micronixTube).values({ id: container.id, collectionId: plate.id, barcode: 'MT-9', position: 'A01' }) + + const res = await ctx.request('/api/activity/recent', { method: 'GET' }) + const data = (await res.json()) as { activity: Array<{ id: number; type: string; label: string; context?: string }> } + + expect(data.activity.find((a) => a.type === 'specimen' && a.id === spec.id)).toMatchObject({ + label: 'Whole Blood • P-001 (MAL)', + context: 'Malaria Cohort', + }) + expect(data.activity.find((a) => a.type === 'container' && a.id === container.id)).toMatchObject({ + label: 'Micronix Tube (MT-9)', + context: 'Whole Blood • P-001 (MAL)', + }) + }) }) }) diff --git a/packages/api/src/routes/__tests__/auth.test.ts b/packages/api/src/routes/__tests__/auth.test.ts index 6d097e8e..4f948666 100644 --- a/packages/api/src/routes/__tests__/auth.test.ts +++ b/packages/api/src/routes/__tests__/auth.test.ts @@ -388,6 +388,31 @@ describe('Auth API', () => { expect(cookies).toContain('session_id') }) + it('matches an email before a username that equals it', async () => { + await testDb.insert(users).values({ + email: 'squatter@example.com', + name: 'Squatter', + username: 'victim@example.com', + passwordHash: await bcrypt.hash('squatter123', 10), + role: 'member', + createdAt: utcNow(), + approvedAt: utcNow(), + }) + await createTestUser(testDb, { email: 'victim@example.com', name: 'Victim', password: 'password123' }) + + const res = await createTestClient(app).api.auth.login.$post({ + json: { emailOrUsername: 'victim@example.com', password: 'password123' }, + }) + expect(res.status).toBe(200) + }) + + it('rejects a username containing "@"', async () => { + const res = await createTestClient(app).api.auth['self-register'].$post({ + json: { email: 'new@example.com', name: 'New', username: 'other@example.com', password: 'password123' }, + }) + expect(res.status).toBe(400) + }) + it('should reject invalid username', async () => { const client = createTestClient(app) const res = await client.api.auth.login.$post({ diff --git a/packages/api/src/routes/__tests__/collections.test.ts b/packages/api/src/routes/__tests__/collections.test.ts index c31fb057..c7d594f5 100644 --- a/packages/api/src/routes/__tests__/collections.test.ts +++ b/packages/api/src/routes/__tests__/collections.test.ts @@ -120,6 +120,14 @@ describe('Collections API', () => { expect(data.results[0].exists).toBe(false) }) + it('rejects sheets, whose names only identify a record within a box or bag', async () => { + const res = await ctx.request('/api/collections/check', { + method: 'POST', + json: { collections: [{ identifier: 'Sheet-1', type: 'sheet' }] }, + }) + expect(res.status).toBe(400) + }) + it('returns 400 for invalid body', async () => { const res = await ctx.request('/api/collections/check', { method: 'POST', diff --git a/packages/api/src/routes/__tests__/containers.test.ts b/packages/api/src/routes/__tests__/containers.test.ts index 9b71d64e..b9bf89dd 100644 --- a/packages/api/src/routes/__tests__/containers.test.ts +++ b/packages/api/src/routes/__tests__/containers.test.ts @@ -201,6 +201,31 @@ describe('Containers API', () => { expect(row?.barcode).toBe('NEW-CRYO-1') }) + it('clears the comment when it is emptied', async () => { + const now = utcNow() + const [boxRecord] = await ctx.db + .insert(cryovialBox) + .values({ name: `BOX-CMT-${Date.now()}`, locationId: testLocation.id, created: now, lastUpdated: now }) + .returning() + const testStudy = await createTestStudy(ctx.db, { title: 'C Study', shortCode: 'CST' + Date.now() }) + const subj = await createTestStudySubject(ctx.db, { studyId: testStudy.id, name: 'S1' }) + const [spec] = await ctx.db + .insert(specimen) + .values({ studySubjectId: subj.id, specimenTypeId: testSpecimenType.id, created: now, lastUpdated: now }) + .returning() + const [container] = await ctx.db + .insert(storageContainer) + .values({ specimenId: spec.id, unitId: testUnit.id, totalQuantity: 1, remainingQuantity: 1, comment: 'hemolyzed', created: now, lastUpdated: now }) + .returning() + await ctx.db.insert(cryovialTube).values({ id: container.id, collectionId: boxRecord.id, barcode: 'CMT-1', position: 'A01' }) + + const res = await ctx.request(`/api/containers/${container.id}`, { method: 'PATCH', json: { comment: '' } }) + + expect(res.status).toBe(200) + const row = await ctx.db.select().from(storageContainer).where(eq(storageContainer.id, container.id)).get() + expect(row?.comment).toBeNull() + }) + it('returns 400 when cryovial_tube barcode is already in use', async () => { const now = utcNow() const [boxRecord] = await ctx.db diff --git a/packages/api/src/routes/__tests__/controls.test.ts b/packages/api/src/routes/__tests__/controls.test.ts index 46cbaca6..284ac411 100644 --- a/packages/api/src/routes/__tests__/controls.test.ts +++ b/packages/api/src/routes/__tests__/controls.test.ts @@ -387,6 +387,48 @@ describe('Controls API', () => { }) }) + describe(`PATCH ${BASE}/:id`, () => { + it('ignores controlType so a definition cannot leave the blood routes', async () => { + const def = await createTestControlDefinition(ctx.db, { name: 'Blood A', controlType: 'blood' }) + + const res = await ctx.request(`${BASE}/${def.id}`, { method: 'PATCH', json: { controlType: 'antibody' } }) + + expect(res.status).toBe(200) + const data = (await res.json()) as { control: { controlType: string } } + expect(data.control.controlType).toBe('blood') + }) + + it('returns 409 when renaming to an existing definition name', async () => { + await createTestControlDefinition(ctx.db, { name: 'Taken' }) + const def = await createTestControlDefinition(ctx.db, { name: 'Mine' }) + + const res = await ctx.request(`${BASE}/${def.id}`, { method: 'PATCH', json: { name: 'Taken' } }) + + expect(res.status).toBe(409) + }) + + it('returns 409 when changing density to match another definition', async () => { + const strain = await createTestStrain(ctx.db, { name: '3D7' }) + const strains = [{ id: strain.id, name: '3D7', percentage: 100 }] + await createTestControlDefinition(ctx.db, { name: 'D100', properties: { strains, targetDensity: 100 } }) + const def = await createTestControlDefinition(ctx.db, { name: 'D200', properties: { strains, targetDensity: 200 } }) + + const res = await ctx.request(`${BASE}/${def.id}`, { method: 'PATCH', json: { targetDensity: 100 } }) + + expect(res.status).toBe(409) + }) + + it('does not count the definition being edited as a duplicate', async () => { + const strain = await createTestStrain(ctx.db, { name: '3D7' }) + const strains = [{ id: strain.id, name: '3D7', percentage: 100 }] + const def = await createTestControlDefinition(ctx.db, { name: 'D200', properties: { strains, targetDensity: 200 } }) + + const res = await ctx.request(`${BASE}/${def.id}`, { method: 'PATCH', json: { targetDensity: 200 } }) + + expect(res.status).toBe(200) + }) + }) + describe(`PATCH ${BASE}/batches/:id`, () => { it('returns 200 and updates batch name', async () => { const strain = await createTestStrain(ctx.db, { name: 'Strain Patch' }) diff --git a/packages/api/src/routes/__tests__/locations.test.ts b/packages/api/src/routes/__tests__/locations.test.ts index 927da0ad..b5970793 100644 --- a/packages/api/src/routes/__tests__/locations.test.ts +++ b/packages/api/src/routes/__tests__/locations.test.ts @@ -6,6 +6,8 @@ import { } from '../../__tests__/helpers/authenticated-route-test' import { createLocationsRoutes } from '../locations' import { createTestStorageType, createTestLocation } from '../../__tests__/helpers/factories' +import { location } from '../../db/schema' +import { eq } from 'drizzle-orm' describe('Locations API', () => { let ctx: AuthenticatedRouteTestContext @@ -18,6 +20,9 @@ describe('Locations API', () => { password: 'password123', role: 'member', }, + additionalUsers: [ + { key: 'admin', email: 'admin@test.com', name: 'Admin', password: 'password123', role: 'admin' }, + ], settings: { pagination: true }, mount: (app, { db, sqlite }) => { app.route('/api/locations', createLocationsRoutes(db, sqlite)) @@ -79,4 +84,33 @@ describe('Locations API', () => { expect(res.status).toBe(401) }) }) + + describe('storage type on root and child locations', () => { + it('creates a root location when parentId is omitted', async () => { + const storageType = await createTestStorageType(ctx.db, { name: 'Freezer' }) + const res = await ctx.request('/api/locations', { + method: 'POST', + cookie: ctx.cookies.admin, + json: { name: 'Root A', storageTypeId: String(storageType.id), canContainCollections: false }, + }) + expect(res.status).toBe(201) + }) + + it('moves a root location under a parent without sending storageTypeId', async () => { + const storageType = await createTestStorageType(ctx.db, { name: 'Freezer' }) + const parent = await createTestLocation(ctx.db, { name: 'Parent', storageTypeId: String(storageType.id) }) + const root = await createTestLocation(ctx.db, { name: 'Other Root', storageTypeId: String(storageType.id) }) + + const res = await ctx.request(`/api/locations/${root.id}`, { + method: 'PUT', + cookie: ctx.cookies.admin, + json: { parentId: parent.id }, + }) + + expect(res.status).toBe(200) + const [row] = await ctx.db.select().from(location).where(eq(location.id, root.id)) + expect(row.parentId).toBe(parent.id) + expect(row.storageTypeId).toBeNull() + }) + }) }) diff --git a/packages/api/src/routes/__tests__/qpcr-experiments.test.ts b/packages/api/src/routes/__tests__/qpcr-experiments.test.ts index 294fb448..3a189e01 100644 --- a/packages/api/src/routes/__tests__/qpcr-experiments.test.ts +++ b/packages/api/src/routes/__tests__/qpcr-experiments.test.ts @@ -3,6 +3,7 @@ import { setupAuthenticatedRouteTest, type AuthenticatedRouteTestContext } from import { createQpcrExperimentsRoutes } from '../qpcr-experiments' import { utcNow } from '../../lib/datetime' import { qpcrExperiment, qpcrExperimentTarget, qpcrExperimentWell } from '../../db/schema' +import { qpcrAmplificationData } from '../../db/schema' describe('qPCR Experiments Template', () => { let ctx: AuthenticatedRouteTestContext @@ -47,6 +48,64 @@ describe('qPCR Experiments Template', () => { }) }) + describe('PATCH / targets', () => { + it('rejects duplicate target names and keeps the existing targets', async () => { + const createRes = await ctx.request('/api/qpcr-experiments', { + method: 'POST', + json: { name: 'Dup targets', templateFormat: 'biorad' }, + }) + const created = (await createRes.json()) as { id: number } + + const res = await ctx.request(`/api/qpcr-experiments/${created.id}`, { + method: 'PATCH', + json: { targets: [{ targetName: '' }, { targetName: 'varATS' }] }, + }) + + expect(res.status).toBe(400) + const detail = (await (await ctx.request(`/api/qpcr-experiments/${created.id}`)).json()) as { + experiment: { targets: Array<{ targetName: string }> } + } + expect(detail.experiment.targets.map((t) => t.targetName)).toEqual(['varATS']) + }) + }) + + describe('POST /:id/results', () => { + it('keeps amplification rows for results that have no target name', async () => { + const XLSX = await import('xlsx') + const wb = XLSX.utils.book_new() + XLSX.utils.book_append_sheet( + wb, + XLSX.utils.aoa_to_sheet([ + ['Well', 'Well Position', 'Sample Name', 'Task', 'CT'], + [1, 'A1', 'S1', 'UNKNOWN', 22.1], + ]), + 'Results', + ) + XLSX.utils.book_append_sheet( + wb, + XLSX.utils.aoa_to_sheet([ + ['Well', 'Well Position', 'Cycle', 'Rn', 'Delta Rn'], + [1, 'A1', 1, 0.5, 0.01], + [1, 'A1', 2, 0.6, 0.02], + ]), + 'Amplification Data', + ) + const fileContent = (XLSX.write(wb, { type: 'buffer', bookType: 'xls' }) as Buffer).toString('base64') + const created = (await ( + await ctx.request('/api/qpcr-experiments', { method: 'POST', json: { name: 'Amp', templateFormat: 'quant_studio' } }) + ).json()) as { id: number } + + const res = await ctx.request(`/api/qpcr-experiments/${created.id}/results`, { + method: 'POST', + json: { fileContent, fileName: 'run.xls', instrumentType: 'QuantStudio' }, + }) + + expect(res.status).toBeLessThan(300) + const amp = await ctx.db.select().from(qpcrAmplificationData).all() + expect(amp).toHaveLength(2) + }) + }) + describe('GET /:id/template', () => { it('includes only rows for wells that have a tube; skips positions with no tube', async () => { const [exp] = await ctx.db diff --git a/packages/api/src/routes/__tests__/settings.test.ts b/packages/api/src/routes/__tests__/settings.test.ts index 66c9638d..b967436b 100644 --- a/packages/api/src/routes/__tests__/settings.test.ts +++ b/packages/api/src/routes/__tests__/settings.test.ts @@ -46,4 +46,16 @@ describe('Settings API', () => { }) }) + describe('PUT /api/settings/:key', () => { + for (const userId of ['abc', -1, 1.5]) { + it(`rejects userId ${JSON.stringify(userId)} with 400`, async () => { + const res = await ctx.request('/api/settings/session_settings', { + method: 'PUT', + json: { userId, timeoutMinutes: 60 }, + }) + expect(res.status).toBe(400) + expect(((await res.json()) as { error: string }).error).toMatch(/userId must be a positive integer or null/) + }) + } + }) }) diff --git a/packages/api/src/routes/__tests__/studies.test.ts b/packages/api/src/routes/__tests__/studies.test.ts index 2c878169..7591f8a1 100644 --- a/packages/api/src/routes/__tests__/studies.test.ts +++ b/packages/api/src/routes/__tests__/studies.test.ts @@ -5,7 +5,7 @@ import { type AuthenticatedRouteTestContext, } from '../../__tests__/helpers/authenticated-route-test' import { createStudiesRoutes } from '../studies' -import { createTestStudy } from '../../__tests__/helpers/factories' +import { createTestStudy, createTestSpecimenType, createTestUnit } from '../../__tests__/helpers/factories' describe('Studies API (list, get, create, update)', () => { let ctx: AuthenticatedRouteTestContext @@ -90,6 +90,15 @@ describe('Studies API (list, get, create, update)', () => { expect(data.study.shortCode).toBe('NEW01') }) + it('returns 409, not 500, for a duplicate title', async () => { + const body = { title: 'Dup Study', shortCode: 'DUP01', isLongitudinal: false, leadPerson: 'Lead' } + await ctx.request('/api/studies', { method: 'POST', json: body }) + const res = await ctx.request('/api/studies', { method: 'POST', json: { ...body, shortCode: 'DUP02' } }) + expect(res.status).toBe(409) + const data = (await res.json()) as { error: string } + expect(data.error).toBe('A record with this title already exists.') + }) + it('returns 401 when not authenticated', async () => { const res = await authenticatedRequest(ctx.createRequestApp(), '/api/studies', { method: 'POST', @@ -139,4 +148,36 @@ describe('Studies API (list, get, create, update)', () => { expect(res.status).toBe(401) }) }) + + describe('GET /api/studies/:id/summary', () => { + it('handles studies with more specimens than SQLite allows as bound parameters', async () => { + const studyRecord = await createTestStudy(ctx.db, { title: 'Big Study', shortCode: 'BIG01' }) + const type = await createTestSpecimenType(ctx.db, { name: 'DBS' }) + const unit = await createTestUnit(ctx.db, { symbol: 'spots', name: 'spots', category: 'count' }) + const count = 70_000 + ctx.sqlite.exec('BEGIN') + const addSubject = ctx.sqlite.prepare( + "INSERT INTO study_subject (study_id, name, created, last_updated) VALUES (?, ?, '2024-01-01', '2024-01-01')", + ) + const addSpecimen = ctx.sqlite.prepare( + "INSERT INTO specimen (study_subject_id, specimen_type_id, collection_date, created, last_updated) VALUES (?, ?, '2024-01-01', '2024-01-01', '2024-01-01')", + ) + const addContainer = ctx.sqlite.prepare( + "INSERT INTO storage_container (specimen_id, unit_id, total_quantity, remaining_quantity, created, last_updated) VALUES (?, ?, 1, 1, '2024-01-01', '2024-01-01')", + ) + for (let i = 0; i < count; i++) { + const subject = addSubject.run(studyRecord.id, `S${i}`) + const spec = addSpecimen.run(subject.lastInsertRowid, type.id) + addContainer.run(spec.lastInsertRowid, unit.id) + } + ctx.sqlite.exec('COMMIT') + + const res = await ctx.request(`/api/studies/${studyRecord.id}/summary`) + + expect(res.status).toBe(200) + const data = (await res.json()) as { summary: { totalSpecimens: number; totalContainers: number } } + expect(data.summary.totalSpecimens).toBe(count) + expect(data.summary.totalContainers).toBe(count) + }, 60_000) + }) }) diff --git a/packages/api/src/routes/activity.ts b/packages/api/src/routes/activity.ts index 8258ec19..c8d7bb03 100644 --- a/packages/api/src/routes/activity.ts +++ b/packages/api/src/routes/activity.ts @@ -21,6 +21,9 @@ import { handleRouteError } from '../lib/error-handler' * Create activity routes with database injection * @param database - Database instance (required) */ +/** Upper bound for GET /activity/recent?limit=. */ +const MAX_RECENT_ACTIVITY = 50 + export function createActivityRoutes(database: Database): Hono { const activity = new Hono() const authMiddleware = createAuthMiddleware(database) @@ -28,100 +31,66 @@ export function createActivityRoutes(database: Database): Hono { // Get recent activity across all entity types activity.get('/recent', authMiddleware, async (c) => { try { - const limit = parseInt(c.req.query('limit') || '10') + // Keep the feed short, and never unbounded (SQLite treats LIMIT -1 as no limit). + const requested = parseInt(c.req.query('limit') || '10') + const limit = Number.isFinite(requested) ? Math.min(Math.max(requested, 1), MAX_RECENT_ACTIVITY) : 10 - // Get recent specimens with enriched data + // Recent specimens, joined to their subject/study or batch/definition in one query + // (never expose DB IDs in labels) const recentSpecimens = await database .select({ id: specimen.id, - type: sql`'specimen'`.as('type'), created: specimen.created, lastUpdated: specimen.lastUpdated, specimenTypeName: specimenType.name, collectionDate: specimen.collectionDate, - studySubjectId: specimen.studySubjectId, controlBatchId: specimen.controlBatchId, + studySubjectId: specimen.studySubjectId, + batchName: controlBatch.name, + productionDate: controlBatch.productionDate, + definitionName: controlDefinition.name, + subjectName: studySubject.name, + studyTitle: study.title, + studyShortCode: study.shortCode, }) .from(specimen) .leftJoin(specimenType, eq(specimen.specimenTypeId, specimenType.id)) + .leftJoin(controlBatch, eq(specimen.controlBatchId, controlBatch.id)) + .leftJoin(controlDefinition, eq(controlBatch.controlDefinitionId, controlDefinition.id)) + .leftJoin(studySubject, eq(specimen.studySubjectId, studySubject.id)) + .leftJoin(study, eq(studySubject.studyId, study.id)) .orderBy(sql`COALESCE(${specimen.lastUpdated}, ${specimen.created}) DESC`) .limit(limit) - - // Enrich specimens with subject/study or batch/definition info (never expose DB IDs in labels) - const enrichedSpecimens = await Promise.all( - recentSpecimens.map(async (spec) => { - const specimenTypeName = spec.specimenTypeName || 'Specimen' - let label: string - let context: string | undefined = undefined - - if (spec.controlBatchId) { - try { - const batchInfo = await database - .select({ - batchName: controlBatch.name, - productionDate: controlBatch.productionDate, - definitionName: controlDefinition.name, - }) - .from(controlBatch) - .innerJoin(controlDefinition, eq(controlBatch.controlDefinitionId, controlDefinition.id)) - .where(eq(controlBatch.id, spec.controlBatchId)) - .get() - - if (batchInfo) { - label = `${specimenTypeName} • ${batchInfo.definitionName} (${batchInfo.batchName})` - context = spec.collectionDate || batchInfo.productionDate || undefined - } else { - label = `${specimenTypeName} • Control batch` - } - } catch (e) { - label = `${specimenTypeName} • Control batch` - } - } else if (spec.studySubjectId) { - try { - const subject = await database - .select({ - name: studySubject.name, - studyId: studySubject.studyId, - }) - .from(studySubject) - .where(eq(studySubject.id, spec.studySubjectId)) - .get() - - if (subject) { - const studyRecord = await database - .select({ - title: study.title, - shortCode: study.shortCode, - }) - .from(study) - .where(eq(study.id, subject.studyId)) - .get() - - if (studyRecord) { - label = `${specimenTypeName} • ${subject.name} (${studyRecord.shortCode})` - context = studyRecord.title - } else { - label = `${specimenTypeName} • ${subject.name}` - } - } else { - label = specimenTypeName - } - } catch (e) { - label = specimenTypeName - } + + const enrichedSpecimens = recentSpecimens.map((spec) => { + const specimenTypeName = spec.specimenTypeName || 'Specimen' + let label: string = specimenTypeName + let context: string | undefined = undefined + + if (spec.controlBatchId) { + if (spec.batchName && spec.definitionName) { + label = `${specimenTypeName} • ${spec.definitionName} (${spec.batchName})` + context = spec.collectionDate || spec.productionDate || undefined } else { - label = specimenTypeName + label = `${specimenTypeName} • Control batch` } - - return { - id: spec.id, - type: 'specimen' as const, - timestamp: spec.lastUpdated || spec.created || '', - label, - context, + } else if (spec.studySubjectId && spec.subjectName) { + if (spec.studyShortCode) { + label = `${specimenTypeName} • ${spec.subjectName} (${spec.studyShortCode})` + context = spec.studyTitle ?? undefined + } else { + label = `${specimenTypeName} • ${spec.subjectName}` } - }) - ) + } + + return { + id: spec.id, + type: 'specimen' as const, + timestamp: spec.lastUpdated || spec.created || '', + label, + context, + } + }) // Get recent studies with enriched data const recentStudies = await database @@ -146,148 +115,90 @@ export function createActivityRoutes(database: Database): Hono { })) ) - // Get recent containers with enriched data + // Recent containers, joined to their subtype row and specimen/subject/study in one query const recentContainers = await database .select({ id: storageContainer.id, created: storageContainer.created, lastUpdated: storageContainer.lastUpdated, - specimenId: storageContainer.specimenId, + micronixId: micronixTube.id, + micronixBarcode: micronixTube.barcode, + micronixPosition: micronixTube.position, + cryovialId: cryovialTube.id, + cryovialBarcode: cryovialTube.barcode, + cryovialPosition: cryovialTube.position, + paperId: paper.id, + paperSublabel: paper.sublabel, + wellId: staticWell.id, + wellPosition: staticWell.position, + specimenTypeName: specimenType.name, + subjectName: studySubject.name, + studyShortCode: study.shortCode, }) .from(storageContainer) + .leftJoin(micronixTube, eq(micronixTube.id, storageContainer.id)) + .leftJoin(cryovialTube, eq(cryovialTube.id, storageContainer.id)) + .leftJoin(paper, eq(paper.id, storageContainer.id)) + .leftJoin(staticWell, eq(staticWell.id, storageContainer.id)) + .leftJoin(specimen, eq(storageContainer.specimenId, specimen.id)) + .leftJoin(specimenType, eq(specimen.specimenTypeId, specimenType.id)) + .leftJoin(studySubject, eq(specimen.studySubjectId, studySubject.id)) + .leftJoin(study, eq(studySubject.studyId, study.id)) .orderBy(sql`COALESCE(${storageContainer.lastUpdated}, ${storageContainer.created}) DESC`) .limit(limit) - - // Enrich containers with type, barcode, and specimen info - const enrichedContainers = await Promise.all( - recentContainers.map(async (container) => { - // Determine container type by checking subtype tables - const [micronixInfo, cryovialInfo, paperInfo, staticWellInfo] = await Promise.all([ - database.select({ barcode: micronixTube.barcode, position: micronixTube.position }) - .from(micronixTube) - .where(eq(micronixTube.id, container.id)) - .get(), - database.select({ barcode: cryovialTube.barcode, position: cryovialTube.position }) - .from(cryovialTube) - .where(eq(cryovialTube.id, container.id)) - .get(), - database.select({ sublabel: paper.sublabel }) - .from(paper) - .where(eq(paper.id, container.id)) - .get(), - database.select({ position: staticWell.position }) - .from(staticWell) - .where(eq(staticWell.id, container.id)) - .get(), - ]) - - let containerType = 'container' - let barcode: string | null = null - let position: string | null = null - - if (micronixInfo) { - containerType = 'micronix_tube' - barcode = micronixInfo.barcode || null - position = micronixInfo.position || null - } else if (cryovialInfo) { - containerType = 'cryovial_tube' - barcode = cryovialInfo.barcode || null - position = cryovialInfo.position || null - } else if (paperInfo) { - containerType = 'paper' - barcode = paperInfo.sublabel || null - position = null - } else if (staticWellInfo) { - containerType = 'static_well' - position = staticWellInfo.position || null - } - - // Format container type name (never expose DB IDs) - const containerTypeName = containerType - .split('_') - .map(word => word.charAt(0).toUpperCase() + word.slice(1)) - .join(' ') - - let label: string - if (barcode) { - label = `${containerTypeName} (${barcode})` - } else if (position) { - label = `${containerTypeName} at ${position}` - } else { - label = `Unnamed ${containerTypeName.toLowerCase()}` - } - - let context: string | undefined = undefined - - // Get specimen info for context - if (container.specimenId) { - try { - const spec = await database - .select({ - id: specimen.id, - specimenTypeId: specimen.specimenTypeId, - studySubjectId: specimen.studySubjectId, - }) - .from(specimen) - .where(eq(specimen.id, container.specimenId)) - .get() - - if (spec) { - const specType = await database - .select({ name: specimenType.name }) - .from(specimenType) - .where(eq(specimenType.id, spec.specimenTypeId)) - .get() - - if (specType) { - context = specType.name - } - - if (spec.studySubjectId) { - try { - const subject = await database - .select({ - name: studySubject.name, - studyId: studySubject.studyId, - }) - .from(studySubject) - .where(eq(studySubject.id, spec.studySubjectId)) - .get() - - if (subject) { - const studyRecord = await database - .select({ - shortCode: study.shortCode, - }) - .from(study) - .where(eq(study.id, subject.studyId)) - .get() - - if (studyRecord) { - context = `${specType?.name || ''} • ${subject.name} (${studyRecord.shortCode})`.trim() - } else { - context = `${specType?.name || ''} • ${subject.name}`.trim() - } - } - } catch (e) { - // Silently fail - } - } - } - } catch (e) { - // Silently fail - context is optional - } - } - - return { - id: container.id, - type: 'container' as const, - timestamp: container.lastUpdated || container.created || '', - label, - context, - } - }) - ) + + const enrichedContainers = recentContainers.map((container) => { + let containerType = 'container' + let barcode: string | null = null + let position: string | null = null + + if (container.micronixId != null) { + containerType = 'micronix_tube' + barcode = container.micronixBarcode || null + position = container.micronixPosition || null + } else if (container.cryovialId != null) { + containerType = 'cryovial_tube' + barcode = container.cryovialBarcode || null + position = container.cryovialPosition || null + } else if (container.paperId != null) { + containerType = 'paper' + barcode = container.paperSublabel || null + } else if (container.wellId != null) { + containerType = 'static_well' + position = container.wellPosition || null + } + + // Format container type name (never expose DB IDs) + const containerTypeName = containerType + .split('_') + .map(word => word.charAt(0).toUpperCase() + word.slice(1)) + .join(' ') + + let label: string + if (barcode) { + label = `${containerTypeName} (${barcode})` + } else if (position) { + label = `${containerTypeName} at ${position}` + } else { + label = `Unnamed ${containerTypeName.toLowerCase()}` + } + + let context: string | undefined = container.specimenTypeName ?? undefined + if (container.subjectName) { + const subject = container.studyShortCode + ? `${container.subjectName} (${container.studyShortCode})` + : container.subjectName + context = `${container.specimenTypeName || ''} • ${subject}`.trim() + } + + return { + id: container.id, + type: 'container' as const, + timestamp: container.lastUpdated || container.created || '', + label, + context, + } + }) // Combine and sort by timestamp const allActivity = [ diff --git a/packages/api/src/routes/auth.ts b/packages/api/src/routes/auth.ts index 99284490..c53df0cd 100644 --- a/packages/api/src/routes/auth.ts +++ b/packages/api/src/routes/auth.ts @@ -13,6 +13,7 @@ import { handleRouteError } from '../lib/error-handler' import { toPublicUser, verifyLoginCredentials, + verifyPasswordConstantTime, createUserSession, registerApprovedUser, } from '../lib/auth/auth-service' @@ -49,6 +50,14 @@ const loginSchema = z.object({ password: z.string().min(1), }) +/** Usernames cannot contain "@", so a username can never be confused with an email at login. */ +const usernameSchema = z + .string() + .min(1) + .refine((value) => !value.includes('@'), { message: 'Username cannot contain "@"' }) + .optional() + .nullable() + // Dynamic register schema - password min length will be set based on settings const createRegisterSchema = async () => { const passwordRequirements = await getPasswordRequirementsFromDb() @@ -59,7 +68,7 @@ const createRegisterSchema = async () => { return z.object({ email: z.string().email(), name: z.string().min(1), - username: z.string().min(1).optional().nullable(), + username: usernameSchema, password: z.string().min(minLength), role: z.enum(['admin', 'member', 'viewer']).default('member'), }) @@ -113,7 +122,7 @@ const createSelfRegisterSchema = async () => { return z.object({ email: z.string().email(), name: z.string().min(1), - username: z.string().min(1).optional().nullable(), + username: usernameSchema, password: z.string().min(minLength), }) } @@ -245,7 +254,7 @@ auth.patch('/me', authMiddleware, async (c) => { const updateSchema = z.object({ name: z.string().min(1).optional(), email: z.string().email().optional(), - username: z.string().min(1).optional().nullable(), + username: usernameSchema, }) const data = updateSchema.parse(body) @@ -452,7 +461,7 @@ auth.post('/switch', rateLimit(10, 60 * 1000), authMiddleware, async (c) => { // Same response for a missing user and a wrong password, so the endpoint does not reveal // which accounts exist; approval status is only reported after the password checks out. - const valid = targetUser ? await bcrypt.compare(password, targetUser.passwordHash) : false + const valid = await verifyPasswordConstantTime(password, targetUser?.passwordHash) if (!targetUser || !valid) { return c.json({ error: 'Invalid password' }, 401) } @@ -527,7 +536,7 @@ auth.put('/users/:id', adminMiddleware, async (c) => { const updateSchema = z.object({ name: z.string().min(1).optional(), email: z.string().email().optional(), - username: z.string().min(1).optional().nullable(), + username: usernameSchema, role: z.enum(['admin', 'member', 'viewer']).optional(), }) const data = updateSchema.parse(body) diff --git a/packages/api/src/routes/collections.ts b/packages/api/src/routes/collections.ts index f07707b6..96f82c3a 100644 --- a/packages/api/src/routes/collections.ts +++ b/packages/api/src/routes/collections.ts @@ -41,7 +41,12 @@ import { deleteWithContentsBodySchema, } from '../lib/collections/collection-resolve' import { collectionDeletePreflightSchema } from '@sampledb/contract' -import { moveSheetsToCollection, SheetMoveTargetNotFoundError, SheetNotFoundError } from '../lib/collections/sheet-move' +import { + moveSheetsToCollection, + SheetMoveTargetNotFoundError, + SheetNameConflictError, + SheetNotFoundError, +} from '../lib/collections/sheet-move' import type { CollectionType } from '../lib/collections/types' function mapCreateCollectionError(error: unknown, c: { json: (body: unknown, status?: number) => Response }) { @@ -287,6 +292,9 @@ export function createCollectionsRoutes(database: Database): Hono { if (error instanceof SheetMoveTargetNotFoundError || error instanceof SheetNotFoundError) { return c.json({ error: error.message }, 404) } + if (error instanceof SheetNameConflictError) { + return c.json({ error: error.message }, 409) + } return handleRouteError(error, c) } }) diff --git a/packages/api/src/routes/containers.ts b/packages/api/src/routes/containers.ts index bbeb7787..b20a905b 100644 --- a/packages/api/src/routes/containers.ts +++ b/packages/api/src/routes/containers.ts @@ -160,7 +160,9 @@ containers.patch('/:id', memberMiddleware, async (c) => { const data = schema.parse(body) const { tagIds, unitId, barcode: rawBarcode, sublabel: rawSublabel, ...restData } = data - const updateData: { comment?: string; remainingQuantity?: number; unitId?: number } = { ...restData } + const updateData: { comment?: string | null; remainingQuantity?: number; unitId?: number } = { ...restData } + // An emptied comment clears it. + if (restData.comment !== undefined && restData.comment.trim() === '') updateData.comment = null const [container, micronixInfo, cryovialInfo, paperInfo, staticWellInfo] = await Promise.all([ database.select().from(storageContainer).where(eq(storageContainer.id, id)).get(), diff --git a/packages/api/src/routes/controls.ts b/packages/api/src/routes/controls.ts index 0cd43021..7328f40c 100644 --- a/packages/api/src/routes/controls.ts +++ b/packages/api/src/routes/controls.ts @@ -21,11 +21,13 @@ import { validateControlBatchCsv } from '../lib/controls/batch-csv-validate' import { createBloodControlBatchSchema, createBatchWithSpecimensSchema, + createBatchesWithSpecimensSchema, addSpecimensToBatchSchema, validateControlBatchCsvSchema, } from '../lib/controls/batch-schemas' import { createBatchWithSpecimens, + createBatchesWithSpecimens, addSpecimensToBatch, } from '../lib/controls/batch-with-specimens' import { getBloodControlBatchSummary } from '../lib/controls/batch-summary' @@ -548,9 +550,9 @@ controls.patch('/:id', memberMiddleware, async (c) => { } const body = await c.req.json() + // controlType is fixed at creation: these routes only serve blood definitions. const schema = z.object({ name: z.string().min(1).optional(), - controlType: z.enum(['blood', 'plasma_positive', 'plasma_negative', 'antibody', 'extraction', 'negative']).optional(), targetDensity: z.number().optional(), targetDensityUnitId: z.number().int().optional(), strains: z.array(strainCompositionEntrySchema).optional(), @@ -585,6 +587,37 @@ controls.patch('/:id', memberMiddleware, async (c) => { } } + if (baseData.name !== undefined && baseData.name !== existing.name) { + const existingByName = await dbInstance + .select({ id: controlDefinition.id }) + .from(controlDefinition) + .where(eq(controlDefinition.name, baseData.name)) + .get() + if (existingByName) { + return c.json({ error: 'A control definition with this name already exists' }, 409) + } + } + + // Same rule as create: no two definitions with the same density and strain mix. + if (strains !== undefined || targetDensity !== undefined || targetDensityUnitId !== undefined) { + const storedStrains = (newProps.strains as Array<{ id: number; percentage: number }> | undefined) ?? [] + const duplicate = await findBloodControlDefinitionByComposition( + dbInstance, + { + strains: storedStrains.map((strain) => ({ strainId: strain.id, percentage: strain.percentage })), + targetDensity: newProps.targetDensity as number | undefined, + targetDensityUnitId: newProps.targetDensityUnitId as number | undefined, + }, + { excludeId: id }, + ) + if (duplicate) { + return c.json({ + error: 'A control definition with this combination of density and strains already exists', + existingDefinition: duplicate, + }, 409) + } + } + // Update control definition const user = c.get('user') const [updatedControl] = await dbInstance @@ -685,6 +718,18 @@ controls.post('/:id/batches', memberMiddleware, async (c) => { } }) +// Create several batches (one per density) all-or-nothing +controls.post('/batches/create-many-with-specimens', memberMiddleware, async (c) => { + try { + const body = await c.req.json() + const data = createBatchesWithSpecimensSchema.parse(body) + const results = await createBatchesWithSpecimens(dbInstance, data.batches) + return c.json({ batches: results }, 201) + } catch (error) { + return handleRouteError(error, c) + } +}) + // Create batch with specimens controls.post('/batches/create-with-specimens', memberMiddleware, async (c) => { try { diff --git a/packages/api/src/routes/error-logs.ts b/packages/api/src/routes/error-logs.ts index d7b0e12e..9ebdfd8f 100644 --- a/packages/api/src/routes/error-logs.ts +++ b/packages/api/src/routes/error-logs.ts @@ -4,11 +4,12 @@ import type { Database } from '../db/client' import { errorLogs } from '../db/schema' import { logFrontendError, cleanupOldErrorLogs, type ErrorLogContext } from '../lib/error-logger' import { handleRouteError } from '../lib/error-handler' -import { eq, and, desc, sql, like, or } from 'drizzle-orm' +import { eq, and, desc, sql, or } from 'drizzle-orm' import { createAdminMiddleware, createAuthMiddleware, createOptionalAuthMiddleware } from '../middleware/auth' import { rateLimit } from '../middleware/rate-limit' import { utcNow } from '../lib/datetime' import { requireParam } from '../lib/common-validators' +import { likeContains } from '../lib/sql-like' // Schema for frontend error submission const frontendErrorSchema = z.object({ @@ -134,8 +135,8 @@ export function createErrorLogsRoutes(database: Database): Hono { if (search) { conditions.push( or( - like(errorLogs.message, `%${search}%`), - like(errorLogs.errorCode, `%${search}%`) + likeContains(errorLogs.message, search), + likeContains(errorLogs.errorCode, search) )! ) } diff --git a/packages/api/src/routes/locations.ts b/packages/api/src/routes/locations.ts index f8491146..f4de95f0 100644 --- a/packages/api/src/routes/locations.ts +++ b/packages/api/src/routes/locations.ts @@ -2,7 +2,7 @@ import { Hono } from 'hono' import type { Database } from '../db/client' import type { Database as SQLiteDatabase } from 'bun:sqlite' import { location, micronixPlate, cryovialBox, box, bag, storageType } from '../db/schema' -import { eq, and, sql, or, like, desc, isNull, inArray } from 'drizzle-orm' +import { eq, and, sql, or, desc, isNull, inArray } from 'drizzle-orm' import { validatePage, validateLimit } from '../lib/constants' import { z } from 'zod' import { @@ -19,6 +19,7 @@ import { handleRouteError, NotFoundError, ValidationError } from '../lib/error-h import { createAdminMiddleware, createAuthMiddleware } from '../middleware/auth' import { utcNow } from '../lib/datetime' import { requireParam } from '../lib/common-validators' +import { likeContains } from '../lib/sql-like' /** * Create locations routes with database injection @@ -51,12 +52,11 @@ locations.get('/', authMiddleware, async (c) => { // Build conditions for each word - each word must match at least one field // Note: storageTypeId is only on root locations, so we search it but it may be null const wordConditions = searchWords.map(word => { - const pattern = `%${word}%` return or( - like(location.name, pattern), - like(location.path, pattern), - sql`${location.storageTypeId} LIKE ${pattern}`, // Handle nullable storageTypeId - like(location.description, pattern) + likeContains(location.name, word), + likeContains(location.path, word), + likeContains(location.storageTypeId, word), // storageTypeId is null on child locations + likeContains(location.description, word) )! }) @@ -325,7 +325,8 @@ locations.post('/', adminMiddleware, async (c) => { .values({ parentId: data.parentId ?? null, name: data.name, - storageTypeId: data.parentId === null ? data.storageTypeId : null, // Only set for root locations + // Only set for root locations; an omitted parentId also means root. + storageTypeId: data.parentId == null ? data.storageTypeId : null, description: data.description, canContainCollections: data.canContainCollections, created: now, @@ -430,6 +431,10 @@ locations.put('/:id', adminMiddleware, async (c) => { return c.json({ error: 'Storage type ID must be null for child locations' }, 400) } } + // A child moved to the root needs a storage type (the table CHECK requires it). + if (isRoot && existing.parentId !== null && !data.storageTypeId) { + return c.json({ error: 'Storage type ID is required when moving a location to the root' }, 400) + } // Check for duplicate name if name or parent is being changed if (data.name !== undefined || data.parentId !== undefined) { @@ -459,8 +464,8 @@ locations.put('/:id', adminMiddleware, async (c) => { } if (data.parentId !== undefined) updateData.parentId = data.parentId ?? null if (data.name !== undefined) updateData.name = data.name - // Only set storageTypeId for root locations - if (data.storageTypeId !== undefined) { + // Only root locations have a storage type; moving a root under a parent clears it. + if (data.storageTypeId !== undefined || !isRoot) { updateData.storageTypeId = isRoot ? data.storageTypeId : null } if (data.description !== undefined) updateData.description = data.description ?? null diff --git a/packages/api/src/routes/qpcr-experiments.ts b/packages/api/src/routes/qpcr-experiments.ts index c09a8a3a..d1e1e526 100644 --- a/packages/api/src/routes/qpcr-experiments.ts +++ b/packages/api/src/routes/qpcr-experiments.ts @@ -27,6 +27,7 @@ import { resolveMicronixBarcodesToContainers } from '../lib/export/query' import { normalizeWellPosition, parsePlateCSV, validateWellPosition } from '../lib/plate-csv' import { utcNow } from '../lib/datetime' import { requireParam } from '../lib/common-validators' +import { withWriteTransaction } from '../db/write-transaction' /** * Attach the Source (subject vs control provenance) to each well, resolving @@ -289,25 +290,38 @@ export function createQpcrExperimentsRoutes(database: Database): Hono { if (data.standardLayout !== undefined) updates.standardLayout = data.standardLayout if (data.status !== undefined) updates.status = data.status if (data.instrumentType !== undefined) updates.instrumentType = data.instrumentType - if (data.targets !== undefined && (exp.status === 'setup' || exp.status === 'in_progress')) { - await database.delete(qpcrExperimentTarget).where(eq(qpcrExperimentTarget.qpcrExperimentId, id)) - if (data.targets.length > 0) { - await database.insert(qpcrExperimentTarget).values( - data.targets.map((t, i) => ({ + const newTargets = + data.targets !== undefined && (exp.status === 'setup' || exp.status === 'in_progress') + ? data.targets.map((t, i) => ({ qpcrExperimentId: id, targetName: t.targetName.trim() || 'varATS', fluorophore: t.fluorophore ?? null, reporter: t.reporter ?? null, sortOrder: i, })) - ) + : null + if (newTargets) { + const names = newTargets.map((t) => t.targetName) + const duplicate = names.find((name, i) => names.indexOf(name) !== i) + if (duplicate) { + return c.json({ error: `Target '${duplicate}' is listed more than once.` }, 400) } } - const [updated] = await database - .update(qpcrExperiment) - .set(updates) - .where(eq(qpcrExperiment.id, id)) - .returning() + // Replace targets and update the experiment together, so a failure keeps the old targets. + const updated = await withWriteTransaction(database, async (tx) => { + if (newTargets) { + await tx.delete(qpcrExperimentTarget).where(eq(qpcrExperimentTarget.qpcrExperimentId, id)) + if (newTargets.length > 0) { + await tx.insert(qpcrExperimentTarget).values(newTargets) + } + } + const [row] = await tx + .update(qpcrExperiment) + .set(updates) + .where(eq(qpcrExperiment.id, id)) + .returning() + return row + }) const targets = await database .select() .from(qpcrExperimentTarget) diff --git a/packages/api/src/routes/settings.ts b/packages/api/src/routes/settings.ts index a5ed9265..0a3b1a30 100644 --- a/packages/api/src/routes/settings.ts +++ b/packages/api/src/routes/settings.ts @@ -259,6 +259,9 @@ const tableViewConfigurationsSchema = z.object({ // Admins can specify userId in body to set for specific user, or omit for system-wide // Non-admins can only set user-specific settings for themselves const targetUserId = body.userId !== undefined ? body.userId : (isAdmin ? null : userId) + if (targetUserId !== null && targetUserId !== undefined && !(Number.isInteger(targetUserId) && targetUserId > 0)) { + return c.json({ error: 'userId must be a positive integer or null' }, 400) + } const actualBody = { ...body } delete actualBody.userId // Remove userId from body before validation diff --git a/packages/api/src/routes/studies.ts b/packages/api/src/routes/studies.ts index 4641f3d6..e006ccd8 100644 --- a/packages/api/src/routes/studies.ts +++ b/packages/api/src/routes/studies.ts @@ -247,8 +247,8 @@ studies.get('/:id/summary', authMiddleware, async (c) => { }) } - // Get all specimens for these subjects - const placeholders = subjectIds.map(() => '?').join(',') + // Get all specimens for these subjects. Subqueries (not id lists) keep large studies + // under SQLite's bound-parameter limit. const specimensQuery = ` SELECT s.id, @@ -258,10 +258,10 @@ studies.get('/:id/summary', authMiddleware, async (c) => { s.created, s.last_updated as lastUpdated FROM specimen s - WHERE s.study_subject_id IN (${placeholders}) + WHERE s.study_subject_id IN (SELECT id FROM study_subject WHERE study_id = ?) ` const stmt = sqliteDatabase.prepare(specimensQuery) - const specimens = stmt.all(...subjectIds) as Array<{ + const specimens = stmt.all(id) as Array<{ id: number studySubjectId: number specimenTypeId: number @@ -307,7 +307,6 @@ studies.get('/:id/summary', authMiddleware, async (c) => { }) } - const specimenIds = specimens.map(s => s.id) const specimenTypeIds = [...new Set(specimens.map(s => s.specimenTypeId))] // Get specimen types @@ -319,29 +318,30 @@ studies.get('/:id/summary', authMiddleware, async (c) => { const specimenTypeMap = new Map(specimenTypes.map(st => [st.id, st.name])) // Get all containers for these specimens + const studySpecimenIds = database + .select({ id: specimen.id }) + .from(specimen) + .where(inArray(specimen.studySubjectId, database.select({ id: studySubject.id }).from(studySubject).where(eq(studySubject.studyId, id)))) const containers = await database .select() .from(storageContainer) - .where(inArray(storageContainer.specimenId, specimenIds)) + .where(inArray(storageContainer.specimenId, studySpecimenIds)) const totalContainers = containers.length - const containerIds = containers.map(c => c.id) + const studyContainerIds = database + .select({ id: storageContainer.id }) + .from(storageContainer) + .where(inArray(storageContainer.specimenId, studySpecimenIds)) // Get container type information - const [micronixTubes, cryovialTubes, papers, staticWells] = await Promise.all([ - containerIds.length > 0 - ? database.select({ id: micronixTube.id }).from(micronixTube).where(inArray(micronixTube.id, containerIds)) - : [], - containerIds.length > 0 - ? database.select({ id: cryovialTube.id }).from(cryovialTube).where(inArray(cryovialTube.id, containerIds)) - : [], - containerIds.length > 0 - ? database.select({ id: paper.id }).from(paper).where(inArray(paper.id, containerIds)) - : [], - containerIds.length > 0 - ? database.select({ id: staticWell.id }).from(staticWell).where(inArray(staticWell.id, containerIds)) - : [], - ]) + const [micronixTubes, cryovialTubes, papers, staticWells] = totalContainers > 0 + ? await Promise.all([ + database.select({ id: micronixTube.id }).from(micronixTube).where(inArray(micronixTube.id, studyContainerIds)), + database.select({ id: cryovialTube.id }).from(cryovialTube).where(inArray(cryovialTube.id, studyContainerIds)), + database.select({ id: paper.id }).from(paper).where(inArray(paper.id, studyContainerIds)), + database.select({ id: staticWell.id }).from(staticWell).where(inArray(staticWell.id, studyContainerIds)), + ]) + : [[], [], [], []] // Create container type map const containerTypeMap = new Map() @@ -720,23 +720,16 @@ studies.delete('/:id', memberMiddleware, async (c) => { .where(eq(studySubject.studyId, id)) const subjectIds = subjects.map((s) => s.id) - let specimenIds: number[] = [] - if (subjectIds.length > 0) { - const specimens = await database - .select({ id: specimen.id }) - .from(specimen) - .where(inArray(specimen.studySubjectId, subjectIds)) - specimenIds = specimens.map((s) => s.id) - } - - let containerIds: number[] = [] - if (specimenIds.length > 0) { - const containers = await database + // Subqueries, not id lists, so large studies stay under SQLite's parameter limit. + const studySubjectIds = database.select({ id: studySubject.id }).from(studySubject).where(eq(studySubject.studyId, id)) + const studySpecimenIds = database.select({ id: specimen.id }).from(specimen).where(inArray(specimen.studySubjectId, studySubjectIds)) + const specimenIds = (await studySpecimenIds).map((s) => s.id) + const containerIds = ( + await database .select({ id: storageContainer.id }) .from(storageContainer) - .where(inArray(storageContainer.specimenId, specimenIds)) - containerIds = containers.map((c) => c.id) - } + .where(inArray(storageContainer.specimenId, studySpecimenIds)) + ).map((c) => c.id) await assertNotUsedInQpcr(database, { containerIds, specimenIds }, `study '${existingStudy.shortCode}'`) diff --git a/packages/api/src/scripts/__tests__/check-integrity.test.ts b/packages/api/src/scripts/__tests__/check-integrity.test.ts new file mode 100644 index 00000000..4c21a39e --- /dev/null +++ b/packages/api/src/scripts/__tests__/check-integrity.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import type { Database } from 'bun:sqlite' +import { setupTestDatabase, cleanupTestDatabase } from '../../__tests__/helpers/db-setup' +import { checkDatabaseIntegrity } from '../check-integrity' + +describe('checkDatabaseIntegrity', () => { + let sqlite: Database + + beforeEach(async () => { + sqlite = (await setupTestDatabase()).sqlite + }) + + afterEach(() => { + cleanupTestDatabase(sqlite) + }) + + it('reports a clean database as clean', () => { + expect(checkDatabaseIntegrity(sqlite).totalIssues).toBe(0) + }) + + it('reports orphaned tag links, which have no id column', () => { + sqlite.exec('PRAGMA foreign_keys = OFF') + sqlite.exec('INSERT INTO storage_container_tag (storage_container_id, tag_id) VALUES (999, 998)') + sqlite.exec('PRAGMA foreign_keys = ON') + + const tables = checkDatabaseIntegrity(sqlite).issues.map((issue) => `${issue.table}.${issue.column}`) + expect(tables).toContain('storage_container_tag.storage_container_id') + expect(tables).toContain('storage_container_tag.tag_id') + }) + + it('reports qPCR wells pointing at missing containers', () => { + sqlite.exec('PRAGMA foreign_keys = OFF') + sqlite.exec(`INSERT INTO qpcr_experiment_well (qpcr_experiment_id, well_position, storage_container_id) VALUES (999, 'A01', 998)`) + sqlite.exec('PRAGMA foreign_keys = ON') + + const tables = checkDatabaseIntegrity(sqlite).issues.map((issue) => `${issue.table}.${issue.column}`) + expect(tables).toContain('qpcr_experiment_well.qpcr_experiment_id') + expect(tables).toContain('qpcr_experiment_well.storage_container_id') + }) +}) diff --git a/packages/api/src/scripts/check-integrity.ts b/packages/api/src/scripts/check-integrity.ts index d731fd41..52b76861 100644 --- a/packages/api/src/scripts/check-integrity.ts +++ b/packages/api/src/scripts/check-integrity.ts @@ -12,7 +12,8 @@ * DATABASE_PATH=path/to/database.sqlite bun src/scripts/check-integrity.ts --export-json */ -import { openOperationalDatabase } from '../db/client' +import { Database } from 'bun:sqlite' +import { resolveOperationalDatabasePath } from '../db/open' import { writeFileSync } from 'fs' import { join } from 'path' @@ -59,7 +60,7 @@ function checkForeignKey( let query: string if (isSelfReference) { query = ` - SELECT COUNT(*) as count, GROUP_CONCAT(t1.id) as ids + SELECT COUNT(*) as count, GROUP_CONCAT(t1.rowid) as ids FROM ${table} t1 LEFT JOIN ${referencedTable} t2 ON t1.${column} = t2.${referencedColumn} WHERE t1.${column} IS NOT NULL @@ -67,7 +68,7 @@ function checkForeignKey( ` } else { query = ` - SELECT COUNT(*) as count, GROUP_CONCAT(${table}.id) as ids + SELECT COUNT(*) as count, GROUP_CONCAT(${table}.rowid) as ids FROM ${table} LEFT JOIN ${referencedTable} ON ${table}.${column} = ${referencedTable}.${referencedColumn} WHERE ${table}.${column} IS NOT NULL @@ -153,7 +154,7 @@ function getOrphanedIds( /** * Main integrity check function */ -function checkDatabaseIntegrity(sqlite: any): IntegrityReport { +export function checkDatabaseIntegrity(sqlite: any): IntegrityReport { const issues: IntegrityIssue[] = [] const startTime = new Date().toISOString() @@ -257,6 +258,15 @@ function checkDatabaseIntegrity(sqlite: any): IntegrityReport { ] issues.push(...derivationIssues.filter((issue): issue is IntegrityIssue => issue !== null)) + // qPCR checks + console.log('Checking qPCR relationships...') + const qpcrIssues = [ + checkForeignKey(sqlite, 'qpcr_experiment_well', 'qpcr_experiment_id', 'qpcr_experiment', 'id', 'qPCR wells with non-existent qpcr_experiment_id'), + checkForeignKey(sqlite, 'qpcr_experiment_well', 'storage_container_id', 'storage_container', 'id', 'qPCR wells with non-existent storage_container_id'), + checkForeignKey(sqlite, 'qpcr_experiment_well', 'specimen_id', 'specimen', 'id', 'qPCR wells with non-existent specimen_id'), + ] + issues.push(...qpcrIssues.filter((issue): issue is IntegrityIssue => issue !== null)) + // Other checks console.log('Checking other relationships...') const otherIssues = [ @@ -277,7 +287,7 @@ function checkDatabaseIntegrity(sqlite: any): IntegrityReport { entry.orphanedRecords += issue.orphanedCount } - const databasePath = process.env.DATABASE_PATH || 'sampledb_dev.sqlite' + const databasePath = sqlite.filename return { timestamp: startTime, @@ -343,8 +353,9 @@ function main() { const exportJson = args.includes('--export-json') try { - // Connect to database - const { sqlite } = openOperationalDatabase() + // Open read-only and without schema evolution: checking a backup must not migrate it. + const dbArg = args.find((arg) => arg.startsWith('--db='))?.slice('--db='.length) + const sqlite = new Database(resolveOperationalDatabasePath(dbArg), { readonly: true }) // Run integrity checks const report = checkDatabaseIntegrity(sqlite) diff --git a/packages/contract/src/__tests__/csv.test.ts b/packages/contract/src/__tests__/csv.test.ts index 6bf732f0..a74192b8 100644 --- a/packages/contract/src/__tests__/csv.test.ts +++ b/packages/contract/src/__tests__/csv.test.ts @@ -119,4 +119,33 @@ describe('serializeCsv + parseCsv round trip', () => { const serialized = serializeCsv(columns, rows) expect(parseCsv(serialized)).toEqual([columns, ...rows]) }) + + describe('delimiter and formula handling', () => { + it('quotes cells containing a non-comma delimiter so they round-trip', () => { + const csv = serializeCsv(['id', 'comment'], [[1, 'low; hemolyzed']], { delimiter: ';', bom: false, lineEnding: 'lf' }) + expect(csv).toBe('id;comment\n1;"low; hemolyzed"') + expect(parseCsv(csv, { delimiter: ';' })[1]).toEqual(['1', 'low; hemolyzed']) + }) + + it('drops blank and comma-only rows when asked', () => { + const text = 'a,b\r\n1,2\r\n,\r\n , \r\n3,4\r\n' + expect(parseCsv(text, { skipBlankRows: true })).toEqual([['a', 'b'], ['1', '2'], ['3', '4']]) + expect(parseCsv(text)).toHaveLength(5) + }) + + it('neutralizes formula-like text cells only when asked', () => { + const rows = [['=SUM(A1)', '+cmd', '@x', '-A1+1', "-1+cmd|'/C calc'!A0", '-131', '-0.5', '-2 in SubjID', -5, 'ok']] + const neutralized = serializeCsv(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'], rows, { + bom: false, + lineEnding: 'lf', + neutralizeFormulas: true, + }) + expect(neutralized.split('\n')[1]).toBe( + "'=SUM(A1),'+cmd,'@x,'-A1+1,'-1+cmd|'/C calc'!A0,-131,-0.5,'-2 in SubjID,-5,ok", + ) + + const plain = serializeCsv(['a'], [['=SUM(A1)']], { bom: false, lineEnding: 'lf' }) + expect(plain.split('\n')[1]).toBe('=SUM(A1)') + }) + }) }) diff --git a/packages/contract/src/csv.ts b/packages/contract/src/csv.ts index cc09bb72..765fd69a 100644 --- a/packages/contract/src/csv.ts +++ b/packages/contract/src/csv.ts @@ -4,6 +4,11 @@ export interface CSVExportOptions { delimiter?: string bom?: boolean lineEnding?: CsvLineEnding + /** + * Prefix text cells a spreadsheet would run as a formula with `'`. + * Use for exports of stored data; leave off for templates meant to be re-imported. + */ + neutralizeFormulas?: boolean } export type CsvCellValue = string | number | null | undefined @@ -12,10 +17,28 @@ const DEFAULT_DELIMITER = ',' const DEFAULT_BOM = true const DEFAULT_LINE_ENDING: CsvLineEnding = 'crlf' -export function escapeCsvCell(value: CsvCellValue): string { +/** A whole negative number such as "-131", "-0.5", or "-1e3". */ +const NEGATIVE_NUMBER = /^-(\d+\.?\d*|\.\d+)(e[-+]?\d+)?$/i + +/** + * Text a spreadsheet would evaluate: leading =, +, @, tab, or CR, and "-" unless the whole cell + * is a number, so stored negative values export unchanged. "-1+cmd|..." is a formula, not a number. + */ +function looksLikeFormula(s: string): boolean { + return /^[=+@\t\r]/.test(s) || (s.startsWith('-') && !NEGATIVE_NUMBER.test(s)) +} + +export function escapeCsvCell( + value: CsvCellValue, + options: { delimiter?: string; neutralizeFormulas?: boolean } = {}, +): string { if (value === null || value === undefined) return '' - const s = String(value) - if (s.includes(',') || s.includes('"') || s.includes('\n') || s.includes('\r')) { + const delimiter = options.delimiter ?? DEFAULT_DELIMITER + let s = String(value) + if (options.neutralizeFormulas && typeof value === 'string' && looksLikeFormula(s)) { + s = `'${s}` + } + if (s.includes(delimiter) || s.includes('"') || s.includes('\n') || s.includes('\r')) { return `"${s.replace(/"/g, '""')}"` } return s @@ -31,16 +54,24 @@ export function serializeCsv( const lineEnding = options.lineEnding ?? DEFAULT_LINE_ENDING const eol = lineEnding === 'crlf' ? '\r\n' : '\n' + const cellOptions = { delimiter, neutralizeFormulas: options.neutralizeFormulas } const formatRow = (row: CsvCellValue[]) => - row.map((cell) => escapeCsvCell(cell)).join(delimiter) + row.map((cell) => escapeCsvCell(cell, cellOptions)).join(delimiter) - const header = formatRow(columns) + const header = columns.map((cell) => escapeCsvCell(cell, { delimiter })).join(delimiter) const body = rows.map(formatRow).join(eol) const content = rows.length ? `${header}${eol}${body}` : header return bom ? `\uFEFF${content}` : content } -export function parseCsv(input: string, options: { delimiter?: string } = {}): string[][] { +/** Parse CSV text into rows of cells. skipBlankRows drops rows whose cells are all empty or whitespace. */ +export function parseCsv( + input: string, + options: { delimiter?: string; skipBlankRows?: boolean } = {}, +): string[][] { + if (options.skipBlankRows) { + return parseCsv(input, { delimiter: options.delimiter }).filter((cells) => cells.some((cell) => cell.trim() !== '')) + } const delimiter = options.delimiter ?? DEFAULT_DELIMITER let text = input if (text.charCodeAt(0) === 0xfeff) { diff --git a/packages/contract/src/export.ts b/packages/contract/src/export.ts index fb9d251b..6eab6c42 100644 --- a/packages/contract/src/export.ts +++ b/packages/contract/src/export.ts @@ -16,13 +16,17 @@ export interface ExportFilters { subject_ids?: number[] /** Global tolerance for exact subject collection dates (defaults to 0). */ date_tolerance?: number + /** + * Per-subject collection date filters. A subject may list several (one per visit); + * listed subjects without an entry match all their dates. + */ subject_dates?: { - [subjectId: number]: - | { exact: string } - | { from?: string; to?: string } + [subjectId: number]: SubjectDateFilter | SubjectDateFilter[] } } +export type SubjectDateFilter = { exact: string } | { from?: string; to?: string } + export interface ContainerExportData { container_id: number container_type: string diff --git a/packages/contract/src/index.ts b/packages/contract/src/index.ts index e50a1ec7..a25ff946 100644 --- a/packages/contract/src/index.ts +++ b/packages/contract/src/index.ts @@ -9,6 +9,7 @@ export { type ContainerExportData, type ExportFilters, + type SubjectDateFilter, } from './export' export { DEFAULT_EXPORT_COLUMN_KEYS, diff --git a/packages/web/src/components/BulkImportFlow.tsx b/packages/web/src/components/BulkImportFlow.tsx index 6a51cd45..a247dd62 100644 --- a/packages/web/src/components/BulkImportFlow.tsx +++ b/packages/web/src/components/BulkImportFlow.tsx @@ -30,6 +30,7 @@ import { } from '../lib/bulk-import-success-message' import LocationPicker from './LocationPicker' import '../styles/storage.css' +import { parseCsv } from '@sampledb/contract' export type ImportType = LibImportType type Step = 'upload' | 'collections' | 'import' @@ -136,14 +137,14 @@ export default function BulkImportFlow({ fixedStudyShortCode, backLink }: BulkIm const reader = new FileReader() reader.onload = (event) => { const text = event.target?.result as string - const lines = text.split('\n').filter(line => line.trim()) + const lines = parseCsv(text, { skipBlankRows: true }) if (lines.length === 0) return - const headers = lines[0].split(',').map(h => h.trim()) + const headers = lines[0].map(h => h.trim()) const previewRows: CSVRow[] = [] for (let i = 1; i < Math.min(6, lines.length); i++) { - const values = lines[i].split(',') + const values = lines[i] const row: CSVRow = {} headers.forEach((header, j) => { row[header] = values[j]?.trim() || '' diff --git a/packages/web/src/components/CollectionSpecimenEntry.tsx b/packages/web/src/components/CollectionSpecimenEntry.tsx index a5fc5e39..e883a7dd 100644 --- a/packages/web/src/components/CollectionSpecimenEntry.tsx +++ b/packages/web/src/components/CollectionSpecimenEntry.tsx @@ -109,13 +109,25 @@ export default function CollectionSpecimenEntry({ try { const validEntries: Array = [] - + const incompletePositions: string[] = [] + for (const [position, entry] of entries.entries()) { if (entry.studyShortCode && entry.subjectName && entry.specimenTypeName) { validEntries.push({ ...entry, position }) + } else if (entry.subjectName || entry.barcode || entry.collectionDate) { + // Started but missing study, subject, or specimen type: say so instead of skipping it. + incompletePositions.push(position) } } + if (incompletePositions.length > 0) { + setWorkflowError( + `Complete or clear these positions (study, subject, and specimen type are required): ${incompletePositions.join(', ')}`, + ) + setSubmitting(false) + return + } + if (validEntries.length === 0) { setWorkflowError('Please fill in at least one specimen entry') setSubmitting(false) @@ -144,7 +156,19 @@ export default function CollectionSpecimenEntry({ onSuccess?.() } } catch (err: unknown) { - setWorkflowError(getQueryErrorMessage(err, 'Failed to create specimens')) + // A 400 carries per-row errors; show them against their positions. + const rowErrors = (err as { response?: { data?: { errors?: Array<{ index: number; error: string }> } } }) + .response?.data?.errors + if (rowErrors && rowErrors.length > 0) { + const positionsByIndex = [...entries.entries()] + .filter(([, entry]) => entry.studyShortCode && entry.subjectName && entry.specimenTypeName) + .map(([position]) => position) + setWorkflowError( + rowErrors.map((e) => `${positionsByIndex[e.index] ?? `Row ${e.index + 1}`}: ${e.error}`).join('; '), + ) + } else { + setWorkflowError(getQueryErrorMessage(err, 'Failed to create specimens')) + } } finally { setSubmitting(false) } @@ -178,7 +202,14 @@ export default function CollectionSpecimenEntry({ )} -
+ { + if (e.key === 'Enter' && e.target instanceof HTMLInputElement) e.preventDefault() + }} + className="space-y-4" + >
diff --git a/packages/web/src/components/CollectionTableWithExport.tsx b/packages/web/src/components/CollectionTableWithExport.tsx index 89f7e6d2..da4ff810 100644 --- a/packages/web/src/components/CollectionTableWithExport.tsx +++ b/packages/web/src/components/CollectionTableWithExport.tsx @@ -21,7 +21,7 @@ export default function CollectionTableWithExport({ const handleExportCsv = () => { const header = columns.map((c) => c.label) const rowArrays = rows.map((row) => columns.map((col) => row[col.key] ?? '')) - const csv = buildCsv(header, rowArrays) + const csv = buildCsv(header, rowArrays, { neutralizeFormulas: true }) downloadCsv(csv, exportFilename) } diff --git a/packages/web/src/components/ContainerDerivationModal.tsx b/packages/web/src/components/ContainerDerivationModal.tsx index f3fe94e1..c44236a0 100644 --- a/packages/web/src/components/ContainerDerivationModal.tsx +++ b/packages/web/src/components/ContainerDerivationModal.tsx @@ -13,6 +13,7 @@ import CollectionSelectOrCreate from './CollectionSelectOrCreate' import { Modal } from '../ui' import { DERIVATION_TYPES } from '../lib/derivation-types' import '../styles/storage.css' +import { localToday } from '../lib/date-utils' interface ContainerDerivationModalProps { isOpen: boolean @@ -67,7 +68,7 @@ function ContainerDerivationModalContent({ unitSymbol: '', quantityUsed: undefined as number | undefined, reduceParentQuantity: true, - derivationDate: new Date().toISOString().split('T')[0], + derivationDate: localToday(), protocol: '', notes: '', properties: undefined as Record | undefined, diff --git a/packages/web/src/components/ContainerEditModal.tsx b/packages/web/src/components/ContainerEditModal.tsx index 81958f17..d600d54f 100644 --- a/packages/web/src/components/ContainerEditModal.tsx +++ b/packages/web/src/components/ContainerEditModal.tsx @@ -105,7 +105,8 @@ function ContainerEditModalForm({ // Only include fields that have changed if (formData.comment !== (container.comment || '')) { - updateData.comment = formData.comment || undefined + // Send '' rather than undefined, which JSON drops, so clearing the comment is saved. + updateData.comment = formData.comment } // Check if remainingQuantity changed diff --git a/packages/web/src/components/DerivationChainView.tsx b/packages/web/src/components/DerivationChainView.tsx index f814cdf5..ebade47b 100644 --- a/packages/web/src/components/DerivationChainView.tsx +++ b/packages/web/src/components/DerivationChainView.tsx @@ -6,6 +6,7 @@ import type { Derivation } from '../lib/api/derivations' import { containerDisplayIdentifier } from '../lib/container-display' import { formatDerivationType } from '../lib/derivation-types' import { getContainerTypeIcon, getContainerTypeName } from '../lib/icons' +import { parseDisplayDate } from '../lib/date-utils' interface DerivationChainViewProps { containerId: number @@ -190,7 +191,7 @@ export default function DerivationChainView({ containerId, onClose }: Derivation {renderContainerIdentifier(item.container)} {item.derivation.derivationDate && (
- {new Date(item.derivation.derivationDate).toLocaleDateString()} + {parseDisplayDate(item.derivation.derivationDate).toLocaleDateString()}
)} diff --git a/packages/web/src/components/SpecimenCard.tsx b/packages/web/src/components/SpecimenCard.tsx index 96968404..421295c1 100644 --- a/packages/web/src/components/SpecimenCard.tsx +++ b/packages/web/src/components/SpecimenCard.tsx @@ -1,5 +1,6 @@ import { Link } from 'react-router-dom' import { getSpecimenTypeIcon, getContainerTypeIcon, getContainerTypeName } from '../lib/icons' +import { parseDisplayDate } from '../lib/date-utils' import type { SubjectSummarySpecimen } from '../lib/api/subjects';interface SpecimenCardProps { specimen: SubjectSummarySpecimen } @@ -7,7 +8,7 @@ import type { SubjectSummarySpecimen } from '../lib/api/subjects';interface Spec export default function SpecimenCard({ specimen }: SpecimenCardProps) { const formatDate = (dateString?: string) => { if (!dateString) return null - return new Date(dateString).toLocaleDateString() + return parseDisplayDate(dateString).toLocaleDateString() } const formatContainerBreakdown = () => { diff --git a/packages/web/src/components/StudyCard.tsx b/packages/web/src/components/StudyCard.tsx index c365e8ea..3dbdc5d0 100644 --- a/packages/web/src/components/StudyCard.tsx +++ b/packages/web/src/components/StudyCard.tsx @@ -1,5 +1,6 @@ import { useState } from 'react' import { Link } from 'react-router-dom' +import { parseDisplayDate } from '../lib/date-utils' import type { Study, StudySummaryBasic } from '../lib/api/studies';interface StudySummaryData extends StudySummaryBasic { averageSpecimensPerSubject?: number studyDurationDays?: number | null @@ -59,7 +60,7 @@ export default function StudyCard({ study, summary, loading, onLoadSummary, vari const [expanded, setExpanded] = useState(false) const formatDate = (dateString: string) => { - return new Date(dateString).toLocaleDateString('en-US', { + return parseDisplayDate(dateString).toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' diff --git a/packages/web/src/components/StudyStats.tsx b/packages/web/src/components/StudyStats.tsx index 2be86191..51505a98 100644 --- a/packages/web/src/components/StudyStats.tsx +++ b/packages/web/src/components/StudyStats.tsx @@ -3,6 +3,7 @@ import type { StudySummary, StudyTimelineData } from '../lib/api/studies'; import StatCard from './StatCard' import StatChart from './StatChart' import { useDateFilter, defaultMinDate } from '../contexts/DateFilterContext' +import { parseDisplayDate } from '../lib/date-utils' interface StudyStatsProps { summary: StudySummary['summary'] @@ -359,13 +360,13 @@ export default function StudyStats({ summary, timelineData, statCardClassName, c

Collection Date Range

- {new Date(displaySummary.collectionDateRange.earliest).toLocaleDateString('en-US', { + {parseDisplayDate(displaySummary.collectionDateRange.earliest).toLocaleDateString('en-US', { month: 'long', day: 'numeric', year: 'numeric', })}{' '} -{' '} - {new Date(displaySummary.collectionDateRange.latest).toLocaleDateString('en-US', { + {parseDisplayDate(displaySummary.collectionDateRange.latest).toLocaleDateString('en-US', { month: 'long', day: 'numeric', year: 'numeric', diff --git a/packages/web/src/components/__tests__/CollectionSpecimenEntry.test.tsx b/packages/web/src/components/__tests__/CollectionSpecimenEntry.test.tsx new file mode 100644 index 00000000..eaffa6c0 --- /dev/null +++ b/packages/web/src/components/__tests__/CollectionSpecimenEntry.test.tsx @@ -0,0 +1,60 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import userEvent from '@testing-library/user-event' +import { render, screen } from '../../__tests__/helpers/render' +import CollectionSpecimenEntry from '../CollectionSpecimenEntry' +import { specimensApi } from '../../lib/api/specimens' + +vi.mock('../../hooks/useStudies', () => ({ + useStudies: () => ({ + data: { studies: [{ id: 1, shortCode: 'ST1', title: 'Study 1' }] }, + isLoading: false, + isError: false, + }), +})) +vi.mock('../../hooks/useReferenceData', () => ({ + useSpecimenTypes: () => ({ data: [{ id: 1, name: 'DNA' }], isLoading: false, isError: false }), +})) +vi.mock('../../lib/api/specimens', async () => { + const { createMockedDomainModule } = await import('../../__tests__/helpers/mock-api') + return createMockedDomainModule('specimens', { specimensApi: { createBulk: vi.fn() } }) +}) + +async function renderEntry() { + await render( + , + ) +} + +describe('CollectionSpecimenEntry', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('does not submit when a barcode scanner sends Enter', async () => { + const user = userEvent.setup() + await renderEntry() + + await user.type(screen.getAllByPlaceholderText('Barcode')[0], 'MT001{Enter}') + + // A submit would validate the half-filled row and show this message. + expect(screen.queryByText(/Complete or clear these positions/)).not.toBeInTheDocument() + expect(specimensApi.createBulk).not.toHaveBeenCalled() + }) + + it('reports partly filled positions instead of skipping them', async () => { + const user = userEvent.setup() + await renderEntry() + await user.type(screen.getAllByPlaceholderText('Subject name')[1], 'S2') + + await user.click(screen.getByRole('button', { name: /create|add|save/i })) + + expect(await screen.findByText(/Complete or clear these positions.*A02/)).toBeInTheDocument() + expect(specimensApi.createBulk).not.toHaveBeenCalled() + }) +}) diff --git a/packages/web/src/components/wizards/BatchInfoStep.tsx b/packages/web/src/components/wizards/BatchInfoStep.tsx index 916abfcc..13556866 100644 --- a/packages/web/src/components/wizards/BatchInfoStep.tsx +++ b/packages/web/src/components/wizards/BatchInfoStep.tsx @@ -8,6 +8,7 @@ import { useStrains } from '../../hooks/useReferenceData' import { PageError, SectionMessage, getQueryErrorMessage } from '../../ui' import { Modal } from '../../ui' import ControlDefinitionForm from '../forms/ControlDefinitionForm' +import { localToday } from '../../lib/date-utils' const STRAIN_BAR_COLORS = [ 'rgb(var(--app-accent))', @@ -133,7 +134,7 @@ export default function BatchInfoStep({ suggestedName = response.name setNameSuggestion(suggestedName) } catch { - const date = batchInfo.productionDate || new Date().toISOString().split('T')[0] + const date = batchInfo.productionDate || localToday() suggestedName = `${definition.name} ${date}` setNameSuggestion(null) } diff --git a/packages/web/src/components/wizards/ReviewStep.tsx b/packages/web/src/components/wizards/ReviewStep.tsx index d9a99450..0420277f 100644 --- a/packages/web/src/components/wizards/ReviewStep.tsx +++ b/packages/web/src/components/wizards/ReviewStep.tsx @@ -5,9 +5,11 @@ import { type FlatControlBatchContainer, } from '../../lib/control-batch-payload' import { settingsApi } from '../../lib/api/settings'; -import type { ControlDefinition } from '../../lib/api/controls'; +import { getQueryErrorMessage } from '../../ui' +import type { ControlDefinition, CreateBatchWithSpecimensRequest } from '../../lib/api/controls'; import { normalizePosition, groupRowsByDensity } from '../../lib/control-batch-csv' import type { BatchInfo, SpecimenTypeConfig, CSVFileData, CompositionStrains } from '../../pages/ControlBatchWizard' +import { localToday } from '../../lib/date-utils' /** Row key for batch definition selection: fileIndex + densityKey (numeric density for matching). */ function batchRowKey(fileIndex: number, densityKey: number | undefined): string { @@ -121,7 +123,7 @@ export default function ReviewStep({ const handleMultiBatchSubmit = async () => { setError(null) - const productionDate = batchInfo.productionDate || new Date().toISOString().split('T')[0] + const productionDate = batchInfo.productionDate || localToday() for (const file of csvFiles) { const hasCollection = @@ -147,7 +149,7 @@ export default function ReviewStep({ setSubmitting(true) try { - let lastBatchId: number | null = null + const batchPayloads: CreateBatchWithSpecimensRequest[] = [] for (const batchRow of multiBatchRows) { const definitionId = batchRow.candidates.length === 1 ? batchRow.candidates[0]!.id @@ -196,7 +198,7 @@ export default function ReviewStep({ specimenTypeName, containers, })) - const res = await controlsApi.createBatchWithSpecimens({ + batchPayloads.push({ batch: { controlDefinitionId: definitionId, name: batchName, @@ -207,16 +209,19 @@ export default function ReviewStep({ containers: containers.map(flatControlBatchContainerToWriteInput), })), }) - const batchId = res.batch?.id - if (batchId != null) lastBatchId = batchId } - if (lastBatchId != null) onSuccess(lastBatchId) - else { - setError('No batches were created') + if (batchPayloads.length === 0) { + setError('No control batches were created') + return } + // One request: the server creates every batch or none, so a retry cannot duplicate. + const res = await controlsApi.createBatchesWithSpecimens({ batches: batchPayloads }) + const created = res.batches + if (created.length > 0) onSuccess(created[created.length - 1].batch.id) + else setError('No control batches were created') } catch (err: unknown) { console.error('Multi-batch submit failed:', err) - setError(err instanceof Error ? err.message : 'Failed to create batches') + setError(getQueryErrorMessage(err, 'Failed to create batches')) } finally { setSubmitting(false) } diff --git a/packages/web/src/components/wizards/__tests__/ReviewStep.test.tsx b/packages/web/src/components/wizards/__tests__/ReviewStep.test.tsx index 051b536c..2c141214 100644 --- a/packages/web/src/components/wizards/__tests__/ReviewStep.test.tsx +++ b/packages/web/src/components/wizards/__tests__/ReviewStep.test.tsx @@ -5,6 +5,7 @@ import type { BatchInfo, CSVFileData, CompositionStrains } from '../../../pages/ import type { ControlDefinition } from '../../../lib/api/controls' const mockCreateBatchWithSpecimens = vi.fn() +const mockCreateBatchesWithSpecimens = vi.fn() const mockSuggestBatchName = vi.fn() vi.mock('../../../lib/api/controls', async () => { @@ -12,6 +13,7 @@ vi.mock('../../../lib/api/controls', async () => { return createMockedDomainModule('controls', { controlsApi: { createBatchWithSpecimens: (...args: unknown[]) => mockCreateBatchWithSpecimens(...args), + createBatchesWithSpecimens: (...args: unknown[]) => mockCreateBatchesWithSpecimens(...args), suggestBatchName: (...args: unknown[]) => mockSuggestBatchName(...args), }, }) @@ -65,7 +67,7 @@ describe('ReviewStep multi-batch CSV', () => { beforeEach(() => { vi.clearAllMocks() mockSuggestBatchName.mockResolvedValue({ name: 'Suggested Batch' }) - mockCreateBatchWithSpecimens.mockResolvedValue({ batch: { id: 101 }, specimens: [], createdCollections: [] }) + mockCreateBatchesWithSpecimens.mockResolvedValue({ batches: [{ batch: { id: 101 }, specimens: [], createdCollections: [] }] }) }) it('does not call createBatchWithSpecimens when a density has no matching definition', async () => { @@ -107,7 +109,7 @@ describe('ReviewStep multi-batch CSV', () => { expect(submitBtn).toBeDisabled() fireEvent.click(submitBtn) - expect(mockCreateBatchWithSpecimens).not.toHaveBeenCalled() + expect(mockCreateBatchesWithSpecimens).not.toHaveBeenCalled() }) it('calls createBatchWithSpecimens with correct definition IDs when all densities match', async () => { @@ -158,11 +160,13 @@ describe('ReviewStep multi-batch CSV', () => { fireEvent.click(submitBtn) await waitFor(() => { - expect(mockCreateBatchWithSpecimens).toHaveBeenCalledTimes(2) + // Both densities go in one all-or-nothing request. + expect(mockCreateBatchesWithSpecimens).toHaveBeenCalledTimes(1) }) expect(mockSuggestBatchName).toHaveBeenCalled() - const createCalls = mockCreateBatchWithSpecimens.mock.calls - const definitionIds = createCalls.map((c) => c[0].batch.controlDefinitionId) + const batches = mockCreateBatchesWithSpecimens.mock.calls[0][0].batches as Array<{ batch: { controlDefinitionId: number } }> + expect(batches).toHaveLength(2) + const definitionIds = batches.map((b) => b.batch.controlDefinitionId) expect(definitionIds).toContain(10) expect(definitionIds).toContain(11) }) @@ -217,13 +221,43 @@ describe('ReviewStep multi-batch CSV', () => { fireEvent.click(submitBtn) await waitFor(() => { - expect(mockCreateBatchWithSpecimens).toHaveBeenCalledTimes(1) + expect(mockCreateBatchesWithSpecimens).toHaveBeenCalledTimes(1) + }) + expect(mockCreateBatchesWithSpecimens).toHaveBeenCalledWith({ + batches: [expect.objectContaining({ batch: expect.objectContaining({ controlDefinitionId: 21 }) })], }) - expect(mockCreateBatchWithSpecimens).toHaveBeenCalledWith( - expect.objectContaining({ - batch: expect.objectContaining({ controlDefinitionId: 21 }), - }) + }) +}) + +describe('ReviewStep multi-batch CSV errors', () => { + it("shows the server's error instead of the HTTP status text", async () => { + vi.clearAllMocks() + mockSuggestBatchName.mockResolvedValue({ name: 'Suggested Batch' }) + mockCreateBatchesWithSpecimens.mockRejectedValue( + Object.assign(new Error('Request failed with status code 400'), { + response: { data: { error: "Control batch 2 ('B'): Specimen type not found. No control batches were created." } }, + }), + ) + const def5000: ControlDefinition = { + id: 10, name: 'Def 5000', controlType: 'blood', created: '', lastUpdated: '', targetDensity: 5000, unitSymbol: 'µL', + } + render( + {}} + onCancel={() => {}} + onSuccess={() => {}} + isAddMode={false} + /> ) + + fireEvent.click(screen.getByRole('button', { name: /Create Batch/i })) + + expect(await screen.findByText(/No control batches were created/)).toBeInTheDocument() }) }) @@ -233,7 +267,7 @@ describe('ReviewStep multi-batch CSV nested collection placement', () => { beforeEach(() => { vi.clearAllMocks() mockSuggestBatchName.mockResolvedValue({ name: 'Suggested Batch' }) - mockCreateBatchWithSpecimens.mockResolvedValue({ batch: { id: 103 }, specimens: [], createdCollections: [] }) + mockCreateBatchesWithSpecimens.mockResolvedValue({ batches: [{ batch: { id: 103 }, specimens: [], createdCollections: [] }] }) }) it('nests collection.parent when CSV file has collectionName but no collectionId', async () => { @@ -281,9 +315,9 @@ describe('ReviewStep multi-batch CSV nested collection placement', () => { fireEvent.click(submitBtn) await waitFor(() => { - expect(mockCreateBatchWithSpecimens).toHaveBeenCalledTimes(1) + expect(mockCreateBatchesWithSpecimens).toHaveBeenCalledTimes(1) }) - const payload = mockCreateBatchWithSpecimens.mock.calls[0][0] + const payload = mockCreateBatchesWithSpecimens.mock.calls[0][0].batches[0] expect(payload).not.toHaveProperty('createCollections') expect(payload.specimens[0].containers[0]).toEqual({ containerType: 'paper', diff --git a/packages/web/src/contexts/UserContext.tsx b/packages/web/src/contexts/UserContext.tsx index 7946c5f3..fc318211 100644 --- a/packages/web/src/contexts/UserContext.tsx +++ b/packages/web/src/contexts/UserContext.tsx @@ -1,5 +1,7 @@ -import { useContext, useState, useEffect, useMemo, useCallback, ReactNode } from 'react' +import { useContext, useState, useEffect, useMemo, useCallback, useRef, ReactNode } from 'react' +import { useQueryClient } from '@tanstack/react-query' import { authApi, type User } from '../lib/api/auth' +import { setSessionExpiredHandler } from '../lib/api/client' import { addRecentUser } from '../lib/localUserHistory' import { UserContext } from './user-context-instance' @@ -9,17 +11,41 @@ let userCachedUser: User | null = null let userCachedLoading = true let userCachedError: string | null = null +/** For tests: forget the once-per-app-load user cache so each test starts signed out. */ +export function resetUserContextForTesting(): void { + userDidInit = false + userCachedUser = null + userCachedLoading = true + userCachedError = null +} + export function UserProvider({ children }: { children: ReactNode }) { const [user, setUserState] = useState(null) const [loading, setLoading] = useState(true) const [error, setError] = useState(null) + const queryClient = useQueryClient() + const currentUserId = useRef(null) + const setUser = useCallback((u: User | null) => { + // Cached queries include per-user data (personal export configs, settings); never show + // one user's cache to the next after a logout, login, or switch. + const nextId = u?.id ?? null + if (currentUserId.current !== null && currentUserId.current !== nextId) { + queryClient.clear() + } + currentUserId.current = nextId setUserState(u) userCachedUser = u userCachedLoading = false userCachedError = null - }, []) + }, [queryClient]) + + // A 401 from a data request means the session is gone: sign out so AuthGuard redirects. + useEffect(() => { + setSessionExpiredHandler(() => setUser(null)) + return () => setSessionExpiredHandler(null) + }, [setUser]) const refreshUser = useCallback(async () => { setLoading(true) @@ -53,7 +79,7 @@ export function UserProvider({ children }: { children: ReactNode }) { setLoading(false) userCachedLoading = false } - }, []) + }, [setUser]) const switchUser = useCallback(async (userId: number, password: string) => { try { @@ -101,7 +127,7 @@ export function UserProvider({ children }: { children: ReactNode }) { // Calling refreshUser() could fail and log the user out even though they're still logged in throw new Error(errorMessage) } - }, []) + }, [setUser]) useEffect(() => { if (!userDidInit) { diff --git a/packages/web/src/contexts/__tests__/UserContext.test.tsx b/packages/web/src/contexts/__tests__/UserContext.test.tsx new file mode 100644 index 00000000..8e944edc --- /dev/null +++ b/packages/web/src/contexts/__tests__/UserContext.test.tsx @@ -0,0 +1,61 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { renderHook, act, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import type { ReactNode } from 'react' +import { UserProvider, useUser, resetUserContextForTesting } from '../UserContext' +import { authApi } from '../../lib/api/auth' +import { axiosApi } from '../../lib/api/client' + +const alice = { id: 1, email: 'alice@lab.org', name: 'Alice', role: 'member' as const } +const bob = { id: 2, email: 'bob@lab.org', name: 'Bob', role: 'member' as const } + +describe('UserProvider', () => { + let queryClient: QueryClient + + beforeEach(() => { + resetUserContextForTesting() + vi.mocked(authApi.getCurrentUser).mockResolvedValue({ user: alice } as never) + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + }) + + function renderUser() { + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ) + return renderHook(() => useUser(), { wrapper }) + } + + it("clears cached queries when a different user signs in, so the next user never sees the last one's data", async () => { + const { result } = renderUser() + await waitFor(() => expect(result.current.user?.id).toBe(alice.id)) + queryClient.setQueryData(['export-workflow', 'configurations'], ['Alice personal config']) + + act(() => result.current.setUser(alice)) + expect(queryClient.getQueryData(['export-workflow', 'configurations'])).toBeDefined() + + act(() => result.current.setUser(bob)) + expect(queryClient.getQueryData(['export-workflow', 'configurations'])).toBeUndefined() + }) + + it('signs the user out when a data request returns 401', async () => { + const { result } = renderUser() + await waitFor(() => expect(result.current.user?.id).toBe(alice.id)) + const originalAdapter = axiosApi.defaults.adapter + axiosApi.defaults.adapter = async (config) => { + throw Object.assign(new Error('Unauthorized'), { + isAxiosError: true, + config, + response: { status: 401, headers: {}, config, data: {} }, + }) + } + + await act(async () => { + await axiosApi.get('/studies').catch(() => undefined) + }) + + axiosApi.defaults.adapter = originalAdapter + expect(result.current.user).toBeNull() + }) +}) diff --git a/packages/web/src/hooks/__tests__/useSingleStudyExportWorkflow.test.tsx b/packages/web/src/hooks/__tests__/useSingleStudyExportWorkflow.test.tsx index ae05f955..dc95fa12 100644 --- a/packages/web/src/hooks/__tests__/useSingleStudyExportWorkflow.test.tsx +++ b/packages/web/src/hooks/__tests__/useSingleStudyExportWorkflow.test.tsx @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { renderHook, waitFor, act } from '@testing-library/react' import { exportApi } from '../../lib/api/export' -import { useSingleStudyExportWorkflow } from '../useSingleStudyExportWorkflow' +import { buildSubjectDatesFromCsv, useSingleStudyExportWorkflow } from '../useSingleStudyExportWorkflow' vi.mock('../../lib/export-filter-csv', () => ({ parseExportModalCsv: vi.fn(), @@ -148,3 +148,16 @@ describe('useSingleStudyExportWorkflow', () => { expect(exportApi.downloadEnvelope).toHaveBeenCalled() }) }) + +describe('buildSubjectDatesFromCsv', () => { + it('keeps every visit per subject and drops subjects that have an undated row', () => { + expect( + buildSubjectDatesFromCsv([ + { subject_name: 'S01', collection_date: '2024-01-01' }, + { subject_name: 'S01', collection_date: '2024-02-01' }, + { subject_name: 'S02', collection_date: '2024-03-01' }, + { subject_name: 'S02' }, + ]), + ).toEqual({ S01: [{ exact: '2024-01-01' }, { exact: '2024-02-01' }] }) + }) +}) diff --git a/packages/web/src/hooks/useControls.ts b/packages/web/src/hooks/useControls.ts index cfde3518..080ea730 100644 --- a/packages/web/src/hooks/useControls.ts +++ b/packages/web/src/hooks/useControls.ts @@ -3,6 +3,7 @@ import { controlsApi, type ControlBatch, type ControlDefinition } from '../lib/a import { strainsApi } from '../lib/api/reference-data' import type { Strain } from '../lib/api/reference-data' import { getCompositionKey } from '../lib/composition-key' +import { localToday } from '../lib/date-utils' export const controlKeys = { all: ['blood-controls'] as const, @@ -113,7 +114,7 @@ export function useControlDefinitionWizardSeed(definitionId: number | undefined) return useQuery({ queryKey: [...controlKeys.all, 'wizard-definition-seed', definitionId] as const, queryFn: async () => { - const today = new Date().toISOString().split('T')[0] + const today = localToday() const [summaryResponse, nameResponse] = await Promise.all([ controlsApi.getDefinitionSummary(definitionId!), controlsApi.suggestBatchName(definitionId!, today), diff --git a/packages/web/src/hooks/useSingleStudyExportWorkflow.ts b/packages/web/src/hooks/useSingleStudyExportWorkflow.ts index bddc0bda..9b655e37 100644 --- a/packages/web/src/hooks/useSingleStudyExportWorkflow.ts +++ b/packages/web/src/hooks/useSingleStudyExportWorkflow.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { exportApi, type ExportFilters } from '../lib/api/export' +import type { SubjectDateFilter } from '@sampledb/contract' import { parseExportModalCsv, type SingleStudyExportCsvRow } from '../lib/export-filter-csv' import { formatLocalDateTime } from '../lib/date-utils' import { toggleArrayFilterValue } from '../lib/filter-array-toggle' @@ -16,18 +17,26 @@ export type SingleStudyExportSubmitParams = { columns: string[] } -function buildSubjectDatesFromCsv(csvData: SingleStudyExportCsvRow[]) { - const subjectDates: { [key: string]: { exact?: string; from?: string; to?: string } } = {} +/** + * Per-subject date filters from the uploaded CSV. A subject on several rows keeps every + * date (one per visit); a row without a date means all of that subject's dates. + */ +export function buildSubjectDatesFromCsv(csvData: SingleStudyExportCsvRow[]) { + const subjectDates: { [key: string]: SubjectDateFilter[] } = {} + const allDates = new Set() for (const row of csvData) { - if (row.collection_date) { - subjectDates[row.subject_name] = { exact: row.collection_date } - } else if (row.date_from || row.date_to) { - subjectDates[row.subject_name] = { - from: row.date_from, - to: row.date_to, - } + const dateFilter: SubjectDateFilter | null = row.collection_date + ? { exact: row.collection_date } + : row.date_from || row.date_to + ? { from: row.date_from, to: row.date_to } + : null + if (!dateFilter) { + allDates.add(row.subject_name) + continue } + ;(subjectDates[row.subject_name] ??= []).push(dateFilter) } + for (const name of allDates) delete subjectDates[name] return Object.keys(subjectDates).length > 0 ? subjectDates : undefined } diff --git a/packages/web/src/lib/__tests__/base64.test.ts b/packages/web/src/lib/__tests__/base64.test.ts new file mode 100644 index 00000000..f788ca29 --- /dev/null +++ b/packages/web/src/lib/__tests__/base64.test.ts @@ -0,0 +1,16 @@ +import { describe, it, expect } from 'vitest' +import { bytesToBase64 } from '../base64' + +describe('bytesToBase64', () => { + it('matches btoa for small input', () => { + const bytes = new TextEncoder().encode('Well,Cq\nA1,22.1') + expect(bytesToBase64(bytes)).toBe(btoa('Well,Cq\nA1,22.1')) + }) + + it('encodes files larger than the argument limit', () => { + const bytes = new Uint8Array(2_000_000).map((_, i) => i % 256) + const encoded = bytesToBase64(bytes) + expect(encoded.length).toBe(Math.ceil(bytes.length / 3) * 4) + expect(atob(encoded).charCodeAt(1_999_999)).toBe(1_999_999 % 256) + }) +}) diff --git a/packages/web/src/lib/__tests__/bulk-import-validation.test.ts b/packages/web/src/lib/__tests__/bulk-import-validation.test.ts index 01e8c5a7..94110996 100644 --- a/packages/web/src/lib/__tests__/bulk-import-validation.test.ts +++ b/packages/web/src/lib/__tests__/bulk-import-validation.test.ts @@ -75,6 +75,11 @@ describe('bulk-import-validation', () => { expect(rows[0].position).toBe('A01') expect(rows[0].study_short_code).toBe('ST1') }) + it('keeps quoted commas inside a cell and handles CRLF', () => { + const csv = 'subject_name,comment,position\r\nS1,"low vol, hemolyzed",A01\r\n' + const rows = parseBulkImportCSV(csv) + expect(rows).toEqual([{ subject_name: 'S1', comment: 'low vol, hemolyzed', position: 'A01' }]) + }) }) describe('mapBulkImportRowsToPayload', () => { diff --git a/packages/web/src/lib/__tests__/date-utils.test.ts b/packages/web/src/lib/__tests__/date-utils.test.ts index 127da2f4..19f7742d 100644 --- a/packages/web/src/lib/__tests__/date-utils.test.ts +++ b/packages/web/src/lib/__tests__/date-utils.test.ts @@ -72,3 +72,22 @@ describe('date-utils', () => { }) }) }) + +describe('calendar dates in local time', () => { + it('parseDisplayDate keeps a bare YYYY-MM-DD on its calendar day', async () => { + const { parseDisplayDate } = await import('../date-utils') + const d = parseDisplayDate('2024-01-15') + expect([d.getFullYear(), d.getMonth(), d.getDate()]).toEqual([2024, 0, 15]) + }) + + it('formatDate shows a collection date on its own day', async () => { + const { formatDate } = await import('../date-utils') + expect(formatDate('2024-01-15')).toBe('Jan 15, 2024') + }) + + it('toLocalDateString uses the local date, not the UTC date', async () => { + const { toLocalDateString } = await import('../date-utils') + // 11pm local on Jan 15 is already Jan 16 in UTC for any zone west of UTC. + expect(toLocalDateString(new Date(2024, 0, 15, 23, 30))).toBe('2024-01-15') + }) +}) diff --git a/packages/web/src/lib/__tests__/scanner-plate-csv.test.ts b/packages/web/src/lib/__tests__/scanner-plate-csv.test.ts index 90654966..e558f79f 100644 --- a/packages/web/src/lib/__tests__/scanner-plate-csv.test.ts +++ b/packages/web/src/lib/__tests__/scanner-plate-csv.test.ts @@ -42,6 +42,11 @@ describe('parseScannerPlateCsv', () => { expect(rows[1]).toMatchObject({ container_barcode: '', target_position: 'B02' }) }) + it('reads quoted scanner exports', () => { + const rows = parseScannerPlateCsv('"container_barcode","target_position"\r\n"MTX1","A01"', singleConfig) + expect(rows[0]).toMatchObject({ container_barcode: 'MTX1', target_position: 'A01' }) + }) + it('returns no rows for an empty or header-only file', () => { expect(parseScannerPlateCsv('', singleConfig)).toEqual([]) expect(parseScannerPlateCsv('container_barcode,target_position', singleConfig)).toEqual([]) diff --git a/packages/web/src/lib/api/__tests__/client.test.ts b/packages/web/src/lib/api/__tests__/client.test.ts index 764a9f5c..e3878ce3 100644 --- a/packages/web/src/lib/api/__tests__/client.test.ts +++ b/packages/web/src/lib/api/__tests__/client.test.ts @@ -1,10 +1,11 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import type { InternalAxiosRequestConfig } from 'axios' import { axiosApi, getLastResponseRequestId, resetRequestIdStateForTesting, REQUEST_ID_HEADER, + setSessionExpiredHandler, } from '../client' describe('axiosApi request correlation', () => { @@ -89,3 +90,42 @@ function readHeader(headers: unknown, name: string): string | undefined { const record = headers as Record return record[name] ?? record[name.toLowerCase()] } + +describe('session expiry', () => { + const originalAdapter = axiosApi.defaults.adapter + + afterEach(() => { + axiosApi.defaults.adapter = originalAdapter + setSessionExpiredHandler(null) + }) + + function respond401() { + axiosApi.defaults.adapter = async (config: InternalAxiosRequestConfig) => { + const error = Object.assign(new Error('Unauthorized'), { + isAxiosError: true, + config, + response: { status: 401, headers: {}, config, data: { error: 'Invalid or expired session' } }, + }) + throw error + } + } + + it('calls the handler when a data request gets 401', async () => { + const handler = vi.fn() + setSessionExpiredHandler(handler) + respond401() + + await expect(axiosApi.get('/studies')).rejects.toThrow() + expect(handler).toHaveBeenCalledTimes(1) + }) + + it('does not call the handler for auth endpoints (wrong password, not signed in)', async () => { + const handler = vi.fn() + setSessionExpiredHandler(handler) + respond401() + + await expect(axiosApi.post('/auth/login', {})).rejects.toThrow() + await expect(axiosApi.get('/auth/me')).rejects.toThrow() + expect(handler).not.toHaveBeenCalled() + }) +}) diff --git a/packages/web/src/lib/api/client.ts b/packages/web/src/lib/api/client.ts index 27d1ae09..16610488 100644 --- a/packages/web/src/lib/api/client.ts +++ b/packages/web/src/lib/api/client.ts @@ -31,6 +31,22 @@ function readRequestIdHeader(headers: unknown): string | undefined { return undefined } +type SessionExpiredHandler = () => void +let sessionExpiredHandler: SessionExpiredHandler | null = null + +/** + * Called when a non-auth request gets 401 (session expired or revoked). The user context + * registers a handler that signs the user out, so the app redirects to login instead of + * showing "Could not load" everywhere. + */ +export function setSessionExpiredHandler(handler: SessionExpiredHandler | null): void { + sessionExpiredHandler = handler +} + +function isAuthEndpoint(url: string | undefined): boolean { + return !!url && /^\/?auth\//.test(url.replace(/^\/api/, '')) +} + function attachRequestIdInterceptor(instance: AxiosInstance): void { instance.interceptors.request.use((config) => { const headers = config.headers ?? {} @@ -52,6 +68,10 @@ function attachRequestIdInterceptor(instance: AxiosInstance): void { if (axios.isAxiosError(error)) { const id = readRequestIdHeader(error.response?.headers) if (id) lastResponseRequestId = id + // Auth endpoints report their own 401s (wrong password, not logged in yet). + if (error.response?.status === 401 && !isAuthEndpoint(error.config?.url)) { + sessionExpiredHandler?.() + } } return Promise.reject(error) }, diff --git a/packages/web/src/lib/api/collections.ts b/packages/web/src/lib/api/collections.ts index 785a3d11..a8482df0 100644 --- a/packages/web/src/lib/api/collections.ts +++ b/packages/web/src/lib/api/collections.ts @@ -146,7 +146,7 @@ export const collectionsApi = { api.get<{ bag: BagResponse; contents: { sheets: Array } }>(`/collections/bags/${id}`), getSheet: (id: number) => api.get<{ sheet: SheetResponse; papers: PaperEntry[] }>(`/collections/sheets/${id}`), - check: (data: { collections: Array<{ identifier: string; type: 'micronix_plate' | 'cryovial_box' | 'box' | 'bag' | 'sheet' }> }) => + check: (data: { collections: Array<{ identifier: string; type: 'micronix_plate' | 'cryovial_box' | 'box' | 'bag' }> }) => api.post<{ results: Array<{ identifier: string; type: string; exists: boolean; id: number | null }> }>('/collections/check', data), /** Resolve collection by name and type; returns found + id/location when existing (for batch creation). */ resolve: (data: { name: string; type: 'box' | 'bag' | 'micronix_plate' | 'cryovial_box' }) => diff --git a/packages/web/src/lib/api/controls.ts b/packages/web/src/lib/api/controls.ts index ba6522fb..5ca103e9 100644 --- a/packages/web/src/lib/api/controls.ts +++ b/packages/web/src/lib/api/controls.ts @@ -122,6 +122,26 @@ export interface ControlDefinitionSummaryResponse { } } +export type CreateBatchWithSpecimensRequest = { + batch: { + controlDefinitionId: number + name: string + productionDate?: string + properties?: ControlBatchProperties + } + specimens: Array<{ + specimenTypeName: string + collectionDate?: string + containers: ControlBatchContainerWriteInput[] + }> +} + +export type CreateBatchWithSpecimensResponse = { + batch: ControlBatch + specimens: Array<{ id: number; specimenTypeName: string; containerCount: number; containerIds: number[] }> + createdCollections: Array<{ type: string; id: number; name: string }> +} + export const controlsApi = { list: (type?: string) => api.get<{ controls: ControlDefinition[] }>('/blood-controls', { params: { type } }), get: (id: number) => api.get<{ control: ControlDefinition }>(`/blood-controls/${id}`), @@ -143,19 +163,11 @@ export const controlsApi = { api.post<{ name: string }>('/blood-controls/batches/suggest-name', { definitionId, productionDate }), getBatch: (id: number) => api.get<{ batch: ControlBatch }>(`/blood-controls/batches/${id}`), getBatchSummary: (id: number) => api.get(`/blood-controls/batches/${id}/summary`), - createBatchWithSpecimens: (data: { - batch: { - controlDefinitionId: number - name: string - productionDate?: string - properties?: ControlBatchProperties - } - specimens: Array<{ - specimenTypeName: string - collectionDate?: string - containers: ControlBatchContainerWriteInput[] - }> - }) => api.post<{ batch: ControlBatch; specimens: Array<{ id: number; specimenTypeName: string; containerCount: number; containerIds: number[] }>; createdCollections: Array<{ type: string; id: number; name: string }> }>('/blood-controls/batches/create-with-specimens', data), + createBatchWithSpecimens: (data: CreateBatchWithSpecimensRequest) => + api.post('/blood-controls/batches/create-with-specimens', data), + /** Create several batches (one per density) all-or-nothing. */ + createBatchesWithSpecimens: (data: { batches: CreateBatchWithSpecimensRequest[] }) => + api.post<{ batches: CreateBatchWithSpecimensResponse[] }>('/blood-controls/batches/create-many-with-specimens', data), addSpecimensToBatch: (batchId: number, data: { specimens: Array<{ specimenTypeName: string diff --git a/packages/web/src/lib/api/export.ts b/packages/web/src/lib/api/export.ts index 971ef90b..84b6ad5e 100644 --- a/packages/web/src/lib/api/export.ts +++ b/packages/web/src/lib/api/export.ts @@ -1,4 +1,4 @@ -import type { ContainerExportData, CSVExportOptions, ExportFilters } from '@sampledb/contract' +import type { ContainerExportData, CSVExportOptions, ExportFilters, SubjectDateFilter } from '@sampledb/contract' import { api } from './client' import { downloadExportFile, @@ -107,7 +107,7 @@ export const exportApi = { containersByNames: (params: { study: string subject_names: string[] - subject_dates?: { [subjectName: string]: { exact?: string; from?: string; to?: string } } + subject_dates?: { [subjectName: string]: SubjectDateFilter | SubjectDateFilter[] } date_tolerance?: number format?: 'csv' | 'xlsx' | 'json' columns?: string[] @@ -138,7 +138,7 @@ export const exportApi = { containersCountByNames: (params: { study: string subject_names: string[] - subject_dates?: { [subjectName: string]: { exact?: string; from?: string; to?: string } } + subject_dates?: { [subjectName: string]: SubjectDateFilter | SubjectDateFilter[] } date_tolerance?: number specimen_type_ids?: number[] container_types?: string[] @@ -176,7 +176,7 @@ export const exportApi = { date_from?: string date_to?: string }> - subject_dates?: { [subjectName: string]: { exact?: string; from?: string; to?: string } } + subject_dates?: { [subjectName: string]: SubjectDateFilter | SubjectDateFilter[] } date_tolerance?: number format?: 'csv' | 'xlsx' | 'json' columns?: string[] diff --git a/packages/web/src/lib/base64.ts b/packages/web/src/lib/base64.ts new file mode 100644 index 00000000..000d0417 --- /dev/null +++ b/packages/web/src/lib/base64.ts @@ -0,0 +1,12 @@ +/** + * Base64-encode bytes in chunks. `String.fromCharCode(...bytes)` on a whole file passes every + * byte as a function argument and throws a RangeError once the file is a few hundred KB. + */ +export function bytesToBase64(bytes: Uint8Array): string { + const CHUNK = 0x8000 + let binary = '' + for (let i = 0; i < bytes.length; i += CHUNK) { + binary += String.fromCharCode(...bytes.subarray(i, i + CHUNK)) + } + return btoa(binary) +} diff --git a/packages/web/src/lib/bulk-import-validation.ts b/packages/web/src/lib/bulk-import-validation.ts index cb496e8c..1027be77 100644 --- a/packages/web/src/lib/bulk-import-validation.ts +++ b/packages/web/src/lib/bulk-import-validation.ts @@ -5,6 +5,7 @@ import type { ContainerType } from './container-types' import { getCollectionNameColumn } from './container-columns' import type { FlatBulkImportContainer } from './bulk-import-payload' +import { parseCsv } from '@sampledb/contract' export interface CSVRow { [key: string]: string @@ -81,15 +82,15 @@ function normalizeHeader(header: string): string { } export function parseBulkImportCSV(text: string): CSVRow[] { - const lines = text.split('\n').filter((line) => line.trim()) + // Shared RFC 4180 parser: quoted commas ("low vol, hemolyzed"), quotes, CRLF, and BOM. + const lines = parseCsv(text, { skipBlankRows: true }) if (lines.length < 2) return [] - const rawHeaders = lines[0].split(',').map((h) => h.trim()) - const headers = rawHeaders.map(normalizeHeader) + const headers = lines[0].map((h) => normalizeHeader(h.trim())) const rows: CSVRow[] = [] for (let i = 1; i < lines.length; i++) { - const values = lines[i].split(',') + const values = lines[i] const row: CSVRow = {} headers.forEach((header, j) => { row[header] = values[j]?.trim() ?? '' diff --git a/packages/web/src/lib/date-utils.ts b/packages/web/src/lib/date-utils.ts index 854947e0..5783f089 100644 --- a/packages/web/src/lib/date-utils.ts +++ b/packages/web/src/lib/date-utils.ts @@ -1,3 +1,26 @@ +/** + * Parse a date for display. A bare YYYY-MM-DD is a calendar date, so it is built in local + * time: new Date('2024-01-15') is UTC midnight, which renders as Jan 14 in the Americas. + */ +export function parseDisplayDate(value: string): Date { + const match = /^(\d{4})-(\d{2})-(\d{2})$/.exec(value) + if (match) return new Date(Number(match[1]), Number(match[2]) - 1, Number(match[3])) + return new Date(value) +} + +/** A Date as a local YYYY-MM-DD calendar date (toISOString() gives the UTC date). */ +export function toLocalDateString(date: Date): string { + const year = date.getFullYear() + const month = String(date.getMonth() + 1).padStart(2, '0') + const day = String(date.getDate()).padStart(2, '0') + return `${year}-${month}-${day}` +} + +/** Today's local calendar date as YYYY-MM-DD, for date input defaults. */ +export function localToday(): string { + return toLocalDateString(new Date()) +} + /** * Formats a date string with relative time for recent dates and absolute dates for older items * @param dateString - ISO date string @@ -6,7 +29,7 @@ export function formatDateWithRelativeTime(dateString: string | null | undefined): string { if (!dateString) return 'N/A' - const date = new Date(dateString) + const date = parseDisplayDate(dateString) const now = new Date() const diffMs = now.getTime() - date.getTime() const diffSeconds = Math.floor(diffMs / 1000) @@ -40,7 +63,7 @@ export function formatDateWithRelativeTime(dateString: string | null | undefined export function formatDate(dateString: string | null | undefined): string { if (!dateString) return 'N/A' - const date = new Date(dateString) + const date = parseDisplayDate(dateString) return date.toLocaleDateString('en-US', { month: 'short', day: 'numeric', diff --git a/packages/web/src/lib/scanner-plate-csv.ts b/packages/web/src/lib/scanner-plate-csv.ts index f6dcefa5..206ee904 100644 --- a/packages/web/src/lib/scanner-plate-csv.ts +++ b/packages/web/src/lib/scanner-plate-csv.ts @@ -1,5 +1,6 @@ import type { ScannerConfiguration } from './api/settings' import { normalizeWellPosition, validateFullPlatePositions } from './micronix-plate-positions' +import { parseCsv } from '@sampledb/contract' export type ScannerPlateCsvRow = Record @@ -17,15 +18,14 @@ function buildPosition(config: ScannerConfiguration, row: ScannerPlateCsvRow): s * Parse a scanner plate CSV using column mapping from scanner configuration. */ export function parseScannerPlateCsv(text: string, config: ScannerConfiguration): ScannerPlateCsvRow[] { - const lines = text.split('\n').filter((line) => line.trim()) + const lines = parseCsv(text, { skipBlankRows: true }) if (lines.length < 2 + config.skipRows) return [] - const headerLine = lines[config.skipRows] - const headers = headerLine.split(',').map((h) => h.trim()) + const headers = lines[config.skipRows].map((h) => h.trim()) const rows: ScannerPlateCsvRow[] = [] for (let i = config.skipRows + 1; i < lines.length; i++) { - const values = lines[i].split(',') + const values = lines[i] const row: ScannerPlateCsvRow = {} headers.forEach((header, j) => { row[header] = values[j]?.trim() || '' diff --git a/packages/web/src/lib/template-generator.ts b/packages/web/src/lib/template-generator.ts index 4ba5e527..de6b8669 100644 --- a/packages/web/src/lib/template-generator.ts +++ b/packages/web/src/lib/template-generator.ts @@ -1,6 +1,7 @@ import type { BulkDerivationSettings } from './api/derivations' import { buildCsv } from './csv' import { getCollectionNameColumn } from './container-columns' +import { localToday } from './date-utils' export interface TemplateOptions { parentType: 'barcode' | 'control_batch' | 'study_subject' | 'cryovial_position' @@ -83,7 +84,7 @@ export function generateDerivationsTemplate(options: TemplateOptions): string { if (parentColumns.includes('specimen_type_name')) defaultDerivationPerRow.specimen_type_name = derivedExampleSpecimenType if (parentColumns.includes('container_type')) defaultDerivationPerRow.container_type = settings.containerType || 'micronix_tube' if (parentColumns.includes('protocol')) defaultDerivationPerRow.protocol = exampleProtocolVal - if (parentColumns.includes('derivation_date')) defaultDerivationPerRow.derivation_date = new Date().toISOString().split('T')[0] + if (parentColumns.includes('derivation_date')) defaultDerivationPerRow.derivation_date = localToday() if (parentColumns.includes('quantity')) defaultDerivationPerRow.quantity = '1' if (parentColumns.includes('quantity_used')) defaultDerivationPerRow.quantity_used = '1' if (parentColumns.includes('reduce_parent_quantity')) defaultDerivationPerRow.reduce_parent_quantity = 'true' diff --git a/packages/web/src/pages/BloodControls.tsx b/packages/web/src/pages/BloodControls.tsx index b3bbe556..c4794d16 100644 --- a/packages/web/src/pages/BloodControls.tsx +++ b/packages/web/src/pages/BloodControls.tsx @@ -11,6 +11,7 @@ import { getContainerTypeIcon } from '../lib/icons' import { getCompositionKey } from '../lib/composition-key' import { DetailPageSkeleton, PageError, fromQuery, getQueryErrorMessage } from '../ui' import '../styles/blood-controls.css' +import { parseDisplayDate } from '../lib/date-utils' export interface CompositionGroup { id: number @@ -446,7 +447,7 @@ export default function BloodControls() { label: 'Production Date', sortable: true, render: (value) => value ? ( - {new Date(value).toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' })} + {parseDisplayDate(value).toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' })} ) : Not set, }, ] diff --git a/packages/web/src/pages/ContainerDetail.tsx b/packages/web/src/pages/ContainerDetail.tsx index 6ac9ab3c..f634cb20 100644 --- a/packages/web/src/pages/ContainerDetail.tsx +++ b/packages/web/src/pages/ContainerDetail.tsx @@ -23,6 +23,7 @@ import { hasContainerDisplayIdentifier, } from '../lib/container-display' import '../styles/storage.css' +import { parseDisplayDate } from '../lib/date-utils' function statusColor(name: string): string { const key = name.toLowerCase() @@ -375,7 +376,7 @@ export default function ContainerDetail() {

Collection date
- {new Date(specimen.collectionDate).toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: 'numeric' })} + {parseDisplayDate(specimen.collectionDate).toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: 'numeric' })}
)} @@ -494,7 +495,7 @@ export default function ContainerDetail() {
Derivation Date
- {new Date(sourceDerivation.derivation.derivationDate).toLocaleDateString('en-US', { + {parseDisplayDate(sourceDerivation.derivation.derivationDate).toLocaleDateString('en-US', { year: 'numeric', month: 'long', day: 'numeric' @@ -622,7 +623,7 @@ export default function ContainerDetail() {
Derived: - {new Date(derivation.derivationDate).toLocaleDateString('en-US', { + {parseDisplayDate(derivation.derivationDate).toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' diff --git a/packages/web/src/pages/ControlBatchDetail.tsx b/packages/web/src/pages/ControlBatchDetail.tsx index d8f884c4..5c54173c 100644 --- a/packages/web/src/pages/ControlBatchDetail.tsx +++ b/packages/web/src/pages/ControlBatchDetail.tsx @@ -13,6 +13,7 @@ import { Modal } from '../ui' import { useUser } from '../contexts/UserContext' import { DetailPageSkeleton, PageError, fromQuery, getQueryErrorMessage } from '../ui' import '../styles/blood-controls.css' +import { parseDisplayDate } from '../lib/date-utils' export default function ControlBatchDetail() { const { id } = useParams<{ id: string }>() @@ -261,7 +262,7 @@ export default function ControlBatchDetail() { )} {batch.productionDate && (

- Production Date: {new Date(batch.productionDate).toLocaleDateString(undefined, { year: 'numeric', month: 'long', day: 'numeric' })} + Production Date: {parseDisplayDate(batch.productionDate).toLocaleDateString(undefined, { year: 'numeric', month: 'long', day: 'numeric' })}

)} diff --git a/packages/web/src/pages/ControlBatchWizard.tsx b/packages/web/src/pages/ControlBatchWizard.tsx index 97198282..dfd475fc 100644 --- a/packages/web/src/pages/ControlBatchWizard.tsx +++ b/packages/web/src/pages/ControlBatchWizard.tsx @@ -17,6 +17,7 @@ import { import { useUser } from '../contexts/UserContext' import { PageError, fromQuery, getQueryErrorMessage } from '../ui' import '../styles/blood-controls.css' +import { localToday } from '../lib/date-utils' /** Strains for composition (used for multi-batch CSV flow). */ export type CompositionStrains = Array<{ id: number; percentage?: number }> @@ -136,7 +137,7 @@ export default function ControlBatchWizard() { controlDefinitionId: null, controlDefinition: null, name: '', - productionDate: new Date().toISOString().split('T')[0], + productionDate: localToday(), }) const [compositionStrains, setCompositionStrains] = useState(null) @@ -165,7 +166,7 @@ export default function ControlBatchWizard() { controlDefinitionId: data.batch.controlDefinitionId, controlDefinition: data.controlDefinition, name: data.batch.name, - productionDate: data.batch.productionDate || new Date().toISOString().split('T')[0], + productionDate: data.batch.productionDate || localToday(), properties: data.batch.properties, }) }, [batchBootstrapQuery.data]) diff --git a/packages/web/src/pages/ControlDefinitionDetail.tsx b/packages/web/src/pages/ControlDefinitionDetail.tsx index a19232c1..1fa9d30b 100644 --- a/packages/web/src/pages/ControlDefinitionDetail.tsx +++ b/packages/web/src/pages/ControlDefinitionDetail.tsx @@ -7,6 +7,7 @@ import { getContainerTypeIcon } from '../lib/icons' import { useControlDefinitionSummary } from '../hooks/useControls' import { DetailPageSkeleton, PageError, fromQuery, getQueryErrorMessage } from '../ui' import '../styles/blood-controls.css' +import { parseDisplayDate } from '../lib/date-utils' export default function ControlDefinitionDetail() { const { id } = useParams<{ id: string }>() @@ -72,7 +73,7 @@ export default function ControlDefinitionDetail() { label: 'Production Date', sortable: true, render: (value) => value ? ( - {new Date(value).toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' })} + {parseDisplayDate(value).toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' })} ) : Not set, }, { diff --git a/packages/web/src/pages/Locations.tsx b/packages/web/src/pages/Locations.tsx index 9354ebc5..64da1ed6 100644 --- a/packages/web/src/pages/Locations.tsx +++ b/packages/web/src/pages/Locations.tsx @@ -169,7 +169,12 @@ export default function Locations() { useClickOutside(searchRef, () => setIsSearchOpen(false), isSearchOpen) + // Latest search text; responses for older text are dropped so a slow "PL-1" reply + // cannot overwrite "PL-12" results or reopen the dropdown after the box is cleared. + const latestSearchRef = useRef('') + useEffect(() => { + latestSearchRef.current = search if (search.length >= 1) { const timeoutId = setTimeout(() => { performCollectionSearch(search) @@ -179,6 +184,7 @@ export default function Locations() { } else { setCollectionResults([]) setIsSearchOpen(false) + setSearchLoading(false) } }, [search]) @@ -186,6 +192,7 @@ export default function Locations() { try { setSearchLoading(true) const response = await searchApi.search(searchQuery, 'collection') + if (searchQuery !== latestSearchRef.current) return // Filter to only collection types (micronix_plate, cryovial_box, box, bag) const collectionResults = response.results.filter( (r): r is CollectionSearchResult => @@ -194,10 +201,11 @@ export default function Locations() { setCollectionResults(collectionResults) setIsSearchOpen(true) } catch (error) { + if (searchQuery !== latestSearchRef.current) return console.error('Collection search failed:', error) setCollectionResults([]) } finally { - setSearchLoading(false) + if (searchQuery === latestSearchRef.current) setSearchLoading(false) } } diff --git a/packages/web/src/pages/QpcrExperimentDetail.tsx b/packages/web/src/pages/QpcrExperimentDetail.tsx index d538a8cb..665c5437 100644 --- a/packages/web/src/pages/QpcrExperimentDetail.tsx +++ b/packages/web/src/pages/QpcrExperimentDetail.tsx @@ -17,6 +17,7 @@ import { getContainerTypeIcon, getContainerTypeName, getSpecimenTypeIcon } from import { useUser } from '../contexts/UserContext' import { useToast } from '../contexts/ToastContext' import '../styles/qpcr.css' +import { bytesToBase64 } from '../lib/base64' const PLATE_ROWS = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H'] as const const PLATE_COLS = 12 @@ -204,24 +205,33 @@ export default function QpcrExperimentDetail() { const well = data.wells.find((w) => w.wellPosition === selectedWellPosition) const containerId = well?.storageContainerId setWellDetailsError(null) + // Ignore a response that arrives after the user picked another well. + let ignore = false if (containerId != null) { setWellDetailsLoading(true) setWellDetails(null) containersApi .get(containerId) .then((res) => { + if (ignore) return setWellDetails(res) setWellDetailsError(null) }) .catch((err: { response?: { data?: { error?: string } } }) => { + if (ignore) return setWellDetailsError(err.response?.data?.error ?? 'Failed to load container details') setWellDetails(null) }) - .finally(() => setWellDetailsLoading(false)) + .finally(() => { + if (!ignore) setWellDetailsLoading(false) + }) } else { setWellDetails(null) setWellDetailsLoading(false) } + return () => { + ignore = true + } }, [selectedWellPosition, data?.wells]) if (!id || Number.isNaN(experimentId)) { @@ -426,12 +436,18 @@ export default function QpcrExperimentDetail() { return w != null && w.contentType === 'negative' }) + /** Show the server's wells and refresh the cached experiment, or revisiting the page shows the old wells. */ + const applyUpdatedWells = (wells: QpcrExperimentWell[]) => { + setData((prev) => (prev ? { ...prev, wells } : null)) + invalidateQpcrExperimentQueries(queryClient) + } + const handleSetWellContentType = async (contentType: 'empty' | 'negative') => { if (!id || !selectedWellPosition) return setWellsUpdating(true) try { const res = await qpcrExperimentsApi.updateWells(parseInt(id), { wellPosition: selectedWellPosition, contentType }) - setData((prev) => (prev ? { ...prev, wells: res.wells } : null)) + applyUpdatedWells(res.wells) showSuccess(contentType === 'negative' ? 'Well set as NTC' : 'Well set as empty') } catch (err: unknown) { const msg = err && typeof err === 'object' && 'response' in err && typeof (err as { response?: { data?: { error?: string } } }).response?.data?.error === 'string' @@ -448,7 +464,7 @@ export default function QpcrExperimentDetail() { setWellsUpdating(true) try { const res = await qpcrExperimentsApi.updateWells(parseInt(id), { positions: emptyWellPositions, contentType: 'negative' }) - setData((prev) => (prev ? { ...prev, wells: res.wells } : null)) + applyUpdatedWells(res.wells) showSuccess(`Set ${emptyWellPositions.length} well(s) to NTC`) } catch (err: unknown) { const msg = err && typeof err === 'object' && 'response' in err && typeof (err as { response?: { data?: { error?: string } } }).response?.data?.error === 'string' @@ -465,7 +481,7 @@ export default function QpcrExperimentDetail() { setWellsUpdating(true) try { const res = await qpcrExperimentsApi.updateWells(parseInt(id), { positions: ntcWellPositions, contentType: 'empty' }) - setData((prev) => (prev ? { ...prev, wells: res.wells } : null)) + applyUpdatedWells(res.wells) showSuccess(`Set ${ntcWellPositions.length} well(s) to empty`) } catch (err: unknown) { const msg = err && typeof err === 'object' && 'response' in err && typeof (err as { response?: { data?: { error?: string } } }).response?.data?.error === 'string' @@ -963,7 +979,7 @@ export default function QpcrExperimentDetail() { setResultsUploading(true) try { const buf = await file.arrayBuffer() - const base64 = btoa(String.fromCharCode(...new Uint8Array(buf))) + const base64 = bytesToBase64(new Uint8Array(buf)) const resultInstrument = (instrumentSelectRef.current?.value as 'Biorad_CFX' | 'QuantStudio' | undefined) ?? 'Biorad_CFX' await qpcrExperimentsApi.uploadResults(parseInt(id), { fileContent: base64, fileName: file.name, instrumentType: resultInstrument }) invalidateQpcrExperimentQueries(queryClient) diff --git a/packages/web/src/pages/QpcrExperiments.tsx b/packages/web/src/pages/QpcrExperiments.tsx index 1b38ecd7..ca2429a8 100644 --- a/packages/web/src/pages/QpcrExperiments.tsx +++ b/packages/web/src/pages/QpcrExperiments.tsx @@ -8,6 +8,7 @@ import { useUser } from '../contexts/UserContext' import { useQpcrExperimentsList } from '../hooks/useQpcrExperiments' import { AsyncPresentation, EmptyState, PageError, fromQuery, getQueryErrorMessage } from '../ui' import '../styles/qpcr.css' +import { parseDisplayDate } from '../lib/date-utils' const TEMPLATE_LABELS: Record = { biorad: 'Bio-Rad CFX 96', @@ -43,7 +44,7 @@ function getStatusLabel(status: string): string { function formatShortDate(iso: string | null | undefined): string { if (!iso) return '—' try { - const d = new Date(iso) + const d = parseDisplayDate(iso) if (Number.isNaN(d.getTime())) return '—' return d.toLocaleDateString(undefined, { year: 'numeric', month: 'short', day: 'numeric' }) } catch { diff --git a/packages/web/src/pages/Settings.tsx b/packages/web/src/pages/Settings.tsx index e481abb7..fe255116 100644 --- a/packages/web/src/pages/Settings.tsx +++ b/packages/web/src/pages/Settings.tsx @@ -3,7 +3,7 @@ import { useSearchParams } from 'react-router-dom' import { useQueryClient } from '@tanstack/react-query' import type { AllSettings } from '../lib/api/settings' import { useUser } from '../contexts/UserContext' -import { settingsKeys, useAllSettings } from '../hooks/useSettings' +import { useAllSettings } from '../hooks/useSettings' import { PageError, fromQuery, getQueryErrorMessage } from '../ui' import { useTheme, THEME_IDS, THEME_LABELS } from '../contexts/ThemeContext' import InfoTooltip from '../components/InfoTooltip' @@ -246,8 +246,10 @@ export default function Settings() { const [mutationError, setMutationError] = useState(null) const [success, setSuccess] = useState(null) + // Settings feed many other queries (export and scanner configurations, move page + // bootstraps, container defaults, table views), so a save marks every query stale. const refreshSettings = () => { - void queryClient.invalidateQueries({ queryKey: settingsKeys.allSettings() }) + void queryClient.invalidateQueries() } const handleSave = async () => { diff --git a/packages/web/src/pages/SpecimenDetail.tsx b/packages/web/src/pages/SpecimenDetail.tsx index 40d7ebca..ea3a1838 100644 --- a/packages/web/src/pages/SpecimenDetail.tsx +++ b/packages/web/src/pages/SpecimenDetail.tsx @@ -21,6 +21,7 @@ import { getQueryErrorMessage, } from '../ui' import '../styles/subject-specimen.css' +import { parseDisplayDate } from '../lib/date-utils' interface Container { id: number @@ -96,7 +97,8 @@ export default function SpecimenDetail() { } navigate('/specimens') }, - { preventDefault: true } + // Not while the Add Container modal is open (focus may be on a button inside it). + { preventDefault: true, enabled: !addContainerModalOpen } ) useModifierHotkey( @@ -219,7 +221,7 @@ export default function SpecimenDetail() {
Collection Date
- {new Date(specimen.collectionDate).toLocaleDateString()} + {parseDisplayDate(specimen.collectionDate).toLocaleDateString()}
)} diff --git a/packages/web/src/pages/Specimens.tsx b/packages/web/src/pages/Specimens.tsx index aa93a411..efbc8d95 100644 --- a/packages/web/src/pages/Specimens.tsx +++ b/packages/web/src/pages/Specimens.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react' +import { useMemo, useState, useEffect } from 'react' import DataTable, { Column } from '../components/DataTable' import { Link, useSearchParams, useNavigate } from 'react-router-dom' import SpecimenFilter, { type SpecimenFilters } from '../components/SpecimenFilter' @@ -15,6 +15,7 @@ import { getQueryErrorMessage, } from '../ui' import '../styles/subject-specimen.css' +import { parseDisplayDate } from '../lib/date-utils' type SpecimenListRow = Specimen & { study?: { shortCode?: string } @@ -55,7 +56,12 @@ export default function Specimens() { const [page, setPage] = useState(1) const pageSize = 50 - const [filters, setFilters] = useState(() => filtersFromSearchParams(searchParams)) + // The URL is the source of truth, so "Clear Filters" and back/forward update the table. + const filters = useMemo(() => filtersFromSearchParams(searchParams), [searchParams]) + // Back to page 1 whenever the filters change, including from outside (Clear Filters). + useEffect(() => { + setPage(1) + }, [searchParams]) const listParams = useMemo(() => filtersToParams(filters), [filters]) const specimensQuery = useSpecimens(listParams) @@ -65,8 +71,7 @@ export default function Specimens() { isEmpty: specimensQuery.isSuccess && specimens.length === 0, }) - const handleFilterChange = (newFilters: SpecimenFilters) => { - setFilters(newFilters) + const handleFilterChange = (newFilters: SpecimenFilters, options?: { replace?: boolean }) => { setPage(1) const params: Record = {} @@ -79,12 +84,12 @@ export default function Specimens() { if (newFilters.createdTo) params.created_to = newFilters.createdTo if (newFilters.search) params.search = newFilters.search - setSearchParams(params) + setSearchParams(params, { replace: options?.replace }) } const formatDate = (dateString?: string) => { if (!dateString) return '—' - return new Date(dateString).toLocaleDateString() + return parseDisplayDate(dateString).toLocaleDateString() } const columns: Column[] = [ @@ -172,7 +177,8 @@ export default function Specimens() {
handleFilterChange(next, { replace: true })} onSubmit={handleFilterChange} isLoading={specimensQuery.isPending} /> diff --git a/packages/web/src/pages/Statistics.tsx b/packages/web/src/pages/Statistics.tsx index f4608750..dff8ca35 100644 --- a/packages/web/src/pages/Statistics.tsx +++ b/packages/web/src/pages/Statistics.tsx @@ -8,6 +8,7 @@ import { SkeletonCard } from '../ui' import { useStatistics } from '../hooks/useStatistics' import { PageError, fromQuery, getQueryErrorMessage } from '../ui' import '../styles/statistics.css' +import { toLocalDateString } from '../lib/date-utils' type BinSize = 'day' | 'week' | 'month' | 'quarter' | 'year' @@ -25,7 +26,7 @@ const TIME_PRESETS = [ function formatDateOffset(daysAgo: number): string { const d = new Date() d.setDate(d.getDate() - daysAgo) - return d.toISOString().slice(0, 10) + return toLocalDateString(d) } /** Build human-readable filter chip labels from applied filters. */ diff --git a/packages/web/src/pages/Studies.tsx b/packages/web/src/pages/Studies.tsx index f0447a86..96fbda21 100644 --- a/packages/web/src/pages/Studies.tsx +++ b/packages/web/src/pages/Studies.tsx @@ -33,19 +33,58 @@ interface StudyWithSummary extends Study { const PAGE_LIMIT = 50 +const SORT_OPTIONS: SortOption[] = ['title', 'date', 'subjects', 'specimens', 'containers', 'lead'] + +/** Names sort A-Z by default; dates and counts sort largest first. */ +function defaultSortDirection(sortBy: SortOption): 'asc' | 'desc' { + return sortBy === 'title' || sortBy === 'lead' ? 'asc' : 'desc' +} + export default function Studies() { const [searchParams, setSearchParams] = useSearchParams() const { canWrite } = useUser() const showDeletedMessage = searchParams.get('deleted') === '1' - const [search, setSearch] = useState('') + // Search, sort, and filters live in the URL so links, back/forward, and the + // "Clear Filters" command all work. Defaults are left out of the URL. + const search = searchParams.get('q') ?? '' + const sortParam = searchParams.get('sort') + const sortBy: SortOption = SORT_OPTIONS.includes(sortParam as SortOption) ? (sortParam as SortOption) : 'date' + const sortDirection: 'asc' | 'desc' = + searchParams.get('dir') === 'asc' || searchParams.get('dir') === 'desc' + ? (searchParams.get('dir') as 'asc' | 'desc') + : defaultSortDirection(sortBy) + const typeParam = searchParams.get('type') + const filterType: FilterType = typeParam === 'longitudinal' || typeParam === 'cross-sectional' ? typeParam : 'all' + const filterLead = searchParams.get('lead') ?? '' + + /** + * Apply several URL changes in one update ('' or null removes a key). Each change is a + * history entry so Back undoes it; pass replace for per-keystroke changes. + */ + const updateListParams = useCallback( + (changes: Record, options?: { replace?: boolean }) => { + setSearchParams( + (prev) => { + const next = new URLSearchParams(prev) + for (const [key, value] of Object.entries(changes)) { + if (value == null || value === '') next.delete(key) + else next.set(key, value) + } + return next + }, + { replace: options?.replace ?? false }, + ) + }, + [setSearchParams], + ) + const setSearch = (value: string) => updateListParams({ q: value }, { replace: true }) + const setFilterType = (value: FilterType) => updateListParams({ type: value === 'all' ? null : value }) + const setFilterLead = (value: string) => updateListParams({ lead: value }) + const [viewMode, setViewMode] = useState(() => { const saved = localStorage.getItem('studies-view-mode') return saved === 'grid' || saved === 'list' ? saved : 'grid' }) - const [sortBy, setSortBy] = useState('date') - const [sortDirection, setSortDirection] = useState<'asc' | 'desc'>('desc') - const [filterType, setFilterType] = useState('all') - const [filterLead, setFilterLead] = useState('') const { prefetch: prefetchSummaries, getCardState } = useStudySummaryCards() const summaryObserverRef = useRef(null) const loadMoreRef = useRef(null) @@ -399,14 +438,8 @@ export default function Studies() { value={sortBy} onChange={(e) => { const newSort = e.target.value as SortOption - setSortBy(newSort) - if (newSort === 'date') { - setSortDirection('desc') - } else if (newSort === 'title' || newSort === 'lead') { - setSortDirection('asc') - } else { - setSortDirection('desc') - } + // New sort resets to its natural direction (dir omitted = default). + updateListParams({ sort: newSort === 'date' ? null : newSort, dir: null }) }} className="form-select text-sm rounded-lg border px-3 py-2" style={{ borderColor: 'rgb(var(--app-border))' }} @@ -420,7 +453,10 @@ export default function Studies() { +} + describe('Studies', () => { beforeEach(() => { vi.clearAllMocks() @@ -45,4 +51,35 @@ describe('Studies', () => { expect(screen.getByText('No studies found')).toBeInTheDocument() }) }) + + it('reads search and filters from the URL and clears them', async () => { + const { default: Studies } = await import('../Studies') + await render(, { initialEntries: ['/studies?q=malaria&type=longitudinal'] }) + + const searchBox = await screen.findByPlaceholderText(/Search by title/) + expect(searchBox).toHaveValue('malaria') + expect(screen.getByDisplayValue(/longitudinal/i)).toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: /clear filters/i })) + + await waitFor(() => expect(searchBox).toHaveValue('')) + }) + + it('undoes a filter change with Back', async () => { + const { default: Studies } = await import('../Studies') + await render( + <> + + + , + { initialEntries: ['/studies'] }, + ) + const typeFilter = await screen.findByDisplayValue(/all types/i) + + fireEvent.change(typeFilter, { target: { value: 'longitudinal' } }) + await waitFor(() => expect(typeFilter).toHaveValue('longitudinal')) + + fireEvent.click(screen.getByRole('button', { name: 'Back' })) + await waitFor(() => expect(typeFilter).toHaveValue('all')) + }) }) diff --git a/packages/web/vitest.config.ts b/packages/web/vitest.config.ts index 07f61e4b..d703b145 100644 --- a/packages/web/vitest.config.ts +++ b/packages/web/vitest.config.ts @@ -4,6 +4,10 @@ import path from 'path' const appBuildId = process.env.VITE_APP_BUILD_ID || process.env.APP_BUILD_ID || 'local-dev' +// Run tests west of UTC so a calendar date parsed as UTC midnight shows the day before and +// fails, wherever CI runs. Test workers inherit this. +process.env.TZ = 'America/Los_Angeles' + /** * Vitest Configuration *