fix(finance): default the journal currency when settings are absent

A fresh install dies creating the first airline:

    App\Models\Airline::initJournal(): Argument #1 ($currency_code)
    must be of type string, null given

setting() returns its $default for a key it cannot read -- retrieve()
throws SettingNotFound and the helper swallows it -- and the created
hook passed no default, so it handed null to a string parameter. The
'USD' default on initJournal() cannot help: an explicit argument always
beats a declared default, even when that argument is null.

The installer creates the airline and user in the same step, and the
settings seed is gated on a separate wizard step, so units.currency is
not guaranteed to exist by then. Every other reader of this setting
already names the fallback -- Money.php:59,74 and
PirepFinanceService.php:57,61 -- so this is the odd one out.

Airline and User are the only models using the trait; both are covered.
The suite could not have caught this because tests/Pest.php seeds
SettingsSeeder before every test, making an empty settings table
unreachable, so the new tests clear it explicitly.
This commit is contained in:
Nabeel Shahzad 2026-07-26 00:46:42 -05:00
parent ab7d3d004c
commit 290544cdd8
No known key found for this signature in database
GPG Key ID: 08C44114D2BF3047
2 changed files with 50 additions and 1 deletions

View File

@ -14,7 +14,13 @@ trait JournalTrait
public static function bootJournalTrait(): void public static function bootJournalTrait(): void
{ {
static::created(function ($model): void { static::created(function ($model): void {
$model->initJournal(setting('units.currency')); // The default on initJournal() cannot cover this: an explicit
// argument always beats a declared default, and setting() returns
// null for a key it cannot read. During install the first airline
// is created before units.currency necessarily exists, so passing
// the bare lookup hands a null to a string parameter. Every other
// caller of this setting already names the fallback.
$model->initJournal(setting('units.currency', 'USD'));
}); });
} }

View File

@ -3,6 +3,8 @@
declare(strict_types=1); declare(strict_types=1);
use App\Models\Airline; use App\Models\Airline;
use App\Models\Setting;
use App\Models\User;
/* /*
* Regression coverage for JournalTrait::initJournal(). * Regression coverage for JournalTrait::initJournal().
@ -27,3 +29,44 @@ test('initJournal populates the journal relation on the same instance', function
// same journal a fresh reload from the database resolves to. // same journal a fresh reload from the database resolves to.
expect($airline->journal->id)->toBe($airline->fresh()->journal->id); expect($airline->journal->id)->toBe($airline->fresh()->journal->id);
}); });
/*
* The `created` hook reads units.currency, and setting() returns null for a key
* it cannot read -- the settings table is empty during install, and the first
* airline is created there. initJournal() declares a 'USD' default, but a
* default only applies when the argument is omitted; passing the bare lookup
* hands an explicit null to a string parameter and the install dies with
* "Argument #1 ($currency_code) must be of type string, null given".
*
* tests/Pest.php seeds SettingsSeeder before every test, so the empty-settings
* state these two cover is otherwise unreachable from the suite.
*/
test('a model with a journal can be created before units.currency is seeded', function (): void {
Setting::query()->delete();
$airline = Airline::factory()->create();
expect($airline->journal)->not->toBeNull()
->and($airline->journal->currency)->toBe('USD');
});
test('users are also covered, not just airlines', function (): void {
Setting::query()->delete();
// Airline and User are the only two models using the trait, and the
// installer creates both back to back.
$user = User::factory()->create();
expect($user->journal)->not->toBeNull()
->and($user->journal->currency)->toBe('USD');
});
test('a seeded units.currency still wins over the fallback', function (): void {
// Settings are already seeded by tests/Pest.php, so this is the normal
// path: the fallback must not shadow a value that is actually present.
// Matched on `key` -- the primary key stores dots as underscores
// (`units_currency`), which SettingService normalises on lookup.
Setting::where('key', 'units.currency')->update(['value' => 'EUR']);
expect(Airline::factory()->create()->journal->currency)->toBe('EUR');
});