Skip to content

bank: two GUI controllers publish four members no gui/qml file binds #296

Description

@Yaraslaut

Summary

bank's six QML controllers expose four QML-visible members that no file
under examples/bank/gui/qml/ binds
— a Q_PROPERTY whose read side is
never used, its NOTIFY signal, and two signals the controllers emit that
nothing handles.

Found on the first run of QmlSurfaceAudit against bank, while adopting the
guard there for #240 — the run that the bank GUI's first test target made
possible at all. Same shape as #239 (ledger, 15), #287 (lims, 15) and #291
(kanban, 9).

AppController, AccountController, CardController and LoanController are
clean.

Note that bank is not a ladder rung — it is absent from examples/rungs.txt
and never calls morph_add_rung() — but the guard it now runs is the ladder
testkit's, so the area: ladder label follows the audit, not the directory.

Verification status

Reproduced, on master plus the fix/qml-surface-drift branch (#294) plus
the bank adoption branch, locally: macOS 15 (arm64), Apple clang 17.0.0,
Qt 6.11.1, Debug.

cmake -S . -B build -G Ninja \
  -DMORPH_BUILD_EXAMPLES=ON -DMORPH_BUILD_BANK_EXAMPLE=ON \
  -DMORPH_BUILD_BANK_GUI=ON -DMORPH_BUILD_TESTS=ON
cmake --build build --target bank_gui_tests
build/examples/bank/bank_gui_tests "[qml-surface]"

No QT_QPA_PLATFORM=offscreen is needed: the audit reads metaobjects and file
text, and bank_gui_tests links Qt6::Core only — no QML engine, no
Qt6::Quick, no display.

The audit's own findings, verbatim, with every exemption removed:

bankgui::TransactionController::selectedAccount is a Q_PROPERTY no scanned .qml reads (bound as 'txns')
bankgui::TransactionController::selectedChanged is a signal no scanned .qml handles (bound as 'txns')
bankgui::TransactionController::posted is a signal no scanned .qml handles (bound as 'txns')
bankgui::PayeeController::paid is a signal no scanned .qml handles (bound as 'payees')

selectedAccount is the one that looks like a real bug

MoveMoneyPage.qml:28-31 drives the account picker one way only:

model: txns.accounts
onActivated: txns.selectAccount(currentValue)

selectAccount is the property's WRITE, and it is called. Nothing ever reads
txns.selectedAccount back, so the picker cannot display a selection the
controller made on its own — and TransactionController::refresh()
(TransactionController.cpp:35-45) does exactly that: when the previously
selected account disappears from the list it falls back to the first one and
emits selectedChanged. After that the combo box and the controller disagree
about which account a deposit will land in, with no visible sign.

selectedChanged is that property's NOTIFY signal and is listed only for
completeness; it stops being a finding the moment the property is read.

posted and paid are emitted and dropped

TransactionController::posted fires after every deposit, withdrawal and
transfer (TransactionController.cpp:100,114,128); PayeeController::paid
fires after every bill payment (PayeeController.cpp:81). No .qml file
handles either. The screens do refresh, because each controller reloads its own
state in the same callback — so these are either dead surface two controllers
should stop publishing, or the hook a success toast was meant to hang from.
The error path already has such a toast (Main.qml's showError), which is
what makes the second reading plausible.

What this is not

No screen is broken. The audit's other direction is clean: no QML file
binds a name its controller lacks. Each member here is either dead surface or a
missing control, and which of those it is has to be decided per member.

It is also not the five refresh findings that a naive run reports.
AppShell.qml:12-14 keeps the five page controllers in a
readonly property var controllers: [accounts, txns, cards, payees, loans] and
refreshes the visible page with controllers[current].refresh(). That is
dynamic member access, which QmlSurfaceAudit documents as outside what a text
scan can see. Those five are exempted with that reason, not this issue's, and
nothing about them needs fixing.

Current state

The exemptions are recorded, itemised, in
examples/bank/tests/gui/test_bank_qml_surface.cpp — so the guard is live and
catches the next drift in either direction, rather than being held back until
this backlog is cleared. The list is checked in both directions: an exemption
for a member that has since been deleted, or one QML has since bound, fails the
test. It can only shrink deliberately, and this issue closes when it is empty.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: ladderSubsystem: ladderarea: qtSubsystem: qtbugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions