From 1180d53b0e3eb6d9cb7d36a4213458568d72f9e1 Mon Sep 17 00:00:00 2001 From: JJ Fullmer <7743340+darksidemilk@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:43:12 -0500 Subject: [PATCH] Open a log entry in a modal, and fix the rows that were landing untyped Two things the live test of the FOS report path turned up. FIRST, a defect in #1208 that only a real report could show. Schema 338 gave `logType` a DEFAULT of 'state', and I wrote in that step that the existing writer could therefore be left alone -- the default would cover it. It does not: a column default applies only when the column is absent from the INSERT, and FOGController::save() writes every declared field, so TaskingElement::taskLog() has been storing '' since the field was declared. On this server that is one row against 52 reading 'state', and those 52 are rows the ALTER backfilled, not rows the app wrote. The consequence is silent and defeats the tab this same series added: the log pane filters on `logType IN ('state')`, so every state row written after the upgrade would be missing from the one view built to show them. TaskLog::__construct() now types the row itself -- guarded, so loading an existing row and saving it cannot retype it -- and schema 340 retypes what the gap produced. SECOND, clicking a row opens the entry in a modal. The message column is the one Responsive truncates, and it is also the only column whose whole point is its text: a FOS report carries the script it came from and the arguments it was passed. The modal is filled from the row the grid already holds, so opening it costs no request, and it defers to the links inside the row so clicking through to the host still works. Entry type is also ranked above task type and state now, because whether the machine stopped or carried on should not be what Responsive collapses away on a laptop. Verified in a browser against the isolated lab database: the tab draws both report types, a row click opens the modal with all seven fields and the full message, Close dismisses it, and clicking the host link navigates instead of opening it. Six new assertions, all mutation-tested. FOG_BCACHE_VER bumped -- fog.task.list.js changed. Co-Authored-By: Claude --- packages/web/commons/schema.php | 21 +++++++ packages/web/lib/fog/system.class.php | 4 +- packages/web/lib/fog/tasklog.class.php | 20 ++++++ .../web/lib/pages/taskmanagement.page.php | 26 ++++++++ .../management/js/fog/task/fog.task.list.js | 63 ++++++++++++++++++- .../de_DE.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../en_US.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../es_ES.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../eu_ES.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../fr_FR.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../it_IT.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../ja_JP.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../web/management/languages/messages.pot | 3 + .../pt_BR.UTF-8/LC_MESSAGES/messages.po | 4 ++ .../zh_CN.UTF-8/LC_MESSAGES/messages.po | 4 ++ tests/task-log-view.test.php | 52 +++++++++++++++ 16 files changed, 221 insertions(+), 4 deletions(-) diff --git a/packages/web/commons/schema.php b/packages/web/commons/schema.php index 56193525d8..6989ef2929 100644 --- a/packages/web/commons/schema.php +++ b/packages/web/commons/schema.php @@ -5878,3 +5878,24 @@ function () { . "(6,'Failed','Host reported that the task could not be completed.'," . "6,'exclamation-triangle')", ]; +// 340 +$this->schema[] = [ + // Retype the rows that landed untyped between step 338 and the model + // learning to type them. + // + // Step 338 gave `logType` a DEFAULT of 'state', which reads as though a + // writer that sets no type gets one. It does not: a column default + // applies only when the column is absent from the INSERT, and + // FOGController::save() writes every declared field -- so + // TaskingElement::taskLog(), which has recorded state changes since long + // before this column existed, has been writing '' ever since the field + // was declared. TaskLog::__construct() now supplies the type, and this + // repairs what the gap produced. + // + // Scoped to '' (and NULL, which the column does not allow today but a + // hand-edited schema might). Nothing else can be mistaken for it: the + // only other values are written deliberately by the FOS report endpoint. + "UPDATE `taskLog` " + . "SET `logType` = 'state' " + . "WHERE `logType` = '' OR `logType` IS NULL", +]; diff --git a/packages/web/lib/fog/system.class.php b/packages/web/lib/fog/system.class.php index dc73b92f7b..d0975f6356 100644 --- a/packages/web/lib/fog/system.class.php +++ b/packages/web/lib/fog/system.class.php @@ -94,8 +94,8 @@ public function __construct() // 1.5.x carried count does, see SchemaReconciler's docstring -- is // permanently "up to date" from the updater's point of view and will // never run another indexed step, whatever this constant says. - define('FOG_SCHEMA', 339); - define('FOG_BCACHE_VER', 286); + define('FOG_SCHEMA', 340); + define('FOG_BCACHE_VER', 287); define('FOG_CLIENT_VERSION', '0.13.0'); // GH-959: iPXE lives in FOGProject/fog-ipxe and its binaries arrive as // a release asset. Pinned here rather than tracked as "latest" so a diff --git a/packages/web/lib/fog/tasklog.class.php b/packages/web/lib/fog/tasklog.class.php index a877a6ec6f..5257830d27 100644 --- a/packages/web/lib/fog/tasklog.class.php +++ b/packages/web/lib/fog/tasklog.class.php @@ -67,6 +67,23 @@ class TaskLog extends FOGController /** * Initializes the class to set the ip from the remote. * + * Also types the row, because the column default cannot. Schema 338 gave + * `logType` a DEFAULT of 'state', and a default only applies when the + * column is left out of the INSERT -- which FOGController::save() never + * does: it writes every declared field, so an unset one arrives as ''. + * So TaskingElement::taskLog(), which has recorded task state changes + * since long before this column existed and sets no type, started writing + * untyped rows the moment the field was declared. Proven on a live + * install 2026-08-19: one row with logType '' against 52 pre-existing + * rows reading 'state', those 52 being rows the ALTER had backfilled. + * + * The consequence is silent: Task Management's log pane filters on + * `logType IN ('state')`, so every state row written after the upgrade + * would be missing from the one view built to show them. + * + * Guarded rather than assigned, so loading an existing row and saving it + * cannot retype it as a state change. + * * @param mixed $data the data to initialize with. * * @return void @@ -75,6 +92,9 @@ public function __construct($data = '') { parent::__construct($data); $this->set('ip', self::$remoteaddr); + if ('' === (string) $this->get('type')) { + $this->set('type', self::TYPE_STATE); + } } /** * Gets the task object. diff --git a/packages/web/lib/pages/taskmanagement.page.php b/packages/web/lib/pages/taskmanagement.page.php index e04563d8d4..74e555f5c7 100644 --- a/packages/web/lib/pages/taskmanagement.page.php +++ b/packages/web/lib/pages/taskmanagement.page.php @@ -536,6 +536,32 @@ private function _logsPane() echo ''; echo ''; $this->render(12, 'task-logs-table'); + // Clicking a row opens this. The message column is the one that gets + // truncated on a narrow viewport, and it is also the only column + // whose whole point is its text -- a FOS report carries the script it + // came from and the arguments it was passed. The modal is filled + // client side from the row the grid already has, so opening it costs + // no request. + // + // Dismiss only: nothing here is editable, so there is no commit + // button for it to sit to the left of, and it takes the outline + // secondary a modal dismiss always takes. + echo self::makeModal( + 'task-log-modal', + '

' + . _('Log entry') + . '

', + '
', + self::makeButton( + 'task-log-close', + _('Close'), + 'btn btn-outline-secondary float-start', + 'data-bs-dismiss="modal"' + ), + '', + 'default', + 'modal-lg' + ); } /** * Get the task log entries. diff --git a/packages/web/management/js/fog/task/fog.task.list.js b/packages/web/management/js/fog/task/fog.task.list.js index 22af9ed6c8..85348fe030 100644 --- a/packages/web/management/js/fog/task/fog.task.list.js +++ b/packages/web/management/js/fog/task/fog.task.list.js @@ -472,9 +472,13 @@ targets: 3 }, { + // Ranked above task type and state, because it is the column that + // says whether the machine stopped or carried on -- the first thing + // anyone opening this tab is looking for, and the one that must not + // be the one Responsive collapses away on a laptop. + responsivePriority: 2, render: function(data) { - // The type is what tells an operator whether the machine stopped - // or carried on, so it is badged rather than left as bare text. + // Badged rather than left as bare text, for the same reason. var cls = {error: 'bg-danger', warning: 'bg-warning text-dark'}; return '' + $.escapeHtml(data || '') @@ -494,10 +498,65 @@ data: function(d) { d.logtypes = $('input[name="log-type-filter"]:checked').val(); } + }, + createdRow: function(row) { + // The whole row is the target, so say so: without a pointer there is + // nothing to suggest the message has more behind it. + $(row).css('cursor', 'pointer'); } }); } + // --------------------------------------------------------------- + // LOG ENTRY DETAIL + // + // Filled from the row the grid already holds -- no request. The message is + // the reason this exists: it is the column that truncates on a narrow + // viewport, and a FOS report carries the script it came from and the + // arguments it was passed, which is exactly what someone reading it needs. + function showLogDetail(row) { + var badge = {error: 'bg-danger', warning: 'bg-warning text-dark'}, + $dl = $('#task-log-detail'), + pairs = [ + ['Time', $.escapeHtml(row.logtime || '')], + ['Host', row.hostid ? + '' + $.escapeHtml(row.hostname || '') + '' : + $.escapeHtml(row.hostname || '')], + ['Task', $.escapeHtml(String(row.taskid || '')) + ' — ' + $.escapeHtml(row.tasktypename || '')], + ['State at the time', $.escapeHtml(row.taskstatename || '') + + ' '], + ['Type', '' + + $.escapeHtml(row.logtype || '') + ''], + ['Recorded by', $.escapeHtml(row.createdBy || '')] + ], + html = ''; + $.each(pairs, function(i, pair) { + html += '
' + pair[0] + '
' + + '
' + pair[1] + '
'; + }); + // A state change has no message at all, which is a fact worth showing + // rather than an empty box. + html += '
Message
' + + (row.logtext ? + '
' + $.escapeHtml(row.logtext) + '
' : + 'none') + + '
'; + $dl.html(html); + $('#task-log-modal').modal('show'); + } + + // Delegated, because the grid replaces its rows on every draw. + $(document).on('click', '#task-logs-table tbody tr', function(e) { + // A row carries links out to the host; let those win. + if ($(e.target).closest('a').length) { + return; + } + var row = panes.logs.table && panes.logs.table.row(this).data(); + if (row) { + showLogDetail(row); + } + }); + // --------------------------------------------------------------- // PER-PANE ACTION BUTTONS (cancel / reload toggle) // diff --git a/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po index c24d6f7b79..54a653be42 100644 --- a/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po @@ -4505,6 +4505,10 @@ msgstr "Log-Viewer" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "Log-Viewer" + #, fuzzy msgid "Log entry type filter" msgstr "Drucker-Update fehlgeschlagen!" diff --git a/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po index 2a98e65c37..bd56b16d37 100644 --- a/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po @@ -4503,6 +4503,10 @@ msgstr "Log Viewer" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "Log Viewer" + #, fuzzy msgid "Log entry type filter" msgstr "Printer update failed!" diff --git a/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po index ad0a33c0b0..ad2f6f693d 100644 --- a/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po @@ -4614,6 +4614,10 @@ msgstr "FOG Visor de registro" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "FOG Visor de registro" + #, fuzzy msgid "Log entry type filter" msgstr "actualización de la impresora ha fallado!" diff --git a/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po index 4ebb418e75..8bb1014bfd 100644 --- a/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po @@ -4506,6 +4506,10 @@ msgstr "Log-Viewer" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "Log-Viewer" + #, fuzzy msgid "Log entry type filter" msgstr "Drucker-Update fehlgeschlagen!" diff --git a/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po index 4aa046fefb..97e81bf711 100644 --- a/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po @@ -4505,6 +4505,10 @@ msgstr "Log Viewer" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "Log Viewer" + #, fuzzy msgid "Log entry type filter" msgstr "mise à jour de l'imprimante a échoué!" diff --git a/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po index 2c1a97cb84..98921c22c4 100644 --- a/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po @@ -4337,6 +4337,10 @@ msgstr "Log Viewer" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "Log Viewer" + #, fuzzy msgid "Log entry type filter" msgstr "Aggiornamento della stampante non è riuscito!" diff --git a/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po index 33edc47287..f6506b92b6 100644 --- a/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po @@ -4293,6 +4293,10 @@ msgstr "ログビューアー" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "ログビューアー" + #, fuzzy msgid "Log entry type filter" msgstr "サイトの更新に失敗しました！" diff --git a/packages/web/management/languages/messages.pot b/packages/web/management/languages/messages.pot index 53af7ddf24..e0f9235f24 100644 --- a/packages/web/management/languages/messages.pot +++ b/packages/web/management/languages/messages.pot @@ -3802,6 +3802,9 @@ msgstr "" msgid "Log contents." msgstr "" +msgid "Log entry" +msgstr "" + msgid "Log entry type filter" msgstr "" diff --git a/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po index 3d44662c95..d4d1e4792f 100644 --- a/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po @@ -4504,6 +4504,10 @@ msgstr "Visualizador de log" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "Visualizador de log" + #, fuzzy msgid "Log entry type filter" msgstr "atualização da impressora falhou!" diff --git a/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po index 712c19d6ba..4f565e8e92 100644 --- a/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po @@ -4504,6 +4504,10 @@ msgstr "日志查看器" msgid "Log contents." msgstr "" +#, fuzzy +msgid "Log entry" +msgstr "日志查看器" + #, fuzzy msgid "Log entry type filter" msgstr "打印机更新失败！" diff --git a/tests/task-log-view.test.php b/tests/task-log-view.test.php index ed5a56f372..33a8b1ce99 100644 --- a/tests/task-log-view.test.php +++ b/tests/task-log-view.test.php @@ -115,6 +115,58 @@ . ' endpoint disagree about what is selected on arrival'; } +// ------------------------------------------------------------ the typing + +// A column DEFAULT does not type these rows: FOGController::save() writes +// every declared field, so a writer that sets no type stores ''. The model +// has to supply it, or every state row written after schema 338 is missing +// from the 'state' filter -- which is the one view built to show them. +$model = file_get_contents($web . '/lib/fog/tasklog.class.php'); +if (!preg_match( + '#__construct.*?get\(\'type\'\).*?set\(\'type\', self::TYPE_STATE\)#s', + $model +)) { + $fails[] = 'TaskLog does not default its own type, so every row written by' + . ' a caller that sets none stores an empty string and disappears from' + . ' the state filter'; +} +$schema = file_get_contents($web . '/commons/schema.php'); +if (!preg_match( + "#UPDATE `taskLog`.*?SET `logType` = 'state'.*?WHERE `logType` = ''#s", + $schema +)) { + $fails[] = 'no schema step retypes the rows written untyped before the' + . ' model was fixed, so they stay invisible to the state filter'; +} + +// ------------------------------------------------------------- the modal + +// The message column truncates, and a FOS report's value is its full text -- +// the script it came from and the arguments it was passed. +if (false === strpos($page, "'task-log-modal'")) { + $fails[] = 'the logs pane has no detail modal, so a truncated message' + . ' cannot be read in full'; +} +if (false === strpos($js, "#task-log-modal") + || false === strpos($js, "#task-logs-table tbody tr") +) { + $fails[] = 'nothing opens the log detail modal from a row, so the markup' + . ' is emitted and unreachable'; +} +if (false === strpos($js, "closest('a').length")) { + $fails[] = 'the row click does not defer to the links inside it, so' + . ' clicking through to a host opens the modal instead'; +} +// Filled, not merely referenced: dropping the write leaves the modal showing +// whatever the last click put there, which reads as the wrong row's detail +// rather than as a bug. +if (false === strpos($js, '$(\'#task-log-detail\')') + || false === strpos($js, '$dl.html(') +) { + $fails[] = 'the modal is opened without being filled, so it shows the' + . ' previous row (or nothing) whatever was clicked'; +} + if ($fails) { echo 'FAIL: ' . count($fails) . " problem(s):\n"; foreach ($fails as $f) {