Task: Single generic HistoryDialog widget in ores.qt

Table of Contents

This page documents a task in the Consolidate history dialogs onto HistoryDialogBase story. It captures the goal, current status, acceptance, and any notes or results.

Goal

Build the single, non-templated HistoryDialog widget described in History Diff Architecture: one concrete Qt class, parameterised at construction by (entity_type, entity_id), consuming the generic history.v1.get response — no per-entity subclass, no typed domain payload. First slice (per 2026-07-13 scoping decision): the widget itself, wired in for currency only, replacing CurrencyHistoryDialog. Diff-span colour highlighting (task 7454EE7E) and the codegen'd Open/Revert action registry are explicitly deferred to follow-on work — see Plan.

Status

Field Value
State DONE
Parent story Consolidate history dialogs onto HistoryDialogBase
Now Nothing.
Waiting on Nothing.
Next Nothing.
Last touched 2026-07-13

Acceptance

  • [X] HistoryDialog widget in ores.qt.api, extending HistoryDialogBase (reusing its toolbar/version-list/changes-tab/action-state machinery) but with no per-entity subclass: constructed with (entity_type, entity_id, ClientManager*).
  • [X] Issues the generic get_entity_history_request and renders entity_history_version::fields (full-details tab, generic Field/Value table) and ::changes (changes tab, flat Field/Old/New — diff-span colour highlighting is task 7454EE7E, not this one).
  • [X] Open/Revert emit generic signals carrying (entity_type, entity_id, version), not a typed domain object.
  • [X] Wired into CurrencyController replacing CurrencyHistoryDialog; Open/Revert handlers adapted to fetch the typed currency at the requested version via the existing get_currency_history_request (one extra round trip, per the architecture doc's documented tradeoff) instead of using an in-memory typed payload.
  • [X] CurrencyHistoryDialog (class + .ui) deleted once HistoryDialog replaces it; the now-orphaned VersionNavigationHelper.hpp deleted too.
  • [X] markAsStale() no longer auto-reloads: it pulses the Reload toolbar action (gold color_constants::stale_indicator, via the existing RecencyPulseManager) so the user notices new versions and reloads on demand. Lives on HistoryDialogBase, so every future entity gets it for free.
  • [X] Manually verified via test scenario Verify the generic HistoryDialog widget for currency: table, changes tab, full details, Open (with full version-nav preserved), Revert all pass. Two unrelated pre-existing issues found and filed separately (see Notes), not blocking.

Deferred to follow-on tasks (do not block this task on them):

  • Diff-span colour highlighting in the changes tab — task 7454EE7E, sequenced after this task per the story owner.
  • The codegen'd per-entity Open/Revert action registry and rollout to every other entity — task CB042B99 retires the remaining per-entity dialogs once that rollout completes.

Plan

  • HistoryDialog extends HistoryDialogBase rather than QWidget directly: the base's toolbar wiring, version-list population, changes- tab rendering, action-state management and async request plumbing (runHistoryRequest) are all still exactly what's needed — what disappears is the per-entity subclass, not the base machinery. HistoryDialogBase::calculateDiffAt(current_index, previous_index) is always called with previous_index = current_index + 1= (adjacent versions only), which matches exactly what the server precomputes per version — so the override just converts versions_[ci].changes to DiffResult, ignoring pi.
  • Lives in ores.qt.api (alongside HistoryDialogBase), not ores.qt.refdata: it is entity-agnostic. Needs new dependencies on ores.history.api.lib (the generic request/response types) and ores.diff.lib (field_value, diff_entry) in ores.qt/api/src/CMakeLists.txt.
  • Full-details tab: previously typed, per-field widgets (isoCodeValue, fractionsPerUnitSpinBox, …) in CurrencyHistoryDialog. The generic dialog has no typed field knowledge, so this becomes a plain two-column Field/Value table populated from entity_history_version::fields, same shape as the changes tab.
  • Open/Revert: emit (entity_type, entity_id, version) signals per the architecture, not a typed domain object — HistoryDialog itself never sees one. CurrencyController adapts: its onOpenVersion=/ =onRevertVersion handlers currently take a typed currency from the dialog's in-memory history; change them to fetch that version via the existing get_currency_history_request (already returns the full typed vector<currency> history) and find the matching version before proceeding as today. One extra round trip, exactly the tradeoff the architecture doc calls out — acceptable since Open/Revert are user-initiated, infrequent actions.
  • Delete CurrencyHistoryDialog (.hpp/.cpp/.ui) once CurrencyController is wired to the generic dialog and the manual verification passes.
  • Not in this slice: diff-span colour highlighting (7454EE7E, changes tab stays plain Field/Old/New text for now) and the codegen'd Open/Revert action registry (CB042B99's prerequisite, deferred until more than one entity needs this dialog).
  • Manual verification surfaced a real gap: markAsStale() silently auto-reloaded on a server-side change event, with no visual cue that anything happened. Fixed on HistoryDialogBase (not just the generic dialog, since every history dialog shares this base): markAsStale() no longer calls loadHistory() directly; it starts a RecencyPulseManager that alternates the Reload action's icon between normal and color_constants::stale_indicator (gold — the existing "new/changed data" visual language used elsewhere for row highlighting, now used on a toolbar action for the first time) for a few seconds, and onReloadClicked() stops the pulse. Reload stays on-demand — the user decides when to fetch, matching how every other entity-changed notification in the app already behaves.

Notes

Two issues surfaced during manual verification, both unrelated to this task's changes and filed separately rather than fixed here:

  • Currency add: Save button stays disabled after filling in all fields, blocking currency creation — capture doc/agile/product_backlog/inbox/currency_add_save_disabled.org.
  • The scenario's own cleanup step (delete the test currency) failed as a direct consequence of the Add failure above (nothing was ever created to delete) — not a separate defect.

Test Scenarios

Manual QA scenarios (scaffolded via compass add test_scenario, run through the QA Validation Runner panel) that verify this task. Link new ones here as they're created; the scenario doc itself links back via its "Verifies task" field.

Scenario State Notes
Verify the generic HistoryDialog widget for currency FAILED All history-dialog-specific steps passed; the 2 FAILs are the pre-existing, unrelated currency-add bug and its downstream cleanup step. Reload-pulse addition reviewed live after this run, approved.

PRs

PR Title
#1553 [qt] Single generic HistoryDialog widget, currency first

Review

# Comment summary File Decision Notes
1 Unused ores.diff.lib link, already transitively available via ores.history.api.lib ores.qt/api/src/CMakeLists.txt Fixed ead565099: dropped the explicit link
2 RecencyPulseManager's ~6s flash disappears with no persistent stale marker if the user misses it HistoryDialogBase.cpp Declined Matches how RecencyPulseManager is used everywhere else in the codebase; acceptable for this slice
3 Currency's model still declares the per-entity history-dialog facet config; a future codegen run could resurrect the deleted CurrencyHistoryDialog ores.refdata.currency.org Declined (documented) Noted on task CB042B99 rather than fixed here — has_version_navigation also drives CurrencyDetailDialog's still-in-use version-nav logic, so disabling it needs its own investigation
4 HistoryDialog's calculateDiffAt/versionRow/openVersionAt/revertToVersionAt don't bounds-check index, unlike displayFullDetails HistoryDialog.cpp Declined Verified safe: all call sites go through HistoryDialogBase methods that already validate against historySize() first
5 fetchCurrencyHistory returned an empty vector on both "not connected"/"request failed" and a genuinely-missing version, misreporting the former as "version not found" CurrencyController.cpp Fixed ead565099: fetchCurrencyHistory now returns std::expected<vector, QString> so failure modes report their real cause

Result

Built HistoryDialog, the single non-templated Qt widget the architecture doc calls for: extends HistoryDialogBase, constructed with (entity_type, entity_id, ClientManager*), no per-entity subclass. Renders the generic history.v1.get response as a plain Field/Value full-details table and a Field/Old/New changes table (diff-span colour highlighting deferred to 7454EE7E). Open/Revert emit generic (entity_type, entity_id, version) signals.

Wired into CurrencyController, replacing CurrencyHistoryDialog (deleted, along with the now-orphaned VersionNavigationHelper.hpp). Open/Revert resolve the generic signal back to a typed currency via one extra get_currency_history_request round trip, preserving full version-nav (first/prev/next/last) in the opened detail dialog.

Manual verification (test scenario 7D6E9249) surfaced a real gap: markAsStale() silently auto-reloaded with no visual cue. Fixed on HistoryDialogBase itself — pulses the Reload action gold via the existing RecencyPulseManager instead of auto-reloading; the user decides when to fetch. Benefits every future entity's history dialog, not just currency's.

Two unrelated pre-existing issues found during verification were filed separately rather than fixed here (see Notes): a currency-add Save-button bug, and its consequent scenario cleanup-step failure.

Emacs 29.3 (Org mode 9.6.15)