feat(app): view an account's transactions in the Qt app#81
Merged
Conversation
The app could only show the aggregated balance chart; individual
transaction records were invisible without going through MCP, so
reconciling against a statement or verifying a recurring rule's output
had no in-app path. Add a master-detail Transactions screen: an account
selector drives a per-account ListTransactions fetch, the master list
shows one row per transaction, and selecting a row fills a detail pane.
The panel is driven entirely through a public property surface (account
index, list currentIndex) rather than clicks, so the offscreen Qt Quick
Test harness — which delivers no synthetic mouse events — can exercise
selection. Amount formatting is sign-correct cents ("-$12.34"), and the
TransactionStatus int maps to a human label, both pinned by spec.
This is read-only (#33); a fetch-error surface rides along with the
later transaction-recording work where write failures make it
load-bearing.
Closes #33
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address findings from the pre-PR adversarial review (all minor): - Collapse the account-list expression to a single source: the selector binds the panel's own `accounts` property (already client-null-guarded) instead of re-deriving `client.accounts`, so the selector and the index->id lookup in onAccountSelected can never diverge. - Strengthen the account-switch spec to deliver the new account's (shorter) data after the switch, proving the selection stays cleared once the swapped list actually arrives — not only at the synchronous reset. - Refresh the mock's stale header comment to note it now also records listTransactions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Selecting account B while account A's ListTransactions fetch was still in flight left A's transactions displayed under B's selector: the re-entrancy guard silently dropped B's fetch, and A's result then populated the list while the selector showed B. This input-ordering race is independent of the feature being read-only — it is about which account the user wants, not a mutation racing the read. The client now tracks the requested vs. in-flight account id. A request arriving mid-fetch is recorded but deferred (starting a concurrent QtConcurrent task would no longer be waited on by the destructor and could outlive the stub); on completion, if the selection has moved on, the stale result is discarded and the requested account re-fetched. With the guard there is still only ever one in-flight fetch, so the destructor's single waitForFinished remains sufficient. The mock is made a faithful double of this single-in-flight reconciliation so panel specs observe the real states during a rapid switch, and a spec locks the defer-discard-refetch contract end to end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new Transactions master–detail screen to the Qt app, backed by a new FinchClient::listTransactions() RPC wrapper, so users can browse and inspect per-account transaction records (issue #33).
Changes:
- Implemented
FinchClient::listTransactions()with single-in-flight request reconciliation for rapid account switching. - Added
TransactionsPanel.qmland wired it intoMain.qmlas the Transactions destination. - Added a dedicated Qt Quick Test suite for
TransactionsPanel, plus CMake wiring, and updated the changelog.
PR-description checks (per repo guidelines):
- Overview section: Pass
- Scope cohesion: Pass
- No private tooling references in tracked files: Pass
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CHANGELOG.md | Documents the new Transactions screen feature. |
| app/CMakeLists.txt | Adds TransactionsPanel to the app module and introduces a new tst_transactionspanel Quick Test target/module. |
| app/qml/Main.qml | Replaces the Transactions placeholder with the real TransactionsPanel and injects finchClient + connection state. |
| app/qml/TransactionsPanel.qml | New master–detail UI for listing transactions per account and showing selected-transaction details. |
| app/src/finchclient.h | Exposes transactions, transactionsLoading, and listTransactions() to QML. |
| app/src/finchclient.cpp | Implements the ListTransactions RPC call and in-flight reconciliation behavior. |
| app/tests/MockFinchClient.qml | Extends the shared QML test double to support listTransactions() behavior and reconciliation. |
| app/tests/tst_transactionspanel.cpp | Quick Test C++ entry point for the new TransactionsPanel QML test directory. |
| app/tests/transactionspanel/tst_TransactionsPanel.qml | Adds QML specs covering selection, rendering, formatting, empty/disconnected states, and mid-fetch switching. |
Comment on lines
+164
to
+174
| void FinchClient::listTransactions(const QString& accountId) | ||
| { | ||
| m_requestedTransactionsAccountId = accountId; | ||
| // A fetch is already in flight; do not start a concurrent one (a second concurrent | ||
| // QtConcurrent task would no longer be waited on by the destructor and could outlive the | ||
| // stub). onListTransactionsFinished reconciles to the requested account once it lands. | ||
| if (m_transactionsLoading) | ||
| return; | ||
|
|
||
| startTransactionsFetch(); | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in 038a716 — the list is now cleared at the start of each fetch, so the in-flight window can't show the previously loaded account's transactions under the new selection.
Comment on lines
+19
to
+21
| // Whether the daemon is connected (host-gated). When false the panel shows a connect | ||
| // hint instead of an account dropdown the user can't populate. | ||
| property bool connected: true |
Owner
Author
There was a problem hiding this comment.
Fixed in 038a716 — the selector is now disabled when disconnected (the account list is empty then) and the comment updated to match the actual behavior.
…ffline Round 1 of Copilot review on the transactions view: - Clear the transaction list at the start of each fetch, so the in-flight window never shows the previously loaded account's transactions under a newly selected account. Without this the PR's "never shows the wrong account's transactions" guarantee held only after the fetch completed, not during it. Mirrors fetchTimeSeries, which already clears at fetch start. The mock is kept faithful and a spec pins the cleared window. - Disable the account selector when the daemon is disconnected: the account list is empty then, so an enabled-but-empty dropdown was misleading. Corrects the property comment to match the behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jakewan
added a commit
that referenced
this pull request
Jun 22, 2026
The transactions-view work (#33, PR #81) surfaced three Qt traps not captured anywhere, so a future app session would re-discover them the hard way. Recorded in the path-conditioned qt-app.md rule: - Offscreen assertions: a control's visible/enabled getter returns effective (ancestor/window-dependent) state, so it reads false in the offscreen harness even when the local binding is true. Assert a logical readonly bool instead, bound to the visual property. - Layout recursive-rearrange: binding a child's Layout.preferredWidth to its parent layout's own size feeds back and makes Qt abort the pass with a runtime warning headless tests pass through. Use fillWidth; give ListView delegates a fixed width sized height-from-content. - Mock faithfulness: when a mock models client-internal behavior, mirror the real client's observable state transitions so specs see realistic states. Follows up the work in #33 (PR #81).
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.
Overview
The Qt app could only show the aggregated balance chart; individual transaction records were invisible without going through the MCP tools, so reconciling against a bank statement or verifying a recurring rule's output had no in-app path. This adds a per-account Transactions screen: pick an account, browse its transactions in a master list, and select one to see its full details (date, name, amount, status, description, and recurring-rule association). The screen is master-detail — a deliberate step up from the flat Accounts list, justified by transactions' richer per-record data.
How it works
The account selector starts unselected, so a fresh screen fetches nothing until a real account is chosen. Amounts are signed cents formatted sign-correct, and the transaction status maps to a human label rather than a raw enum value.
Rapid account switching is reconciled in the client: if you select a second account while the first account's fetch is still in flight, the client defers the second request, then on completion discards the now-stale result and re-fetches the account you actually have selected — so the list can never show one account's transactions under another's selector. Error surfacing is intentionally left to the later transaction-recording work, where write failures make it load-bearing; this screen covers loading, empty, and disconnected states only.
Closes #33