Skip to content

fix(buy): open order details from buy success - #2695

Merged
ethicnology merged 3 commits into
developfrom
fix/buy-success-view-details-route
Aug 19, 2026
Merged

fix(buy): open order details from buy success #2695
ethicnology merged 3 commits into
developfrom
fix/buy-success-view-details-route

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

No description provided.

@wired-pasteque wired-pasteque self-assigned this Aug 19, 2026
@claude

This comment was marked as outdated.

@SatoshiPortal SatoshiPortal deleted a comment from claude Bot Aug 19, 2026
@wired-pasteque
wired-pasteque force-pushed the fix/buy-success-view-details-route branch from abcdf11 to 09c29cc Compare August 19, 2026 18:24
@wired-pasteque
wired-pasteque force-pushed the fix/buy-success-view-details-route branch from 09c29cc to 29917cd Compare August 19, 2026 18:27
@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude, review the last fixes

@SatoshiPortal SatoshiPortal deleted a comment from claude Bot Aug 19, 2026
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 3m 49s —— View job


Review of the latest fixes

  • Gather context (diff vs origin/develop, read changed files)
  • Review the changes for correctness and quality
  • Post review feedback

Verdict: the fixes look correct and well-tested. No blocking issues; two minor observations below.

What the changes do (and why they hold together)

  1. buy_success_screen.dart — "View details" now always routes to orderTransactionDetails instead of branching on the payjoin txid. This works because the resolution logic moved into the cubit, which is the right place for it. The payjoinTransactionDetailsByTxId route is still used by the pay/sell success screens, so nothing became dead code.

  2. transaction_details_cubit.dart:810_loadDetailsByOrderId falls back to order.payjoin?.txid when order.transactionId is null. I verified the chain end-to-end: the fallback txid feeds GetTransactionsByTxIdUsecase, whose order lookup (getOrderByTxIdOrderModel.matchesTxId, order_model.dart:222) explicitly matches payjoinDetails['txid'] — so the order is re-found from the payjoin txid, and initByWalletTxId arms the wallet-tx watcher plus the payjoin watcher via _loadDetailsByWalletTxId. Ordering transactionId ?? payjoin?.txid correctly prefers the settled payout txid once the exchange reports it.

  3. get_transactions_by_tx_id_usecase.dart:101-104 — attaching order to the payjoin-only branch is consistent with the wallet-transactions branch (which already passes order) and is exactly what keeps the order-anchored details screen from degrading to a bare payjoin session before the wallet indexes the broadcast.

  4. Tests — the three new cubit tests cover the resolution matrix (payjoin-only fallback, both-known preference, neither-known order-only), and the capture-based expectResolvedTxIds is the right assertion given the double lookup (_loadDetailsByOrderIdinitByWalletTxId both call the usecase). The new usecase test file covers both keep-order and no-order cases.

Minor observations (non-blocking)

  1. Exception-as-control-flow on the order-only path — at transaction_details_cubit.dart:814, txs.first.walletId uses Transaction.walletId (transaction.dart:247), which ends in payjoin!.walletId and throws for an order-only Transaction. The payjoin fallback makes this path more reachable: if the order has a payjoin txid but the local payjoin session is gone (fresh install/restore) and the wallet hasn't indexed the tx, the usecase returns [Transaction(order: order)], the null-assertion throws, and the inner catch silently downgrades to order-only details. The end state is correct, but it works by accident of the catch block. An explicit guard (e.g. skip initByWalletTxId when the resolved transaction has neither a wallet tx, swap, orderSwap, nor payjoin) would make the intent visible. Fix this →

  2. Branch consistency in the usecase — the payjoin-only branch now carries order, but the swap/orderSwap branches still return bare Transaction(swap: swap) / Transaction(orderSwap: orderSwap) without it. Out of scope for this fix, just noting the asymmetry in case an order-linked swap ever hits the same "details screen loses the order" symptom.

Also worth knowing: since pay/sell payjoin-details screens resolve through this same usecase, they'll now also get the order attached on payjoin-only lookups — that looks like a desirable side effect, not a regression.

Not verified

I could not run the test suite — no Flutter SDK is installed on this runner (the repo pins Flutter 3.44.9 via .fvmrc, but neither flutter nor fvm is on PATH). Review is static; CI should confirm the new tests pass.

@ethicnology
ethicnology merged commit a242425 into develop Aug 19, 2026
2 checks passed
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