GitHub Issue 947, 955 & 988

api/src/org/labkey/api/exp/property/DomainUtil.java

@@ -110,8 +110,10 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import static org.labkey.api.data.ColumnRenderPropertiesImpl.TEXT_CHOICE_CONCEPT_URI;
 import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior;
 import static org.labkey.api.gwt.client.ui.PropertyType.CALCULATED_CONCEPT_URI;
 import static org.labkey.api.util.StringExpressionFactory.SUBSTITUTION_EXP_PATTERN;
@@ -1430,6 +1432,38 @@ private static boolean _copyValidator(IPropertyValidator pv, GWTPropertyValidato
         return hasChange;
     }
 
+    // GitHub Issue 955: limit option length to 200
+    private static final int TEXT_CHOICE_MAX_VALUE_LENGTH = 200;
+    // GitHub Issue 988: don't allow json array like values for text choice options
+    private static final Pattern JSON_FILTER_VALUE_PATTERN = Pattern.compile("\\{json:\\s*\\[.*]}", Pattern.DOTALL);
+
+    /**
+     * Validate text choice options for a field. Returns an error message if invalid, or null if valid.
+     */
+    private static @Nullable String validateTextChoiceOptions(GWTPropertyDescriptor field)
+    {
+        for (GWTPropertyValidator validator : field.getPropertyValidators())
+        {
+            if (PropertyValidatorType.TextChoice.equals(validator.getType()))
+            {
+                String expression = validator.getExpression();
+                List<String> options = PageFlowUtil.splitStringToValues(expression != null ? expression : "", '|');
+                for (String option : options)
+                {
+                    if (option.length() > TEXT_CHOICE_MAX_VALUE_LENGTH)
+                    {
+                        return "Text choice value for field '" + field.getName() + "' must not exceed " + TEXT_CHOICE_MAX_VALUE_LENGTH + " characters: '" + StringUtils.abbreviate(option, 50) + "'";
+                    }
+                    if (JSON_FILTER_VALUE_PATTERN.matcher(option).matches())
+                    {
+                        return "Text choice value for field '" + field.getName() + "' must not use the reserved format '{json:[...]}': '" + StringUtils.abbreviate(option, 50) + "'";
+                    }
+                }
+            }
+        }
+        return null;
+    }
+
     private static String getDomainErrorMessage(@Nullable GWTDomain<?> domain, String message)
     {
         if (domain != null && domain.getName() != null)
@@ -1477,6 +1511,16 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N
                 continue;
             }
 
+            if (PropertyType.MULTI_CHOICE.getTypeUri().equals(field.getRangeURI()) || TEXT_CHOICE_CONCEPT_URI.equals(field.getConceptURI()))
+            {
+                String textChoiceError = validateTextChoiceOptions(field);
+                if (textChoiceError != null)
+                {
+                    exception.addFieldError(name, getDomainErrorMessage(updates, textChoiceError));
+                    continue;
+                }
+            }
+
             Matcher expMatcher = SUBSTITUTION_EXP_PATTERN.matcher(name);
             if (expMatcher.find())
             {

core/webapp/internal/ViewDesigner/field/FilterTextValueUtil.js

@@ -16,10 +16,18 @@ Ext4.define('LABKEY.internal.ViewDesigner.field.FilterTextValueUtil', {
         // convert the filter value into a user-editable string using filter.getURLParameterValue()
         var valueString = filter.getURLParameterValue();
 
-        // replace ; with \n on UI
+        // Display multi-valued filters with one value per line in the textarea.
+        // getURLParameterValue() returns {json:[...]} when values contain the ';' separator,
+        // so we need to parse that format before converting to newline-separated display.
         if (filterType.isMultiValued() && (urlSuffix !== 'notbetween' && urlSuffix !== 'between')) {
-            if (typeof valueString === 'string' && valueString.indexOf('\n') === -1 && valueString.indexOf(';') > 0)
-                valueString = valueString.replaceAll(';', '\n');
+            if (typeof valueString === 'string') {
+                var parsed = this._parseJsonFilterValue(valueString);
+                if (parsed !== null) {
+                    valueString = parsed.join('\n');
+                } else if (valueString.indexOf('\n') === -1 && valueString.indexOf(';') > 0) {
+                    valueString = valueString.replaceAll(';', '\n');
+                }
+            }
         }
 
         this.setValue(valueString);
@@ -48,15 +56,55 @@ Ext4.define('LABKEY.internal.ViewDesigner.field.FilterTextValueUtil', {
 
     setRecordValue : function (valueString) {
         // parse the value string into parts for multi-value filters
+        var filterValue;
         try {
+            // For multi-valued filters (excluding between/notbetween), values are displayed
+            // one per line in the textarea. When saving, split by newline only and encode
+            // using {json:[...]} if any value contains the separator character (e.g. ';')
+            // to prevent parseValue from incorrectly splitting those values.
+            var op = this.record.get("items")[this.clauseIndex].op;
+            var filterType = LABKEY.Filter.getFilterTypeForURLSuffix(op);
+            var urlSuffix = filterType ? filterType.getURLSuffix() : null;
+
+            if (filterType && filterType.isMultiValued() && typeof valueString === 'string'
+                    && urlSuffix !== 'notbetween' && urlSuffix !== 'between') {
+                var sep = filterType.getMultiValueSeparator();
+                var values = valueString.split('\n');
+                if (sep && values.some(function(v) { return v.indexOf(sep) !== -1; })) {
+                    valueString = '{json:' + JSON.stringify(values) + '}';
+                } else {
+                    valueString = values.join(sep);
+                }
+            }
+
             var filter = this.createFilter(valueString);
-            var filterValue = filter.getValue();
+            filterValue = filter.getValue();
         }
         catch (e) {
             console.warn("Error parsing filter value: " + valueString);
             filterValue = valueString;
         }
 
         this.record.get("items")[this.clauseIndex].value = filterValue;
+    },
+
+    /**
+     * GitHub Issue 947: Multi value text choice values with semicolon mangled in LKS grid view editor
+     * Parse a {json:[...]} encoded filter value string.
+     * Returns the parsed array, or null if the string is not in {json:...} format.
+     */
+    _parseJsonFilterValue : function (valueString) {
+        if (typeof valueString === 'string'
+                && valueString.indexOf('{json:') === 0
+                && valueString.lastIndexOf('}') === valueString.length - 1) {
+            try {
+                var parsed = JSON.parse(valueString.substring('{json:'.length, valueString.length - 1));
+                if (Array.isArray(parsed))
+                    return parsed;
+            } catch (e) {
+                // Not valid JSON, return null to fall through to default handling
+            }
+        }
+        return null;
     }
 });