feat: add basic support for PaymentFailure and NextTrade payload types #241
Conversation
WalkthroughNew Dart model classes Changes
Sequence Diagram(s)sequenceDiagram
participant EventSource
participant Notifier
participant OrderState
participant UI
EventSource->>Notifier: Receives event with payment_failed payload
Notifier->>OrderState: Updates state with PaymentFailed instance
OrderState->>UI: Provides paymentFailed data via provider
UI->>UI: Renders details using paymentAttempts and paymentRetriesInterval
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/order/models/order_state.dart (1)
4-4: Consider using the barrel export instead of direct import.Since
PaymentFailedshould be exported throughdata/models.dart(line 2), you can remove this direct import if the model is properly exported in the barrel file.-import 'package:mostro_mobile/data/models/payment_failed.dart';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/data/models/next_trade.dart(1 hunks)lib/data/models/payload.dart(2 hunks)lib/data/models/payment_failed.dart(1 hunks)lib/features/order/models/order_state.dart(9 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(2 hunks)lib/features/trades/widgets/mostro_message_detail_widget.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/next_trade.dartlib/data/models/payment_failed.dartlib/features/order/models/order_state.dartlib/data/models/payload.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/next_trade.dartlib/data/models/payment_failed.dartlib/features/order/models/order_state.dartlib/data/models/payload.dart
🧠 Learnings (7)
📓 Common learnings
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through `data/models.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
lib/features/trades/widgets/mostro_message_detail_widget.dart (2)
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart
lib/features/order/notfiers/abstract_mostro_notifier.dart (16)
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, eventStorageProvider is exported from package:mostro_mobile/shared/providers/mostro_service_provider.dart and not from a separate event_storage_provider.dart file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom timeAgoWithLocale() method is implemented in the NostrEvent extension
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through data/models.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/repositories/**/*.dart : All data access should go through repository classes in data/repositories/
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//providers/**/.dart : Providers are organized by feature in features/{feature}/providers/
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/background/**/*.dart : Background services are implemented in the background/ directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with @Riverpod annotations. Providers like eventStorageProvider are generated in .g.dart files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., mostro_service_provider.dart), not by importing a separate provider file.
lib/data/models/next_trade.dart (3)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through data/models.dart
Learnt from: chebizarro
PR: #74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the TradeState class is specifically constructed using the tradeStateProvider. While some fields are nullable (lastAction and orderPayload), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
lib/data/models/payment_failed.dart (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through data/models.dart
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
lib/features/order/models/order_state.dart (6)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart
Learnt from: chebizarro
PR: #74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the TradeState class is specifically constructed using the tradeStateProvider. While some fields are nullable (lastAction and orderPayload), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: chebizarro
PR: #110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via tradeState.peer?.publicKey from the OrderState instance rather than through session?.peer?.publicKey, as the OrderState encapsulates peer information directly.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through data/models.dart
Learnt from: chebizarro
PR: #110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
lib/data/models/payload.dart (10)
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through data/models.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/repositories/**/*.dart : All data access should go through repository classes in data/repositories/
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, eventStorageProvider is exported from package:mostro_mobile/shared/providers/mostro_service_provider.dart and not from a separate event_storage_provider.dart file.
Learnt from: chebizarro
PR: #110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (16)
lib/data/models/payment_failed.dart (1)
1-31: LGTM! Clean implementation following established patterns.The
PaymentFailedmodel correctly implements thePayloadinterface with proper JSON serialization/deserialization. The implementation follows the established pattern used by other payload models in the codebase.Key strengths:
- Proper field types (
intfor both fields)- Consistent JSON key naming using snake_case
- Standard toJson structure with type as key and nested field map
- Appropriate factory constructor for deserialization
lib/features/order/notfiers/abstract_mostro_notifier.dart (2)
5-5: Proper import added for new PaymentFailed model.The import follows the established pattern and is necessary for the payload handling changes below.
216-222: Excellent improvement: replaced hardcoded values with dynamic payload data.The change properly extracts
paymentAttemptsandpaymentRetriesIntervalfrom thePaymentFailedpayload instead of using static placeholders. This follows the established pattern used for other payload types in the same method and provides accurate information to users.The null-safe access with
?.operator is appropriate for handling cases where the payload might not be present.lib/features/trades/widgets/mostro_message_detail_widget.dart (1)
186-191: Proper integration of dynamic PaymentFailed data.The widget now correctly fetches the actual
paymentAttemptsandpaymentRetriesIntervalvalues from thepaymentFailedpayload via theorderNotifierProvider, replacing the previous static placeholders.The null-safe access with appropriate fallback values (0) ensures the UI remains functional even when the payload is not available. This change aligns with the broader integration of the
PaymentFailedmodel throughout the codebase.lib/data/models/payload.dart (2)
3-5: Proper imports added for new payload models.The imports for
NextTradeandPaymentFailedare correctly added and necessary for the factory method extensions below.
27-30: Factory method properly extended for new payload types.The
Payload.fromJsonfactory correctly handles the newpayment_failedandnext_tradepayload types, following the established pattern used for other payload types. The JSON key extraction and model instantiation are implemented consistently.This maintains backward compatibility while enabling deserialization of the new payload types introduced in the Mostro protocol.
lib/data/models/next_trade.dart (1)
1-26: Well-implemented NextTrade model following established patterns.The
NextTrademodel correctly implements thePayloadinterface with proper:
- Field types (
Stringfor key,intfor index)- Required constructor parameters
- JSON serialization with nested structure
- Factory constructor with safe type casting
- Consistent naming convention
The implementation follows the same pattern as other payload models in the codebase.
lib/features/order/models/order_state.dart (9)
14-14: LGTM!The field declaration follows the established pattern for nullable payload fields in the class.
25-25: LGTM!The constructor parameter follows the established pattern for optional payload parameters in the OrderState class.
37-37: LGTM!The payload extraction follows the established pattern used for other payload types in the factory method.
43-43: LGTM!The toString method correctly includes the new paymentFailed field, maintaining consistency with the existing implementation.
55-55: LGTM!The equality operator correctly includes the paymentFailed field in the comparison, ensuring proper equality semantics.
67-67: LGTM!The hashCode implementation correctly includes the paymentFailed field, maintaining the equals/hashCode contract.
78-78: LGTM!The copyWith parameter follows the established pattern for optional nullable parameters.
88-88: LGTM!The copyWith implementation correctly uses the null-aware operator pattern to preserve or update the paymentFailed field.
146-146: LGTM!The updateWith method correctly handles the PaymentFailed payload extraction and preservation, following the established pattern used for other payload types.
| import 'package:mostro_mobile/data/models/payload.dart'; | ||
|
|
||
| class NextTrade implements Payload { | ||
| final String key; | ||
| final int index; | ||
|
|
||
| NextTrade({required this.key, required this.index}); | ||
|
|
||
| @override | ||
| String get type => 'next_trade'; | ||
|
|
||
| @override | ||
| Map<String, dynamic> toJson() { | ||
| return { | ||
| type: {'key': key, 'index': index}, | ||
| }; | ||
| } | ||
|
|
||
| factory NextTrade.fromJson(Map<String, dynamic> json) { | ||
| return NextTrade( | ||
| key: json['key'] as String, | ||
| index: json['index'] as int, | ||
| ); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify model export through data/models.dart.
Based on the coding guidelines, models should be exported through data/models.dart. Please ensure this new NextTrade model is added to the exports in that file.
🏁 Script executed:
#!/bin/bash
# Check if NextTrade is exported through data/models.dart
echo "Checking if NextTrade model is exported through data/models.dart..."
rg "NextTrade" lib/data/models.dart || echo "NextTrade not found in models.dart exports"
# Also check if PaymentFailed is exported
echo "Checking if PaymentFailed model is exported through data/models.dart..."
rg "PaymentFailed" lib/data/models.dart || echo "PaymentFailed not found in models.dart exports"Length of output: 571
Export the NextTrade model in data/models.dart
Please add the new NextTrade model to your central exports so it’s discoverable throughout the app. In lib/data/models.dart, include:
export 'models/payload.dart';
+export 'models/next_trade.dart';
// …other exportsThis aligns with our guideline to re-export all models from data/models.dart.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import 'package:mostro_mobile/data/models/payload.dart'; | |
| class NextTrade implements Payload { | |
| final String key; | |
| final int index; | |
| NextTrade({required this.key, required this.index}); | |
| @override | |
| String get type => 'next_trade'; | |
| @override | |
| Map<String, dynamic> toJson() { | |
| return { | |
| type: {'key': key, 'index': index}, | |
| }; | |
| } | |
| factory NextTrade.fromJson(Map<String, dynamic> json) { | |
| return NextTrade( | |
| key: json['key'] as String, | |
| index: json['index'] as int, | |
| ); | |
| } | |
| } | |
| // lib/data/models.dart | |
| export 'models/payload.dart'; | |
| export 'models/next_trade.dart'; | |
| // …other exports |
🤖 Prompt for AI Agents
In lib/data/models/next_trade.dart lines 1 to 26, the NextTrade model is defined
but not exported centrally. To fix this, open lib/data/models.dart and add an
export statement for next_trade.dart, such as "export 'models/next_trade.dart';"
to ensure the NextTrade model is discoverable and accessible throughout the app
as per project guidelines.
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
Hi @chebizarro
It works, but when the payment fails, the order appears in "My Trades" screen with the status "waiting-invoice" and the same screen as when the user started the exchange, even with the 15-minute clock ticking. This shouldn't be the case. Instead, it should:
- Order label in My Trades screen must be
payment-failed(sorry, i know is a new one but we hadn't considered it before, I think it's the most descriptive possible, because it's not Success, but if we put Waiting-invoice it can confuse the user thinking it's a recently new order.) - In Order Details screen it must stays on the last screen (where the payment-failed message appears).
- A button appears next to the Close button, saying "Add Invoice."
- When the "Add Invoice" button is selected, the usual "Add Invoice" screen appears.
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
@chebizarro
There are two parts:
1- When the first payment fails and Mostro sends the payment-failed message (it's still trying to pay more times but isn't asking for a new invoice).
2- When all payment attempts have failed and Mostro sends the add-invoice action, but with the status settled-hold-invoice.
- In the first part, only the payment failure message and the close button should appear. The user must wait for Mostro to complete the payment or request a new invoice. The Dispute, Add Invoice, and Cancel buttons should not appear.
Also, mostro sendspayment_retries_intervalin seconds, so it must be converted to minutes to show the user the time in minutes (in this case, payment_retries_interval=60 sec, but it shows 60 minutes because it hasn't been converted).
- In the second part, mostro sends the addinvoice message. In My Trades screen, the
payment-failedstatus should remain. In Order details screen, the same screen should appear as above, but with only the Close and Add Invoice buttons, but not with the clock or 15 min message
|
Hi @chebizarro
|
…trict available actions
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (2)
8-33: Add the core feature of this PR to the changelogThe
Addedlist omits the headline feature delivered by this PR—support for the newPaymentFailureandNextTradepayloads plus the related state/UI work—making the entry incomplete and potentially confusing for users scanning release notes.### Added ... - Debug-only display mode for Current Trade Index Card (hidden in release builds) +- Support for new Mostro protocol payloads **PaymentFailure** and **NextTrade** +- Order/notification/UI updates that show retry attempts and new status +- Back-compat deserialization in `Payload.fromJson` for these types
11-14: Tighten phrasing for clarity
- Line 12: “during relay validation and testing process” → “during the relay validation and testing process”.
- Line 14: “hidden in release builds” → “hidden in production builds” for consistency with other docs.
Pure wording tweaks—no functional impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)lib/features/key_manager/key_management_screen.dart(2 hunks)lib/features/relays/widgets/relay_selector.dart(2 hunks)lib/features/settings/about_screen.dart(1 hunks)lib/features/settings/settings_screen.dart(1 hunks)lib/features/trades/screens/trade_detail_screen.dart(2 hunks)pubspec.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/trades/screens/trade_detail_screen.dart
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/relays/widgets/relay_selector.dartlib/features/settings/settings_screen.dartlib/features/settings/about_screen.dartlib/features/key_manager/key_management_screen.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/relays/widgets/relay_selector.dartlib/features/settings/settings_screen.dartlib/features/settings/about_screen.dartlib/features/key_manager/key_management_screen.dart
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by `core/mostro_fsm.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through `data/models.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
lib/features/settings/settings_screen.dart (1)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
lib/features/settings/about_screen.dart (1)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
CHANGELOG.md (1)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
lib/features/key_manager/key_management_screen.dart (3)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/**/*.dart : Use S.of(context).yourKey for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
lib/features/settings/about_screen.dart (1)
46-51: LGTM! Safe area padding correctly implemented.The dynamic bottom padding using
MediaQuery.of(context).viewPadding.bottomensures content is not obscured by system UI elements like gesture navigation bars. This follows Flutter best practices for responsive design across different device form factors.lib/features/settings/settings_screen.dart (1)
46-51: LGTM! Consistent safe area handling.The padding adjustment matches the pattern used in other screens, ensuring consistent layout behavior across the app with proper safe area handling.
lib/features/relays/widgets/relay_selector.dart (2)
104-144: LGTM! Dialog layout improvements enhance consistency.Removing unnecessary
Flexiblewidgets and increasing button spacing from 8 to 12 pixels improves the visual layout of the add relay dialog. The button functionality remains unchanged.
188-228: LGTM! Consistent dialog styling applied.The edit relay dialog now matches the improved layout from the add dialog, with cleaner button structure and better spacing.
lib/features/key_manager/key_management_screen.dart (2)
142-147: LGTM! Safe area padding consistently applied.The padding adjustment follows the same pattern used in other screens for proper safe area handling.
651-685: LGTM! Dialog layout improvements match app-wide consistency.The dialog action buttons now follow the improved layout pattern with removed
Flexiblewidgets and increased spacing, consistent with similar changes in other dialogs throughout the app.
|
@chebizarro Están los botones intercambiados, cuando está en Cuando está en |
- Add separate message for payment-failed add-invoice state - Update paymentFailed message with clearer retry information
|
Hi @chebizarro I made some changes to the texts to improve the UX. I've already tested everything and it works fine. |
This PR adds basic support for the PaymentFailure and NextTrade payload types recently added to the mostro protocol
Users notifications and the Order Details views will now display the number of retry attempts correctly in the event of a PaymentFailure
These changes should be backwards compatible with earlier mostrod versions
Summary by CodeRabbit
New Features
Improvements