FE+TESTS: review fixes

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<object[]>} 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<object>}
+   */
   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=<plugin_prefix> (an existing deep-link into
-  // Plugin View) and ?field=<field_key> (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=<plugin_prefix> (an existing deep-link into Plugin View) and
+   * ?field=<field_key> (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);
 

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);
         });
       }
     }

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

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

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,
 };