feat(navigation): auto-open invoice screens on daemon request - #289
Conversation
- emit_trade_update now fires from every dispatch arm that persists a status: escrow-locked, add-invoice, pay-invoice and the generic status-sync arm, not just cancellations. - Emission happens after book and DB are synced, so push listeners read consistent state. - Take replies are untouched: the take waiter consumes them before the arms, so takers produce no emission. - Stream, buffer and TradeUpdate docs updated to the new semantics.
- New TradeActionListener wraps the app root and listens to trade updates pushed by the Rust ingest. - WaitingBuyerInvoice sends the buyer to add-invoice; WaitingPayment sends the seller to pay-invoice, from any screen in the app. - Role check keeps informational syncs (the counterparty's copy of the same status) from navigating. - Skips when already on the destination route; in-flight dedupe collapses bursts of identical emissions. - Populates tradeRoleProvider before navigating, as the screens expect.
- WaitingBuyerInvoice and WaitingPayment no longer navigate from the maker's waiting screen; TradeActionListener pushes the invoice screen for the actionable role from anywhere, this screen included. - The counterparty's informational copy of those statuses is tracked but never navigated, same as before. - Post-frame navigation collapses to the trade detail route.
- on_trade_updated now covers every daemon-driven status sync, not just cancellations; consumers and the no-emission take-reply case listed. - Inbound actions table notes the emission after both stores are synced.
|
Warning Review limit reached
Next review available in: 40 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe app now emits trade updates after daemon status synchronization. A root listener opens role-specific invoice or payment routes globally. ChangesTrade-action navigation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Daemon
participant OrdersApi
participant TradeDatabase
participant tradeUpdatesProvider
participant TradeActionListener
participant NavigationRouter
Daemon->>OrdersApi: dispatch trade status action
OrdersApi->>TradeDatabase: attempt status persistence
OrdersApi->>tradeUpdatesProvider: emit TradeUpdate
tradeUpdatesProvider->>TradeActionListener: deliver update
TradeActionListener->>NavigationRouter: open role-specific action route
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/shared/widgets/trade_action_listener.dart`:
- Around line 36-68: Update _handle to track the latest status per orderId
before awaiting getTradeRole, then recheck that stored status still matches
update.status after the await and return when it has changed. Add a regression
test covering WaitingPayment followed by Active to ensure no stale pay-invoice
navigation occurs.
In `@rust/src/api/orders.rs`:
- Around line 2005-2013: Move each emit_trade_update call in the Kind 14
dispatch arms of rust/src/api/orders.rs (lines 2005-2013, 2056-2067, 2134,
2181-2189, and 3629-3636) into the successful update_trade_fields path, or
defer/retry it until synchronization succeeds; failed writes must not broadcast
TradeUpdate events. Update the corresponding contract at
specs/004-mostro-p2p-client/contracts/orders.md:264-266 to reflect that
TradeUpdate is emitted only after both book and persisted trade state are
consistent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 79bcb13f-8c2d-4a78-828b-009cbe5001c2
📒 Files selected for processing (6)
lib/core/app.dartlib/features/order/screens/my_order_screen.dartlib/shared/widgets/trade_action_listener.dartrust/src/api/orders.rsrust/src/api/types.rsspecs/004-mostro-p2p-client/contracts/orders.md
- Latest status per order is recorded synchronously on emission; a handler that finishes its role lookup after a newer status landed (startup replay delivers them milliseconds apart) no longer navigates. - resolveRole and navigate are injectable test seams. - Widget tests: actionable navigation, informational-copy skip, and the WaitingPayment-superseded-by-Active regression.
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
Thanks for the update — the stale WaitingPayment → Active navigation race is fixed on the current head, and the Rust/Flutter CI checks are green. I found one remaining blocker: the updated stream contract still says TradeUpdate is emitted after both stores are consistent, but the implementation intentionally emits after the DB write is only attempted (or when there is no DB). That mismatch is now the public contract for this stream, so it should be corrected before merge.
Validated locally:
cargo checkcargo test(249 passed, 8 ignored)
GitHub checks on c9ea11df2c12b60d21eddf3f3f84730ee3ea4688 are green: Flutter analyze/test, Rust build/test/clippy/wasm, and Web wasm smoke test.
- Emission happens after the in-memory book sync and the DB persistence attempt; DB failures or a memory-only session are logged and never suppress the notification. - Listeners must tolerate a trade row that is missing or behind the book; the role lookup in TradeActionListener already does. - Spec, on_trade_updated rustdoc and the listener doc now say so.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/shared/widgets/trade_action_listener_test.dart`:
- Around line 45-59: Add a targeted widget test alongside “actionable status
navigates and records the role” for OrderStatus.waitingBuyerInvoice, configuring
pumpListener to resolve TradeRole.buyer and asserting navigation to
AppRoute.addInvoicePath('o1') and tradeRoleProvider contains {'o1': true}.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 899db27e-fa26-4ca2-b7d1-ebe56a48ae89
📒 Files selected for processing (4)
lib/shared/widgets/trade_action_listener.dartrust/src/api/orders.rsspecs/004-mostro-p2p-client/contracts/orders.mdtest/shared/widgets/trade_action_listener_test.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- rust/src/api/orders.rs
- specs/004-mostro-p2p-client/contracts/orders.md
- WaitingBuyerInvoice with a buyer role navigates to add-invoice and records the role, mirroring the existing seller pay-invoice test.
There was a problem hiding this comment.
Review summary
Approved. The previous blocker is resolved on the current head: the TradeUpdate contract now describes the real semantics — emitted after the in-memory book update and DB persistence attempt, with DB failures logged and not suppressing the lifecycle notification.
I also re-checked the role-gated global navigation path and the new widget coverage for both WaitingPayment and WaitingBuyerInvoice, including the stale WaitingPayment → Active race.
Validated locally:
cargo checkcargo test(249 passed, 8 ignored)
GitHub checks on 248ebd7d523cd019fbf78a831564bf356d4a2443 are green: Flutter analyze/test, Rust build/test/clippy/wasm, and Web wasm smoke test.
1 participant
Closes #283
When the daemon sends
add-invoiceorpay-invoice, the app only navigatedautomatically if the user happened to be sitting on
MyOrderScreen(maker) orin the take-order flow (taker) — the navigation logic was local to those
screens. If the message arrived while the user was anywhere else (home, chats,
another trade), nothing happened on screen:they
must act, and both actions expire on a time#287,
where the trade died precisely because the
Fix
Two pieces, mirroring v1's global `Navigatiism:
emit_trade_updatenow fires rmthat persists a status (escrow-locked, ade
generic status-sync arm), not just canceler
the order book and the trade DB are syncethe
push reads consistent state. Take replieske
waiter consumes them before the arms run ng
local navigation with zero double-navigat
TradeActionListenerwraptradeUpdatesProvider:WaitingBuyerInvo add-invoice screen,WaitingPaymentpushice screen, from any screen in the app. It reso the counterparty's informational copy ofwaiting-seller-to-paypersistsWaitingo)never navigates. It also skips when the dn
top, dedupes bursts of identical emission
tradeRoleProviderbefore navigating, asMyOrderScreendrops its two invoice-navigr isthe single source of that navigation; it stfrom
Activeon. Existing consumers of the stredeslist already refetches on any emission (it , on
every daemon-driven transition), and the invoice screens only act on
cancellation statuses.
Deliberate deviation from v1: no Lightning-ation —
with NWC connected the add-invoice screen amits
the invoice, and LN-address pre-fill is tra
Changes
rust/src/api/orders.rs: emissions in the four dispatch arms; stream,buffer and
TradeUpdatedocs updated.rust/src/api/types.rs:TradeUpdatedoc updated.lib/core/app.dart: listener mounted atremoved, now global.
documented.
Summary by CodeRabbit