From 005d9ce5ab7d0bee850450df3bea01b3ad8ee5cc Mon Sep 17 00:00:00 2001 From: alanv Date: Thu, 7 May 2026 16:46:31 -0500 Subject: [PATCH 1/3] Add UpdateSampleStatus action type and validation Add validation for sample status changes during Aliquot/Derive/Pool actions --- api/src/org/labkey/api/workflow/Action.java | 106 +++++++++++++----- .../labkey/api/workflow/WorkflowService.java | 3 +- 2 files changed, 79 insertions(+), 30 deletions(-) diff --git a/api/src/org/labkey/api/workflow/Action.java b/api/src/org/labkey/api/workflow/Action.java index 6ba2c550783..d6e91c40a71 100644 --- a/api/src/org/labkey/api/workflow/Action.java +++ b/api/src/org/labkey/api/workflow/Action.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -126,6 +127,37 @@ public void setInputParameters(JSONObject inputParameters) _inputParameters = inputParameters; } + private List validateStatus(Container container, String prefix, boolean updateKeyExpected) + { + boolean updateStatus = true; + + if (updateKeyExpected) + { + updateStatus = _inputParameters.getBoolean(UPDATE_STATUS_KEY); + if (updateStatus && !_inputParameters.has(STATUS_KEY)) + return List.of(prefix + STATUS_KEY + " is required for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is true."); + if (!updateStatus && _inputParameters.has(STATUS_KEY)) + return List.of(prefix + STATUS_KEY + " is not allowed for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is false."); + } + + if (updateStatus && container != null) + { + try + { + long statusId = _inputParameters.getLong(STATUS_KEY); + SampleStatusService sampleStatusService = SampleStatusService.get(); + if (sampleStatusService.getStateForRowId(container, statusId) == null) + return List.of(prefix + "Invalid " + STATUS_KEY + " (" + statusId + ")."); + } + catch (Exception e) + { + return List.of(prefix + "Invalid " + STATUS_KEY + "."); + } + } + + return Collections.emptyList(); + } + @JsonIgnore public List validateInputParameters(int ordinal, Container container) { @@ -185,15 +217,34 @@ else if (_type == WorkflowService.ActionType.AliquotSamples) } else if (_type == WorkflowService.ActionType.DeriveSamples || _type == WorkflowService.ActionType.PoolSamples) { - if (_inputParameters == null || _inputParameters.isEmpty()) - return List.of(prefix + "data about sample types and sample counts per parent is required for action of type " + _type + "."); - if (_type == WorkflowService.ActionType.PoolSamples && _inputParameters.length() > 1) + String emptyMessage = prefix + "data about sample types and sample counts per parent is required for action of type " + _type + "."; + + if (_inputParameters == null) return List.of(emptyMessage); + + // We can't just check _inputParameters size because it may include sample status keys, so we extract the + // sample type IDs and validate against those. + List sampleTypeIds = new ArrayList<>(); + _inputParameters.keys().forEachRemaining(id -> { + if (id.equals(UPDATE_STATUS_KEY) || id.equals(STATUS_KEY)) return; + sampleTypeIds.add(id); + }); + + if (sampleTypeIds.isEmpty()) + return List.of(emptyMessage); + + if (_type == WorkflowService.ActionType.PoolSamples && sampleTypeIds.size() > 1) return List.of(prefix + "only one sample type can be specified for action of type " + _type + "."); + SampleTypeService sampleTypeService = SampleTypeService.get(); Set invalidSampleTypeIds = new HashSet<>(); List invalidCounts = new ArrayList<>(); + List messages = new ArrayList<>(); + + for (String id : sampleTypeIds) + { + // validated above + if (id.equals(UPDATE_STATUS_KEY) || id.equals(STATUS_KEY)) continue; - _inputParameters.keys().forEachRemaining(id -> { try { if (sampleTypeService.getSampleType(Long.valueOf(id)) == null) @@ -204,54 +255,51 @@ else if (_type == WorkflowService.ActionType.DeriveSamples || _type == WorkflowS invalidSampleTypeIds.add(id); } Object countObj = _inputParameters.get(id); - if ((countObj instanceof String)) + if (countObj instanceof String countStr) try { - if (Integer.parseInt((String) countObj) < 0) + if (Integer.parseInt(countStr) < 0) invalidCounts.add(countObj); } catch (NumberFormatException e) { invalidCounts.add(countObj); } - else if (countObj instanceof Integer) + else if (countObj instanceof Integer count) { - if (((Integer) countObj) < 0) + if (count < 0) invalidCounts.add(countObj); } else invalidCounts.add(countObj); - }); - List messages = new ArrayList<>(); + } + if (!invalidSampleTypeIds.isEmpty()) messages.add(prefix + "invalid sample type IDs " + invalidSampleTypeIds + "."); if (!invalidCounts.isEmpty()) messages.add(prefix + "invalid sample count values " + invalidCounts + "."); + + if (_inputParameters.has(UPDATE_STATUS_KEY) || _inputParameters.has(STATUS_KEY)) + { + List statusMessages = validateStatus(container, prefix, true); + + if (!statusMessages.isEmpty()) + messages.addAll(statusMessages); + } + return messages; } else if (_type == WorkflowService.ActionType.RemoveFromStorage) { if (_inputParameters == null || _inputParameters.isEmpty()) return Collections.emptyList(); - boolean updateStatus = _inputParameters.getBoolean(UPDATE_STATUS_KEY); - if (updateStatus && !_inputParameters.has(STATUS_KEY)) - return List.of(prefix + STATUS_KEY + " is required for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is true."); - if (!updateStatus && _inputParameters.has(STATUS_KEY)) - return List.of(prefix + STATUS_KEY + " is not allowed for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is false."); - if (updateStatus && container != null) - { - try - { - long statusId = _inputParameters.getLong(STATUS_KEY); - SampleStatusService sampleStatusService = SampleStatusService.get(); - if (sampleStatusService.getStateForRowId(container, statusId) == null) - return List.of(prefix + "Invalid " + STATUS_KEY + " (" + statusId + ")."); - } - catch (Exception e) - { - return List.of(prefix + "Invalid " + STATUS_KEY + "."); - } - } + + return validateStatus(container, prefix, true); + } else if (_type == WorkflowService.ActionType.UpdateSampleStatus) { + if (_inputParameters == null || _inputParameters.isEmpty()) + return Collections.emptyList(); + + return validateStatus(container, prefix, false); } else { if (_inputParameters != null && !_inputParameters.isEmpty()) return List.of(prefix + "input parameters are not allowed for action of type " + _type + "."); diff --git a/api/src/org/labkey/api/workflow/WorkflowService.java b/api/src/org/labkey/api/workflow/WorkflowService.java index b402ebaf1bb..3fd0f40d214 100644 --- a/api/src/org/labkey/api/workflow/WorkflowService.java +++ b/api/src/org/labkey/api/workflow/WorkflowService.java @@ -29,7 +29,8 @@ enum ActionType MoveInStorage("input parameters", "Moved samples in storage"), CheckOut("input parameters", "Checked out samples"), CheckIn("input parameters", "Checked in samples"), - RemoveFromStorage("sample status value", "Removed samples from storage"); + RemoveFromStorage("sample status value", "Removed samples from storage"), + UpdateSampleStatus("sample status value", "Updated sample status"); private final String _inputDescription; private final String _auditMessage; From 6ede52e7d4fb662abc99f8d79ae707e439dcb9cb Mon Sep 17 00:00:00 2001 From: alanv Date: Mon, 18 May 2026 12:26:26 -0500 Subject: [PATCH 2/3] workflow/Action: Account for removeFromStorage key when validating workflow actions --- api/src/org/labkey/api/workflow/Action.java | 27 ++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/workflow/Action.java b/api/src/org/labkey/api/workflow/Action.java index d6e91c40a71..c1af701c871 100644 --- a/api/src/org/labkey/api/workflow/Action.java +++ b/api/src/org/labkey/api/workflow/Action.java @@ -15,7 +15,6 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -27,6 +26,7 @@ public abstract class Action extends CreatedModified public static final String ASSAY_TYPES_KEY = "assayTypes"; public static final String NUM_PER_PARENT_KEY = "numPerParent"; public static final String UPDATE_STATUS_KEY = "updateStatus"; + public static final String REMOVE_FROM_STORAGE_KEY = "removeFromStorage"; public static final String STATUS_KEY = "sampleStatus"; protected Long _rowId; protected int _ordinal; @@ -158,6 +158,11 @@ private List validateStatus(Container container, String prefix, boolean return Collections.emptyList(); } + private static boolean isSampleStatusKey(String key) + { + return key.equals(STATUS_KEY) || key.equals(UPDATE_STATUS_KEY) || key.equals(REMOVE_FROM_STORAGE_KEY); + } + @JsonIgnore public List validateInputParameters(int ordinal, Container container) { @@ -201,19 +206,29 @@ public List validateInputParameters(int ordinal, Container container) } else if (_type == WorkflowService.ActionType.AliquotSamples) { + List messages = new ArrayList<>(); + if (_inputParameters == null || !_inputParameters.has(NUM_PER_PARENT_KEY)) - return List.of(prefix + NUM_PER_PARENT_KEY + " is required for action of type " + _type + "."); + messages.add(prefix + NUM_PER_PARENT_KEY + " is required for action of type " + _type + "."); else { try { int numPerParent = _inputParameters.getInt(NUM_PER_PARENT_KEY); if (numPerParent < 0) - return List.of(prefix + NUM_PER_PARENT_KEY + " cannot be negative."); + messages.add(prefix + NUM_PER_PARENT_KEY + " cannot be negative."); } catch (Exception e) { - return List.of(prefix + NUM_PER_PARENT_KEY + " must be an integer."); + messages.add(prefix + NUM_PER_PARENT_KEY + " must be an integer."); } } + + if (_inputParameters != null && (_inputParameters.has(UPDATE_STATUS_KEY) || _inputParameters.has(STATUS_KEY))) + { + List statusMessages = validateStatus(container, prefix, true); + messages.addAll(statusMessages); + } + + return messages; } else if (_type == WorkflowService.ActionType.DeriveSamples || _type == WorkflowService.ActionType.PoolSamples) { @@ -225,7 +240,7 @@ else if (_type == WorkflowService.ActionType.DeriveSamples || _type == WorkflowS // sample type IDs and validate against those. List sampleTypeIds = new ArrayList<>(); _inputParameters.keys().forEachRemaining(id -> { - if (id.equals(UPDATE_STATUS_KEY) || id.equals(STATUS_KEY)) return; + if (isSampleStatusKey(id)) return; sampleTypeIds.add(id); }); @@ -243,7 +258,7 @@ else if (_type == WorkflowService.ActionType.DeriveSamples || _type == WorkflowS for (String id : sampleTypeIds) { // validated above - if (id.equals(UPDATE_STATUS_KEY) || id.equals(STATUS_KEY)) continue; + if (isSampleStatusKey(id)) continue; try { From 6775ce66f2da3e8fc6ed3c3490440f3556ae9e7b Mon Sep 17 00:00:00 2001 From: alanv Date: Tue, 19 May 2026 17:31:58 -0500 Subject: [PATCH 3/3] Action: update validateStatus to check for consumed status when using removeFromStorage --- api/src/org/labkey/api/workflow/Action.java | 33 ++++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/workflow/Action.java b/api/src/org/labkey/api/workflow/Action.java index c1af701c871..84672279ade 100644 --- a/api/src/org/labkey/api/workflow/Action.java +++ b/api/src/org/labkey/api/workflow/Action.java @@ -10,6 +10,8 @@ import org.labkey.api.exp.api.ExpProtocol; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.SampleTypeService; +import org.labkey.api.exp.query.ExpSchema; +import org.labkey.api.qc.DataState; import org.labkey.api.qc.SampleStatusService; import org.labkey.api.util.GUID; @@ -130,29 +132,44 @@ public void setInputParameters(JSONObject inputParameters) private List validateStatus(Container container, String prefix, boolean updateKeyExpected) { boolean updateStatus = true; + boolean removeFromStorage = _inputParameters.has(REMOVE_FROM_STORAGE_KEY) && _inputParameters.getBoolean(REMOVE_FROM_STORAGE_KEY); if (updateKeyExpected) { - updateStatus = _inputParameters.getBoolean(UPDATE_STATUS_KEY); + updateStatus = _inputParameters.has(UPDATE_STATUS_KEY) && _inputParameters.getBoolean(UPDATE_STATUS_KEY); if (updateStatus && !_inputParameters.has(STATUS_KEY)) return List.of(prefix + STATUS_KEY + " is required for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is true."); if (!updateStatus && _inputParameters.has(STATUS_KEY)) return List.of(prefix + STATUS_KEY + " is not allowed for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is false."); } - if (updateStatus && container != null) + if (removeFromStorage && !_inputParameters.has(STATUS_KEY)) + return List.of(prefix + STATUS_KEY + " is required for action of type " + _type + " when " + REMOVE_FROM_STORAGE_KEY + " is true."); + + if ((updateStatus || removeFromStorage) && _inputParameters.has(STATUS_KEY) && container != null) { + DataState state; + long statusId; try { - long statusId = _inputParameters.getLong(STATUS_KEY); - SampleStatusService sampleStatusService = SampleStatusService.get(); - if (sampleStatusService.getStateForRowId(container, statusId) == null) - return List.of(prefix + "Invalid " + STATUS_KEY + " (" + statusId + ")."); + statusId = _inputParameters.getLong(STATUS_KEY); + state = SampleStatusService.get().getStateForRowId(container, statusId); } catch (Exception e) { return List.of(prefix + "Invalid " + STATUS_KEY + "."); } + + if (state == null) + return List.of(prefix + "Invalid " + STATUS_KEY + " (" + statusId + ")."); + + if (removeFromStorage && !ExpSchema.SampleStateType.Consumed.name().equals(state.getStateType())) + return List.of(prefix + STATUS_KEY + " (" + statusId + ") must represent a " + ExpSchema.SampleStateType.Consumed.name() + " state when " + REMOVE_FROM_STORAGE_KEY + " is true."); + } + else if (!updateKeyExpected && !_inputParameters.has(STATUS_KEY) && !removeFromStorage) + { + // UpdateSampleStatus with non-empty params but no STATUS_KEY or REMOVE_FROM_STORAGE_KEY is invalid + return List.of(prefix + "Invalid " + STATUS_KEY + "."); } return Collections.emptyList(); @@ -222,7 +239,7 @@ else if (_type == WorkflowService.ActionType.AliquotSamples) } } - if (_inputParameters != null && (_inputParameters.has(UPDATE_STATUS_KEY) || _inputParameters.has(STATUS_KEY))) + if (_inputParameters != null && (_inputParameters.has(UPDATE_STATUS_KEY) || _inputParameters.has(STATUS_KEY) || _inputParameters.has(REMOVE_FROM_STORAGE_KEY))) { List statusMessages = validateStatus(container, prefix, true); messages.addAll(statusMessages); @@ -294,7 +311,7 @@ else if (countObj instanceof Integer count) if (!invalidCounts.isEmpty()) messages.add(prefix + "invalid sample count values " + invalidCounts + "."); - if (_inputParameters.has(UPDATE_STATUS_KEY) || _inputParameters.has(STATUS_KEY)) + if (_inputParameters.has(UPDATE_STATUS_KEY) || _inputParameters.has(STATUS_KEY) || _inputParameters.has(REMOVE_FROM_STORAGE_KEY)) { List statusMessages = validateStatus(container, prefix, true);