Feat/maker order ux - #81
Conversation
- Add isMine field to OrderItem and map it from OrderInfo - Show own orders on all tabs regardless of kind filter - Display "YOU ARE SELLING" / "YOU ARE BUYING" pill on maker's order cards - Navigate to a dedicated MyOrderScreen when tapping a maker's own order - MyOrderScreen shows order details with a Cancel button and no Buy/Sell action - Cancel sends a cancel message to the Mostro node and navigates home on success - Also store trade key by local UUID so cancel_order can look it up before the daemon's real order ID is reconciled via resolve_maker_order
- Replace nested ternary with switch expression for pill label - Externalize all hardcoded strings to ARB localization files with translations for en, es, fr, de, it - Make MyOrderScreen status card dynamic based on order.status with matching icon and color per state instead of hardcoded pending text
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
WalkthroughAdds a per-user order detail screen and route, propagates maker ownership via an Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Home as HomeScreen
participant Provider as OrderBookProvider
participant Router as AppRouter
participant MyOrder as MyOrderScreen
participant API as orders_api
participant Backend as Daemon
User->>Home: Tap order list item
Home->>Provider: read orders
Provider-->>Home: return orders
Home->>Home: find order by id, check isMine
alt isMine == true
Home->>Router: navigate to myOrderPath(id)
Router->>MyOrder: construct(orderId)
MyOrder->>Provider: request order by id
Provider-->>MyOrder: return order (or null)
MyOrder->>User: render details or "Order Not Found"
User->>MyOrder: confirm cancel
MyOrder->>API: cancelOrder(orderId)
API->>Backend: cancel request (uses trade-key index)
Backend-->>API: acknowledge
API-->>MyOrder: success
MyOrder->>Router: navigate home
else not mine
Home->>Router: navigate to buy/sell flow (existing)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
lib/features/order/screens/my_order_screen.dart (2)
33-73: Consider logging the exception for debugging.The caught exception
eis discarded, which can make production debugging difficult. Consider logging it or including it in the error message.🔧 Proposed fix to log the exception
} catch (e) { if (!mounted) return; + debugPrint('Cancel order failed: $e'); ScaffoldMessenger.of(context).showSnackBar( SnackBar(content: Text(l10n.cancelOrderFailed)), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/my_order_screen.dart` around lines 33 - 73, The catch block in _onCancel currently swallows the exception from orders_api.cancelOrder; update the catch to capture the exception and stack trace (e and stackTrace) and log them (e.g., using debugPrint or your app logger) before showing the SnackBar so you retain the existing user-facing behavior; locate the _onCancel method and modify the try/catch around orders_api.cancelOrder to log the error details.
335-340: Consider usingintlpackage for locale-aware date formatting.Manual date formatting doesn't respect the user's locale preferences. Since the app already has localization infrastructure, using
DateFormatfrom theintlpackage would provide consistent locale-aware formatting.♻️ Proposed refactor using intl
+import 'package:intl/intl.dart'; - String _formatDate(DateTime dt) { - return '${dt.year}-${dt.month.toString().padLeft(2, '0')}-' - '${dt.day.toString().padLeft(2, '0')} ' - '${dt.hour.toString().padLeft(2, '0')}:' - '${dt.minute.toString().padLeft(2, '0')}'; - } + String _formatDate(DateTime dt) { + return DateFormat.yMd().add_Hm().format(dt); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/my_order_screen.dart` around lines 335 - 340, The _formatDate(DateTime dt) function currently builds a fixed-format string; replace its implementation to use the intl package's DateFormat to produce locale-aware output (e.g., create a DateFormat with a pattern or use DateFormat.yMd().add_jm() and format(dt)), import 'package:intl/intl.dart', and ensure it uses the app locale (Context locale or Intl.defaultLocale) so callers of _formatDate get localized dates consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/home/providers/home_order_providers.dart`:
- Around line 181-184: filteredOrdersProvider is hiding maker orders on cold
start because order_from_event sets isMine=false for relay-created (Kind 38383)
orders and resolve_maker_order only manages trade-key mappings; fix by restoring
ownership before filtering: either update order_from_event (or the code that
hydrates orders) to set o.isMine=true when a corresponding maker/trade-key
mapping exists (use resolve_maker_order or the same mapping lookup), or change
the filteredOrdersProvider filter to treat relay-origin maker orders as owned by
checking the trade-key/mapping (instead of relying solely on o.isMine) when
evaluating o.status == OrderStatus.pending and o.kind == targetKind so own
orders remain visible after restart.
In `@lib/features/order/screens/my_order_screen.dart`:
- Around line 176-190: Replace the hardcoded English strings used for the
clipboard feedback with localized strings: in the IconButton block that calls
Clipboard.setData(...) and ScaffoldMessenger.of(context).showSnackBar(...), swap
'Order ID copied' and 'Copy order ID' for localized values (e.g.,
AppLocalizations.of(context).orderIdCopied and
AppLocalizations.of(context).copyOrderIdTooltip or the equivalent keys in your
i18n class), ensure you import the localization class and remove the const where
needed so the localized Text and Tooltip receive runtime strings, and keep the
rest of the IconButton (icon, padding, constraints) unchanged.
- Around line 87-92: The hardcoded strings in my_order_screen.dart for the
null-order branch should be replaced with localized lookups and new ARB keys:
add "orderNotFoundTitle" and "orderNotFoundMessage" to your app_en.arb (and
other locales), then update the Scaffold returned in the order == null block to
use your localization accessor (e.g.,
AppLocalizations.of(context).orderNotFoundTitle or
S.of(context).orderNotFoundTitle and the corresponding .orderNotFoundMessage)
instead of the literal 'Order Not Found' and 'This order is no longer
available.' so the screen uses i18n consistently.
---
Nitpick comments:
In `@lib/features/order/screens/my_order_screen.dart`:
- Around line 33-73: The catch block in _onCancel currently swallows the
exception from orders_api.cancelOrder; update the catch to capture the exception
and stack trace (e and stackTrace) and log them (e.g., using debugPrint or your
app logger) before showing the SnackBar so you retain the existing user-facing
behavior; locate the _onCancel method and modify the try/catch around
orders_api.cancelOrder to log the error details.
- Around line 335-340: The _formatDate(DateTime dt) function currently builds a
fixed-format string; replace its implementation to use the intl package's
DateFormat to produce locale-aware output (e.g., create a DateFormat with a
pattern or use DateFormat.yMd().add_jm() and format(dt)), import
'package:intl/intl.dart', and ensure it uses the app locale (Context locale or
Intl.defaultLocale) so callers of _formatDate get localized dates consistently.
🪄 Autofix (Beta)
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
Run ID: 02e29a61-4216-4312-b77e-161a209c5cc1
📒 Files selected for processing (17)
lib/core/app_routes.dartlib/features/home/providers/home_order_providers.dartlib/features/home/screens/home_screen.dartlib/features/home/widgets/order_list_item.dartlib/features/order/screens/my_order_screen.dartlib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartrust/src/api/orders.rs
| IconButton( | ||
| onPressed: () { | ||
| Clipboard.setData(ClipboardData(text: order.id)); | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| const SnackBar( | ||
| content: Text('Order ID copied'), | ||
| duration: Duration(seconds: 1), | ||
| ), | ||
| ); | ||
| }, | ||
| icon: const Icon(Icons.copy, size: 18), | ||
| tooltip: 'Copy order ID', | ||
| padding: EdgeInsets.zero, | ||
| constraints: const BoxConstraints(), | ||
| ), |
There was a problem hiding this comment.
Localize the clipboard snackbar and tooltip.
The "Order ID copied" message and "Copy order ID" tooltip are hardcoded in English while the rest of the screen uses localization.
🌐 Proposed fix to localize clipboard UI strings
IconButton(
onPressed: () {
Clipboard.setData(ClipboardData(text: order.id));
ScaffoldMessenger.of(context).showSnackBar(
- const SnackBar(
- content: Text('Order ID copied'),
- duration: Duration(seconds: 1),
+ SnackBar(
+ content: Text(l10n.orderIdCopied),
+ duration: const Duration(seconds: 1),
),
);
},
icon: const Icon(Icons.copy, size: 18),
- tooltip: 'Copy order ID',
+ tooltip: l10n.copyOrderIdTooltip,
padding: EdgeInsets.zero,
constraints: const BoxConstraints(),
),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/order/screens/my_order_screen.dart` around lines 176 - 190,
Replace the hardcoded English strings used for the clipboard feedback with
localized strings: in the IconButton block that calls Clipboard.setData(...) and
ScaffoldMessenger.of(context).showSnackBar(...), swap 'Order ID copied' and
'Copy order ID' for localized values (e.g.,
AppLocalizations.of(context).orderIdCopied and
AppLocalizations.of(context).copyOrderIdTooltip or the equivalent keys in your
i18n class), ensure you import the localization class and remove the const where
needed so the localized Text and Tooltip receive runtime strings, and keep the
rest of the IconButton (icon, padding, constraints) unchanged.
…strings - Wire order_content_key fingerprint into create_order and subscription loop so maker orders survive a cold restart without needing the daemon UUID - Localize clipboard tooltip, order-not-found texts; use DateFormat for dates - Add exception + stack trace logging to cancel catch block
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/features/order/screens/my_order_screen.dart (2)
338-340: Consider passing explicit locale to DateFormat for consistency with app language.
DateFormat.yMd().add_jm()uses the device's default locale. If the user has set a different language in-app (e.g., app in German but device in English), the date format may not match the app language.♻️ Optional: Use app locale explicitly
- String _formatDate(DateTime dt) { - return DateFormat.yMd().add_jm().format(dt); - } + String _formatDate(BuildContext context, DateTime dt) { + final locale = Localizations.localeOf(context).toString(); + return DateFormat.yMd(locale).add_jm().format(dt); + }Then update the call site at line 157:
- _formatDate(order.createdAt), + _formatDate(context, order.createdAt),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/my_order_screen.dart` around lines 338 - 340, The _formatDate(DateTime dt) helper uses the device default locale; change it to accept an explicit locale string (e.g., _formatDate(DateTime dt, String locale) or Locale) and construct the DateFormat with that locale (DateFormat.yMd(locale).add_jm()). Then update every call site (where _formatDate is used) to pass the app locale (e.g., Localizations.localeOf(context).toString() or the app's chosen locale) so date/time formatting matches the app language.
62-64: Consider using a distinct success message for maker order cancellation.
l10n.cancelRequestSent(translates to "Cancel request sent") appears to be designed for cooperative trade cancellation, where both parties must agree. For maker order cancellation (before a taker joins), the message is slightly misleading since no "request" is involved—the order is cancelled immediately.Consider adding a dedicated string like
orderCancelledSuccessor reusing an existing string that better communicates immediate cancellation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/my_order_screen.dart` around lines 62 - 64, The success SnackBar currently uses l10n.cancelRequestSent which implies a cooperative cancel request; update the SnackBar call in my_order_screen.dart (where ScaffoldMessenger.of(context).showSnackBar is invoked) to use a clearer localization key such as l10n.orderCancelledSuccess (or another existing string that conveys immediate cancellation). Add the new key orderCancelledSuccess to the app's localization resources and translations, and replace l10n.cancelRequestSent with l10n.orderCancelledSuccess in the relevant branch that handles maker order cancellation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/features/order/screens/my_order_screen.dart`:
- Around line 338-340: The _formatDate(DateTime dt) helper uses the device
default locale; change it to accept an explicit locale string (e.g.,
_formatDate(DateTime dt, String locale) or Locale) and construct the DateFormat
with that locale (DateFormat.yMd(locale).add_jm()). Then update every call site
(where _formatDate is used) to pass the app locale (e.g.,
Localizations.localeOf(context).toString() or the app's chosen locale) so
date/time formatting matches the app language.
- Around line 62-64: The success SnackBar currently uses l10n.cancelRequestSent
which implies a cooperative cancel request; update the SnackBar call in
my_order_screen.dart (where ScaffoldMessenger.of(context).showSnackBar is
invoked) to use a clearer localization key such as l10n.orderCancelledSuccess
(or another existing string that conveys immediate cancellation). Add the new
key orderCancelledSuccess to the app's localization resources and translations,
and replace l10n.cancelRequestSent with l10n.orderCancelledSuccess in the
relevant branch that handles maker order cancellation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93ef723f-eb09-461a-b615-434d72d21152
📒 Files selected for processing (13)
lib/features/order/screens/my_order_screen.dartlib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartrust/src/api/orders.rs
✅ Files skipped from review due to trivial changes (1)
- lib/l10n/app_es.arb
🚧 Files skipped from review as they are similar to previous changes (10)
- rust/src/api/orders.rs
- lib/l10n/app_fr.arb
- lib/l10n/app_it.arb
- lib/l10n/app_localizations_de.dart
- lib/l10n/app_localizations_en.dart
- lib/l10n/app_de.arb
- lib/l10n/app_localizations_it.dart
- lib/l10n/app_en.arb
- lib/l10n/app_localizations_fr.dart
- lib/l10n/app_localizations_es.dart
The Mostro node rejected cancels with "Order id not present in database" because cancel_order was sending the locally-generated UUID, which the daemon never knew about. - Add PENDING_LOCAL_IDS map (content_key → local_uuid) populated at create_order time - Add OrderBook::remove_order to evict a stale entry by ID - In subscription loop: when daemon's Kind 38383 arrives and matches via content fingerprint, remove the local UUID entry and add the daemon's UUID entry — cancel_order now always sends the daemon's real UUID
The subscription loop was racing against the daemon's K38383 response: publish_event_json fired, the daemon replied within milliseconds, and the loop called get_trade_key_index before store_trade_key_index had run. Move all bookkeeping (trade key index, content fingerprint, pending maps, order book upsert) to before publish_event_json so the lookup is always populated when the K38383 event arrives.
…essage - _formatDate now accepts an explicit locale string and passes it to DateFormat.yMd(locale) so dates render in the app language - Replace cancelRequestSent (cooperative) with new orderCancelledSuccess key to accurately reflect direct maker cancellation; translated into es, fr, de, it
Summary by CodeRabbit
New Features
Improvements
Localization