Skip to content

Update Amounts and Units for better sorting and filtering behavior #2678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cnathe merged 7 commits into from
Sep 15, 2025
Merged

Update Amounts and Units for better sorting and filtering behavior #2678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dd8ae10
SampleTypeTest test cases for amounts and units (error when missing a…
cnathe Sep 9, 2025
8b9c0c1
SampleTypeTest test cases for amounts and units sorting and filtering…
cnathe Sep 9, 2025
a03d796
EditableGridForm cancelChanges()
cnathe Sep 10, 2025
532e0b2
Merge branch 'develop' into fb_amountsAndUnits
cnathe Sep 10, 2025
e188855
QueryApiHelper selectRows param for ContainerFilter
cnathe Sep 11, 2025
e7349a2
Merge remote-tracking branch 'origin/develop' into fb_amountsAndUnits
labkey-susanh Sep 12, 2025
d7bde04
Merge branch 'develop' into fb_amountsAndUnits
cnathe Sep 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/org/labkey/test/components/ui/grids/EditableGridForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,12 @@ public T saveChanges()
return getComponentAfterSave();
}

public T cancelChanges()
{
Locator.tagWithClass("button", "btn-default").withText("Cancel").findElement(_outerScope).click();
getWrapper().shortWait().until(ExpectedConditions.stalenessOf(getComponentElement()));
return getComponentAfterSave();
}

protected abstract T getComponentAfterSave();
}
2 changes: 1 addition & 1 deletion src/org/labkey/test/params/experiment/SampleTypeDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ protected MetricUnit getInventoryMetricUnit()
return _inventoryMetricUnit;
}

protected SampleTypeDefinition setInventoryMetricUnit(MetricUnit inventoryMetricUnit)
public SampleTypeDefinition setInventoryMetricUnit(MetricUnit inventoryMetricUnit)
{
_inventoryMetricUnit = inventoryMetricUnit;
return this;
Expand Down
178 changes: 178 additions & 0 deletions src/org/labkey/test/tests/SampleTypeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.labkey.remoteapi.query.SelectRowsResponse;
import org.labkey.test.BaseWebDriverTest;
import org.labkey.test.Locator;
import org.labkey.test.SortDirection;
import org.labkey.test.TestFileUtils;
import org.labkey.test.WebTestHelper;
import org.labkey.test.categories.Daily;
Expand All @@ -51,6 +52,7 @@
import org.labkey.test.params.FieldDefinition.ColumnType;
import org.labkey.test.params.FieldDefinition.LookupInfo;
import org.labkey.test.params.FieldInfo;
import org.labkey.test.params.experiment.MetricUnit;
import org.labkey.test.params.experiment.SampleTypeDefinition;
import org.labkey.test.util.ApiPermissionsHelper;
import org.labkey.test.util.AuditLogHelper;
Expand Down Expand Up @@ -1858,6 +1860,150 @@ public void testFieldUniqueConstraint()
assertTextNotPresent("unique_constraint_test_fieldname_2");
}

@Test
public void testAmountsAndUnitsWithDisplayUnit()
{
SampleTypeHelper sampleHelper = new SampleTypeHelper(this);
String sampleTypeName = "Sample Amounts and Units with Display Unit Test";

clickProject(PROJECT_NAME);
SampleTypeDefinition sampleTypeDefinition = new SampleTypeDefinition(sampleTypeName);
sampleTypeDefinition.setInventoryMetricUnit(TestMetricUnit.L);
SampleTypeAPIHelper.createEmptySampleType(getProjectName(), sampleTypeDefinition);
refresh();
sampleHelper.goToSampleType(sampleTypeName);

log("verify error when inserting a row with an amount but no unit");
sampleHelper.insertRow(Map.of("Name", "AU-ERR-1", "StoredAmount", "0.0"));
assertTextPresent("No Units value provided for Amount 0.0.");
clickButton("Cancel");
log("verify error when inserting a row with a unit but no amount");
sampleHelper.insertRow(Map.of("Name", "AU-ERR-2", "Units", "mL"));
assertTextPresent("No Amount value provided for Units mL.");
clickButton("Cancel");

log("verify error when inserting a row with incompatible units");
sampleHelper.insertRow(Map.of("Name", "AU-ERR-1", "StoredAmount", "5.0", "Units", "mg"));
assertTextPresent("Units value (mg) is not compatible with the display units (mL).");
clickButton("Cancel");
sampleHelper.insertRow(Map.of("Name", "AU-ERR-1", "StoredAmount", "5.0", "Units", "unit"));
assertTextPresent("Units value (unit) is not compatible with the display units (mL).");
clickButton("Cancel");

log("verify inserting a row with compatible units succeeds and are converted");
sampleHelper.insertRow(Map.of("Name", "AU-SUCCESS-1", "StoredAmount", "1.0", "Units", "mL"));
verifySampleAmountUnitValues("AU-SUCCESS-1", "0.001", "L");
sampleHelper.insertRow(Map.of("Name", "AU-SUCCESS-2", "StoredAmount", "0.002", "Units", "L"));
verifySampleAmountUnitValues("AU-SUCCESS-2", "0.002", "L");
sampleHelper.insertRow(Map.of("Name", "AU-SUCCESS-3", "StoredAmount", "3000", "Units", "uL"));
verifySampleAmountUnitValues("AU-SUCCESS-3", "0.003", "L");

log("verify updating a row with incompatible units fails");
sampleHelper.updateRow(0, Map.of("Units", "mg"));
assertTextPresent("Units value (mg) is not compatible with the display units (mL).");
clickButton("Cancel");

log("verify updating a row with compatible units succeeds and are converted");
sampleHelper.updateRow(2, Map.of("StoredAmount", "0.00123", "Units", "mL"));
verifySampleAmountUnitValues("AU-SUCCESS-1", "1.23E-6", "L");

log("verify rounding precision for display units");
sampleHelper.updateRow(2, Map.of("StoredAmount", "0.12345678999", "Units", "L"));
verifySampleAmountUnitValues("AU-SUCCESS-1", "0.12345679", "L");

log("verify bulk import with incompatible units fails");
sampleHelper.bulkImportExpectingError(List.of(Map.of("Name", "AU-BULK-ERR-1", "StoredAmount", "0", "Units", "kg")), SampleTypeHelper.IMPORT_OPTION);
assertTextPresent("Units value (kg) is not compatible with the display units (mL).");
clickButton("Cancel");

log("verify bulk import with compatible units succeeds and are converted");
sampleHelper.bulkImport(List.of(Map.of("Name", "AU-BULK-SUCCESS-1", "StoredAmount", "0", "Units", "mL")));
verifySampleAmountUnitValues("AU-BULK-SUCCESS-1", "0.0", "L");
sampleHelper.bulkImport(List.of(Map.of("Name", "AU-BULK-SUCCESS-2", "StoredAmount", "0.005", "Units", "L")));
verifySampleAmountUnitValues("AU-BULK-SUCCESS-2", "0.005", "L");
sampleHelper.bulkImport(List.of(Map.of("Name", "AU-BULK-SUCCESS-3", "StoredAmount", "4000", "Units", "uL")));
verifySampleAmountUnitValues("AU-BULK-SUCCESS-3", "0.004", "L");

log("verify sorting on converted amounts works as expected");
sampleHelper.getSamplesDataRegionTable().setSort("Amount", SortDirection.ASC);
assertEquals("Sample order sorted asc not as expected",
List.of("AU-BULK-SUCCESS-1", "AU-SUCCESS-2", "AU-SUCCESS-3", "AU-BULK-SUCCESS-3", "AU-BULK-SUCCESS-2", "AU-SUCCESS-1"),
sampleHelper.getSamplesDataRegionTable().getColumnDataAsText("Name"));

log("verify filtering on converted amounts works as expected");
sampleHelper.getSamplesDataRegionTable().setFilter("Amount", "Is Greater Than", "0.004");
assertEquals("Sample order filtered not as expected",
List.of("AU-BULK-SUCCESS-2", "AU-SUCCESS-1"),
sampleHelper.getSamplesDataRegionTable().getColumnDataAsText("Name"));
}

@Test
public void testAmountsAndUnitsWithoutDisplayUnit()
{
SampleTypeHelper sampleHelper = new SampleTypeHelper(this);
String sampleTypeName = "Sample Amounts and Units without Display Unit Test";

clickProject(PROJECT_NAME);
CreateSampleTypePage createPage = sampleHelper
.goToCreateNewSampleType()
.setName(sampleTypeName);
assertTextNotPresent("Amount Display Units");
createPage.clickSave();
sampleHelper.goToSampleType(sampleTypeName);

log("verify that inserting a row with an amount or unit requires both fields to be filled in");
// insert row with amount but not unit (error expected)
sampleHelper.insertRow(Map.of("Name", "AU-ERR-1", "StoredAmount", "5.0"));
assertTextPresent("No Units value provided for Amount 5.0.");
clickButton("Cancel");
// insert row with unit but not amount (error expected)
sampleHelper.insertRow(Map.of("Name", "AU-ERR-2", "Units", "mg"));
assertTextPresent("No Amount value provided for Units mg.");
clickButton("Cancel");
// insert row with both amount and unit (success)
sampleHelper.insertRow(Map.of("Name", "AU-SUCCESS-1", "StoredAmount", "5.0", "Units", "mg"));
verifySampleAmountUnitValues("AU-SUCCESS-1", "5", "mg");

log("verify that updating a row with an amount or unit requires both fields to be filled in");
// update row with amount but not unit (error expected)
sampleHelper.updateRow(0, Map.of("Units", ""));
assertTextPresent("No Units value provided for Amount 5.0.");
clickButton("Cancel");
// update row with unit but not amount (error expected)
sampleHelper.updateRow(0, Map.of("StoredAmount", ""));
assertTextPresent("No Amount value provided for Units mg.");
clickButton("Cancel");
// update row with both amount and unit (success)
sampleHelper.updateRow(0, Map.of("StoredAmount", "10.0123", "Units", "g"));
verifySampleAmountUnitValues("AU-SUCCESS-1", "10.0123", "g");

log("verify that bulk import with an amount or unit requires both fields to be filled in");
// bulk import with amount but not unit (error expected)
sampleHelper.bulkImportExpectingError(List.of(Map.of("Name", "AU-BULK-ERR-1", "StoredAmount", "0")), SampleTypeHelper.IMPORT_OPTION);
assertTextPresent("When adding or updating samples, a Units value must be provided when there is a value for Amount.");
clickButton("Cancel");
// bulk import with unit but not amount (error expected)
sampleHelper.bulkImportExpectingError(List.of(Map.of("Name", "AU-BULK-ERR-2", "Units", "mL")), SampleTypeHelper.IMPORT_OPTION);
assertTextPresent("When adding or updating samples, a Amount value must be provided when there is a value for Units.");
clickButton("Cancel");
// bulk import with both amount and unit (success expected)
sampleHelper.bulkImport(List.of(Map.of("Name", "AU-BULK-SUCCESS-1", "StoredAmount", "0", "Units", "L")));
verifySampleAmountUnitValues("AU-BULK-SUCCESS-1", "0", "L");

log("verify the bulk import with RawAmount and RawUnits are ignored");
sampleHelper.bulkImport(List.of(Map.of("Name", "AU-BULK-SUCCESS-2", "RawAmount", "1000", "RawUnits", "kg")));
verifySampleAmountUnitValues("AU-BULK-SUCCESS-2", " ", " ");
}

private void verifySampleAmountUnitValues(String name, String expectedAmount, String expectedUnits)
{
DataRegionTable drt = DataRegionTable.findDataRegionWithinWebpart(this, "Sample Type Contents");
drt.setFilter("Name", "Equals", name);
checker().verifyEquals("StoredAmount value not as expected for sample " + name, expectedAmount, drt.getDataAsText(0, "Amount"));
checker().verifyEquals("Units value not as expected for sample " + name, expectedUnits, drt.getDataAsText(0, "Units"));
drt.clearAllFilters();
}

private void viewRawTableMetadata(String sampleTypeName)
{
beginAt(WebTestHelper.buildURL("query", getProjectName(), "rawTableMetaData", Map.of("schemaName", "samples", "query.queryName", sampleTypeName)));
Expand Down Expand Up @@ -1922,4 +2068,36 @@ public BrowserType bestBrowser()
{
return BrowserType.CHROME;
}

public enum TestMetricUnit implements MetricUnit
{
G("g", "g (grams)"),
MG("mg", "mg (milligrams)"),
KG("kg", "kg (kilograms)"),
ML("mL", "mL (milliliters)"),
UL("uL", "uL (microliters)"),
L("L", "L (liters)"),
UNIT("unit", "unit");
Comment on lines +2074 to +2080
Copy link
Contributor

@labkey-danield labkey-danield Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is InventoryMetricUnit enum class in the Inventory module. Maybe that should be pulled out to the TestAutomation module?

I don't think you need to change this, just tossing an idea out.

Copy link
Contributor Author

@cnathe cnathe Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think InventoryMetricUnit should be moved to testAutomation next to the interface it implements. But I am going to hold off an make that change in a separate PR since there are a lot of usages and I want to reduce the churn on this current PR.


private final String _value;
private final String _label;

TestMetricUnit(String value, String label)
{
_value = value;
_label = label;
}

@Override
public String getLabel()
{
return _label;
}

@Override
public String getValue()
{
return _value;
}
}
}
11 changes: 11 additions & 0 deletions src/org/labkey/test/util/SampleTypeHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,17 @@ public void insertRow(Map<String, String> fieldValues)
clickButton("Submit");
}

public void updateRow(int rowIndex, Map<String, String> fieldValues)
{
DataRegionTable drt = getSamplesDataRegionTable();
drt.clickEditRow(rowIndex);
for (Map.Entry<String, String> fieldValue : fieldValues.entrySet())
{
setFormElement(Locator.name("quf_" + fieldValue.getKey()), fieldValue.getValue());
}
clickButton("Submit");
}

public void bulkImport(File dataFile)
{
fileImport(dataFile, IMPORT_OPTION);
Expand Down
20 changes: 10 additions & 10 deletions src/org/labkey/test/util/query/QueryApiHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.labkey.remoteapi.domain.DropDomainCommand;
import org.labkey.remoteapi.domain.GetDomainDetailsCommand;
import org.labkey.remoteapi.query.BaseRowsCommand;
import org.labkey.remoteapi.query.ContainerFilter;
import org.labkey.remoteapi.query.DeleteRowsCommand;
import org.labkey.remoteapi.query.Filter;
import org.labkey.remoteapi.query.ImportDataCommand;
Expand Down Expand Up @@ -71,21 +72,20 @@ public SelectRowsResponse selectRows(List<String> columns, @Nullable List<Filter

public SelectRowsResponse selectRows(List<String> columns, @Nullable List<Filter> filters, @Nullable List<Sort> sorts) throws IOException, CommandException
{
SelectRowsCommand cmd = new SelectRowsCommand(_schema, _query);
return selectRows(columns, filters, sorts, null);
}

if(filters != null)
{
public SelectRowsResponse selectRows(List<String> columns, @Nullable List<Filter> filters, @Nullable List<Sort> sorts, @Nullable ContainerFilter cf) throws IOException, CommandException
{
SelectRowsCommand cmd = new SelectRowsCommand(_schema, _query);
if (filters != null)
cmd.setFilters(new ArrayList<>(filters));
}

if(sorts != null)
{
if (sorts != null)
cmd.setSorts(sorts);
}

if (columns!=null)
cmd.setColumns(columns);

if (cf != null)
cmd.setContainerFilter(cf);
return cmd.execute(_connection, _containerPath);
}

Expand Down