fix: address second round of review findings

- Tests: replace conditional `if response.status_code == 200` guards
  with unconditional `assert response.status_code == 200` so failures
  are not silently swallowed
- dotToNested: guard finalKey write with `if (i < parts.length)` to
  prevent empty-string key pollution when tail-matching consumed all
  parts
- Extract normalizeFormDataForConfig() helper from handlePluginConfigSubmit
  and call it from both handlePluginConfigSubmit and syncFormToJson so
  the JSON tab sync uses the same robust FormData processing (including
  _data JSON inputs, bracket-notation checkboxes, array-of-objects,
  file-upload widgets, checkbox DOM detection, and unchecked boolean
  handling via collectBooleanFields)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

test/test_web_api.py

@@ -583,3 +583,130 @@ class TestAPIErrorHandling:
         response = client.get('/api/v3/display/on-demand/start')
         
         assert response.status_code in [200, 405]  # Depends on implementation
+
+
+class TestDottedKeyNormalization:
+    """Regression tests for fix_array_structures / ensure_array_defaults with dotted schema keys."""
+
+    def test_save_plugin_config_dotted_key_arrays(self, client, mock_config_manager):
+        """Nested dotted-key objects with numeric-keyed dicts are converted to arrays."""
+        from web_interface.blueprints.api_v3 import api_v3
+
+        api_v3.config_manager = mock_config_manager
+        mock_config_manager.load_config.return_value = {}
+
+        schema_mgr = MagicMock()
+        schema = {
+            'type': 'object',
+            'properties': {
+                'leagues': {
+                    'type': 'object',
+                    'properties': {
+                        'eng.1': {
+                            'type': 'object',
+                            'properties': {
+                                'enabled': {'type': 'boolean', 'default': True},
+                                'favorite_teams': {
+                                    'type': 'array',
+                                    'items': {'type': 'string'},
+                                    'default': [],
+                                },
+                            },
+                        },
+                    },
+                },
+            },
+        }
+        schema_mgr.load_schema.return_value = schema
+        schema_mgr.generate_default_config.return_value = {
+            'leagues': {'eng.1': {'enabled': True, 'favorite_teams': []}},
+        }
+        schema_mgr.merge_with_defaults.side_effect = lambda config, defaults: {**defaults, **config}
+        schema_mgr.validate_config_against_schema.return_value = []
+        api_v3.schema_manager = schema_mgr
+
+        request_data = {
+            'plugin_id': 'soccer-scoreboard',
+            'config': {
+                'leagues': {
+                    'eng.1': {
+                        'enabled': True,
+                        'favorite_teams': ['Arsenal', 'Chelsea'],
+                    },
+                },
+            },
+        }
+
+        response = client.post(
+            '/api/v3/plugins/config',
+            data=json.dumps(request_data),
+            content_type='application/json',
+        )
+
+        assert response.status_code == 200, f"Expected 200, got {response.status_code}: {response.data}"
+        saved = mock_config_manager.save_config_atomic.call_args[0][0]
+        soccer_cfg = saved.get('soccer-scoreboard', {})
+        leagues = soccer_cfg.get('leagues', {})
+        assert 'eng.1' in leagues, f"Expected 'eng.1' key, got: {list(leagues.keys())}"
+        assert isinstance(leagues['eng.1'].get('favorite_teams'), list)
+        assert leagues['eng.1']['favorite_teams'] == ['Arsenal', 'Chelsea']
+
+    def test_save_plugin_config_none_array_gets_default(self, client, mock_config_manager):
+        """None array fields under dotted-key parents are replaced with defaults."""
+        from web_interface.blueprints.api_v3 import api_v3
+
+        api_v3.config_manager = mock_config_manager
+        mock_config_manager.load_config.return_value = {}
+
+        schema_mgr = MagicMock()
+        schema = {
+            'type': 'object',
+            'properties': {
+                'leagues': {
+                    'type': 'object',
+                    'properties': {
+                        'eng.1': {
+                            'type': 'object',
+                            'properties': {
+                                'favorite_teams': {
+                                    'type': 'array',
+                                    'items': {'type': 'string'},
+                                    'default': [],
+                                },
+                            },
+                        },
+                    },
+                },
+            },
+        }
+        schema_mgr.load_schema.return_value = schema
+        schema_mgr.generate_default_config.return_value = {
+            'leagues': {'eng.1': {'favorite_teams': []}},
+        }
+        schema_mgr.merge_with_defaults.side_effect = lambda config, defaults: {**defaults, **config}
+        schema_mgr.validate_config_against_schema.return_value = []
+        api_v3.schema_manager = schema_mgr
+
+        request_data = {
+            'plugin_id': 'soccer-scoreboard',
+            'config': {
+                'leagues': {
+                    'eng.1': {
+                        'favorite_teams': None,
+                    },
+                },
+            },
+        }
+
+        response = client.post(
+            '/api/v3/plugins/config',
+            data=json.dumps(request_data),
+            content_type='application/json',
+        )
+
+        assert response.status_code == 200, f"Expected 200, got {response.status_code}: {response.data}"
+        saved = mock_config_manager.save_config_atomic.call_args[0][0]
+        soccer_cfg = saved.get('soccer-scoreboard', {})
+        teams = soccer_cfg.get('leagues', {}).get('eng.1', {}).get('favorite_teams')
+        assert isinstance(teams, list), f"Expected list, got: {type(teams)}"
+        assert teams == [], f"Expected empty default list, got: {teams}"

web_interface/blueprints/api_v3.py

@@ -4113,8 +4113,7 @@ def save_plugin_config():
                         nested_dict = config_dict.get(prop_key)
 
                         if nested_dict is None:
-                            if prop_key not in config_dict:
-                                config_dict[prop_key] = {}
+                            config_dict[prop_key] = {}
                             nested_dict = config_dict[prop_key]
 
                         if isinstance(nested_dict, dict):

web_interface/static/v3/plugins_manager.js

@@ -2334,6 +2334,15 @@ function dotToNested(obj, schema) {
         while (i < parts.length - 1) {
             let matched = false;
             if (currentSchema) {
+                // First, check if the full remaining tail is a leaf property
+                // (e.g., "eng.1" as a complete dotted key with no sub-properties)
+                const tailCandidate = parts.slice(i).join('.');
+                if (tailCandidate in currentSchema) {
+                    current[tailCandidate] = obj[key];
+                    matched = true;
+                    i = parts.length; // consumed all parts
+                    break;
+                }
                 // Try progressively longer candidates (longest first) to greedily
                 // match dotted property names like "eng.1"
                 for (let j = parts.length - 1; j > i; j--) {
@@ -2369,8 +2378,11 @@ function dotToNested(obj, schema) {
         }
 
         // Set the final key (remaining parts joined — may itself be dotted)
-        const finalKey = parts.slice(i).join('.');
-        current[finalKey] = obj[key];
+        // Skip if tail-matching already consumed all parts and wrote the value
+        if (i < parts.length) {
+            const finalKey = parts.slice(i).join('.');
+            current[finalKey] = obj[key];
+        }
     }
 
     return result;
@@ -2395,42 +2407,20 @@ function collectBooleanFields(schema, prefix = '') {
     return boolFields;
 }
 
-function handlePluginConfigSubmit(e) {
-    e.preventDefault();
-    console.log('Form submitted');
-    
-    if (!currentPluginConfig) {
-        showNotification('Plugin configuration not loaded', 'error');
-        return;
-    }
-    
-    const pluginId = currentPluginConfig.pluginId;
-    const schema = currentPluginConfig.schema;
-    const form = e.target;
-    
-    // Fix invalid hidden fields before submission
-    // This prevents "invalid form control is not focusable" errors
-    const allInputs = form.querySelectorAll('input[type="number"]');
-    allInputs.forEach(input => {
-        const min = parseFloat(input.getAttribute('min'));
-        const max = parseFloat(input.getAttribute('max'));
-        const value = parseFloat(input.value);
-        
-        if (!isNaN(value)) {
-            if (!isNaN(min) && value < min) {
-                input.value = min;
-            } else if (!isNaN(max) && value > max) {
-                input.value = max;
-            }
-        }
-    });
-    
+/**
+ * Normalize FormData from a plugin config form into a nested config object.
+ * Handles _data JSON inputs, bracket-notation checkboxes, array-of-objects,
+ * file-upload widgets, proper checkbox DOM detection, unchecked boolean
+ * handling, and schema-aware dotted-key nesting.
+ *
+ * @param {HTMLFormElement} form - The form element (needed for checkbox DOM detection)
+ * @param {Object|null} schema - The plugin's JSON Schema
+ * @returns {Object} Nested config object ready for saving
+ */
+function normalizeFormDataForConfig(form, schema) {
     const formData = new FormData(form);
     const flatConfig = {};
-    
-    console.log('Schema loaded:', schema ? 'Yes' : 'No');
-    
-    // Process form data with type conversion (using dot notation for nested fields)
+
     for (const [key, value] of formData.entries()) {
         // Check if this is a patternProperties or array-of-objects hidden input (contains JSON data)
         // Only match keys ending with '_data' to avoid false positives like 'meta_data_field'
@@ -2442,36 +2432,35 @@ function handlePluginConfigSubmit(e) {
                 // Only treat as JSON-backed when it's a non-null object (null is typeof 'object' in JavaScript)
                 if (jsonValue !== null && typeof jsonValue === 'object') {
                     flatConfig[baseKey] = jsonValue;
-                    console.log(`JSON data field ${baseKey}: parsed ${Array.isArray(jsonValue) ? 'array' : 'object'}`, jsonValue);
                     continue; // Skip normal processing for JSON data fields
                 }
             } catch (e) {
                 // Not valid JSON, continue with normal processing
             }
         }
-        
+
         // Skip checkbox-group inputs with bracket notation (they're handled by the hidden _data input)
         // Pattern: fieldName[] - these are individual checkboxes, actual data is in fieldName_data
         if (key.endsWith('[]')) {
             continue;
         }
-        
+
         // Skip key_value pair inputs (they're handled by the hidden _data input)
         if (key.includes('[key_') || key.includes('[value_')) {
             continue;
         }
-        
+
         // Skip array-of-objects per-item inputs (they're handled by the hidden _data input)
         // Pattern: feeds_item_0_name, feeds_item_1_url, etc.
         if (key.includes('_item_') && /_item_\d+_/.test(key)) {
             continue;
         }
-        
+
         // Try to get schema property - handle both dot notation and underscore notation
         let propSchema = getSchemaPropertyType(schema, key);
         let actualKey = key;
         let actualValue = value;
-        
+
         // If not found with dots, try converting underscores to dots (for nested fields)
         if (!propSchema && key.includes('_')) {
             const dotKey = key.replace(/_/g, '.');
@@ -2482,10 +2471,10 @@ function handlePluginConfigSubmit(e) {
                 actualValue = value;
             }
         }
-        
+
         if (propSchema) {
             const propType = propSchema.type;
-            
+
             if (propType === 'array') {
                 // Check if this is a file upload widget (JSON array)
                 if (propSchema['x-widget'] === 'file-upload') {
@@ -2499,11 +2488,10 @@ function handlePluginConfigSubmit(e) {
                             tempDiv.innerHTML = actualValue;
                             decodedValue = tempDiv.textContent || tempDiv.innerText || actualValue;
                         }
-                        
+
                         const jsonValue = JSON.parse(decodedValue);
                         if (Array.isArray(jsonValue)) {
                             flatConfig[actualKey] = jsonValue;
-                            console.log(`File upload array field ${actualKey}: parsed JSON array with ${jsonValue.length} items`);
                         } else {
                             // Fallback to comma-separated
                             const arrayValue = decodedValue ? decodedValue.split(',').map(v => v.trim()).filter(v => v) : [];
@@ -2513,13 +2501,11 @@ function handlePluginConfigSubmit(e) {
                         // Not JSON, use comma-separated
                         const arrayValue = actualValue ? actualValue.split(',').map(v => v.trim()).filter(v => v) : [];
                         flatConfig[actualKey] = arrayValue;
-                        console.log(`Array field ${actualKey}: "${actualValue}" -> `, arrayValue);
                     }
                 } else {
                     // Regular array: convert comma-separated string to array
                     const arrayValue = actualValue ? actualValue.split(',').map(v => v.trim()).filter(v => v) : [];
                     flatConfig[actualKey] = arrayValue;
-                    console.log(`Array field ${actualKey}: "${actualValue}" -> `, arrayValue);
                 }
             } else if (propType === 'integer') {
                 flatConfig[actualKey] = parseInt(actualValue, 10);
@@ -2530,14 +2516,13 @@ function handlePluginConfigSubmit(e) {
                 // Escape special CSS selector characters in the name
                 const escapedKey = escapeCssSelector(key);
                 const formElement = form.querySelector(`input[type="checkbox"][name="${escapedKey}"]`);
-                
+
                 if (formElement) {
                     // Element found - use its checked state
                     flatConfig[actualKey] = formElement.checked;
                 } else {
                     // Element not found - normalize string booleans and check FormData value
                     // Checkboxes send "on" when checked, nothing when unchecked
-                    // Normalize string representations of booleans
                     if (typeof actualValue === 'string') {
                         const lowerValue = actualValue.toLowerCase().trim();
                         if (lowerValue === 'true' || lowerValue === '1' || lowerValue === 'on') {
@@ -2545,13 +2530,11 @@ function handlePluginConfigSubmit(e) {
                         } else if (lowerValue === 'false' || lowerValue === '0' || lowerValue === 'off' || lowerValue === '') {
                             flatConfig[actualKey] = false;
                         } else {
-                            // Non-empty string that's not a boolean representation - treat as truthy
                             flatConfig[actualKey] = true;
                         }
                     } else if (actualValue === undefined || actualValue === null) {
                         flatConfig[actualKey] = false;
                     } else {
-                        // Non-string value - coerce to boolean
                         flatConfig[actualKey] = Boolean(actualValue);
                     }
                 }
@@ -2568,10 +2551,9 @@ function handlePluginConfigSubmit(e) {
                     const tempDiv = document.createElement('div');
                     tempDiv.innerHTML = actualValue;
                     decodedValue = tempDiv.textContent || tempDiv.innerText || actualValue;
-                    
+
                     const parsed = JSON.parse(decodedValue);
                     flatConfig[actualKey] = parsed;
-                    console.log(`No schema for ${actualKey}, but parsed as JSON:`, parsed);
                 } catch (e) {
                     // Not valid JSON, save as string
                     flatConfig[actualKey] = actualValue;
@@ -2580,12 +2562,10 @@ function handlePluginConfigSubmit(e) {
                 // No schema - try to detect checkbox by finding the element
                 const escapedKey = escapeCssSelector(key);
                 const formElement = form.querySelector(`input[type="checkbox"][name="${escapedKey}"]`);
-                
+
                 if (formElement && formElement.type === 'checkbox') {
-                    // Found checkbox element - use its checked state
                     flatConfig[actualKey] = formElement.checked;
                 } else {
-                    // Not a checkbox or element not found - normalize string booleans
                     if (typeof actualValue === 'string') {
                         const lowerValue = actualValue.toLowerCase().trim();
                         if (lowerValue === 'true' || lowerValue === '1' || lowerValue === 'on') {
@@ -2593,18 +2573,16 @@ function handlePluginConfigSubmit(e) {
                         } else if (lowerValue === 'false' || lowerValue === '0' || lowerValue === 'off' || lowerValue === '') {
                             flatConfig[actualKey] = false;
                         } else {
-                            // Non-empty string that's not a boolean representation - keep as string
                             flatConfig[actualKey] = actualValue;
                         }
                     } else {
-                        // Non-string value - use as-is
                         flatConfig[actualKey] = actualValue;
                     }
                 }
             }
         }
     }
-    
+
     // Handle unchecked checkboxes (not in FormData) - including nested ones
     if (schema && schema.properties) {
         const allBoolFields = collectBooleanFields(schema);
@@ -2614,11 +2592,43 @@ function handlePluginConfigSubmit(e) {
             }
         });
     }
-    
+
     // Convert dot notation to nested object
-    const config = dotToNested(flatConfig, schema);
-    
-    console.log('Flat config:', flatConfig);
+    return dotToNested(flatConfig, schema);
+}
+
+function handlePluginConfigSubmit(e) {
+    e.preventDefault();
+    console.log('Form submitted');
+
+    if (!currentPluginConfig) {
+        showNotification('Plugin configuration not loaded', 'error');
+        return;
+    }
+
+    const pluginId = currentPluginConfig.pluginId;
+    const schema = currentPluginConfig.schema;
+    const form = e.target;
+
+    // Fix invalid hidden fields before submission
+    // This prevents "invalid form control is not focusable" errors
+    const allInputs = form.querySelectorAll('input[type="number"]');
+    allInputs.forEach(input => {
+        const min = parseFloat(input.getAttribute('min'));
+        const max = parseFloat(input.getAttribute('max'));
+        const value = parseFloat(input.value);
+
+        if (!isNaN(value)) {
+            if (!isNaN(min) && value < min) {
+                input.value = min;
+            } else if (!isNaN(max) && value > max) {
+                input.value = max;
+            }
+        }
+    });
+
+    const config = normalizeFormDataForConfig(form, schema);
+
     console.log('Nested config to save:', config);
     
     // Save the configuration
@@ -4463,42 +4473,9 @@ function switchPluginConfigView(view) {
 function syncFormToJson() {
     const form = document.getElementById('plugin-config-form');
     if (!form) return;
-    
-    const formData = new FormData(form);
-    const config = {};
-    
-    // Get schema for type conversion
+
     const schema = currentPluginConfigState.schema;
-    
-    for (let [key, value] of formData.entries()) {
-        if (key === 'enabled') continue; // Skip enabled, managed separately
-        
-        // Handle nested keys (dot notation)
-        const keys = key.split('.');
-        let current = config;
-        for (let i = 0; i < keys.length - 1; i++) {
-            if (!current[keys[i]]) {
-                current[keys[i]] = {};
-            }
-            current = current[keys[i]];
-        }
-        
-        const finalKey = keys[keys.length - 1];
-        const prop = schema?.properties?.[finalKey] || (keys.length > 1 ? null : schema?.properties?.[key]);
-        
-        // Type conversion based on schema
-        if (prop?.type === 'array') {
-            current[finalKey] = value.split(',').map(item => item.trim()).filter(item => item.length > 0);
-        } else if (prop?.type === 'integer' || key === 'display_duration') {
-            current[finalKey] = parseInt(value) || 0;
-        } else if (prop?.type === 'number') {
-            current[finalKey] = parseFloat(value) || 0;
-        } else if (prop?.type === 'boolean') {
-            current[finalKey] = value === 'true' || value === true;
-        } else {
-            current[finalKey] = value;
-        }
-    }
+    const config = normalizeFormDataForConfig(form, schema);
     
     // Deep merge with existing config to preserve nested structures
     function deepMerge(target, source) {