From fd9006555a0784bddf4735de1592849b999cf905 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sat, 1 Aug 2026 15:18:33 -0600 Subject: [PATCH 1/8] Simplify location hierarchy by removing floors from room keys Rooms are now keyed by building and name instead of floor, buildings drop the area suffix, and the derived cage location key is persisted via a managed column. Cagemates now matches on room as well as cage so group-pen animals, which have no cage, resolve correctly. --- .../queries/ehr_lookups/buildings.js | 2 +- .../resources/queries/ehr_lookups/cage.js | 9 +++ .../queries/ehr_lookups/cage.query.xml | 5 ++ .../resources/queries/ehr_lookups/rooms.js | 7 +- .../queries/ehr_lookups/rooms.query.xml | 2 +- .../queries/study/demographicsCagemates.sql | 9 ++- .../resources/queries/study/housing.query.xml | 2 +- .../nbri_ehr/table/NBRI_EHRCustomizer.java | 64 ------------------- 8 files changed, 25 insertions(+), 75 deletions(-) diff --git a/nbri_ehr/resources/queries/ehr_lookups/buildings.js b/nbri_ehr/resources/queries/ehr_lookups/buildings.js index 3fca085..dd9066f 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/buildings.js +++ b/nbri_ehr/resources/queries/ehr_lookups/buildings.js @@ -25,7 +25,7 @@ function onUpsert(row, oldRow, errors){ return; } - row.name = row.description + '-' + row.area; + row.name = row.description; } } } diff --git a/nbri_ehr/resources/queries/ehr_lookups/cage.js b/nbri_ehr/resources/queries/ehr_lookups/cage.js index e19a2b7..d374e86 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/cage.js +++ b/nbri_ehr/resources/queries/ehr_lookups/cage.js @@ -7,6 +7,15 @@ var LABKEY = require("labkey"); var triggerHelper = new org.labkey.nbri_ehr.query.NBRI_EHRTriggerHelper(LABKEY.Security.currentUser.id, LABKEY.Security.currentContainer.id); +// 'location' is not user editable, so it is absent from the incoming row map and the value this script +// derives has nowhere to land. Declaring it managed reserves a slot so the derived key is persisted. +function managedColumns() { + return { + insert: ["location"], + update: ["location"], + }; +} + function onUpsert(row, oldRow, errors){ if (extraContext.dataSource != "etl") { if (!row.location) { diff --git a/nbri_ehr/resources/queries/ehr_lookups/cage.query.xml b/nbri_ehr/resources/queries/ehr_lookups/cage.query.xml index 97b1549..3d15c46 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/cage.query.xml +++ b/nbri_ehr/resources/queries/ehr_lookups/cage.query.xml @@ -9,6 +9,11 @@ Room + + ehr_lookups + rooms + room + true diff --git a/nbri_ehr/resources/queries/ehr_lookups/rooms.js b/nbri_ehr/resources/queries/ehr_lookups/rooms.js index caea28b..9a03aad 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/rooms.js +++ b/nbri_ehr/resources/queries/ehr_lookups/rooms.js @@ -15,18 +15,13 @@ function onUpsert(row, oldRow, errors){ return; } - if (!row.floor) { - errors['floor'] = 'Floor is required.'; - return; - } - if (!row.room) { if (oldRow && oldRow.room && oldRow.room[0]) { row.room = oldRow.room[0]; return; } - row.room = row.name + '-' + row.floor; + row.room = row.building + '-' + row.name; } } } diff --git a/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml b/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml index dd7b89f..3672017 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml +++ b/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml @@ -6,7 +6,7 @@ Rooms - + true false diff --git a/nbri_ehr/resources/queries/study/demographicsCagemates.sql b/nbri_ehr/resources/queries/study/demographicsCagemates.sql index b3464b7..c8e2529 100644 --- a/nbri_ehr/resources/queries/study/demographicsCagemates.sql +++ b/nbri_ehr/resources/queries/study/demographicsCagemates.sql @@ -22,10 +22,15 @@ SELECT FROM study.housing h JOIN study.housing h2 -ON (h2.Id.demographics.calculated_status = 'Alive' - AND (h.cage = h2.cage)) +-- cage is null for group/pen rooms, whose location is the room alone, so the room match bounds the null case +ON ((h.cage = h2.cage OR (h.cage IS NULL AND h2.cage IS NULL)) + AND h.room = h2.room + AND h2.Id.demographics.calculated_status = 'Alive' + AND h2.enddateTimeCoalesced >= now() + AND h2.qcstate.publicdata = true) WHERE h.enddateTimeCoalesced >= now() +AND h.qcstate.publicdata = true GROUP BY h.id, h.room, h.cage ) t ON (t.id = d.id) diff --git a/nbri_ehr/resources/queries/study/housing.query.xml b/nbri_ehr/resources/queries/study/housing.query.xml index eeb5e0d..f1473f6 100644 --- a/nbri_ehr/resources/queries/study/housing.query.xml +++ b/nbri_ehr/resources/queries/study/housing.query.xml @@ -21,7 +21,7 @@ ehr_lookups cage location - cage + diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java b/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java index 38fd24d..7ed4cf2 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java @@ -93,16 +93,6 @@ public void customize(TableInfo table) customizeTasks(ti); } - if (matches(ti, "ehr_lookups", "rooms")) - { - customizeRooms(ti); - } - - if (matches(ti, "ehr_lookups", "floors")) - { - customizeFloors(ti); - } - if (matches(ti, "nbri_ehr", "necropsyTasks")) { addNecropsyReportLink(ti); @@ -581,60 +571,6 @@ public boolean isEditable() } } - private void customizeRooms(AbstractTableInfo ti) - { - ColumnInfo roomCol = ti.getColumn("name"); - ColumnInfo floorCol = ti.getColumn("floor"); - if (roomCol != null && floorCol != null && ti.getColumn("fullRoom") == null) - { - ExprColumn col = new ExprColumn(ti, new FieldKey(null, "fullRoom"), new SQLFragment("##ERROR"), JdbcType.VARCHAR, roomCol, floorCol) { - @Override - public SQLFragment getValueSql(String tableAlias) - { - // Need to subclass and override this function due to issue using ExprColumn.STR_TABLE_ALIAS with extensible columns - SQLFragment sql = new SQLFragment("(SELECT COALESCE(r.room, 'Room N/A') || ', ' || COALESCE(r.floor, 'Floor N/A') || ', ' || COALESCE(r.building, 'Building N/A') \n" + - " FROM (\n" + - " SELECT \n").append(roomCol.getValueSql(tableAlias)); - sql.append(" AS room,\n" + - " ff.name as floor,\n" + - " bb.description as building\n" + - " FROM ehr_lookups.floors ff \n" + - " JOIN ehr_lookups.buildings bb ON ff.building = bb.name\n" + - " WHERE ff.floor =\n").append(floorCol.getValueSql(tableAlias)); - sql.append(" ) r\n" + - " )"); - - return sql; - } - }; - col.setName("fullRoom"); - col.setLabel("Full Room"); - ti.addColumn(col); - } - } - - private void customizeFloors(AbstractTableInfo ti) - { - ColumnInfo floorCol = ti.getColumn("name"); - ColumnInfo bldgCol = ti.getColumn("building"); - if (floorCol != null && bldgCol != null && ti.getColumn("fullFloor") == null) - { - SQLFragment sql = new SQLFragment("(SELECT COALESCE(r.floor, 'Floor N/A') || ', ' || COALESCE(r.building, 'Building N/A') \n" + - " FROM (\n" + - " SELECT " + ExprColumn.STR_TABLE_ALIAS + ".name\n"); - sql.append(" AS floor,\n" + - " bb.description as building\n" + - " FROM ehr_lookups.buildings bb \n" + - " WHERE bb.name = " + ExprColumn.STR_TABLE_ALIAS + ".building\n"); - sql.append(" ) r\n" + - " )"); - - ExprColumn col = new ExprColumn(ti, "fullFloor", sql, JdbcType.VARCHAR, floorCol, bldgCol); - col.setLabel("Full Floor"); - ti.addColumn(col); - } - } - private void customizeTasks(AbstractTableInfo ti) { DetailsURL detailsURL = DetailsURL.fromString("/ehr/dataEntryFormDetails.view?formType=${formtype}&taskid=${taskid}"); From bcbbec69d49f416fdfc55f4650b0e66d901305a5 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sat, 1 Aug 2026 15:21:36 -0600 Subject: [PATCH 2/8] Drop commented-out cage display column from housing metadata --- nbri_ehr/resources/queries/study/housing.query.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/nbri_ehr/resources/queries/study/housing.query.xml b/nbri_ehr/resources/queries/study/housing.query.xml index f1473f6..de3337b 100644 --- a/nbri_ehr/resources/queries/study/housing.query.xml +++ b/nbri_ehr/resources/queries/study/housing.query.xml @@ -21,7 +21,6 @@ ehr_lookups cage location - From 18170c89b6b4150d5104215f878ddc1aa7acf233 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sat, 1 Aug 2026 15:32:41 -0600 Subject: [PATCH 3/8] Remove the historicalOther dataset and its report --- .../queries/study/historicalOther.query.xml | 13 ---------- .../study/datasets/datasets_manifest.xml | 1 - .../study/datasets/datasets_metadata.xml | 24 ------------------- .../resources/reports/additionalReports.tsv | 1 - 4 files changed, 39 deletions(-) delete mode 100644 nbri_ehr/resources/queries/study/historicalOther.query.xml diff --git a/nbri_ehr/resources/queries/study/historicalOther.query.xml b/nbri_ehr/resources/queries/study/historicalOther.query.xml deleted file mode 100644 index 37e2754..0000000 --- a/nbri_ehr/resources/queries/study/historicalOther.query.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - Type - - -
- - - \ No newline at end of file diff --git a/nbri_ehr/resources/referenceStudy/study/datasets/datasets_manifest.xml b/nbri_ehr/resources/referenceStudy/study/datasets/datasets_manifest.xml index 1d03f01..a4cd2a4 100644 --- a/nbri_ehr/resources/referenceStudy/study/datasets/datasets_manifest.xml +++ b/nbri_ehr/resources/referenceStudy/study/datasets/datasets_manifest.xml @@ -20,7 +20,6 @@ - diff --git a/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml b/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml index 42317f7..47283a3 100644 --- a/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml +++ b/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml @@ -623,30 +623,6 @@ - - - - varchar - http://cpas.labkey.com/Study#ParticipantId - - ptid - - - - timestamp - http://cpas.labkey.com/Study#VisitDate - http://cpas.labkey.com/Study#VisitDate - - - varchar - - - varchar - - - Historical Other - Captures miscellaneous historical health and research records for primates that do not fit other specific categories. -
HousingTracks housing assignments and location transfers for primates within the research facility. diff --git a/nbri_ehr/resources/reports/additionalReports.tsv b/nbri_ehr/resources/reports/additionalReports.tsv index 8eddd91..efaa2ad 100644 --- a/nbri_ehr/resources/reports/additionalReports.tsv +++ b/nbri_ehr/resources/reports/additionalReports.tsv @@ -49,5 +49,4 @@ behaviorRemarks Behavior query Behavior Remarks true study BehaviorClinRemarks clinObsBehavior Behavior query Observations true study behaviorObservations date false false qcstate/publicdata This report contains one record for each encounter with each animal, including surergies, exams, procedures, etc. clinremarks Clinical query Clinical Remarks true study ClinicalClinRemarks date false false qcstate/publicdata This report contains the clinical remarks entered about each animal physicalExam Clinical query Exam History True study physicalExam date false false qcstate/publicdata This report displays physical exam data for the selected animal -historicalOther General query Historical True study historicalOther date false false qcstate/publicdata This report displays historical events from legacy systems conceptionsByDam Reproductive Management query Conceptions by Dam true nbri_ehr ConceptionsByDam ConceptDate false false qcstate/publicdata This report displays conception records where the selected animal is the dam \ No newline at end of file From 5afd2c8d631c9c247b07b7953ccb66e964013b88 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sat, 1 Aug 2026 17:49:29 -0600 Subject: [PATCH 4/8] Fix location lookups and views broken by the floor removal Removing floors deleted the fullRoom and fullFloor display columns but left the room and floor lookups, the cage details page, and roughly fifty views still pointing through them, so Room and Building rendered blank. Room entry now requires a building, which the derived room key needs and previously went unchecked. --- .../resources/queries/ehr_lookups/rooms.js | 5 ++ .../queries/ehr_lookups/rooms.query.xml | 2 + .../queries/nbri_ehr/AnimalReqOrder.query.xml | 2 +- .../study/BehaviorClinRemarks/.qview.xml | 2 +- .../study/ClinicalClinRemarks/.qview.xml | 2 +- .../study/activeBehaviorCases/.qview.xml | 2 +- .../study/activeClinicalCases/.qview.xml | 2 +- .../queries/study/alopecia/.qview.xml | 2 +- .../queries/study/behaviorCases/.qview.xml | 2 +- .../study/behaviorObservations/.qview.xml | 2 +- .../resources/queries/study/blood/.qview.xml | 2 +- .../queries/study/breeder/.qview.xml | 2 +- .../resources/queries/study/cases/.qview.xml | 2 +- .../cases/Active Behavior Cases.qview.xml | 2 +- .../cases/Active Clinical Cases.qview.xml | 2 +- .../study/cases/All Behavior Cases.qview.xml | 2 +- .../study/cases/All Clinical Cases.qview.xml | 2 +- .../queries/study/chemistryResults/.qview.xml | 2 +- .../queries/study/clinicalCases/.qview.xml | 2 +- .../study/clinicalObservations/.qview.xml | 2 +- .../study/clinical_observations/.qview.xml | 2 +- .../Alopecia Scores.qview.xml | 2 +- .../clinical_observations/Behavior.qview.xml | 2 +- .../clinical_observations/Clinical.qview.xml | 2 +- .../clinical_observationsSummary/.qview.xml | 2 +- .../queries/study/clinremarks/.qview.xml | 2 +- .../study/clinremarks/Behavior.qview.xml | 2 +- .../study/clinremarks/Clinical.qview.xml | 2 +- .../queries/study/demographics/.qview.xml | 2 +- .../resources/queries/study/drug/.qview.xml | 2 +- .../queries/study/drug/Behavior.qview.xml | 2 +- .../queries/study/exemptions/.qview.xml | 2 +- .../resources/queries/study/flags/.qview.xml | 2 +- .../resources/queries/study/housing.query.xml | 2 +- .../queries/study/housing/.qview.xml | 2 +- .../study/housing/Active Housing.qview.xml | 1 + .../resources/queries/study/notes/.qview.xml | 2 +- .../study/observationSchedule/.qview.xml | 4 +- .../study/observation_order/.qview.xml | 2 +- .../Active Behavior Orders.qview.xml | 2 +- .../Active Clinical Orders.qview.xml | 2 +- .../Behavior Orders.qview.xml | 2 +- .../Clinical Orders.qview.xml | 2 +- .../queries/study/pairingSummary/.qview.xml | 2 +- .../pairingSummary/Active Pairing.qview.xml | 2 +- .../pairingSummary/Pairing History.qview.xml | 2 +- .../queries/study/pairings/.qview.xml | 2 +- .../queries/study/physicalExam/.qview.xml | 2 +- .../resources/queries/study/prc/.qview.xml | 2 +- .../queries/study/prcOverdue/.qview.xml | 2 +- .../queries/study/prcSchedule/.qview.xml | 4 +- .../queries/study/prc_order/.qview.xml | 2 +- .../queries/study/prc_order_report/.qview.xml | 2 +- .../study/treatmentSchedule/.qview.xml | 4 +- .../queries/study/treatment_order/.qview.xml | 2 +- .../resources/queries/study/vitals/.qview.xml | 2 +- .../resources/queries/study/weight/.qview.xml | 2 +- nbri_ehr/resources/views/cageDetails.html | 4 +- .../history/NBRIHousingDataSource.java | 4 +- .../nbri_ehr/table/NBRI_EHRCustomizer.java | 4 +- .../tests.nbri_ehr/NBRI_EHRTest.java | 48 +++++++++++++++---- 61 files changed, 110 insertions(+), 72 deletions(-) diff --git a/nbri_ehr/resources/queries/ehr_lookups/rooms.js b/nbri_ehr/resources/queries/ehr_lookups/rooms.js index 9a03aad..a35d8de 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/rooms.js +++ b/nbri_ehr/resources/queries/ehr_lookups/rooms.js @@ -15,6 +15,11 @@ function onUpsert(row, oldRow, errors){ return; } + if (!row.building) { + errors['building'] = 'Building is required.'; + return; + } + if (!row.room) { if (oldRow && oldRow.room && oldRow.room[0]) { row.room = oldRow.room[0]; diff --git a/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml b/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml index 3672017..a58dd7a 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml +++ b/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml @@ -8,6 +8,8 @@ + + /nbri_ehr/cageDetails.view?room=${room} true false false diff --git a/nbri_ehr/resources/queries/nbri_ehr/AnimalReqOrder.query.xml b/nbri_ehr/resources/queries/nbri_ehr/AnimalReqOrder.query.xml index 5c829e1..e282d4d 100644 --- a/nbri_ehr/resources/queries/nbri_ehr/AnimalReqOrder.query.xml +++ b/nbri_ehr/resources/queries/nbri_ehr/AnimalReqOrder.query.xml @@ -96,7 +96,7 @@ ehr_lookups rooms room - name + room diff --git a/nbri_ehr/resources/queries/study/BehaviorClinRemarks/.qview.xml b/nbri_ehr/resources/queries/study/BehaviorClinRemarks/.qview.xml index 4e381e4..243ffad 100644 --- a/nbri_ehr/resources/queries/study/BehaviorClinRemarks/.qview.xml +++ b/nbri_ehr/resources/queries/study/BehaviorClinRemarks/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/ClinicalClinRemarks/.qview.xml b/nbri_ehr/resources/queries/study/ClinicalClinRemarks/.qview.xml index c3ca1ea..86bc194 100644 --- a/nbri_ehr/resources/queries/study/ClinicalClinRemarks/.qview.xml +++ b/nbri_ehr/resources/queries/study/ClinicalClinRemarks/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/activeBehaviorCases/.qview.xml b/nbri_ehr/resources/queries/study/activeBehaviorCases/.qview.xml index b08797c..6026d97 100644 --- a/nbri_ehr/resources/queries/study/activeBehaviorCases/.qview.xml +++ b/nbri_ehr/resources/queries/study/activeBehaviorCases/.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/activeClinicalCases/.qview.xml b/nbri_ehr/resources/queries/study/activeClinicalCases/.qview.xml index b08797c..6026d97 100644 --- a/nbri_ehr/resources/queries/study/activeClinicalCases/.qview.xml +++ b/nbri_ehr/resources/queries/study/activeClinicalCases/.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/alopecia/.qview.xml b/nbri_ehr/resources/queries/study/alopecia/.qview.xml index f889f92..b514468 100644 --- a/nbri_ehr/resources/queries/study/alopecia/.qview.xml +++ b/nbri_ehr/resources/queries/study/alopecia/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/behaviorCases/.qview.xml b/nbri_ehr/resources/queries/study/behaviorCases/.qview.xml index b08797c..6026d97 100644 --- a/nbri_ehr/resources/queries/study/behaviorCases/.qview.xml +++ b/nbri_ehr/resources/queries/study/behaviorCases/.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/behaviorObservations/.qview.xml b/nbri_ehr/resources/queries/study/behaviorObservations/.qview.xml index a48d5a5..5a68b93 100644 --- a/nbri_ehr/resources/queries/study/behaviorObservations/.qview.xml +++ b/nbri_ehr/resources/queries/study/behaviorObservations/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/blood/.qview.xml b/nbri_ehr/resources/queries/study/blood/.qview.xml index 831d6b5..c382d83 100644 --- a/nbri_ehr/resources/queries/study/blood/.qview.xml +++ b/nbri_ehr/resources/queries/study/blood/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/breeder/.qview.xml b/nbri_ehr/resources/queries/study/breeder/.qview.xml index 448bd0f..bba85e1 100644 --- a/nbri_ehr/resources/queries/study/breeder/.qview.xml +++ b/nbri_ehr/resources/queries/study/breeder/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/cases/.qview.xml b/nbri_ehr/resources/queries/study/cases/.qview.xml index 340d770..a2f1fbc 100644 --- a/nbri_ehr/resources/queries/study/cases/.qview.xml +++ b/nbri_ehr/resources/queries/study/cases/.qview.xml @@ -3,7 +3,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/cases/Active Behavior Cases.qview.xml b/nbri_ehr/resources/queries/study/cases/Active Behavior Cases.qview.xml index 8333562..7f83fe9 100644 --- a/nbri_ehr/resources/queries/study/cases/Active Behavior Cases.qview.xml +++ b/nbri_ehr/resources/queries/study/cases/Active Behavior Cases.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/cases/Active Clinical Cases.qview.xml b/nbri_ehr/resources/queries/study/cases/Active Clinical Cases.qview.xml index 6081418..7554046 100644 --- a/nbri_ehr/resources/queries/study/cases/Active Clinical Cases.qview.xml +++ b/nbri_ehr/resources/queries/study/cases/Active Clinical Cases.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/cases/All Behavior Cases.qview.xml b/nbri_ehr/resources/queries/study/cases/All Behavior Cases.qview.xml index 261b1bb..25baf96 100644 --- a/nbri_ehr/resources/queries/study/cases/All Behavior Cases.qview.xml +++ b/nbri_ehr/resources/queries/study/cases/All Behavior Cases.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/cases/All Clinical Cases.qview.xml b/nbri_ehr/resources/queries/study/cases/All Clinical Cases.qview.xml index 73fb0b2..e0dd66f 100644 --- a/nbri_ehr/resources/queries/study/cases/All Clinical Cases.qview.xml +++ b/nbri_ehr/resources/queries/study/cases/All Clinical Cases.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/chemistryResults/.qview.xml b/nbri_ehr/resources/queries/study/chemistryResults/.qview.xml index 02467cb..3cc536b 100644 --- a/nbri_ehr/resources/queries/study/chemistryResults/.qview.xml +++ b/nbri_ehr/resources/queries/study/chemistryResults/.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinicalCases/.qview.xml b/nbri_ehr/resources/queries/study/clinicalCases/.qview.xml index b08797c..6026d97 100644 --- a/nbri_ehr/resources/queries/study/clinicalCases/.qview.xml +++ b/nbri_ehr/resources/queries/study/clinicalCases/.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinicalObservations/.qview.xml b/nbri_ehr/resources/queries/study/clinicalObservations/.qview.xml index 7b1fc1a..0fe5c32 100644 --- a/nbri_ehr/resources/queries/study/clinicalObservations/.qview.xml +++ b/nbri_ehr/resources/queries/study/clinicalObservations/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinical_observations/.qview.xml b/nbri_ehr/resources/queries/study/clinical_observations/.qview.xml index daaa82d..8d99f0c 100644 --- a/nbri_ehr/resources/queries/study/clinical_observations/.qview.xml +++ b/nbri_ehr/resources/queries/study/clinical_observations/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinical_observations/Alopecia Scores.qview.xml b/nbri_ehr/resources/queries/study/clinical_observations/Alopecia Scores.qview.xml index 0a05b82..589eba9 100644 --- a/nbri_ehr/resources/queries/study/clinical_observations/Alopecia Scores.qview.xml +++ b/nbri_ehr/resources/queries/study/clinical_observations/Alopecia Scores.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinical_observations/Behavior.qview.xml b/nbri_ehr/resources/queries/study/clinical_observations/Behavior.qview.xml index f7566f3..6c017a8 100644 --- a/nbri_ehr/resources/queries/study/clinical_observations/Behavior.qview.xml +++ b/nbri_ehr/resources/queries/study/clinical_observations/Behavior.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinical_observations/Clinical.qview.xml b/nbri_ehr/resources/queries/study/clinical_observations/Clinical.qview.xml index adf8cf8..e8cff6b 100644 --- a/nbri_ehr/resources/queries/study/clinical_observations/Clinical.qview.xml +++ b/nbri_ehr/resources/queries/study/clinical_observations/Clinical.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinical_observationsSummary/.qview.xml b/nbri_ehr/resources/queries/study/clinical_observationsSummary/.qview.xml index a48d5a5..5a68b93 100644 --- a/nbri_ehr/resources/queries/study/clinical_observationsSummary/.qview.xml +++ b/nbri_ehr/resources/queries/study/clinical_observationsSummary/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinremarks/.qview.xml b/nbri_ehr/resources/queries/study/clinremarks/.qview.xml index 903f88d..d96725e 100644 --- a/nbri_ehr/resources/queries/study/clinremarks/.qview.xml +++ b/nbri_ehr/resources/queries/study/clinremarks/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinremarks/Behavior.qview.xml b/nbri_ehr/resources/queries/study/clinremarks/Behavior.qview.xml index 24f5a40..007434b 100644 --- a/nbri_ehr/resources/queries/study/clinremarks/Behavior.qview.xml +++ b/nbri_ehr/resources/queries/study/clinremarks/Behavior.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/clinremarks/Clinical.qview.xml b/nbri_ehr/resources/queries/study/clinremarks/Clinical.qview.xml index 6cb3771..3123f8c 100644 --- a/nbri_ehr/resources/queries/study/clinremarks/Clinical.qview.xml +++ b/nbri_ehr/resources/queries/study/clinremarks/Clinical.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/demographics/.qview.xml b/nbri_ehr/resources/queries/study/demographics/.qview.xml index 439cd0a..d232062 100644 --- a/nbri_ehr/resources/queries/study/demographics/.qview.xml +++ b/nbri_ehr/resources/queries/study/demographics/.qview.xml @@ -4,7 +4,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/drug/.qview.xml b/nbri_ehr/resources/queries/study/drug/.qview.xml index 3b72268..8489e4d 100644 --- a/nbri_ehr/resources/queries/study/drug/.qview.xml +++ b/nbri_ehr/resources/queries/study/drug/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/drug/Behavior.qview.xml b/nbri_ehr/resources/queries/study/drug/Behavior.qview.xml index a9769cb..1688ad3 100644 --- a/nbri_ehr/resources/queries/study/drug/Behavior.qview.xml +++ b/nbri_ehr/resources/queries/study/drug/Behavior.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/exemptions/.qview.xml b/nbri_ehr/resources/queries/study/exemptions/.qview.xml index 7e4278b..cdd0721 100644 --- a/nbri_ehr/resources/queries/study/exemptions/.qview.xml +++ b/nbri_ehr/resources/queries/study/exemptions/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/flags/.qview.xml b/nbri_ehr/resources/queries/study/flags/.qview.xml index bf12e57..05657ca 100644 --- a/nbri_ehr/resources/queries/study/flags/.qview.xml +++ b/nbri_ehr/resources/queries/study/flags/.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/housing.query.xml b/nbri_ehr/resources/queries/study/housing.query.xml index de3337b..b08ae42 100644 --- a/nbri_ehr/resources/queries/study/housing.query.xml +++ b/nbri_ehr/resources/queries/study/housing.query.xml @@ -30,7 +30,7 @@ ehr_lookups rooms room - name + room diff --git a/nbri_ehr/resources/queries/study/housing/.qview.xml b/nbri_ehr/resources/queries/study/housing/.qview.xml index bc780a5..5f01093 100644 --- a/nbri_ehr/resources/queries/study/housing/.qview.xml +++ b/nbri_ehr/resources/queries/study/housing/.qview.xml @@ -7,7 +7,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/housing/Active Housing.qview.xml b/nbri_ehr/resources/queries/study/housing/Active Housing.qview.xml index 4fc3eeb..54e07b9 100644 --- a/nbri_ehr/resources/queries/study/housing/Active Housing.qview.xml +++ b/nbri_ehr/resources/queries/study/housing/Active Housing.qview.xml @@ -4,6 +4,7 @@ + diff --git a/nbri_ehr/resources/queries/study/notes/.qview.xml b/nbri_ehr/resources/queries/study/notes/.qview.xml index 6cd9bab..0f3a556 100644 --- a/nbri_ehr/resources/queries/study/notes/.qview.xml +++ b/nbri_ehr/resources/queries/study/notes/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/observationSchedule/.qview.xml b/nbri_ehr/resources/queries/study/observationSchedule/.qview.xml index b91d5d4..628bcd9 100644 --- a/nbri_ehr/resources/queries/study/observationSchedule/.qview.xml +++ b/nbri_ehr/resources/queries/study/observationSchedule/.qview.xml @@ -3,7 +3,7 @@ - + @@ -11,7 +11,7 @@ - + \ No newline at end of file diff --git a/nbri_ehr/resources/queries/study/observation_order/.qview.xml b/nbri_ehr/resources/queries/study/observation_order/.qview.xml index c02d2fb..40bb429 100644 --- a/nbri_ehr/resources/queries/study/observation_order/.qview.xml +++ b/nbri_ehr/resources/queries/study/observation_order/.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/observation_order/Active Behavior Orders.qview.xml b/nbri_ehr/resources/queries/study/observation_order/Active Behavior Orders.qview.xml index d4c5c11..c2c3d7e 100644 --- a/nbri_ehr/resources/queries/study/observation_order/Active Behavior Orders.qview.xml +++ b/nbri_ehr/resources/queries/study/observation_order/Active Behavior Orders.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/observation_order/Active Clinical Orders.qview.xml b/nbri_ehr/resources/queries/study/observation_order/Active Clinical Orders.qview.xml index 1638c87..f8bc797 100644 --- a/nbri_ehr/resources/queries/study/observation_order/Active Clinical Orders.qview.xml +++ b/nbri_ehr/resources/queries/study/observation_order/Active Clinical Orders.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/observation_order/Behavior Orders.qview.xml b/nbri_ehr/resources/queries/study/observation_order/Behavior Orders.qview.xml index 343d9b8..ad56df0 100644 --- a/nbri_ehr/resources/queries/study/observation_order/Behavior Orders.qview.xml +++ b/nbri_ehr/resources/queries/study/observation_order/Behavior Orders.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/observation_order/Clinical Orders.qview.xml b/nbri_ehr/resources/queries/study/observation_order/Clinical Orders.qview.xml index 5e1e749..24a0828 100644 --- a/nbri_ehr/resources/queries/study/observation_order/Clinical Orders.qview.xml +++ b/nbri_ehr/resources/queries/study/observation_order/Clinical Orders.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/pairingSummary/.qview.xml b/nbri_ehr/resources/queries/study/pairingSummary/.qview.xml index f0fdcf4..48d82d6 100644 --- a/nbri_ehr/resources/queries/study/pairingSummary/.qview.xml +++ b/nbri_ehr/resources/queries/study/pairingSummary/.qview.xml @@ -6,7 +6,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/pairingSummary/Active Pairing.qview.xml b/nbri_ehr/resources/queries/study/pairingSummary/Active Pairing.qview.xml index 14e0dfe..689f99f 100644 --- a/nbri_ehr/resources/queries/study/pairingSummary/Active Pairing.qview.xml +++ b/nbri_ehr/resources/queries/study/pairingSummary/Active Pairing.qview.xml @@ -9,7 +9,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/pairingSummary/Pairing History.qview.xml b/nbri_ehr/resources/queries/study/pairingSummary/Pairing History.qview.xml index 1a11617..81ee864 100644 --- a/nbri_ehr/resources/queries/study/pairingSummary/Pairing History.qview.xml +++ b/nbri_ehr/resources/queries/study/pairingSummary/Pairing History.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/pairings/.qview.xml b/nbri_ehr/resources/queries/study/pairings/.qview.xml index 26a8731..1a55dfa 100644 --- a/nbri_ehr/resources/queries/study/pairings/.qview.xml +++ b/nbri_ehr/resources/queries/study/pairings/.qview.xml @@ -6,7 +6,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/physicalExam/.qview.xml b/nbri_ehr/resources/queries/study/physicalExam/.qview.xml index caf3fb0..3fb7d6a 100644 --- a/nbri_ehr/resources/queries/study/physicalExam/.qview.xml +++ b/nbri_ehr/resources/queries/study/physicalExam/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/prc/.qview.xml b/nbri_ehr/resources/queries/study/prc/.qview.xml index f5ed643..089bd36 100644 --- a/nbri_ehr/resources/queries/study/prc/.qview.xml +++ b/nbri_ehr/resources/queries/study/prc/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/prcOverdue/.qview.xml b/nbri_ehr/resources/queries/study/prcOverdue/.qview.xml index b8bf120..7957d9f 100644 --- a/nbri_ehr/resources/queries/study/prcOverdue/.qview.xml +++ b/nbri_ehr/resources/queries/study/prcOverdue/.qview.xml @@ -3,7 +3,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/prcSchedule/.qview.xml b/nbri_ehr/resources/queries/study/prcSchedule/.qview.xml index 4604b77..227ae69 100644 --- a/nbri_ehr/resources/queries/study/prcSchedule/.qview.xml +++ b/nbri_ehr/resources/queries/study/prcSchedule/.qview.xml @@ -3,7 +3,7 @@ - + @@ -17,7 +17,7 @@ - + \ No newline at end of file diff --git a/nbri_ehr/resources/queries/study/prc_order/.qview.xml b/nbri_ehr/resources/queries/study/prc_order/.qview.xml index b9de6c1..8da38d4 100644 --- a/nbri_ehr/resources/queries/study/prc_order/.qview.xml +++ b/nbri_ehr/resources/queries/study/prc_order/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/prc_order_report/.qview.xml b/nbri_ehr/resources/queries/study/prc_order_report/.qview.xml index 62d20f0..3402400 100644 --- a/nbri_ehr/resources/queries/study/prc_order_report/.qview.xml +++ b/nbri_ehr/resources/queries/study/prc_order_report/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/treatmentSchedule/.qview.xml b/nbri_ehr/resources/queries/study/treatmentSchedule/.qview.xml index 4c57f0b..e773c14 100644 --- a/nbri_ehr/resources/queries/study/treatmentSchedule/.qview.xml +++ b/nbri_ehr/resources/queries/study/treatmentSchedule/.qview.xml @@ -3,7 +3,7 @@ - + @@ -22,7 +22,7 @@ - + \ No newline at end of file diff --git a/nbri_ehr/resources/queries/study/treatment_order/.qview.xml b/nbri_ehr/resources/queries/study/treatment_order/.qview.xml index 27608af..2b14b7b 100644 --- a/nbri_ehr/resources/queries/study/treatment_order/.qview.xml +++ b/nbri_ehr/resources/queries/study/treatment_order/.qview.xml @@ -2,7 +2,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/vitals/.qview.xml b/nbri_ehr/resources/queries/study/vitals/.qview.xml index f3b831c..b6bc349 100644 --- a/nbri_ehr/resources/queries/study/vitals/.qview.xml +++ b/nbri_ehr/resources/queries/study/vitals/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/queries/study/weight/.qview.xml b/nbri_ehr/resources/queries/study/weight/.qview.xml index b27ffe4..b6d1a95 100644 --- a/nbri_ehr/resources/queries/study/weight/.qview.xml +++ b/nbri_ehr/resources/queries/study/weight/.qview.xml @@ -5,7 +5,7 @@ - + diff --git a/nbri_ehr/resources/views/cageDetails.html b/nbri_ehr/resources/views/cageDetails.html index a0c5da2..fde98de 100644 --- a/nbri_ehr/resources/views/cageDetails.html +++ b/nbri_ehr/resources/views/cageDetails.html @@ -42,7 +42,7 @@ schemaName: 'ehr_lookups', queryName: 'cage', filterArray: detailFilterArray, - columns: 'cage,room,room/floor,room/floor/building,room/floor/building/area,outdoor', + columns: 'cage,room,room/building,room/building/area,outdoor', }, title: 'Cage Details', renderTo: 'cageDetails_' + webpart.wrapperDivId, @@ -56,7 +56,7 @@ queryName: 'cage', filterArray: detailFilterArray, sort: 'cage', - columns: 'cage,room,room/floor,room/floor/building,room/floor/building/area,outdoor', + columns: 'cage,room,room/building,room/building/area,outdoor', }).render('cageDetails_' + webpart.wrapperDivId); } diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/history/NBRIHousingDataSource.java b/nbri_ehr/src/org/labkey/nbri_ehr/history/NBRIHousingDataSource.java index 91fdf41..0749f01 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/history/NBRIHousingDataSource.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/history/NBRIHousingDataSource.java @@ -36,7 +36,7 @@ public NBRIHousingDataSource(Module module) @Override protected Set getColumnNames() { - return PageFlowUtil.set("Id", "date", "cage/cage", "room/fullRoom", "reason", "remark"); + return PageFlowUtil.set("Id", "date", "cage/cage", "room/room", "reason", "remark"); } @Override @@ -44,7 +44,7 @@ protected String getHtml(Container c, Results rs, boolean redacted) throws SQLEx { StringBuilder sb = new StringBuilder(); - FieldKey room = FieldKey.fromString("room/fullRoom"); + FieldKey room = FieldKey.fromString("room/room"); FieldKey cage = FieldKey.fromString("cage/cage"); String value = "Unknown"; if (rs.hasColumn(cage) && rs.getObject(cage) != null) diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java b/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java index 7ed4cf2..b365680 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/table/NBRI_EHRCustomizer.java @@ -713,7 +713,7 @@ public void doSharedCustomization(AbstractTableInfo ti) { UserSchema us = getEHRUserSchema(ti, "ehr_lookups"); col.setLabel("Room"); - col.setFk(new QueryForeignKey(ti.getUserSchema(), ti.getContainerFilter(), us, null, "rooms", "room", "fullRoom")); + col.setFk(new QueryForeignKey(ti.getUserSchema(), ti.getContainerFilter(), us, null, "rooms", "room", "room")); col.setURL(StringExpressionFactory.createURL("/nbri_ehr/cageDetails.view?room=${room}")); } if ("building".equalsIgnoreCase(col.getName()) && !ti.getName().equalsIgnoreCase("buildings")) @@ -726,7 +726,7 @@ public void doSharedCustomization(AbstractTableInfo ti) { UserSchema us = getEHRUserSchema(ti, "ehr_lookups"); col.setLabel("Floor"); - col.setFk(new QueryForeignKey(ti.getUserSchema(), ti.getContainerFilter(), us, null, "floors", "floor", "fullFloor")); + col.setFk(new QueryForeignKey(ti.getUserSchema(), ti.getContainerFilter(), us, null, "floors", "floor", "name")); } if ("area".equalsIgnoreCase(col.getName()) && !ti.getName().equalsIgnoreCase("areas")) { diff --git a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java index a06c389..1d94cb4 100644 --- a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java +++ b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java @@ -107,6 +107,11 @@ public class NBRI_EHRTest extends AbstractGenericEHRTest implements PostgresOnly // createTestSubjects so the clinical case form raises no warnings that would keep the validation banner up. private static final String taskGroupAnimalId = "TESTGRP9090"; + // Rooms are keyed by building and name, so every room fixture needs a building to hang off of. + // 'buildings' derives its key from the description, and 'SPF' is one of the areas seeded with the ehr_lookups schema. + private static final String BUILDING_ID = "TestBuilding"; + private static final String BUILDING_AREA = "SPF"; + private final String[] weightFields = {"Id", "date", "enddate", "project", "weight", FIELD_QCSTATELABEL, FIELD_OBJECTID, FIELD_LSID, "_recordid", "performedby"}; private final Object[] weightData1 = {getExpectedAnimalIDCasing("TESTSUBJECT1"), EHRClientAPIHelper.DATE_SUBSTITUTION, null, null, "12", EHRQCState.IN_PROGRESS.label, null, null, "_recordID", 1004}; @@ -244,20 +249,44 @@ private void populateFormulary() throws IOException, CommandException RowsResponse saveRowsResponse = insertRowsCommand.execute(getApiHelper().getConnection(), getContainerPath()); } + /** + * The key a room trigger derives for the given room name. Mirrors ehr_lookups/rooms.js. + */ + private static String roomKey(String roomName) + { + return BUILDING_ID + "-" + roomName; + } + + @LogMethod + private void populateBuildingRecords() throws Exception + { + InsertRowsCommand insertCmd = new InsertRowsCommand("ehr_lookups", "buildings"); + Map rowMap = new HashMap<>(); + // Supply the derived key rather than relying on the trigger to fill it, matching how the base class seeds rooms. + rowMap.put("name", BUILDING_ID); + rowMap.put("description", BUILDING_ID); + rowMap.put("area", BUILDING_AREA); + insertCmd.addRow(rowMap); + + insertCmd.execute(createDefaultConnection(), getContainerPath()); + } + @Override protected void populateRoomRecords() throws Exception { + populateBuildingRecords(); + InsertRowsCommand insertCmd = new InsertRowsCommand("ehr_lookups", "rooms"); Map rowMap = new HashMap<>(); rowMap.put("name", ROOM_ID); - rowMap.put("floor", "floor1"); + rowMap.put("building", BUILDING_ID); rowMap.put("housingType", 1); rowMap.put("housingCondition", 1); insertCmd.addRow(rowMap); rowMap = new HashMap<>(); rowMap.put("name", ROOM_ID2); - rowMap.put("floor", "floor2"); + rowMap.put("building", BUILDING_ID); rowMap.put("housingType", 1); rowMap.put("housingCondition", 1); insertCmd.addRow(rowMap); @@ -321,19 +350,20 @@ private void enableNotification(String notification) private void populateLocations() throws IOException, CommandException { goToEHRFolder(); + // BUILDING_ID is created by populateRoomRecords, which runs earlier as part of initProject. log("Inserting values in rooms"); InsertRowsCommand roomCmd = new InsertRowsCommand("ehr_lookups", "rooms"); - roomCmd.addRow(Map.of("name", "R1", "floor", "F1")); - roomCmd.addRow(Map.of("name", "R2", "floor", "F2")); - roomCmd.addRow(Map.of("name", "R3", "floor", "F3")); + roomCmd.addRow(Map.of("name", "R1", "building", BUILDING_ID)); + roomCmd.addRow(Map.of("name", "R2", "building", BUILDING_ID)); + roomCmd.addRow(Map.of("name", "R3", "building", BUILDING_ID)); roomCmd.execute(getApiHelper().getConnection(), getContainerPath()); log("Inserting values in cage"); InsertRowsCommand cageCmd = new InsertRowsCommand("ehr_lookups", "cage"); - cageCmd.addRow(Map.of("location", "L1", "cage", "C1", "room", "R1")); - cageCmd.addRow(Map.of("location", "L2", "cage", "C2", "room", "R1")); - cageCmd.addRow(Map.of("location", "L3", "cage", "C3", "room", "R2")); - cageCmd.addRow(Map.of("location", "L4", "cage", "C4", "room", "R3")); + cageCmd.addRow(Map.of("location", "L1", "cage", "C1", "room", roomKey("R1"))); + cageCmd.addRow(Map.of("location", "L2", "cage", "C2", "room", roomKey("R1"))); + cageCmd.addRow(Map.of("location", "L3", "cage", "C3", "room", roomKey("R2"))); + cageCmd.addRow(Map.of("location", "L4", "cage", "C4", "room", roomKey("R3"))); cageCmd.execute(getApiHelper().getConnection(), getContainerPath()); } From e51fd086d57945b0a71246634145a96de765bc7a Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sat, 1 Aug 2026 18:02:59 -0600 Subject: [PATCH 5/8] Fix housing test fixtures and cover the new location rules The fixtures housed animals in rooms and cages that nothing ever created, so every room lookup in the housing views resolved to nothing. Adds coverage for the derived room key, the building requirement, and cagemates in a group pen. --- .../tests.nbri_ehr/NBRI_EHRTest.java | 112 +++++++++++++++++- 1 file changed, 106 insertions(+), 6 deletions(-) diff --git a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java index 1d94cb4..00688ea 100644 --- a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java +++ b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java @@ -34,6 +34,8 @@ import org.labkey.remoteapi.query.ImportDataCommand; import org.labkey.remoteapi.query.InsertRowsCommand; import org.labkey.remoteapi.query.RowsResponse; +import org.labkey.remoteapi.query.SelectRowsCommand; +import org.labkey.remoteapi.query.SelectRowsResponse; import org.labkey.remoteapi.security.CreateUserResponse; import org.labkey.test.Locator; import org.labkey.test.TestFileUtils; @@ -112,6 +114,15 @@ public class NBRI_EHRTest extends AbstractGenericEHRTest implements PostgresOnly private static final String BUILDING_ID = "TestBuilding"; private static final String BUILDING_AREA = "SPF"; + // Cage locations seeded by populateLocations, named for the room each one sits in. Housing records key off the + // location rather than the cage name, so these are what belongs in a housing row's 'cage' field. + private static final String CAGE_IN_R1 = "L1"; + private static final String CAGE_IN_R3 = "L4"; + + // A group pen has no cage, so its location is the room key alone. Created by testGroupPenCagemates. + private static final String PEN_ROOM_NAME = "PEN1"; + private static final String[] PEN_ANIMALS = {"PEN0001", "PEN0002"}; + private final String[] weightFields = {"Id", "date", "enddate", "project", "weight", FIELD_QCSTATELABEL, FIELD_OBJECTID, FIELD_LSID, "_recordid", "performedby"}; private final Object[] weightData1 = {getExpectedAnimalIDCasing("TESTSUBJECT1"), EHRClientAPIHelper.DATE_SUBSTITUTION, null, null, "12", EHRQCState.IN_PROGRESS.label, null, null, "_recordID", 1004}; @@ -257,6 +268,16 @@ private static String roomKey(String roomName) return BUILDING_ID + "-" + roomName; } + /** + * Housing records store the derived room key, so the fixture rooms have to be named by key for the room + * lookups to resolve. The base implementation returns names that match no room in this study. + */ + @Override + protected String[] getRooms() + { + return new String[]{roomKey("R1"), roomKey("R2"), roomKey("R3")}; + } + @LogMethod private void populateBuildingRecords() throws Exception { @@ -469,10 +490,10 @@ protected void createTestSubjects() throws Exception log("Creating initial housing records"); fields = new String[]{"Id", "date", "enddate", "room", "cage", "performedby"}; data = new Object[][]{ - {SUBJECTS[0], pastDate1, pastDate2, getRooms()[0], CAGES[0], 1004}, - {SUBJECTS[0], pastDate2, null, getRooms()[0], CAGES[0], 1004}, - {SUBJECTS[1], pastDate1, pastDate2, getRooms()[0], CAGES[0], 1004}, - {SUBJECTS[1], pastDate2, null, getRooms()[2], CAGES[2], 1004} + {SUBJECTS[0], pastDate1, pastDate2, getRooms()[0], CAGE_IN_R1, 1004}, + {SUBJECTS[0], pastDate2, null, getRooms()[0], CAGE_IN_R1, 1004}, + {SUBJECTS[1], pastDate1, pastDate2, getRooms()[0], CAGE_IN_R1, 1004}, + {SUBJECTS[1], pastDate2, null, getRooms()[2], CAGE_IN_R3, 1004} }; insertCommand = getApiHelper().prepareInsertCommand("study", "Housing", "lsid", fields, data); getApiHelper().deleteAllRecords("study", "Housing", new Filter("Id", StringUtils.join(SUBJECTS, ";"), Filter.Operator.IN)); @@ -516,7 +537,7 @@ protected void createTestSubjects() throws Exception fields = new String[]{"Id", "date", "enddate", "room", "cage", "performedby"}; data = new Object[][]{ - {taskGroupAnimalId, pastDate1, null, getRooms()[0], CAGES[0], 1004} + {taskGroupAnimalId, pastDate1, null, getRooms()[0], CAGE_IN_R1, 1004} }; insertCommand = getApiHelper().prepareInsertCommand("study", "Housing", "lsid", fields, data); getApiHelper().deleteAllRecords("study", "Housing", new Filter("Id", taskGroupAnimalId)); @@ -1263,7 +1284,7 @@ public void createSubjectsForDeathForm() throws IOException, CommandException project.execute(getApiHelper().getConnection(), getContainerPath()); InsertRowsCommand housing = new InsertRowsCommand("study", "housing"); - housing.addRow(Map.of("Id", aliveAnimalId, "date", LocalDateTime.now().minusDays(10), "cage", "C4", "QCStateLabel", "Completed", "performedby", 1004)); + housing.addRow(Map.of("Id", aliveAnimalId, "date", LocalDateTime.now().minusDays(10), "room", getRooms()[2], "cage", CAGE_IN_R3, "QCStateLabel", "Completed", "performedby", 1004)); housing.execute(getApiHelper().getConnection(), getContainerPath()); log("Marking an animal dead"); @@ -1566,6 +1587,85 @@ public void testCalculatedAgeColumns() assertEquals("Calculated ages are incorrect", Arrays.asList("4.8", "4.0", "58.0"), row.subList(columnCount - 3, columnCount)); } + @Test + public void testRoomKeyDerivation() throws Exception + { + log("Verifying a room derives its key from its building and name"); + SelectRowsCommand selectCmd = new SelectRowsCommand("ehr_lookups", "rooms"); + selectCmd.setColumns(List.of("room", "name", "building")); + selectCmd.addFilter(new Filter("name", "R1")); + SelectRowsResponse response = selectCmd.execute(getApiHelper().getConnection(), getContainerPath()); + + Assert.assertEquals("Expected exactly one room named R1", 1, response.getRows().size()); + Map room = response.getRows().get(0); + Assert.assertEquals("Room key should combine the building and the name", roomKey("R1"), room.get("room")); + Assert.assertEquals("Room should belong to the seeded building", BUILDING_ID, room.get("building")); + } + + @Test + public void testRoomRequiresBuilding() + { + log("Verifying a room cannot be created without a building"); + InsertRowsCommand insertCmd = new InsertRowsCommand("ehr_lookups", "rooms"); + insertCmd.addRow(Map.of("name", "NOBUILDING")); + + try + { + insertCmd.execute(getApiHelper().getConnection(), getContainerPath()); + Assert.fail("Room insert should have been rejected when no building was supplied"); + } + catch (IOException | CommandException e) + { + Assert.assertTrue("Unexpected failure inserting a room without a building: " + e.getMessage(), + e.getMessage() != null && e.getMessage().contains("Building is required")); + } + } + + @Test + public void testGroupPenCagemates() throws Exception + { + String penRoom = roomKey(PEN_ROOM_NAME); + + log("Creating a group pen, whose location is the room with no cage"); + InsertRowsCommand roomCmd = new InsertRowsCommand("ehr_lookups", "rooms"); + roomCmd.addRow(Map.of("name", PEN_ROOM_NAME, "building", BUILDING_ID)); + roomCmd.execute(getApiHelper().getConnection(), getContainerPath()); + + InsertRowsCommand penCmd = new InsertRowsCommand("ehr_lookups", "cage"); + penCmd.addRow(Map.of("location", penRoom, "room", penRoom)); + penCmd.execute(getApiHelper().getConnection(), getContainerPath()); + + log("Housing two animals in the pen"); + String[] fields = new String[]{"Id", "Species", "Birth", "Gender", "date", "calculated_status", "objectid", "performedby"}; + Object[][] data = new Object[][]{ + {PEN_ANIMALS[0], "Rhesus", (new Date()).toString(), getMale(), new Date(), "Alive", UUID.randomUUID().toString(), 1004}, + {PEN_ANIMALS[1], "Rhesus", (new Date()).toString(), getMale(), new Date(), "Alive", UUID.randomUUID().toString(), 1004} + }; + SimplePostCommand insertCommand = getApiHelper().prepareInsertCommand("study", "demographics", "lsid", fields, data); + getApiHelper().deleteAllRecords("study", "demographics", new Filter("Id", StringUtils.join(PEN_ANIMALS, ";"), Filter.Operator.IN)); + getApiHelper().doSaveRows(DATA_ADMIN.getEmail(), insertCommand, getExtraContext()); + + fields = new String[]{"Id", "date", "enddate", "room", "cage", "QCStateLabel", "performedby"}; + data = new Object[][]{ + {PEN_ANIMALS[0], new Date(), null, penRoom, penRoom, EHRQCState.COMPLETED.label, 1004}, + {PEN_ANIMALS[1], new Date(), null, penRoom, penRoom, EHRQCState.COMPLETED.label, 1004} + }; + insertCommand = getApiHelper().prepareInsertCommand("study", "Housing", "lsid", fields, data); + getApiHelper().deleteAllRecords("study", "Housing", new Filter("Id", StringUtils.join(PEN_ANIMALS, ";"), Filter.Operator.IN)); + getApiHelper().doSaveRows(DATA_ADMIN.getEmail(), insertCommand, getExtraContext()); + + log("Verifying penned animals resolve as each other's cagemates"); + SelectRowsCommand selectCmd = new SelectRowsCommand("study", "demographicsCagemates"); + selectCmd.addFilter(new Filter("Id", PEN_ANIMALS[0])); + SelectRowsResponse response = selectCmd.execute(getApiHelper().getConnection(), getContainerPath()); + + Assert.assertEquals("Expected one cagemates row for " + PEN_ANIMALS[0], 1, response.getRows().size()); + Map cagemates = response.getRows().get(0); + Assert.assertEquals("Both penned animals should be counted", 2, ((Number) cagemates.get("total")).intValue()); + Assert.assertTrue("Cagemate list should name the other penned animal, was: " + cagemates.get("animals"), + String.valueOf(cagemates.get("animals")).contains(PEN_ANIMALS[1])); + } + @Test public void testLookupPage() throws Exception { From 1173c24af1d46a7803b487516782e5b082f99716 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sat, 1 Aug 2026 20:01:39 -0600 Subject: [PATCH 6/8] Point housing sample data at registered locations The sample housing rows used a free-text location that matches no registered room or cage, and omitted the room entirely, so the housing trigger skipped every one of them. --- .../study/study/datasets/datasetHousing.tsv | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv b/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv index 23969c0..5831519 100644 --- a/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv +++ b/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv @@ -1,9 +1,9 @@ -objectid Id QCStateLabel date enddate cage performedby -1 44444 Completed -1381d -1321d 11 - Rm 202B - ZZA 1004 -2 44444 Completed -1321d 11 - Rm 202B - ZZB 1004 -3 44446 Completed -1406d -1316d 11 - Rm 202B - ZZA 1004 -4 44446 Completed -1316d 11 - Rm 202B - ZZC 1004 -5 TEST4551032 Completed -6362d 11 - Rm 202B - ZZD 1004 -6 44442 Completed -6100d -5920d 11 - Rm 202B - ZZA 1004 -7 44442 Completed -5920d 11 - Rm 202B - ZZC 1004 -8 44443 Completed -6100d 11 - Rm 202B - ZZF 1004 +objectid Id QCStateLabel date enddate room cage performedby +1 44444 Completed -1381d -1321d TestBuilding-R1 L1 1004 +2 44444 Completed -1321d TestBuilding-R1 L2 1004 +3 44446 Completed -1406d -1316d TestBuilding-R1 L1 1004 +4 44446 Completed -1316d TestBuilding-R2 L3 1004 +5 TEST4551032 Completed -6362d TestBuilding-R3 L4 1004 +6 44442 Completed -6100d -5920d TestBuilding-R1 L1 1004 +7 44442 Completed -5920d TestBuilding-R2 L3 1004 +8 44443 Completed -6100d TestBuilding-R1 L2 1004 From 713d0e63e27cd7e9f680e1191ce96a8e4f2885fb Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sat, 1 Aug 2026 20:22:45 -0600 Subject: [PATCH 7/8] Let the cage trigger derive test locations The fixture supplied a location outright, so the derivation the trigger performs for real entry was never exercised and the test keys looked nothing like production ones. --- .../study/study/datasets/datasetHousing.tsv | 16 +++++----- .../tests.nbri_ehr/NBRI_EHRTest.java | 30 +++++++++++++------ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv b/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv index 5831519..487f797 100644 --- a/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv +++ b/nbri_ehr/test/sampledata/nbri_ehr/study/study/datasets/datasetHousing.tsv @@ -1,9 +1,9 @@ objectid Id QCStateLabel date enddate room cage performedby -1 44444 Completed -1381d -1321d TestBuilding-R1 L1 1004 -2 44444 Completed -1321d TestBuilding-R1 L2 1004 -3 44446 Completed -1406d -1316d TestBuilding-R1 L1 1004 -4 44446 Completed -1316d TestBuilding-R2 L3 1004 -5 TEST4551032 Completed -6362d TestBuilding-R3 L4 1004 -6 44442 Completed -6100d -5920d TestBuilding-R1 L1 1004 -7 44442 Completed -5920d TestBuilding-R2 L3 1004 -8 44443 Completed -6100d TestBuilding-R1 L2 1004 +1 44444 Completed -1381d -1321d TestBuilding-R1 TestBuilding-R1-C1 1004 +2 44444 Completed -1321d TestBuilding-R1 TestBuilding-R1-C2 1004 +3 44446 Completed -1406d -1316d TestBuilding-R1 TestBuilding-R1-C1 1004 +4 44446 Completed -1316d TestBuilding-R2 TestBuilding-R2-C3 1004 +5 TEST4551032 Completed -6362d TestBuilding-R3 TestBuilding-R3-C4 1004 +6 44442 Completed -6100d -5920d TestBuilding-R1 TestBuilding-R1-C1 1004 +7 44442 Completed -5920d TestBuilding-R2 TestBuilding-R2-C3 1004 +8 44443 Completed -6100d TestBuilding-R1 TestBuilding-R1-C2 1004 diff --git a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java index 00688ea..830b3b8 100644 --- a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java +++ b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java @@ -114,10 +114,11 @@ public class NBRI_EHRTest extends AbstractGenericEHRTest implements PostgresOnly private static final String BUILDING_ID = "TestBuilding"; private static final String BUILDING_AREA = "SPF"; - // Cage locations seeded by populateLocations, named for the room each one sits in. Housing records key off the - // location rather than the cage name, so these are what belongs in a housing row's 'cage' field. - private static final String CAGE_IN_R1 = "L1"; - private static final String CAGE_IN_R3 = "L4"; + // Cage locations seeded by populateLocations, named for the room each one sits in. The cage trigger derives these + // from the room and cage, and housing records key off the location, so these are what belongs in a housing row's + // 'cage' field. datasetHousing.tsv spells the same values out, since a TSV cannot call cageLocation. + private static final String CAGE_IN_R1 = cageLocation("R1", "C1"); + private static final String CAGE_IN_R3 = cageLocation("R3", "C4"); // A group pen has no cage, so its location is the room key alone. Created by testGroupPenCagemates. private static final String PEN_ROOM_NAME = "PEN1"; @@ -268,6 +269,15 @@ private static String roomKey(String roomName) return BUILDING_ID + "-" + roomName; } + /** + * The location a cage trigger derives for the given room and cage. Mirrors ehr_lookups/cage.js. Note that + * ehr_lookups.cage.location is varchar(24), so these keys cannot grow much beyond the fixture's short names. + */ + private static String cageLocation(String roomName, String cageName) + { + return roomKey(roomName) + "-" + cageName; + } + /** * Housing records store the derived room key, so the fixture rooms have to be named by key for the room * lookups to resolve. The base implementation returns names that match no room in this study. @@ -379,12 +389,13 @@ private void populateLocations() throws IOException, CommandException roomCmd.addRow(Map.of("name", "R3", "building", BUILDING_ID)); roomCmd.execute(getApiHelper().getConnection(), getContainerPath()); + // 'location' is left out so the cage trigger derives it, exercising the same path production entry takes. log("Inserting values in cage"); InsertRowsCommand cageCmd = new InsertRowsCommand("ehr_lookups", "cage"); - cageCmd.addRow(Map.of("location", "L1", "cage", "C1", "room", roomKey("R1"))); - cageCmd.addRow(Map.of("location", "L2", "cage", "C2", "room", roomKey("R1"))); - cageCmd.addRow(Map.of("location", "L3", "cage", "C3", "room", roomKey("R2"))); - cageCmd.addRow(Map.of("location", "L4", "cage", "C4", "room", roomKey("R3"))); + cageCmd.addRow(Map.of("cage", "C1", "room", roomKey("R1"))); + cageCmd.addRow(Map.of("cage", "C2", "room", roomKey("R1"))); + cageCmd.addRow(Map.of("cage", "C3", "room", roomKey("R2"))); + cageCmd.addRow(Map.of("cage", "C4", "room", roomKey("R3"))); cageCmd.execute(getApiHelper().getConnection(), getContainerPath()); } @@ -1631,8 +1642,9 @@ public void testGroupPenCagemates() throws Exception roomCmd.addRow(Map.of("name", PEN_ROOM_NAME, "building", BUILDING_ID)); roomCmd.execute(getApiHelper().getConnection(), getContainerPath()); + // With no cage supplied the trigger derives the location as the room key alone. InsertRowsCommand penCmd = new InsertRowsCommand("ehr_lookups", "cage"); - penCmd.addRow(Map.of("location", penRoom, "room", penRoom)); + penCmd.addRow(Map.of("room", penRoom)); penCmd.execute(getApiHelper().getConnection(), getContainerPath()); log("Housing two animals in the pen"); From 85050c756604d69c8303940bf90dcd74e385b301 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sun, 2 Aug 2026 13:37:39 -0600 Subject: [PATCH 8/8] Tighten location key derivation and cagemate matching Cagemates match on the cage where there is one and fall back to the room only for group pens, so a record entered without a room still resolves. Derived room, building, and cage keys are persisted the same way as the cage location and rejected when they cannot be stored or would collide, and floor is kept out of the room views. --- .../queries/ehr_lookups/buildings.js | 25 ++++ .../resources/queries/ehr_lookups/cage.js | 15 ++- .../resources/queries/ehr_lookups/rooms.js | 21 ++- .../queries/ehr_lookups/rooms.query.xml | 8 ++ .../queries/ehr_lookups/rooms/.qview.xml | 2 +- .../queries/study/demographicsCagemates.sql | 6 +- .../tests.nbri_ehr/NBRI_EHRTest.java | 121 +++++++++++++++--- 7 files changed, 170 insertions(+), 28 deletions(-) diff --git a/nbri_ehr/resources/queries/ehr_lookups/buildings.js b/nbri_ehr/resources/queries/ehr_lookups/buildings.js index dd9066f..b5e9890 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/buildings.js +++ b/nbri_ehr/resources/queries/ehr_lookups/buildings.js @@ -7,6 +7,19 @@ var LABKEY = require("labkey"); var triggerHelper = new org.labkey.nbri_ehr.query.NBRI_EHRTriggerHelper(LABKEY.Security.currentUser.id, LABKEY.Security.currentContainer.id); +// Width of ehr_lookups.buildings.name. The description it is derived from is a wider column, so it can overrun the +// key; reject it here rather than letting the database raise an unreadable error. +var MAX_NAME_LENGTH = 100; + +// 'name' is not user editable, so it is absent from the incoming row map and the value this script derives has +// nowhere to land. Declaring it managed reserves a slot so the derived key is persisted. +function managedColumns() { + return { + insert: ["name"], + update: ["name"], + }; +} + function onUpsert(row, oldRow, errors){ if (extraContext.dataSource != "etl") { if (!row.description) { @@ -25,6 +38,18 @@ function onUpsert(row, oldRow, errors){ return; } + if (row.description.length > MAX_NAME_LENGTH) { + errors['description'] = 'Description is too long: it becomes the building key, which cannot exceed ' + MAX_NAME_LENGTH + ' characters.'; + return; + } + + // The description alone identifies the building now that the area is no longer folded in, so a duplicate + // would collide on the key. Say so here instead of surfacing a constraint violation on a hidden column. + if (triggerHelper.totalRecords("ehr_lookups", "buildings", "name", row.description) > 0) { + errors['description'] = 'A building described as ' + row.description + ' already exists. Building descriptions must be unique.'; + return; + } + row.name = row.description; } } diff --git a/nbri_ehr/resources/queries/ehr_lookups/cage.js b/nbri_ehr/resources/queries/ehr_lookups/cage.js index d374e86..0e71832 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/cage.js +++ b/nbri_ehr/resources/queries/ehr_lookups/cage.js @@ -7,6 +7,10 @@ var LABKEY = require("labkey"); var triggerHelper = new org.labkey.nbri_ehr.query.NBRI_EHRTriggerHelper(LABKEY.Security.currentUser.id, LABKEY.Security.currentContainer.id); +// Width of ehr_lookups.cage.location. The derived key builds on the room key, which is itself derived, so it can +// overrun the column; reject it here rather than letting the database raise an unreadable error. +var MAX_LOCATION_LENGTH = 100; + // 'location' is not user editable, so it is absent from the incoming row map and the value this script // derives has nowhere to land. Declaring it managed reserves a slot so the derived key is persisted. function managedColumns() { @@ -29,9 +33,16 @@ function onUpsert(row, oldRow, errors){ return; } - row.location = row.room; + let location = row.room; if (row.cage) - row.location += '-' + row.cage; + location += '-' + row.cage; + + if (location.length > MAX_LOCATION_LENGTH) { + errors['cage'] = 'Room and cage are too long: they combine to a ' + location.length + ' character location key, which cannot exceed ' + MAX_LOCATION_LENGTH + '.'; + return; + } + + row.location = location; } } } diff --git a/nbri_ehr/resources/queries/ehr_lookups/rooms.js b/nbri_ehr/resources/queries/ehr_lookups/rooms.js index a35d8de..18a10a2 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/rooms.js +++ b/nbri_ehr/resources/queries/ehr_lookups/rooms.js @@ -8,6 +8,19 @@ var console = require("console"); var triggerHelper = new org.labkey.nbri_ehr.query.NBRI_EHRTriggerHelper(LABKEY.Security.currentUser.id, LABKEY.Security.currentContainer.id); +// Width of ehr_lookups.rooms.room. The derived key is built from values the user supplies, so it can overrun the +// column; reject it here rather than letting the database raise an unreadable error. +var MAX_ROOM_LENGTH = 100; + +// 'room' is not user editable, so it is absent from the incoming row map and the value this script derives has +// nowhere to land. Declaring it managed reserves a slot so the derived key is persisted. +function managedColumns() { + return { + insert: ["room"], + update: ["room"], + }; +} + function onUpsert(row, oldRow, errors){ if (extraContext.dataSource != "etl") { if (!row.name) { @@ -26,7 +39,13 @@ function onUpsert(row, oldRow, errors){ return; } - row.room = row.building + '-' + row.name; + let room = row.building + '-' + row.name; + if (room.length > MAX_ROOM_LENGTH) { + errors['name'] = 'Building and room name are too long: they combine to a ' + room.length + ' character room key, which cannot exceed ' + MAX_ROOM_LENGTH + '.'; + return; + } + + row.room = room; } } } diff --git a/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml b/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml index a58dd7a..0b522c5 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml +++ b/nbri_ehr/resources/queries/ehr_lookups/rooms.query.xml @@ -19,6 +19,14 @@ false false + + + true + false + false + false +
diff --git a/nbri_ehr/resources/queries/ehr_lookups/rooms/.qview.xml b/nbri_ehr/resources/queries/ehr_lookups/rooms/.qview.xml index 0bfbf5b..f8c6e5e 100644 --- a/nbri_ehr/resources/queries/ehr_lookups/rooms/.qview.xml +++ b/nbri_ehr/resources/queries/ehr_lookups/rooms/.qview.xml @@ -1,6 +1,6 @@ - + \ No newline at end of file diff --git a/nbri_ehr/resources/queries/study/demographicsCagemates.sql b/nbri_ehr/resources/queries/study/demographicsCagemates.sql index c8e2529..4219925 100644 --- a/nbri_ehr/resources/queries/study/demographicsCagemates.sql +++ b/nbri_ehr/resources/queries/study/demographicsCagemates.sql @@ -22,9 +22,9 @@ SELECT FROM study.housing h JOIN study.housing h2 --- cage is null for group/pen rooms, whose location is the room alone, so the room match bounds the null case -ON ((h.cage = h2.cage OR (h.cage IS NULL AND h2.cage IS NULL)) - AND h.room = h2.room +-- cage holds a location key that already encodes the room, so caged animals match on cage alone. Group/pen rooms have +-- no cage, so those fall back to the room, which is only consulted when neither side has a cage. +ON ((h.cage = h2.cage OR (h.cage IS NULL AND h2.cage IS NULL AND h.room = h2.room)) AND h2.Id.demographics.calculated_status = 'Alive' AND h2.enddateTimeCoalesced >= now() AND h2.qcstate.publicdata = true) diff --git a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java index 830b3b8..487d1f2 100644 --- a/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java +++ b/nbri_ehr/test/src/org.labkey.test/tests.nbri_ehr/NBRI_EHRTest.java @@ -124,6 +124,13 @@ public class NBRI_EHRTest extends AbstractGenericEHRTest implements PostgresOnly private static final String PEN_ROOM_NAME = "PEN1"; private static final String[] PEN_ANIMALS = {"PEN0001", "PEN0002"}; + // Housed with a cage but no room, which is how a record entered against a cage alone lands. Every cage seeded by + // populateLocations already has occupants from datasetHousing.tsv, so testCagematesWithoutRoom creates its own to + // keep the expected cagemate count exact. + private static final String ROOMLESS_CAGE_NAME = "C9"; + private static final String ROOMLESS_CAGE = cageLocation("R1", ROOMLESS_CAGE_NAME); + private static final String[] ROOMLESS_ANIMALS = {"CAGE0001", "CAGE0002"}; + private final String[] weightFields = {"Id", "date", "enddate", "project", "weight", FIELD_QCSTATELABEL, FIELD_OBJECTID, FIELD_LSID, "_recordid", "performedby"}; private final Object[] weightData1 = {getExpectedAnimalIDCasing("TESTSUBJECT1"), EHRClientAPIHelper.DATE_SUBSTITUTION, null, null, "12", EHRQCState.IN_PROGRESS.label, null, null, "_recordID", 1004}; @@ -270,8 +277,7 @@ private static String roomKey(String roomName) } /** - * The location a cage trigger derives for the given room and cage. Mirrors ehr_lookups/cage.js. Note that - * ehr_lookups.cage.location is varchar(24), so these keys cannot grow much beyond the fixture's short names. + * The location a cage trigger derives for the given room and cage. Mirrors ehr_lookups/cage.js. */ private static String cageLocation(String roomName, String cageName) { @@ -1632,6 +1638,26 @@ public void testRoomRequiresBuilding() } } + @Test + public void testDuplicateBuildingRejected() + { + log("Verifying a second building cannot reuse an existing description"); + // The description alone is the building key now, so reusing the seeded building's description would collide. + InsertRowsCommand insertCmd = new InsertRowsCommand("ehr_lookups", "buildings"); + insertCmd.addRow(Map.of("description", BUILDING_ID, "area", BUILDING_AREA)); + + try + { + insertCmd.execute(getApiHelper().getConnection(), getContainerPath()); + Assert.fail("Building insert should have been rejected when the description was already in use"); + } + catch (IOException | CommandException e) + { + Assert.assertTrue("Unexpected failure inserting a duplicate building: " + e.getMessage(), + e.getMessage() != null && e.getMessage().contains("already exists")); + } + } + @Test public void testGroupPenCagemates() throws Exception { @@ -1647,35 +1673,88 @@ public void testGroupPenCagemates() throws Exception penCmd.addRow(Map.of("room", penRoom)); penCmd.execute(getApiHelper().getConnection(), getContainerPath()); - log("Housing two animals in the pen"); + createAliveAnimals(PEN_ANIMALS); + + // The cage is deliberately left null: a penned animal is housed against the room, which is the case the + // cagemates query has to bound by room rather than by cage. + log("Housing two animals in the pen, with no cage"); + houseAnimals(PEN_ANIMALS, penRoom, null); + + log("Verifying penned animals resolve as each other's cagemates"); + assertCagemates(PEN_ANIMALS[0], 2, PEN_ANIMALS[1]); + } + + @Test + public void testCagematesWithoutRoom() throws Exception + { + log("Creating an unoccupied cage"); + InsertRowsCommand cageCmd = new InsertRowsCommand("ehr_lookups", "cage"); + cageCmd.addRow(Map.of("cage", ROOMLESS_CAGE_NAME, "room", roomKey("R1"))); + cageCmd.execute(getApiHelper().getConnection(), getContainerPath()); + + createAliveAnimals(ROOMLESS_ANIMALS); + + // The cage is a location key that already names its room, so the room is redundant here and nothing requires + // it. Cagemates must still resolve when it is absent. + log("Housing two animals in the same cage, with no room"); + houseAnimals(ROOMLESS_ANIMALS, null, ROOMLESS_CAGE); + + log("Verifying caged animals resolve as each other's cagemates without a room"); + assertCagemates(ROOMLESS_ANIMALS[0], 2, ROOMLESS_ANIMALS[1]); + } + + /** + * Creates living demographics records for the given animals, replacing any left behind by an earlier run. + */ + private void createAliveAnimals(String[] animalIds) throws Exception + { + log("Creating animals " + StringUtils.join(animalIds, ", ")); String[] fields = new String[]{"Id", "Species", "Birth", "Gender", "date", "calculated_status", "objectid", "performedby"}; - Object[][] data = new Object[][]{ - {PEN_ANIMALS[0], "Rhesus", (new Date()).toString(), getMale(), new Date(), "Alive", UUID.randomUUID().toString(), 1004}, - {PEN_ANIMALS[1], "Rhesus", (new Date()).toString(), getMale(), new Date(), "Alive", UUID.randomUUID().toString(), 1004} - }; + Object[][] data = new Object[animalIds.length][]; + for (int i = 0; i < animalIds.length; i++) + { + data[i] = new Object[]{animalIds[i], "Rhesus", (new Date()).toString(), getMale(), new Date(), "Alive", UUID.randomUUID().toString(), 1004}; + } + SimplePostCommand insertCommand = getApiHelper().prepareInsertCommand("study", "demographics", "lsid", fields, data); - getApiHelper().deleteAllRecords("study", "demographics", new Filter("Id", StringUtils.join(PEN_ANIMALS, ";"), Filter.Operator.IN)); + getApiHelper().deleteAllRecords("study", "demographics", new Filter("Id", StringUtils.join(animalIds, ";"), Filter.Operator.IN)); getApiHelper().doSaveRows(DATA_ADMIN.getEmail(), insertCommand, getExtraContext()); + } - fields = new String[]{"Id", "date", "enddate", "room", "cage", "QCStateLabel", "performedby"}; - data = new Object[][]{ - {PEN_ANIMALS[0], new Date(), null, penRoom, penRoom, EHRQCState.COMPLETED.label, 1004}, - {PEN_ANIMALS[1], new Date(), null, penRoom, penRoom, EHRQCState.COMPLETED.label, 1004} - }; - insertCommand = getApiHelper().prepareInsertCommand("study", "Housing", "lsid", fields, data); - getApiHelper().deleteAllRecords("study", "Housing", new Filter("Id", StringUtils.join(PEN_ANIMALS, ";"), Filter.Operator.IN)); + /** + * Opens a completed housing record for each animal at the given location, replacing any left behind by an earlier + * run. Either the room or the cage may be null, which is how records entered against one alone land. + */ + private void houseAnimals(String[] animalIds, String room, String cage) throws Exception + { + String[] fields = new String[]{"Id", "date", "enddate", "room", "cage", "QCStateLabel", "performedby"}; + Object[][] data = new Object[animalIds.length][]; + for (int i = 0; i < animalIds.length; i++) + { + data[i] = new Object[]{animalIds[i], new Date(), null, room, cage, EHRQCState.COMPLETED.label, 1004}; + } + + SimplePostCommand insertCommand = getApiHelper().prepareInsertCommand("study", "Housing", "lsid", fields, data); + getApiHelper().deleteAllRecords("study", "Housing", new Filter("Id", StringUtils.join(animalIds, ";"), Filter.Operator.IN)); getApiHelper().doSaveRows(DATA_ADMIN.getEmail(), insertCommand, getExtraContext()); + } - log("Verifying penned animals resolve as each other's cagemates"); + /** + * Asserts the cagemates report resolves the expected companions for an animal. A null total means the join matched + * nothing at all, which is reported as its own failure rather than as an unexpected count. + */ + private void assertCagemates(String animalId, int expectedTotal, String expectedCompanion) throws Exception + { SelectRowsCommand selectCmd = new SelectRowsCommand("study", "demographicsCagemates"); - selectCmd.addFilter(new Filter("Id", PEN_ANIMALS[0])); + selectCmd.addFilter(new Filter("Id", animalId)); SelectRowsResponse response = selectCmd.execute(getApiHelper().getConnection(), getContainerPath()); - Assert.assertEquals("Expected one cagemates row for " + PEN_ANIMALS[0], 1, response.getRows().size()); + Assert.assertEquals("Expected one cagemates row for " + animalId, 1, response.getRows().size()); Map cagemates = response.getRows().get(0); - Assert.assertEquals("Both penned animals should be counted", 2, ((Number) cagemates.get("total")).intValue()); - Assert.assertTrue("Cagemate list should name the other penned animal, was: " + cagemates.get("animals"), - String.valueOf(cagemates.get("animals")).contains(PEN_ANIMALS[1])); + Assert.assertNotNull("Cagemates resolved no location for " + animalId, cagemates.get("total")); + Assert.assertEquals("Unexpected cagemate count for " + animalId, expectedTotal, ((Number) cagemates.get("total")).intValue()); + Assert.assertTrue("Cagemate list should name " + expectedCompanion + ", was: " + cagemates.get("animals"), + String.valueOf(cagemates.get("animals")).contains(expectedCompanion)); } @Test