Skip to content

Commit aa1c3bc

Browse files
labkey-susanhlabkey-susanh
authored
Issues 53975, 53955, 53961, 53963: misc messaging issues (#7073)
1 parent d3ac307 commit aa1c3bc

File tree

10 files changed

+153
-117
lines changed

10 files changed

+153
-117
lines changed

‎api/src/org/labkey/api/exp/api/SampleTypeService.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.labkey.api.gwt.client.model.GWTIndex;
3131
import org.labkey.api.gwt.client.model.GWTPropertyDescriptor;
3232
import org.labkey.api.lists.permissions.ManagePicklistsPermission;
33+
import org.labkey.api.ontology.Unit;
3334
import org.labkey.api.qc.DataState;
3435
import org.labkey.api.query.BatchValidationException;
3536
import org.labkey.api.query.ValidationException;
@@ -54,8 +55,8 @@
5455

5556
public interface SampleTypeService
5657
{
57-
String MISSING_COLUMN_ERROR_MESSAGE_PATTERN = "When adding or updating samples, the %s column must be provided when the %s column is.";
58-
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.";
58+
String MISSING_AMOUNT_ERROR_MESSAGE = "An Amount value must be provided when Units are provided.";
59+
String MISSING_UNITS_ERROR_MESSAGE = "A Units value must be provided when Amounts are provided.";
5960
String UNPROVIDED_VALUE_ERROR_MESSAGE_PATTERN = "No %s value provided for %s %s.";
6061
String NEW_SAMPLE_TYPE_ALIAS_VALUE = "{{this_sample_set}}";
6162
String MATERIAL_INPUTS_PREFIX = "MaterialInputs/";
@@ -114,6 +115,12 @@ static void setInstance(SampleTypeService impl)
114115
ServiceRegistry.get().registerService(SampleTypeService.class, impl);
115116
}
116117

118+
@NotNull
119+
List<Unit> getSupportedUnits();
120+
121+
@Nullable
122+
Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits, @Nullable String sampleTypeName);
123+
117124
Map<String, ExpSampleType> getSampleTypesForRoles(Container container, ContainerFilter filter, ExpProtocol.ApplicationType type);
118125

119126
/**

‎api/src/org/labkey/api/ontology/KindOfQuantity.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public enum KindOfQuantity
2020
@Override
2121
public List<Unit> getCommonUnits()
2222
{
23-
return List.of(Unit.L, Unit.mL, Unit.uL);
23+
return List.of(Unit.kL, Unit.L, Unit.mL, Unit.uL);
2424
}
2525
},
2626

‎api/src/org/labkey/api/ontology/Unit.java‎

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.labkey.api.data.ConversionExceptionWithMessage;
1010

1111
import java.util.HashMap;
12-
import java.util.List;
1312
import java.util.function.Function;
1413

1514
public enum Unit
@@ -76,8 +75,6 @@ public enum Unit
7675
Quantity.Mass_pg.class,
7776
"picogram", "picograms");
7877

79-
private static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the display units (%s).";
80-
8178
@Getter
8279
final @NotNull KindOfQuantity kindOfQuantity;
8380
@Getter
@@ -198,38 +195,6 @@ public Quantity convert(@Nullable Object value)
198195
return Quantity.convert(value, this);
199196
}
200197

201-
public static Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits)
202-
{
203-
if (rawUnits == null)
204-
return null;
205-
if (rawUnits instanceof Unit u)
206-
{
207-
if (defaultUnits == null)
208-
return u;
209-
else if (u.kindOfQuantity != defaultUnits.kindOfQuantity)
210-
throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits));
211-
else
212-
return u;
213-
}
214-
if (!(rawUnits instanceof String rawUnitsString))
215-
throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits));
216-
if (!StringUtils.isBlank(rawUnitsString))
217-
{
218-
rawUnitsString = rawUnitsString.trim();
219-
220-
Unit mUnit = Unit.fromName(rawUnitsString);
221-
if (mUnit == null)
222-
{
223-
List<Unit> commonUnits = KindOfQuantity.getSupportedUnits();
224-
throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + ".");
225-
}
226-
if (defaultUnits != null && mUnit.kindOfQuantity != defaultUnits.kindOfQuantity)
227-
throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits));
228-
return mUnit;
229-
}
230-
return null;
231-
}
232-
233198
public static class TestCase extends Assert
234199
{
235200
@Test
@@ -325,56 +290,5 @@ public void testFromName()
325290
assertNull(Unit.fromName(""));
326291
}
327292

328-
@Test
329-
public void testGetValidatedUnit()
330-
{
331-
try
332-
{
333-
Unit.getValidatedUnit("g", Unit.mg);
334-
Unit.getValidatedUnit("g ", Unit.mg);
335-
Unit.getValidatedUnit(" g ", Unit.mg);
336-
}
337-
catch (ConversionExceptionWithMessage e)
338-
{
339-
fail("Compatible unit should not throw exception.");
340-
}
341-
try
342-
{
343-
assertNull(Unit.getValidatedUnit(null, Unit.unit));
344-
}
345-
catch (ConversionExceptionWithMessage e)
346-
{
347-
fail("null units should be null");
348-
}
349-
try
350-
{
351-
assertNull(Unit.getValidatedUnit("", Unit.unit));
352-
}
353-
catch (ConversionExceptionWithMessage e)
354-
{
355-
fail("empty units should be null");
356-
}
357-
try
358-
{
359-
Unit.getValidatedUnit("g", Unit.unit);
360-
fail("Units that are not comparable should throw exception.");
361-
}
362-
catch (ConversionExceptionWithMessage ignore)
363-
{
364-
365-
}
366-
367-
try
368-
{
369-
Unit.getValidatedUnit("nonesuch", Unit.unit);
370-
fail("Invalid units should throw exception.");
371-
}
372-
catch (ConversionExceptionWithMessage ignore)
373-
{
374-
375-
}
376-
377-
}
378-
379293
}
380294
}

‎experiment/resources/schemas/exp.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@
223223
<column columnName="Name" >
224224
<!--
225225
Resolve lists by "name" instead of "listId" as "name" will resolve cross-folder
226-
where as "listId"s are only unique within a folder.
226+
whereas "listId"s are only unique within a folder.
227227
-->
228228
<url>/list/grid.view?name=${Name}</url>
229229
</column>

‎experiment/src/org/labkey/experiment/ExperimentModule.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,7 @@ public Set<Class> getIntegrationTests()
10391039
LineageTest.class,
10401040
OntologyManager.TestCase.class,
10411041
PropertyServiceImpl.TestCase.class,
1042+
SampleTypeServiceImpl.TestCase.class,
10421043
StorageNameGenerator.TestCase.class,
10431044
StorageProvisionerImpl.TestCase.class,
10441045
UniqueValueCounterTestCase.class

‎experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -557,11 +557,11 @@ public void createSampleNames(@NotNull List<Map<String, Object>> maps,
557557
{
558558
// Failed to generate a name due to some part of the expression not in the row
559559
if (hasNameExpression())
560-
throw new ExperimentException("Failed to generate name for sample on row " + e.getRowNumber(), e);
560+
throw new ExperimentException("Failed to generate name for sample.", e);
561561
else if (hasNameAsIdCol())
562-
throw new ExperimentException("SampleID or Name is required for sample on row " + e.getRowNumber(), e);
562+
throw new ExperimentException("SampleID or Name is required for sample.", e);
563563
else
564-
throw new ExperimentException("All id columns are required for sample on row " + e.getRowNumber(), e);
564+
throw new ExperimentException("All id columns are required for sample.", e);
565565
}
566566
}
567567

@@ -609,7 +609,7 @@ public String createSampleName(@NotNull Map<String, Object> rowMap,
609609
}
610610
catch (NameGenerator.NameGenerationException e)
611611
{
612-
throw new ExperimentException("Failed to generate name for Sample", e);
612+
throw new ExperimentException("Failed to generate name for sample.", e);
613613
}
614614
}
615615

‎experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,16 @@ public MutableColumnInfo createColumn(String alias, Column column)
6464
case MaterialLSIDPrefix:
6565
case Name:
6666
case LabelColor:
67-
case MetricUnit:
6867
case AutoLinkTargetContainer:
6968
case AutoLinkCategory:
7069
case RowId:
7170
return wrapColumn(alias, _rootTable.getColumn(column.toString()));
71+
case MetricUnit:
72+
{
73+
var columnInfo = wrapColumn(alias, _rootTable.getColumn(column.toString()));
74+
columnInfo.setLabel("Display Units");
75+
return columnInfo;
76+
}
7277
case Created:
7378
return wrapColumn(alias, _rootTable.getColumn("Created"));
7479
case CreatedBy:

‎experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ public void idColsSet_nameExpressionNull_hasNameProperty() throws Exception
390390
errors = new BatchValidationException();
391391
svc.insertRows(user, c, rows, errors, null, null);
392392
assertTrue(errors.hasErrors());
393-
assertTrue(errors.getMessage().contains("SampleID or Name is required for sample on row 1"));
393+
assertTrue(errors.getMessage().contains("SampleID or Name is required for sample"));
394394
}
395395
396396
// idCols not null, nameExpression not null, 'name' property (not used) -- fail

‎experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.apache.logging.log4j.Logger;
2424
import org.jetbrains.annotations.NotNull;
2525
import org.jetbrains.annotations.Nullable;
26+
import org.junit.Assert;
27+
import org.junit.Test;
2628
import org.labkey.api.action.ApiUsageException;
2729
import org.labkey.api.audit.AbstractAuditHandler;
2830
import org.labkey.api.audit.AbstractAuditTypeProvider;
@@ -42,6 +44,7 @@
4244
import org.labkey.api.data.Container;
4345
import org.labkey.api.data.ContainerFilter;
4446
import org.labkey.api.data.ContainerManager;
47+
import org.labkey.api.data.ConversionExceptionWithMessage;
4548
import org.labkey.api.data.DatabaseCache;
4649
import org.labkey.api.data.DbSchema;
4750
import org.labkey.api.data.DbScope;
@@ -94,6 +97,7 @@
9497
import org.labkey.api.inventory.InventoryService;
9598
import org.labkey.api.miniprofiler.MiniProfiler;
9699
import org.labkey.api.miniprofiler.Timing;
100+
import org.labkey.api.ontology.KindOfQuantity;
97101
import org.labkey.api.ontology.Quantity;
98102
import org.labkey.api.ontology.Unit;
99103
import org.labkey.api.qc.DataState;
@@ -174,6 +178,16 @@ public class SampleTypeServiceImpl extends AbstractAuditHandler implements Sampl
174178
public static final String SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:sampleCount";
175179
public static final String ROOT_SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:rootSampleCount";
176180

181+
public static final List<Unit> SUPPORTED_UNITS = new ArrayList<>();
182+
public static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the %s display units (%s).";
183+
184+
static
185+
{
186+
SUPPORTED_UNITS.addAll(KindOfQuantity.Volume.getCommonUnits());
187+
SUPPORTED_UNITS.addAll(KindOfQuantity.Mass.getCommonUnits());
188+
SUPPORTED_UNITS.addAll(KindOfQuantity.Count.getCommonUnits());
189+
}
190+
177191
// columns that may appear in a row when only the sample status is updating.
178192
public static final Set<String> statusUpdateColumns = Set.of(
179193
ExpMaterialTable.Column.Modified.name().toLowerCase(),
@@ -208,6 +222,44 @@ Cache<String, SortedSet<MaterialSource>> getMaterialSourceCache()
208222
return materialSourceCache;
209223
}
210224

225+
@Override @NotNull
226+
public List<Unit> getSupportedUnits()
227+
{
228+
return SUPPORTED_UNITS;
229+
}
230+
231+
@Nullable @Override
232+
public Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits, String sampleTypeName)
233+
{
234+
if (rawUnits == null)
235+
return null;
236+
if (rawUnits instanceof Unit u)
237+
{
238+
if (defaultUnits == null)
239+
return u;
240+
else if (u.getKindOfQuantity() != defaultUnits.getKindOfQuantity())
241+
throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits));
242+
else
243+
return u;
244+
}
245+
if (!(rawUnits instanceof String rawUnitsString))
246+
throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits));
247+
if (!StringUtils.isBlank(rawUnitsString))
248+
{
249+
rawUnitsString = rawUnitsString.trim();
250+
251+
Unit mUnit = Unit.fromName(rawUnitsString);
252+
List<Unit> commonUnits = getSupportedUnits();
253+
if (mUnit == null || !commonUnits.contains(mUnit))
254+
{
255+
throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + ".");
256+
}
257+
if (defaultUnits != null && mUnit.getKindOfQuantity() != defaultUnits.getKindOfQuantity())
258+
throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits));
259+
return mUnit;
260+
}
261+
return null;
262+
}
211263

212264
public void clearMaterialSourceCache(@Nullable Container c)
213265
{
@@ -2259,4 +2311,60 @@ public void refreshSampleTypeMaterializedView(@NotNull ExpSampleType st, SampleC
22592311
{
22602312
ExpMaterialTableImpl.refreshMaterializedView(st.getLSID(), reason);
22612313
}
2314+
2315+
2316+
public static class TestCase extends Assert
2317+
{
2318+
@Test
2319+
public void testGetValidatedUnit()
2320+
{
2321+
SampleTypeService service = SampleTypeService.get();
2322+
try
2323+
{
2324+
service.getValidatedUnit("g", Unit.mg, "Sample Type");
2325+
service.getValidatedUnit("g ", Unit.mg, "Sample Type");
2326+
service.getValidatedUnit(" g ", Unit.mg, "Sample Type");
2327+
}
2328+
catch (ConversionExceptionWithMessage e)
2329+
{
2330+
fail("Compatible unit should not throw exception.");
2331+
}
2332+
try
2333+
{
2334+
assertNull(service.getValidatedUnit(null, Unit.unit, "Sample Type"));
2335+
}
2336+
catch (ConversionExceptionWithMessage e)
2337+
{
2338+
fail("null units should be null");
2339+
}
2340+
try
2341+
{
2342+
assertNull(service.getValidatedUnit("", Unit.unit, "Sample Type"));
2343+
}
2344+
catch (ConversionExceptionWithMessage e)
2345+
{
2346+
fail("empty units should be null");
2347+
}
2348+
try
2349+
{
2350+
service.getValidatedUnit("g", Unit.unit, "Sample Type");
2351+
fail("Units that are not comparable should throw exception.");
2352+
}
2353+
catch (ConversionExceptionWithMessage ignore)
2354+
{
2355+
2356+
}
2357+
2358+
try
2359+
{
2360+
service.getValidatedUnit("nonesuch", Unit.unit, "Sample Type");
2361+
fail("Invalid units should throw exception.");
2362+
}
2363+
catch (ConversionExceptionWithMessage ignore)
2364+
{
2365+
2366+
}
2367+
2368+
}
2369+
}
22622370
}

0 commit comments

Comments
 (0)