diff --git a/front/deviceDetails.php b/front/deviceDetails.php index 09a0c8f85..1162c11e5 100755 --- a/front/deviceDetails.php +++ b/front/deviceDetails.php @@ -255,12 +255,18 @@ switch ($UI_THEME) { var emptyArr = ['undefined', "", undefined, null]; // ----------------------------------------------------------------------------- - // Shared Sources data: Field View and the Session Info tab's Known IPs chip - // list both read the same pluginsObjects(objectPrimaryId = mac) query - - // cached by mac so a second caller gets the cached promise, not a second + // Shared Sources data cache: Field View and the Session Info tab's Known IPs + // chip list both read the same pluginsObjects(objectPrimaryId = mac) query - + // keyed by mac so a second caller gets the cached promise, not a second // network call. Neither consumer should issue its own pluginsObjects query. const _sourcesFieldDataCache = {}; + /** + * Fetch (or return the cached/in-flight promise for) this mac's pluginsObjects + * entries - shared by Field View and the Session Info tab's Known IPs chip list. + * @param {string} mac + * @returns {Promise} Rejects on an ajax failure or a GraphQL-level error. + */ function getSourcesFieldData(mac) { if (_sourcesFieldDataCache[mac]) { return _sourcesFieldDataCache[mac]; @@ -293,7 +299,10 @@ switch ($UI_THEME) { }).then(function(response) { if (response.errors) { console.error("[Sources] pluginsObjects GraphQL errors:", response.errors); - return []; + // Reject (not resolve with []) - an empty-but-successful result would + // skip the cache eviction below, permanently poisoning this mac's + // entry with no way to retry. + throw new Error("pluginsObjects GraphQL error"); } return response.data.pluginsObjects.entries; }); @@ -312,12 +321,17 @@ switch ($UI_THEME) { // IPs chip list need to know, PER PLUGIN, which exact Plugins_Objects column // (objectSecondaryId/watchedValue1-4) holds a given field's value - that // mapping is computed server-side (get_plugin_columns_for_field()) and shipped - // in plugins.json's field_views block. Reading it here once, shared by both - // consumers, is what keeps either one from falling back to "grab whichever - // column happens to be non-empty" - which silently picks up unrelated data - // (e.g. a port number) from a plugin that was never registered for this field. + // in plugins.json's field_views block. let _fieldViewDefinitionsCache = null; + /** + * Fetch (or return the cached) field_views block from plugins.json - which + * plugin/column answers each DEVICE_FIELD_VIEWS field. Shared by Field View + * and the Session Info tab's Known IPs chip list, so neither one falls back + * to "grab whichever column happens to be non-empty" and silently picks up + * unrelated data (e.g. a port number) from a plugin never registered for the field. + * @returns {Promise} + */ async function getFieldViewDefinitions() { if (_fieldViewDefinitionsCache) { return _fieldViewDefinitionsCache; @@ -329,9 +343,14 @@ switch ($UI_THEME) { return _fieldViewDefinitionsCache; } - // Resolve a field's value for one plugin's pluginsObjects entry, using the - // exact column role field_views registered for that plugin - not a fallback - // chain over whichever watchedValue happens to be non-empty. + /** + * Resolve a field's value for one plugin's pluginsObjects entry, using the + * exact column role field_views registered for that plugin - not a fallback + * chain over whichever watchedValue happens to be non-empty. + * @param {{plugin: string, column: string}[]} fieldColumns + * @param {object} entry - One pluginsObjects entry. + * @returns {*} The entry's value for this field, or undefined if entry's plugin isn't registered for it. + */ function resolveFieldValueForPlugin(fieldColumns, entry) { const match = fieldColumns.find(c => c.plugin === entry.plugin); return match ? entry[match.column] : undefined; @@ -340,10 +359,15 @@ switch ($UI_THEME) { // ----------------------------------------------------------------------------- // Sources tab: Plugin View / Field View toggle. Plugin View is the default // and the fallback for anyone who's never touched the toggle, matching - // today's only behavior. ?tab= (an existing deep-link into - // Plugin View) and ?field= (new, for Field View) are two - // separate URL params on purpose - neither should have to disambiguate - // the other's value. + // today's only behavior. + /** + * Switch the Sources tab between Plugin View and Field View: toggles the two + * panes, updates the toggle button's icon/data-view, and persists the choice. + * ?tab= (an existing deep-link into Plugin View) and + * ?field= (Field View) are two separate URL params on purpose - + * neither should have to disambiguate the other's value. + * @param {('plugin'|'field')} view + */ function setSourcesView(view) { const isField = view === 'field'; $('#sourcesFieldView').toggle(isField); @@ -354,6 +378,11 @@ switch ($UI_THEME) { setCache('activeSourcesView', view); } + /** + * Set the Sources tab's initial view on page load, from (in priority order) + * the ?field= URL param, the ?tab= URL param, the cached last-used view, or + * the 'plugin' default. + */ function initSourcesViewToggle() { const urlParams = new URLSearchParams(window.location.search); diff --git a/front/deviceDetailsSessionInfo.php b/front/deviceDetailsSessionInfo.php index c9f2b99aa..d5da3846f 100644 --- a/front/deviceDetailsSessionInfo.php +++ b/front/deviceDetailsSessionInfo.php @@ -239,8 +239,16 @@ function sessionInfoUpdater() { // way fieldViewUpdater() gates renderFieldView(), so this never fires // (and never poisons getSourcesFieldData()'s per-mac cache with a // too-early failure) before the server is actually ready to answer. + // On a rejected render (e.g. getSourcesFieldData()'s GraphQL error + // path), reset sessionInfoInitialized so the next tick retries instead + // of leaving this mac permanently stuck unrendered. callAfterAppInitialized(() => { - Promise.resolve(renderIpLists(currentMac)).finally(hideSessionInfoTabSkeleton); + Promise.resolve(renderIpLists(currentMac)) + .catch(err => { + console.error('[Session Info] renderIpLists failed, will retry next tick:', err); + sessionInfoInitialized = false; + }) + .finally(hideSessionInfoTabSkeleton); }); } } diff --git a/front/pluginsFieldView.php b/front/pluginsFieldView.php index 1091ead67..29cfaa506 100644 --- a/front/pluginsFieldView.php +++ b/front/pluginsFieldView.php @@ -123,28 +123,33 @@ async function renderFieldView(mac) { }); } -// ----------------------------------------------------------------------------- -// Builds one field's DataTable - Plugin / Value / Status / Last Changed, one -// row per participating plugin's entry. Reuses the exact same shared options -// (getStandardDataTableOptions()) and cell-rendering approach -// (createdCell + getFormControl()) pluginsCore.php's own DataTables use, -// rather than a parallel hand-rolled table - only the data source differs -// (a small, already-fetched client-side array here vs. a server-paginated -// GraphQL query there). The column each plugin's value is read from is -// whatever get_plugin_columns_for_field() determined server-side, not a -// hardcoded column role; value_type comes from the field's own -// DEVICE_FIELD_VIEWS declaration (e.g. 'device_ip' for the IP field's -// clickable link, 'none' for a plain value) rather than being hardcoded to -// the IP field's rendering for every field. +/** + * Build fieldKey's DataTable - Plugin / Value / Status / Last Changed, one row + * per participating plugin entry (a plugin can report more than one entry for + * the same device, e.g. multiple IPs - every matching entry becomes its own + * row, not just the first). Reuses the exact same shared options + * (getStandardDataTableOptions()) and cell-rendering approach + * (createdCell + getFormControl()) pluginsCore.php's own DataTables use, + * rather than a parallel hand-rolled table - only the data source differs + * (a small, already-fetched client-side array here vs. a server-paginated + * GraphQL query there). The column each plugin's value is read from is + * whatever get_plugin_columns_for_field() determined server-side, not a + * hardcoded column role; value_type comes from the field's own + * DEVICE_FIELD_VIEWS declaration (e.g. 'device_ip' for the IP field's + * clickable link, 'none' for a plain value) rather than being hardcoded to + * the IP field's rendering for every field. + * @param {string} fieldKey - The field view's key (e.g. 'ip'). + * @param {object} fieldView - fieldViews[fieldKey] from getFieldViewDefinitions(). + * @param {object[]} entries - This device's pluginsObjects entries from getSourcesFieldData(). + */ function buildFieldPaneTable(fieldKey, fieldView, entries) { const { columns: pluginColumns, value_type } = fieldView; - const rows = pluginColumns - .map(({ plugin, column }) => { - const entry = entries.find(e => e.plugin === plugin); - return entry ? { plugin, value: entry[column], status: entry.status, dateTimeChanged: entry.dateTimeChanged || '' } : null; - }) - .filter(row => row !== null); + const rows = pluginColumns.flatMap(({ plugin, column }) => + entries + .filter(e => e.plugin === plugin) + .map(entry => ({ plugin, value: entry[column], status: entry.status, dateTimeChanged: entry.dateTimeChanged || '' })) + ); $(`#fieldTable_${fieldKey}`).DataTable({ ...getStandardDataTableOptions(null), // no per-table skeleton - the whole tab shares #skel-tab-field-view @@ -171,10 +176,14 @@ function buildFieldPaneTable(fieldKey, fieldView, entries) { }); } -// ----------------------------------------------------------------------------- -// Poll for visibility the same way pluginsCore.php's own updater() does - -// #sourcesFieldView only reports :visible once both the Sources tab is the -// active top-level tab AND the Plugin/Field toggle has selected Field View. +/** + * Poll for visibility the same way pluginsCore.php's own updater() does - + * #sourcesFieldView only reports :visible once both the Sources tab is the + * active top-level tab AND the Plugin/Field toggle has selected Field View. + * Resets fieldViewInitialized on a rejected render so a transient failure + * (e.g. getSourcesFieldData()'s GraphQL error path) gets retried on the next + * tick instead of leaving this mac permanently stuck unrendered. + */ function fieldViewUpdater() { if ($('#sourcesFieldView').is(':visible')) { const currentMac = getMac(); @@ -182,7 +191,14 @@ function fieldViewUpdater() { fieldViewInitialized = true; fieldViewLastMac = currentMac; showFieldViewSkeleton(); - callAfterAppInitialized(() => renderFieldView(currentMac).finally(hideFieldViewSkeleton)); + callAfterAppInitialized(() => { + renderFieldView(currentMac) + .catch(err => { + console.error('[Field View] render failed, will retry next tick:', err); + fieldViewInitialized = false; + }) + .finally(hideFieldViewSkeleton); + }); } } setTimeout(fieldViewUpdater, 200); diff --git a/test/db/test_plugin_field_views.py b/test/db/test_plugin_field_views.py index cc7ed0a2e..2817300d3 100644 --- a/test/db/test_plugin_field_views.py +++ b/test/db/test_plugin_field_views.py @@ -127,9 +127,6 @@ class TestRealPluginInventory: for prefix, column in participants: assert column in valid_roles, f"{prefix} returned an invalid column role: {column}" - # Incidental today - not frozen. A future plugin addition legitimately changes this. - assert len(participants) == 23 - class TestGetAllDeviceFieldViewsCache: def test_same_object_returns_cached_result(self): diff --git a/test/e2e/fixtures.js b/test/e2e/fixtures.js index c68a15ab2..4da2b2244 100644 --- a/test/e2e/fixtures.js +++ b/test/e2e/fixtures.js @@ -38,6 +38,16 @@ const PLUGINS_OBJECTS_FIXTURE = [ }, ]; +// Minimal settings fixture matching the real settings GraphQL query's response +// shape (data.settings.settings) - deviceDetailsEdit.php's getDeviceData() +// reads response.data.settings.settings unconditionally, so a generic empty +// `{ data: {} }` fallback would throw there instead of letting +// renderSessionInfoLabels() populate the Known IPs labels. +const SETTINGS_FIXTURE = [ + { setKey: 'NEWDEV_devPrimaryIPv4', setName: 'Primary IPv4' }, + { setKey: 'NEWDEV_devPrimaryIPv6', setName: 'Primary IPv6' }, +]; + const DEVICE_DATA_FIXTURE = { devMac: TEST_MAC, devName: 'Test Device', @@ -75,8 +85,15 @@ async function mockSourcesBackend(page, { mac = TEST_MAC, pluginsObjects = PLUGI return; } - // Any other GraphQL query (settings, etc.) this flow happens to fire - - // return an empty-but-valid shape rather than failing the request. + if (body.query.includes('settings')) { + route.fulfill({ + json: { data: { settings: { settings: SETTINGS_FIXTURE, count: SETTINGS_FIXTURE.length } } }, + }); + return; + } + + // Any other GraphQL query this flow happens to fire - return an + // empty-but-valid shape rather than failing the request. route.fulfill({ json: { data: {} } }); }); @@ -93,6 +110,7 @@ module.exports = { TEST_MAC, PLUGINS_JSON_FIXTURE, PLUGINS_OBJECTS_FIXTURE, + SETTINGS_FIXTURE, DEVICE_DATA_FIXTURE, mockSourcesBackend, };