Skip to content

fix: reset payment methods when fiat currency changes (#352) - #353

Merged
grunch merged 1 commit into
MostroP2P:mainfrom
abcb1122:fix/reset-payment-methods-on-fiat-change
Oct 30, 2025
Merged

fix: reset payment methods when fiat currency changes (#352)#353
grunch merged 1 commit into
MostroP2P:mainfrom
abcb1122:fix/reset-payment-methods-on-fiat-change

Conversation

@abcb1122

@abcb1122 abcb1122 commented Oct 30, 2025

Copy link
Copy Markdown
Contributor

Fixes #352 by automatically resetting payment methods when fiat currency changes using a Riverpod listener.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed payment method state persisting incorrectly when changing currencies. Payment selections and custom payment fields now properly reset when switching to a different currency, ensuring consistent UI behavior.

- Clear selected payment methods when user changes fiat currency
- Clear custom payment method field and visibility flag
- Prevents invalid payment methods being sent for wrong currency
- Fixes #352
@coderabbitai

coderabbitai Bot commented Oct 30, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

A Riverpod listener was added to monitor the selectedFiatCodeProvider in the add order screen. When the fiat currency changes, the listener resets payment-related state: clears selected payment methods, hides the custom payment method field, and clears the associated input controller. The listener skips the initial load and only executes when the widget is mounted.

Changes

Cohort / File(s) Summary
Payment method reset on currency change
lib/features/order/screens/add_order_screen.dart
Adds a Riverpod listener that monitors selectedFiatCodeProvider changes and resets payment method selections, custom payment field visibility, and controller state when the currency changes (excluding initial load) and the widget is mounted

Sequence Diagram

sequenceDiagram
    participant User
    participant Screen as Add Order Screen
    participant Provider as selectedFiatCodeProvider
    participant Listener as Currency Change Listener
    participant State as _AddOrderScreenState

    User->>Provider: Changes fiat currency
    Provider->>Listener: notifies (previous value → new value)
    alt Previous value exists and widget mounted
        Listener->>State: Clear _selectedPaymentMethods
        Listener->>State: Set _showCustomPaymentMethod = false
        Listener->>State: Clear _customPaymentMethodController
        State->>Screen: Rebuild with reset state
    else First load or widget unmounted
        Listener->>Listener: Skip reset (no-op)
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the listener's condition logic correctly identifies currency changes (previous != null check)
  • Confirm all three state resets (payment methods, custom field visibility, controller) are necessary and sufficient
  • Ensure the mounted check prevents race conditions or errors during widget disposal
  • Check that the listener has proper cleanup to avoid memory leaks

Possibly related PRs

Poem

🐰 When currencies dance and change their face,
The payment methods reset to their place,
No stale selections linger behind,
Fresh state for each fiat we find! 💱

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: reset payment methods when fiat currency changes (#352)" directly and clearly describes the main change in the pull request. It is concise, specific, and follows conventional commit formatting. The title accurately reflects the core objective of resetting payment methods when users change the fiat currency during order creation, which aligns perfectly with the implementation shown in the code changes.
Linked Issues Check ✅ Passed The implementation directly addresses the requirement from issue #352. The Riverpod listener monitors the selectedFiatCodeProvider for currency changes and automatically resets the payment-related state (_selectedPaymentMethods, _showCustomPaymentMethod, and _customPaymentMethodController) when a currency switch is detected. This prevents the issue where payment methods selected in one currency would incorrectly persist when switching to a different fiat currency, which matches the expected behavior described in the linked issue.
Out of Scope Changes Check ✅ Passed All changes are narrowly scoped to the payment method reset functionality and are directly related to solving issue #352. The modifications to lib/features/order/screens/add_order_screen.dart add only a currency change listener and related state reset logic without introducing unrelated features or changes. No alterations were made to public API declarations or manifest files, keeping the changeset focused and contained.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2fd3b3 and 5c84d4d.

📒 Files selected for processing (1)
  • lib/features/order/screens/add_order_screen.dart (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
lib/features/**

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • lib/features/order/screens/add_order_screen.dart
**/*.dart

📄 CodeRabbit inference engine (AGENTS.md)

**/*.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)

**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores

Files:

  • lib/features/order/screens/add_order_screen.dart
lib/**/*.dart

📄 CodeRabbit inference engine (AGENTS.md)

Name Riverpod providers as Provider or Notifier

lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible

Files:

  • lib/features/order/screens/add_order_screen.dart
lib/**/screens/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)

Files:

  • lib/features/order/screens/add_order_screen.dart
🧠 Learnings (3)
📓 Common learnings
Learnt from: Catrya
PR: MostroP2P/mobile#327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Learnt from: Catrya
PR: MostroP2P/mobile#272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
PR: MostroP2P/mobile#327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.

Applied to files:

  • lib/features/order/screens/add_order_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/*/providers/**/*.dart : Organize Riverpod providers by feature under features/{feature}/providers/

Applied to files:

  • lib/features/order/screens/add_order_screen.dart
🔇 Additional comments (1)
lib/features/order/screens/add_order_screen.dart (1)

195-208: LGTM! Listener correctly resets payment methods on currency change.

The implementation properly uses Riverpod's ref.listen pattern in the build method to react to currency changes. The condition checks are well-structured:

  • previous != null correctly skips the initial load
  • previous != next ensures the listener only fires on actual changes
  • context.mounted safely guards the setState call

The state reset is comprehensive, clearing the payment method list, hiding the custom field, and clearing the controller. This correctly addresses issue #352.


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 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tACK

@grunch grunch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@grunch
grunch merged commit 7247917 into MostroP2P:main Oct 30, 2025
2 checks passed
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.

Wrong payment method at fiat change (Creating orders)

3 participants