From 06e80315507a4900e3dca3c26b56da5ae6ac6c76 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Tue, 14 Jul 2026 04:38:40 -0700 Subject: [PATCH] Block death and departure completion while animal data is in Review Required state (#721) ## Rationale A death or departure record could be finalized while other data for the animal was still in the Review Required QC state, leaving unreviewed records behind for an animal no longer at the center. Completing a death or departure now fails validation until that data is reviewed, except for records on the completing record's own task, which move to Completed in the same save. ## Related Pull Requests None. ## Changes - Added the `study.studyDataReviewRequired` module query (`study.StudyData` joined to `core.QCState`) listing records in Review Required state. - Added `NIRC_EHRTriggerHelper.getReviewRequiredDatasets`, returning the dataset labels still in Review Required for an animal, excluding records on the given taskid. - The `deaths.js` and `departure.js` triggers now block saving a record as Completed while review-required data exists, naming the offending dataset(s) in the error. - Added `NIRC_EHRTest.testDeathAndDepartureBlockedByReviewRequiredData` covering the blocked path and the own-task exclusion for both datasets via validate-only API saves. --- nirc_ehr/resources/queries/study/cases.js | 2 +- nirc_ehr/resources/queries/study/deaths.js | 10 ++++ nirc_ehr/resources/queries/study/departure.js | 15 +++++ .../queries/study/studyDataReviewRequired.sql | 9 +++ .../org/labkey/nirc_ehr/NIRC_EHRModule.java | 5 -- .../nirc_ehr/query/NIRC_EHRTriggerHelper.java | 21 ++++++- .../tests.nirc_ehr/NIRC_EHRTest.java | 55 +++++++++++++++++++ 7 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 nirc_ehr/resources/queries/study/studyDataReviewRequired.sql 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; } 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/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)); 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() {