fix(budgets): remove Custom period type to fix duplicate-key crash (#355)
## Why merge this Production bug **[PHP-LARAVEL-1B](https://whisper-money.sentry.io/issues/PHP-LARAVEL-1B)** — `/budgets/{budget}` throws a 500 (`UniqueConstraintViolationException` on `budget_periods_budget_id_start_date_unique`) for the affected user. Page is unusable for them until fixed. ## Root cause The Custom period type let users create budgets with arbitrary `period_duration`. Combined with `calculatePeriodDates()`'s day-of-month snap (`startDate->day(period_start_day)`), short durations produced periods where `end_date == start_date == period_start_day`. On the next visit after that day, regeneration computed a new period whose start snapped backward to the same day → unique key collision. Concrete trace from the Sentry event: - Budget: `Seguro de Salud`, `period_type=custom, period_duration=1, period_start_day=1`, created `2026-05-05`. - Initial period created at budget creation: `start=2026-05-01, end=2026-05-01` (already 4 days stale because Custom snapped `now()=05-05` back to day 1, then `addDays(1).subDay()` produced same date). - Today `05-05` → `getCurrentPeriod()` returns null → `generatePeriod()` snaps back to `05-01` again → `INSERT (budget_id, 05-01)` → 1062. Verified in prod DB: this is the **only** budget in the entire database with `period_type='custom'` and the only period row with `end_date <= start_date`. ## Fix Custom is the only period type that exposes this snap-back collision (Monthly/Weekly/Biweekly all use a self-consistent advance). It also has no defensible UX — every legitimate use case is covered by Monthly/Weekly/Biweekly. So we remove it end-to-end: - `BudgetPeriodType::Custom` enum case dropped. - `BudgetPeriodService`: Custom branches removed from `calculatePeriodDates()` and `calculateNextPeriodStartDate()`. - `Budget` model: `period_duration` removed from `$fillable` and `casts()`. - `StoreBudgetRequest` / `UpdateBudgetRequest`: `period_duration` rule removed. - `BudgetController`: `period_duration` no longer passed in store/update. - Create + edit budget dialogs: Custom option and `period_duration` input removed. - `ResetDemoAccountCommand`: Custom switch case removed. - `BudgetFactory`: `custom()` state and `period_duration` default removed. - `types/budget.ts`: `'custom'` removed from `BUDGET_PERIOD_TYPES` and `Budget.period_duration` field dropped. - Migration `2026_05_05_132023_convert_custom_budgets_to_monthly`: rewrites any existing `custom` rows to `monthly` with `period_duration=null, period_start_day=1` so the dropped enum case can't crash on hydration. Period generation logic for Monthly/Weekly/Biweekly is **unchanged**. ## Manual prod cleanup (separate, after merge) The single buggy budget in prod will be deleted manually: ```sql DELETE FROM budget_periods WHERE id = '019df7fd-6073-731b-9c08-f3723515292b'; DELETE FROM budgets WHERE id = '019df7fd-6071-7219-b190-ece22ccdb63f'; ``` ## Tests `tests/Feature/BudgetPeriodServiceTest.php` covers Monthly + Weekly advance and Monthly first-period generation. Existing `BudgetPeriodDateTest` unaffected. ## Risk - `period_duration` column kept in DB (nullable) to avoid data loss; no migration needed. - Migration ensures any environment with `period_type='custom'` rows is converted before the enum case is removed, preventing hydration errors. - No customers actively rely on Custom: prod query confirmed only the single broken budget uses it, and that one is being deleted. Fixes PHP-LARAVEL-1B
This commit is contained in:
Víctor Falcón
GitHub
parent
3fea230d58
commit
22043ced29
|
|
@ -468,8 +468,6 @@ class ResetDemoAccountCommand extends Command
|
|||
$currentDate->addDay();
|
||||
}
|
||||
break;
|
||||
case BudgetPeriodType::Custom:
|
||||
break;
|
||||
}
|
||||
|
||||
$iteration++;
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ enum BudgetPeriodType: string
|
|||
case Monthly = 'monthly';
|
||||
case Weekly = 'weekly';
|
||||
case Biweekly = 'biweekly';
|
||||
case Custom = 'custom';
|
||||
|
||||
public function label(): string
|
||||
{
|
||||
|
|
@ -15,7 +14,6 @@ enum BudgetPeriodType: string
|
|||
self::Monthly => 'Monthly',
|
||||
self::Weekly => 'Weekly',
|
||||
self::Biweekly => 'Bi-weekly',
|
||||
self::Custom => 'Custom',
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -119,7 +119,6 @@ class BudgetController extends Controller
|
|||
$budget = $request->user()->budgets()->create([
|
||||
'name' => $request->name,
|
||||
'period_type' => $request->period_type,
|
||||
'period_duration' => $request->period_duration,
|
||||
'period_start_day' => $request->period_start_day,
|
||||
'category_id' => $request->category_id,
|
||||
'label_id' => $request->label_id,
|
||||
|
|
@ -145,7 +144,6 @@ class BudgetController extends Controller
|
|||
$budget->update($request->only([
|
||||
'name',
|
||||
'period_type',
|
||||
'period_duration',
|
||||
'period_start_day',
|
||||
'category_id',
|
||||
'label_id',
|
||||
|
|
|
|||
|
|
@ -21,7 +21,6 @@ class StoreBudgetRequest extends FormRequest
|
|||
return [
|
||||
'name' => ['required', 'string', 'max:255'],
|
||||
'period_type' => ['required', Rule::enum(BudgetPeriodType::class)],
|
||||
'period_duration' => ['nullable', 'integer', 'min:1', 'max:365'],
|
||||
'period_start_day' => ['nullable', 'integer', 'min:0', 'max:31'],
|
||||
'category_id' => ['nullable', Rule::exists('categories', 'id')->where('user_id', $userId)],
|
||||
'label_id' => ['nullable', Rule::exists('labels', 'id')->where('user_id', $userId)],
|
||||
|
|
|
|||
|
|
@ -21,7 +21,6 @@ class UpdateBudgetRequest extends FormRequest
|
|||
return [
|
||||
'name' => ['sometimes', 'string', 'max:255'],
|
||||
'period_type' => ['sometimes', Rule::enum(BudgetPeriodType::class)],
|
||||
'period_duration' => ['nullable', 'integer', 'min:1', 'max:365'],
|
||||
'period_start_day' => ['nullable', 'integer', 'min:0', 'max:31'],
|
||||
'category_id' => ['nullable', Rule::exists('categories', 'id')->where('user_id', $userId)],
|
||||
'label_id' => ['nullable', Rule::exists('labels', 'id')->where('user_id', $userId)],
|
||||
|
|
|
|||
|
|
@ -23,7 +23,6 @@ class Budget extends Model
|
|||
'user_id',
|
||||
'name',
|
||||
'period_type',
|
||||
'period_duration',
|
||||
'period_start_day',
|
||||
'category_id',
|
||||
'label_id',
|
||||
|
|
@ -35,7 +34,6 @@ class Budget extends Model
|
|||
return [
|
||||
'period_type' => BudgetPeriodType::class,
|
||||
'rollover_type' => RolloverType::class,
|
||||
'period_duration' => 'integer',
|
||||
'period_start_day' => 'integer',
|
||||
];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -80,17 +80,6 @@ class BudgetPeriodService
|
|||
$endDate = $startDate->copy()->addWeeks(2)->subDay();
|
||||
break;
|
||||
|
||||
case BudgetPeriodType::Custom:
|
||||
$duration = $budget->period_duration ?? 30;
|
||||
$startDate->day($budget->period_start_day ?? 1);
|
||||
if ($startDate > $referenceDate) {
|
||||
$startDate->subDays($duration);
|
||||
}
|
||||
$endDate = $startDate->copy()->addDays($duration)->subDay();
|
||||
break;
|
||||
|
||||
default:
|
||||
$endDate = $startDate->copy()->addMonth()->subDay();
|
||||
}
|
||||
|
||||
return [$startDate, $endDate];
|
||||
|
|
|
|||
|
|
@ -23,7 +23,6 @@ class BudgetFactory extends Factory
|
|||
'user_id' => User::factory(),
|
||||
'name' => fake()->words(2, true).' Budget',
|
||||
'period_type' => fake()->randomElement(BudgetPeriodType::cases()),
|
||||
'period_duration' => null,
|
||||
'period_start_day' => 1,
|
||||
];
|
||||
}
|
||||
|
|
@ -43,13 +42,4 @@ class BudgetFactory extends Factory
|
|||
'period_start_day' => fake()->numberBetween(0, 6),
|
||||
]);
|
||||
}
|
||||
|
||||
public function custom(): static
|
||||
{
|
||||
return $this->state(fn (array $attributes) => [
|
||||
'period_type' => BudgetPeriodType::Custom,
|
||||
'period_duration' => fake()->numberBetween(7, 90),
|
||||
'period_start_day' => fake()->numberBetween(1, 28),
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,23 @@
|
|||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
public function up(): void
|
||||
{
|
||||
DB::table('budgets')
|
||||
->where('period_type', 'custom')
|
||||
->update([
|
||||
'period_type' => 'monthly',
|
||||
'period_duration' => null,
|
||||
'period_start_day' => 1,
|
||||
]);
|
||||
}
|
||||
|
||||
public function down(): void
|
||||
{
|
||||
// Irreversible: original period_type/duration cannot be recovered.
|
||||
}
|
||||
};
|
||||
|
|
@ -52,7 +52,6 @@ export function CreateBudgetDialog({
|
|||
const [open, setOpen] = useState(false);
|
||||
const [name, setName] = useState('');
|
||||
const [periodType, setPeriodType] = useState<BudgetPeriodType>('monthly');
|
||||
const [periodDuration, setPeriodDuration] = useState<number | null>(null);
|
||||
const [periodStartDay, setPeriodStartDay] = useState<number>(1);
|
||||
const [selectedCategoryId, setSelectedCategoryId] = useState<string>('');
|
||||
const [selectedLabelId, setSelectedLabelId] = useState<string>('');
|
||||
|
|
@ -89,7 +88,6 @@ export function CreateBudgetDialog({
|
|||
{
|
||||
name,
|
||||
period_type: periodType,
|
||||
period_duration: periodDuration,
|
||||
period_start_day: periodStartDay,
|
||||
category_id: selectedCategoryId || null,
|
||||
label_id: selectedLabelId || null,
|
||||
|
|
@ -101,7 +99,6 @@ export function CreateBudgetDialog({
|
|||
setOpen(false);
|
||||
setName('');
|
||||
setPeriodType('monthly');
|
||||
setPeriodDuration(null);
|
||||
setPeriodStartDay(1);
|
||||
setSelectedCategoryId('');
|
||||
setSelectedLabelId('');
|
||||
|
|
@ -184,29 +181,6 @@ export function CreateBudgetDialog({
|
|||
</Select>
|
||||
</div>
|
||||
|
||||
{periodType === 'custom' && (
|
||||
<div className="space-y-2">
|
||||
<UILabel htmlFor="period-duration">
|
||||
{__('Period Duration (days)')}
|
||||
</UILabel>
|
||||
<Input
|
||||
id="period-duration"
|
||||
type="number"
|
||||
min="1"
|
||||
max="365"
|
||||
value={periodDuration ?? ''}
|
||||
onChange={(e) =>
|
||||
setPeriodDuration(
|
||||
e.target.value
|
||||
? parseInt(e.target.value)
|
||||
: null,
|
||||
)
|
||||
}
|
||||
required={periodType === 'custom'}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
||||
<div className="space-y-2">
|
||||
<UILabel htmlFor="period-start-day">
|
||||
{periodType === 'monthly'
|
||||
|
|
|
|||
|
|
@ -50,9 +50,6 @@ export function EditBudgetDialog({
|
|||
const [periodType, setPeriodType] = useState<BudgetPeriodType>(
|
||||
budget.period_type as BudgetPeriodType,
|
||||
);
|
||||
const [periodDuration, setPeriodDuration] = useState<number | null>(
|
||||
budget.period_duration,
|
||||
);
|
||||
const [periodStartDay, setPeriodStartDay] = useState<number>(
|
||||
budget.period_start_day || 1,
|
||||
);
|
||||
|
|
@ -68,7 +65,6 @@ export function EditBudgetDialog({
|
|||
if (open && budget) {
|
||||
setName(budget.name);
|
||||
setPeriodType(budget.period_type as BudgetPeriodType);
|
||||
setPeriodDuration(budget.period_duration);
|
||||
setPeriodStartDay(budget.period_start_day || 1);
|
||||
setAllocatedAmount(currentPeriod.allocated_amount);
|
||||
setRolloverType(budget.rollover_type as RolloverType);
|
||||
|
|
@ -84,7 +80,6 @@ export function EditBudgetDialog({
|
|||
{
|
||||
name,
|
||||
period_type: periodType,
|
||||
period_duration: periodDuration,
|
||||
period_start_day: periodStartDay,
|
||||
allocated_amount: allocatedAmount,
|
||||
rollover_type: rolloverType,
|
||||
|
|
@ -150,31 +145,6 @@ export function EditBudgetDialog({
|
|||
</div>
|
||||
</div>
|
||||
|
||||
{periodType === 'custom' && (
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="period-duration">
|
||||
{__('Period Duration (days)')}
|
||||
</Label>
|
||||
<Input
|
||||
disabled
|
||||
id="period-duration"
|
||||
type="number"
|
||||
min="1"
|
||||
max="365"
|
||||
className="mt-1"
|
||||
value={periodDuration ?? ''}
|
||||
onChange={(e) =>
|
||||
setPeriodDuration(
|
||||
e.target.value
|
||||
? parseInt(e.target.value)
|
||||
: null,
|
||||
)
|
||||
}
|
||||
required={periodType === 'custom'}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="period-start-day">
|
||||
{periodType === 'monthly'
|
||||
|
|
|
|||
|
|
@ -4,12 +4,7 @@ import { Label } from './label';
|
|||
import { Transaction } from './transaction';
|
||||
import { UUID } from './uuid';
|
||||
|
||||
export const BUDGET_PERIOD_TYPES = [
|
||||
'monthly',
|
||||
'weekly',
|
||||
'biweekly',
|
||||
'custom',
|
||||
] as const;
|
||||
export const BUDGET_PERIOD_TYPES = ['monthly', 'weekly', 'biweekly'] as const;
|
||||
|
||||
export type BudgetPeriodType = (typeof BUDGET_PERIOD_TYPES)[number];
|
||||
|
||||
|
|
@ -22,7 +17,6 @@ export interface Budget {
|
|||
user_id: UUID;
|
||||
name: string;
|
||||
period_type: BudgetPeriodType;
|
||||
period_duration: number | null;
|
||||
period_start_day: number | null;
|
||||
category_id: UUID | null;
|
||||
label_id: UUID | null;
|
||||
|
|
@ -70,7 +64,6 @@ export function getBudgetPeriodTypeLabel(type: BudgetPeriodType): string {
|
|||
monthly: __('Monthly'),
|
||||
weekly: __('Weekly'),
|
||||
biweekly: __('Bi-weekly'),
|
||||
custom: __('Custom'),
|
||||
};
|
||||
return labels[type];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,74 @@
|
|||
<?php
|
||||
|
||||
use App\Enums\BudgetPeriodType;
|
||||
use App\Models\Budget;
|
||||
use App\Models\BudgetPeriod;
|
||||
use App\Models\User;
|
||||
use App\Services\BudgetPeriodService;
|
||||
use Carbon\Carbon;
|
||||
|
||||
afterEach(function () {
|
||||
Carbon::setTestNow();
|
||||
});
|
||||
|
||||
test('generatePeriod advances monthly periods to next month', function () {
|
||||
Carbon::setTestNow(Carbon::parse('2026-06-02 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,
|
||||
]);
|
||||
|
||||
BudgetPeriod::factory()->create([
|
||||
'budget_id' => $budget->id,
|
||||
'start_date' => '2026-05-01',
|
||||
'end_date' => '2026-05-31',
|
||||
'allocated_amount' => 10000,
|
||||
]);
|
||||
|
||||
$next = app(BudgetPeriodService::class)->generatePeriod($budget);
|
||||
|
||||
expect($next->start_date->toDateString())->toBe('2026-06-01');
|
||||
expect($next->end_date->toDateString())->toBe('2026-06-30');
|
||||
});
|
||||
|
||||
test('generatePeriod advances weekly periods to next week', function () {
|
||||
Carbon::setTestNow(Carbon::parse('2026-05-12 09:00:00'));
|
||||
|
||||
$user = User::factory()->create(['onboarded_at' => now()]);
|
||||
$budget = Budget::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'period_type' => BudgetPeriodType::Weekly,
|
||||
'period_start_day' => 1,
|
||||
]);
|
||||
|
||||
BudgetPeriod::factory()->create([
|
||||
'budget_id' => $budget->id,
|
||||
'start_date' => '2026-05-04',
|
||||
'end_date' => '2026-05-10',
|
||||
'allocated_amount' => 10000,
|
||||
]);
|
||||
|
||||
$next = app(BudgetPeriodService::class)->generatePeriod($budget);
|
||||
|
||||
expect($next->start_date->toDateString())->toBe('2026-05-11');
|
||||
expect($next->end_date->toDateString())->toBe('2026-05-17');
|
||||
});
|
||||
|
||||
test('generatePeriod uses period_start_day snap when no prior periods exist', function () {
|
||||
Carbon::setTestNow(Carbon::parse('2026-05-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,
|
||||
]);
|
||||
|
||||
$period = app(BudgetPeriodService::class)->generatePeriod($budget);
|
||||
|
||||
expect($period->start_date->toDateString())->toBe('2026-05-01');
|
||||
expect($period->end_date->toDateString())->toBe('2026-05-31');
|
||||
});
|
||||
Loading…
Reference in New Issue