Skip to content

Conversation

@grunch
Copy link
Member

@grunch grunch commented Aug 20, 2025

Key Features Added:

  • Real-time validation: Checks fiat amounts against Mostro instance min/max limits
  • Multi-language support: Added validation messages in English, Spanish, and Italian
  • Exchange rate integration: Automatically converts fiat to sats using current exchange rates
  • User-friendly errors: Shows specific messages for amounts that are too high or too low
  • Form integration: Prevents submission when amounts are outside allowed range

Files Modified:

  • lib/l10n/intl_*.arb - Added localized validation messages
  • lib/features/order/screens/add_order_screen.dart - Added helper functions and validation logic
  • lib/features/order/widgets/amount_section.dart - Enhanced form validation

Technical Details:

  • Uses ref.read(orderRepositoryProvider).mostroInstance to get min/max limits
  • Calculates sats using formula: fiatAmount / exchangeRate * 100000000
  • Validates both min and max amount fields in range orders
  • Shows validation errors immediately as user types
  • Prevents form submission with clear error messages

Validation Messages:

  • Too high: "The fiat amount is too high, please add a new fiat amount, the allowed range is between X and Y sats"
  • Too low: "The fiat amount is too low, please add a new fiat amount, the allowed range is between X and Y sats"

Quality Assurance:

  • ✅ Zero Flutter analyzer issues
  • ✅ Successful build compilation
  • ✅ Follows existing code patterns
  • ✅ No changes to Mostro message sending logic
  • ✅ Maintains backward compatibility

Summary by CodeRabbit

  • New Features

    • Inline fiat→sats validation on Add Order with visible validation messages that can block submission; sanitized custom payment input.
  • Localization

    • Added EN/ES/IT translations for "amount too high/low", "exchange rate not available", and "Mostro instance not available."
  • Style / UI

    • Submit and reactive buttons show disabled visuals when inactive; switches now color the thumb when active for consistent theming.
  • Chores

    • Android build now sources SDK/NDK/min SDK values from Flutter configuration.
    • Removed the PrivacySwitch widget from the UI.

Key Features Added:
- Real-time validation: Checks fiat amounts against Mostro instance min/max limits
- Multi-language support: Added validation messages in English, Spanish, and Italian
- Exchange rate integration: Automatically converts fiat to sats using current exchange rates
- User-friendly errors: Shows specific messages for amounts that are too high or too low
- Form integration: Prevents submission when amounts are outside allowed range

Files Modified:
- lib/l10n/intl_*.arb - Added localized validation messages
- lib/features/order/screens/add_order_screen.dart - Added helper functions and validation logic
- lib/features/order/widgets/amount_section.dart - Enhanced form validation

Technical Details:
- Uses ref.read(orderRepositoryProvider).mostroInstance to get min/max limits
- Calculates sats using formula: fiatAmount / exchangeRate * 100000000
- Validates both min and max amount fields in range orders
- Shows validation errors immediately as user types
- Prevents form submission with clear error messages

Validation Messages:
- Too high: "The fiat amount is too high, please add a new fiat amount, the allowed range is
between X and Y sats"
- Too low: "The fiat amount is too low, please add a new fiat amount, the allowed range is between
X and Y sats"

Quality Assurance:
- ✅ Zero Flutter analyzer issues
- ✅ Successful build compilation
- ✅ Follows existing code patterns
- ✅ No changes to Mostro message sending logic
- ✅ Maintains backward compatibility
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 20, 2025

Warning

Rate limit exceeded

@grunch has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 36 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 2dd429f and dfcdc9f.

📒 Files selected for processing (1)
  • lib/features/order/screens/add_order_screen.dart (9 hunks)

Walkthrough

Replaces hard-coded Android Gradle SDK/NDK values with Flutter-provided values; adds fiat→sats conversion and sats-range validation to AddOrderScreen; surfaces validationError in AmountSection UI; blocks submission when exchange rate or Mostro instance is missing; makes submit/onPressed callbacks nullable; adds EN/ES/IT i18n strings.

Changes

Cohort / File(s) Summary of changes
Android build config
android/app/build.gradle
Replace hard-coded compileSdk, ndkVersion, and minSdk with flutter.compileSdkVersion, flutter.ndkVersion, and flutter.minSdkVersion.
Add order screen & validation
lib/features/order/screens/add_order_screen.dart
Add fiat→sats conversion, sats-range validation and _validationError state; sanitize custom payment-method input; preflight checks block submission when exchange rate or Mostro instance missing; wire validation into AmountSection; add repository/Mostro imports.
AmountSection widget API / UI
lib/features/order/widgets/amount_section.dart
Add final String? Function(double)? validateSatsRange and final String? validationError; update constructor; simplify numeric validators to format-only; render validation message block inside card when validationError present.
ActionButtons (submit nullable)
lib/features/order/widgets/action_buttons.dart
Make onSubmit nullable (VoidCallback?), constructor param optional, and toggle submit visuals/enabled state based on onSubmit != null.
MostroReactiveButton (onPressed nullable)
lib/shared/widgets/mostro_reactive_button.dart
Make onPressed nullable (VoidCallback?); early-return if null; disable button when loading or onPressed == null.
MostroMessage small refactor
lib/data/models/mostro_message.dart
Use direct assignment tradeIndex = keyIndex; instead of this.tradeIndex = keyIndex; in wrap.
Mostro storage timestamp refactor
lib/data/repositories/mostro_storage.dart
Replace explicit null-check assignment with null-coalescing assignment message.timestamp ??= DateTime.now().millisecondsSinceEpoch.
Switch thumb color adjustments & Privacy widget deletion
lib/features/auth/screens/register_screen.dart, lib/features/key_manager/key_management_screen.dart, lib/features/order/widgets/price_type_section.dart, lib/shared/widgets/privacy_switch_widget.dart
Replace activeColor with activeThumbColor for Switch widgets (UI-only color positioning). Note: lib/shared/widgets/privacy_switch_widget.dart was deleted (PrivacySwitch widget removed).
Localization (EN/ES/IT) & l10n config
lib/l10n/intl_en.arb, lib/l10n/intl_es.arb, lib/l10n/intl_it.arb, l10n.yaml
Add fiatAmountTooHigh, fiatAmountTooLow (with minAmount/maxAmount placeholders), exchangeRateNotAvailable, and mostroInstanceNotAvailable keys; remove explicit synthetic-package: false from l10n.yaml.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant ASec as AmountSection
  participant AOS as AddOrderScreen
  participant ERP as ExchangeRateProvider
  participant ORP as OrderRepositoryProvider

  U->>ASec: Enter fiat amount
  ASec->>AOS: onAmountChanged(fiat)
  AOS->>AOS: _calculateSatsFromFiat & _validateSatsRange
  alt rate or Mostro instance missing
    AOS-->>ASec: set validationError (exchange/instance not available)
    ASec-->>U: show validation message
  else sats outside allowed range
    AOS-->>ASec: set validationError (too low/high)
    ASec-->>U: show validation message
  else valid
    AOS-->>ASec: clear validationError
  end

  U->>AOS: Tap Submit
  AOS->>ERP: request exchange rate
  AOS->>ORP: request Mostro instance
  alt preflight fails
    AOS-->>U: show SnackBar (exchange/instance unavailable) — block submit
  else preflight ok
    AOS->>AOS: check _validationError
    alt validationError present
      AOS-->>U: block submit
    else
      AOS->>AOS: proceed with submission
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • Catrya

Poem

I nibble numbers, count the sats so bright,
I sniff the rates, and check by night.
If nodes go quiet or rates delay,
I thump a warning, halt the way.
Hop on—orders safe and light. 🥕🐇

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/validates-fiat-amount-in-range

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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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: 0

🧹 Nitpick comments (10)
android/app/build.gradle (1)

29-29: Use the modern AGP DSL: switch to minSdk = ... (not minSdkVersion)

You're already using the new DSL for compileSdk and targetSdk. For consistency with AGP 8+, prefer minSdk = flutter.minSdkVersion.

-        minSdkVersion flutter.minSdkVersion // flutter.minSdkVersion
+        minSdk = flutter.minSdkVersion // flutter.minSdkVersion
lib/features/order/widgets/amount_section.dart (2)

130-141: Consider supporting decimal fiat inputs and aligning numeric validation

Currently the min validator checks int.tryParse(value) earlier, but the callback is fed a double. If you plan to allow decimal fiat amounts, unify validation to double and allow decimal input.

If you decide to support decimals, update these (outside this hunk):

  • Change validators to use double.tryParse(...) instead of int.tryParse(...) for both min/max numeric checks.
  • Allow decimals in the input:
// Replace digitsOnly with a decimal-friendly formatter
inputFormatters: [
  FilteringTextInputFormatter.allow(RegExp(r'[0-9]+[.,]?[0-9]*')),
],
keyboardType: const TextInputType.numberWithOptions(decimal: true),

Also normalize commas to dots before parsing:

final normalized = value.replaceAll(',', '.');
final fiatAmount = double.tryParse(normalized);

158-169: Mirror the decimal/validation adjustments for the max field

Same reasoning as for min: if decimals are desired, use double.tryParse in the numeric check and allow decimal input in the field’s formatter and keyboard type.

lib/features/order/screens/add_order_screen.dart (7)

95-101: Guard for non-positive exchange rates should surface a user-facing availability error (not “too low”)

Currently, _calculateSatsFromFiat returns 0 for non-positive rates, which can mislead users with a “too low” message instead of an “exchange rate unavailable” message during validation. Prefer handling the non-positive rate in the validator to show the correct message.

Apply this diff to defer non-positive rate handling to the validator:

 int _calculateSatsFromFiat(double fiatAmount, double exchangeRate) {
-  if (exchangeRate <= 0) return 0;
   return (fiatAmount / exchangeRate * 100000000).round();
 }

And then add the rate check in the validator (see separate suggestion below for lines 111–124).


102-124: Simplify AsyncValue handling and treat non-positive exchange rate as unavailable

You can avoid multiple branches (loading/error/fallback) since all return the same string. Also treat exchangeRate <= 0 as unavailable to avoid mislabeling range errors.

Apply this diff:

   // Get exchange rate - return error if not available
   final exchangeRateAsync = ref.read(exchangeRateProvider(selectedFiatCode));
-  final exchangeRate = exchangeRateAsync.asData?.value;
-  if (exchangeRate == null) {
-    // Check if it's loading or error
-    if (exchangeRateAsync.isLoading) {
-      return S.of(context)!.exchangeRateNotAvailable;
-    } else if (exchangeRateAsync.hasError) {
-      return S.of(context)!.exchangeRateNotAvailable;
-    }
-    // Fallback for any other case where rate is null
-    return S.of(context)!.exchangeRateNotAvailable;
-  }
+  final exchangeRate = exchangeRateAsync.asData?.value;
+  if (exchangeRate == null || exchangeRate <= 0) {
+    return S.of(context)!.exchangeRateNotAvailable;
+  }

136-138: Guard debug logs behind kDebugMode

Avoid noisy logs in release by guarding debugPrint. This also aligns with privacy practices for user input.

Apply this diff:

-    debugPrint('Validation: fiat=$fiatAmount, rate=$exchangeRate, sats=$satsAmount, min=$minAllowed, max=$maxAllowed');
+    if (kDebugMode) {
+      debugPrint('Validation: fiat=$fiatAmount, rate=$exchangeRate, sats=$satsAmount, min=$minAllowed, max=$maxAllowed');
+    }

Add this import at the top of the file (outside the selected lines):

import 'package:flutter/foundation.dart';

139-151: Optional: Localize number formatting for min/max sats in error messages

Passing raw integers can be hard to read (e.g., 100000). Consider formatting using locale-aware separators.

Example change within this block:

-      return S.of(context)!.fiatAmountTooLow(
-        minAllowed.toString(),
-        maxAllowed.toString(),
-      );
+      final nf = NumberFormat.decimalPattern(Localizations.localeOf(context).toString());
+      return S.of(context)!.fiatAmountTooLow(
+        nf.format(minAllowed),
+        nf.format(maxAllowed),
+      );

And similarly for the “too high” branch. Add the import (outside the selected lines):

import 'package:intl/intl.dart';

340-371: Submission blocking on range errors: correct, but prefer unifying error surface with field-level validation

This final gate is good. Minor UX tweak: if only one bound is invalid, consider programmatically focusing/scolling to that field to guide the user. Also, to avoid duplicated strings, consider reusing a small helper to show SnackBars for these cases.

Example minimal refactor inside this block:

-          ScaffoldMessenger.of(context).showSnackBar(
-            SnackBar(
-              content: Text(minRangeError),
-              duration: const Duration(seconds: 4),
-              backgroundColor: Colors.red.withValues(alpha: 0.8),
-            ),
-          );
+          _showErrorSnackBar(minRangeError);

Then outside the block add:

void _showErrorSnackBar(String message) {
  ScaffoldMessenger.of(context).showSnackBar(
    SnackBar(
      content: Text(message),
      duration: const Duration(seconds: 4),
      backgroundColor: Colors.red.withValues(alpha: 0.8),
    ),
  );
}

372-386: Show the correct preflight message (exchange rate vs Mostro instance) and perform a stronger rate validity check

If mostroInstance is null, the code still shows exchangeRateNotAvailable. Also, checking hasValue alone may pass a zero/negative rate. Pick the message based on what’s missing, and treat non-positive rates as unavailable.

Apply this diff:

-      final exchangeRateAsync = ref.read(exchangeRateProvider(fiatCode));
-      final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance;
-      
-      if (!exchangeRateAsync.hasValue || mostroInstance == null) {
-        debugPrint('Submission blocked: Required data not available - Exchange rate: ${exchangeRateAsync.hasValue}, Mostro instance: ${mostroInstance != null}');
-        ScaffoldMessenger.of(context).showSnackBar(
-          SnackBar(
-            content: Text(S.of(context)!.exchangeRateNotAvailable),
-            duration: const Duration(seconds: 3),
-            backgroundColor: Colors.orange.withValues(alpha: 0.8),
-          ),
-        );
-        return;
-      }
+      final exchangeRateAsync = ref.read(exchangeRateProvider(fiatCode));
+      final mostroInstance = ref.read(orderRepositoryProvider).mostroInstance;
+      final rate = exchangeRateAsync.asData?.value;
+      final hasValidRate = rate != null && rate > 0;
+      final hasMostroInstance = mostroInstance != null;
+
+      if (!hasValidRate || !hasMostroInstance) {
+        if (kDebugMode) {
+          debugPrint(
+            'Submission blocked: Required data not available - Exchange rate valid: $hasValidRate, Mostro instance: $hasMostroInstance',
+          );
+        }
+        final msg = !hasValidRate
+            ? S.of(context)!.exchangeRateNotAvailable
+            : S.of(context)!.mostroInstanceNotAvailable;
+        ScaffoldMessenger.of(context).showSnackBar(
+          SnackBar(
+            content: Text(msg),
+            duration: const Duration(seconds: 3),
+            backgroundColor: Colors.orange.withValues(alpha: 0.8),
+          ),
+        );
+        return;
+      }

Note: This uses kDebugMode; ensure you have import 'package:flutter/foundation.dart'; at the top.


18-20: Remove unused mostro_instance.dart import

The MostroInstance symbol isn’t referenced in lib/features/order/screens/add_order_screen.dart, so you can drop this line:

- import 'package:mostro_mobile/features/mostro/mostro_instance.dart';

• Confirm that settings_provider.dart is still required in this file; if not, remove its import as well.

📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between eb297d9 and 82b9770.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • android/app/build.gradle (1 hunks)
  • lib/features/order/screens/add_order_screen.dart (6 hunks)
  • lib/features/order/widgets/amount_section.dart (3 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
lib/l10n/intl_{en,es,it}.arb

📄 CodeRabbit Inference Engine (CLAUDE.md)

When adding a localization key, add it to all three ARB files: intl_en.arb, intl_es.arb, and intl_it.arb

Files:

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

📄 CodeRabbit Inference Engine (CLAUDE.md)

After modifying ARB files, regenerate localization outputs (e.g., dart run build_runner build -d or flutter gen-l10n)

Files:

  • lib/l10n/intl_es.arb
  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
lib/**/@(screens|widgets)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods

Files:

  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/amount_section.dart
lib/!(generated)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/amount_section.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/

Files:

  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/amount_section.dart
🧠 Learnings (1)
📚 Learning: 2025-08-15T13:19:31.999Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-15T13:19:31.999Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : When adding a localization key, add it to all three ARB files: intl_en.arb, intl_es.arb, and intl_it.arb

Applied to files:

  • lib/l10n/intl_es.arb
  • lib/l10n/intl_en.arb
🔇 Additional comments (8)
lib/l10n/intl_en.arb (1)

372-399: Localization strings validated – ready to generate code

All new keys (fiatAmountTooHigh, fiatAmountTooLow, exchangeRateNotAvailable, mostroInstanceNotAvailable) are referenced in the Dart code as expected. Please regenerate the localization outputs so the generated code is in sync with your ARB changes:

  • If you use Flutter’s tool:
    flutter gen-l10n
  • If you use build_runner:
    dart run build_runner build -d
lib/l10n/intl_it.arb (1)

333-361: Italian translations added with correct placeholders

Parity with EN/ES is maintained; placeholder metadata aligns across languages.

lib/features/order/widgets/amount_section.dart (1)

10-17: Validation callback wiring verified

  • AddOrderScreen at lib/features/order/screens/add_order_screen.dart:202-206 correctly passes validateSatsRange: _validateSatsRange.
  • No other AmountSection instantiations found.
lib/l10n/intl_es.arb (1)

328-356: Spanish translations and metadata are consistent with EN/IT

Placeholders and messages correctly match the validation flow.

After merging, regenerate l10n outputs to include these keys in generated classes:

  • flutter gen-l10n
lib/features/order/screens/add_order_screen.dart (4)

66-76: Good use of post-frame callback for initial state setup

Resetting selected fiat code from settings and pre-filling the lightning address inside addPostFrameCallback adheres to the UI side-effect guideline. Looks solid.


205-206: Nice: Live, contextual validation wiring into AmountSection

Passing validateSatsRange enables immediate, localized feedback. Good UX alignment with the PR goals.


248-249: Good: Using Color.withValues instead of withOpacity

You’re using the latest API per the guidelines. No action needed.


416-426: Input sanitization for custom payment methods looks good

Replacing problematic characters and collapsing whitespace is a sensible approach. This matches prior patterns and avoids breaking serialization.

All these errors now display in the same red widget at bottom of card:

1. Empty field: "Please enter an amount"
2. Max ≤ Min: "Max must be greater than min" ✅ Fixed!
3. Too low sats: "The fiat amount is too low, please add a new fiat amount, the allowed range is
between X and Y sats"
4. Too high sats: "The fiat amount is too high, please add a new fiat amount, the allowed range is
  between X and Y sats"
5. Exchange rate unavailable: "Exchange rate not available, please wait or refresh"
6. Mostro instance unavailable: "Mostro instance data not available, please wait"
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

🧹 Nitpick comments (6)
lib/features/order/widgets/amount_section.dart (3)

11-13: Remove unused validateSatsRange prop or wire it up.

The widget accepts validateSatsRange but never invokes it. This widens the public API without behavior. Either wire it (e.g., call in _onMinAmountChanged/_onMaxAmountChanged) or remove it to avoid confusion and reduce maintenance.

If you decide to remove it now (the screen already handles validation via validationError), apply:

 class AmountSection extends StatefulWidget {
   final OrderType orderType;
   final Function(int? minAmount, int? maxAmount) onAmountChanged;
-  final String? Function(double)? validateSatsRange;
   final String? validationError;

   const AmountSection({
     super.key,
     required this.orderType,
     required this.onAmountChanged,
-    this.validateSatsRange,
     this.validationError,
   });

And update the call site in lib/features/order/screens/add_order_screen.dart:

 AmountSection(
   orderType: _orderType,
   onAmountChanged: _onAmountChanged,
-  validateSatsRange: _validateSatsRange,
   validationError: _validationError,
 ),

Also applies to: 18-20


126-130: Avoid returning empty string from validator; hide field-level error UI instead.

Returning '' marks the field invalid and renders an empty error space. Since FilteringTextInputFormatter.digitsOnly enforces numeric input, this validator will almost never trigger; if you still want the red error state without a message, hide the error layout to prevent layout jitter.

Consider adding errorStyle to the InputDecoration of the min field:

decoration: InputDecoration(
  border: InputBorder.none,
  hintText: S.of(context)!.enterAmountHint,
  hintStyle: const TextStyle(color: Colors.grey),
  errorStyle: const TextStyle(height: 0),
),

135-139: Same as min: suppress empty error text layout for max field.

Mirror the min-input fix by adding errorStyle: const TextStyle(height: 0) to the max field’s InputDecoration.

lib/features/order/screens/add_order_screen.dart (3)

106-159: Validation path is solid; consider one edge-case tweak.

The flow correctly handles currency selection, exchange-rate readiness, and instance availability. One small improvement: treat non-positive exchange rates as “not available” instead of converting to 0 sats, which currently yields a “too low” error.

You can short-circuit in _validateSatsRange:

-    final satsAmount = _calculateSatsFromFiat(fiatAmount, exchangeRate);
+    if (exchangeRate <= 0) {
+      return S.of(context)!.exchangeRateNotAvailable;
+    }
+    final satsAmount = _calculateSatsFromFiat(fiatAmount, exchangeRate);

242-244: Unused prop passed to AmountSection.

validateSatsRange is passed but not used by AmountSection. Either wire it in the widget or omit here to keep the API tight. See widget review for removal diff.


386-401: Clarify availability SnackBar and re-validate just-in-time.

  • Message precision: If rates are fine but Mostro instance is null, the SnackBar still says exchangeRateNotAvailable. Prefer a targeted message.
  • Just-in-time validation: Recompute _validationError right before submit to capture any rate/limits changes since the user last typed.

Suggested refinements:

-      if (!exchangeRateAsync.hasValue || mostroInstance == null) {
-        debugPrint('Submission blocked: Required data not available - Exchange rate: ${exchangeRateAsync.hasValue}, Mostro instance: ${mostroInstance != null}');
-        ScaffoldMessenger.of(context).showSnackBar(
-          SnackBar(
-            content: Text(S.of(context)!.exchangeRateNotAvailable),
-            duration: const Duration(seconds: 3),
-            backgroundColor: Colors.orange.withValues(alpha: 0.8),
-          ),
-        );
-        return;
-      }
+      // Re-validate just-in-time in case rates/limits changed since last keystroke
+      final latestValidation = _validateAllAmounts();
+      if (latestValidation != null) {
+        setState(() => _validationError = latestValidation);
+        debugPrint('Submission blocked: Validation error present: $_validationError');
+        return;
+      }
+
+      if (!exchangeRateAsync.hasValue || mostroInstance == null) {
+        debugPrint(
+            'Submission blocked: Required data not available - Exchange rate: ${exchangeRateAsync.hasValue}, Mostro instance: ${mostroInstance != null}');
+        final msg = !exchangeRateAsync.hasValue
+            ? S.of(context)!.exchangeRateNotAvailable
+            : S.of(context)!.mostroInstanceNotAvailable;
+        ScaffoldMessenger.of(context).showSnackBar(
+          SnackBar(
+            content: Text(msg),
+            duration: const Duration(seconds: 3),
+            backgroundColor: Colors.orange.withValues(alpha: 0.8),
+          ),
+        );
+        return;
+      }
📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 82b9770 and 0993168.

📒 Files selected for processing (2)
  • lib/features/order/screens/add_order_screen.dart (7 hunks)
  • lib/features/order/widgets/amount_section.dart (3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/@(screens|widgets)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods

Files:

  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/amount_section.dart
lib/!(generated)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/amount_section.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/

Files:

  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/amount_section.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (11)
lib/features/order/widgets/amount_section.dart (3)

3-3: LGTM: Centralized theming and modern Color API.

Good use of AppTheme and Color.withValues to keep styling consistent and future-proof.


142-177: LGTM: Clear, themed inline validation block.

Good UX choice to centralize error display with consistent colors and iconography. Implementation is clean and readable.


252-256: LGTM: Inline validation rendering is well-integrated.

Conditional rendering inside the card keeps the layout stable and the error visible where it matters.

lib/features/order/screens/add_order_screen.dart (8)

43-46: LGTM: Centralized validation error state.

Adding _validationError simplifies gating submission and passing a single message to the AmountSection for display.


69-75: LGTM: Safe post-frame initialization.

Resetting selectedFiatCode from settings and pre-filling the lightning address post-frame avoids build-time side effects. No async gap here, so no mounted check needed.


93-96: LGTM: Recompute validation on amount changes.

Synchronizing _validationError with amount edits ensures the UI/submit gating stays consistent in real time.


99-105: LGTM: Fiat→sats conversion formula matches spec.

Implements sats = fiatAmount / exchangeRate * 1e8 with rounding and guards against non-positive rates.


160-189: LGTM: Comprehensive, ordered validations.

Covers required fields, range relationship, then range validation per bound. Straightforward and readable.


378-385: LGTM: Submission gated on validation state.

Short-circuiting when _validationError is present avoids duplicate messaging and keeps the user feedback inline.


431-438: LGTM: Payment method sanitization is prudent.

Good normalization and removal of problematic characters; preserves readability and safety for server-side parsing.


18-20: Remove unused mostro_instance.dart import

The import in lib/features/order/screens/add_order_screen.dart at line 19 isn’t referenced in the file and can be removed to keep the code tidy.

• File: lib/features/order/screens/add_order_screen.dart, line 19

- import 'package:mostro_mobile/features/mostro/mostro_instance.dart';

grunch added 2 commits August 20, 2025 11:16
Now when validation errors exist (like sats amount out of range), the submit button:
  - ✅ Appears disabled (grey background, inactive text color)
  - ✅ Does NOT show loading state when tapped
  - ✅ Remains truly disabled until validation passes
  - ✅ Follows the exact same behavior as payment method validation
 switch to minSdk = ... (not minSdkVersion)
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

♻️ Duplicate comments (1)
lib/features/order/screens/add_order_screen.dart (1)

232-235: Re-run validation on currency change (duplicate of prior review).

Clearing the error hides issues for the new currency/rate. Recompute instead.

-                            setState(() {
-                              // Clear validation error when currency changes since rates may be different
-                              _validationError = null;
-                            });
+                            setState(() {
+                              // Re-validate after currency change since rates/limits differ
+                              _validationError = _validateAllAmounts();
+                            });
🧹 Nitpick comments (9)
android/app/build.gradle (1)

10-10: Optional: Also source compileSdk from Flutter plugin for consistency.

You can reduce drift by using flutter.compileSdkVersion.

Apply:

-    compileSdk = 35 // flutter.compileSdkVersion
+    compileSdk = flutter.compileSdkVersion
lib/shared/widgets/mostro_reactive_button.dart (1)

32-32: Nullable onPressed with a required parameter is confusing.

The field is VoidCallback? but the constructor marks it required. Consider making the named param optional for cleaner call sites.

   const MostroReactiveButton({
     super.key,
     required this.label,
     required this.buttonStyle,
-    required this.onPressed,
+    this.onPressed,
     required this.orderId,
     required this.action,
lib/features/order/widgets/action_buttons.dart (1)

9-9: onSubmit nullable but required in constructor — simplify API.

For clarity, drop required so callers don't need to explicitly pass null.

   const ActionButtons({
     super.key,
     required this.onCancel,
-    required this.onSubmit,
+    this.onSubmit,
     required this.currentRequestId,
   });
lib/features/order/screens/add_order_screen.dart (6)

18-20: Remove unused import.

mostro_instance.dart isn’t referenced directly; keeping it violates “Remove unused imports”.

 import 'package:mostro_mobile/shared/providers/order_repository_provider.dart';
-import 'package:mostro_mobile/features/mostro/mostro_instance.dart';

54-77: Add mounted check inside post-frame callback.

The callback may fire after dispose on rapid navigations; guard to avoid setState/context usage when unmounted.

     WidgetsBinding.instance.addPostFrameCallback((_) {
+      if (!mounted) return;
       final GoRouterState state = GoRouterState.of(context);
       final extras = state.extra;
@@
       // Pre-populate lightning address from settings if available
       if (settings.defaultLightningAddress != null &&
           settings.defaultLightningAddress!.isNotEmpty) {
         _lightningAddressController.text = settings.defaultLightningAddress!;
       }
     });

106-158: Solid range validation; consider re-validating on rate updates.

If the exchange rate transitions from loading→data, _validationError may remain stale until the user types again. Optionally listen to the current fiat’s exchangeRateProvider and re-run _validateAllAmounts() when it changes.

If you choose to add it, a simple approach (pseudo-diff) in build:

// near build():
final selectedFiat = ref.watch(selectedFiatCodeProvider);
ref.listen(exchangeRateProvider(selectedFiat ?? ''), (_, __) {
  if (!mounted) return;
  setState(() => _validationError = _validateAllAmounts());
});

Ensure you handle the null/empty fiat code case appropriately.


351-375: Deduplicate _validationError checks and streamline enabling logic.

The double-check adds noise; condense for readability.

   VoidCallback? _getSubmitCallback() {
-    // Don't allow submission if validation errors exist
-    if (_validationError != null) {
-      return null; // Disables button, prevents loading state
-    }
-
-    // Check other basic conditions that would prevent submission
+    // Block when validation fails or required context is missing
+    if (_validationError != null) return null;
     final selectedFiatCode = ref.read(selectedFiatCodeProvider);
-    if (selectedFiatCode == null || selectedFiatCode.isEmpty) {
-      return null;
-    }
-
-    if (_selectedPaymentMethods.isEmpty) {
-      return null;
-    }
-
-    if (_validationError != null) {
-      return null;
-    }
+    if (selectedFiatCode == null || selectedFiatCode.isEmpty) return null;
+    if (_selectedPaymentMethods.isEmpty) return null;
 
     return _submitOrder; // Form is valid - allow submission
   }

403-410: Re-run final validation at submit time to avoid stale state.

Rates/limits may change between last keystroke and tap. Recompute before blocking/allowing submission.

-      // Enhanced validation: check sats range for both min and max amounts
-      // This is a critical final validation before submission
-      if (_validationError != null) {
+      // Final validation: recompute in case context changed since last keystroke
+      setState(() {
+        _validationError = _validateAllAmounts();
+      });
+      if (_validationError != null) {
         debugPrint('Submission blocked: Validation error present: $_validationError');
         // Validation error is already displayed inline, just prevent submission
         return;
       }

411-425: Use a specific error message for missing Mostro instance.

Currently both missing-rate and missing-instance show exchangeRateNotAvailable, which is misleading.

-      if (!exchangeRateAsync.hasValue || mostroInstance == null) {
-        debugPrint('Submission blocked: Required data not available - Exchange rate: ${exchangeRateAsync.hasValue}, Mostro instance: ${mostroInstance != null}');
-        ScaffoldMessenger.of(context).showSnackBar(
-          SnackBar(
-            content: Text(S.of(context)!.exchangeRateNotAvailable),
-            duration: const Duration(seconds: 3),
-            backgroundColor: Colors.orange.withValues(alpha: 0.8),
-          ),
-        );
-        return;
-      }
+      if (!exchangeRateAsync.hasValue) {
+        debugPrint('Submission blocked: Exchange rate not available');
+        ScaffoldMessenger.of(context).showSnackBar(
+          SnackBar(
+            content: Text(S.of(context)!.exchangeRateNotAvailable),
+            duration: const Duration(seconds: 3),
+            backgroundColor: Colors.orange.withValues(alpha: 0.8),
+          ),
+        );
+        return;
+      }
+      if (mostroInstance == null) {
+        debugPrint('Submission blocked: Mostro instance not available');
+        ScaffoldMessenger.of(context).showSnackBar(
+          SnackBar(
+            content: Text(S.of(context)!.mostroInstanceNotAvailable),
+            duration: const Duration(seconds: 3),
+            backgroundColor: Colors.orange.withValues(alpha: 0.8),
+          ),
+        );
+        return;
+      }
📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0993168 and 41b6abd.

📒 Files selected for processing (4)
  • android/app/build.gradle (1 hunks)
  • lib/features/order/screens/add_order_screen.dart (9 hunks)
  • lib/features/order/widgets/action_buttons.dart (2 hunks)
  • lib/shared/widgets/mostro_reactive_button.dart (5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/@(screens|widgets)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods

Files:

  • lib/shared/widgets/mostro_reactive_button.dart
  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/action_buttons.dart
lib/!(generated)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/shared/widgets/mostro_reactive_button.dart
  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/action_buttons.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/

Files:

  • lib/features/order/screens/add_order_screen.dart
  • lib/features/order/widgets/action_buttons.dart
🔇 Additional comments (8)
android/app/build.gradle (1)

29-29: LGTM: minSdk now sourced from Flutter plugin.

Using flutter.minSdkVersion keeps Android config aligned with Flutter toolchain settings.

lib/shared/widgets/mostro_reactive_button.dart (2)

90-97: Good null-guard before invoking onPressed.

Early-return avoids unnecessary state transitions when disabled and the ! cast is safe after the guard.


160-160: Button enable/disable logic is correct and consistent across styles.

Disabling on _loading or onPressed == null aligns with Material conventions and your updated API.

Also applies to: 176-176, 189-189

lib/features/order/widgets/action_buttons.dart (1)

53-56: Nice: submit button visuals reflect enabled state.

Conditioning success indicator and colors on onSubmit != null makes the UX consistent with the disabled callback.

lib/features/order/screens/add_order_screen.dart (4)

93-96: Good: centralizing validation on amount changes.

Updating _validationError via _validateAllAmounts() avoids fragmented validation logic.


99-105: Conversion helper is correct and rounded appropriately.

sats = fiat / rate * 1e8 with a non-positive-rate guard is sound.


242-244: Verify AmountSection consumes the new API as intended.

You pass both validateSatsRange and validationError. Ensure AmountSection either calls the callback or displays validationError so the user sees immediate feedback.

Would you like me to scan amount_section.dart to confirm the message rendering and callback usage?


456-456: Payment method sanitization looks good.

Removing localized “Other”, stripping problematic chars, collapsing whitespace, and trimming should prevent malformed payloads.

Also applies to: 458-463

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/features/order/screens/add_order_screen.dart (1)

377-387: Recompute validation just before submission to avoid stale decisions.

Rates/limits can change between keystrokes and tap. Re-run _validateAllAmounts() at the start of _submitOrder so _validationError reflects the latest data.

Apply this diff:

   void _submitOrder() {
+    // Final preflight: re-validate in case rates/limits changed recently
+    setState(() {
+      _validationError = _validateAllAmounts();
+    });
+
     if (_formKey.currentState?.validate() ?? false) {
♻️ Duplicate comments (1)
lib/features/order/screens/add_order_screen.dart (1)

233-236: Resolved: re-validation on currency change addresses prior review.

You now recompute _validationError on currency changes as previously suggested. This prevents invalid submissions when limits/rates differ across currencies.

🧹 Nitpick comments (9)
android/app/build.gradle (1)

29-29: Nit: remove redundant trailing comment.

// flutter.minSdkVersion appears twice in the same line. Clean up for readability.

Apply this diff:

-        minSdk = flutter.minSdkVersion // flutter.minSdkVersion
+        minSdk = flutter.minSdkVersion
lib/features/order/screens/add_order_screen.dart (8)

18-19: Confirm necessity of mostro_instance.dart import.

The file doesn’t reference the MostroInstance type explicitly. If the import is unused, remove it per guidelines; otherwise, keep it.

If unused, apply:

-import 'package:mostro_mobile/features/mostro/mostro_instance.dart';

99-105: Define a shared constant for sats-per-BTC and guard against invalid rates.

Minor: replace the magic number with a named constant for clarity and reuse.

Apply this diff here and add a file-level constant:

+static const int _kSatsPerBtc = 100000000;
@@
-  int _calculateSatsFromFiat(double fiatAmount, double exchangeRate) {
-    if (exchangeRate <= 0) return 0;
-    return (fiatAmount / exchangeRate * 100000000).round();
-  }
+  int _calculateSatsFromFiat(double fiatAmount, double exchangeRate) {
+    if (exchangeRate <= 0) return 0;
+    return (fiatAmount / exchangeRate * _kSatsPerBtc).round();
+  }

115-127: UX: avoid surfacing “rate not available” while the rate is still loading.

Returning an error during isLoading briefly shows an error before the rate arrives. Prefer returning null (no error) during loading and only fail on actual error/absence.

Apply this diff:

-    if (exchangeRate == null) {
-      // Check if it's loading or error
-      if (exchangeRateAsync.isLoading) {
-        return S.of(context)!.exchangeRateNotAvailable;
-      } else if (exchangeRateAsync.hasError) {
-        return S.of(context)!.exchangeRateNotAvailable;
-      }
-      // Fallback for any other case where rate is null
-      return S.of(context)!.exchangeRateNotAvailable;
-    }
+    if (exchangeRate == null) {
+      if (exchangeRateAsync.isLoading) {
+        // Defer validation error until the rate actually fails to load
+        return null;
+      }
+      return S.of(context)!.exchangeRateNotAvailable;
+    }

140-143: Nit: remove verbose debug logs or guard them for debug-only.

Consider gating with kDebugMode or removing before release to keep logs quiet.

Apply this diff:

-    debugPrint(
-        'Validation: fiat=$fiatAmount, rate=$exchangeRate, sats=$satsAmount, min=$minAllowed, max=$maxAllowed');
+    assert(() {
+      debugPrint(
+          'Validation: fiat=$fiatAmount, rate=$exchangeRate, sats=$satsAmount, min=$minAllowed, max=$maxAllowed');
+      return true;
+    }());

161-190: Validation flow looks solid; consider formatting numbers for readability.

Comprehensive checks (presence, min<max, min/max sats). As a UX polish, format minAllowed/maxAllowed with locale-aware separators to improve readability in messages.

If desired, add:

// At top of file
import 'package:intl/intl.dart';

// In _validateSatsRange before returning messages
final formatter = NumberFormat.decimalPattern();
final minText = formatter.format(minAllowed);
final maxText = formatter.format(maxAllowed);

Then pass minText and maxText to the localized messages.


352-376: Remove the duplicate validation check; it’s redundant.

_validationError is checked at the top and again at lines 370-372. Keep a single check for clarity.

Apply this diff:

   VoidCallback? _getSubmitCallback() {
     // Don't allow submission if validation errors exist
     if (_validationError != null) {
       return null; // Disables button, prevents loading state
     }
@@
     if (_selectedPaymentMethods.isEmpty) {
       return null;
     }
-
-    if (_validationError != null) {
-      return null;
-    }
 
     return _submitOrder; // Form is valid - allow submission
   }

413-429: Show a specific message when the Mostro instance is unavailable.

Currently you always display exchangeRateNotAvailable. If the missing piece is the Mostro instance, use the dedicated localization.

Apply this diff:

-      if (!exchangeRateAsync.hasValue || mostroInstance == null) {
+      if (!exchangeRateAsync.hasValue || mostroInstance == null) {
         debugPrint(
             'Submission blocked: Required data not available - Exchange rate: ${exchangeRateAsync.hasValue}, Mostro instance: ${mostroInstance != null}');
-        ScaffoldMessenger.of(context).showSnackBar(
+        final msg = mostroInstance == null
+            ? S.of(context)!.mostroInstanceNotAvailable
+            : S.of(context)!.exchangeRateNotAvailable;
+        ScaffoldMessenger.of(context).showSnackBar(
           SnackBar(
-            content: Text(S.of(context)!.exchangeRateNotAvailable),
+            content: Text(msg),
             duration: const Duration(seconds: 3),
             backgroundColor: Colors.orange.withValues(alpha: 0.8),
           ),
         );
         return;
       }

404-411: Minor: consider surfacing the inline error on submit (scroll or SnackBar).

You silently return when an inline error exists. Optionally, scroll to the Amount section or show a brief SnackBar to draw attention.

Example:

if (_validationError != null) {
  _scrollController.animateTo(
    0, // or the offset where AmountSection starts
    duration: const Duration(milliseconds: 300),
    curve: Curves.easeOut,
  );
  return;
}
📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 41b6abd and 2a292cf.

📒 Files selected for processing (4)
  • android/app/build.gradle (2 hunks)
  • lib/features/order/screens/add_order_screen.dart (9 hunks)
  • lib/features/order/widgets/action_buttons.dart (2 hunks)
  • lib/shared/widgets/mostro_reactive_button.dart (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • lib/features/order/widgets/action_buttons.dart
  • lib/shared/widgets/mostro_reactive_button.dart
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/@(screens|widgets)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods

Files:

  • lib/features/order/screens/add_order_screen.dart
lib/!(generated)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/features/order/screens/add_order_screen.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/

Files:

  • lib/features/order/screens/add_order_screen.dart
🔇 Additional comments (7)
android/app/build.gradle (1)

10-12: Good move: delegate SDK/NDK versions to Flutter.

Using flutter.compileSdkVersion and flutter.ndkVersion keeps Android config in sync with the Flutter toolchain and reduces maintenance.

lib/features/order/screens/add_order_screen.dart (6)

69-75: LGTM: initialize currency and pre-fill lightning address post-frame.

Using a post-frame callback avoids build-time side effects, and setting selectedFiatCode from settings keeps UX consistent across new orders.


93-96: LGTM: revalidate on amount changes.

Updating _validationError via _validateAllAmounts() on amount edits ensures real-time feedback and keeps the submit button state accurate.


130-139: LGTM: validate against Mostro instance min/max and compute sats from fiat.

Reads limits via orderRepositoryProvider and applies the documented conversion formula correctly. The localized “too low/high” messages match the PR goals.


243-245: LGTM: wire validator into AmountSection for inline errors.

Passing validateSatsRange and the current validationError enables real-time, localized messaging in the UI.


336-337: LGTM: use a nullable submit callback to control button enablement.

Returning null keeps the button disabled and prevents entering the loading state when validation fails.


496-505: LGTM: use Color.withValues instead of withOpacity.

Using withValues(alpha: …) is aligned with modern Flutter APIs.

grunch added 2 commits August 20, 2025 15:51
1. Added validation check to _getSubmitCallback() - Now returns null when validation errors exist
2. Updated MostroReactiveButton - Properly handles null callbacks without triggering loading state

3. Cleaned up ActionButtons - Removed workaround no-op callback

Result: When sats validation errors are present, the submit button is truly disabled and does NOT
show loading state when tapped, exactly like when payment methods aren't selected.

🔧 Code Quality: All Analyzer Issues Fixed

1. ✅ Removed unnecessary this. qualifier
2. ✅ Simplified conditional assignment
3. ✅ Updated 4 deprecated activeColor to activeThumbColor in Switch widgets

Result: Clean codebase with zero analyzer issues (No issues found!)
from l10n.yaml configuration file. This eliminates the deprecation warning
while maintaining all existing localization functionality.
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 (7)
lib/features/order/widgets/price_type_section.dart (3)

53-59: Consider centralizing Switch styling via SwitchTheme.

To avoid repeating colors on every Switch and ensure consistency, move thumb/track colors into ThemeData.switchTheme (MaterialStateProperty-based) and drop per-widget overrides where possible.

Would you like a small patch showing how to define SwitchTheme in your app theme?


54-55: Nit: Key name doesn’t reflect switch semantics.

Key('fixedSwitch') reads confusing given the value is isMarketRate. Consider renaming to Key('marketRateSwitch') for test readability.

-              Switch(
-                key: const Key('fixedSwitch'),
+              Switch(
+                key: const Key('marketRateSwitch'),

19-23: Nit: Use const for priceTypeIcon if AppTheme.textPrimary is const.

If AppTheme.textPrimary is a compile-time const Color, mark the Icon as const to reduce rebuild cost.

-    final priceTypeIcon = Icon(
+    const priceTypeIcon = Icon(
       Icons.attach_money,
       size: 18,
       color: AppTheme.textPrimary,
     );
lib/features/auth/screens/register_screen.dart (1)

185-186: Prefer design-system color over raw Colors.

Use AppTheme to keep styling centralized.

-                        activeThumbColor: Colors.green,
+                        activeThumbColor: AppTheme.activeColor,
lib/shared/widgets/privacy_switch_widget.dart (2)

53-56: Localize user-facing labels.

Avoid hardcoded strings; use S.of(context)!.… keys per project guidelines. Add ARB entries if they don’t exist yet.

-        Text(
-          _isFullPrivacy ? 'Full Privacy' : 'Normal Privacy',
-          style: const TextStyle(color: AppTheme.cream1),
-        ),
+        Text(
+          _isFullPrivacy
+              ? S.of(context)!.fullPrivacy /* add to ARB */
+              : S.of(context)!.normalPrivacy /* add to ARB */,
+          style: const TextStyle(color: AppTheme.cream1),
+        ),

I can draft the ARB keys (EN/ES/IT) if you want.


49-50: Optional: Avoid raw Colors for consistency.

Consider using an AppTheme color for inactiveThumbColor to keep all colors in the design system.

-          inactiveThumbColor: Colors.grey,
+          inactiveThumbColor: AppTheme.textSecondary,
lib/features/key_manager/key_management_screen.dart (1)

60-61: Nit: Typo in variable name.

Rename sessionNotifer → sessionNotifier for readability.

-    final sessionNotifer = ref.read(sessionNotifierProvider.notifier);
-    await sessionNotifer.reset();
+    final sessionNotifier = ref.read(sessionNotifierProvider.notifier);
+    await sessionNotifier.reset();
📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2a292cf and a09d4eb.

📒 Files selected for processing (6)
  • lib/data/models/mostro_message.dart (1 hunks)
  • lib/data/repositories/mostro_storage.dart (1 hunks)
  • lib/features/auth/screens/register_screen.dart (1 hunks)
  • lib/features/key_manager/key_management_screen.dart (1 hunks)
  • lib/features/order/widgets/price_type_section.dart (1 hunks)
  • lib/shared/widgets/privacy_switch_widget.dart (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • lib/data/repositories/mostro_storage.dart
  • lib/data/models/mostro_message.dart
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/@(screens|widgets)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use post-frame callbacks for UI side effects (e.g., SnackBars/dialogs) instead of inline side effects in build methods

Files:

  • lib/shared/widgets/privacy_switch_widget.dart
  • lib/features/order/widgets/price_type_section.dart
  • lib/features/auth/screens/register_screen.dart
lib/!(generated)/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/!(generated)/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Pass BuildContext to methods that need localization rather than accessing localization without context
Use latest, non-deprecated Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Check mounted before using BuildContext after async gaps
Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/shared/widgets/privacy_switch_widget.dart
  • lib/features/order/widgets/price_type_section.dart
  • lib/features/auth/screens/register_screen.dart
  • lib/features/key_manager/key_management_screen.dart
lib/features/*/{screens,providers,notifiers,widgets}/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Follow the feature-based structure: features/{feature}/{screens|providers|notifiers|widgets}/

Files:

  • lib/features/order/widgets/price_type_section.dart
  • lib/features/auth/screens/register_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (3)
lib/features/order/widgets/price_type_section.dart (1)

56-59: Switch color API migration looks good.

Using activeThumbColor aligns with the app-wide switch styling update and keeps the brand color consistent.

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

48-50: Switch styling change matches PR direction.

activeThumbColor with AppTheme.mostroGreen is consistent with the rest of the app’s switch updates.

lib/features/key_manager/key_management_screen.dart (1)

399-402: Thumb color update is consistent and clear.

activeThumbColor: AppTheme.activeColor is in line with the global switch theming changes.

grunch added 3 commits August 20, 2025 16:08
  - ✅ File deleted: /home/gruncho/dev/mobile/lib/shared/widgets/privacy_switch_widget.dart
  - ✅ No breaking changes: Flutter analyze shows "No issues found!"
  - ✅ Clean codebase: Removed dead code that was not being used anywhere
@grunch grunch merged commit 84b2905 into main Aug 20, 2025
1 of 2 checks passed
@grunch grunch deleted the feat/validates-fiat-amount-in-range branch August 20, 2025 19:15
@coderabbitai coderabbitai bot mentioned this pull request Nov 10, 2025
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.

2 participants