From 26957179cec2adf41f48b349c64942d3cd70ad9c Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Fri, 31 Jul 2026 13:30:41 -0600 Subject: [PATCH 1/4] Add Bulk Deaths form and death type lookup, fix departure destination fk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds NBRIBulkDeathFormType, an admin-only grid form for entering completed historical deaths in bulk, with a Death.js model source supplying its grid metadata. Adds a `type` (Death Type) column to `study.deaths`, backed by a new `ehr_lookups.death_type` lookup of the 13 codes the source data uses. Note that `A` and `X` share the title `Experimental`, so the title is ambiguous on import and only the code round-trips reliably. `DeathNecropsy.js` now requires `type` rather than `reason`, and `deathWeight` becomes optional — not every death record carries a weight. `NBRI_EHRTriggerHelper.upsertWeightRecord` gains an `announceChanges` overload so a caller writing many weight rows in one transaction can suppress the per-row participant announcement and announce once at completion. Its only caller is `study/deaths.js`, which is not part of this branch. `study.departure.destination` resolves through the lookup's `code` column instead of `meaning`, matching how the values are stored. --- nbri_ehr/resources/data/death_type.tsv | 14 ++++ nbri_ehr/resources/data/editable_lookups.tsv | 1 + nbri_ehr/resources/data/lookup_sets.tsv | 1 + nbri_ehr/resources/data/lookupsManifest.tsv | 1 + .../resources/data/lookupsManifestTest.tsv | 1 + .../resources/queries/study/deaths.query.xml | 9 +++ .../resources/queries/study/deaths/.qview.xml | 1 + .../queries/study/departure.query.xml | 2 +- .../study/datasets/datasets_metadata.xml | 3 + .../web/nbri_ehr/model/sources/Death.js | 54 +++++++++++++++ .../nbri_ehr/model/sources/DeathNecropsy.js | 8 +-- .../org/labkey/nbri_ehr/NBRI_EHRModule.java | 1 + .../dataentry/form/NBRIBulkDeathFormType.java | 66 +++++++++++++++++++ .../nbri_ehr/query/NBRI_EHRTriggerHelper.java | 18 ++++- .../tests.nbri_ehr/NBRI_EHRTest.java | 1 + 15 files changed, 173 insertions(+), 8 deletions(-) create mode 100644 nbri_ehr/resources/data/death_type.tsv create mode 100644 nbri_ehr/resources/web/nbri_ehr/model/sources/Death.js create mode 100644 nbri_ehr/src/org/labkey/nbri_ehr/dataentry/form/NBRIBulkDeathFormType.java diff --git a/nbri_ehr/resources/data/death_type.tsv b/nbri_ehr/resources/data/death_type.tsv new file mode 100644 index 0000000..700a88d --- /dev/null +++ b/nbri_ehr/resources/data/death_type.tsv @@ -0,0 +1,14 @@ +value title sort_order +A Experimental 1 +D Spontaneous/Normal 2 +F Fetal 3 +FD Fetal Death 4 +FL Fetal Live 5 +FN Fetal found at necropsy 6 +FX Fetal experimental 7 +K Cull (scheduled) 8 +M Medical cull (non-scheduled) 9 +ND Non-vaginal (C-section) dead 10 +NT Not pregnant at assessment 11 +S Cull 12 +X Experimental 13 \ No newline at end of file diff --git a/nbri_ehr/resources/data/editable_lookups.tsv b/nbri_ehr/resources/data/editable_lookups.tsv index 90cc8af..1875f3d 100644 --- a/nbri_ehr/resources/data/editable_lookups.tsv +++ b/nbri_ehr/resources/data/editable_lookups.tsv @@ -37,6 +37,7 @@ ehr_lookups country_category Colony Management Country Category ehr_lookups daily_enrich_codes Behavior Daily enrichment codes. ehr_lookups data_category Clinical Data Categories Used in datasets. ehr_lookups death_reason Colony Management Death Reason +ehr_lookups death_type Colony Management Death Type Death type codes. ehr_lookups delivery_state Colony Management Delivery State ehr_lookups dental_obs Clinical Dental Observation Types Clinical observation values. ehr_lookups derm_obs Clinical Dermatologic Observation Types Clinical observation values. diff --git a/nbri_ehr/resources/data/lookup_sets.tsv b/nbri_ehr/resources/data/lookup_sets.tsv index 613eb8e..d9e5c93 100644 --- a/nbri_ehr/resources/data/lookup_sets.tsv +++ b/nbri_ehr/resources/data/lookup_sets.tsv @@ -31,6 +31,7 @@ country_category Country Category value title daily_enrich_codes Daily Enrichment Codes value data_category Data Category Field Values value death_reason Death Reason value +death_type Death Type value title delivery_state Delivery State value title dental_obs Dental Observations value derm_obs Dermatologic Observations value diff --git a/nbri_ehr/resources/data/lookupsManifest.tsv b/nbri_ehr/resources/data/lookupsManifest.tsv index defb466..1c8b08d 100644 --- a/nbri_ehr/resources/data/lookupsManifest.tsv +++ b/nbri_ehr/resources/data/lookupsManifest.tsv @@ -35,6 +35,7 @@ country_category daily_enrich_codes data_category death_reason +death_type delivery_state dental_obs derm_obs diff --git a/nbri_ehr/resources/data/lookupsManifestTest.tsv b/nbri_ehr/resources/data/lookupsManifestTest.tsv index b2006c8..c0699c9 100644 --- a/nbri_ehr/resources/data/lookupsManifestTest.tsv +++ b/nbri_ehr/resources/data/lookupsManifestTest.tsv @@ -34,6 +34,7 @@ country_category daily_enrich_codes data_category death_reason +death_type delivery_state dental_obs derm_obs diff --git a/nbri_ehr/resources/queries/study/deaths.query.xml b/nbri_ehr/resources/queries/study/deaths.query.xml index 35c2e6a..e28e2f0 100644 --- a/nbri_ehr/resources/queries/study/deaths.query.xml +++ b/nbri_ehr/resources/queries/study/deaths.query.xml @@ -7,6 +7,15 @@ Death Date + + Death Type + + ehr_lookups + death_type + value + title + + Disposition diff --git a/nbri_ehr/resources/queries/study/deaths/.qview.xml b/nbri_ehr/resources/queries/study/deaths/.qview.xml index fdc063c..56eecec 100644 --- a/nbri_ehr/resources/queries/study/deaths/.qview.xml +++ b/nbri_ehr/resources/queries/study/deaths/.qview.xml @@ -6,6 +6,7 @@ + diff --git a/nbri_ehr/resources/queries/study/departure.query.xml b/nbri_ehr/resources/queries/study/departure.query.xml index 19845d1..d5d0f5c 100644 --- a/nbri_ehr/resources/queries/study/departure.query.xml +++ b/nbri_ehr/resources/queries/study/departure.query.xml @@ -13,7 +13,7 @@ ehr_lookups source - meaning + code diff --git a/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml b/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml index 55e48ea..cd39c22 100644 --- a/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml +++ b/nbri_ehr/resources/referenceStudy/study/datasets/datasets_metadata.xml @@ -530,6 +530,9 @@ double + + varchar + diff --git a/nbri_ehr/resources/web/nbri_ehr/model/sources/Death.js b/nbri_ehr/resources/web/nbri_ehr/model/sources/Death.js new file mode 100644 index 0000000..568a410 --- /dev/null +++ b/nbri_ehr/resources/web/nbri_ehr/model/sources/Death.js @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0 + */ +/** + * Metadata for the grid-based Bulk Deaths form. The columnConfig widths only take effect in the grid; they are ignored + * when the same fields render in a form panel. + */ +EHR.model.DataModelManager.registerMetadata('Death', { + allQueries: { + }, + byQuery: { + 'study.deaths': { + qcstate: { + hidden: true + }, + date: { + xtype: 'xdatetime', + editorConfig: { + dateFormat: 'Y-m-d', + timeFormat: 'H:i' + }, + columnConfig: { + width: 160 + } + }, + deathWeight: { + label: 'Weight (kg)', + columnConfig: { + width: 150 + } + }, + type: { + allowBlank: false, + nullable: false, + columnConfig: { + width: 160 + } + }, + reason: { + columnConfig: { + width: 160 + } + }, + remark: { + xtype: 'ehr-remarkfield', + columnConfig: { + width: 200 + } + } + } + } +}); diff --git a/nbri_ehr/resources/web/nbri_ehr/model/sources/DeathNecropsy.js b/nbri_ehr/resources/web/nbri_ehr/model/sources/DeathNecropsy.js index cb988e9..2f7b703 100644 --- a/nbri_ehr/resources/web/nbri_ehr/model/sources/DeathNecropsy.js +++ b/nbri_ehr/resources/web/nbri_ehr/model/sources/DeathNecropsy.js @@ -19,13 +19,11 @@ EHR.model.DataModelManager.registerMetadata('DeathNecropsy', { }, }, deathWeight: { - label: 'Weight (kg)', - allowBlank: false, - nullable: false, + label: 'Weight (kg)' }, - reason: { + type: { allowBlank: false, - nullable: false, + nullable: false } }, 'study.necropsy': { diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/NBRI_EHRModule.java b/nbri_ehr/src/org/labkey/nbri_ehr/NBRI_EHRModule.java index 5339e54..aa76225 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/NBRI_EHRModule.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/NBRI_EHRModule.java @@ -212,6 +212,7 @@ private void registerDataEntry() EHRService.get().registerFormType(new DefaultDataEntryFormFactory(NBRIBulkClinicalFormType.class, this)); EHRService.get().registerFormType(new DefaultDataEntryFormFactory(NBRIDepartureFormType.class, this)); EHRService.get().registerFormType(new DefaultDataEntryFormFactory(NBRIDeathNecropsyFormType.class, this)); + EHRService.get().registerFormType(new DefaultDataEntryFormFactory(NBRIBulkDeathFormType.class, this)); EHRService.get().registerFormType(new DefaultDataEntryFormFactory(NBRIHousingFormType.class, this)); EHRService.get().registerFormType(new DefaultDataEntryFormFactory(NBRIMedicationTreatmentFormType.class, this)); EHRService.get().registerFormType(new DefaultDataEntryFormFactory(NBRIProjectFormType.class, this)); diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/form/NBRIBulkDeathFormType.java b/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/form/NBRIBulkDeathFormType.java new file mode 100644 index 0000000..5d95343 --- /dev/null +++ b/nbri_ehr/src/org/labkey/nbri_ehr/dataentry/form/NBRIBulkDeathFormType.java @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.nbri_ehr.dataentry.form; + +import org.labkey.api.ehr.EHRService; +import org.labkey.api.ehr.dataentry.DataEntryFormContext; +import org.labkey.api.ehr.dataentry.FormSection; +import org.labkey.api.ehr.security.EHRCompletedInsertPermission; +import org.labkey.api.module.Module; +import org.labkey.api.security.permissions.AdminPermission; +import org.labkey.api.view.template.ClientDependency; +import org.labkey.nbri_ehr.dataentry.section.BaseFormSection; +import org.labkey.nbri_ehr.dataentry.section.NBRIAnimalDetailsFormSection; +import org.labkey.nbri_ehr.dataentry.section.NBRITaskFormSection; + +import java.util.Arrays; + +/** + * Admin-only form that records deaths only, as a grid so several animals can be entered at once. + */ +public class NBRIBulkDeathFormType extends NBRIBaseTaskFormType +{ + public static final String NAME = "BulkDeaths"; + public static final String LABEL = "Bulk Deaths"; + + public NBRIBulkDeathFormType(DataEntryFormContext ctx, Module owner) + { + super(ctx, owner, NAME, LABEL, "Colony Management", Arrays.asList( + new NBRITaskFormSection(), + new NBRIAnimalDetailsFormSection(), + new BaseFormSection("study", "deaths", "Deaths", EHRService.FORM_SECTION_LOCATION.Body, true, false) + )); + + addClientDependency(ClientDependency.supplierFromPath("nbri_ehr/model/sources/Death.js")); + + for (FormSection s : getFormSections()) + { + s.addConfigSource("Death"); + } + } + + @Override + public boolean isAvailable() + { + return super.isAvailable() && getCtx().getContainer().hasPermission(getCtx().getUser(), AdminPermission.class); + } + + @Override + protected boolean canInsert() + { + return EHRService.get().hasPermission("study", "deaths", getCtx().getContainer(), getCtx().getUser(), EHRCompletedInsertPermission.class); + } +} diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java b/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java index a61a5fc..5e1eff9 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java @@ -324,6 +324,16 @@ public boolean deathExists(String id) } public void upsertWeightRecord(Map row) throws QueryUpdateServiceException, DuplicateKeyException, SQLException, BatchValidationException, InvalidKeyException + { + upsertWeightRecord(row, true); + } + + /** + * When announceChanges is false, the nested weight trigger will not announce the modified id + * (skipAnnounceChangedParticipants). Callers must mark study.weight as modified on the outer helper + * (addTableModified) so the single announcement at trigger completion covers it. + */ + public void upsertWeightRecord(Map row, boolean announceChanges) throws QueryUpdateServiceException, DuplicateKeyException, SQLException, BatchValidationException, InvalidKeyException { BatchValidationException errors = new BatchValidationException(); Date date = ConvertHelper.convert(row.get("date"), Date.class); @@ -362,13 +372,17 @@ public void upsertWeightRecord(Map row) throws QueryUpdateServic List> rows = new ArrayList<>(); rows.add(saveRow); + Map context = getExtraContext(); + if (!announceChanges) + context.put("skipAnnounceChangedParticipants", true); + if (updateRecord) { - ti.getUpdateService().updateRows(_user, _container, rows, null, null, getExtraContext()); + ti.getUpdateService().updateRows(_user, _container, rows, null, null, context); } else { - ti.getUpdateService().insertRows(_user, _container, rows, errors, null, getExtraContext()); + ti.getUpdateService().insertRows(_user, _container, rows, errors, null, context); } if (errors.hasErrors()) 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 c7587ae..89590a3 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 @@ -1185,6 +1185,7 @@ public void testDeathNecropsyForm() throws IOException, CommandException waitForText("Id: ERROR: Animal is not at the center."); setFormElement(Locator.name("Id"), aliveAnimalId); + _ext4Helper.selectComboBoxItem("Death Type:", "Spontaneous/Normal"); _ext4Helper.selectComboBoxItem("Disposition:", "Euthaniasia (project)"); waitForElement(Locator.name("deathWeight")); setFormElement(Locator.name("deathWeight"), "23"); From 1962aab0345bcdb646405da550cd055e3c016272 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Fri, 31 Jul 2026 19:59:21 -0600 Subject: [PATCH 2/4] Harden the deaths trigger against duplicate and unknown-animal rows The duplicate checks dereferenced `deathIdMap[row.Id].QCStateLabel` and `row.QCStateLabel` without guards, so a death row for an animal with no prior death record threw a TypeError out of the trigger instead of validating. Every lookup now goes through a null-safe local, and record existence is tested rather than QC state, since ETL- and import-sourced rows can carry a null QCState. study.deaths is demographic, so a second row for an animal hit the unique constraint and surfaced as a database error. Two new checks report it as a validation error instead: one against the pre-save snapshot for rows already in the table, and one against a helper property tracking animals saved earlier in the same transaction, which the snapshot cannot see. Only rows that passed validation are recorded there, so a duplicate of a failing row still reports that row's underlying error. An Id absent from demographics previously fell through silently; inserts now reject it. Updates keep their prior behavior. The weight upsert moves inside the success branch and is gated on !isValidateOnly, so validation passes and failed rows no longer write weight records. It suppresses the per-row participant announcement and calls addTableModified instead, letting the framework announce once for the save. Drops the unused validIds array and the dead `validIds.indexOf(row.id)` check, which compared against a lowercase property that never exists. NBRI_EHRTest covers the unknown-Id and existing-record rejections in the Death form, and asserts through the API that a second death insert returns the validation error rather than a constraint violation. --- nbri_ehr/resources/queries/study/deaths.js | 91 ++++++++++++------- .../tests.nbri_ehr/NBRI_EHRTest.java | 17 ++++ 2 files changed, 74 insertions(+), 34 deletions(-) diff --git a/nbri_ehr/resources/queries/study/deaths.js b/nbri_ehr/resources/queries/study/deaths.js index 79883b4..abe7bc4 100644 --- a/nbri_ehr/resources/queries/study/deaths.js +++ b/nbri_ehr/resources/queries/study/deaths.js @@ -6,7 +6,6 @@ require("ehr/triggers").initScript(this); var triggerHelper = new org.labkey.nbri_ehr.query.NBRI_EHRTriggerHelper(LABKEY.Security.currentUser.id, LABKEY.Security.currentContainer.id); -var validIds = []; var idMap = {}; var deathIdMap = {}; @@ -26,9 +25,7 @@ function onInit(event, helper){ return; for(var i=0; i < results.rows.length; i++) { - validIds.push(results.rows[i]["Id"]["value"]) idMap[results.rows[i]["Id"]["value"]] = {calculated_status: results.rows[i]["calculated_status"]["value"], QCStateLabel: results.rows[i]["QCState/Label"]["value"]}; - // console.log(idMap[results.rows[i]["Id"]["value"]]); } }, failure: function (error) { @@ -84,13 +81,27 @@ function onUpsert(helper, scriptErrors, row, oldRow) { //only allow death record to be created if the animal is in the demographics table if (idMap[row.Id]) { + // deathIdMap has no entry for the animal on initial import, and any of these values can be null + var status = idMap[row.Id].calculated_status ? idMap[row.Id].calculated_status.toUpperCase() : null; + var priorDeathQCState = deathIdMap[row.Id] && deathIdMap[row.Id].QCStateLabel ? deathIdMap[row.Id].QCStateLabel.toUpperCase() : null; + var rowQCState = row.QCStateLabel ? row.QCStateLabel.toUpperCase() : null; + + // deathIdMap is a snapshot taken before any row was processed, so it cannot see earlier rows of this same + // save. Track them separately: study.deaths is demographic, so a second row for one animal cannot be saved. + var deathsInTransaction = helper.getProperty('deathsInTransaction') || {}; + + var errorMsg = null; + // check if a death record already exists for this animal - if (idMap[row.Id].calculated_status.toUpperCase() === 'DEAD' && deathIdMap[row.Id].QCStateLabel.toUpperCase() === 'COMPLETED') { - EHR.Server.Utils.addError(scriptErrors, 'Id', 'Death record already exists for this animal.', 'ERROR'); + if (status === 'DEAD' && priorDeathQCState === 'COMPLETED') { + errorMsg = 'Death record already exists for this animal.'; } // check if the animal is at the center - else if (idMap[row.Id].calculated_status.toUpperCase() === 'SHIPPED') { - EHR.Server.Utils.addError(scriptErrors, 'Id', 'Animal is not at the center.', 'ERROR'); + else if (status === 'SHIPPED') { + errorMsg = 'Animal is not at the center.'; + } + else if (deathsInTransaction[row.Id]) { + errorMsg = 'This animal is entered more than once. Only one death record per animal can be saved.'; } // Check if an animal that's being entered is pending any request/review. // Note 1: When trying to enter a new record for an animal, the QCState = 'IN PROGRESS'. @@ -98,23 +109,28 @@ function onUpsert(helper, scriptErrors, row, oldRow) { // the QCState will get set to 'Review Required' - this way we can distinguish between the two states in the Death/Necropsy workflow. // If a user tries to submit a new Death record (identified by QCState = 'IN PROGRESS') for an animal that // already has a pending request/review status in study.deaths, then below error message will be displayed. - else if (row.QCStateLabel.toUpperCase() === 'IN PROGRESS' && - deathIdMap[row.Id] && deathIdMap[row.Id].QCStateLabel && - (deathIdMap[row.Id].QCStateLabel.toUpperCase() === 'REQUEST: PENDING' || - deathIdMap[row.Id].QCStateLabel.toUpperCase() === 'REVIEW REQUIRED')) { - EHR.Server.Utils.addError(scriptErrors, 'Id', 'Death record is pending review for this animal', 'ERROR'); + else if (rowQCState === 'IN PROGRESS' && + (priorDeathQCState === 'REQUEST: PENDING' || priorDeathQCState === 'REVIEW REQUIRED')) { + errorMsg = 'Death record is pending review for this animal'; } // if 'Save Draft' record already exists, it doesn't allow to 'Save Draft' or 'Submit Death' // on the same animal again - throws an error "duplicate key value violates unique constraint" // So, added this check to allow 'Save Draft' record to be saved only once. - else if (oldRow === undefined && row.QCStateLabel.toUpperCase() === 'IN PROGRESS' && - deathIdMap[row.Id] && deathIdMap[row.Id].QCStateLabel && - deathIdMap[row.Id].QCStateLabel.toUpperCase() === 'IN PROGRESS') { - EHR.Server.Utils.addError(scriptErrors, 'Id', 'Death/Necropsy data entry is in progress for this animal', 'ERROR'); + else if (oldRow === undefined && rowQCState === 'IN PROGRESS' && priorDeathQCState === 'IN PROGRESS') { + errorMsg = 'Death/Necropsy data entry is in progress for this animal'; + } + // study.deaths is demographic (one row per animal), so any other new row for an animal with an existing + // record would fail on the unique constraint; report it as a validation error instead. Test record + // existence, not QC state: ETL/import-sourced rows can carry a null QCState. + else if (oldRow === undefined && deathIdMap[row.Id]) { + errorMsg = 'A death record already exists for this animal (' + (deathIdMap[row.Id].QCStateLabel || 'unknown state') + ').'; } - else if (!helper.isValidateOnly() && row.Id && row.date && row.QCStateLabel.toUpperCase() === 'COMPLETED') { - if (validIds.indexOf(row.id) !== -1) { + if (errorMsg) { + EHR.Server.Utils.addError(scriptErrors, 'Id', errorMsg, 'ERROR'); + } + else { + if (!helper.isValidateOnly() && row.Id && row.date && rowQCState === 'COMPLETED') { // update demographics demographicsUpdates.push({ @@ -128,26 +144,33 @@ function onUpsert(helper, scriptErrors, row, oldRow) { helper.getJavaHelper().updateDemographicsRecord(demographicsUpdates); console.log('updated demographics death date for animal: ' + row.Id); } - else { - console.log(row.id + " is not a valid animal id"); + + if (!helper.isValidateOnly() && row.date && row.QCStateLabel && EHR.Server.Security.getQCStateByLabel(row.QCStateLabel).PublicData) { + var qcstate = helper.getJavaHelper().getQCStateForLabel(row.QCStateLabel).getRowId(); + + //add/update weight record + var weightRecord = { + Id: row.Id, + date: row.date, + weight: row.deathWeight, + taskid: row.taskid, + qcstate: qcstate, + performedby: row.performedby + }; + if (triggerHelper.upsertWeightRecord(weightRecord, false)) { + helper.addTableModified('study', 'weight'); + } } - } - if(row.QCStateLabel && EHR.Server.Security.getQCStateByLabel(row.QCStateLabel).PublicData) { - var qcstate = helper.getJavaHelper().getQCStateForLabel(row.QCStateLabel).getRowId(); - - //add/update weight record - var weightRecord = { - Id: row.Id, - date: row.date, - weight: row.deathWeight, - taskid: row.taskid, - qcstate: qcstate, - performedby: row.performedby - }; - triggerHelper.upsertWeightRecord(weightRecord); + // mark only rows that passed, so a duplicate of a failed row reports that row's underlying error + deathsInTransaction[row.Id] = true; + helper.setProperty('deathsInTransaction', deathsInTransaction); } } + // insert-only: updates of existing death records keep their prior behavior + else if (oldRow === undefined) { + EHR.Server.Utils.addError(scriptErrors, 'Id', 'Id not found in the demographics table.', 'ERROR'); + } } } 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 89590a3..2154e5c 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 @@ -103,6 +103,8 @@ public class NBRI_EHRTest extends AbstractGenericEHRTest implements PostgresOnly private static final String deadAnimalId = "D5454"; private static final String departedAnimalId = "H6767"; private static final String aliveAnimalId = "A4545"; + // never inserted into demographics; exercises the deaths trigger's unknown-Id rejection + private static final String unknownAnimalId = "X9999"; // Dedicated animal for testScheduledObservationTaskGrouping; provisioned (alive, housed, assigned) in // createTestSubjects so the clinical case form raises no warnings that would keep the validation banner up. private static final String taskGroupAnimalId = "TESTGRP9090"; @@ -1184,6 +1186,12 @@ public void testDeathNecropsyForm() throws IOException, CommandException setFormElement(Locator.name("Id"), departedAnimalId); waitForText("Id: ERROR: Animal is not at the center."); + setFormElement(Locator.name("Id"), unknownAnimalId); + waitForText("Id: ERROR: Id not found in the demographics table."); + + setFormElement(Locator.name("Id"), deadAnimalId); + waitForText("Id: ERROR: Death record already exists for this animal."); + setFormElement(Locator.name("Id"), aliveAnimalId); _ext4Helper.selectComboBoxItem("Death Type:", "Spontaneous/Normal"); _ext4Helper.selectComboBoxItem("Disposition:", "Euthaniasia (project)"); @@ -1194,6 +1202,15 @@ public void testDeathNecropsyForm() throws IOException, CommandException submitForm("Submit Death", "Confirm"); stopImpersonating(); + log("Verify a second death insert is rejected with a validation error, not a unique constraint violation"); + SimplePostCommand duplicateDeath = getApiHelper().prepareInsertCommand("study", "deaths", "lsid", + new String[]{"Id", "date", "reason", "performedby"}, + new Object[][]{{aliveAnimalId, LocalDateTime.now(), "4", 1004}}); + CommandException duplicateError = getApiHelper().doSaveRowsExpectingError(DATA_ADMIN.getEmail(), duplicateDeath, getExtraContext()); + Map> duplicateErrors = getApiHelper().extractErrors(duplicateError.getProperties()); + Assert.assertTrue("Expected duplicate death validation error, got: " + duplicateErrors, + duplicateErrors.getOrDefault("Id", List.of()).contains("ERROR: A death record already exists for this animal (Request: Pending).")); + log("Trigger notifications"); goToEHRFolder(); NotificationAdminPage adminPage = NotificationAdminPage.beginAt(this); From 608a1afa2c5938494604162ae2029fd62952d90a Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Fri, 31 Jul 2026 20:01:31 -0600 Subject: [PATCH 3/4] Remove stray quote that left study.aliases unparseable A lone apostrophe followed the final UNION branch, so the query failed to parse. --- nbri_ehr/resources/queries/study/aliases.sql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nbri_ehr/resources/queries/study/aliases.sql b/nbri_ehr/resources/queries/study/aliases.sql index a1b279f..4ed9962 100644 --- a/nbri_ehr/resources/queries/study/aliases.sql +++ b/nbri_ehr/resources/queries/study/aliases.sql @@ -15,5 +15,4 @@ FROM nbri_ehr.IdHistory UNION SELECT Id, Alias as alias -FROM study.alias where Id.demographics.calculated_status != 'Alive - In Progress' -' \ No newline at end of file +FROM study.alias where Id.demographics.calculated_status != 'Alive - In Progress' \ No newline at end of file From bc64ce78ea426fc0be9d013050a9a1062fdefc18 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Fri, 31 Jul 2026 20:11:54 -0600 Subject: [PATCH 4/4] Handle an optional weight in upsertWeightRecord deathWeight is optional on the Death/Necropsy form and the caller in study/deaths.js does not gate the upsert on it, so the helper can be reached with no weight. It wrote the null through, leaving a study.weight row with no weight: the weight trigger only WARNs on that, and the default WARN threshold filters it, so the save succeeded and the empty row stayed. A weightless save now deletes a record left by an earlier save and otherwise writes nothing. Both overloads return whether a row was written, so the caller can call addTableModified only when there is a change to announce -- with announceChanges false the nested trigger skips the per-row participant announcement, and the outer helper announces once at completion. The existing-record lookup is skipped when taskid is null, making task-less entry insert-only. A null taskid filter flips to "taskid IS NULL" and would match unrelated historical weights for the animal. --- .../nbri_ehr/query/NBRI_EHRTriggerHelper.java | 70 ++++++++++++------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java b/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java index 5e1eff9..662ab54 100644 --- a/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java +++ b/nbri_ehr/src/org/labkey/nbri_ehr/query/NBRI_EHRTriggerHelper.java @@ -323,17 +323,19 @@ public boolean deathExists(String id) return false; } - public void upsertWeightRecord(Map row) throws QueryUpdateServiceException, DuplicateKeyException, SQLException, BatchValidationException, InvalidKeyException + public boolean upsertWeightRecord(Map row) throws QueryUpdateServiceException, DuplicateKeyException, SQLException, BatchValidationException, InvalidKeyException { - upsertWeightRecord(row, true); + return upsertWeightRecord(row, true); } /** * When announceChanges is false, the nested weight trigger will not announce the modified id * (skipAnnounceChangedParticipants). Callers must mark study.weight as modified on the outer helper * (addTableModified) so the single announcement at trigger completion covers it. + * + * @return whether a weight record was written; false when there was nothing to record */ - public void upsertWeightRecord(Map row, boolean announceChanges) throws QueryUpdateServiceException, DuplicateKeyException, SQLException, BatchValidationException, InvalidKeyException + public boolean upsertWeightRecord(Map row, boolean announceChanges) throws QueryUpdateServiceException, DuplicateKeyException, SQLException, BatchValidationException, InvalidKeyException { BatchValidationException errors = new BatchValidationException(); Date date = ConvertHelper.convert(row.get("date"), Date.class); @@ -341,25 +343,16 @@ public void upsertWeightRecord(Map row, boolean announceChanges) TableInfo ti = getTableInfo("study", "weight"); - // If there is already a weight record for this task, update that record - SimpleFilter filter = new SimpleFilter(FieldKey.fromString("Id"), row.get("Id")); - filter.addCondition(FieldKey.fromString("taskid"), taskId); - TableSelector ts = new TableSelector(ti, PageFlowUtil.set("lsid", "objectid"), filter, null); - boolean updateRecord = ts.exists(); - - Map saveRow = new CaseInsensitiveHashMap<>(); - saveRow.put("Id", row.get("Id")); - saveRow.put("date", date); - saveRow.put("taskid", taskId); - saveRow.put("qcstate", row.get("qcstate")); - saveRow.put("performedby", row.get("performedby")); - if (updateRecord) + // If there is already a weight record for this task, update that record. A null taskid filter flips to + // "taskid IS NULL" and would match unrelated historical weights, so task-less entry (e.g. a non-EHR bulk + // import form) is insert-only. + Map existingRecord = null; + if (taskId != null) { - saveRow.put("objectid", ts.getMap().get("objectid")); - } - else - { - saveRow.put("objectid", new GUID().toString()); + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("Id"), row.get("Id")); + filter.addCondition(FieldKey.fromString("taskid"), taskId); + TableSelector ts = new TableSelector(ti, PageFlowUtil.set("lsid", "objectid"), filter, null); + existingRecord = ts.getMap(); } Double weight = null; @@ -367,16 +360,39 @@ public void upsertWeightRecord(Map row, boolean announceChanges) { weight = ConvertHelper.convert(row.get("weight"), Double.class); } - saveRow.put("weight", weight); - - List> rows = new ArrayList<>(); - rows.add(saveRow); Map context = getExtraContext(); if (!announceChanges) context.put("skipAnnounceChangedParticipants", true); - if (updateRecord) + // Weight is optional, so with none entered there is nothing to record. Delete any record left by an earlier + // save rather than blanking it: the weight trigger only WARNs on a null weight and the default threshold + // filters that out, so the emptied record would survive the save. + if (weight == null) + { + if (existingRecord == null) + return false; + + Map keyRow = new CaseInsensitiveHashMap<>(); + keyRow.put("lsid", existingRecord.get("lsid")); + ti.getUpdateService().deleteRows(_user, _container, List.of(keyRow), null, context); + + return true; + } + + Map saveRow = new CaseInsensitiveHashMap<>(); + saveRow.put("Id", row.get("Id")); + saveRow.put("date", date); + saveRow.put("taskid", taskId); + saveRow.put("qcstate", row.get("qcstate")); + saveRow.put("performedby", row.get("performedby")); + saveRow.put("objectid", existingRecord != null ? existingRecord.get("objectid") : new GUID().toString()); + saveRow.put("weight", weight); + + List> rows = new ArrayList<>(); + rows.add(saveRow); + + if (existingRecord != null) { ti.getUpdateService().updateRows(_user, _container, rows, null, null, context); } @@ -387,6 +403,8 @@ public void upsertWeightRecord(Map row, boolean announceChanges) if (errors.hasErrors()) throw errors; + + return true; } public void clinicalMoveNotification(final String animalId, final String date)