diff --git a/src/org/labkey/test/components/ui/grids/EditableGrid.java b/src/org/labkey/test/components/ui/grids/EditableGrid.java index 562cd704e6..b58765eadf 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,7 @@ public void waitForReady() */ public static String quoteForPaste(String... values) { - return Arrays.stream(values).map(CSVFormat.DEFAULT::format).collect(Collectors.joining(",")); + return quoteValues(",", values); } public void clickDelete() 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 8544e75329..bad2b39ff2 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; @@ -86,7 +87,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 +1744,132 @@ 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)); + + // --- 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 5 audit events (3 inserts + 2 updates)", 5, auditEvents.size()); + + Set foundInsertAuditValues = new HashSet<>(); + // Track each update as a [oldValue, newValue] pair; order across updates is not guaranteed. + List updateAuditPairs = new ArrayList<>(); + + 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)) + { + 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}); + } + } + + // Insert row 1: "1" needs no escaping. + checker().verifyTrue("Insert row 1: plain value not in audit", + 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(_auditLogHelper.joinMultiChoiceForAudit(quotedValue1))); + // Insert row 3: "2" sorts before 1: """2""", 1. + checker().verifyTrue("Insert row 3: mixed values not in audit", + foundInsertAuditValues.contains(_auditLogHelper.joinMultiChoiceForAudit(quotedValue1, plainValue))); + + 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(); } + 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<>(); 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); + } + }