feat(#281): currency-aware payment method suggestions in create-order form - #297
Conversation
…ate-order form The create-order form showed one hardcoded list of generic methods regardless of the selected currency. Port the v1 approach: ship assets/data/payment_methods.json (currency -> methods, 28 currencies + a default fallback) and a currency-aware provider, and make the payment-method picker show the list matching the selected fiat currency (falling back to default for unknown currencies). When the currency changes, selections that are no longer valid are pruned; the custom free-text field is left untouched. Verified on a physical device (Nokia C31): the method list updates per currency and stale selections clear on currency switch. Adds a test covering the currency->list contract and the default fallback.
|
Warning Review limit reached
Next review available in: 38 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe order form now loads payment methods from a bundled currency mapping. Providers resolve currency-specific and fallback methods. The payment picker updates when fiat currency changes and removes unsupported selections. Provider tests validate asset loading and lookup behavior. ChangesCurrency-aware payment methods
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PaymentMethodSection
participant PaymentMethodsProvider
participant MethodPickerDialog
User->>PaymentMethodSection: select fiat currency
PaymentMethodSection->>PaymentMethodsProvider: request methods for currency
PaymentMethodsProvider-->>PaymentMethodSection: return supported methods
PaymentMethodSection->>PaymentMethodSection: remove unsupported selections
User->>MethodPickerDialog: open payment method picker
MethodPickerDialog-->>PaymentMethodSection: return selected methods
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/data/payment_methods.json`:
- Around line 2-30: In assets/data/payment_methods.json, add localization
entries for every payment-method label, including the default fallback labels,
while preserving each existing value exactly. Update
payment_methods_provider.dart to resolve these labels for picker display and
payment_method_section.dart to resolve labels for selected chips; keep
serialized and comparison values unchanged so presets, order filters, and
NewOrderParams.paymentMethod continue using the original strings.
In `@lib/features/order/widgets/payment_method_section.dart`:
- Around line 48-56: The currency-change listener around
selectedFiatCodeProvider must not prune selections when
paymentMethodsDataProvider is still loading; gate the valid-method lookup and
pruning until the provider has produced data, preserving selections during the
loading interval. Add a test covering a currency change before the asset future
completes and verify existing selections remain intact.
In `@test/features/order/providers/payment_methods_provider_test.dart`:
- Around line 6-15: The tests currently duplicate the provider’s asset-loading
and currency-fallback logic instead of exercising the providers. Replace direct
use of _loadShipped and _forCurrency with ProviderContainer tests that await
paymentMethodsDataProvider.future and override it when testing
paymentMethodsForCurrencyProvider for known, default, and loading cases; retain
separate assertions for the asset contract, then run flutter analyze and flutter
test.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b2974020-266e-4c77-8be6-ff8842012a40
📒 Files selected for processing (5)
assets/data/payment_methods.jsonlib/features/order/providers/payment_methods_provider.dartlib/features/order/widgets/payment_method_section.dartpubspec.yamltest/features/order/providers/payment_methods_provider_test.dart
…loading The currency-change listener pruned selected methods against the currency provider, which returns an empty list until the asset loads — so a currency change before the load wiped every selection. Gate pruning on the data provider having resolved. Rework the provider tests to exercise the real providers via a ProviderContainer with an overridden data source (known, default, and loading cases), keeping the shipped-asset contract assertions separate.
|
#281 is a direct port of v1's currency-aware payment-method list, and v1 ships these labels as plain strings. The labels are used as serialized/comparison values in presets, order filters, and NewOrderParams.paymentMethod, so localizing them requires a display-vs-serialized split across 100+ labels a separate enhancement (CodeRabbit flags it "Heavy lift") rather than part of this port. Happy to open a follow-up issue for label localization if the team wants it. |
There was a problem hiding this comment.
Reviewed the current head 07a05e0. The currency-aware payment-method provider matches the issue scope, the shipped asset is registered and well-formed, and the loading-state pruning issue from the earlier bot review is guarded by paymentMethodsDataProvider.hasValue. I also checked the unresolved localization suggestion and agree it is a serialized-value/display-label split that can be handled separately rather than blocking this v1 parity port.
Verified:
- GitHub checks are green for this head.
- Local
git diff --checkpassed. assets/data/payment_methods.jsonparses and has non-empty, duplicate-free method lists.
No blocking issues found.
Problem
The create-order form showed a single hardcoded list of 10 generic payment methods (
_commonMethodsinpayment_method_section.dart) regardless of the selected fiat currency. A user trading ARS saw Zelle or SEPA, while locally relevant options (MODO, CVU, etc.) were missing and had to be typed by hand.Solution (v1 parity)
Ports the v1 mobile approach:
assets/data/payment_methods.jsonacurrency code -> payment methodsmap covering 28 currencies plus adefaultfallback (copied from the v1 mobile app), and registers it inpubspec.yaml.paymentMethodsForCurrencyProvider(lib/features/order/providers/payment_methods_provider.dart):paymentMethodsDataProvideraFutureProviderthat loads and decodes the asset once.paymentMethodsForCurrencyProvideraProvider.family<List<String>, String>that returns the list for a given currency, falling back to thedefaultlist (and then a hardcoded fallback) for unknown currencies. Returns an empty list while the asset is still loading, so the section renders its custom field without flashing placeholder chips.PaymentMethodSectionto be currency-aware:selectedFiatCodeProvider) instead of the hardcoded_commonMethods.ref.listenprunes any selected methods that are no longer valid for the new currency._commonMethodsconstant.Testing
test/features/order/providers/payment_methods_provider_test.dart): validates the shipped asset and the lookup contract a default fallback plus 20+ currencies, a known currency (ARS) resolves to its specific list, the African currencies added in #625/#627 (MWK -> Airtel Money, KES -> M-PESA) are present, and an unknown currency falls back todefault.flutter analyzeclean across the order feature and tests.Notes
rootBundleat runtime; the contract test validates the same lookup logic against the shipped JSON directly (asset loading throughrootBundleisn't wired for plain unit tests), while the full provider wiring is exercised on-device.Closes #281.
Summary by CodeRabbit