From 08dfb07a90ac4e29b10d5412853d6d11579f3d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Thu, 12 Mar 2026 11:57:54 +0000 Subject: [PATCH] fix(banking): correct backfill-ibans endpoint and handle expired sessions gracefully (#222) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `banking:backfill-ibans` was calling `GET /accounts/{uid}` but the correct Enable Banking endpoint is `GET /accounts/{uid}/details`. This caused every account to return 404, and all 404s were counted as failures making the command exit with failure status. Additionally, accounts whose sessions have expired or been revoked will always return 404 — this is expected and should not be treated as an error. ## Changes - **`EnableBankingProvider`** — fix endpoint from `/accounts/{uid}` to `/accounts/{uid}/details` - **`BackfillAccountIbans`** — catch `RequestException` 404s separately and count them as `expired/revoked session` skips rather than failures; command exits successfully when the only issues are expired sessions - **Tests** — add test for the 404 expired-session path; update output string assertions ## Expected output after fix ``` Found 5 account(s) with missing IBAN. IBAN updated for 2 account(s). Skipped (no IBAN in API response): 0. Skipped (expired/revoked session): 3. Failed: 0. ``` The 2 active-session accounts will be backfilled; the 3 from the expired/revoked connection will be skipped cleanly. --- app/Console/Commands/BackfillAccountIbans.php | 12 ++++++- .../Banking/EnableBankingProvider.php | 2 +- .../BackfillAccountIbansCommandTest.php | 32 +++++++++++++++++-- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/app/Console/Commands/BackfillAccountIbans.php b/app/Console/Commands/BackfillAccountIbans.php index 12b4bf23..0f962e72 100644 --- a/app/Console/Commands/BackfillAccountIbans.php +++ b/app/Console/Commands/BackfillAccountIbans.php @@ -6,6 +6,7 @@ use App\Contracts\BankingProviderInterface; use App\Models\Account; use App\Models\User; use Illuminate\Console\Command; +use Illuminate\Http\Client\RequestException; use Throwable; class BackfillAccountIbans extends Command @@ -64,6 +65,7 @@ class BackfillAccountIbans extends Command $updated = 0; $skipped = 0; + $expiredSessions = 0; $failed = 0; foreach ($accounts as $account) { @@ -83,6 +85,14 @@ class BackfillAccountIbans extends Command } $updated++; + } catch (RequestException $e) { + if ($e->response->status() === 404) { + $expiredSessions++; + } else { + $this->newLine(); + $this->warn("Failed for account {$account->id} ({$account->external_account_id}): {$e->getMessage()}"); + $failed++; + } } catch (Throwable $e) { $this->newLine(); $this->warn("Failed for account {$account->id} ({$account->external_account_id}): {$e->getMessage()}"); @@ -96,7 +106,7 @@ class BackfillAccountIbans extends Command $this->newLine(); $verb = $isDryRun ? 'would be updated' : 'updated'; - $this->info("IBAN {$verb} for {$updated} account(s). Skipped (no IBAN): {$skipped}. Failed: {$failed}."); + $this->info("IBAN {$verb} for {$updated} account(s). Skipped (no IBAN in API response): {$skipped}. Skipped (expired/revoked session): {$expiredSessions}. Failed: {$failed}."); return $failed > 0 ? Command::FAILURE : Command::SUCCESS; } diff --git a/app/Services/Banking/EnableBankingProvider.php b/app/Services/Banking/EnableBankingProvider.php index 8bb1cfe5..0cfc5a20 100644 --- a/app/Services/Banking/EnableBankingProvider.php +++ b/app/Services/Banking/EnableBankingProvider.php @@ -121,7 +121,7 @@ class EnableBankingProvider implements BankingProviderInterface public function getAccount(string $accountId): array { - $response = $this->client()->get("/accounts/{$accountId}"); + $response = $this->client()->get("/accounts/{$accountId}/details"); $response->throw(); diff --git a/tests/Feature/BackfillAccountIbansCommandTest.php b/tests/Feature/BackfillAccountIbansCommandTest.php index 0a9cc52a..fb5f02b7 100644 --- a/tests/Feature/BackfillAccountIbansCommandTest.php +++ b/tests/Feature/BackfillAccountIbansCommandTest.php @@ -4,6 +4,9 @@ use App\Contracts\BankingProviderInterface; use App\Models\Account; use App\Models\BankingConnection; use App\Models\User; +use GuzzleHttp\Psr7\Response as GuzzleResponse; +use Illuminate\Http\Client\RequestException; +use Illuminate\Http\Client\Response as HttpClientResponse; beforeEach(function () { config([ @@ -53,7 +56,7 @@ test('skips accounts where api returns no iban', function () { $this->artisan('banking:backfill-ibans') ->assertSuccessful() - ->expectsOutputToContain('Skipped (no IBAN): 1'); + ->expectsOutputToContain('Skipped (no IBAN in API response): 1'); expect($account->fresh()->iban)->toBeNull(); }); @@ -184,7 +187,32 @@ test('filters by connection id', function () { $this->artisan('banking:backfill-ibans', ['--connection' => $connection->id])->assertSuccessful(); }); -test('continues and reports failure when api call throws', function () { +test('treats 404 as expired session and does not count as failure', function () { + $user = User::factory()->create(); + $connection = BankingConnection::factory()->for($user)->create(['provider' => 'enablebanking']); + $account = Account::factory()->for($user)->create([ + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'uid-expired', + 'iban' => null, + ]); + + $exception = new RequestException(new HttpClientResponse(new GuzzleResponse(404))); + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('getAccount') + ->once() + ->andThrow($exception); + + $this->app->instance(BankingProviderInterface::class, $mockProvider); + + $this->artisan('banking:backfill-ibans') + ->assertSuccessful() + ->expectsOutputToContain('Skipped (expired/revoked session): 1'); + + expect($account->fresh()->iban)->toBeNull(); +}); + +test('continues and reports failure when api call throws a non-404 error', function () { $user = User::factory()->create(); $connection = BankingConnection::factory()->for($user)->create(['provider' => 'enablebanking']); $account = Account::factory()->for($user)->create([