Workflow: Add Update Sample Status Action

api/src/org/labkey/api/query/AbstractQueryUpdateService.java

@@ -907,7 +907,7 @@ public List<Map<String, Object>> updateRows(User user, Container container, List
         addAuditEvent(user, container, QueryService.AuditAction.UPDATE, configParameters, result, oldRows, providedValues);
         WorkflowService service = WorkflowService.get();
         if (service != null && configParameters != null && configParameters.containsKey(WorkflowService.WorkflowConfigs.ActionId))
-            service.onActionComplete(container, user, (Long) configParameters.get(WorkflowService.WorkflowConfigs.ActionId));
+            service.onActionComplete(container, user, (Long) configParameters.get(WorkflowService.WorkflowConfigs.ActionId), (String) configParameters.get(AuditUserComment));
 
         return result;
     }

api/src/org/labkey/api/workflow/Action.java

@@ -2,14 +2,18 @@
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import org.apache.commons.lang3.StringUtils;
+import org.jetbrains.annotations.Nullable;
 import org.json.JSONArray;
+import org.json.JSONException;
 import org.json.JSONObject;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.data.CreatedModified;
 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;
 
@@ -26,6 +30,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;
@@ -126,6 +131,88 @@ public void setInputParameters(JSONObject inputParameters)
         _inputParameters = inputParameters;
     }
 
+    /**
+     * Validates that the status parameters are valid, according to the following rules:
+     *  - If updateKeyRequired is true, then STATUS_KEY and REMOVE_FROM_STORAGE_KEY may only be present if
+     *  UPDATE_STATUS_KEY is true
+     *  - STATUS_KEY is optional, but when present the value must be a valid status
+     *  - REMOVE_FROM_STORAGE_KEY may only be present when STATUS_KEY is also present
+     *  - If REMOVE_FROM_STORAGE_KEY is true, then the value for STATUS_KEY must represent a consumed state
+     */
+    private @Nullable String validateStatus(Container container, String prefix, boolean updateKeyRequired)
+    {
+        boolean hasStatusKey = _inputParameters.has(STATUS_KEY);
+        boolean hasUpdateStatusKey = _inputParameters.has(UPDATE_STATUS_KEY);
+        boolean hasRemoveFromStorageKey = _inputParameters.has(REMOVE_FROM_STORAGE_KEY);
+
+        boolean updateStatus;
+        try
+        {
+            updateStatus = hasUpdateStatusKey && _inputParameters.getBoolean(UPDATE_STATUS_KEY);
+        }
+        catch (JSONException e)
+        {
+            return prefix + UPDATE_STATUS_KEY + " must be a boolean.";
+        }
+
+        boolean removeFromStorage;
+        try
+        {
+            removeFromStorage = hasRemoveFromStorageKey && _inputParameters.getBoolean(REMOVE_FROM_STORAGE_KEY);
+        }
+        catch (JSONException e)
+        {
+            return prefix + REMOVE_FROM_STORAGE_KEY + " must be a boolean.";
+        }
+
+        // If we're expecting an UPDATE_STATUS_KEY (aliquot / derive / pool actions), then the key must be present and
+        // set to true in order for us to honor the STATUS_KEY and REMOVE_FROM_STORAGE_KEY.
+        if (updateKeyRequired && !updateStatus)
+        {
+            if (hasStatusKey)
+                return prefix + STATUS_KEY + " is not allowed for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is false.";
+
+            if (hasRemoveFromStorageKey)
+                return prefix + REMOVE_FROM_STORAGE_KEY + " is not allowed for action of type " + _type + " when " + UPDATE_STATUS_KEY + " is false.";
+        }
+
+        if (removeFromStorage && !hasStatusKey)
+            return prefix + STATUS_KEY + " is required for action of type " + _type + " when " + REMOVE_FROM_STORAGE_KEY + " is true.";
+
+        if (hasStatusKey && container != null)
+        {
+            long statusId;
+            try
+            {
+                statusId = _inputParameters.getLong(STATUS_KEY);
+            }
+            catch (JSONException e)
+            {
+                return prefix + STATUS_KEY + " must be a number.";
+            }
+
+            DataState state = SampleStatusService.get().getStateForRowId(container, statusId);
+
+            if (state == null)
+                return prefix + "Invalid " + STATUS_KEY + " (" + statusId + ") for container " + container.getPath() + ".";
+
+            if (removeFromStorage && !ExpSchema.SampleStateType.Consumed.name().equals(state.getStateType()))
+                return prefix + STATUS_KEY + " (" + statusId + ") must represent a " + ExpSchema.SampleStateType.Consumed.name() + " state when " + REMOVE_FROM_STORAGE_KEY + " is true.";
+        }
+
+        return null;
+    }
+
+    private static boolean isSampleStatusKey(String key)
+    {
+        return key.equals(STATUS_KEY) || key.equals(UPDATE_STATUS_KEY) || key.equals(REMOVE_FROM_STORAGE_KEY);
+    }
+
+    private boolean hasAnySampleStatusKey()
+    {
+        return _inputParameters != null && (_inputParameters.has(UPDATE_STATUS_KEY) || _inputParameters.has(STATUS_KEY) || _inputParameters.has(REMOVE_FROM_STORAGE_KEY));
+    }
+
     @JsonIgnore
     public List<String> validateInputParameters(int ordinal, Container container)
     {
@@ -169,31 +256,57 @@ public List<String> validateInputParameters(int ordinal, Container container)
         }
         else if (_type == WorkflowService.ActionType.AliquotSamples)
         {
+            List<String> 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 (hasAnySampleStatusKey())
+            {
+                String statusMessage = validateStatus(container, prefix, true);
+                if (statusMessage != null) messages.add(statusMessage);
+            }
+
+            return messages;
         }
         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<String> sampleTypeIds = new ArrayList<>();
+            _inputParameters.keys().forEachRemaining(id -> {
+                if (isSampleStatusKey(id)) 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<String> invalidSampleTypeIds = new HashSet<>();
             List<Object> invalidCounts = new ArrayList<>();
+            List<String> messages = new ArrayList<>();
 
-            _inputParameters.keys().forEachRemaining(id -> {
+            for (String id : sampleTypeIds)
+            {
                 try
                 {
                     if (sampleTypeService.getSampleType(Long.valueOf(id)) == null)
@@ -204,55 +317,53 @@ 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<String> 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 (hasAnySampleStatusKey())
+            {
+                String statusMessage = validateStatus(container, prefix, true);
+
+                if (statusMessage != null) messages.add(statusMessage);
+            }
+
             return messages;
         }
-        else if (_type == WorkflowService.ActionType.RemoveFromStorage)
+        else if (_type == WorkflowService.ActionType.RemoveFromStorage || _type == WorkflowService.ActionType.UpdateSampleStatus)
         {
             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 + ".");
-                }
-            }
-        } else {
+
+            boolean updateKeyRequired = _type == WorkflowService.ActionType.RemoveFromStorage;
+            String statusMessage = validateStatus(container, prefix, updateKeyRequired);
+
+            if (statusMessage != null) return List.of(statusMessage);
+
+            return Collections.emptyList();
+        }
+        else
+        {
             if (_inputParameters != null && !_inputParameters.isEmpty())
                 return List.of(prefix + "input parameters are not allowed for action of type " + _type + ".");
         }

api/src/org/labkey/api/workflow/WorkflowService.java

@@ -2,14 +2,14 @@
 
 import jakarta.servlet.http.HttpServletRequest;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.Container;
 import org.labkey.api.dataiterator.DataIteratorBuilder;
 import org.labkey.api.query.ValidationException;
 import org.labkey.api.security.User;
 import org.labkey.api.services.ServiceRegistry;
 
 import java.util.Map;
-import org.jetbrains.annotations.Nullable;
 
 public interface WorkflowService
 {
@@ -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;
@@ -65,8 +66,9 @@ static WorkflowService get()
 
     void populateConfigParams(HttpServletRequest request, Map<Enum, Object> configParameters) throws ValidationException;
     Map<String, Object> getConfigParameters(HttpServletRequest request) throws ValidationException;
-    void onActionComplete(@NotNull Container container, @NotNull User user, @NotNull Long actionId);
+    void onActionComplete(@NotNull Container container, @NotNull User user, @NotNull Long actionId, @Nullable String userAuditComment);
     void onActionComplete(@NotNull Container container, @NotNull User user, @NotNull Long taskId, @NotNull ActionType actionType);
+    boolean actionWillAddSamples(Long actionId);
 
     DataIteratorBuilder getSampleCreationDataIteratorBuilder(DataIteratorBuilder data, Container container, User user);
 

experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java

@@ -420,7 +420,13 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI
             {
                 if (context.getConfigParameter(WorkflowService.WorkflowConfigs.ActionId) != null)
                 {
-                    dib = workService.getSampleCreationDataIteratorBuilder(dib, userSchema.getContainer(), userSchema.getUser());
+                    Long actionId = (Long) context.getConfigParameter(WorkflowService.WorkflowConfigs.ActionId);
+
+                    if (WorkflowService.get().actionWillAddSamples(actionId))
+                    {
+                        dib = workService.getSampleCreationDataIteratorBuilder(dib, userSchema.getContainer(), userSchema.getUser());
+                    }
+
                     dib = workService.getActionAuditDataIteratorBuilder(dib, userSchema.getContainer(), userSchema.getUser());
                 }
             }