From cd323bbe529678e68bca23e84a9cdb0d78c33373 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= <victoor89@gmail.com>
Date: Mon, 15 Jun 2026 12:44:44 +0200
Subject: [PATCH] fix(budgets): make period generation idempotent (#533)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## 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.
---
 app/Services/BudgetPeriodService.php      | 26 ++++++++++++++++-------
 tests/Feature/BudgetPeriodServiceTest.php | 24 +++++++++++++++++++++
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/app/Services/BudgetPeriodService.php b/app/Services/BudgetPeriodService.php
index 38c283fb..7e105c43 100644
--- a/app/Services/BudgetPeriodService.php
+++ b/app/Services/BudgetPeriodService.php
@@ -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
diff --git a/tests/Feature/BudgetPeriodServiceTest.php b/tests/Feature/BudgetPeriodServiceTest.php
index 7435507f..39bec78b 100644
--- a/tests/Feature/BudgetPeriodServiceTest.php
+++ b/tests/Feature/BudgetPeriodServiceTest.php
@@ -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'));