Skip to content

Conversation

@Catrya
Copy link
Member

@Catrya Catrya commented Oct 21, 2025

fix #317
- Automatically send configured Lightning address when receiving add-invoice with waiting-buyer-invoice status instead of requiring manual input
- Skip auto-send for payment-failed scenarios to allow manual intervention
- Add Lightning address validation with regex pattern
- Add user feedback via snackbar when Lightning address is auto-sent
- fix: resolve Lightning address field clearing remnants

Summary by CodeRabbit

  • New Features

    • Automatic Lightning address handling for invoice creation when configured; falls back to manual input if needed
  • Settings

    • Option to immediately clear your saved default Lightning address; settings UI now syncs instantly with changes
  • Notifications

    • New notification indicating when your configured Lightning address is used
  • Localization

    • Added Lightning address translations in Spanish and Italian

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

Adds automatic use of a configured Lightning address for incoming add-invoice events with auto-send and fallbacks, adds clearing support for the default Lightning address in settings and UI sync, introduces a localization string and notification mapping, always includes amount in PaymentRequest.toJson, and updates generated test mocks for the new settings flag.

Changes

Cohort / File(s) Summary
Mostro notifier (auto-send)
lib/features/order/notfiers/abstract_mostro_notifier.dart
Implements automatic Lightning-address handling for add-invoice events: reads/validates defaultLightningAddress, attempts auto-send with amount: null, logs and notifies on success, falls back to manual invoice input on failure, adds private helpers (_handleAddInvoiceWithAutoLightningAddress, _isValidLightningAddress, _navigateToManualInvoiceInput), updates add-invoice control flow to await the handler, and cleans up a session timeout timer on dispose.
Settings model & notifier
lib/features/settings/settings.dart, lib/features/settings/settings_notifier.dart
Adds bool clearDefaultLightningAddress = false to Settings.copyWith. updateDefaultLightningAddress now clears the stored address when the new value is null/empty (via clearDefaultLightningAddress) or trims/sets it otherwise, then persists preferences.
Settings UI
lib/features/settings/settings_screen.dart
Subscribes to settingsProvider and updates the lightning address text controller when defaultLightningAddress changes; onChanged now clears the text field immediately for empty input to keep UI in sync.
Notifications / Localization usage
lib/shared/widgets/notification_listener_widget.dart, lib/l10n/intl_en.arb, lib/l10n/intl_es.arb, lib/l10n/intl_it.arb
Adds lightningAddressUsed localization key (EN/ES/IT) and maps a new customMessage value to S.of(context)!.lightningAddressUsed in the notification listener.
Payment request model
lib/data/models/payment_request.dart
PaymentRequest.toJson now always appends amount to the values list, even when amount is null.
Test mocks
test/mocks.mocks.dart
Adds clearDefaultLightningAddress parameter to MockSettings constructor and copyWith, and propagates #clearDefaultLightningAddress through the generated noSuchMethod argument and fake return mappings.

Sequence Diagram(s)

sequenceDiagram
    participant Mostrod
    participant Notifier
    participant Settings
    participant MostroService
    participant Notification
    Note over Notifier,Settings: Incoming add-invoice handling (new flow)

    Mostrod->>Notifier: add-invoice event
    activate Notifier
    Notifier->>Notifier: if state.status == paymentFailed -> manual flow
    Notifier->>Settings: get defaultLightningAddress
    Notifier->>Notifier: _isValidLightningAddress(address)

    alt valid address & not paymentFailed
        Notifier->>MostroService: sendInvoice(invoice, amount: null)
        alt send success
            MostroService-->>Notifier: success
            Notifier->>Notification: show "lightningAddressUsed"
        else send failure
            MostroService-->>Notifier: error
            Notifier->>Notifier: _navigateToManualInvoiceInput()
        end
    else invalid/no address or paymentFailed
        Notifier->>Notifier: _navigateToManualInvoiceInput()
    end
    deactivate Notifier
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • grunch

Poem

🐰
I sniff the code and find the stream,
A lightning address fulfills the dream.
Invoices hop off, no typing fuss,
Auto-sent neatly — joy for us.
Rabbit nods, the network hums ⚡🐇

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "feat: auto-send Lightning address on add-invoice for waiting-buyer-invoice" directly reflects the main feature being implemented across the changeset. The title is concise, specific, and clearly communicates that the primary change involves automatically sending a configured Lightning address when processing add-invoice actions. This accurately captures the core objective from the linked issue and the PR description without being vague or overly broad.
Linked Issues Check ✅ Passed The implementation comprehensively addresses the objective from linked issue #317 to automatically use the configured ln-address upon receiving an add-invoice message. The core feature is implemented in abstract_mostro_notifier.dart with automatic Lightning address validation and sending, plus appropriate safeguards to skip auto-send when state.status is paymentFailed. Supporting changes include settings management for the Lightning address (settings.dart, settings_notifier.dart), UI synchronization (settings_screen.dart), user feedback localization across multiple languages, and integration of the notification message display. All changes align with the stated PR objectives including validation, user feedback via snackbar, and UI fixes.
Out of Scope Changes Check ✅ Passed All code changes directly support the auto-send Lightning address feature or address the specific PR objectives. The payment_request.dart modification to always append amount (even when null) is a necessary supporting change for sending invoices via mostroService with a null amount during auto-send. The settings management, UI synchronization, and localization changes all implement the feature and user feedback requirements. The test mock updates appropriately reflect the Settings.copyWith signature changes. No changes appear unrelated to the stated objectives of automatically sending the configured Lightning address and providing appropriate user feedback and safeguards.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch lnaddress

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@Catrya Catrya requested a review from grunch October 21, 2025 03:08
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: 0

🧹 Nitpick comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)

557-561: Consider enhancing Lightning address validation.

The current regex provides basic email-like validation, which is acceptable for Lightning addresses. However, you could make it more robust:

-  bool _isValidLightningAddress(String address) {
-    final emailRegex = RegExp(r'^[^@]+@[^@]+\.[^@]+$');
-    return emailRegex.hasMatch(address);
-  }
+  bool _isValidLightningAddress(String address) {
+    // Lightning address format: user@domain.tld
+    // More robust validation with character constraints
+    final lnAddressRegex = RegExp(
+      r'^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$'
+    );
+    return lnAddressRegex.hasMatch(address);
+  }

This enhanced regex:

  • Restricts characters to typical username/domain characters
  • Requires at least 2 characters in the TLD
  • Better handles multi-level domains like user@example.co.uk
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2b4808 and 140ce79.

📒 Files selected for processing (10)
  • lib/data/models/payment_request.dart (1 hunks)
  • lib/features/order/notfiers/abstract_mostro_notifier.dart (3 hunks)
  • lib/features/settings/settings.dart (2 hunks)
  • lib/features/settings/settings_notifier.dart (1 hunks)
  • lib/features/settings/settings_screen.dart (3 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/shared/widgets/notification_listener_widget.dart (1 hunks)
  • test/mocks.mocks.dart (3 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
lib/l10n/intl_{en,es,it}.arb

📄 CodeRabbit inference engine (CLAUDE.md)

Add new localization keys to all three ARB files (en, es, it)

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

For parameterized strings, include proper ARB metadata

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
Use S.of(context).keyName for all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (use S.of(context))
Check mounted before using BuildContext after async gaps
Use const constructors where possible

Name Riverpod providers as Provider or Notifier

Files:

  • lib/shared/widgets/notification_listener_widget.dart
  • lib/features/settings/settings.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/data/models/payment_request.dart
  • lib/features/settings/settings_notifier.dart
  • lib/features/settings/settings_screen.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 zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., prefer withValues() over withOpacity())
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/shared/widgets/notification_listener_widget.dart
  • lib/features/settings/settings.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/data/models/payment_request.dart
  • test/mocks.mocks.dart
  • lib/features/settings/settings_notifier.dart
  • lib/features/settings/settings_screen.dart
lib/shared/**

📄 CodeRabbit inference engine (AGENTS.md)

Put shared utilities in lib/shared/

Files:

  • lib/shared/widgets/notification_listener_widget.dart
lib/features/**

📄 CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/settings/settings.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/settings/settings_notifier.dart
  • lib/features/settings/settings_screen.dart
lib/data/**

📄 CodeRabbit inference engine (AGENTS.md)

Store persistence and API code under lib/data/

Files:

  • lib/data/models/payment_request.dart
**/*.mocks.dart

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.mocks.dart: Never manually edit generated *.mocks.dart files
Do not add ignore comments to *.mocks.dart; regenerate instead

Files:

  • test/mocks.mocks.dart
test/mocks.mocks.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Never modify test/mocks.mocks.dart (has file-level ignores; regenerate via build_runner)

Files:

  • test/mocks.mocks.dart
test/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Place unit tests under test/

test/**/*.dart: Mirror feature layout for tests under test/
Prefer Mockito fakes generated via build_runner in tests

Files:

  • test/mocks.mocks.dart
🔇 Additional comments (13)
lib/data/models/payment_request.dart (1)

27-27: Acknowledge this is an intentional protocol change, but verify version compatibility strategy before merging.

The change is confirmed as intentional and well-tested: test fixtures already expect the new 3-element array format [order, invoice, null/amount] instead of variable-length. However, this IS a breaking protocol change that requires careful version management.

Current status:

  • Tests validate the new fixed-length format
  • Protocol versioning exists (Config.mostroVersion = 1)
  • Version is already included in serialized messages
  • Backward compatibility patterns exist in the codebase

Before merging, verify:

  1. Mostrod server handles the new format with explicit null at position 2
  2. Version negotiation prevents older client versions from sending this format (or older servers from receiving it)
  3. This breaking change is documented (protocol v1 feature or version bump needed)

The deserialization at line 62 (if (json.length > 2)) will always be true now, which changes the semantics of amount detection. Ensure your versioning strategy prevents mix-and-match between old/new protocol versions.

lib/l10n/intl_en.arb (1)

229-229: Localization entry synced

The new lightningAddressUsed string is in place and keeps the English bundle aligned with the other locales.

lib/shared/widgets/notification_listener_widget.dart (1)

73-75: Snackbar wiring complete

Great to see the new lightningAddressUsed custom message routed through localization, keeping notification text consistent.

lib/l10n/intl_es.arb (1)

195-195: Spanish string aligned

The Spanish bundle now includes lightningAddressUsed, keeping parity across locales.

lib/features/settings/settings.dart (1)

29-41: CopyWith handles clearing

The clearDefaultLightningAddress toggle elegantly disambiguates between clearing and updating the stored address.

lib/features/settings/settings_screen.dart (1)

9-9: UI/state sync looks solid

Listening for Settings updates to push changes into _lightningAddressController, plus the immediate reset on empty values, keeps the text field and provider perfectly aligned.

Also applies to: 45-52, 315-317

lib/l10n/intl_it.arb (1)

229-229: Italian string updated

lightningAddressUsed is now present in Italian, preserving localization coverage.

lib/features/settings/settings_notifier.dart (1)

82-88: Notifier logic tightened

Trimming non-empty inputs and using clearDefaultLightningAddress for null/empty cases keeps persisted state clean and unambiguous.

test/mocks.mocks.dart (1)

1947-1984: Mocks regenerated correctly

The mock now mirrors the Settings.copyWith signature, so tests will exercise the new clear flag without drift.

lib/features/order/notfiers/abstract_mostro_notifier.dart (4)

12-12: LGTM!

The settings provider import is necessary for accessing the configured Lightning address.


189-193: LGTM!

Good refactoring to delegate to a specialized handler. The async/await pattern is correctly used, and session persistence is maintained before the auto-send logic.


563-567: LGTM!

Clean helper method that improves code readability and maintainability by extracting the navigation logic.


515-555: Amount handling and state transitions are intentionally designed; consider adding clarifying comments.

Verification confirms the implementation is correct:

  1. Amount=null is valid: PaymentRequest accepts null amount as long as lnInvoice is provided. Lightning addresses autonomously determine payment amounts, making null a valid design choice.

  2. Navigation happens implicitly: The method publishes a MostroMessage, triggering order state transitions through the FSM. Action.addInvoice transitions from waitingBuyerInvoice to waitingPayment, and the reactive UI updates automatically via Riverpod. No explicit navigation is needed—the state machine and reactive architecture handle it.

Suggestion: Add an inline comment explaining that state updates trigger automatic UI refresh through the reactive architecture, so explicit navigation is intentionally omitted. This clarifies the async event-driven flow for future reviewers.

…voice

  - Automatically send configured Lightning address when receiving add-invoice with waiting-buyer-invoice status instead of requiring manual input
  - Skip auto-send for payment-failed scenarios to allow manual intervention
  - Add Lightning address validation with regex pattern
  - Add user feedback via snackbar when Lightning address is auto-sent
  - Fix TextEditingController desync when clearing Lightning address field
  - Add immediate controller.text = '' when field is emptied to prevent residual characters from appearing in UI
  - Surgical fix that only affects clearing behavior, preserves normal typing
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: 0

🧹 Nitpick comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)

557-561: Consider strengthening Lightning address validation.

The current regex provides basic email-like validation and will catch obvious typos. However, it's quite permissive and would accept invalid formats like user@..com or user@domain_with_underscore.com.

Consider enhancing the validation to be more strict about allowed characters and domain format, or add a comment noting that full validation happens server-side.

Example of stricter validation:

 bool _isValidLightningAddress(String address) {
-  final emailRegex = RegExp(r'^[^@]+@[^@]+\.[^@]+$');
+  // Basic Lightning address validation (user@domain.tld format)
+  // Full validation is performed by the Lightning service
+  final emailRegex = RegExp(r'^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$');
   return emailRegex.hasMatch(address);
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 140ce79 and f5651f1.

📒 Files selected for processing (10)
  • lib/data/models/payment_request.dart (1 hunks)
  • lib/features/order/notfiers/abstract_mostro_notifier.dart (3 hunks)
  • lib/features/settings/settings.dart (2 hunks)
  • lib/features/settings/settings_notifier.dart (1 hunks)
  • lib/features/settings/settings_screen.dart (3 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/shared/widgets/notification_listener_widget.dart (1 hunks)
  • test/mocks.mocks.dart (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • lib/shared/widgets/notification_listener_widget.dart
  • lib/features/settings/settings.dart
  • test/mocks.mocks.dart
  • lib/features/settings/settings_notifier.dart
  • lib/features/settings/settings_screen.dart
🧰 Additional context used
📓 Path-based instructions (6)
lib/l10n/intl_{en,es,it}.arb

📄 CodeRabbit inference engine (CLAUDE.md)

Add new localization keys to all three ARB files (en, es, it)

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

For parameterized strings, include proper ARB metadata

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

lib/**/*.dart: Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs)
Use S.of(context).keyName for all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (use S.of(context))
Check mounted before using BuildContext after async gaps
Use const constructors where possible

Name Riverpod providers as Provider or Notifier

Files:

  • lib/data/models/payment_request.dart
  • 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 zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., prefer withValues() over withOpacity())
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/data/models/payment_request.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
lib/data/**

📄 CodeRabbit inference engine (AGENTS.md)

Store persistence and API code under lib/data/

Files:

  • lib/data/models/payment_request.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
🔇 Additional comments (8)
lib/data/models/payment_request.dart (1)

27-27: LGTM! Consistent serialization behavior.

The change to always include amount in the values list (even when null) makes the serialization consistent with deserialization, which already handles a null amount at index 2 (lines 62-77). This aligns with supporting null-amount invoice transmission for Lightning address workflows.

lib/l10n/intl_en.arb (1)

229-229: LGTM! Localization key added correctly.

The new lightningAddressUsed key follows the coding guidelines by being added to all three locale files (en, es, it) with appropriate translations. As per coding guidelines.

lib/l10n/intl_es.arb (1)

195-195: LGTM! Spanish translation added.

The Spanish translation is consistent with the English version and properly added. As per coding guidelines.

lib/l10n/intl_it.arb (1)

229-229: LGTM! Italian translation added.

The Italian translation is consistent with the English version and properly added. As per coding guidelines.

lib/features/order/notfiers/abstract_mostro_notifier.dart (4)

12-12: LGTM! Required import added.

The import of settings_provider is necessary to access the default Lightning address configuration.


192-193: LGTM! Improved control flow with auto Lightning address.

The change to use the new helper method instead of direct navigation implements the automatic Lightning address feature while maintaining proper fallback to manual input.


563-567: LGTM! Clean navigation helper.

The helper method provides a clean abstraction for navigating to manual invoice input, improving code readability.


515-555: No issues found — implementation correctly handles null amounts for Lightning addresses.

The code demonstrates intentional design that aligns with Lightning address protocol expectations:

  • PaymentRequest explicitly validates and handles null amounts (line 15 allows null when lnInvoice is provided; line 27 serializes null amounts)
  • sendInvoice accepts nullable amounts and passes them directly to PaymentRequest without transformation
  • The pattern differentiates between Lightning addresses (null amount for flexible pricing) and standard invoices (specified amount)
  • Serialization/deserialization correctly preserves null amounts throughout the payload

The implementation matches the architecture's design for handling both payment types.

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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5651f1 and 4086f19.

📒 Files selected for processing (1)
  • lib/features/order/notfiers/abstract_mostro_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)
Use S.of(context).keyName for all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (use S.of(context))
Check mounted before using BuildContext after async gaps
Use const constructors where possible

Name 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 zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., prefer withValues() over withOpacity())
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
🔇 Additional comments (5)
lib/features/order/notfiers/abstract_mostro_notifier.dart (5)

12-12: LGTM!

The settings provider import is correctly added to support reading the default Lightning address configuration.


189-193: LGTM!

The add-invoice case now correctly delegates to the new async handler to support automatic Lightning address sending while maintaining proper async/await patterns.


557-565: LGTM!

The Lightning address validation uses an appropriate regex pattern for the user@domain.tld format, with reasonable character constraints for both username and domain parts.


567-571: LGTM!

The navigation helper method cleanly encapsulates the manual invoice input navigation logic, improving code reusability.


543-543: No issues found. The custom notification message is properly handled.

The lightningAddressUsed message is correctly processed: the case statement in notification_listener_widget.dart maps it to S.of(context)!.lightningAddressUsed, and the localization key is defined across language files (English, Spanish, Italian). The notification system integrates properly with localization as required.

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: 0

🧹 Nitpick comments (2)
lib/features/order/notfiers/abstract_mostro_notifier.dart (2)

549-555: LGTM!

The error handling correctly includes a mounted check and falls back gracefully to manual input. The silent fallback provides good UX by not blocking the user flow.

Optional: Consider showing a brief error notification to inform the user why they were redirected to manual input:

       } catch (e) {
         logger.e('Failed to send Lightning address automatically: $e');
         // Check if still mounted after async operation
         if (!mounted) return;
+        // Optionally notify user of auto-send failure
+        final notificationNotifier = ref.read(notificationActionsProvider.notifier);
+        notificationNotifier.showCustomMessage('lightningAddressAutoSendFailed');
         // Fallback to manual input if auto-send fails
         _navigateToManualInvoiceInput();
       }

This would require adding a new localized string key for lightningAddressAutoSendFailed.


562-570: Basic validation is functional.

The regex provides reasonable validation for Lightning addresses in the standard user@domain.tld format.

Optional: The regex could be more robust to handle edge cases:

   bool _isValidLightningAddress(String address) {
     // Lightning address format: user@domain.tld
-    // More robust validation with character constraints
+    // Validates basic format with improved constraints
     final lnAddressRegex = RegExp(
-      r'^[a-zA-Z0-9._-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$'
+      r'^[a-zA-Z0-9]([a-zA-Z0-9._+-]*[a-zA-Z0-9])?@[a-zA-Z0-9]([a-zA-Z0-9.-]*[a-zA-Z0-9])?\.[a-zA-Z]{2,}$'
     );
     return lnAddressRegex.hasMatch(address);
   }

This improved version:

  • Prevents dots/hyphens at the start or end of local and domain parts
  • Allows the + character commonly used in email-style addresses
  • Handles single-character local parts (e.g., a@domain.tld)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4086f19 and 4973a4e.

📒 Files selected for processing (1)
  • lib/features/order/notfiers/abstract_mostro_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)
Use S.of(context).keyName for all user-facing strings (no hardcoded text)
Always use localized strings instead of hardcoded text
Pass BuildContext to methods that need localization (use S.of(context))
Check mounted before using BuildContext after async gaps
Use const constructors where possible

Name 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 zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., prefer withValues() over withOpacity())
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 (1)
📚 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 (9)
lib/features/order/notfiers/abstract_mostro_notifier.dart (9)

12-12: LGTM!

The import is necessary for accessing the default Lightning address configuration.


189-193: LGTM!

The control flow change to use the new handler is appropriate. The await ensures the auto-send logic completes before proceeding, and saving the session before handling the invoice is correct.


517-523: LGTM!

The payment-failed check correctly implements the requirement to skip auto-send after payment failures, allowing manual intervention. The early return prevents unnecessary processing.


525-528: LGTM!

Good defensive validation: reading from settings, trimming whitespace, and checking for null, empty, and valid format before proceeding with auto-send.


541-548: LGTM! Past review comment addressed.

The mounted check after the async operation correctly prevents using a disposed StateNotifier. The notification uses a localized string key as required by the coding guidelines.


556-559: LGTM!

The fallback to manual input when no valid Lightning address is available is correct and handles the expected path for users without configured Lightning addresses.


572-576: LGTM!

Clean helper method that encapsulates the navigation logic, improving code readability and maintainability.


578-585: LGTM!

The dispose method properly cleans up the session timeout timer for this order, preventing memory leaks. Good resource management practice.


531-539: No issues found — code is correct as written.

The sendInvoice method signature accepts int? amount (nullable integer), so passing null for Lightning addresses is syntactically valid. The null amount is intentional for LNURL-style Lightning addresses, which handle payment amounts independently. The code includes proper async handling with the mounted check at line 542, and the intent is clearly documented in comments.

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

tACK

@grunch grunch merged commit 3a2644f into main Oct 21, 2025
2 checks passed
@grunch grunch deleted the lnaddress branch October 21, 2025 18:56
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.

If the user has an ln-address set, use it automatically.

3 participants