From 4ec48da6d1992668a7161dd56b940b1f8001145c Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 9 Oct 2026 16:11:03 -0500 Subject: [PATCH 1/6] Record user-provided amounts under a new audit metadata key, new entries use :::user_provided:::Amount. --- api/src/org/labkey/api/audit/AuditHandler.java | 2 ++ .../experiment/api/SampleProvidedAmountAuditTestCase.java | 4 ++-- .../labkey/experiment/api/SampleTypeUpdateServiceDI.java | 8 +++++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index ce7a237cc4d..e66cbae97ff 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -53,6 +53,8 @@ public interface AuditHandler { String PROVIDED_DATA_PREFIX = ":::provided:::"; String DELTA_PROVIDED_DATA_PREFIX = ":::delta_provided:::"; + // GH Issue 1640: sample timeline metadata recorded under PROVIDED_DATA_PREFIX before 26.7 may be wrong, so user-entered amounts now use this prefix + String USER_PROVIDED_DATA_PREFIX = ":::user_provided:::"; void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment); diff --git a/experiment/src/org/labkey/experiment/api/SampleProvidedAmountAuditTestCase.java b/experiment/src/org/labkey/experiment/api/SampleProvidedAmountAuditTestCase.java index 374605fbbc5..37d08069c79 100644 --- a/experiment/src/org/labkey/experiment/api/SampleProvidedAmountAuditTestCase.java +++ b/experiment/src/org/labkey/experiment/api/SampleProvidedAmountAuditTestCase.java @@ -49,7 +49,7 @@ import java.util.List; import java.util.Map; -import static org.labkey.api.audit.AuditHandler.PROVIDED_DATA_PREFIX; +import static org.labkey.api.audit.AuditHandler.USER_PROVIDED_DATA_PREFIX; import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; import static org.labkey.api.exp.query.ExpMaterialTable.Column.Units; @@ -60,7 +60,7 @@ public class SampleProvidedAmountAuditTestCase extends Assert { private static final int ROW_COUNT = 2500; // spans three audit batches - private static final String PROVIDED_AMOUNT_KEY = PROVIDED_DATA_PREFIX + StoredAmount.label(); + private static final String PROVIDED_AMOUNT_KEY = USER_PROVIDED_DATA_PREFIX + StoredAmount.label(); private static Container _c; private static User _user; diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index c333bf633ae..921d2b9d702 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -136,6 +136,7 @@ import static java.util.Collections.emptyMap; import static org.labkey.api.audit.AuditHandler.DELTA_PROVIDED_DATA_PREFIX; import static org.labkey.api.audit.AuditHandler.PROVIDED_DATA_PREFIX; +import static org.labkey.api.audit.AuditHandler.USER_PROVIDED_DATA_PREFIX; import static org.labkey.api.data.TableSelector.ALL_COLUMNS; import static org.labkey.api.dataiterator.DataIteratorUtil.DUPLICATE_COLUMN_IN_DATA_ERROR; import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs; @@ -379,14 +380,19 @@ protected Map extractProvidedAmountsAndUnits(@NotNull Map result = new HashMap<>(); String unitsStr = ""; String prefix; + String metadataPrefix; if (dataRow.containsKey(DELTA_PROVIDED_DATA_PREFIX + StoredAmount.name())) + { prefix = DELTA_PROVIDED_DATA_PREFIX; + metadataPrefix = DELTA_PROVIDED_DATA_PREFIX; + } else { // with no sample type display unit, no conversion will happen if (_sampleType == null || _sampleType.getMetricUnit() == null) return null; prefix = PROVIDED_DATA_PREFIX; + metadataPrefix = USER_PROVIDED_DATA_PREFIX; } Object amountVal = dataRow.get(prefix + StoredAmount.name()); if (amountVal == null) @@ -395,7 +401,7 @@ protected Map extractProvidedAmountsAndUnits(@NotNull Map Date: Fri, 9 Oct 2026 16:13:08 -0500 Subject: [PATCH 2/6] Flag legacy provided amounts in sample timeline audit metadata --- .../labkey/experiment/ExperimentModule.java | 2 + .../samples/SampleTimelineAuditProvider.java | 4 + .../SampleTimelineMetadataDisplayColumn.java | 104 ++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index c30ae2e102d..f0bb83522c2 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -166,6 +166,7 @@ import org.labkey.experiment.samples.DataClassFolderWriter; import org.labkey.experiment.samples.SampleStatusFolderImporter; import org.labkey.experiment.samples.SampleTimelineAuditProvider; +import org.labkey.experiment.samples.SampleTimelineMetadataDisplayColumn; import org.labkey.experiment.samples.SampleTypeFolderImporter; import org.labkey.experiment.samples.SampleTypeFolderWriter; import org.labkey.experiment.security.DataClassDesignerRole; @@ -1170,6 +1171,7 @@ public Collection getSummary(Container c) LsidUtils.TestCase.class, PropertyController.TestCase.class, Quantity.TestCase.class, + SampleTimelineMetadataDisplayColumn.TestCase.class, Unit.TestCase.class ); } diff --git a/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java b/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java index 598064024ad..50315729ad5 100644 --- a/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java +++ b/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java @@ -140,6 +140,10 @@ else if (COLUMN_NAME_USER_COMMENT.equalsIgnoreCase(col.getName())) { col.setLabel("Reason"); } + else if (METADATA_COLUMN_NAME.equalsIgnoreCase(col.getName())) + { + col.setDisplayColumnFactory(SampleTimelineMetadataDisplayColumn::new); + } } }; table.setTitleColumn(SAMPLE_NAME_COLUMN_NAME); diff --git a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java new file mode 100644 index 00000000000..c54a31cf8cb --- /dev/null +++ b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.experiment.samples; + +import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.DataColumn; +import org.labkey.api.data.RenderContext; + +import java.util.Map; + +import static org.labkey.api.audit.AuditHandler.DELTA_PROVIDED_DATA_PREFIX; +import static org.labkey.api.audit.AuditHandler.PROVIDED_DATA_PREFIX; +import static org.labkey.api.audit.AuditHandler.USER_PROVIDED_DATA_PREFIX; +import static org.labkey.api.audit.SampleTimelineAuditEvent.SAMPLE_TIMELINE_EVENT_TYPE; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; + +/** GH Issue 1640: flags metadata that has a provided amount recorded before 26.7, which may be wrong for large imports. */ +public class SampleTimelineMetadataDisplayColumn extends DataColumn +{ + private static final String LEGACY_PROVIDED_AMOUNT_KEY = PROVIDED_DATA_PREFIX + StoredAmount.label(); + private static final String WARNING_STYLE = "background-color:#fcf8e3"; + private static final String WARNING_TITLE = "Provided Amount May Be Inaccurate"; + private static final String WARNING_MESSAGE = "This provided amount was recorded before release 26.7 and may be inaccurate for operations of more than 1,000 rows. The stored sample amount is not affected."; + + public SampleTimelineMetadataDisplayColumn(ColumnInfo col) + { + super(col); + } + + static boolean containsLegacyProvidedAmount(@Nullable Object metadata) + { + return metadata instanceof String str + && AbstractAuditTypeProvider.decodeFromDataMap(str).containsKey(LEGACY_PROVIDED_AMOUNT_KEY); + } + + private boolean hasLegacyProvidedAmount(RenderContext ctx) + { + return containsLegacyProvidedAmount(getValue(ctx)); + } + + @Override + public @NotNull String getCssStyle(RenderContext ctx) + { + String style = super.getCssStyle(ctx); + if (!hasLegacyProvidedAmount(ctx)) + return style; + return StringUtils.isEmpty(style) ? WARNING_STYLE : style + ";" + WARNING_STYLE; + } + + @Override + protected String getHoverContent(RenderContext ctx) + { + return hasLegacyProvidedAmount(ctx) ? WARNING_MESSAGE : super.getHoverContent(ctx); + } + + @Override + protected String getHoverTitle(RenderContext ctx) + { + return hasLegacyProvidedAmount(ctx) ? WARNING_TITLE : super.getHoverTitle(ctx); + } + + public static class TestCase extends Assert + { + private static String metadata(String key, String value) + { + return AbstractAuditTypeProvider.encodeForDataMap(Map.of(key, value, SAMPLE_TIMELINE_EVENT_TYPE, "INSERT")); + } + + @Test + public void testLegacyProvidedAmountIsFlagged() + { + assertTrue(containsLegacyProvidedAmount(metadata(PROVIDED_DATA_PREFIX + StoredAmount.label(), "2 L"))); + } + + @Test + public void testOtherMetadataIsNotFlagged() + { + assertFalse(containsLegacyProvidedAmount(metadata(USER_PROVIDED_DATA_PREFIX + StoredAmount.label(), "2 L"))); + assertFalse(containsLegacyProvidedAmount(metadata(DELTA_PROVIDED_DATA_PREFIX + StoredAmount.label(), "-1 mL"))); + assertFalse(containsLegacyProvidedAmount(AbstractAuditTypeProvider.encodeForDataMap(Map.of(SAMPLE_TIMELINE_EVENT_TYPE, "INSERT")))); + assertFalse(containsLegacyProvidedAmount("")); + assertFalse(containsLegacyProvidedAmount(null)); + } + } +} From a4f2fe2ddec60a791aa11e41d58a7282838367c9 Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 9 Oct 2026 16:34:14 -0500 Subject: [PATCH 3/6] update the tooltip warning message to remove release version number --- api/src/org/labkey/api/audit/AuditHandler.java | 2 +- .../samples/SampleTimelineMetadataDisplayColumn.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index e66cbae97ff..fc7870749d5 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -53,7 +53,7 @@ public interface AuditHandler { String PROVIDED_DATA_PREFIX = ":::provided:::"; String DELTA_PROVIDED_DATA_PREFIX = ":::delta_provided:::"; - // GH Issue 1640: sample timeline metadata recorded under PROVIDED_DATA_PREFIX before 26.7 may be wrong, so user-entered amounts now use this prefix + // GH Issue 1640: values under PROVIDED_DATA_PREFIX were written before the large-import fix and may be wrong, so user-entered amounts now use this prefix String USER_PROVIDED_DATA_PREFIX = ":::user_provided:::"; void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment); diff --git a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java index c54a31cf8cb..8bd37693bff 100644 --- a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java +++ b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java @@ -33,13 +33,13 @@ import static org.labkey.api.audit.SampleTimelineAuditEvent.SAMPLE_TIMELINE_EVENT_TYPE; import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; -/** GH Issue 1640: flags metadata that has a provided amount recorded before 26.7, which may be wrong for large imports. */ +/** GH Issue 1640: flags metadata that has a provided amount recorded before the large-import fix, which may be wrong. */ public class SampleTimelineMetadataDisplayColumn extends DataColumn { private static final String LEGACY_PROVIDED_AMOUNT_KEY = PROVIDED_DATA_PREFIX + StoredAmount.label(); private static final String WARNING_STYLE = "background-color:#fcf8e3"; private static final String WARNING_TITLE = "Provided Amount May Be Inaccurate"; - private static final String WARNING_MESSAGE = "This provided amount was recorded before release 26.7 and may be inaccurate for operations of more than 1,000 rows. The stored sample amount is not affected."; + private static final String WARNING_MESSAGE = "This provided amount was recorded before a fix for operations of more than 1,000 rows and may be inaccurate. The stored sample amount is not affected."; public SampleTimelineMetadataDisplayColumn(ColumnInfo col) { From a34e2cb7e8df3bbeb5774c991aabf0a3bd437efa Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 9 Oct 2026 16:38:43 -0500 Subject: [PATCH 4/6] update the tooltip warning message to remove release version number --- .../experiment/samples/SampleTimelineMetadataDisplayColumn.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java index 8bd37693bff..390481ddc18 100644 --- a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java +++ b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java @@ -39,7 +39,7 @@ public class SampleTimelineMetadataDisplayColumn extends DataColumn private static final String LEGACY_PROVIDED_AMOUNT_KEY = PROVIDED_DATA_PREFIX + StoredAmount.label(); private static final String WARNING_STYLE = "background-color:#fcf8e3"; private static final String WARNING_TITLE = "Provided Amount May Be Inaccurate"; - private static final String WARNING_MESSAGE = "This provided amount was recorded before a fix for operations of more than 1,000 rows and may be inaccurate. The stored sample amount is not affected."; + private static final String WARNING_MESSAGE = "This provided amount was recorded before a fix for insert or update operations of more than 1,000 rows and may be inaccurate. The stored sample amount is not affected."; public SampleTimelineMetadataDisplayColumn(ColumnInfo col) { From 4309987bebeac2b5e9a6d91039ad703d8f39c82f Mon Sep 17 00:00:00 2001 From: cnathe Date: Mon, 12 Oct 2026 09:14:57 -0500 Subject: [PATCH 5/6] update the tooltip warning message --- .../experiment/samples/SampleTimelineMetadataDisplayColumn.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java index 390481ddc18..271643a99c1 100644 --- a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java +++ b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java @@ -39,7 +39,7 @@ public class SampleTimelineMetadataDisplayColumn extends DataColumn private static final String LEGACY_PROVIDED_AMOUNT_KEY = PROVIDED_DATA_PREFIX + StoredAmount.label(); private static final String WARNING_STYLE = "background-color:#fcf8e3"; private static final String WARNING_TITLE = "Provided Amount May Be Inaccurate"; - private static final String WARNING_MESSAGE = "This provided amount was recorded before a fix for insert or update operations of more than 1,000 rows and may be inaccurate. The stored sample amount is not affected."; + private static final String WARNING_MESSAGE = "This provided amount value may be inaccurate if recorded during an import or update of more than 1,000 samples. To confirm the provided amount, check the original import file or source data. The stored sample amount is not affected."; public SampleTimelineMetadataDisplayColumn(ColumnInfo col) { From 78da3c59661557b482b4f061f813f873c505a921 Mon Sep 17 00:00:00 2001 From: cnathe Date: Mon, 12 Oct 2026 13:55:30 -0500 Subject: [PATCH 6/6] update sample timeline warning to be more accessible --- .../org/labkey/api/audit/AuditHandler.java | 1 + .../SampleTimelineMetadataDisplayColumn.java | 23 ++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index fc7870749d5..48ba87d5c16 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -51,6 +51,7 @@ public interface AuditHandler { + // Still the key prefix for provided values sent in with rows; only the audit metadata moved to USER_PROVIDED_DATA_PREFIX String PROVIDED_DATA_PREFIX = ":::provided:::"; String DELTA_PROVIDED_DATA_PREFIX = ":::delta_provided:::"; // GH Issue 1640: values under PROVIDED_DATA_PREFIX were written before the large-import fix and may be wrong, so user-entered amounts now use this prefix diff --git a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java index 271643a99c1..b10c91a457d 100644 --- a/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java +++ b/experiment/src/org/labkey/experiment/samples/SampleTimelineMetadataDisplayColumn.java @@ -24,21 +24,27 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.DataColumn; import org.labkey.api.data.RenderContext; +import org.labkey.api.writer.HtmlWriter; import java.util.Map; import static org.labkey.api.audit.AuditHandler.DELTA_PROVIDED_DATA_PREFIX; -import static org.labkey.api.audit.AuditHandler.PROVIDED_DATA_PREFIX; import static org.labkey.api.audit.AuditHandler.USER_PROVIDED_DATA_PREFIX; import static org.labkey.api.audit.SampleTimelineAuditEvent.SAMPLE_TIMELINE_EVENT_TYPE; import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; +import static org.labkey.api.util.DOM.Attribute.aria_hidden; +import static org.labkey.api.util.DOM.Attribute.style; +import static org.labkey.api.util.DOM.SPAN; +import static org.labkey.api.util.DOM.at; +import static org.labkey.api.util.DOM.cl; /** GH Issue 1640: flags metadata that has a provided amount recorded before the large-import fix, which may be wrong. */ public class SampleTimelineMetadataDisplayColumn extends DataColumn { - private static final String LEGACY_PROVIDED_AMOUNT_KEY = PROVIDED_DATA_PREFIX + StoredAmount.label(); + private static final String LEGACY_PROVIDED_AMOUNT_KEY = ":::provided:::" + StoredAmount.label(); private static final String WARNING_STYLE = "background-color:#fcf8e3"; private static final String WARNING_TITLE = "Provided Amount May Be Inaccurate"; + private static final String WARNING_SR_TEXT = "Warning: provided amount may be inaccurate. "; private static final String WARNING_MESSAGE = "This provided amount value may be inaccurate if recorded during an import or update of more than 1,000 samples. To confirm the provided amount, check the original import file or source data. The stored sample amount is not affected."; public SampleTimelineMetadataDisplayColumn(ColumnInfo col) @@ -66,6 +72,17 @@ private boolean hasLegacyProvidedAmount(RenderContext ctx) return StringUtils.isEmpty(style) ? WARNING_STYLE : style + ";" + WARNING_STYLE; } + @Override + public void renderGridCellContents(RenderContext ctx, HtmlWriter out) + { + if (hasLegacyProvidedAmount(ctx)) + { + SPAN(at(aria_hidden, "true").cl("fa fa-exclamation-triangle text-warning").at(style, "margin-right:4px")).appendTo(out); + SPAN(cl("sr-only"), WARNING_SR_TEXT).appendTo(out); + } + super.renderGridCellContents(ctx, out); + } + @Override protected String getHoverContent(RenderContext ctx) { @@ -88,7 +105,7 @@ private static String metadata(String key, String value) @Test public void testLegacyProvidedAmountIsFlagged() { - assertTrue(containsLegacyProvidedAmount(metadata(PROVIDED_DATA_PREFIX + StoredAmount.label(), "2 L"))); + assertTrue(containsLegacyProvidedAmount(metadata(LEGACY_PROVIDED_AMOUNT_KEY, "2 L"))); } @Test