Open a log entry in a modal, and fix the rows that were landing untyped - #1213
Merged
Conversation
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 <noreply@anthropic.com>
mastacontrola
added a commit
that referenced
this pull request
Aug 19, 2026
Ported from 1.6 #1213. Schema 280 gave `logType` a DEFAULT of 'state' and that step said the existing writer could be left alone, since 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(), which has recorded task state changes since long before this column existed and sets no type, has been storing '' ever since the field was declared. Found on 1.6 against a live install, where one such row appeared as soon as a task was created; the code is identical here, so the defect is too. TaskLog::__construct() now types the row itself, guarded so that loading an existing row and saving it cannot retype it, and schema 282 repairs what the gap produced. Two assertions, both mutation-tested. Less visible on 1.5 than on 1.6, which has a pane that filters on the type; here it is the difference between a stored value that says what a row is and one that says nothing. Co-authored-by: JJ Fullmer <7743340+darksidemilk@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two things, both out of live-testing the FOS report path end to end (a VM PXE-booted into the
EXP_20260819-141018FOS build, failing on a missing image store).1. A defect in #1208 that only a real report could show
Schema 338 gave
logTypeaDEFAULT 'state', and that step's comment said 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. SoTaskingElement::taskLog(), which has recorded task state changes since long before this column existed and sets no type, has been storing''ever since the field was declared.On this server: one row with
''(the state row for the test task) against 52 readingstate— and those 52 are rows theALTERbackfilled, 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 on it being empty so loading an existing row and saving it cannot retype it2. Clicking a row opens the entry in a modal
Asked for directly. The message column is the one Responsive truncates, and it is the only column whose whole point is its text — a FOS report carries the script it came from and the arguments it was passed.
Verification
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 the modal. The untyped-row defect was reproduced and the fix confirmed against the same data.
Six new assertions in
tests/task-log-view.test.php, every one mutation-tested (model default removed, backfill removed, modal markup renamed, nothing opening it, link guard removed, modal opened unfilled). Full suite green.FOG_BCACHE_VERbumped —fog.task.list.jschanged.