Merge branch 'develop' into fb_typeConversionMVTC

Author: DariaBod

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()

src/org/labkey/test/pages/query/UpdateQueryRowPage.java

@@ -119,7 +119,10 @@ public UpdateQueryRowPage setField(String fieldName, List<String> 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;
     }
 

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("'><script>alert(\":(\")</script>'");
         String columnName = TestDataGenerator.randomFieldName("MultiChoiceField");
-        List<String> 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<String> 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<String> 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<Map<String, Object>> auditEvents = getListAuditEventsSince(encodedListName, baselineRowId);
+        assertEquals("Expected 5 audit events (3 inserts + 2 updates)", 5, auditEvents.size());
+
+        Set<String> foundInsertAuditValues = new HashSet<>();
+        // Track each update as a [oldValue, newValue] pair; order across updates is not guaranteed.
+        List<String[]> updateAuditPairs = new ArrayList<>();
+
+        for (Map<String, Object> 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<Map<String, Object>> getListAuditEventsSince(String listName, int previousRowId)
+            throws IOException, CommandException
+    {
+        List<Filter> 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<String> getQueryFormFieldNamesDecoded()
     {
         ArrayList<String> ret = new ArrayList<>();

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<String, Object> 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);
+    }
+
 }