From 5e326c89944c907687b502c48593a622d7a18aa2 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Sun, 12 Jul 2026 16:46:46 -0600 Subject: [PATCH 1/3] Block death/departure completion while animal data is in Review Required state Adds a study.studyDataReviewRequired module query over study.StudyData joined to core.QCState, and a NIRC_EHRTriggerHelper.getReviewRequiredDatasets helper that returns the dataset labels still in Review Required for an animal, excluding records on the completing record's own task since those move to Completed in the same save. The deaths and departure triggers call it when a record is being saved as Completed and raise an error naming the offending dataset(s). Covered by NIRC_EHRTest.testDeathAndDepartureBlockedByReviewRequiredData, which verifies both the blocked path and the own-task exclusion for each dataset via validate-only API saves. --- nirc_ehr/resources/queries/study/deaths.js | 10 ++++ nirc_ehr/resources/queries/study/departure.js | 15 +++++ .../queries/study/studyDataReviewRequired.sql | 9 +++ .../nirc_ehr/query/NIRC_EHRTriggerHelper.java | 21 ++++++- .../tests.nirc_ehr/NIRC_EHRTest.java | 55 +++++++++++++++++++ 5 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 nirc_ehr/resources/queries/study/studyDataReviewRequired.sql diff --git a/nirc_ehr/resources/queries/study/deaths.js b/nirc_ehr/resources/queries/study/deaths.js index 626223e7..27595e60 100644 --- a/nirc_ehr/resources/queries/study/deaths.js +++ b/nirc_ehr/resources/queries/study/deaths.js @@ -79,6 +79,13 @@ 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]) { + // Do not allow a death record to be completed while other data for this animal is still in 'Review Required' state. + // Records belonging to this death's own task are excluded, since they move to Completed in the same save. + var reviewRequiredDatasets = null; + if (row.QCStateLabel && row.QCStateLabel.toUpperCase() === 'COMPLETED') { + reviewRequiredDatasets = triggerHelper.getReviewRequiredDatasets(row.Id, row.taskid || 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'); @@ -107,6 +114,9 @@ function onUpsert(helper, scriptErrors, row, oldRow) { 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 (reviewRequiredDatasets) { + EHR.Server.Utils.addError(scriptErrors, 'Id', 'Death record cannot be completed. There is still data in Review Required state for this animal in the following dataset(s): ' + reviewRequiredDatasets, 'ERROR'); + } else if (!helper.isValidateOnly() && row.Id && row.date && row.QCStateLabel.toUpperCase() === 'COMPLETED') { if (validIds.indexOf(row.id) !== -1) { diff --git a/nirc_ehr/resources/queries/study/departure.js b/nirc_ehr/resources/queries/study/departure.js index f209fdc0..5c110767 100644 --- a/nirc_ehr/resources/queries/study/departure.js +++ b/nirc_ehr/resources/queries/study/departure.js @@ -10,6 +10,21 @@ function onInit(event, helper){ } +function onUpsert(helper, scriptErrors, row, oldRow) { + + if (!helper.isETL()) { + + // Do not allow a departure record to be completed while other data for this animal is still in 'Review Required' state. + // Records belonging to this departure's own task are excluded, since they move to Completed in the same save. + if (row.QCStateLabel && row.QCStateLabel.toUpperCase() === 'COMPLETED') { + var reviewRequiredDatasets = triggerHelper.getReviewRequiredDatasets(row.Id, row.taskid || null); + if (reviewRequiredDatasets) { + EHR.Server.Utils.addError(scriptErrors, 'Id', 'Departure record cannot be completed. There is still data in Review Required state for this animal in the following dataset(s): ' + reviewRequiredDatasets, 'ERROR'); + } + } + } +} + EHR.Server.TriggerManager.registerHandlerForQuery(EHR.Server.TriggerManager.Events.AFTER_INSERT, 'study', 'departure', function(helper, scriptErrors, row, oldRow) { if (row.id) { diff --git a/nirc_ehr/resources/queries/study/studyDataReviewRequired.sql b/nirc_ehr/resources/queries/study/studyDataReviewRequired.sql new file mode 100644 index 00000000..5eab6828 --- /dev/null +++ b/nirc_ehr/resources/queries/study/studyDataReviewRequired.sql @@ -0,0 +1,9 @@ +SELECT sd.Id, + sd.Date, + sd.DataSet.Label AS datasetLabel, + sd.taskid, + sd.lsid + +FROM study.StudyData sd +INNER JOIN core.QCState qc ON sd.QCState = qc.RowId +WHERE qc.Label = 'Review Required' diff --git a/nirc_ehr/src/org/labkey/nirc_ehr/query/NIRC_EHRTriggerHelper.java b/nirc_ehr/src/org/labkey/nirc_ehr/query/NIRC_EHRTriggerHelper.java index a4b08fd2..b3d58396 100644 --- a/nirc_ehr/src/org/labkey/nirc_ehr/query/NIRC_EHRTriggerHelper.java +++ b/nirc_ehr/src/org/labkey/nirc_ehr/query/NIRC_EHRTriggerHelper.java @@ -5,6 +5,7 @@ import org.apache.commons.lang3.time.DateUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.Nullable; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.CompareType; @@ -59,6 +60,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; public class NIRC_EHRTriggerHelper { @@ -309,6 +311,23 @@ public boolean deathExists(String id) return false; } + /** + * Returns a comma-separated list of dataset labels that still have records in the 'Review Required' + * QC state for the given animal, or null if there are none. Records belonging to {@code taskId} are + * excluded, since they transition to Completed in the same save as the death record itself. + */ + @Nullable + public String getReviewRequiredDatasets(String animalId, @Nullable String taskId) + { + TableInfo ti = getTableInfo("study", "studyDataReviewRequired"); + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("Id"), animalId); + if (taskId != null) + filter.addCondition(FieldKey.fromString("taskid"), taskId, CompareType.NEQ_OR_NULL); + + Set datasets = new TreeSet<>(new TableSelector(ti, Collections.singleton("datasetLabel"), filter, null).getArrayList(String.class)); + return datasets.isEmpty() ? null : String.join(", ", datasets); + } + public void upsertWeightRecord(Map row) throws QueryUpdateServiceException, DuplicateKeyException, SQLException, BatchValidationException, InvalidKeyException { BatchValidationException errors = new BatchValidationException(); @@ -680,7 +699,7 @@ public long totalRecords(String schemaName, String queryName, String columnName, return ts.getRowCount(); } - public boolean canCloseCase(String category) + public boolean canCloseCase() { if (_container.hasPermission(_user, EHRVeterinarianPermission.class)) return true; diff --git a/nirc_ehr/test/src/org.labkey.test/tests.nirc_ehr/NIRC_EHRTest.java b/nirc_ehr/test/src/org.labkey.test/tests.nirc_ehr/NIRC_EHRTest.java index 8d99ca6b..d2d0aede 100644 --- a/nirc_ehr/test/src/org.labkey.test/tests.nirc_ehr/NIRC_EHRTest.java +++ b/nirc_ehr/test/src/org.labkey.test/tests.nirc_ehr/NIRC_EHRTest.java @@ -1225,6 +1225,61 @@ public void testDeathNecropsyForm() throws IOException, CommandException } + @Test + public void testDeathAndDepartureBlockedByReviewRequiredData() throws Exception + { + String animalId = "REVREQ1"; + String taskId = UUID.randomUUID().toString(); + + goToEHRFolder(); + + log("Creating a live animal with a weight record in Review Required state"); + getApiHelper().deleteAllRecords("study", "deaths", new Filter("Id", animalId)); + getApiHelper().deleteAllRecords("study", "departure", new Filter("Id", animalId)); + getApiHelper().deleteAllRecords("study", "weight", new Filter("Id", animalId)); + getApiHelper().deleteAllRecords("study", "demographics", new Filter("Id", animalId)); + + String[] demographicsFields = {"Id", "Species", "Birth", "Gender", "date", "calculated_status", "objectid", "performedby"}; + Object[][] demographicsData = {{animalId, "Rhesus", (new Date()).toString(), getMale(), new Date(), "Alive", UUID.randomUUID().toString(), 1004}}; + getApiHelper().doSaveRows(DATA_ADMIN.getEmail(), getApiHelper().prepareInsertCommand("study", "demographics", "lsid", demographicsFields, demographicsData), getExtraContext()); + + // The weight record carries the same taskid used by the death/departure rows below so the exclusion + // scenarios can prove that records belonging to the completing record's own task are excluded from + // the review-required check. Both scenarios are validate-only, so nothing persists between them. + String[] weightInsertFields = {"Id", "date", "weight", "taskid", FIELD_QCSTATELABEL, "performedby"}; + Object[][] weightInsertData = {{animalId, new Date(), 8.5, taskId, EHRQCState.REVIEW_REQUIRED.label, 1004}}; + getApiHelper().doSaveRows(DATA_ADMIN.getEmail(), getApiHelper().prepareInsertCommand("study", "weight", "lsid", weightInsertFields, weightInsertData), getExtraContext()); + + String[] deathFields = {"Id", "date", "reason", "deathWeight", "taskid", FIELD_QCSTATELABEL, FIELD_OBJECTID, FIELD_LSID, "_recordid", "performedby"}; + verifyCompletionBlockedByReviewRequired("deaths", "Death", deathFields, + new Object[]{animalId, new Date(), "4", 8.5, null, EHRQCState.COMPLETED.label, null, null, "recordID", 1004}, + new Object[]{animalId, new Date(), "4", 8.5, taskId, EHRQCState.COMPLETED.label, null, null, "recordID", 1004}); + + String[] departureFields = {"Id", "date", "destination", "taskid", FIELD_QCSTATELABEL, FIELD_OBJECTID, FIELD_LSID, "_recordid", "performedby"}; + verifyCompletionBlockedByReviewRequired("departure", "Departure", departureFields, + new Object[]{animalId, new Date(), "Oregon NPRC", null, EHRQCState.COMPLETED.label, null, null, "recordID", 1004}, + new Object[]{animalId, new Date(), "Oregon NPRC", taskId, EHRQCState.COMPLETED.label, null, null, "recordID", 1004}); + } + + // Asserts that completing a record (death, departure) is blocked while other data for the animal is in + // Review Required state, and that a record whose taskid matches the review-required data is NOT blocked + // (its records move to Completed in the same save). rowWithoutTaskId/rowWithTaskId differ only in taskid. + private void verifyCompletionBlockedByReviewRequired(String queryName, String recordNoun, String[] fields, Object[] rowWithoutTaskId, Object[] rowWithTaskId) + { + // testValidationMessage defaults extraContext.targetQC to 'In Progress', and the global targetQC always + // overrides row-level QCStateLabel (see ehr/security.js normalizeQCState), so it must be forced to + // 'Completed' for the trigger to see a completing record. + Map completedTargetQC = Map.of("targetQC", EHRQCState.COMPLETED.label); + + log("Completing a " + queryName + " record while other data is in Review Required state should be blocked"); + Map> expected = new HashMap<>(); + expected.put("Id", Collections.singletonList("ERROR: " + recordNoun + " record cannot be completed. There is still data in Review Required state for this animal in the following dataset(s): Weight")); + getApiHelper().testValidationMessage(DATA_ADMIN.getEmail(), "study", queryName, fields, new Object[][]{rowWithoutTaskId}, expected, completedTargetQC); + + log("Review Required records on the " + queryName + " record's own task should not block completion"); + getApiHelper().testValidationMessage(DATA_ADMIN.getEmail(), "study", queryName, fields, new Object[][]{rowWithTaskId}, new HashMap<>(), completedTargetQC); + } + @Test public void testClinicalCasesWorkflow() { From 0161108d26413b05842c994f8fae55eb176a37ec Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Mon, 13 Jul 2026 06:36:03 -0600 Subject: [PATCH 2/3] Remove category argument from canCloseCase call in cases trigger --- nirc_ehr/resources/queries/study/cases.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nirc_ehr/resources/queries/study/cases.js b/nirc_ehr/resources/queries/study/cases.js index a8e3b6c3..1e6959e2 100644 --- a/nirc_ehr/resources/queries/study/cases.js +++ b/nirc_ehr/resources/queries/study/cases.js @@ -22,7 +22,7 @@ EHR.Server.TriggerManager.registerHandlerForQuery(EHR.Server.TriggerManager.Even error = true; } - if(!triggerHelper.canCloseCase(row.category)) { + if(!triggerHelper.canCloseCase()) { EHR.Server.Utils.addError(errors, 'enddate', 'Veterinarian permission required to close a case.', 'ERROR'); error = true; } From 3112d65b056c5c7bc78ea13325cc5b17c5219ec4 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Mon, 13 Jul 2026 10:10:25 -0600 Subject: [PATCH 3/3] Remove duplicate history data source and demographics provider registrations CagematesDemographicsProvider was registered twice, and DefaultClinicalRemarksDataSource and DefaultVitalsDataSource were shadowed by the NIRC-specific sources of the same name (NIRCClinicalRemarksDataSource, NIRCVitalsDataSource), which register later and win in ClinicalHistoryManager's dedup by name. The shadowed registrations only produced duplicate-source warnings on every history fetch. --- nirc_ehr/src/org/labkey/nirc_ehr/NIRC_EHRModule.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/nirc_ehr/src/org/labkey/nirc_ehr/NIRC_EHRModule.java b/nirc_ehr/src/org/labkey/nirc_ehr/NIRC_EHRModule.java index 8339e493..b087a268 100644 --- a/nirc_ehr/src/org/labkey/nirc_ehr/NIRC_EHRModule.java +++ b/nirc_ehr/src/org/labkey/nirc_ehr/NIRC_EHRModule.java @@ -27,9 +27,7 @@ import org.labkey.api.ehr.demographics.SourceDemographicsProvider; import org.labkey.api.ehr.history.DefaultAlopeciaDataSource; import org.labkey.api.ehr.history.DefaultAnimalRecordFlagDataSource; -import org.labkey.api.ehr.history.DefaultClinicalRemarksDataSource; import org.labkey.api.ehr.history.DefaultNotesDataSource; -import org.labkey.api.ehr.history.DefaultVitalsDataSource; import org.labkey.api.ehr.security.EHRDataAdminPermission; import org.labkey.api.ldk.ExtendedSimpleModule; import org.labkey.api.ldk.buttons.ShowEditUIButton; @@ -131,7 +129,6 @@ protected void doStartupAfterSpringConfig(ModuleContext moduleContext) ehrService.registerDemographicsProvider(new ProtocolAssignmentDemographicsProvider(this)); ehrService.registerDemographicsProvider(new HousingDemographicsProvider(this)); ehrService.registerDemographicsProvider(new CagematesDemographicsProvider(this)); - ehrService.registerDemographicsProvider(new CagematesDemographicsProvider(this)); ehrService.registerDemographicsProvider(new ActiveCasesDemographicsProvider(this)); ehrService.registerDemographicsProvider(new ActiveTreatmentsDemographicsProvider(this)); ehrService.registerDemographicsProvider(new SourceDemographicsProvider(this)); @@ -145,9 +142,7 @@ protected void doStartupAfterSpringConfig(ModuleContext moduleContext) EHRService.get().registerHistoryDataSource(new DeathDataSource(this)); EHRService.get().registerHistoryDataSource(new DefaultAlopeciaDataSource(this)); EHRService.get().registerHistoryDataSource(new DefaultAnimalRecordFlagDataSource(this)); - EHRService.get().registerHistoryDataSource(new DefaultClinicalRemarksDataSource(this)); EHRService.get().registerHistoryDataSource(new DefaultNotesDataSource(this)); - EHRService.get().registerHistoryDataSource(new DefaultVitalsDataSource(this)); EHRService.get().registerHistoryDataSource(new DepartureDataSource(this)); EHRService.get().registerHistoryDataSource(new DrugAdminDataSource(this)); EHRService.get().registerHistoryDataSource(new FlagsDataSource(this));