Skip to content

Conversation

@grunch
Copy link
Member

@grunch grunch commented Jul 20, 2025

Fix #79

  1. Full Message Decryption Integration
  • ✅ Added Riverpod integration to access session management
  • ✅ Implemented complete message decryption using existing unWrap method
  • ✅ Added session matching logic to find correct trade keys for decryption
  • ✅ Integrated with MostroMessage parsing for action extraction
  1. Action-Based Notification System
  • ✅ Created comprehensive action-to-title mapping with user-friendly titles:
    • "Order Published", "Payment Required", "Bitcoin Released", etc.
  • ✅ Implemented protocol-compliant message content following Mostro suggestions:
    • "Your offer has been published! Please wait until another user picks your order."
    • "Please pay the hold invoice of 1000 Sats"
    • "Bitcoin has been released! Your invoice should be paid shortly."
  1. Enhanced Payload & Deep Linking
  • ✅ Rich notification payload with action context: {
    "eventId": "abc123", "kind": "1059", "orderId": "order_456", "action": "release" }
  1. Robust Error Handling
  • ✅ Graceful fallback to generic messages when:
    • Session not found for decryption
    • Message decryption fails
    • MostroMessage parsing errors
  • ✅ Comprehensive logging for debugging
  1. Background Service Integration
  • ✅ Created showSimpleNotification() for background isolates without Riverpod
  • ✅ Updated both desktop and mobile background services
  • ✅ Maintained functionality when app is in background

🔧 Key Features Implemented:

Smart Session Matching

// Finds the correct trade key by matching event target pubkey Session? _findSessionForEvent(NostrEvent event, List sessions)

Complete Message Decryption Flow

// 1. Find session with matching trade key
// 2. Decrypt using event.unWrap(session.tradeKey.private) // 3. Parse MostroMessage from decrypted JSON
// 4. Extract action and payload for notification content

Dynamic Content Generation

  • Payment Actions: Extract amounts from PaymentRequest/Order payloads
  • Error Actions: Extract specific error reasons from CantDo payloads
  • Status Updates: Context-aware messages for all trade states

📱 User Experience Improvements:

Before:

  • Generic "New Mostro Event" for all notifications
  • No context about what action occurred
  • No payload for deep linking

After:

  • Action-specific titles: "Payment Required", "Bitcoin Released"
  • Rich context messages: "Please pay the hold invoice of 1000 Sats"
  • Deep linking ready: Full payload with action and order details
  • Graceful degradation: Falls back to generic message if decryption fails

🔒 Security & Privacy:

  • Only kind 1059 events: Filters out all other event types
  • Session-based decryption: Uses correct trade keys for each order
  • No key exposure: Leverages existing secure key management
  • Error resilience: Never exposes sensitive data in error states

Added New Localization Keys to ARB Files

  • English (intl_en.arb): Added 22+ new notification-specific keys including:
    • notificationChannelName, notificationChannelDescription
    • notificationOrderPublished, notificationOrderTaken, notificationPaymentRequired
    • notificationInvoiceRequired, notificationPaymentSent, notificationBitcoinReleased
    • notificationOrderCanceled, notificationDisputeStarted, notificationTradeComplete
    • And many more notification-specific strings with proper parameterization
  • Spanish (intl_es.arb): Added complete Spanish translations for all notification keys
  • Italian (intl_it.arb): Added complete Italian translations for all notification keys

Updated Notification Service to Use Localized Strings

  • Modified lib/notifications/notification_service.dart to use Flutter's localization system
  • Implemented locale-aware notifications using ui.PlatformDispatcher.instance.locale
  • Updated all hardcoded strings to use proper localized methods
  • Both generic and action-based notifications now support multiple languages

Summary by CodeRabbit

  • New Features

    • Enhanced notification system with action-based, localized notifications for various trade and message events.
    • Notifications now display only when the app is in the background, reducing unnecessary interruptions.
    • Notifications include event-specific details and support English, Spanish, and Italian languages.
    • Added safe nickname retrieval to chat and trade screens to improve stability with invalid or missing public keys.
  • Bug Fixes

    • Improved reliability and context-awareness of notifications, ensuring correct display based on app state.
    • Added validation and fallback handling for user handle generation from public keys.
  • Documentation

    • Added new localization strings for notifications in English, Spanish, and Italian.

Fix #79

1. Full Message Decryption Integration

- ✅ Added Riverpod integration to access session management
- ✅ Implemented complete message decryption using existing unWrap method
- ✅ Added session matching logic to find correct trade keys for decryption
- ✅ Integrated with MostroMessage parsing for action extraction

2. Action-Based Notification System

- ✅ Created comprehensive action-to-title mapping with user-friendly titles:
  - "Order Published", "Payment Required", "Bitcoin Released", etc.
- ✅ Implemented protocol-compliant message content following Mostro suggestions:
  - "Your offer has been published! Please wait until another user picks your order."
  - "Please pay the hold invoice of 1000 Sats"
  - "Bitcoin has been released! Your invoice should be paid shortly."

3. Enhanced Payload & Deep Linking

- ✅ Rich notification payload with action context:
{
  "eventId": "abc123",
  "kind": "1059",
  "orderId": "order_456",
  "action": "release"
}

4. Robust Error Handling

- ✅ Graceful fallback to generic messages when:
  - Session not found for decryption
  - Message decryption fails
  - MostroMessage parsing errors
- ✅ Comprehensive logging for debugging

5. Background Service Integration

- ✅ Created showSimpleNotification() for background isolates without Riverpod
- ✅ Updated both desktop and mobile background services
- ✅ Maintained functionality when app is in background

🔧 Key Features Implemented:

Smart Session Matching

// Finds the correct trade key by matching event target pubkey
Session? _findSessionForEvent(NostrEvent event, List<Session> sessions)

Complete Message Decryption Flow

// 1. Find session with matching trade key
// 2. Decrypt using event.unWrap(session.tradeKey.private)
// 3. Parse MostroMessage from decrypted JSON
// 4. Extract action and payload for notification content

Dynamic Content Generation

- Payment Actions: Extract amounts from PaymentRequest/Order payloads
- Error Actions: Extract specific error reasons from CantDo payloads
- Status Updates: Context-aware messages for all trade states

📱 User Experience Improvements:

Before:

- Generic "New Mostro Event" for all notifications
- No context about what action occurred
- No payload for deep linking

After:

- Action-specific titles: "Payment Required", "Bitcoin Released"
- Rich context messages: "Please pay the hold invoice of 1000 Sats"
- Deep linking ready: Full payload with action and order details
- Graceful degradation: Falls back to generic message if decryption fails

🔒 Security & Privacy:

- Only kind 1059 events: Filters out all other event types
- Session-based decryption: Uses correct trade keys for each order
- No key exposure: Leverages existing secure key management
- Error resilience: Never exposes sensitive data in error states

Added New Localization Keys to ARB Files

- English (intl_en.arb): Added 22+ new notification-specific keys including:
  - notificationChannelName, notificationChannelDescription
  - notificationOrderPublished, notificationOrderTaken, notificationPaymentRequired
  - notificationInvoiceRequired, notificationPaymentSent, notificationBitcoinReleased
  - notificationOrderCanceled, notificationDisputeStarted, notificationTradeComplete
  - And many more notification-specific strings with proper parameterization
- Spanish (intl_es.arb): Added complete Spanish translations for all notification keys
- Italian (intl_it.arb): Added complete Italian translations for all notification keys

Updated Notification Service to Use Localized Strings

- Modified lib/notifications/notification_service.dart to use Flutter's localization system
- Implemented locale-aware notifications using ui.PlatformDispatcher.instance.locale
- Updated all hardcoded strings to use proper localized methods
- Both generic and action-based notifications now support multiple languages
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 20, 2025

Walkthrough

The changes overhaul the notification system to provide event-specific, localized, and context-aware notifications for Nostr events, especially of kind 1059. Notification logic now distinguishes between foreground and background app states, uses session data for decryption and message parsing, and supports richer notification content and payloads. Localization strings for notifications were added in English, Spanish, and Italian.

Changes

File(s) Change Summary
lib/background/background.dart, lib/background/desktop_background_service.dart Updated event subscription logic to show notifications only when app is backgrounded; switched to simple notification method.
lib/notifications/notification_service.dart Refactored notification logic: now action-based, localized, session-aware, with retry/backoff and payload support.
lib/services/mostro_service.dart Integrated notification trigger into event subscription handler, with error logging.
lib/features/chat/screens/chat_room_screen.dart, lib/features/chat/screens/chat_rooms_list.dart, lib/features/trades/widgets/mostro_message_detail_widget.dart Added private helper _getSafeNickname methods to safely retrieve nicknames with validation, improving robustness against invalid or null public keys.
lib/shared/providers/legible_handle_provider.dart Enhanced deterministicHandleFromHexKey with input validation, error handling, and fallback handle generation.
lib/l10n/intl_en.arb, lib/l10n/intl_es.arb, lib/l10n/intl_it.arb Added comprehensive notification-related localization strings in English, Spanish, and Italian; updated cooperative cancel string for clarity.

Sequence Diagram(s)

sequenceDiagram
    participant App
    participant MostroService
    participant NotificationService
    participant User

    App->>MostroService: Receives Nostr event
    MostroService->>NotificationService: showLocalNotificationFromService(event, ref)
    NotificationService->>NotificationService: Check event kind (1059)
    NotificationService->>NotificationService: Find session & decrypt content
    NotificationService->>NotificationService: Parse message, determine action
    NotificationService->>NotificationService: Generate localized title/message
    NotificationService->>User: Show notification with payload
Loading

Estimated code review effort

3 (~45 minutes)

Possibly related PRs

Poem

🐰
In the warren, news arrives anew,
Now each alert tells what and who!
With languages three, and actions clear,
Notifications bring context near.
Whether in front or out of sight,
Mostro’s messages now feel just right!
— A rabbit, hopping with delight


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f3b5e65 and f039572.

📒 Files selected for processing (8)
  • lib/features/chat/screens/chat_room_screen.dart (3 hunks)
  • lib/features/chat/screens/chat_rooms_list.dart (2 hunks)
  • lib/features/trades/widgets/mostro_message_detail_widget.dart (4 hunks)
  • lib/l10n/intl_en.arb (3 hunks)
  • lib/l10n/intl_es.arb (3 hunks)
  • lib/l10n/intl_it.arb (3 hunks)
  • lib/notifications/notification_service.dart (3 hunks)
  • lib/shared/providers/legible_handle_provider.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/l10n/intl_en.arb
🧰 Additional context used
📓 Path-based instructions (5)
lib/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/**/*.dart: Use S.of(context).yourKey for all user-facing strings
Always check mounted before using context after async operations

Files:

  • lib/features/chat/screens/chat_rooms_list.dart
  • lib/features/trades/widgets/mostro_message_detail_widget.dart
  • lib/shared/providers/legible_handle_provider.dart
  • lib/features/chat/screens/chat_room_screen.dart
  • lib/notifications/notification_service.dart
**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

**/*.dart: Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/features/chat/screens/chat_rooms_list.dart
  • lib/features/trades/widgets/mostro_message_detail_widget.dart
  • lib/shared/providers/legible_handle_provider.dart
  • lib/features/chat/screens/chat_room_screen.dart
  • lib/notifications/notification_service.dart
lib/l10n/intl_{en,es,it}.arb

📄 CodeRabbit Inference Engine (CLAUDE.md)

Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)

Files:

  • lib/l10n/intl_es.arb
  • lib/l10n/intl_it.arb
lib/l10n/*.arb

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use proper ARB metadata for parameterized strings

Files:

  • lib/l10n/intl_es.arb
  • lib/l10n/intl_it.arb
lib/l10n/**

📄 CodeRabbit Inference Engine (CLAUDE.md)

Internationalization files are located in lib/l10n/

Files:

  • lib/l10n/intl_es.arb
  • lib/l10n/intl_it.arb
🧠 Learnings (7)
📓 Common learnings
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.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Encapsulate business logic in Notifiers
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
lib/features/chat/screens/chat_rooms_list.dart (1)

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.

lib/features/trades/widgets/mostro_message_detail_widget.dart (5)

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.306Z
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.305Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores

Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.

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.

lib/features/chat/screens/chat_room_screen.dart (1)

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.

lib/notifications/notification_service.dart (19)

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart

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: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.659Z
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.659Z
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.306Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom timeAgoWithLocale() method is implemented in the NostrEvent extension

Learnt from: chebizarro
PR: #74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.

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.

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.306Z
Learning: All changes should pass flutter analyze before commit

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Regenerate localization files after ARB modifications using dart run build_runner build -d

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.305Z
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.306Z
Learning: Use Riverpod for all state management

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.306Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in main.dart with timeago.setLocaleMessages()

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Encapsulate business logic in Notifiers

lib/l10n/intl_es.arb (2)

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings

lib/l10n/intl_it.arb (2)

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings

🪛 RuboCop (1.76.1)
lib/l10n/intl_es.arb

[warning] 736-736: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

lib/l10n/intl_it.arb

[warning] 744-744: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

🔇 Additional comments (17)
lib/features/chat/screens/chat_rooms_list.dart (2)

76-89: LGTM! Excellent defensive programming for nickname retrieval.

The _getSafeNickname method provides robust input validation with hex format checking and proper fallbacks. This pattern is consistently applied across multiple UI components in the PR, improving overall app stability.


94-95: Good use of null-aware operators for pubkey access.

The safe extraction of pubkey with null fallback and consistent usage in _getSafeNickname prevents potential runtime errors.

lib/shared/providers/legible_handle_provider.dart (1)

121-178: Excellent enhancement with comprehensive input validation and deterministic fallbacks.

The function now handles edge cases gracefully:

  • Input cleaning and hex validation prevent parsing errors
  • Padding/truncation ensures consistent 64-character processing
  • Try-catch wrapper prevents BigInt parsing exceptions
  • Hash-based fallback generation maintains deterministic output for invalid inputs

This robust implementation perfectly supports the nickname safety improvements throughout the PR.

lib/features/trades/widgets/mostro_message_detail_widget.dart (2)

18-31: Consistent implementation of safe nickname retrieval.

The _getSafeNickname method follows the same robust validation pattern used across other components in this PR, providing centralized input validation and hex format checking.


130-130: Proper usage of safe nickname method throughout action text generation.

All nickname retrieval calls now use the validated _getSafeNickname method, preventing potential runtime errors from invalid public keys while maintaining the correct access pattern via tradeState.peer?.publicKey.

Also applies to: 140-140, 149-149, 154-154, 159-159

lib/features/chat/screens/chat_room_screen.dart (3)

29-42: Consistent safe nickname implementation.

The _getSafeNickname method maintains the same robust validation pattern used throughout the PR, ensuring consistent error handling for invalid public keys.


48-48: Good defensive programming with null-aware operators and session checks.

The safe extraction of peer pubkey and the null guard for session before calling _buildMessageHeader prevent potential runtime errors.

Also applies to: 82-82


168-169: Proper usage of safe nickname method for both peer and session keys.

Both nickname retrievals now use the validated _getSafeNickname method, ensuring consistent error handling for both peer and session trade key public keys.

lib/notifications/notification_service.dart (3)

38-88: Excellent comprehensive notification enhancement with proper error handling.

The refactored notification system provides:

  • Secure kind 1059 event filtering
  • Session-based decryption with proper key matching
  • Graceful fallback to generic notifications on errors
  • Comprehensive logging for debugging

The implementation follows security best practices by validating event types and using session-specific keys for decryption.


90-140: Robust retry mechanism with persistent failure tracking.

The retry logic implements:

  • Exponential backoff for failed notifications
  • Persistent storage of failed events for later retry
  • Proper error logging and attempt limits

This ensures reliable notification delivery even under adverse conditions.


262-302: Comprehensive action-based title generation with full localization.

All notification titles properly use localized strings from the generated S class, ensuring multi-language support across all supported actions. The switch statement provides good coverage of Mostro protocol actions.

lib/l10n/intl_es.arb (3)

47-48: LGTM: Well-formatted generic payment strings added.

The new generic payment strings payInvoiceGeneric and addInvoiceGeneric provide appropriate fallback text for simplified prompts when detailed payment information isn't available.


82-82: LGTM: Improved cooperative cancellation message.

The updated message provides clearer instructions for users when their counterpart wants to cancel an order, with better formatting and more explicit guidance about pressing the cancel button.


693-753: LGTM: Comprehensive notification localization strings added.

The notification strings section provides thorough Spanish translations for the enhanced notification system, including proper ARB metadata for parameterized strings. These translations support the new action-based, localized notifications mentioned in the PR objectives.

lib/l10n/intl_it.arb (3)

47-48: LGTM: Well-formatted generic payment strings added.

The new generic payment strings payInvoiceGeneric and addInvoiceGeneric provide appropriate Italian fallback text for simplified prompts when detailed payment information isn't available.


82-82: LGTM: Improved cooperative cancellation message.

The updated message provides clearer and more formal Italian instructions for users when their counterpart wants to cancel an order, with better phrasing and more explicit guidance.


701-761: LGTM: Comprehensive notification localization strings added.

The notification strings section provides thorough Italian translations for the enhanced notification system, including proper ARB metadata for parameterized strings. These translations support the new action-based, localized notifications mentioned in the PR objectives.


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
lib/notifications/notification_service.dart (2)

89-119: Well-implemented retry logic with exponential backoff.

The retry mechanism provides good resilience with configurable attempts and exponential backoff. The TODO comment about persistent queue is a good future enhancement.

Consider implementing the persistent queue for failed notifications to handle app crashes or long background periods:

+import 'package:shared_preferences/shared_preferences.dart';
+
+class FailedNotificationQueue {
+  static const String _queueKey = 'failed_notifications';
+  
+  static Future<void> add(String eventId) async {
+    final prefs = await SharedPreferences.getInstance();
+    final queue = prefs.getStringList(_queueKey) ?? [];
+    if (!queue.contains(eventId)) {
+      queue.add(eventId);
+      await prefs.setStringList(_queueKey, queue);
+    }
+  }
+  
+  static Future<List<String>> getAndClear() async {
+    final prefs = await SharedPreferences.getInstance();
+    final queue = prefs.getStringList(_queueKey) ?? [];
+    await prefs.remove(_queueKey);
+    return queue;
+  }
+}

   // Optionally store failed notifications for later retry when app returns to foreground
   if (!success) {
-    // Store the event ID in a persistent queue for later retry
-    // await failedNotificationsQueue.add(event.id!);
+    await FailedNotificationQueue.add(event.id!);
   }

140-226: Excellent notification display implementation with proper localization.

The notification functions correctly:

  • Load localized strings based on platform locale
  • Include rich payloads for notification tap handling
  • Configure platform-specific settings appropriately
  • Set high priority for trade-related notifications

Consider extracting the notification channel configuration to reduce duplication:

+NotificationDetails _getNotificationDetails(S localizations) {
+  return NotificationDetails(
+    android: AndroidNotificationDetails(
+      'mostro_channel',
+      localizations.notificationChannelName,
+      channelDescription: localizations.notificationChannelDescription,
+      importance: Importance.max,
+      priority: Priority.high,
+      visibility: NotificationVisibility.public,
+      playSound: true,
+      enableVibration: true,
+      ticker: 'ticker',
+      icon: '@drawable/ic_notification',
+    ),
+    iOS: DarwinNotificationDetails(
+      presentAlert: true,
+      presentBadge: true,
+      presentSound: true,
+      interruptionLevel: InterruptionLevel.critical,
+    ),
+  );
+}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0fc08b and a6509e9.

📒 Files selected for processing (7)
  • lib/background/background.dart (1 hunks)
  • lib/background/desktop_background_service.dart (2 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/notifications/notification_service.dart (3 hunks)
  • lib/services/mostro_service.dart (2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
lib/**/*.dart

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • CLAUDE.md
**/*.dart

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • CLAUDE.md
lib/background/**/*.dart

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • CLAUDE.md
lib/l10n/intl_{en,es,it}.arb

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • CLAUDE.md
lib/l10n/*.arb

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • CLAUDE.md
lib/l10n/**

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • CLAUDE.md
🧠 Learnings (8)
📓 Common learnings
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.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Encapsulate business logic in Notifiers
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
lib/background/desktop_background_service.dart (14)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.305Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify `test/mocks.mocks.dart` - it already has file-level ignores
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in `shared/providers/mostro_database_provider.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#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.306Z
Learning: Applies to lib/data/models.dart : Models should be exported through `data/models.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#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.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.306Z
Learning: Applies to lib/features/*/providers/**/*.dart : Providers are organized by feature in `features/{feature}/providers/`
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
lib/services/mostro_service.dart (13)
Learnt from: chebizarro
PR: MostroP2P/mobile#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.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through `services/nostr_service.dart`
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
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.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/background/**/*.dart : Background services are implemented in the `background/` directory
Learnt from: chebizarro
PR: MostroP2P/mobile#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.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.306Z
Learning: Applies to lib/features/*/providers/**/*.dart : Providers are organized by feature in `features/{feature}/providers/`
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/background/background.dart (9)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
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.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.305Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify `test/mocks.mocks.dart` - it already has file-level ignores
lib/l10n/intl_en.arb (3)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (`intl_en.arb`, `intl_es.arb`, `intl_it.arb`)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
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.
lib/notifications/notification_service.dart (16)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/features/*/notifiers/**/*.dart : Notifiers for complex state logic are placed in `features/{feature}/notifiers/`
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.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through `services/nostr_service.dart`
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.659Z
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.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in the NostrEvent extension
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: chebizarro
PR: MostroP2P/mobile#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.
Learnt from: chebizarro
PR: MostroP2P/mobile#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.306Z
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.306Z
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.305Z
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.306Z
Learning: Applies to lib/main.dart : Locales for the `timeago` package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Use Riverpod for all state management
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
lib/l10n/intl_es.arb (3)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (`intl_en.arb`, `intl_es.arb`, `intl_it.arb`)
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.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
lib/l10n/intl_it.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.306Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (`intl_en.arb`, `intl_es.arb`, `intl_it.arb`)
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.
🪛 RuboCop (1.76.1)
lib/l10n/intl_es.arb

[warning] 732-732: Duplicated key in hash literal.

(Lint/DuplicateHashKey)


[warning] 733-733: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

lib/l10n/intl_it.arb

[warning] 740-740: Duplicated key in hash literal.

(Lint/DuplicateHashKey)


[warning] 741-741: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

🔇 Additional comments (9)
lib/background/desktop_background_service.dart (1)

9-9: LGTM! Proper notification handling for background isolate.

The implementation correctly uses showSimpleNotification which is designed for background services without Riverpod context. The conditional check ensures notifications are only shown when the app is in the background, maintaining proper separation of concerns as per the architecture.

Also applies to: 78-80

lib/services/mostro_service.dart (1)

14-14: LGTM! Well-integrated foreground notification handling.

The notification trigger is properly placed after message storage and uses the correct showLocalNotificationFromService overload for services. The error handling ensures notification failures don't interrupt the core message processing flow.

Also applies to: 98-103

lib/background/background.dart (1)

72-74: LGTM! Consistent background notification handling.

The implementation correctly uses showSimpleNotification and only triggers notifications when the app is in the background. This maintains consistency with the desktop background service and aligns with the architectural principle of separation between foreground and background notification handling.

lib/notifications/notification_service.dart (4)

37-44: Excellent API design with proper overloading.

The dual API supporting both WidgetRef and Ref provides flexibility for different contexts (UI components vs services) while maintaining a single implementation through delegation. This design maintains backward compatibility and reduces code duplication.


46-87: Robust notification processing with excellent error handling.

The implementation correctly:

  • Filters for kind 1059 events as per Nostr protocol
  • Implements graceful degradation at each processing step
  • Ensures users always receive notifications (generic fallback)
  • Provides detailed error logging for debugging

The multi-step approach (session lookup → decryption → parsing → display) aligns perfectly with the PR objectives for enhanced notifications.


122-137: Correct session matching implementation.

The function properly extracts the target public key from Nostr event tags and matches it against session trade keys. The use of elementAtOrNull and try-catch ensures safe handling of malformed events.


341-348: Perfect implementation for background notification handling.

The showSimpleNotification function provides exactly what background services need - a simple way to show generic notifications for kind 1059 events without requiring Riverpod context or session information.

lib/l10n/intl_it.arb (1)

697-757: Comprehensive notification localization successfully added.

The Italian notification strings are well-structured with proper ARB metadata and placeholder support. The translations provide appropriate context for various trade lifecycle events, payment statuses, and system notifications, aligning perfectly with the enhanced notification system objectives.

lib/l10n/intl_es.arb (1)

689-749: Excellent Spanish notification localization implementation.

The Spanish notification strings are comprehensive and well-translated, providing clear and contextual messages for all notification scenarios. The ARB metadata is properly structured with appropriate placeholders for dynamic content, supporting the enhanced notification system's multilingual capabilities.

grunch added 2 commits July 21, 2025 12:14
  1. Fixed null amount values: Updated payInvoice and addInvoice cases to properly extract
  amount, fiatAmount, and fiatCode from the PaymentRequest and Order payloads
  2. Added expandable notifications: Implemented BigTextStyleInformation for both action-based
  and generic notifications so users can see complete messages
  3. Proper data extraction: The code now correctly accesses nested data structures
  (PaymentRequest.order.fiatCode, etc.) with proper null safety
  1. Input Validation & Error Handling: Enhanced deterministicHandleFromHexKey function with:
    - Null/empty string validation
    - Hex character validation with regex
    - Automatic padding/truncation for different string lengths
    - Try-catch error handling around BigInt.parse()
    - Fallback nickname generation using string hash
  2. Null Safety Improvements: Updated multiple widgets to safely handle peer data:
    - MostroMessageDetail widget: Added _getSafeNickname helper method
    - ChatListItem widget: Safe access to peer publicKey
    - ChatRoomScreen widget: Safe session and peer handling
    - Replaced all unsafe session!.peer!.publicKey patterns with null-safe alternatives
  3. Comprehensive Coverage: Fixed similar issues in all files that use nickname generation:
    - lib/shared/providers/legible_handle_provider.dart
    - lib/features/trades/widgets/mostro_message_detail_widget.dart
    - lib/features/chat/screens/chat_rooms_list.dart
    - lib/features/chat/screens/chat_room_screen.dart

  ✅ Results:

  - No more "Could not parse BigInt" crashes when viewing trade details
  - Graceful degradation with fallback nicknames for invalid/corrupted peer data
  - Consistent user experience even when backend data has quality issues
  - All Flutter analyze checks pass without errors

  The app will now handle cases where:
  - Peer public keys are null, empty, or contain invalid characters
  - Public keys are shorter/longer than expected (64 characters)
  - Session data is incomplete or corrupted
  - Any BigInt parsing operation fails for any reason

  Users like Alice can now view trade details without crashes, and the app will show
  appropriate fallback nicknames when peer data is problematic.
@grunch
Copy link
Member Author

grunch commented Sep 17, 2025

closing in favor of #297

@grunch grunch closed this Sep 17, 2025
@grunch grunch deleted the fix/push-notifications-messages branch September 17, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: Add event-specific information to notifications

2 participants