From 95d7c8c960228670dde891883b9fc217815426cd Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 17 Apr 2026 13:01:53 -0700 Subject: [PATCH 1/3] GitHub Issue 1073: Updating a List MVTC field shows array in audit for values with quotes --- .../components/ui/grids/EditableGrid.java | 12 +- src/org/labkey/test/tests/list/ListTest.java | 110 +++++++++++++++--- 2 files changed, 104 insertions(+), 18 deletions(-) diff --git a/src/org/labkey/test/components/ui/grids/EditableGrid.java b/src/org/labkey/test/components/ui/grids/EditableGrid.java index 562cd704e6..41bbff8ed4 100644 --- a/src/org/labkey/test/components/ui/grids/EditableGrid.java +++ b/src/org/labkey/test/components/ui/grids/EditableGrid.java @@ -100,6 +100,11 @@ public void waitForReady() Locators.spinner.waitForElementToDisappear(this, 30000); } + public static String quoteValues(String delimiter, String... sorted) + { + return Arrays.stream(sorted).map(CSVFormat.DEFAULT::format).collect(Collectors.joining(delimiter)); + } + /** * Quote values to be pasted into lookup columns. Prevents a value containing a comma from being interpreted as * multiple values. @@ -108,7 +113,12 @@ public void waitForReady() */ public static String quoteForPaste(String... values) { - return Arrays.stream(values).map(CSVFormat.DEFAULT::format).collect(Collectors.joining(",")); + return quoteValues(",", values); + } + + public static String joinMultiChoiceForExport(String... sorted) + { + return quoteValues(", ", sorted); } public void clickDelete() diff --git a/src/org/labkey/test/tests/list/ListTest.java b/src/org/labkey/test/tests/list/ListTest.java index 8544e75329..503e04ec2e 100644 --- a/src/org/labkey/test/tests/list/ListTest.java +++ b/src/org/labkey/test/tests/list/ListTest.java @@ -28,6 +28,7 @@ import org.labkey.remoteapi.domain.DomainResponse; import org.labkey.remoteapi.domain.PropertyDescriptor; import org.labkey.remoteapi.domain.SaveDomainCommand; +import org.labkey.remoteapi.query.ContainerFilter; import org.labkey.remoteapi.query.Filter; import org.labkey.serverapi.reader.TabLoader; import org.labkey.test.BaseWebDriverTest; @@ -46,6 +47,7 @@ import org.labkey.test.components.domain.DomainFormPanel; import org.labkey.test.components.ext4.Checkbox; import org.labkey.test.components.list.AdvancedListSettingsDialog; +import org.labkey.test.components.ui.grids.EditableGrid; import org.labkey.test.pages.ImportDataPage; import org.labkey.test.pages.list.EditListDefinitionPage; import org.labkey.test.pages.list.GridPage; @@ -86,7 +88,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -1744,42 +1745,117 @@ public void testAutoIncrementKeyEncoded() } @Test - public void testMultiChoiceValues() + public void testMultiChoiceValues() throws IOException, CommandException { OptionalFeatureHelper.enableOptionalFeature(getCurrentTest().createDefaultConnection(), "multiChoiceDataType"); Assume.assumeTrue("Multi-choice text fields are only supported on PostgreSQL", WebTestHelper.getDatabaseType() == WebTestHelper.DatabaseType.PostgreSQL); - // setup a list with an auto-increment key and multi text choice field + // Setup a list with an auto-increment key and a multi-value text choice field. String encodedListName = TestDataGenerator.randomDomainName("multiChoiceList", DomainUtils.DomainKind.IntList); String keyName = TestDataGenerator.randomFieldName("'>'"); String columnName = TestDataGenerator.randomFieldName("MultiChoiceField"); - List tcValues = List.of("~`!@#$%^&*()_+=[]{}\\|';:\"<>?,./", "1", "2"); + String plainValue = "1"; // no special characters + String quotedValue1 = "\"2\""; // literal string: "2" (contains double-quote chars) + String quotedValue2 = "\"3\""; // literal string: "3" + List tcValues = List.of(plainValue, quotedValue1, quotedValue2); _listHelper.createList(PROJECT_VERIFY, encodedListName, keyName, col(columnName, ColumnType.MultiValueTextChoice) .setMultiChoiceValues(tcValues)); _listHelper.goToList(encodedListName); + // Capture baseline after list creation so the "list created" event is not counted below. + int baselineRowId = _auditLogHelper.getLatestAuditRowId(AuditLogHelper.AuditEvent.LIST_AUDIT_EVENT.getName()); + DataRegionTable table = new DataRegionTable("query", getDriver()); + + // --- Insert row 1: single non-quoted value --- UpdateQueryRowPage insertNewRow = table.clickInsertNewRow(); - List valuesToChoose = tcValues.subList(1, 3); - insertNewRow.setField(columnName, valuesToChoose); + insertNewRow.setField(columnName, List.of(plainValue)); + insertNewRow.submit(); + checker().withScreenshot().verifyEquals("Row 1: display not as expected", plainValue, table.getDataAsText(0, columnName)); + + // --- Insert row 2: single quoted value --- + insertNewRow = table.clickInsertNewRow(); + insertNewRow.setField(columnName, List.of(quotedValue1)); + insertNewRow.submit(); + // The cell displays the literal string including the double-quote characters. + checker().withScreenshot().verifyEquals("Row 2: display not as expected", quotedValue1, table.getDataAsText(1, columnName)); + + // --- Insert row 3: mixed (plain + quoted) --- + insertNewRow = table.clickInsertNewRow(); + insertNewRow.setField(columnName, List.of(plainValue, quotedValue1)); insertNewRow.submit(); - String expectedList = valuesToChoose.stream() - .sorted() - .collect(Collectors.joining(" ")); - checker().withScreenshot().verifyEquals("Multi choice value not as expected", expectedList, table.getDataAsText(0, columnName)); + // MultiChoice.Array sorts case-insensitively; '"' (ASCII 34) < '1' (ASCII 49), so "2" sorts before 1. + String mixedDisplay = quotedValue1 + " " + plainValue; + checker().withScreenshot().verifyEquals("Row 3: display not as expected", mixedDisplay, table.getDataAsText(2, columnName)); + // --- Update row 0: change from plain "1" to quoted "3" --- UpdateQueryRowPage editRow = table.clickEditRow(0); - valuesToChoose = tcValues.subList(1, 3); - editRow.setField(columnName, valuesToChoose); + editRow.setField(columnName, List.of(quotedValue2)); editRow.submit(); - expectedList = valuesToChoose.stream() - .sorted() - .collect(Collectors.joining(" ")); - // verify the multi choice value is persisted - checker().withScreenshot().verifyEquals("Multi choice value not as expected", expectedList, table.getDataAsText(0, columnName)); + checker().withScreenshot().verifyEquals("Row 0 after update: display not as expected", + quotedValue2, table.getDataAsText(0, columnName)); + + // GitHub Issue 1073: multi-choice values containing quotes were stored as raw + // PostgreSQL array syntax (e.g. {"2"}) instead of the proper export-encoded format (e.g. """2"""). + List> auditEvents = getListAuditEventsSince(encodedListName, baselineRowId); + assertEquals("Expected 4 audit events (3 inserts + 1 update)", 4, auditEvents.size()); + + Set foundInsertAuditValues = new HashSet<>(); + String updateOldAuditValue = null; + String updateNewAuditValue = null; + + for (Map event : auditEvents) + { + String comment = (String) event.get("Comment"); + String newMapRaw = (String) event.get("NewRecordMap"); + String oldMapRaw = (String) event.get("OldRecordMap"); + + if ("A new list record was inserted".equals(comment) && newMapRaw != null) + { + foundInsertAuditValues.add(AuditLogHelper.decodeValues(newMapRaw).get(columnName)); + } + else if ("An existing list record was modified".equals(comment)) + { + if (oldMapRaw != null) updateOldAuditValue = AuditLogHelper.decodeValues(oldMapRaw).get(columnName); + if (newMapRaw != null) updateNewAuditValue = AuditLogHelper.decodeValues(newMapRaw).get(columnName); + } + } + + // Insert row 1: "1" needs no escaping. + checker().verifyTrue("Insert row 1: plain value not in audit", + foundInsertAuditValues.contains(EditableGrid.joinMultiChoiceForExport(plainValue))); + // Insert row 2: "2" contains double-quotes, escaped as: """2""". + checker().verifyTrue("Insert row 2: quoted value not in audit", + foundInsertAuditValues.contains(EditableGrid.joinMultiChoiceForExport(quotedValue1))); + // Insert row 3: "2" sorts before 1: """2""", 1. + checker().verifyTrue("Insert row 3: mixed values not in audit", + foundInsertAuditValues.contains(EditableGrid.joinMultiChoiceForExport(quotedValue1, plainValue))); + + // Update: old = "1", new = """3""". + checker().verifyEquals("Update: old audit value not as expected", + EditableGrid.joinMultiChoiceForExport(plainValue), updateOldAuditValue); + checker().verifyEquals("Update: new audit value not as expected", + EditableGrid.joinMultiChoiceForExport(quotedValue2), updateNewAuditValue); _listHelper.deleteList(); } + private List> getListAuditEventsSince(String listName, int previousRowId) + throws IOException, CommandException + { + List filters = List.of( + new Filter("ListName", listName, Filter.Operator.EQUAL), + new Filter("RowId", previousRowId, Filter.Operator.GT) + ); + + return _auditLogHelper.getAuditLogsFromLKS(getProjectName(), + AuditLogHelper.AuditEvent.LIST_AUDIT_EVENT, + List.of("RowId", "Comment", "OldRecordMap", "NewRecordMap"), + filters, + null, + ContainerFilter.CurrentAndSubfolders + ).getRows(); + } + private List getQueryFormFieldNamesDecoded() { ArrayList ret = new ArrayList<>(); From 7ffc7ea7f15a4e4e679b2cee157760eddb398495 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 17 Apr 2026 13:12:35 -0700 Subject: [PATCH 2/3] CC feedback --- .../test/components/ui/grids/EditableGrid.java | 5 ----- src/org/labkey/test/tests/list/ListTest.java | 13 ++++++------- src/org/labkey/test/util/AuditLogHelper.java | 8 ++++++++ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/org/labkey/test/components/ui/grids/EditableGrid.java b/src/org/labkey/test/components/ui/grids/EditableGrid.java index 41bbff8ed4..b58765eadf 100644 --- a/src/org/labkey/test/components/ui/grids/EditableGrid.java +++ b/src/org/labkey/test/components/ui/grids/EditableGrid.java @@ -116,11 +116,6 @@ public static String quoteForPaste(String... values) return quoteValues(",", values); } - public static String joinMultiChoiceForExport(String... sorted) - { - return quoteValues(", ", sorted); - } - public void clickDelete() { doAndWaitForRowCountUpdate(() -> elementCache().deleteRowsBtn.click()); diff --git a/src/org/labkey/test/tests/list/ListTest.java b/src/org/labkey/test/tests/list/ListTest.java index 503e04ec2e..b04ae9593f 100644 --- a/src/org/labkey/test/tests/list/ListTest.java +++ b/src/org/labkey/test/tests/list/ListTest.java @@ -47,7 +47,6 @@ import org.labkey.test.components.domain.DomainFormPanel; import org.labkey.test.components.ext4.Checkbox; import org.labkey.test.components.list.AdvancedListSettingsDialog; -import org.labkey.test.components.ui.grids.EditableGrid; import org.labkey.test.pages.ImportDataPage; import org.labkey.test.pages.list.EditListDefinitionPage; import org.labkey.test.pages.list.GridPage; @@ -1756,7 +1755,7 @@ public void testMultiChoiceValues() throws IOException, CommandException String plainValue = "1"; // no special characters String quotedValue1 = "\"2\""; // literal string: "2" (contains double-quote chars) String quotedValue2 = "\"3\""; // literal string: "3" - List tcValues = List.of(plainValue, quotedValue1, quotedValue2); + List tcValues = List.of("~`!@#$%^&*()_+=[]{}\\|';:\"<>?,./", plainValue, quotedValue1, quotedValue2); _listHelper.createList(PROJECT_VERIFY, encodedListName, keyName, col(columnName, ColumnType.MultiValueTextChoice) .setMultiChoiceValues(tcValues)); _listHelper.goToList(encodedListName); @@ -1822,19 +1821,19 @@ else if ("An existing list record was modified".equals(comment)) // Insert row 1: "1" needs no escaping. checker().verifyTrue("Insert row 1: plain value not in audit", - foundInsertAuditValues.contains(EditableGrid.joinMultiChoiceForExport(plainValue))); + foundInsertAuditValues.contains(_auditLogHelper.joinMultiChoiceForAudit(plainValue))); // Insert row 2: "2" contains double-quotes, escaped as: """2""". checker().verifyTrue("Insert row 2: quoted value not in audit", - foundInsertAuditValues.contains(EditableGrid.joinMultiChoiceForExport(quotedValue1))); + foundInsertAuditValues.contains(_auditLogHelper.joinMultiChoiceForAudit(quotedValue1))); // Insert row 3: "2" sorts before 1: """2""", 1. checker().verifyTrue("Insert row 3: mixed values not in audit", - foundInsertAuditValues.contains(EditableGrid.joinMultiChoiceForExport(quotedValue1, plainValue))); + foundInsertAuditValues.contains(_auditLogHelper.joinMultiChoiceForAudit(quotedValue1, plainValue))); // Update: old = "1", new = """3""". checker().verifyEquals("Update: old audit value not as expected", - EditableGrid.joinMultiChoiceForExport(plainValue), updateOldAuditValue); + _auditLogHelper.joinMultiChoiceForAudit(plainValue), updateOldAuditValue); checker().verifyEquals("Update: new audit value not as expected", - EditableGrid.joinMultiChoiceForExport(quotedValue2), updateNewAuditValue); + _auditLogHelper.joinMultiChoiceForAudit(quotedValue2), updateNewAuditValue); _listHelper.deleteList(); } diff --git a/src/org/labkey/test/util/AuditLogHelper.java b/src/org/labkey/test/util/AuditLogHelper.java index 16f7a5b5db..0ae9f6fb61 100644 --- a/src/org/labkey/test/util/AuditLogHelper.java +++ b/src/org/labkey/test/util/AuditLogHelper.java @@ -15,6 +15,7 @@ import org.labkey.remoteapi.query.Sort; import org.labkey.test.Locator; import org.labkey.test.WebDriverWrapper; +import org.labkey.test.components.ui.grids.EditableGrid; import org.labkey.test.pages.core.admin.ShowAdminPage; import org.labkey.test.pages.core.admin.ShowAuditLogPage; @@ -869,4 +870,11 @@ private Integer getLogColumnIntValue(Map rowEntry, String column throw new IllegalArgumentException(je); } } + + // returns the expected audit-log serialization format for multi-choice values + public String joinMultiChoiceForAudit(String... sorted) + { + return EditableGrid.quoteValues(", ", sorted); + } + } From 5349fb96eaa4ebabc0c7f7abf5782b7bbe41d835 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 17 Apr 2026 13:46:08 -0700 Subject: [PATCH 3/3] add test for clearing mvtc field values --- .../test/pages/query/UpdateQueryRowPage.java | 5 ++- src/org/labkey/test/tests/list/ListTest.java | 35 +++++++++++++------ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/org/labkey/test/pages/query/UpdateQueryRowPage.java b/src/org/labkey/test/pages/query/UpdateQueryRowPage.java index 98b87d52bf..e4d35cc87a 100644 --- a/src/org/labkey/test/pages/query/UpdateQueryRowPage.java +++ b/src/org/labkey/test/pages/query/UpdateQueryRowPage.java @@ -119,7 +119,10 @@ public UpdateQueryRowPage setField(String fieldName, List values) { Select field = elementCache().getMultiChoiceSelect(fieldName); field.deselectAll(); - values.forEach(field::selectByVisibleText); + if (values != null && !values.isEmpty()) + values.forEach(field::selectByVisibleText); + else + field.selectByIndex(0); // the 1st option is a blank option to remove existing values return this; } diff --git a/src/org/labkey/test/tests/list/ListTest.java b/src/org/labkey/test/tests/list/ListTest.java index b04ae9593f..bad2b39ff2 100644 --- a/src/org/labkey/test/tests/list/ListTest.java +++ b/src/org/labkey/test/tests/list/ListTest.java @@ -1793,14 +1793,21 @@ public void testMultiChoiceValues() throws IOException, CommandException checker().withScreenshot().verifyEquals("Row 0 after update: display not as expected", quotedValue2, table.getDataAsText(0, columnName)); + // --- Update row 1: change from quoted value to blank --- + editRow = table.clickEditRow(1); + editRow.setField(columnName, List.of()); + editRow.submit(); + checker().withScreenshot().verifyEquals("Row 1 after clearing: display not as expected", + "", table.getDataAsText(1, columnName)); + // GitHub Issue 1073: multi-choice values containing quotes were stored as raw // PostgreSQL array syntax (e.g. {"2"}) instead of the proper export-encoded format (e.g. """2"""). List> auditEvents = getListAuditEventsSince(encodedListName, baselineRowId); - assertEquals("Expected 4 audit events (3 inserts + 1 update)", 4, auditEvents.size()); + assertEquals("Expected 5 audit events (3 inserts + 2 updates)", 5, auditEvents.size()); Set foundInsertAuditValues = new HashSet<>(); - String updateOldAuditValue = null; - String updateNewAuditValue = null; + // Track each update as a [oldValue, newValue] pair; order across updates is not guaranteed. + List updateAuditPairs = new ArrayList<>(); for (Map event : auditEvents) { @@ -1814,8 +1821,9 @@ public void testMultiChoiceValues() throws IOException, CommandException } else if ("An existing list record was modified".equals(comment)) { - if (oldMapRaw != null) updateOldAuditValue = AuditLogHelper.decodeValues(oldMapRaw).get(columnName); - if (newMapRaw != null) updateNewAuditValue = AuditLogHelper.decodeValues(newMapRaw).get(columnName); + String oldVal = oldMapRaw != null ? AuditLogHelper.decodeValues(oldMapRaw).get(columnName) : null; + String newVal = newMapRaw != null ? AuditLogHelper.decodeValues(newMapRaw).get(columnName) : null; + updateAuditPairs.add(new String[]{oldVal, newVal}); } } @@ -1829,11 +1837,18 @@ else if ("An existing list record was modified".equals(comment)) checker().verifyTrue("Insert row 3: mixed values not in audit", foundInsertAuditValues.contains(_auditLogHelper.joinMultiChoiceForAudit(quotedValue1, plainValue))); - // Update: old = "1", new = """3""". - checker().verifyEquals("Update: old audit value not as expected", - _auditLogHelper.joinMultiChoiceForAudit(plainValue), updateOldAuditValue); - checker().verifyEquals("Update: new audit value not as expected", - _auditLogHelper.joinMultiChoiceForAudit(quotedValue2), updateNewAuditValue); + assertEquals("Expected 2 update audit events", 2, updateAuditPairs.size()); + + // Update row 0: old = "1", new = """3""". + String expectedUpdate0Old = _auditLogHelper.joinMultiChoiceForAudit(plainValue); + String expectedUpdate0New = _auditLogHelper.joinMultiChoiceForAudit(quotedValue2); + checker().verifyTrue("Update row 0: audit pair not found", + updateAuditPairs.stream().anyMatch(p -> expectedUpdate0Old.equals(p[0]) && expectedUpdate0New.equals(p[1]))); + + // Update row 1: old = """2""", new = null (clearing all selections removes the field from the audit record). + String expectedUpdate1Old = _auditLogHelper.joinMultiChoiceForAudit(quotedValue1); + checker().verifyTrue("Update row 1 (remove value): audit pair not found", + updateAuditPairs.stream().anyMatch(p -> expectedUpdate1Old.equals(p[0]) && p[1] == null)); _listHelper.deleteList(); }