From c18debf7da8ae992b2befc067da0f29d8b9ed1e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vi=CC=81ctor=20Falco=CC=81n?= Date: Sat, 27 Jun 2026 17:49:52 +0200 Subject: [PATCH] fix(open-banking): never store the XXX no-currency placeholder on imported accounts Banks may report the ISO 4217 "XXX" (no currency) code, which has no exchange rate, so every conversion for such an account fell back to an unconverted (wrong) amount and spammed warning logs. Resolve a bank-reported currency to the user's base currency, then the app default, when it is XXX/empty/missing. --- .../OpenBanking/AccountMappingController.php | 2 +- .../Concerns/CreatesAccountsFromPending.php | 2 +- app/Services/AccountUserCurrencyService.php | 20 ++++++++++ .../AccountUserCurrencyServiceTest.php | 33 ++++++++++++++++ .../OpenBanking/AccountMappingTest.php | 39 +++++++++++++++++++ 5 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 tests/Feature/AccountUserCurrencyServiceTest.php diff --git a/app/Http/Controllers/OpenBanking/AccountMappingController.php b/app/Http/Controllers/OpenBanking/AccountMappingController.php index 61aa1cb5..558ee0b1 100644 --- a/app/Http/Controllers/OpenBanking/AccountMappingController.php +++ b/app/Http/Controllers/OpenBanking/AccountMappingController.php @@ -88,7 +88,7 @@ class AccountMappingController extends Controller } if ($action === 'create') { - $currency = $accountData['currency'] ?? 'EUR'; + $currency = $accountUserCurrencyService->resolveImportedCurrency($accountData['currency'] ?? null, $user); $name = $accountData['name'] ?? $accountData['account_id']['iban'] ?? $connection->aspsp_name.' Account'; diff --git a/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php b/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php index 95ef5fe5..49ae2f45 100644 --- a/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php +++ b/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php @@ -37,7 +37,7 @@ trait CreatesAccountsFromPending continue; } - $currency = $accountData['currency'] ?? 'EUR'; + $currency = $accountUserCurrencyService->resolveImportedCurrency($accountData['currency'] ?? null, $user); $name = $accountData['name'] ?? $accountData['account_id']['iban'] ?? $connection->aspsp_name.' Account'; diff --git a/app/Services/AccountUserCurrencyService.php b/app/Services/AccountUserCurrencyService.php index d2ba435c..72f31d7e 100644 --- a/app/Services/AccountUserCurrencyService.php +++ b/app/Services/AccountUserCurrencyService.php @@ -7,6 +7,26 @@ use App\Models\User; class AccountUserCurrencyService { + /** + * Resolve the currency code to store for a bank-imported account. + * + * Providers may report "XXX" (ISO 4217 "no currency") or omit the field; + * in those cases fall back to the user's base currency, then to the app + * default, so amounts stay convertible. + */ + public function resolveImportedCurrency(?string $reported, User $user): string + { + foreach ([$reported, $user->currency_code] as $candidate) { + $candidate = strtoupper(trim((string) $candidate)); + + if ($candidate !== '' && $candidate !== 'XXX') { + return $candidate; + } + } + + return strtoupper(config('cashier.currency', 'eur')); + } + public function syncFromFirstAccount(Account $account): void { $user = $account->user; diff --git a/tests/Feature/AccountUserCurrencyServiceTest.php b/tests/Feature/AccountUserCurrencyServiceTest.php new file mode 100644 index 00000000..763a41aa --- /dev/null +++ b/tests/Feature/AccountUserCurrencyServiceTest.php @@ -0,0 +1,33 @@ +service = app(AccountUserCurrencyService::class); + config(['cashier.currency' => 'eur']); +}); + +test('keeps a valid reported currency', function () { + $user = User::factory()->make(['currency_code' => 'USD']); + + expect($this->service->resolveImportedCurrency('GBP', $user))->toBe('GBP'); +}); + +test('uppercases the reported currency', function () { + $user = User::factory()->make(['currency_code' => 'USD']); + + expect($this->service->resolveImportedCurrency('gbp', $user))->toBe('GBP'); +}); + +test('falls back to the user currency for XXX, empty or missing codes', function (?string $reported) { + $user = User::factory()->make(['currency_code' => 'USD']); + + expect($this->service->resolveImportedCurrency($reported, $user))->toBe('USD'); +})->with(['XXX', 'xxx', '', null]); + +test('falls back to the app default when both the bank and the user lack a currency', function () { + $user = User::factory()->make(['currency_code' => 'XXX']); + + expect($this->service->resolveImportedCurrency('XXX', $user))->toBe('EUR'); +}); diff --git a/tests/Feature/OpenBanking/AccountMappingTest.php b/tests/Feature/OpenBanking/AccountMappingTest.php index 48558b7c..9764f73d 100644 --- a/tests/Feature/OpenBanking/AccountMappingTest.php +++ b/tests/Feature/OpenBanking/AccountMappingTest.php @@ -182,6 +182,45 @@ test('store creates investment accounts for crypto provider connections', functi 'coinbase' => ['coinbase', 'Coinbase', 'coinbase-portfolio'], ]); +test('store falls back to the user currency when the bank reports XXX', function () { + Queue::fake(); + + $user = User::factory()->onboarded()->create(['currency_code' => 'USD']); + Account::factory()->create(['user_id' => $user->id, 'currency_code' => 'USD']); + + $connection = BankingConnection::factory()->awaitingMapping()->create([ + 'user_id' => $user->id, + 'aspsp_name' => 'Test Bank', + 'pending_accounts_data' => [ + [ + 'uid' => 'ext-1', + 'currency' => 'XXX', + 'name' => 'No-currency Account', + 'account_id' => [], + ], + ], + ]); + + $this->actingAs($user) + ->post(route('open-banking.map-accounts.store', $connection), [ + 'mappings' => [ + [ + 'bank_account_uid' => 'ext-1', + 'action' => 'create', + 'existing_account_id' => null, + ], + ], + ]) + ->assertRedirect(route('settings.connections.index')); + + $this->assertDatabaseHas('accounts', [ + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-1', + 'currency_code' => 'USD', + ]); + $this->assertDatabaseMissing('accounts', ['currency_code' => 'XXX']); +}); + test('store updates user currency from first account created from mapping', function () { Queue::fake();