diff --git a/app/Http/Controllers/BudgetController.php b/app/Http/Controllers/BudgetController.php index 15ded9f4..19eacf79 100644 --- a/app/Http/Controllers/BudgetController.php +++ b/app/Http/Controllers/BudgetController.php @@ -12,8 +12,6 @@ use App\Models\Budget; use App\Models\Category; use App\Models\Label; use App\Models\SavingsGoal; -use App\Models\Transaction; -use App\Models\User; use App\Services\BudgetPeriodService; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Http\RedirectResponse; @@ -41,51 +39,16 @@ class BudgetController extends Controller }]) ->get(); + $savingsGoalsEnabled = Feature::active(SavingsGoals::class); + return Inertia::render('budgets/index', [ 'budgets' => $budgets, - 'savingsGoals' => $this->savingsGoalsForIndex($user), - 'savingsGoalsEnabled' => Feature::active(SavingsGoals::class), + 'savingsGoals' => $savingsGoalsEnabled ? SavingsGoal::withStatsForUser($user) : [], + 'savingsGoalsEnabled' => $savingsGoalsEnabled, 'currencyCode' => $user->currency_code ?? 'USD', ]); } - /** - * Savings goals shown alongside budgets on the index, each enriched with its - * computed progress. Returns an empty list when the feature is off. - * - * @return list> - */ - private function savingsGoalsForIndex(User $user): array - { - if (! Feature::active(SavingsGoals::class)) { - return []; - } - - $goals = $user->savingsGoals()->with('label')->get(); - - // ponytail: one grouped sum for all goals' labels avoids N+1 across the list. - $savedByLabel = Transaction::query() - ->join('label_transaction', 'label_transaction.transaction_id', '=', 'transactions.id') - ->whereIn('label_transaction.label_id', $goals->pluck('label_id')->filter()) - ->groupBy('label_transaction.label_id') - ->selectRaw('label_transaction.label_id as label_id, SUM(transactions.amount) as total') - ->pluck('total', 'label_id'); - - return $goals->map(function (SavingsGoal $goal) use ($savedByLabel): array { - $saved = -1 * (int) ($savedByLabel[$goal->label_id] ?? 0); - - return array_merge($goal->toArray(), [ - 'stats' => SavingsGoal::project( - $saved, - $goal->target_amount, - $goal->created_at, - $goal->target_date, - now(), - ), - ]); - })->all(); - } - public function show(Request $request, Budget $budget): Response { $this->authorize('view', $budget); diff --git a/app/Http/Controllers/SavingsGoalController.php b/app/Http/Controllers/SavingsGoalController.php index da388225..5c963875 100644 --- a/app/Http/Controllers/SavingsGoalController.php +++ b/app/Http/Controllers/SavingsGoalController.php @@ -12,22 +12,38 @@ use App\Models\Bank; use App\Models\Category; use App\Models\Label; use App\Models\SavingsGoal; +use Closure; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; +use Illuminate\Routing\Controllers\HasMiddleware; use Illuminate\Support\Facades\DB; use Inertia\Inertia; use Inertia\Response; use Laravel\Pennant\Feature; -class SavingsGoalController extends Controller +class SavingsGoalController extends Controller implements HasMiddleware { use AuthorizesRequests; + /** + * Hide the whole savings-goals surface behind the rollout feature flag. + * + * @return array + */ + public static function middleware(): array + { + return [ + function (Request $request, Closure $next): mixed { + abort_unless(Feature::active(SavingsGoals::class), 404); + + return $next($request); + }, + ]; + } + public function store(StoreSavingsGoalRequest $request): RedirectResponse { - abort_unless(Feature::active(SavingsGoals::class), 404); - $goal = DB::transaction(function () use ($request) { $label = $request->user()->labels()->create([ 'name' => $request->name, @@ -48,7 +64,6 @@ class SavingsGoalController extends Controller public function show(Request $request, SavingsGoal $savingsGoal): Response { - abort_unless(Feature::active(SavingsGoals::class), 404); $this->authorize('view', $savingsGoal); $user = $request->user(); @@ -61,10 +76,8 @@ class SavingsGoalController extends Controller ->get() : collect(); - $saved = -1 * (int) $transactions->sum('amount'); - $stats = SavingsGoal::project( - $saved, + $savingsGoal->savedAmountInCents(), $savingsGoal->target_amount, $savingsGoal->created_at, $savingsGoal->target_date, @@ -98,7 +111,6 @@ class SavingsGoalController extends Controller public function update(UpdateSavingsGoalRequest $request, SavingsGoal $savingsGoal): RedirectResponse { - abort_unless(Feature::active(SavingsGoals::class), 404); $this->authorize('update', $savingsGoal); DB::transaction(function () use ($request, $savingsGoal) { @@ -114,7 +126,6 @@ class SavingsGoalController extends Controller public function destroy(Request $request, SavingsGoal $savingsGoal): RedirectResponse { - abort_unless(Feature::active(SavingsGoals::class), 404); $this->authorize('delete', $savingsGoal); DB::transaction(function () use ($savingsGoal) { diff --git a/app/Models/SavingsGoal.php b/app/Models/SavingsGoal.php index 962212fc..2779d616 100644 --- a/app/Models/SavingsGoal.php +++ b/app/Models/SavingsGoal.php @@ -64,6 +64,41 @@ class SavingsGoal extends Model return -1 * (int) $this->label->transactions()->sum('transactions.amount'); } + /** + * All of a user's goals with their computed progress, for the combined + * budgets/goals index. Kept here (not in the budget controller) so budgets + * stay decoupled from goals. + * + * @return list> + */ + public static function withStatsForUser(User $user): array + { + $goals = $user->savingsGoals()->with('label')->get(); + + // ponytail: one grouped sum for all goals' labels avoids N+1 across the list. + $savedByLabel = Transaction::query() + ->join('label_transaction', 'label_transaction.transaction_id', '=', 'transactions.id') + ->whereIn('label_transaction.label_id', $goals->pluck('label_id')->filter()) + ->groupBy('label_transaction.label_id') + ->selectRaw('label_transaction.label_id as label_id, SUM(transactions.amount) as total') + ->pluck('total', 'label_id'); + + return $goals->map(function (SavingsGoal $goal) use ($savedByLabel): array { + // Negated net flow, mirroring savedAmountInCents(); batched to avoid N+1. + $saved = -1 * (int) ($savedByLabel[$goal->label_id] ?? 0); + + return array_merge($goal->toArray(), [ + 'stats' => self::project( + $saved, + $goal->target_amount, + $goal->created_at, + $goal->target_date, + now(), + ), + ]); + })->all(); + } + /** * Linear progress + projection, computed from primitives so it stays a pure, * testable function. Dates are day-granular. `rate_per_day` is cents/day and diff --git a/resources/js/components/budgets/create-budget-dialog.tsx b/resources/js/components/budgets/create-budget-dialog.tsx index 8ff38821..971660f8 100644 --- a/resources/js/components/budgets/create-budget-dialog.tsx +++ b/resources/js/components/budgets/create-budget-dialog.tsx @@ -21,6 +21,7 @@ import { SelectTrigger, SelectValue, } from '@/components/ui/select'; +import { useControllableOpen } from '@/hooks/use-controllable-open'; import { buildCategoryTree, flattenCategoryTree } from '@/lib/category-tree'; import { cn } from '@/lib/utils'; import { SharedData } from '@/types'; @@ -57,16 +58,11 @@ export function CreateBudgetDialog({ onOpenChange, }: Props) { const page = usePage(); - const [internalOpen, setInternalOpen] = useState(false); - const isControlled = open !== undefined; - const dialogOpen = isControlled ? open : internalOpen; - const setOpen = (next: boolean) => { - if (isControlled) { - onOpenChange?.(next); - } else { - setInternalOpen(next); - } - }; + const { + open: dialogOpen, + setOpen, + isControlled, + } = useControllableOpen({ open, onOpenChange }); const [name, setName] = useState(''); const [periodType, setPeriodType] = useState('monthly'); const [periodStartDay, setPeriodStartDay] = useState(1); diff --git a/resources/js/components/savings-goals/create-savings-goal-dialog.tsx b/resources/js/components/savings-goals/create-savings-goal-dialog.tsx index 90429c75..6b20d6b7 100644 --- a/resources/js/components/savings-goals/create-savings-goal-dialog.tsx +++ b/resources/js/components/savings-goals/create-savings-goal-dialog.tsx @@ -13,6 +13,7 @@ import { } from '@/components/ui/dialog'; import { Input } from '@/components/ui/input'; import { Label as UILabel } from '@/components/ui/label'; +import { useControllableOpen } from '@/hooks/use-controllable-open'; import { cn } from '@/lib/utils'; import { __ } from '@/utils/i18n'; import { router } from '@inertiajs/react'; @@ -34,16 +35,11 @@ export function CreateSavingsGoalDialog({ open, onOpenChange, }: Props) { - const [internalOpen, setInternalOpen] = useState(false); - const isControlled = open !== undefined; - const dialogOpen = isControlled ? open : internalOpen; - const setDialogOpen = (next: boolean) => { - if (isControlled) { - onOpenChange?.(next); - } else { - setInternalOpen(next); - } - }; + const { + open: dialogOpen, + setOpen: setDialogOpen, + isControlled, + } = useControllableOpen({ open, onOpenChange }); const [name, setName] = useState(''); const [targetAmount, setTargetAmount] = useState(0); diff --git a/resources/js/hooks/use-controllable-open.ts b/resources/js/hooks/use-controllable-open.ts new file mode 100644 index 00000000..2f59ebe3 --- /dev/null +++ b/resources/js/hooks/use-controllable-open.ts @@ -0,0 +1,30 @@ +import { useState } from 'react'; + +interface Options { + open?: boolean; + onOpenChange?: (open: boolean) => void; +} + +/** + * Lets a dialog work both controlled (parent passes open/onOpenChange) and + * uncontrolled (own internal state) without each component reimplementing the + * branching. + */ +export function useControllableOpen({ open, onOpenChange }: Options) { + const [internalOpen, setInternalOpen] = useState(false); + const isControlled = open !== undefined; + + const setOpen = (next: boolean) => { + if (isControlled) { + onOpenChange?.(next); + } else { + setInternalOpen(next); + } + }; + + return { + open: isControlled ? open : internalOpen, + setOpen, + isControlled, + }; +}