fix(banking): correct backfill-ibans endpoint and handle expired sessions gracefully (#222)
## 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.
This commit is contained in:
parent
07ab9d5b96
commit
08dfb07a90
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
||||
|
|
|
|||
|
|
@ -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([
|
||||
|
|
|
|||
Loading…
Reference in New Issue