From 897bbba427a0547c0e3ea28ea7e690767f04b2b0 Mon Sep 17 00:00:00 2001 From: Catrya <140891948+Catrya@users.noreply.github.com> Date: Sat, 12 Sep 2026 21:48:17 -0600 Subject: [PATCH 1/2] fix(trades): a public pending is never a take's status tradeStatusProvider reads the order book first, and the book holds the order's public view. The trade screen showed that view as the trade's own status, so a take left Canceled by the old optimistic cancel (#434) read as the user's pending order once the daemon put the order back in the book, with a Cancel the daemon refuses (IsNotYourOrder). The My Trades list already kept an ended row's status, so the list said cancelled and the screen said pending. Both now go through one rule, shownTradeStatus. The row wins when it has ended, and when it is a take and the book says pending: a public pending means nobody holds the order, which also covers a take parked at WaitingTakerBond, whose order is still pending in public. Every other live status still wins over an open row. In the trade screen the rule applies wherever the live status was read: the status, the post-cancel decision, the cancel dialog's copy, and the step nudge, so a book change the row outranks is not a step. --- .../order/providers/trade_state_provider.dart | 34 ++++- .../trades/providers/trade_rows_provider.dart | 25 ++-- .../trades/screens/trade_detail_screen.dart | 35 ++++- .../004-mostro-p2p-client/contracts/orders.md | 14 ++ .../providers/trade_state_provider_test.dart | 60 +++++++++ .../providers/trade_rows_provider_test.dart | 23 ++++ .../trades/trade_detail_screen_test.dart | 127 ++++++++++++++++++ 7 files changed, 296 insertions(+), 22 deletions(-) diff --git a/lib/features/order/providers/trade_state_provider.dart b/lib/features/order/providers/trade_state_provider.dart index 811e8816..78583e2f 100644 --- a/lib/features/order/providers/trade_state_provider.dart +++ b/lib/features/order/providers/trade_state_provider.dart @@ -88,7 +88,7 @@ final tradeStatusProvider = StreamProvider.family final status = await lookup(orderId); if (status != null) { yield status; - if (_isTerminal(status)) return; + if (isTerminalTradeStatus(status)) return; } await Future.delayed(const Duration(seconds: 2)); } @@ -111,8 +111,9 @@ final tradeUpdatesProvider = StreamProvider.autoDispose(( } }); -/// Whether the UI can stop polling. Escrow settlement still awaits payout. -bool _isTerminal(OrderStatus s) => const { +/// Whether a trade in [s] has ended: nothing moves it out again, so the UI +/// can stop polling. Escrow settlement still awaits payout. +bool isTerminalTradeStatus(OrderStatus s) => const { OrderStatus.success, OrderStatus.settledByAdmin, OrderStatus.completedByAdmin, @@ -122,6 +123,33 @@ bool _isTerminal(OrderStatus s) => const { OrderStatus.canceledByAdmin, }.contains(s); +/// The status a trade shows: its [row]'s persisted one, or the [live] one +/// from [tradeStatusProvider], which reads the order book first. +/// +/// The row wins in two cases: +/// * **It has ended** ([isTerminalTradeStatus]). Whatever the book says about +/// the order later is no longer this trade. The one way such a row can be +/// wrong is the cancel's optimistic write on an active trade: a cooperative +/// cancel the peer never accepts, on a trade that then completes. +/// * **It is a take ([isTake]) and the book says `pending`.** A public +/// `pending` means nobody holds the order, so it is never a take's status. +/// Older builds marked a take `Canceled` as soon as its cancel went out, +/// even before it went active; once the daemon put the order back in the +/// book, that `pending` read as the user's own order, with a Cancel the +/// daemon refuses (`IsNotYourOrder`). A take parked at `WaitingTakerBond` +/// is another: publicly its order is still `pending`. +/// +/// Otherwise the live status, or the row's while there is none yet. +OrderStatus shownTradeStatus({ + required OrderStatus row, + required OrderStatus? live, + required bool isTake, +}) { + if (live == null || isTerminalTradeStatus(row)) return row; + if (isTake && live == OrderStatus.pending) return row; + return live; +} + /// Loads the buyer/seller role for a trade from the persistent DB. /// /// Returns `true` when the local user is the buyer, `false` for seller, or diff --git a/lib/features/trades/providers/trade_rows_provider.dart b/lib/features/trades/providers/trade_rows_provider.dart index b1213890..a63a52d4 100644 --- a/lib/features/trades/providers/trade_rows_provider.dart +++ b/lib/features/trades/providers/trade_rows_provider.dart @@ -34,7 +34,8 @@ class TradeRow { final String orderId; - /// Live when the order is still moving, else the persisted status. + /// Live when the order is still moving, else the persisted status — see + /// [shownTradeStatus], which the trade screen shares. final rust_types.OrderStatus status; final TradeRowState state; final bool isSelling; @@ -59,16 +60,6 @@ class TradeRow { final String? peerHandle; } -const _terminal = { - rust_types.OrderStatus.success, - rust_types.OrderStatus.settledByAdmin, - rust_types.OrderStatus.completedByAdmin, - rust_types.OrderStatus.canceled, - rust_types.OrderStatus.expired, - rust_types.OrderStatus.cooperativelyCanceled, - rust_types.OrderStatus.canceledByAdmin, -}; - const _successes = { rust_types.OrderStatus.success, rust_types.OrderStatus.settledByAdmin, @@ -89,10 +80,14 @@ final tradeRowsProvider = Provider>>((ref) { TradeRow _row(Ref ref, rust_types.TradeInfo trade, {required bool canRate}) { final order = trade.order; final persisted = order.status; - final status = - _terminal.contains(persisted) - ? persisted - : ref.watch(tradeStatusProvider(order.id)).valueOrNull ?? persisted; + final status = shownTradeStatus( + row: persisted, + live: + isTerminalTradeStatus(persisted) + ? null + : ref.watch(tradeStatusProvider(order.id)).valueOrNull, + isTake: !order.isMine, + ); final ratedByMe = trade.ratedAt != null || (_successes.contains(status) && ref.watch(ratedByMeProvider(order.id))); diff --git a/lib/features/trades/screens/trade_detail_screen.dart b/lib/features/trades/screens/trade_detail_screen.dart index 4abaaa76..07cbf4e7 100644 --- a/lib/features/trades/screens/trade_detail_screen.dart +++ b/lib/features/trades/screens/trade_detail_screen.dart @@ -36,6 +36,7 @@ import 'package:mostro/shared/widgets/mostro_reactive_button.dart'; import 'package:mostro/src/rust/api/disputes.dart' as disputes_api; import 'package:mostro/src/rust/api/orders.dart' as orders_api; import 'package:mostro/src/rust/api/reputation.dart' as reputation_api; +import 'package:mostro/src/rust/api/types.dart' show TradeInfo; export 'package:mostro/features/trades/models/trade_status.dart'; @@ -189,9 +190,23 @@ class _TradeDetailScreenState extends ConsumerState { /// await: the seller's payment can land while the cancel dialog is open. TradeStatus _liveStatus(TradeStatus fallback) { final live = ref.read(tradeStatusProvider(widget.orderId)).valueOrNull; - return live == null ? fallback : tradeStatusFromOrderStatus(live); + if (live == null) return fallback; + final trade = ref.read(tradeInfoProvider(widget.orderId)).valueOrNull; + return tradeStatusFromOrderStatus(_shown(live, trade)); } + /// What [live] shows for this trade once its [trade] row is read in + /// ([shownTradeStatus]); [live] itself while the row has not loaded, or + /// when there is none. + static OrderStatus _shown(OrderStatus live, TradeInfo? trade) => + trade == null + ? live + : shownTradeStatus( + row: trade.order.status, + live: live, + isTake: !trade.order.isMine, + ); + Future _cancelOrder(TradeStatus status) async { final l10n = AppLocalizations.of(context); final confirmed = await showDialog( @@ -207,8 +222,14 @@ class _TradeDetailScreenState extends ConsumerState { dialogRef .watch(tradeStatusProvider(widget.orderId)) .valueOrNull; + final trade = + dialogRef + .watch(tradeInfoProvider(widget.orderId)) + .valueOrNull; final now = - live == null ? status : tradeStatusFromOrderStatus(live); + live == null + ? status + : tradeStatusFromOrderStatus(_shown(live, trade)); return Text(_cancelDialogContent(l10n, now)); }, ), @@ -405,7 +426,8 @@ class _TradeDetailScreenState extends ConsumerState { debugPrint('[TradeDetailScreen] trade status failed: ${live.error}'); } if (!live.hasValue) return TradeStatus.loading; - final status = tradeStatusFromOrderStatus(live.value!); + final trade = ref.watch(tradeInfoProvider(widget.orderId)).valueOrNull; + final status = tradeStatusFromOrderStatus(_shown(live.value!, trade)); if (status != TradeStatus.pendingRating) return status; final rating = ref.watch(tradeRatingProvider(widget.orderId)); if (rating.isLoading && !rating.hasValue) return TradeStatus.loading; @@ -439,13 +461,18 @@ class _TradeDetailScreenState extends ConsumerState { // A step advanced by the relay: a nudge, the crossfades below, and a // fresh deadline — every step has its own expiration, owned by a // different party, so the clock loaded for the previous step is stale. + // Judged on what the screen shows: a book change the row outranks is + // not a step of this trade. ref.listen>(tradeStatusProvider(widget.orderId), ( previous, next, ) { + final trade = ref.read(tradeInfoProvider(widget.orderId)).valueOrNull; final before = previous?.valueOrNull; final after = next.valueOrNull; - if (before != null && after != null && before != after) { + if (before != null && + after != null && + _shown(before, trade) != _shown(after, trade)) { HapticFeedback.mediumImpact(); _loadExpiresAt(); } diff --git a/specs/004-mostro-p2p-client/contracts/orders.md b/specs/004-mostro-p2p-client/contracts/orders.md index 0a356ea1..f8d4148f 100644 --- a/specs/004-mostro-p2p-client/contracts/orders.md +++ b/specs/004-mostro-p2p-client/contracts/orders.md @@ -749,3 +749,17 @@ The seller pay-invoice flow uses two complementary providers from advancing past the pay-invoice screen; the NWC widget's local `onPaymentSuccess` callback only flips a spinner flag and does not navigate. + +`tradeStatusProvider` reads the order book first, and the book holds the +order's public view. So the My Trades list and `TradeDetailScreen` show a +trade through `shownTradeStatus`, where the trade row wins in two cases: + +- **The row has ended** (success or a cancelled family): whatever the book + says about the order afterwards is no longer this trade. +- **A take, and the book says `pending`**: a public `pending` means nobody + holds the order, so it is never a take's status. That covers a take left + `Canceled` by builds that wrote the status before the daemon answered + (the daemon later put the order back in the book), and a take parked at + `WaitingTakerBond`, whose order is still `pending` in public. + +Every other live status wins over an open row. diff --git a/test/features/order/providers/trade_state_provider_test.dart b/test/features/order/providers/trade_state_provider_test.dart index cb44d0b4..327eed89 100644 --- a/test/features/order/providers/trade_state_provider_test.dart +++ b/test/features/order/providers/trade_state_provider_test.dart @@ -47,4 +47,64 @@ void main() { expect(lookups, 2); expect(requestedOrders, ['order-payout', 'order-payout']); }); + + group('shownTradeStatus', () { + test('an ended row wins over any live status', () { + for (final row in OrderStatus.values.where(isTerminalTradeStatus)) { + for (final live in OrderStatus.values) { + for (final isTake in [true, false]) { + expect( + shownTradeStatus(row: row, live: live, isTake: isTake), + row, + reason: 'row=$row live=$live isTake=$isTake', + ); + } + } + } + }); + + test('a public pending never shows over a take that is still open', () { + final open = OrderStatus.values.where((s) => !isTerminalTradeStatus(s)); + for (final row in open) { + expect( + shownTradeStatus(row: row, live: OrderStatus.pending, isTake: true), + row, + reason: 'row=$row', + ); + // The maker's own order is `pending` for real. + expect( + shownTradeStatus(row: row, live: OrderStatus.pending, isTake: false), + OrderStatus.pending, + reason: 'row=$row', + ); + } + }); + + test('any other live status wins over an open row', () { + final open = OrderStatus.values.where((s) => !isTerminalTradeStatus(s)); + for (final row in open) { + for (final live in OrderStatus.values) { + if (live == OrderStatus.pending) continue; + for (final isTake in [true, false]) { + expect( + shownTradeStatus(row: row, live: live, isTake: isTake), + live, + reason: 'row=$row live=$live isTake=$isTake', + ); + } + } + } + }); + + test('the row stands in while there is no live status yet', () { + expect( + shownTradeStatus( + row: OrderStatus.waitingPayment, + live: null, + isTake: true, + ), + OrderStatus.waitingPayment, + ); + }); + }); } diff --git a/test/features/trades/providers/trade_rows_provider_test.dart b/test/features/trades/providers/trade_rows_provider_test.dart index 575e5ea6..ff6e3b0b 100644 --- a/test/features/trades/providers/trade_rows_provider_test.dart +++ b/test/features/trades/providers/trade_rows_provider_test.dart @@ -69,6 +69,29 @@ void main() { expect((await _rows(c)).single.status, OrderStatus.canceled); }); + test('a take keeps its own status over a public pending', () async { + // A take parked at its bond: publicly the order is still `pending`. + final c = _container( + [fakeTrade(id: 'a', status: OrderStatus.waitingTakerBond)], + live: {'order-a': OrderStatus.pending}, + ); + expect((await _rows(c)).single.status, OrderStatus.waitingTakerBond); + }); + + test("a maker's pending order still follows the book", () async { + final c = _container( + [ + fakeTrade( + id: 'a', + status: OrderStatus.waitingBuyerInvoice, + isMine: true, + ), + ], + live: {'order-a': OrderStatus.pending}, + ); + expect((await _rows(c)).single.status, OrderStatus.pending); + }); + test('a durable rating marker closes a successful trade', () async { final rated = fakeTrade(id: 'a', status: OrderStatus.success); final c = _container([ diff --git a/test/features/trades/trade_detail_screen_test.dart b/test/features/trades/trade_detail_screen_test.dart index 609d5842..02debdc5 100644 --- a/test/features/trades/trade_detail_screen_test.dart +++ b/test/features/trades/trade_detail_screen_test.dart @@ -1,6 +1,7 @@ import 'dart:async'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_localizations/flutter_localizations.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -1267,4 +1268,130 @@ void main() { ); }); }); + + group('the trade row outranks what the book says about the order', () { + const orderId = 'order-row'; + + testWidgets('a take left Canceled reads cancelled over a public pending', ( + tester, + ) async { + // Older builds marked a take Canceled as soon as its cancel went out; + // the daemon then put the order back in the book, where it reads + // `pending`. Shown as is, that was the user's own order with a Cancel + // the daemon refuses (IsNotYourOrder). + await _pumpRoutedTradeDetail( + tester, + orderId: orderId, + status: OrderStatus.pending, + loadTrades: + () async => [fakeTrade(id: 'row', status: OrderStatus.canceled)], + book: [fakeOrder(id: orderId)], + ); + await _finishPageTransition(tester); + + expect(find.byType(TradeDetailScreen), findsOneWidget); + expect(find.text(_en.tradeHeadlineCancelled), findsOneWidget); + expect(find.text(_en.tradeHeadlinePending), findsNothing); + expect(_cancelButton(), findsNothing); + }); + + testWidgets('an ended take stays ended when someone else completes it', ( + tester, + ) async { + // The same leftover row, its order since taken and completed by + // someone else: the book's `success` is not this user's trade. The + // My Trades list reads it the same way. + await _pumpRoutedTradeDetail( + tester, + orderId: orderId, + status: OrderStatus.success, + loadTrades: + () async => [fakeTrade(id: 'row', status: OrderStatus.canceled)], + ); + await _finishPageTransition(tester); + + expect(find.text(_en.tradeHeadlineCancelled), findsOneWidget); + expect(find.byType(TradeCompletedCard), findsNothing); + }); + + testWidgets('an open take keeps its own step over a public pending', ( + tester, + ) async { + await _pumpRoutedTradeDetail( + tester, + orderId: orderId, + status: OrderStatus.pending, + loadTrades: + () async => [ + fakeTrade(id: 'row', status: OrderStatus.waitingPayment), + ], + book: [fakeOrder(id: orderId)], + ); + await _finishPageTransition(tester); + + expect(find.text(_en.tradeHeadlineWaitingPaymentBuyer), findsOneWidget); + expect(find.text(_en.tradeHeadlinePending), findsNothing); + }); + + testWidgets("a maker's order still reads pending from the book", ( + tester, + ) async { + await _pumpRoutedTradeDetail( + tester, + orderId: orderId, + status: OrderStatus.pending, + loadTrades: + () async => [ + fakeTrade( + id: 'row', + status: OrderStatus.waitingPayment, + isMine: true, + ), + ], + book: [fakeOrder(id: orderId, isMine: true)], + ); + await _finishPageTransition(tester); + + expect(find.text(_en.tradeHeadlinePending), findsOneWidget); + }); + + testWidgets('a book change the row outranks is not a step: no nudge', ( + tester, + ) async { + final nudges = []; + tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + SystemChannels.platform, + (call) async { + if (call.method == 'HapticFeedback.vibrate') nudges.add('$call'); + return null; + }, + ); + addTearDown( + () => tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + SystemChannels.platform, + null, + ), + ); + final book = StreamController(); + addTearDown(() => unawaited(book.close())); + book.add(OrderStatus.pending); + await _pumpRoutedTradeDetail( + tester, + orderId: orderId, + status: OrderStatus.pending, + statusUpdates: book.stream, + loadTrades: + () async => [fakeTrade(id: 'row', status: OrderStatus.canceled)], + ); + await _finishPageTransition(tester); + + // Someone else takes the order: the book moves, this trade does not. + book.add(OrderStatus.inProgress); + await tester.pump(); + await tester.pump(); + + expect(nudges, isEmpty); + expect(find.text(_en.tradeHeadlineCancelled), findsOneWidget); + }); + }); } From c0aa804e5a2476ce82b7b54787f1b20be61b65d2 Mon Sep 17 00:00:00 2001 From: Catrya <140891948+Catrya@users.noreply.github.com> Date: Sat, 12 Sep 2026 22:55:11 -0600 Subject: [PATCH 2/2] fix(order): a take that already ended no longer blocks taking the order The take screen sends a user who already takes part in the order to the trade instead of offering to take it again, and it counted any trade row on the order as taking part. A take left Canceled by the old optimistic cancel (#434) therefore held the order forever: once the daemon put it back in the book, opening it landed on that dead trade, and the take button did the same, so the user could never take it again. tradeRoleLookupProvider now answers through participatingRole: a take whose row has ended does not count. Once its order can be taken again, such a row can only be what a take that never went active left behind, since an order whose trade truly ended never goes back to Pending, and take_order already replaces it with the new take's row. A maker's row always counts, and so does any row still open. Every row of the order is read, because a database from before takes replaced their order's earlier row can hold two. --- .../order/providers/trade_state_provider.dart | 42 ++++++++- .../004-mostro-p2p-client/contracts/orders.md | 7 ++ .../providers/trade_state_provider_test.dart | 70 +++++++++++++++ .../order/screens/take_order_screen_test.dart | 88 +++++++++++++++++-- test/support/fake_trades.dart | 6 +- 5 files changed, 202 insertions(+), 11 deletions(-) diff --git a/lib/features/order/providers/trade_state_provider.dart b/lib/features/order/providers/trade_state_provider.dart index 78583e2f..e23b4fa2 100644 --- a/lib/features/order/providers/trade_state_provider.dart +++ b/lib/features/order/providers/trade_state_provider.dart @@ -41,13 +41,47 @@ final tradeStatusLookupProvider = }, ); -/// Reads the local user's role in a trade through the bridge; injectable so -/// screens that must know whether they already participate can be tested -/// without the Rust side. +/// The local user's role in an order they still take part in +/// ([participatingRole]), read through the bridge; injectable so screens that +/// must know whether they already participate can be tested without the Rust +/// side. final tradeRoleLookupProvider = Provider Function(String)>( - (ref) => (orderId) => orders_api.getTradeRole(orderId: orderId), + (ref) => + (orderId) async => + participatingRole(await orders_api.listTrades(), orderId), ); +/// The role of the user's trade on [orderId] among [trades], or null when +/// they no longer take part in it: no row at all, or only a take that has +/// ended ([isEndedTake]). +/// +/// Every row for the order is read, not just one: a database from before +/// takes replaced their order's earlier row can hold two, and a live one +/// among them still makes the user a participant. +TradeRole? participatingRole(Iterable trades, String orderId) { + for (final trade in trades) { + if (trade.order.id == orderId && !isEndedTake(trade)) return trade.role; + } + return null; +} + +/// Whether [trade] is a take whose row has ended, which leaves the user +/// nothing to follow on its order. +/// +/// A trade that truly ended leaves its order in a status mostrod never takes +/// it out of: a take needs `Pending`, and only a waiting state goes back to +/// it. So once the order can be taken again, such a row is what a take that +/// never went active left behind: older builds marked it `Canceled` as soon +/// as its cancel went out. Holding on to it sent the user to that dead trade +/// instead of letting them take the order again (#434). Rust already takes +/// over such a row: the confirmed take replaces every earlier row of its +/// order. +/// +/// Never a maker's row: its order is theirs, and the take screen is not +/// where they manage it. +bool isEndedTake(TradeInfo trade) => + !trade.order.isMine && isTerminalTradeStatus(trade.order.status); + /// Takes an order through the bridge; injectable so the take screen's /// outcomes (loading, already taken, rejected) can be tested without Rust. final takeOrderActionProvider = Provider< diff --git a/specs/004-mostro-p2p-client/contracts/orders.md b/specs/004-mostro-p2p-client/contracts/orders.md index f8d4148f..f65ffce6 100644 --- a/specs/004-mostro-p2p-client/contracts/orders.md +++ b/specs/004-mostro-p2p-client/contracts/orders.md @@ -763,3 +763,10 @@ trade through `shownTradeStatus`, where the trade row wins in two cases: `WaitingTakerBond`, whose order is still `pending` in public. Every other live status wins over an open row. + +The take screen sends a user who already takes part in the order to the +trade instead of offering to take it again (`tradeRoleLookupProvider`, +`participatingRole`). A take whose row has ended does not count: once its +order can be taken again, such a row can only be what a take that never +went active left behind, and `take_order` replaces it with the new take's +row. A maker's row always counts, and so does any row still open. diff --git a/test/features/order/providers/trade_state_provider_test.dart b/test/features/order/providers/trade_state_provider_test.dart index 327eed89..cd0e5be8 100644 --- a/test/features/order/providers/trade_state_provider_test.dart +++ b/test/features/order/providers/trade_state_provider_test.dart @@ -4,6 +4,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mostro/features/order/providers/trade_state_provider.dart'; import 'package:mostro/src/rust/api/types.dart'; +import '../../../support/fake_trades.dart'; import '../../../support/provider_harness.dart'; void main() { @@ -107,4 +108,73 @@ void main() { ); }); }); + + group('participatingRole', () { + const orderId = 'order-x'; + + test('no row on the order is no role', () { + expect(participatingRole(const [], orderId), isNull); + expect( + participatingRole([fakeTrade(id: 'other')], orderId), + isNull, + reason: 'another order\'s row', + ); + }); + + test('a take still open is a participant', () { + for (final status in OrderStatus.values.where( + (s) => !isTerminalTradeStatus(s), + )) { + expect( + participatingRole([ + fakeTrade( + id: 'x', + orderId: orderId, + status: status, + role: TradeRole.seller, + ), + ], orderId), + TradeRole.seller, + reason: '$status', + ); + } + }); + + test('a take that has ended is not', () { + for (final status in OrderStatus.values.where(isTerminalTradeStatus)) { + final ended = fakeTrade(id: 'x', orderId: orderId, status: status); + expect(isEndedTake(ended), isTrue, reason: '$status'); + expect(participatingRole([ended], orderId), isNull, reason: '$status'); + } + }); + + test("a maker's row always is, ended or not", () { + final maker = fakeTrade( + id: 'x', + orderId: orderId, + status: OrderStatus.canceled, + isMine: true, + ); + expect(isEndedTake(maker), isFalse); + expect(participatingRole([maker], orderId), TradeRole.buyer); + }); + + test('an open row wins over an ended one on the same order', () { + // Databases from before a take replaced its order's earlier row can + // hold both, in either order. + final ended = fakeTrade( + id: 'ended', + orderId: orderId, + status: OrderStatus.canceled, + ); + final open = fakeTrade( + id: 'open', + orderId: orderId, + status: OrderStatus.active, + role: TradeRole.seller, + ); + expect(participatingRole([ended, open], orderId), TradeRole.seller); + expect(participatingRole([open, ended], orderId), TradeRole.seller); + }); + }); } diff --git a/test/features/order/screens/take_order_screen_test.dart b/test/features/order/screens/take_order_screen_test.dart index ebd89c40..07bb015d 100644 --- a/test/features/order/screens/take_order_screen_test.dart +++ b/test/features/order/screens/take_order_screen_test.dart @@ -4,6 +4,8 @@ import 'package:clock/clock.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; +import 'package:mostro/core/app_routes.dart'; import 'package:mostro/core/app_theme.dart'; import 'package:mostro/core/automation/automation_id.dart'; import 'package:mostro/core/automation/automation_ids.dart'; @@ -18,6 +20,7 @@ import 'package:mostro/src/rust/api/types.dart'; import 'package:mostro/shared/utils/fiat_currencies.dart'; import '../../../support/fake_orders.dart'; +import '../../../support/fake_trades.dart'; import '../../../support/provider_harness.dart'; const _id = '09150348-1a2b-4c3d-8e9f-0a1b2c3d99b5'; @@ -25,8 +28,9 @@ const _dark = OrderDetailPalette.dark; const _book = OrderBookPalette.dark; /// Pumps the take-order screen over a book fed by [books], with the node's -/// rate and the (absent) trade role stubbed. 1 000 ARS is 1 000 sats at -/// the stubbed rate, so the estimates are easy to read. +/// rate stubbed and the user's trade rows being [trades] (none by default). +/// 1 000 ARS is 1 000 sats at the stubbed rate, so the estimates are easy to +/// read. typedef _Take = Future Function({ required String orderId, required TradeRole role, @@ -39,6 +43,7 @@ Future>> _pump( bool isBuying = true, double? rate = 100000000, _Take? take, + List trades = const [], }) async { tester.view.physicalSize = const Size(360, 760); tester.view.devicePixelRatio = 1.0; @@ -51,7 +56,9 @@ Future>> _pump( yield [order]; yield* books.stream; }), - tradeRoleLookupProvider.overrideWithValue((_) async => null), + tradeRoleLookupProvider.overrideWithValue( + (orderId) async => participatingRole(trades, orderId), + ), if (take != null) takeOrderActionProvider.overrideWithValue(take), exchangeRateProvider.overrideWith((ref, code) async => rate), fiatCurrenciesProvider.overrideWith( @@ -61,15 +68,32 @@ Future>> _pump( ), ], ); + // Under a router, so a redirect to the trade is observable: it lands on a + // stand-in reading `trade`. + final router = GoRouter( + initialLocation: + isBuying ? AppRoute.takeSellPath(_id) : AppRoute.takeBuyPath(_id), + routes: [ + GoRoute( + path: isBuying ? AppRoute.takeSell : AppRoute.takeBuy, + builder: (_, __) => TakeOrderScreen(orderId: _id, isBuying: isBuying), + ), + GoRoute( + path: AppRoute.tradeDetail, + builder: (_, __) => const Scaffold(body: Text('trade')), + ), + ], + ); + addTearDown(router.dispose); await tester.pumpWidget( UncontrolledProviderScope( container: container, - child: MaterialApp( + child: MaterialApp.router( theme: buildDarkTheme(), locale: const Locale('en'), localizationsDelegates: AppLocalizations.localizationsDelegates, supportedLocales: AppLocalizations.supportedLocales, - home: TakeOrderScreen(orderId: _id, isBuying: isBuying), + routerConfig: router, ), ), ); @@ -349,4 +373,58 @@ void main() { }); }); }); + + group('TakeOrderScreen and a trade the user already has on the order', () { + testWidgets('a take still open on the order lands on its trade', ( + tester, + ) async { + await withClock(Clock.fixed(kFakeNow), () async { + await _pump( + tester, + order: _order(), + trades: [ + fakeTrade( + id: 'open', + orderId: _id, + status: OrderStatus.waitingBuyerInvoice, + ), + ], + ); + + expect(find.byType(TakeOrderScreen), findsNothing); + expect(find.text('trade'), findsOneWidget); + }); + }); + + testWidgets('a take that already ended leaves the order takeable', ( + tester, + ) async { + // What older builds left behind: a take cancelled before it went + // active, marked Canceled, its order since back in the book. Sent to + // that dead trade, the user could never take the order again (#434). + await withClock(Clock.fixed(kFakeNow), () async { + final taken = []; + await _pump( + tester, + order: _order(), + trades: [ + fakeTrade(id: 'ended', orderId: _id, status: OrderStatus.canceled), + ], + take: ({required orderId, required role, fiatAmount}) { + taken.add(orderId); + // Left unanswered: only the dispatch is under test. + return Completer().future; + }, + ); + expect(find.byType(TakeOrderScreen), findsOneWidget); + + await tester.tap(find.text('Take order')); + await tester.pump(); + await tester.pump(); + + expect(taken, [_id]); + expect(find.text('trade'), findsNothing); + }); + }); + }); } diff --git a/test/support/fake_trades.dart b/test/support/fake_trades.dart index 87c24c88..ddabd730 100644 --- a/test/support/fake_trades.dart +++ b/test/support/fake_trades.dart @@ -2,9 +2,11 @@ import 'package:mostro/src/rust/api/types.dart'; /// Builds a [TradeInfo] exposing only the fields the trades-list mapping reads. /// [startedAt] is the newest-first sort key. `currentStep` is unread by the -/// mapping, so it takes an arbitrary value. +/// mapping, so it takes an arbitrary value. The order id is `order-$id` +/// unless [orderId] names one. TradeInfo fakeTrade({ String id = 'trade-1', + String? orderId, OrderStatus status = OrderStatus.active, TradeRole role = TradeRole.buyer, String fiatCode = 'USD', @@ -18,7 +20,7 @@ TradeInfo fakeTrade({ int? peerDays, }) { final order = OrderInfo( - id: 'order-$id', + id: orderId ?? 'order-$id', kind: OrderKind.sell, status: status, amountSats: amountSats,