From 5cf0a44a2d36b1a2a94fafd1b759ada279010089 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 24 Sep 2025 11:40:29 -0700 Subject: [PATCH 01/10] Issue 53955: validate against currently supported units --- api/src/org/labkey/api/ontology/Unit.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/ontology/Unit.java b/api/src/org/labkey/api/ontology/Unit.java index 7a547dbd156..e722e6306db 100644 --- a/api/src/org/labkey/api/ontology/Unit.java +++ b/api/src/org/labkey/api/ontology/Unit.java @@ -218,9 +218,9 @@ else if (u.kindOfQuantity != defaultUnits.kindOfQuantity) rawUnitsString = rawUnitsString.trim(); Unit mUnit = Unit.fromName(rawUnitsString); - if (mUnit == null) + List commonUnits = KindOfQuantity.getSupportedUnits(); + if (mUnit == null || !commonUnits.contains(mUnit)) { - List commonUnits = KindOfQuantity.getSupportedUnits(); throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + "."); } if (defaultUnits != null && mUnit.kindOfQuantity != defaultUnits.kindOfQuantity) From c012ad540b748835d4704053cfe9ccf6e401c1e5 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 24 Sep 2025 11:40:45 -0700 Subject: [PATCH 02/10] Typo --- experiment/resources/schemas/exp.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/resources/schemas/exp.xml b/experiment/resources/schemas/exp.xml index 0330b5c5e69..1a3347f50df 100644 --- a/experiment/resources/schemas/exp.xml +++ b/experiment/resources/schemas/exp.xml @@ -223,7 +223,7 @@ /list/grid.view?name=${Name} From 66a2e97c1a610420ac41a58f776d6a9e73d88746 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 24 Sep 2025 14:54:53 -0700 Subject: [PATCH 03/10] Issue 53963: Update messaging for name expression failure to not include row number --- .../experiment/api/ExpSampleTypeImpl.java | 8 ++++---- .../api/SampleTypeUpdateServiceDI.java | 19 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java index c15476b08db..b208fb4ce11 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java @@ -557,11 +557,11 @@ public void createSampleNames(@NotNull List> maps, { // Failed to generate a name due to some part of the expression not in the row if (hasNameExpression()) - throw new ExperimentException("Failed to generate name for sample on row " + e.getRowNumber(), e); + throw new ExperimentException("Failed to generate name for sample.", e); else if (hasNameAsIdCol()) - throw new ExperimentException("SampleID or Name is required for sample on row " + e.getRowNumber(), e); + throw new ExperimentException("SampleID or Name is required for sample.", e); else - throw new ExperimentException("All id columns are required for sample on row " + e.getRowNumber(), e); + throw new ExperimentException("All id columns are required for sample.", e); } } @@ -609,7 +609,7 @@ public String createSampleName(@NotNull Map rowMap, } catch (NameGenerator.NameGenerationException e) { - throw new ExperimentException("Failed to generate name for Sample", e); + throw new ExperimentException("Failed to generate name for sample.", e); } } diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 92001fe2d5e..38abc449451 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -868,15 +868,15 @@ protected Map _update(User user, Container c, Map Date: Wed, 24 Sep 2025 14:55:51 -0700 Subject: [PATCH 04/10] Issue 53961: Use "Display Units" as label for MetricUnit column --- .../org/labkey/experiment/api/ExpSampleTypeTableImpl.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java index 93af4125c9d..e844b9e20a2 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java @@ -64,11 +64,16 @@ public MutableColumnInfo createColumn(String alias, Column column) case MaterialLSIDPrefix: case Name: case LabelColor: - case MetricUnit: case AutoLinkTargetContainer: case AutoLinkCategory: case RowId: return wrapColumn(alias, _rootTable.getColumn(column.toString())); + case MetricUnit: + { + var columnInfo = wrapColumn(alias, _rootTable.getColumn(column.toString())); + columnInfo.setLabel("Display Units"); + return columnInfo; + } case Created: return wrapColumn(alias, _rootTable.getColumn("Created")); case CreatedBy: From 055a9bea744450d342624ca40b4ec73eb41eecaf Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 24 Sep 2025 14:58:38 -0700 Subject: [PATCH 05/10] Issue 53955: Move method for getSupportedUnits and getValidatedUnit to SampleTypeService --- .../labkey/api/exp/api/SampleTypeService.java | 7 ++ api/src/org/labkey/api/ontology/Unit.java | 49 ++------ .../labkey/experiment/ExperimentModule.java | 1 + .../experiment/api/SampleTypeServiceImpl.java | 110 ++++++++++++++++++ 4 files changed, 125 insertions(+), 42 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 0a5b601ad73..7deb7b3c174 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -30,6 +30,7 @@ import org.labkey.api.gwt.client.model.GWTIndex; import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import org.labkey.api.lists.permissions.ManagePicklistsPermission; +import org.labkey.api.ontology.Unit; import org.labkey.api.qc.DataState; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.ValidationException; @@ -114,6 +115,12 @@ static void setInstance(SampleTypeService impl) ServiceRegistry.get().registerService(SampleTypeService.class, impl); } + @NotNull + List getSupportedUnits(); + + @Nullable + Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits); + Map getSampleTypesForRoles(Container container, ContainerFilter filter, ExpProtocol.ApplicationType type); /** diff --git a/api/src/org/labkey/api/ontology/Unit.java b/api/src/org/labkey/api/ontology/Unit.java index e722e6306db..36f275c2a3c 100644 --- a/api/src/org/labkey/api/ontology/Unit.java +++ b/api/src/org/labkey/api/ontology/Unit.java @@ -9,7 +9,6 @@ import org.labkey.api.data.ConversionExceptionWithMessage; import java.util.HashMap; -import java.util.List; import java.util.function.Function; public enum Unit @@ -76,8 +75,6 @@ public enum Unit Quantity.Mass_pg.class, "picogram", "picograms"); - private static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the display units (%s)."; - @Getter final @NotNull KindOfQuantity kindOfQuantity; @Getter @@ -198,38 +195,6 @@ public Quantity convert(@Nullable Object value) return Quantity.convert(value, this); } - public static Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits) - { - if (rawUnits == null) - return null; - if (rawUnits instanceof Unit u) - { - if (defaultUnits == null) - return u; - else if (u.kindOfQuantity != defaultUnits.kindOfQuantity) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); - else - return u; - } - if (!(rawUnits instanceof String rawUnitsString)) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); - if (!StringUtils.isBlank(rawUnitsString)) - { - rawUnitsString = rawUnitsString.trim(); - - Unit mUnit = Unit.fromName(rawUnitsString); - List commonUnits = KindOfQuantity.getSupportedUnits(); - if (mUnit == null || !commonUnits.contains(mUnit)) - { - throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + "."); - } - if (defaultUnits != null && mUnit.kindOfQuantity != defaultUnits.kindOfQuantity) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); - return mUnit; - } - return null; - } - public static class TestCase extends Assert { @Test @@ -330,9 +295,9 @@ public void testGetValidatedUnit() { try { - Unit.getValidatedUnit("g", Unit.mg); - Unit.getValidatedUnit("g ", Unit.mg); - Unit.getValidatedUnit(" g ", Unit.mg); + SampleTypeServiceImpl.getValidatedUnit("g", Unit.mg); + SampleTypeServiceImpl.getValidatedUnit("g ", Unit.mg); + SampleTypeServiceImpl.getValidatedUnit(" g ", Unit.mg); } catch (ConversionExceptionWithMessage e) { @@ -340,7 +305,7 @@ public void testGetValidatedUnit() } try { - assertNull(Unit.getValidatedUnit(null, Unit.unit)); + assertNull(SampleTypeServiceImpl.getValidatedUnit(null, Unit.unit)); } catch (ConversionExceptionWithMessage e) { @@ -348,7 +313,7 @@ public void testGetValidatedUnit() } try { - assertNull(Unit.getValidatedUnit("", Unit.unit)); + assertNull(SampleTypeServiceImpl.getValidatedUnit("", Unit.unit)); } catch (ConversionExceptionWithMessage e) { @@ -356,7 +321,7 @@ public void testGetValidatedUnit() } try { - Unit.getValidatedUnit("g", Unit.unit); + SampleTypeServiceImpl.getValidatedUnit("g", Unit.unit); fail("Units that are not comparable should throw exception."); } catch (ConversionExceptionWithMessage ignore) @@ -366,7 +331,7 @@ public void testGetValidatedUnit() try { - Unit.getValidatedUnit("nonesuch", Unit.unit); + SampleTypeServiceImpl.getValidatedUnit("nonesuch", Unit.unit); fail("Invalid units should throw exception."); } catch (ConversionExceptionWithMessage ignore) diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 8529150b446..ddf89a9b3bd 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -1039,6 +1039,7 @@ public Set getIntegrationTests() LineageTest.class, OntologyManager.TestCase.class, PropertyServiceImpl.TestCase.class, + SampleTypeServiceImpl.TestCase.class, StorageNameGenerator.TestCase.class, StorageProvisionerImpl.TestCase.class, UniqueValueCounterTestCase.class diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index a7a7d9da82b..c0b977dd6fd 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -23,6 +23,8 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.action.ApiUsageException; import org.labkey.api.audit.AbstractAuditHandler; import org.labkey.api.audit.AbstractAuditTypeProvider; @@ -42,6 +44,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.ConversionExceptionWithMessage; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbSequence; @@ -93,6 +96,7 @@ import org.labkey.api.inventory.InventoryService; import org.labkey.api.miniprofiler.MiniProfiler; import org.labkey.api.miniprofiler.Timing; +import org.labkey.api.ontology.KindOfQuantity; import org.labkey.api.ontology.Quantity; import org.labkey.api.ontology.Unit; import org.labkey.api.qc.DataState; @@ -175,6 +179,16 @@ public class SampleTypeServiceImpl extends AbstractAuditHandler implements Sampl public static final String SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:sampleCount"; public static final String ROOT_SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:rootSampleCount"; + public static final List SUPPORTED_UNITS = new ArrayList<>(); + public static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the display units (%s)."; + + static + { + SUPPORTED_UNITS.addAll(KindOfQuantity.Volume.getCommonUnits()); + SUPPORTED_UNITS.addAll(KindOfQuantity.Mass.getCommonUnits()); + SUPPORTED_UNITS.addAll(KindOfQuantity.Count.getCommonUnits()); + } + // columns that may appear in a row when only the sample status is updating. public static final Set statusUpdateColumns = Set.of( ExpMaterialTable.Column.Modified.name().toLowerCase(), @@ -203,12 +217,52 @@ public static SampleTypeServiceImpl get() return Collections.unmodifiableSortedSet(new TreeSet<>(new TableSelector(getTinfoMaterialSource(), filter, null).getCollection(MaterialSource.class))); }); + Cache> getMaterialSourceCache() { return materialSourceCache; } + @Override + public List getSupportedUnits() + { + return SUPPORTED_UNITS; + } + + @Nullable @Override + public Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits) + { + if (rawUnits == null) + return null; + if (rawUnits instanceof Unit u) + { + if (defaultUnits == null) + return u; + else if (u.getKindOfQuantity() != defaultUnits.getKindOfQuantity()) + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); + else + return u; + } + if (!(rawUnits instanceof String rawUnitsString)) + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); + if (!StringUtils.isBlank(rawUnitsString)) + { + rawUnitsString = rawUnitsString.trim(); + + Unit mUnit = Unit.fromName(rawUnitsString); + List commonUnits = getSupportedUnits(); + if (mUnit == null || !commonUnits.contains(mUnit)) + { + throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + "."); + } + if (defaultUnits != null && mUnit.getKindOfQuantity() != defaultUnits.getKindOfQuantity()) + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); + return mUnit; + } + return null; + } + public void clearMaterialSourceCache(@Nullable Container c) { LOG.debug("clearMaterialSourceCache: " + (c == null ? "all" : c.getPath())); @@ -2265,4 +2319,60 @@ public void refreshSampleTypeMaterializedView(@NotNull ExpSampleType st, SampleC { ExpMaterialTableImpl.refreshMaterializedView(st.getLSID(), reason); } + + + public static class TestCase extends Assert + { + @Test + public void testGetValidatedUnit() + { + SampleTypeService service = SampleTypeService.get(); + try + { + service.getValidatedUnit("g", Unit.mg); + service.getValidatedUnit("g ", Unit.mg); + service.getValidatedUnit(" g ", Unit.mg); + } + catch (ConversionExceptionWithMessage e) + { + fail("Compatible unit should not throw exception."); + } + try + { + assertNull(service.getValidatedUnit(null, Unit.unit)); + } + catch (ConversionExceptionWithMessage e) + { + fail("null units should be null"); + } + try + { + assertNull(service.getValidatedUnit("", Unit.unit)); + } + catch (ConversionExceptionWithMessage e) + { + fail("empty units should be null"); + } + try + { + service.getValidatedUnit("g", Unit.unit); + fail("Units that are not comparable should throw exception."); + } + catch (ConversionExceptionWithMessage ignore) + { + + } + + try + { + service.getValidatedUnit("nonesuch", Unit.unit); + fail("Invalid units should throw exception."); + } + catch (ConversionExceptionWithMessage ignore) + { + + } + + } + } } From 96f958329df5054eaa00814063f5686a501672d9 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 24 Sep 2025 17:46:20 -0700 Subject: [PATCH 06/10] Issue 53975: Update error messaging for missing amounts and units --- .../org/labkey/api/exp/api/SampleTypeService.java | 4 ++-- .../experiment/api/SampleTypeUpdateServiceDI.java | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 7deb7b3c174..519c7f5c7b3 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -55,8 +55,8 @@ public interface SampleTypeService { - String MISSING_COLUMN_ERROR_MESSAGE_PATTERN = "When adding or updating samples, the %s column must be provided when the %s column is."; - String MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN = "When adding or updating samples, a %s value must be provided when there is a value for %s."; + String MISSING_AMOUNT_ERROR_MESSAGE = "An Amount value must be provided when Units are provided."; + String MISSING_UNITS_ERROR_MESSAGE = "A Units value must be provided when Amounts are provided."; String UNPROVIDED_VALUE_ERROR_MESSAGE_PATTERN = "No %s value provided for %s %s."; String NEW_SAMPLE_TYPE_ALIAS_VALUE = "{{this_sample_set}}"; String MATERIAL_INPUTS_PREFIX = "MaterialInputs/"; diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 38abc449451..8aad7300955 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -148,8 +148,8 @@ import static org.labkey.api.exp.api.SampleTypeDomainKind.ALIQUOT_ROLLUP_FIELD_LABELS; import static org.labkey.api.exp.api.SampleTypeService.ConfigParameters.SkipAliquotRollup; import static org.labkey.api.exp.api.SampleTypeService.ConfigParameters.SkipMaxSampleCounterFunction; -import static org.labkey.api.exp.api.SampleTypeService.MISSING_COLUMN_ERROR_MESSAGE_PATTERN; -import static org.labkey.api.exp.api.SampleTypeService.MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN; +import static org.labkey.api.exp.api.SampleTypeService.MISSING_AMOUNT_ERROR_MESSAGE; +import static org.labkey.api.exp.api.SampleTypeService.MISSING_UNITS_ERROR_MESSAGE; import static org.labkey.api.exp.api.SampleTypeService.UNPROVIDED_VALUE_ERROR_MESSAGE_PATTERN; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotCount; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotVolume; @@ -534,9 +534,9 @@ public static void confirmAmountAndUnitsColumns(Collection columns, bool if (hasUnits == hasAmount) return; // both columns are present or neither is if (!hasAmount) - throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_ERROR_MESSAGE_PATTERN, StoredAmount.label(), Units.name())); + throw new ConversionExceptionWithMessage(MISSING_AMOUNT_ERROR_MESSAGE); - throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_ERROR_MESSAGE_PATTERN, Units.name(), StoredAmount.label())); + throw new ConversionExceptionWithMessage(MISSING_UNITS_ERROR_MESSAGE); } @Override @@ -2046,7 +2046,7 @@ public static Object getValue(Object o, Object amountObj, boolean haveAmountCol, // when there's a units value but no amount column, this is an error if (!haveAmountCol) - throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN, StoredAmount.label(), Units.name())); + throw new ConversionExceptionWithMessage(MISSING_AMOUNT_ERROR_MESSAGE); // When an amount column is present but no amount value is provided, this is an error if (amountObj == null || ((amountObj instanceof String) && ((String) amountObj).isEmpty())) @@ -2083,7 +2083,7 @@ public static Object getValue(Object amountObj, boolean hasUnitsCol, Object unit // When there is an amount value, if there isn't a units column, this is an error. if (!hasUnitsCol) - throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN, Units.name(), StoredAmount.label())); + throw new ConversionExceptionWithMessage(MISSING_UNITS_ERROR_MESSAGE); // Have a units column, but no units value if (unitsObj == null || ((unitsObj instanceof String) && ((String) unitsObj).trim().isEmpty())) From 320bdd8a11b191bcd86afc9a9faec8a44726d822 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 24 Sep 2025 17:50:59 -0700 Subject: [PATCH 07/10] Remove test that was moved --- api/src/org/labkey/api/ontology/Unit.java | 51 ----------------------- 1 file changed, 51 deletions(-) diff --git a/api/src/org/labkey/api/ontology/Unit.java b/api/src/org/labkey/api/ontology/Unit.java index 36f275c2a3c..1cb963687a7 100644 --- a/api/src/org/labkey/api/ontology/Unit.java +++ b/api/src/org/labkey/api/ontology/Unit.java @@ -290,56 +290,5 @@ public void testFromName() assertNull(Unit.fromName("")); } - @Test - public void testGetValidatedUnit() - { - try - { - SampleTypeServiceImpl.getValidatedUnit("g", Unit.mg); - SampleTypeServiceImpl.getValidatedUnit("g ", Unit.mg); - SampleTypeServiceImpl.getValidatedUnit(" g ", Unit.mg); - } - catch (ConversionExceptionWithMessage e) - { - fail("Compatible unit should not throw exception."); - } - try - { - assertNull(SampleTypeServiceImpl.getValidatedUnit(null, Unit.unit)); - } - catch (ConversionExceptionWithMessage e) - { - fail("null units should be null"); - } - try - { - assertNull(SampleTypeServiceImpl.getValidatedUnit("", Unit.unit)); - } - catch (ConversionExceptionWithMessage e) - { - fail("empty units should be null"); - } - try - { - SampleTypeServiceImpl.getValidatedUnit("g", Unit.unit); - fail("Units that are not comparable should throw exception."); - } - catch (ConversionExceptionWithMessage ignore) - { - - } - - try - { - SampleTypeServiceImpl.getValidatedUnit("nonesuch", Unit.unit); - fail("Invalid units should throw exception."); - } - catch (ConversionExceptionWithMessage ignore) - { - - } - - } - } } From f6139320a58342bd48229d174f1688ee0be236e1 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 25 Sep 2025 08:54:38 -0700 Subject: [PATCH 08/10] Update expected error message to remove row number --- .../src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index e44db7a69d5..035d1407080 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -390,7 +390,7 @@ public void idColsSet_nameExpressionNull_hasNameProperty() throws Exception errors = new BatchValidationException(); svc.insertRows(user, c, rows, errors, null, null); assertTrue(errors.hasErrors()); - assertTrue(errors.getMessage().contains("SampleID or Name is required for sample on row 1")); + assertTrue(errors.getMessage().contains("SampleID or Name is required for sample")); } // idCols not null, nameExpression not null, 'name' property (not used) -- fail From 30cbf6cf5ebf6c01dbb1e83798c1e9494bb70f1a Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 25 Sep 2025 13:55:14 -0700 Subject: [PATCH 09/10] Add sample type name to incompatible units messaging --- .../labkey/api/exp/api/SampleTypeService.java | 2 +- .../experiment/api/SampleTypeServiceImpl.java | 29 +++++++++---------- .../api/SampleTypeUpdateServiceDI.java | 20 ++++++------- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 519c7f5c7b3..bcfbb85c86c 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -119,7 +119,7 @@ static void setInstance(SampleTypeService impl) List getSupportedUnits(); @Nullable - Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits); + Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits, @Nullable String sampleTypeName); Map getSampleTypesForRoles(Container container, ContainerFilter filter, ExpProtocol.ApplicationType type); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index c0b977dd6fd..bce9fb49986 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -106,7 +106,6 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.MetadataUnavailableException; import org.labkey.api.query.QueryChangeListener; -import org.labkey.api.query.QueryKey; import org.labkey.api.query.QueryService; import org.labkey.api.query.SchemaKey; import org.labkey.api.query.SimpleValidationError; @@ -145,7 +144,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.SortedSet; @@ -180,7 +178,7 @@ public class SampleTypeServiceImpl extends AbstractAuditHandler implements Sampl public static final String ROOT_SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:rootSampleCount"; public static final List SUPPORTED_UNITS = new ArrayList<>(); - public static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the display units (%s)."; + public static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the %s display units (%s)."; static { @@ -223,15 +221,14 @@ Cache> getMaterialSourceCache() return materialSourceCache; } - - @Override + @Override @NotNull public List getSupportedUnits() { return SUPPORTED_UNITS; } @Nullable @Override - public Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits) + public Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits, String sampleTypeName) { if (rawUnits == null) return null; @@ -240,12 +237,12 @@ public Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUn if (defaultUnits == null) return u; else if (u.getKindOfQuantity() != defaultUnits.getKindOfQuantity()) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits)); else return u; } if (!(rawUnits instanceof String rawUnitsString)) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits)); if (!StringUtils.isBlank(rawUnitsString)) { rawUnitsString = rawUnitsString.trim(); @@ -257,7 +254,7 @@ else if (u.getKindOfQuantity() != defaultUnits.getKindOfQuantity()) throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + "."); } if (defaultUnits != null && mUnit.getKindOfQuantity() != defaultUnits.getKindOfQuantity()) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits)); return mUnit; } return null; @@ -2329,9 +2326,9 @@ public void testGetValidatedUnit() SampleTypeService service = SampleTypeService.get(); try { - service.getValidatedUnit("g", Unit.mg); - service.getValidatedUnit("g ", Unit.mg); - service.getValidatedUnit(" g ", Unit.mg); + service.getValidatedUnit("g", Unit.mg, "Sample Type"); + service.getValidatedUnit("g ", Unit.mg, "Sample Type"); + service.getValidatedUnit(" g ", Unit.mg, "Sample Type"); } catch (ConversionExceptionWithMessage e) { @@ -2339,7 +2336,7 @@ public void testGetValidatedUnit() } try { - assertNull(service.getValidatedUnit(null, Unit.unit)); + assertNull(service.getValidatedUnit(null, Unit.unit, "Sample Type")); } catch (ConversionExceptionWithMessage e) { @@ -2347,7 +2344,7 @@ public void testGetValidatedUnit() } try { - assertNull(service.getValidatedUnit("", Unit.unit)); + assertNull(service.getValidatedUnit("", Unit.unit, "Sample Type")); } catch (ConversionExceptionWithMessage e) { @@ -2355,7 +2352,7 @@ public void testGetValidatedUnit() } try { - service.getValidatedUnit("g", Unit.unit); + service.getValidatedUnit("g", Unit.unit, "Sample Type"); fail("Units that are not comparable should throw exception."); } catch (ConversionExceptionWithMessage ignore) @@ -2365,7 +2362,7 @@ public void testGetValidatedUnit() try { - service.getValidatedUnit("nonesuch", Unit.unit); + service.getValidatedUnit("nonesuch", Unit.unit, "Sample Type"); fail("Invalid units should throw exception."); } catch (ConversionExceptionWithMessage ignore) diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 8aad7300955..7cb1dd30e4b 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -665,11 +665,11 @@ protected Map coerceTypes(Map row, Map _update(User user, Container c, Map Date: Fri, 26 Sep 2025 07:06:14 -0700 Subject: [PATCH 10/10] Add kL as a supported unit (required for LKB Mixture Batches) --- api/src/org/labkey/api/ontology/KindOfQuantity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/ontology/KindOfQuantity.java b/api/src/org/labkey/api/ontology/KindOfQuantity.java index ad80ae0cd00..cd51aeb5e6b 100644 --- a/api/src/org/labkey/api/ontology/KindOfQuantity.java +++ b/api/src/org/labkey/api/ontology/KindOfQuantity.java @@ -20,7 +20,7 @@ public enum KindOfQuantity @Override public List getCommonUnits() { - return List.of(Unit.L, Unit.mL, Unit.uL); + return List.of(Unit.kL, Unit.L, Unit.mL, Unit.uL); } },