From b88842e672766394efc8a240e3ab99f2e79886f1 Mon Sep 17 00:00:00 2001 From: Chuck Date: Thu, 8 Jan 2026 12:26:08 -0500 Subject: [PATCH] fix(array-objects): Fix schema lookup, reindexing, and disable file upload Address PR review feedback for array-of-objects helpers: 1. Schema resolution: Use getSchemaProperty() instead of manual traversal - Fixes nested array-of-objects schema lookup (e.g., news.custom_feeds) - Now properly descends through .properties for nested objects 2. Reindexing: Replace brittle regex with targeted patterns - Only replace index in bracket notation [0], [1], etc. for names - Only replace _item_ pattern for IDs (not arbitrary digits) - Use specific function parameter patterns for onclick handlers - Prevents corruption of fieldId, pluginId, or other numeric values 3. File upload: Disable widget until properly implemented - Hide/disable upload button with clear message - Show existing logos if present but disable upload functionality - Prevents silent failures when users attempt to upload files - Added TODO comments for future implementation Also fixes exit code handling in one-shot-install.sh to properly capture first_time_install.sh exit status before error trap fires. --- scripts/install/one-shot-install.sh | 4 +- web_interface/static/v3/plugins_manager.js | 119 +++++++++++++-------- 2 files changed, 78 insertions(+), 45 deletions(-) diff --git a/scripts/install/one-shot-install.sh b/scripts/install/one-shot-install.sh index 8478d04f..c477d4bb 100755 --- a/scripts/install/one-shot-install.sh +++ b/scripts/install/one-shot-install.sh @@ -343,14 +343,16 @@ main() { echo "" # Execute with proper error handling + # Temporarily disable errexit to capture exit code instead of exiting immediately + set +e # Use sudo if we're not root, otherwise run directly if [ "$EUID" -eq 0 ]; then bash ./first_time_install.sh else sudo bash ./first_time_install.sh fi - INSTALL_EXIT_CODE=$? + set -e # Re-enable errexit if [ $INSTALL_EXIT_CODE -eq 0 ]; then echo "" diff --git a/web_interface/static/v3/plugins_manager.js b/web_interface/static/v3/plugins_manager.js index 0e6f5f0e..ce4330e5 100644 --- a/web_interface/static/v3/plugins_manager.js +++ b/web_interface/static/v3/plugins_manager.js @@ -2485,6 +2485,10 @@ function renderArrayObjectItem(fieldId, fullKey, itemProperties, itemValue, inde html += `
`; // Handle file-upload widget (for logo field) + // NOTE: File upload for array-of-objects items is not yet implemented. + // The widget is disabled to prevent silent failures when users try to upload files. + // TODO: Implement handleArrayObjectFileUpload and removeArrayObjectFile with proper + // endpoint support and [data-file-data] attribute updates before enabling this widget. if (propSchema['x-widget'] === 'file-upload') { html += ``; if (propDescription) { @@ -2494,29 +2498,26 @@ function renderArrayObjectItem(fieldId, fullKey, itemProperties, itemValue, inde const pluginId = uploadConfig.plugin_id || (typeof currentPluginConfig !== 'undefined' ? currentPluginConfig?.pluginId : null) || (typeof window.currentPluginConfig !== 'undefined' ? window.currentPluginConfig?.pluginId : null) || 'ledmatrix-news'; const logoValue = propValue || {}; - html += ` -
- - - `; - + // Display existing logo if present, but disable upload functionality if (logoValue.path) { html += ` -
- Logo +
+
+ Logo + File upload not yet available for array items +
+
+ `; + } else { + html += ` +
+

File upload functionality for array items is coming soon

`; } @@ -6433,19 +6434,13 @@ if (typeof window !== 'undefined') { const schema = (typeof currentPluginConfig !== 'undefined' && currentPluginConfig?.schema) || (typeof window.currentPluginConfig !== 'undefined' && window.currentPluginConfig?.schema); if (!schema) return; - // Navigate to the items schema - const keys = fullKey.split('.'); - let itemsSchema = schema.properties; - for (const key of keys) { - if (itemsSchema && itemsSchema[key]) { - itemsSchema = itemsSchema[key]; - if (itemsSchema.type === 'array' && itemsSchema.items) { - itemsSchema = itemsSchema.items; - break; - } - } + // Use getSchemaProperty to properly handle nested schemas (e.g., news.custom_feeds) + const arraySchema = getSchemaProperty(schema, fullKey); + if (!arraySchema || arraySchema.type !== 'array' || !arraySchema.items) { + return; } + const itemsSchema = arraySchema.items; if (!itemsSchema || !itemsSchema.properties) return; const newIndex = currentItems.length; @@ -6489,24 +6484,55 @@ if (typeof window !== 'undefined') { if (item) { item.remove(); // Re-index remaining items + // Use data-index for index storage - no need to encode index in onclick strings or IDs const remainingItems = itemsContainer.querySelectorAll('.array-object-item'); remainingItems.forEach((itemEl, newIndex) => { itemEl.setAttribute('data-index', newIndex); - // Update all inputs within this item - need to update name/id attributes + // Update all inputs within this item - only update index in array bracket notation itemEl.querySelectorAll('input, select, textarea').forEach(input => { - const name = input.getAttribute('name') || input.id; + const name = input.getAttribute('name'); + const id = input.id; if (name) { - // Update name/id attribute with new index - const newName = name.replace(/\[\d+\]/, `[${newIndex}]`); - if (input.getAttribute('name')) input.setAttribute('name', newName); - if (input.id) input.id = input.id.replace(/\d+/, newIndex); + // Only replace index in bracket notation like [0], [1], etc. + // Match pattern: field_name[index] but not field_name123 + const newName = name.replace(/\[(\d+)\]/, `[${newIndex}]`); + input.setAttribute('name', newName); + } + if (id) { + // Only update index in specific patterns like _item_0, _item_1 + // Match pattern: _item_ but be careful not to break other numeric IDs + const newId = id.replace(/_item_(\d+)/, `_item_${newIndex}`); + input.id = newId; } }); - // Update button onclick attributes + // Update button onclick attributes - only update the index parameter + // Since we use data-index for tracking, we can compute index from closest('.array-object-item') + // For now, update onclick strings but be more careful with the regex itemEl.querySelectorAll('button[onclick]').forEach(button => { const onclick = button.getAttribute('onclick'); if (onclick) { - button.setAttribute('onclick', onclick.replace(/\d+/, newIndex)); + // Match patterns like: + // removeArrayObjectItem('fieldId', 0) + // handleArrayObjectFileUpload(event, 'fieldId', 0, 'propKey', 'pluginId') + // removeArrayObjectFile('fieldId', 0, 'propKey') + // Only replace the numeric index parameter (second or third argument depending on function) + let newOnclick = onclick; + // For removeArrayObjectItem('fieldId', index) - second param + newOnclick = newOnclick.replace( + /removeArrayObjectItem\s*\(\s*['"]([^'"]+)['"]\s*,\s*\d+\s*\)/g, + `removeArrayObjectItem('$1', ${newIndex})` + ); + // For handleArrayObjectFileUpload(event, 'fieldId', index, ...) - third param + newOnclick = newOnclick.replace( + /handleArrayObjectFileUpload\s*\(\s*event\s*,\s*['"]([^'"]+)['"]\s*,\s*\d+\s*,/g, + `handleArrayObjectFileUpload(event, '$1', ${newIndex},` + ); + // For removeArrayObjectFile('fieldId', index, ...) - second param + newOnclick = newOnclick.replace( + /removeArrayObjectFile\s*\(\s*['"]([^'"]+)['"]\s*,\s*\d+\s*,/g, + `removeArrayObjectFile('$1', ${newIndex},` + ); + button.setAttribute('onclick', newOnclick); } }); }); @@ -6514,12 +6540,17 @@ if (typeof window !== 'undefined') { // Update add button state const addButton = itemsContainer.nextElementSibling; - if (addButton) { - const maxItems = parseInt(addButton.getAttribute('onclick').match(/\d+/)[0]); - if (remainingItems.length < maxItems) { - addButton.disabled = false; - addButton.style.opacity = '1'; - addButton.style.cursor = 'pointer'; + if (addButton && addButton.getAttribute('onclick')) { + // Extract maxItems from onclick attribute more safely + // Pattern: addArrayObjectItem('fieldId', 'fullKey', maxItems) + const onclickMatch = addButton.getAttribute('onclick').match(/addArrayObjectItem\s*\([^,]+,\s*[^,]+,\s*(\d+)\)/); + if (onclickMatch && onclickMatch[1]) { + const maxItems = parseInt(onclickMatch[1]); + if (remainingItems.length < maxItems) { + addButton.disabled = false; + addButton.style.opacity = '1'; + addButton.style.cursor = 'pointer'; + } } } }