feat(us6): phase 8 — buyer trade flow, invoice screens, trade detail - #57
Merged
Conversation
Rust: - mostro/session.rs: per-trade Session struct with SessionManager singleton (create, update, get, remove, stale cleanup) - api/orders.rs: send_invoice(order_id, invoice, sats) with state validation; send_fiat_sent(order_id) with Active state check Dart: - AddLightningInvoiceScreen: invoice text input with IME guards, Cancel/Submit bottom bar, navigates to trade detail on success - NwcInvoiceWidget: auto-generate invoice stub (falls back to manual until NWC is wired in Phase 14) - LnAddressConfirmationWidget: confirm/change LN address card - TradeDetailScreen: 5 info cards (summary, payment, date, order ID, instructions+status), countdown timer with color-coded urgency, buyer action buttons (FIAT SENT, CANCEL, DISPUTE, CONTACT) - TradeInfoCards: reusable TradeInfoCard, OrderIdCard with copy, InstructionsCard with status pill - MostroReactiveButton: spinner→check→error state machine button - app_routes: wired AddLightningInvoiceScreen and TradeDetailScreen
instruction dedup, countdown fix, null safety Rust: - session.rs: manual Debug impl redacting shared_key/admin_shared_key; create_session rejects duplicate order_id; use std::sync::OnceLock for race-free lazy init Dart: - mostro_reactive_button: block re-entry during success/error states (idle-only guard); add optional onError callback; expose error to callers instead of swallowing - trade_detail_screen: consolidate instruction text into _getInstructionText() helper; remove duplicate InstructionsCard; fix countdown off-by-one (check <= 0 before decrement); fix lint ignore syntax - ln_address_confirmation_widget: null-safe bodyLarge fallback - add_lightning_invoice_screen: null-safe bodySmall fallback
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 43 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds Flutter screens/widgets for invoice submission and trade details, new Rust API endpoints for invoice/fiat actions (currently NotImplemented), and a Rust in-memory per-trade SessionManager with session lifecycle and cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant Flutter as Flutter UI
participant API as Rust API (orders)
participant OrderBook as OrderBook
participant SessionMgr as SessionManager
Flutter->>API: send_invoice(orderId, invoice, amount)
API->>OrderBook: load_order(orderId)
alt Order not found
OrderBook-->>API: OrderNotFound
API-->>Flutter: Error (OrderNotFound)
else Order found
API->>OrderBook: validate_state(WaitingBuyerInvoice|Pending)
alt Invalid state
OrderBook-->>API: WrongTradeState
API-->>Flutter: Error (WrongTradeState)
else Valid state
API->>SessionMgr: (optional) get/create session
Note right of API: further action not implemented
API-->>Flutter: Error (NotImplemented)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
lib/shared/widgets/mostro_reactive_button.dart (1)
79-90: Add semantic labels for icon-only transient states.Loading/success/error render icons/spinner only; adding semantics improves screen-reader feedback.
♿ Suggested refinement
case _ButtonState.loading: - return const SizedBox( - width: 20, - height: 20, - child: CircularProgressIndicator(strokeWidth: 2), + return const Semantics( + label: 'Processing', + child: SizedBox( + width: 20, + height: 20, + child: CircularProgressIndicator(strokeWidth: 2), + ), ); case _ButtonState.success: - return const Icon(Icons.check, size: 20); + return const Semantics(label: 'Success', child: Icon(Icons.check, size: 20)); case _ButtonState.error: - return const Icon(Icons.error_outline, size: 20); + return const Semantics(label: 'Error', child: Icon(Icons.error_outline, size: 20));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/shared/widgets/mostro_reactive_button.dart` around lines 79 - 90, The switch on _state (cases _ButtonState.loading, _ButtonState.success, _ButtonState.error) returns icon/spinner-only widgets; wrap each of those returns in a Semantics widget that supplies an appropriate label (e.g., "Loading", "Success", "Error") and marks them as liveRegion/accessible so screen readers announce the transient state; update the build branch that handles _state to return Semantics-wrapped widgets for loading/success/error while leaving _ButtonState.idle unchanged and ensure the semantic role/flags match the original interactive behavior.lib/features/order/screens/add_lightning_invoice_screen.dart (1)
47-47: Verify navigation intent:pushvsgo.Using
context.push()stacks theTradeDetailScreenon top of this screen, so pressing back returns here. If the intent is to replace this screen (since the invoice was successfully submitted), consider usingcontext.go()instead to avoid the user returning to a completed form.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/add_lightning_invoice_screen.dart` at line 47, The current navigation call context.push(AppRoute.tradeDetailPath(widget.orderId)) in AddLightningInvoiceScreen stacks TradeDetailScreen on top of the invoice screen; if you want to replace the current screen so the user can't navigate back to the completed form, change this to context.go(AppRoute.tradeDetailPath(widget.orderId)) (or another replacement API) in the invoice submission success path so the route is replaced rather than pushed.lib/features/trades/screens/trade_detail_screen.dart (1)
159-159: Consider extracting magic number for countdown total.The value
900(15 minutes in seconds) duplicates the initial_remainingduration. Consider defining a constant to keep them in sync.♻️ Suggested refactor
class _TradeDetailScreenState extends ConsumerState<TradeDetailScreen> { + static const _countdownDuration = Duration(minutes: 15); + Timer? _countdownTimer; - Duration _remaining = const Duration(minutes: 15); + Duration _remaining = _countdownDuration; ... child: CircularProgressIndicator( - value: (_remaining.inSeconds / 900).clamp(0.0, 1.0), + value: (_remaining.inSeconds / _countdownDuration.inSeconds).clamp(0.0, 1.0),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/trades/screens/trade_detail_screen.dart` at line 159, Replace the hard-coded 900 with a shared constant so the countdown total stays in sync with _remaining; define a constant like countdownTotalSeconds (or kCountdownTotalSeconds) and use it when initializing _remaining and in the progress calculation value: (_remaining.inSeconds / countdownTotalSeconds).clamp(0.0, 1.0) so both the initial duration and the progress fraction reference the same symbol (look for _remaining and the current value expression to update).
🤖 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/trades/screens/trade_detail_screen.dart`:
- Around line 181-191: Wrap the MostroReactiveButton onPressed async handler in
a try/catch and call the Rust bridge function (send_fiat_sent()) inside the try;
on success update state (setState(() => _status = 'Fiat Sent')) as now, and on
error catch the exception, log it and show user feedback (e.g., a SnackBar or
dialog) similar to AddLightningInvoiceScreen._submit; ensure you still check
mounted before calling setState and rethrow or handle the error appropriately.
In `@lib/features/trades/widgets/trade_info_cards.dart`:
- Around line 40-46: Replace the force-unwrapped style access
theme.textTheme.bodySmall! with a null-safe fallback like
theme.textTheme.bodySmall?.copyWith(...) ??
theme.textTheme.bodyMedium?.copyWith(...) ?? const TextStyle(fontFamily:
'monospace') so the Text widget in trade_info_cards.dart (the Text showing
orderId) never dereferences a null TextStyle; preserve the copyWith(fontFamily:
'monospace') call and follow the same defensive pattern used in
AddLightningInvoiceScreen by chaining ?. and ?? to provide safe fallbacks.
In `@lib/shared/widgets/nwc_invoice_widget.dart`:
- Around line 44-49: The current fallback treats the expected manual path as an
error by setting _error = 'NWC not configured'; instead update the setState in
the fallback branch (around _loading, _error, widget.onFallbackToManual) to
clear any error and treat it as neutral: set _loading = false and _error = null
(or remove assignment), and if you want user-facing context use a neutral
info/status field (e.g., _statusMessage = 'Using manual entry') rather than an
error string; then call widget.onFallbackToManual() as before.
In `@rust/src/api/orders.rs`:
- Around line 322-324: The code currently returns Ok(()) in unimplemented
dispatch branches (the AddInvoice MostroMessage/NIP-59 path and the other branch
around the second Ok(())), which falsely signals success; change these to return
an explicit error instead of Ok: replace the placeholder Ok(()) with a
descriptive Err value (e.g.
Err(ApiError::UnimplementedDispatch("AddInvoice/NIP-59 dispatch not
implemented")) or Err(anyhow!("dispatch path not implemented"))) so callers and
the UI will not be desynced, and keep a TODO comment referencing AddInvoice,
MostroMessage and NIP-59 for future implementation; apply the same replacement
for the other unimplemented branch currently returning Ok(()).
- Around line 310-320: After fetching the order via
order_book().get_order(&order_id), additionally require and validate the active
session and role: retrieve the current session (e.g., session or get_session()),
confirm session.user_id (or equivalent) matches order.buyer_id and that
session.role == Role::Buyer (or Buyer equivalent) before performing the
OrderStatus checks (OrderStatus::WaitingBuyerInvoice / OrderStatus::Pending); if
the session is missing or does not match the buyer role, return an authorization
error (e.g., Err(anyhow::anyhow!("UnauthorizedSession")) or similar) instead of
allowing the order_id alone to proceed.
- Around line 304-308: The code currently ignores the _amount_sats parameter and
allows zero-valued amounts; change the parameter name from _amount_sats to
amount_sats (so it is used) and add an early validation after the
invoice_or_address check that returns Err(anyhow::anyhow!("Amount must be
greater than zero")) if amount_sats == 0; reference the existing symbols
invoice_or_address and amount_sats (formerly _amount_sats) so the function's
input is validated before proceeding.
In `@rust/src/mostro/session.rs`:
- Around line 61-67: Ensure the create_session path validates that the passed
order_id matches order.id before constructing or storing the Session: in the
async function create_session(&self, order_id: String, role: TradeRole,
trade_key_index: u32, order: OrderInfo) -> Result<Session> check equality
(order_id == order.id) and return an Err with a clear message (including both
ids) if they differ; apply the same guard to the other session-creation
overload(s) around the same area (the other create_session variant referenced in
the review) so no session can be created/stored with mismatched IDs.
---
Nitpick comments:
In `@lib/features/order/screens/add_lightning_invoice_screen.dart`:
- Line 47: The current navigation call
context.push(AppRoute.tradeDetailPath(widget.orderId)) in
AddLightningInvoiceScreen stacks TradeDetailScreen on top of the invoice screen;
if you want to replace the current screen so the user can't navigate back to the
completed form, change this to
context.go(AppRoute.tradeDetailPath(widget.orderId)) (or another replacement
API) in the invoice submission success path so the route is replaced rather than
pushed.
In `@lib/features/trades/screens/trade_detail_screen.dart`:
- Line 159: Replace the hard-coded 900 with a shared constant so the countdown
total stays in sync with _remaining; define a constant like
countdownTotalSeconds (or kCountdownTotalSeconds) and use it when initializing
_remaining and in the progress calculation value: (_remaining.inSeconds /
countdownTotalSeconds).clamp(0.0, 1.0) so both the initial duration and the
progress fraction reference the same symbol (look for _remaining and the current
value expression to update).
In `@lib/shared/widgets/mostro_reactive_button.dart`:
- Around line 79-90: The switch on _state (cases _ButtonState.loading,
_ButtonState.success, _ButtonState.error) returns icon/spinner-only widgets;
wrap each of those returns in a Semantics widget that supplies an appropriate
label (e.g., "Loading", "Success", "Error") and marks them as
liveRegion/accessible so screen readers announce the transient state; update the
build branch that handles _state to return Semantics-wrapped widgets for
loading/success/error while leaving _ButtonState.idle unchanged and ensure the
semantic role/flags match the original interactive behavior.
🪄 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: b7061e8b-6a0a-4237-92fc-0465478bb79b
📒 Files selected for processing (11)
lib/core/app_routes.dartlib/features/order/screens/add_lightning_invoice_screen.dartlib/features/trades/screens/trade_detail_screen.dartlib/features/trades/widgets/trade_info_cards.dartlib/shared/widgets/ln_address_confirmation_widget.dartlib/shared/widgets/mostro_reactive_button.dartlib/shared/widgets/nwc_invoice_widget.dartrust/src/api/orders.rsrust/src/mostro/mod.rsrust/src/mostro/session.rsspecs/004-mostro-p2p-client/tasks.md
Comment on lines
+181
to
+191
| MostroReactiveButton( | ||
| label: 'FIAT SENT', | ||
| backgroundColor: green, | ||
| icon: Icons.send, | ||
| onPressed: () async { | ||
| // TODO: Call send_fiat_sent() via Rust bridge. | ||
| await Future.delayed(const Duration(milliseconds: 500)); | ||
| if (mounted) { | ||
| setState(() => _status = 'Fiat Sent'); | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
Missing error handling in FIAT SENT callback.
The async callback lacks try/catch, so any exception from the future Rust bridge call would propagate unhandled. Consider wrapping in try/catch with user feedback, similar to _submit() in AddLightningInvoiceScreen.
🛡️ Proposed fix
MostroReactiveButton(
label: 'FIAT SENT',
backgroundColor: green,
icon: Icons.send,
onPressed: () async {
- // TODO: Call send_fiat_sent() via Rust bridge.
- await Future.delayed(const Duration(milliseconds: 500));
- if (mounted) {
- setState(() => _status = 'Fiat Sent');
+ try {
+ // TODO: Call send_fiat_sent() via Rust bridge.
+ await Future.delayed(const Duration(milliseconds: 500));
+ if (mounted) {
+ setState(() => _status = 'Fiat Sent');
+ }
+ } catch (e) {
+ if (mounted) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text('Error: $e')),
+ );
+ }
}
},
+ onError: (e) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text('Error: $e')),
+ );
+ },
),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/trades/screens/trade_detail_screen.dart` around lines 181 - 191,
Wrap the MostroReactiveButton onPressed async handler in a try/catch and call
the Rust bridge function (send_fiat_sent()) inside the try; on success update
state (setState(() => _status = 'Fiat Sent')) as now, and on error catch the
exception, log it and show user feedback (e.g., a SnackBar or dialog) similar to
AddLightningInvoiceScreen._submit; ensure you still check mounted before calling
setState and rethrow or handle the error appropriately.
Comment on lines
+310
to
+320
| // Validate the order exists and is in the right state. | ||
| let order = order_book() | ||
| .get_order(&order_id) | ||
| .await | ||
| .ok_or_else(|| anyhow::anyhow!("OrderNotFound"))?; | ||
|
|
||
| if order.status != OrderStatus::WaitingBuyerInvoice | ||
| && order.status != OrderStatus::Pending | ||
| { | ||
| return Err(anyhow::anyhow!("WrongTradeState")); | ||
| } |
Contributor
There was a problem hiding this comment.
Gate trade actions by active session + role, not only order_id.
At Line 311 and Line 333, validating against OrderBook alone lets any known order ID reach trade actions. Require a matching session and buyer-role authorization before state checks.
🔧 Proposed fix
+use crate::mostro::session::session_manager;
+use crate::api::types::TradeRole;
...
pub async fn send_invoice(
order_id: String,
invoice_or_address: String,
- _amount_sats: u64,
+ amount_sats: u64,
) -> Result<()> {
+ let session = session_manager()
+ .get_session(&order_id)
+ .await
+ .ok_or_else(|| anyhow::anyhow!("SessionNotFound"))?;
+ if session.role != TradeRole::Buyer {
+ return Err(anyhow::anyhow!("UnauthorizedRole"));
+ }Also applies to: 332-339
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/src/api/orders.rs` around lines 310 - 320, After fetching the order via
order_book().get_order(&order_id), additionally require and validate the active
session and role: retrieve the current session (e.g., session or get_session()),
confirm session.user_id (or equivalent) matches order.buyer_id and that
session.role == Role::Buyer (or Buyer equivalent) before performing the
OrderStatus checks (OrderStatus::WaitingBuyerInvoice / OrderStatus::Pending); if
the session is missing or does not match the buyer role, return an authorization
error (e.g., Err(anyhow::anyhow!("UnauthorizedSession")) or similar) instead of
allowing the order_id alone to proceed.
…tion, null safety, semantics, navigation Rust: - orders.rs: send_invoice validates amount_sats > 0; both send_invoice and send_fiat_sent return NotImplemented error instead of Ok(()) - session.rs: validate order_id matches order.id in create_session Dart: - trade_info_cards: null-safe bodySmall fallback in OrderIdCard - nwc_invoice_widget: treat manual fallback as neutral (not error) - add_lightning_invoice_screen: use context.go instead of push to replace screen after invoice submission; null-safe bodySmall - trade_detail_screen: extract countdown constant _kCountdownSeconds; use in both _remaining init and progress calculation - mostro_reactive_button: add Semantics with liveRegion for loading/success/error states
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/features/trades/screens/trade_detail_screen.dart (1)
185-196:⚠️ Potential issue | 🟡 MinorProvide
onErrorcallback to surface failures to the user.
MostroReactiveButtoninternally catches exceptions and invokeswidget.onError, but without supplying the callback, errors are silently swallowed. Add anonErrorhandler to show user feedback.🛡️ Proposed fix
MostroReactiveButton( label: 'FIAT SENT', backgroundColor: green, icon: Icons.send, onPressed: () async { // TODO: Call send_fiat_sent() via Rust bridge. await Future.delayed(const Duration(milliseconds: 500)); if (mounted) { setState(() => _status = 'Fiat Sent'); } }, + onError: (e) { + if (mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text('Error: $e')), + ); + } + }, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/trades/screens/trade_detail_screen.dart` around lines 185 - 196, The MostroReactiveButton currently omits the onError callback so exceptions inside its internal try/catch are silently dropped; add an onError handler on this MostroReactiveButton that surfaces failures to the user (e.g., call ScaffoldMessenger.of(context).showSnackBar(...) or setState to an error _status) and ensure the onPressed implementation calls the real send_fiat_sent() bridge (or forwards the thrown error) so onError receives and displays the error; locate the MostroReactiveButton instance and add the onError parameter to display a user-facing message when errors occur.
🧹 Nitpick comments (2)
lib/shared/widgets/nwc_invoice_widget.dart (1)
40-40: Avoid artificial wait before guaranteed manual fallback.Since this path is a known placeholder, the 2-second delay adds avoidable friction before users can continue manually.
♻️ Suggested adjustment
- await Future.delayed(const Duration(seconds: 2)); + // Placeholder path: fall back immediately until NWC is wired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/shared/widgets/nwc_invoice_widget.dart` at line 40, The artificial 2-second pause (await Future.delayed(const Duration(seconds: 2));) in the NWC invoice widget should be removed or made conditional so the manual fallback is available immediately; locate the delay in the NwcInvoiceWidget (or the widget/state method that renders the placeholder) and either delete the Future.delayed line or wrap it with a debug-only/feature-flag check (e.g., only await when kDebugMode or a local test flag is true) so production UX has no unnecessary wait and the manual fallback UI is shown immediately.lib/features/trades/screens/trade_detail_screen.dart (1)
198-244: CANCEL and DISPUTE buttons are placeholders with no-op handlers.The
onPressedcallbacks are empty with TODO comments. This is acceptable for Phase 8, but consider disabling the buttons or showing a "Coming soon" message to avoid user confusion when tapped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/trades/screens/trade_detail_screen.dart` around lines 198 - 244, The CANCEL and DISPUTE OutlinedButton.icon instances currently have empty onPressed callbacks (the handlers for the 'CANCEL' and 'DISPUTE' buttons); update their behavior to avoid no-op taps by either setting onPressed to null to disable the buttons or wiring a lightweight UX (e.g., show a SnackBar/toast or modal saying "Coming soon") from the same widget (trade_detail_screen.dart) so taps on the 'CANCEL' and 'DISPUTE' buttons produce a clear result; adjust both OutlinedButton.icon usages and ensure any temporary message uses the existing BuildContext and app messaging utilities rather than leaving TODO handlers.
🤖 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/shared/widgets/nwc_invoice_widget.dart`:
- Around line 49-54: In the catch block inside the NWc invoice widget where you
call setState and assign _error = e.toString(), replace the raw exception string
with a generic, user-friendly message (e.g., "An unexpected error occurred") and
set _loading = false as before; concurrently log the full exception and stack
trace to a developer log (using debugPrint, developer.log, or your app logger)
so technical details are preserved for debugging. Locate the catch handling
around mounted/setState and update only the UI-facing _error value while adding
a separate detailed log call for e and stackTrace.
In `@rust/src/mostro/session.rs`:
- Around line 100-108: In update_session validate that the provided
session.order_id matches the order_id parameter before inserting into the
sessions map: inside update_session (and before sessions.insert) compare
session.order_id (from the Session struct) to the order_id parameter and return
an Err(anyhow!("SessionOrderIdMismatch")) (or similar) if they differ; only
proceed to insert when they are equal to prevent storing a session under a key
that disagrees with its internal order_id.
---
Duplicate comments:
In `@lib/features/trades/screens/trade_detail_screen.dart`:
- Around line 185-196: The MostroReactiveButton currently omits the onError
callback so exceptions inside its internal try/catch are silently dropped; add
an onError handler on this MostroReactiveButton that surfaces failures to the
user (e.g., call ScaffoldMessenger.of(context).showSnackBar(...) or setState to
an error _status) and ensure the onPressed implementation calls the real
send_fiat_sent() bridge (or forwards the thrown error) so onError receives and
displays the error; locate the MostroReactiveButton instance and add the onError
parameter to display a user-facing message when errors occur.
---
Nitpick comments:
In `@lib/features/trades/screens/trade_detail_screen.dart`:
- Around line 198-244: The CANCEL and DISPUTE OutlinedButton.icon instances
currently have empty onPressed callbacks (the handlers for the 'CANCEL' and
'DISPUTE' buttons); update their behavior to avoid no-op taps by either setting
onPressed to null to disable the buttons or wiring a lightweight UX (e.g., show
a SnackBar/toast or modal saying "Coming soon") from the same widget
(trade_detail_screen.dart) so taps on the 'CANCEL' and 'DISPUTE' buttons produce
a clear result; adjust both OutlinedButton.icon usages and ensure any temporary
message uses the existing BuildContext and app messaging utilities rather than
leaving TODO handlers.
In `@lib/shared/widgets/nwc_invoice_widget.dart`:
- Line 40: The artificial 2-second pause (await Future.delayed(const
Duration(seconds: 2));) in the NWC invoice widget should be removed or made
conditional so the manual fallback is available immediately; locate the delay in
the NwcInvoiceWidget (or the widget/state method that renders the placeholder)
and either delete the Future.delayed line or wrap it with a
debug-only/feature-flag check (e.g., only await when kDebugMode or a local test
flag is true) so production UX has no unnecessary wait and the manual fallback
UI is shown immediately.
🪄 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: 0f24d605-9692-4589-8faf-ecde1a04550b
📒 Files selected for processing (7)
lib/features/order/screens/add_lightning_invoice_screen.dartlib/features/trades/screens/trade_detail_screen.dartlib/features/trades/widgets/trade_info_cards.dartlib/shared/widgets/mostro_reactive_button.dartlib/shared/widgets/nwc_invoice_widget.dartrust/src/api/orders.rsrust/src/mostro/session.rs
✅ Files skipped from review due to trivial changes (1)
- lib/features/trades/widgets/trade_info_cards.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- rust/src/api/orders.rs
- lib/shared/widgets/mostro_reactive_button.dart
validation, NWC UX, button feedback Rust: - session.rs: validate session.order_id matches param in update_session Dart: - nwc_invoice_widget: remove artificial 2s delay; use user-friendly error message with debugPrint for developer detail - trade_detail_screen: add onError callback to FIAT SENT button showing snackbar on failure; CANCEL/DISPUTE buttons show "Coming soon" snackbar instead of empty no-op handlers
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/features/trades/screens/trade_detail_screen.dart (1)
32-36: Consider using an enum for status values.The string-based status comparisons (
'Active','Fiat Sent') are fragile and could lead to subtle bugs from typos. When wiring up the Rust bridge provider in Phase 9+, consider defining a type-safe enum:enum TradeStatus { active, fiatSent, completed, cancelled, disputed }This is optional since the current code is placeholder, but flagging for consideration when the real implementation is added.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/trades/screens/trade_detail_screen.dart` around lines 32 - 36, Replace the fragile String-based trade status with a type-safe enum: add a TradeStatus enum (e.g., active, fiatSent, completed, cancelled, disputed), change the field String _status to TradeStatus _status = TradeStatus.active, update any string comparisons/usages in TradeDetailScreen (and related methods/widgets) to use the enum values, and provide a mapping function or extension to convert TradeStatus to the display string used in the UI and to/from the Rust bridge provider when wiring the real implementation.
🤖 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/trades/screens/trade_detail_screen.dart`:
- Around line 32-36: Replace the fragile String-based trade status with a
type-safe enum: add a TradeStatus enum (e.g., active, fiatSent, completed,
cancelled, disputed), change the field String _status to TradeStatus _status =
TradeStatus.active, update any string comparisons/usages in TradeDetailScreen
(and related methods/widgets) to use the enum values, and provide a mapping
function or extension to convert TradeStatus to the display string used in the
UI and to/from the Rust bridge provider when wiring the real implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6eac2024-0284-4c72-a7b4-4baaa4a2170e
📒 Files selected for processing (3)
lib/features/trades/screens/trade_detail_screen.dartlib/shared/widgets/nwc_invoice_widget.dartrust/src/mostro/session.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- rust/src/mostro/session.rs
Add TradeStatus enum (active, fiatSent, completed, cancelled, disputed) with display labels. Replace all string comparisons and assignments in TradeDetailScreen with enum values.
This was referenced Mar 30, 2026
Merged
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rust:
singleton (create, update, get, remove, stale cleanup)
validation; send_fiat_sent(order_id) with Active state check
Dart:
Cancel/Submit bottom bar, navigates to trade detail on success
until NWC is wired in Phase 14)
instructions+status), countdown timer with color-coded urgency,
buyer action buttons (FIAT SENT, CANCEL, DISPUTE, CONTACT)
InstructionsCard with status pill
Summary by CodeRabbit
New Features
Chores