Skip to content

Open a log entry in a modal, and fix the rows that were landing untyped - #1213

Merged
mastacontrola merged 1 commit into
working-1.6from
task-log-detail-modal
Aug 19, 2026
Merged

Open a log entry in a modal, and fix the rows that were landing untyped#1213
mastacontrola merged 1 commit into
working-1.6from
task-log-detail-modal

Conversation

@mastacontrola

Copy link
Copy Markdown
Member

Two things, both out of live-testing the FOS report path end to end (a VM PXE-booted into the EXP_20260819-141018 FOS build, failing on a missing image store).

1. A defect in #1208 that only a real report could show

Schema 338 gave logType a DEFAULT '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. 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.

On this server: one row with '' (the state row for the test task) 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 on it being empty so loading an existing row and saving it cannot retype it
  • schema 340 retypes the rows the gap produced

2. 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.

  • Filled from the row the grid already holds, so opening it costs no request
  • Defers to the links inside the row, so clicking through to the host still works
  • Entry type is now ranked above task type and state: whether the machine stopped or carried on should not be what Responsive collapses away on a laptop

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_VER bumped — fog.task.list.js changed.

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
mastacontrola merged commit 48514a6 into working-1.6 Aug 19, 2026
3 checks passed
@mastacontrola
mastacontrola deleted the task-log-detail-modal branch August 19, 2026 14:46
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants