Refactor: flow from waiting to pending status #327
Conversation
- Replace timeout detection for waiting-buyer-invoice/waiting-payment → pending - Handle via direct gift wraps: Action.canceled (taker) / Action.newOrder (maker) - Remove 38383 event monitoring for timeout scenarios only - Eliminate timeout-specific logic: _checkTimeoutAndCleanup, _subscribeToPublicEvents - Preserve other functionality unchanged
- Remove Action.timeoutReversal enum and all related logic - Remove MostroMessage.createTimeoutReversal() factory method - Clean up FSM transitions and notification handling - Remove timeout detection constants from Config
WalkthroughReplaces timestamp/public-event timeout detection with gift-wrap instruction processing. Removes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Mostro
participant App as App (AbstractMostroNotifier)
participant Sess as SessionStore
participant UI as UI / Notifier
rect #E6F5FF
note right of Mostro: Gift-wrap messages drive timeout/cancel flow
Mostro->>App: gift-wrap(Action.newOrder, orderId, payload)
alt Existing session in waiting-buyer-invoice / waiting-payment
App->>UI: emit orderTimeoutMaker notification
App->>Sess: update session -> Pending
UI-->>User: show pending + notification
else No existing session
App->>Sess: store/init session
end
end
rect #FFF2E6
Mostro->>App: gift-wrap(Action.canceled, orderId)
App->>Sess: delete session
App->>UI: emit orderCanceled notification
App->>UI: conditional navigate home
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (1)
145-145: Fix markdown linting issues.Three fenced code blocks (lines 145, 154, 486) are missing language specifiers. While this doesn't affect functionality, it's good practice to specify the language for proper syntax highlighting.
Apply this diff to add language specifiers:
#### **Behavior by Gift Wrap Action** **MAKER (Order Creator) - Receives `Action.newOrder`**: -``` +```text 1. User creates order → Someone takes it → waiting state in My Trades 2. Taker doesn't respond → Mostro sends Action.newOrder gift wrap to maker**TAKER (Order Accepter) - Receives `Action.canceled`**: -``` +```text 1. User takes order → Order appears in My Trades with waiting state 2. User doesn't respond in time → Mostro sends Action.canceled gift wrap to taker### Complete Gift Wrap Flow -``` +```text 1. Order in waiting state (waitingBuyerInvoice or waitingPayment) 2. Timeout occurs on Mostro serverAlso applies to: 154-154, 486-486
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CLAUDE.md(2 hunks)docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md(6 hunks)lib/core/config.dart(0 hunks)lib/data/models/enums/action.dart(1 hunks)lib/data/models/mostro_message.dart(0 hunks)lib/data/models/notification.dart(0 hunks)lib/features/notifications/utils/notification_data_extractor.dart(0 hunks)lib/features/notifications/utils/notification_message_mapper.dart(0 hunks)lib/features/notifications/widgets/notification_item.dart(0 hunks)lib/features/order/models/order_state.dart(0 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(1 hunks)lib/features/order/notfiers/order_notifier.dart(1 hunks)lib/features/trades/widgets/mostro_message_detail_widget.dart(1 hunks)
💤 Files with no reviewable changes (7)
- lib/features/order/models/order_state.dart
- lib/data/models/mostro_message.dart
- lib/features/notifications/utils/notification_data_extractor.dart
- lib/core/config.dart
- lib/data/models/notification.dart
- lib/features/notifications/utils/notification_message_mapper.dart
- lib/features/notifications/widgets/notification_item.dart
🧰 Additional context used
📓 Path-based instructions (5)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dartlib/data/models/enums/action.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dartlib/data/models/enums/action.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Store persistence and API code under lib/data/
Files:
lib/data/models/enums/action.dart
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Store additional project guidance and documentation in docs/
Files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
🧠 Learnings (2)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
PR: MostroP2P/mobile#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<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/order/notfiers/order_notifier.dart
🪛 markdownlint-cli2 (0.18.1)
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
145-145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
486-486: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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 (7)
lib/features/trades/widgets/mostro_message_detail_widget.dart (1)
45-45: LGTM! Simplified UI rendering aligns with timeoutReversal removal.The change simplifies the UI by always displaying the combined status and action, eliminating the previous conditional path for
timeoutReversal. This is consistent with the removal ofAction.timeoutReversalacross the codebase and the shift to gift wrap-based timeout handling.lib/data/models/enums/action.dart (1)
41-41: LGTM! timeoutReversal removal is consistent with gift wrap-based flow.The removal of the
timeoutReversalenum member aligns with the PR's shift from synthetic timeout messages to direct gift wrap instructions (Action.newOrderandAction.canceled). This change is consistent with updates across notification handling, message factories, and UI components.lib/features/order/notfiers/order_notifier.dart (1)
19-24: LGTM! Timeout handling correctly delegated to AbstractMostroNotifier.The simplified implementation correctly delegates timeout and cancellation logic to
AbstractMostroNotifier. The synchronoushandleEventcall (withoutawait) is correct per the established pattern wherehandleEventreturnsvoid, notFuture<void>.Based on learnings.
lib/features/order/notfiers/abstract_mostro_notifier.dart (2)
141-154: LGTM! Maker timeout reactivation correctly implemented.The
newOrderhandling correctly detects timeout reactivation for makers:
- Validates session exists and order is in a waiting state (
waitingBuyerInvoiceorwaitingPayment)- Preserves the session (allows order to return to pending)
- Shows appropriate notification to maker
- Consistent with PR objectives where makers receive
Action.newOrderafter taker timeout
156-180: Verify error handling for session deletion.The cancellation handling correctly deletes sessions and navigates appropriately. However, the
await sessionNotifier.deleteSession(orderId)call at line 164 lacks explicit error handling. If session deletion fails, the user still sees the notification and navigation occurs, potentially leaving orphaned state.Consider wrapping the session deletion in a try-catch block:
// Delete session - this applies to both maker and taker scenarios final sessionNotifier = ref.read(sessionNotifierProvider.notifier); - await sessionNotifier.deleteSession(orderId); + try { + await sessionNotifier.deleteSession(orderId); + } catch (e) { + logger.e('Failed to delete session for canceled order $orderId', error: e); + // Continue with notification/navigation even if deletion fails + } logger.i('Session deleted for canceled order $orderId');Do you want me to verify if
deleteSessionalready has internal error handling that makes this wrapper unnecessary?CLAUDE.md (1)
144-184: LGTM! Documentation accurately reflects gift wrap-based implementation.The updated documentation correctly describes the new gift wrap-based timeout detection:
- Direct instruction processing via encrypted messages (no monitoring/timestamp comparison)
- Maker scenario receives
Action.newOrderwith session preservation- Taker scenario receives
Action.canceledwith session deletion- Simplified logic compared to previous public event monitoring
Documentation is consistent with the implementation in
abstract_mostro_notifier.dart.docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (1)
42-137: LGTM! Comprehensive documentation of gift wrap-based timeout detection.The updated timeout detection section thoroughly documents the new implementation:
- Direct gift wrap instruction processing replaces public event monitoring
- Clear code examples from
AbstractMostroNotifiershowingnewOrderandcanceledhandling- Maker/taker scenarios correctly described with session lifecycle details
- Simplified detection logic well explained
Documentation accurately reflects the implementation changes across the PR.
Catrya
changed the title
|
@coderabbitai review all |
|
@Catrya: I'll review all the changes in this PR. ✅ Actions performedReview triggered.
|
… events - Add selective public event monitoring in OrderNotifier - Detect when Mostro automatically cancels pending orders (pending → canceled) - Delete session to remove expired orders from My Trades - Show cancellation notification to user - Maintain existing gift wrap timeout system unchanged - Only affects maker-created orders that expire automatically
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/order/notfiers/order_notifier.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/order/notfiers/order_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/order/notfiers/order_notifier.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/notfiers/order_notifier.dart
🧠 Learnings (2)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#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/order/notfiers/order_notifier.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
PR: MostroP2P/mobile#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<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/order/notfiers/order_notifier.dart
⏰ 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 (2)
lib/features/order/notfiers/order_notifier.dart (2)
24-29: LGTM: Timeout logic correctly delegated to superclass.The simplified handleEvent implementation correctly delegates timeout and cancellation handling to AbstractMostroNotifier, centralizing the logic as intended by the refactoring.
163-163: Verify localization of notification message.The hardcoded string
'orderCanceled'is passed toshowCustomMessage(). Per coding guidelines, user-facing strings must useS.of(context).keyNamefor localization.Verify that
showCustomMessage()internally handles localization for this key, or if the caller should pass a localized string instead.As per coding guidelines.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/features/order/notfiers/abstract_mostro_notifier.dart(1 hunks)lib/features/order/notfiers/order_notifier.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
🧠 Learnings (2)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dartlib/features/order/notfiers/order_notifier.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
PR: MostroP2P/mobile#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<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/order/notfiers/order_notifier.dart
⏰ 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 (3)
lib/features/order/notfiers/order_notifier.dart (3)
20-20: LGTM!The placement of
_subscribeToPublicEvents()in the constructor is appropriate, ensuring public event monitoring starts when the notifier is initialized.
25-28: LGTM!The delegation to the superclass is correct. The comment accurately reflects that timeout/cancellation logic has been centralized in
AbstractMostroNotifier.
180-183: LGTM!Proper cleanup of the subscription in the dispose method.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/features/order/models/order_state.dart(0 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(1 hunks)
💤 Files with no reviewable changes (1)
- lib/features/order/models/order_state.dart
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
🧠 Learnings (2)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dart
🔇 Additional comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
142-155: LGTM - Maker reactivation handling is well-implemented.The logic correctly identifies when a maker receives order republication after taker timeout by checking for an existing session with
waitingBuyerInvoiceorwaitingPaymentstatus. The notification pattern follows the established two-layer localization architecture.Based on learnings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/features/order/notfiers/abstract_mostro_notifier.dart (2)
60-70: Maker reactivation condition uses post-hydration status; likely never true. Capture previousStatus before update and use it here.Since state is updated with msg before handleEvent, state.status will already be pending on Action.newOrder, so the waitingBuyerInvoice/waitingPayment check won’t match. Also tighten the timestamp gate and ensure session is set.
Apply:
@@ - if (mounted) { - state = state.updateWith(msg); - } + // capture pre-hydration status for side-effect decisions + final previousStatus = state.status; + if (mounted) { + state = state.updateWith(msg); + } @@ - logger.i('Message timestamp check passed, calling handleEvent for ${msg.action}'); - unawaited(handleEvent(msg)); + logger.i('Message timestamp check passed, calling handleEvent for ${msg.action}'); + unawaited(handleEvent(msg, previousStatus: previousStatus)); @@ - Future<void> handleEvent(MostroMessage event, {bool bypassTimestampGate = false}) async { + Future<void> handleEvent(MostroMessage event, {bool bypassTimestampGate = false, Status? previousStatus}) async { @@ case Action.newOrder: - // Check if Mostro is republishing the order after timeout + // Check if Mostro is republishing the order after timeout final currentSession = ref.read(sessionProvider(orderId)); - if (currentSession != null && - (state.status == Status.waitingBuyerInvoice || state.status == Status.waitingPayment)) { + if (currentSession != null && + (previousStatus == Status.waitingBuyerInvoice || previousStatus == Status.waitingPayment)) { + // keep local session in sync + session = currentSession; // This is a maker receiving order republication after taker timeout logger.i('MAKER: Received order reactivation from Mostro - taker timed out, order returned to pending'); // Show notification: counterpart didn't respond, order will be republished - if (isRecent || !bypassTimestampGate) { + if (isRecent && !bypassTimestampGate) { final notifProvider = ref.read(notificationActionsProvider.notifier); notifProvider.showCustomMessage('orderTimeoutMaker'); } } break;This ensures the branch triggers correctly and side effects remain gated to recent events. Based on learnings.
Also applies to: 105-105, 142-155
20-21: Uninitializedlate Session sessionfield creates LateInitializationError risk at line 124.Constructor initialization (line 41) is conditional: if
getSessionByOrderId(orderId)returns null, thesessionfield is never set. WhenhandleEventlater callsextractFromMostroMessage(event, ref, session: session)at line 124, accessing the uninitialized late field crashes.Since the extractor accepts nullable
Session?, changelate Session session;toSession? session;and initialize it to null, then pass it safely to the extractor.
🧹 Nitpick comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
123-136: Tighten notification gating to match intent (“Only notify for recent events”).Current condition (isRecent || !bypassTimestampGate) allows notifications whenever the gate isn’t bypassed. Make it strictly recent + not bypassed.
- if (notificationData != null && (isRecent || !bypassTimestampGate)) { + if (notificationData != null && isRecent && !bypassTimestampGate) { sendNotification( notificationData.action, values: notificationData.values, isTemporary: notificationData.isTemporary, eventId: notificationData.eventId, ); - } else if (notificationData != null && bypassTimestampGate) { + } else if (notificationData != null && bypassTimestampGate) { logger.i('Skipping notification for old event: ${event.action} (timestamp: ${event.timestamp})'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
🧠 Learnings (3)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-10-01T14:10:17.742Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-01T14:10:17.742Z
Learning: Applies to lib/**/*.dart : Check `mounted` before using BuildContext after async gaps
Applied to files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
🔇 Additional comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
157-184: Verified: no double-notification risk. Extractor returns null for Action.canceled.The NotificationDataExtractor explicitly returns null for Action.canceled with the comment "Canceled orders don't generate persistent notifications," so the manual
showCustomMessage('orderCanceled')call at line 175 is the only notification path and poses no duplication risk. The implementation is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
UseS.of(context).keyNamefor all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (useS.of(context))
Checkmountedbefore using BuildContext after async gaps
Useconstconstructors where possibleName Riverpod providers as Provider or Notifier
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: All code comments must be in English
Use clear, concise English for variable and function names
Maintain zeroflutter analyzeissues
Use latest Flutter/Dart APIs (e.g., preferwithValues()overwithOpacity())
Remove unused imports and dependencies
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/notfiers/abstract_mostro_notifier.dart
🧠 Learnings (2)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
PR: MostroP2P/mobile#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/order/notfiers/abstract_mostro_notifier.dart
🔇 Additional comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
141-155: LGTM! Maker reactivation logic correctly implemented.The detection of order republication after taker timeout is sound. The session and status checks properly identify the maker scenario, and the notification uses the correct two-layer localization pattern established in the codebase.
close #326
Summary by CodeRabbit
New Features
Notifications
UI
Documentation