Removed hardcoded 24h order expiration and used expiration_hours for trade messages and new orders #392
Conversation
…ng expiration hours of kind 38383 sent from mostro
2. updated arb files to have {expiration_hours} place holder for "newOrder" and "yourCeatedOfferMessage", keys
WalkthroughFetches dynamic expirationHours from the Mostro instance via the order repository provider and surfaces it in UI strings. Localization signatures and ARB metadata were updated to accept the expiration_hours placeholder; a single backup reminder string wording was adjusted. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Screen (Order / Trade / Widget)
participant Provider as orderRepositoryProvider
participant Mostro as MostroInstance
participant I18n as Localization (S)
UI->>Provider: read(orderRepositoryProvider)
Provider->>Mostro: access mostroInstance
Mostro-->>Provider: expirationHours (int or null)
Provider-->>UI: expirationHours (default 24 if null)
UI->>I18n: youCreatedOfferMessage(expirationHours.toString()) / newOrder(expirationHours.toString())
I18n-->>UI: localized string with {expiration_hours}
UI-->>User: render message with dynamic expiration
Note over UI,Mostro: small defaulting logic if expiration missing\n(expirationHours ?? 24)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/features/trades/screens/trade_detail_screen.dart (1)
987-1011: Pending‑status countdown now based on exact expires_at timestampFor
Status.pending,_CountdownWidgetnow:
- Rejects missing/invalid
expiresAtTimestamp.- Builds
expirationfromexpiresAtTimestamp.- Passes
expirationandcreatedAtintoDynamicCountdownWidget.This switches from derived durations to protocol‑accurate timestamps, which matches the earlier guidance to use
order_expires_atfor countdown precision. Based on learnings, this is the desired behavior.If
tradeState.order?.createdAtcan ever benullor0, consider falling back to a more meaningful value (e.g.,expiration - Duration(hours: expirationHours)usingmostroInstance.expirationHours) instead of epoch to avoid odd timelines in the UI.lib/features/order/screens/order_confirmation_screen.dart (1)
1-9: Remove unused MostroInstance import and consider localizing app bar title
mostro_instance.dartis imported but not referenced in this file;order_repository_provider.dartalready exposes the instance. You can safely drop the unused import to avoid analyzer warnings. Also,OrderAppBar(title: 'Order Confirmed')is still hardcoded; consider moving this to a localized key for consistency with the rest of the app.After removing the import, re-run
flutter analyzeto confirm there are no remaining unused‑import warnings, and, if you add a key for the title, regenerate l10n.lib/features/trades/widgets/mostro_message_detail_widget.dart (1)
61-66: Mostro “new order” message now respects instance expirationHoursThe
newOrderaction text now pullsexpirationHoursfromorderRepositoryProvider.mostroInstanceand passes it as a String intoS.of(context)!.newOrder(...), which matches the updated ARB placeholder. That removes the hidden 24h assumption in Mostro’s own message.To keep types consistent, consider using a numeric fallback instead of a String:
- final expHrs = - ref.read(orderRepositoryProvider).mostroInstance?.expirationHours ?? - '24'; - return S.of(context)!.newOrder(expHrs.toString()); + final expHrs = + ref.read(orderRepositoryProvider).mostroInstance?.expirationHours ?? + 24; + return S.of(context)!.newOrder(expHrs.toString());After this adjustment, re-run
flutter analyzeto confirm there are no type inference regressions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
lib/features/order/screens/order_confirmation_screen.dartlib/features/trades/screens/trade_detail_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dartlib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
🧰 Additional context used
📓 Path-based instructions (4)
lib/l10n/**/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it) - use
S.of(context)!.keyNamefor all user-facing strings
Files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/order/screens/order_confirmation_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/order/screens/order_confirmation_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
lib/features/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/**/screens/**/*.dart: Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
UseS.of(context)!.yourKeyfor all user-facing strings instead of hardcoded text
Files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/order/screens/order_confirmation_screen.dart
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Use exact `order_expires_at` timestamps from Mostro protocol for DynamicCountdownWidget precision and localize display with `S.of(context)!.timeLeftLabel()`
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/l10n/**/*.arb : Add new localization keys to all three ARB files (en, es, it) - use `S.of(context)!.keyName` for all user-facing strings
Applied to files:
lib/l10n/intl_it.arblib/l10n/intl_es.arblib/l10n/intl_en.arb
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Use exact `order_expires_at` timestamps from Mostro protocol for DynamicCountdownWidget precision and localize display with `S.of(context)!.timeLeftLabel()`
Applied to files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/order/screens/order_confirmation_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.
Applied to files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/order/screens/order_confirmation_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Use `S.of(context)!.yourKey` for all user-facing strings instead of hardcoded text
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Use `startSessionTimeoutCleanupForRequestId()` for order creation timeout protection and cancel timer automatically when any Mostro response received
Applied to files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/order/screens/order_confirmation_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/orders/notifiers/add_order_notifier.dart : Start session timeout cleanup in `submitOrder()` method to prevent orphan sessions when Mostro doesn't respond within 10 seconds
Applied to files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-05-08T16:06:33.665Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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<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.
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Start 10-second cleanup timer automatically when taking orders via `startSessionTimeoutCleanup()` to prevent orphan sessions
Applied to files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/mostro_fsm.dart : Use MostroFSM for managing order state transitions - all state changes must go through FSM state methods
Applied to files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
lib/features/trades/screens/trade_detail_screen.dartlib/features/order/screens/order_confirmation_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Always check `mounted` before using BuildContext after async operations to prevent errors on disposed widgets
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Implement DynamicCountdownWidget with automatic day/hour scaling: day scale (>24h) shows 'd h m' format, hour scale (≤24h) shows HH:MM:SS format
Applied to files:
lib/features/order/screens/order_confirmation_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/generated/l10n*.dart : Never manually edit generated localization files - regenerate with `dart run build_runner build -d` after modifying ARB files
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Localize all user-facing strings via ARB files and access them with `S.of(context)` rather than hard-coded literals
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Name Riverpod providers using the `<Feature>Provider` or `<Feature>Notifier` convention
Applied to files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
🔇 Additional comments (9)
lib/l10n/intl_it.arb (1)
4-11: Expiration placeholders correctly defined for Italian locale
newOrderandyouCreatedOfferMessagenow expose{expiration_hours}with matching@...placeholder metadata of typeString, which aligns with the updated Dart calls passingexpirationHours.toString(). No issues spotted.Also applies to: 664-671
lib/l10n/intl_en.arb (2)
5-12: Localization metadata matches new dynamic expiration usage
@newOrderand@youCreatedOfferMessagecorrectly declare anexpiration_hoursplaceholder of typeStringand are wired into the English strings via{expiration_hours}. This matches the updatedS.of(context)!.newOrder(expirationHours.toString())/youCreatedOfferMessage(...)calls in Dart and should work once l10n is regenerated.Please ensure you rerun localization generation (e.g.,
flutter gen-l10nor your existing task) andflutter testso the generatedSAPI matches these ARB changes.Also applies to: 274-282
1235-1237: Backup reminder wording tweak is safeChanging
backupAccountReminderMessageto "Save your secret words to recover your account" is a purely textual improvement and keeps the original meaning, with no parameter/placeholder changes.lib/l10n/intl_es.arb (1)
5-11: Spanish expiration_hours placeholders aligned with codeBoth
newOrderandyouCreatedOfferMessagenow expose{expiration_hours}with matching@...placeholder entries of typeString. This mirrors the other locales and matches the Dart calls passingexpirationHours.toString(). Looks good.Also applies to: 621-628
lib/features/trades/screens/trade_detail_screen.dart (3)
108-110: Dynamic expiration hours correctly wired into creator messageUsing
mostroInstance?.expirationHours ?? 24and passingexpirationHours.toString()intoyouCreatedOfferMessage(...)removes the hardcoded 24h while keeping a sensible fallback. This matches the new ARB placeholder type (String) and fulfills the PR objective for trade detail creator messaging.Please run
flutter analyzeandflutter testto confirm there are no type or localization signature mismatches after this change.Also applies to: 154-161
790-804: VIEW DISPUTE button behavior looks correct and null‑safeThe extra
VIEW DISPUTEbutton is now shown whenever the action is one ofdisputeInitiatedByYou,disputeInitiatedByPeer, oradminTookDisputeandtradeState.dispute?.disputeIdis non‑null. The null check before usingtradeState.dispute!.disputeIdavoids NPEs and cleanly separates this from the FSM action button list.
722-751: Dispute creation error messaging now fully localized (with detail)The dispute creation flow now:
- Uses
disputeCreationErrorWithMessage(error)when an exception is thrown.- Shows success/failure SnackBars only when
context.mounted.This aligns with the new ARB keys and follows the guideline to check
mountedbefore usingBuildContextafter async work.Since this depends on the new
disputeCreationErrorWithMessagelocalization, confirm that all three ARB files define this key and re-run your l10n generation soS.of(context)exposes it with aString errorparameter.lib/features/order/screens/order_confirmation_screen.dart (1)
18-31: Order confirmation now uses Mostro’s expirationHours instead of hardcoded 24Reading
mostroInstancefromorderRepositoryProviderand passingexpirationHours.toString()intoS.of(context)!.newOrder(...)removes the fixed 24‑hour wording and aligns the confirmation message with the Mostro instance configuration, with a safe default of 24h when the instance isn’t yet available.Please run
flutter analyze/flutter testto ensure the generatedS.newOrdersignature (String parameter) matches and no lints are introduced.lib/features/trades/widgets/mostro_message_detail_widget.dart (1)
225-231: paymentFailed interval calculation remains correct
intervalInMinutesis still derived by dividingpaymentRetriesInterval(seconds) by 60 and rounding, which matches the ARB expectation of minutes for{payment_retries_interval}. Only formatting changed here.Because this parameter is fed into a localized message, ensure the ARB definition for
paymentFailedin all locales still documents this as minutes and matches the Dart argument type.
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
Hi @kufre19 good job! I tested it and it works well, but please implement the suggestions that coderabbit mentions, specifically this:
- mostro_message_detail_widget.dart: Use ?? 24 instead of ?? '24' for type consistency
This is minor cleanup items that will improve code quality. Thanks!
|
hey hello @Catrya thanks for pointing that out, i've fixed that now, |
3 participants
This PR is for the code changes fixing the issue that was raised here #388
Summary by CodeRabbit
New Features
Localization
Style
✏️ Tip: You can customize this high-level summary in your review settings.