fix(budgets): make period generation idempotent (#533)
## Sentry Fixes **[PHP-LARAVEL-39](https://whisper-money.sentry.io/issues/PHP-LARAVEL-39)** — `UniqueConstraintViolationException` (1062) on `budget_periods_budget_id_start_date_unique`. 7 events over 6 days, 0 users (background command), regressed. ## Root cause `BudgetPeriodService::generatePeriod()` did a blind `BudgetPeriod::create()`. The scheduled `budgets:generate-periods` command reaches `generatePeriod()` from three paths (`handle()` + `closePeriod()`), each computing the next start date as `max(end_date) + 1 day`. Across **overlapping or repeated runs** two paths compute the same `start_date`, and the second insert collides with the unique key: ``` insert into budget_periods (budget_id, start_date, ...) values (019ea713..., 2026-06-30 00:00:00, ...) -> 1062 Duplicate entry '019ea713...-2026-06-30' ``` ## Fix Make creation idempotent on the `(budget_id, start_date)` unique key via `firstOrCreate()`. It delegates to Laravel's `createOrFirst()`, which catches the unique violation and re-queries — so the call is **concurrency-safe** and the command is safe to re-run. Period dates are normalized to start-of-day so the lookup matches the stored date key. ## Test `generatePeriod is idempotent when a period already exists for the start date` — reproduces the 1062 (verified failing against the pre-fix code with the exact `BudgetPeriodService.php:26` stack), passes after the fix. Returns the existing period, no duplicate row. ## Notes Behavior change: when the next period already exists, `generatePeriod()` now returns it unchanged instead of throwing. `closePeriod()` still updates `carried_over_amount` on the returned period.
This commit is contained in:
parent
2bfb569a22
commit
cd323bbe52
|
|
@ -17,20 +17,30 @@ class BudgetPeriodService
|
|||
|
||||
[$periodStart, $periodEnd] = $this->calculatePeriodDates($budget, $startDate);
|
||||
|
||||
$periodStart = $periodStart->startOfDay();
|
||||
$periodEnd = $periodEnd->startOfDay();
|
||||
|
||||
// If no allocated amount provided, use the last period's amount or 0
|
||||
if ($allocatedAmount === null) {
|
||||
$lastPeriod = $budget->periods()->orderBy('end_date', 'desc')->first();
|
||||
$allocatedAmount = $lastPeriod !== null ? $lastPeriod->allocated_amount : 0;
|
||||
}
|
||||
|
||||
return BudgetPeriod::create([
|
||||
'budget_id' => $budget->id,
|
||||
'start_date' => $periodStart,
|
||||
'end_date' => $periodEnd,
|
||||
'allocated_amount' => $allocatedAmount,
|
||||
'carried_over_amount' => 0,
|
||||
'processing_historical' => $processHistorical,
|
||||
]);
|
||||
// Idempotent on the (budget_id, start_date) unique key: the scheduled
|
||||
// command can recompute the same next start date across overlapping or
|
||||
// repeated runs, so return the existing period instead of colliding.
|
||||
return BudgetPeriod::firstOrCreate(
|
||||
[
|
||||
'budget_id' => $budget->id,
|
||||
'start_date' => $periodStart,
|
||||
],
|
||||
[
|
||||
'end_date' => $periodEnd,
|
||||
'allocated_amount' => $allocatedAmount,
|
||||
'carried_over_amount' => 0,
|
||||
'processing_historical' => $processHistorical,
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
public function generatePreviousPeriod(Budget $budget, BudgetPeriod $period, ?int $allocatedAmount = null, bool $processHistorical = false): BudgetPeriod
|
||||
|
|
|
|||
|
|
@ -96,6 +96,30 @@ test('generatePeriod uses period_start_day snap when no prior periods exist', fu
|
|||
expect($period->end_date->toDateString())->toBe('2026-05-31');
|
||||
});
|
||||
|
||||
test('generatePeriod is idempotent when a period already exists for the start date', function () {
|
||||
Carbon::setTestNow(Carbon::parse('2026-06-15 09:00:00'));
|
||||
|
||||
$user = User::factory()->create(['onboarded_at' => now()]);
|
||||
$budget = Budget::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'period_type' => BudgetPeriodType::Monthly,
|
||||
'period_start_day' => 1,
|
||||
]);
|
||||
|
||||
$existing = BudgetPeriod::factory()->create([
|
||||
'budget_id' => $budget->id,
|
||||
'start_date' => '2026-06-01',
|
||||
'end_date' => '2026-06-30',
|
||||
'allocated_amount' => 44500,
|
||||
]);
|
||||
|
||||
$period = app(BudgetPeriodService::class)->generatePeriod($budget, 100, Carbon::parse('2026-06-15'));
|
||||
|
||||
expect($period->id)->toBe($existing->id);
|
||||
expect($period->allocated_amount)->toBe(44500);
|
||||
expect(BudgetPeriod::where('budget_id', $budget->id)->count())->toBe(1);
|
||||
});
|
||||
|
||||
test('generatePeriod creates current calendar year when yearly budget has no prior periods', function () {
|
||||
Carbon::setTestNow(Carbon::parse('2026-05-15 09:00:00'));
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue