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]HistoryDialogwidget inores.qt.api, extendingHistoryDialogBase(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 genericget_entity_history_requestand rendersentity_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 intoCurrencyControllerreplacingCurrencyHistoryDialog; Open/Revert handlers adapted to fetch the typed currency at the requested version via the existingget_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 onceHistoryDialogreplaces it; the now-orphanedVersionNavigationHelper.hppdeleted too.[X]markAsStale()no longer auto-reloads: it pulses the Reload toolbar action (goldcolor_constants::stale_indicator, via the existingRecencyPulseManager) so the user notices new versions and reloads on demand. Lives onHistoryDialogBase, 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
HistoryDialogextendsHistoryDialogBaserather thanQWidgetdirectly: 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 withprevious_index =current_index + 1= (adjacent versions only), which matches exactly what the server precomputes per version — so the override just convertsversions_[ci].changestoDiffResult, ignoringpi.- Lives in
ores.qt.api(alongsideHistoryDialogBase), notores.qt.refdata: it is entity-agnostic. Needs new dependencies onores.history.api.lib(the generic request/response types) andores.diff.lib(field_value,diff_entry) inores.qt/api/src/CMakeLists.txt. - Full-details tab: previously typed, per-field widgets
(
isoCodeValue,fractionsPerUnitSpinBox, …) inCurrencyHistoryDialog. The generic dialog has no typed field knowledge, so this becomes a plain two-column Field/Value table populated fromentity_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 —HistoryDialogitself never sees one.CurrencyControlleradapts: itsonOpenVersion=/ =onRevertVersionhandlers currently take a typedcurrencyfrom the dialog's in-memory history; change them to fetch that version via the existingget_currency_history_request(already returns the full typedvector<currency>history) and find the matchingversionbefore 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) onceCurrencyControlleris 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 onHistoryDialogBase(not just the generic dialog, since every history dialog shares this base):markAsStale()no longer callsloadHistory()directly; it starts aRecencyPulseManagerthat alternates the Reload action's icon between normal andcolor_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, andonReloadClicked()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.