From 7edbff10acd7641fade235fce0ca3f5224527ca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vi=CC=81ctor=20Falco=CC=81n?= Date: Wed, 12 Aug 2026 11:29:07 +0200 Subject: [PATCH] refactor(budgets): extract the tracked-period lookup out of assignTransaction It carried the whole category/label matching inline, which put the method over the complexity threshold once precedence added a branch. --- app/Services/BudgetTransactionService.php | 101 ++++++++++++---------- 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/app/Services/BudgetTransactionService.php b/app/Services/BudgetTransactionService.php index 6fde6d8a..18b08a93 100644 --- a/app/Services/BudgetTransactionService.php +++ b/app/Services/BudgetTransactionService.php @@ -33,50 +33,7 @@ class BudgetTransactionService // Ensure labels are available for matching (safe if already loaded). $transaction->loadMissing('labels'); - $transactionLabelIds = $transaction->labels->pluck('id'); - - // A budget tracking a parent category also covers its children, so a - // transaction matches a budget when any of its category's ancestors - // (or itself) is attached to that budget. - $categoryMatchIds = $transaction->category_id - ? $this->tree->ancestorAndSelfIds($userId, $transaction->category_id) - : []; - - // Find budget periods that potentially match this transaction. - $budgetPeriods = BudgetPeriod::query() - ->whereHas('budget', function ($query) use ($categoryMatchIds, $transactionLabelIds, $userId) { - $query->where('user_id', $userId) - ->where(function ($q) use ($categoryMatchIds, $transactionLabelIds) { - $q->whereHas('categories', function ($cq) use ($categoryMatchIds) { - $cq->whereIn('categories.id', $categoryMatchIds); - }) - ->orWhereHas('labels', function ($lq) use ($transactionLabelIds) { - $lq->whereIn('labels.id', $transactionLabelIds); - }); - }); - }) - ->where('start_date', '<=', $transaction->transaction_date) - ->where('end_date', '>=', $transaction->transaction_date) - ->with('budget.categories:id', 'budget.labels:id') - ->get(); - - // Narrow down to periods whose budget actually matches the transaction. - $matchingPeriodIds = []; - - foreach ($budgetPeriods as $period) { - $budget = $period->budget; - - $matchesCategory = $categoryMatchIds !== [] - && $budget->categories->pluck('id')->intersect($categoryMatchIds)->isNotEmpty(); - $matchesLabel = $budget->labels - ->pluck('id') - ->intersect($transactionLabelIds) - ->isNotEmpty(); - - if ($matchesCategory || $matchesLabel) { - $matchingPeriodIds[] = $period->id; - } - } + $matchingPeriodIds = $this->trackedPeriodIds($transaction, $userId); // A catch-all budget only absorbs what nothing else counts. Any budget // already tracking this transaction — by category or by label — in a @@ -231,6 +188,62 @@ class BudgetTransactionService ->whereHas('category', fn ($q) => $q->where('type', CategoryType::Expense->value)); } + /** + * Budget periods that track this transaction by category or by label and + * cover its date. + * + * @return array + */ + private function trackedPeriodIds(Transaction $transaction, string $userId): array + { + $transactionLabelIds = $transaction->labels->pluck('id'); + + // A budget tracking a parent category also covers its children, so a + // transaction matches a budget when any of its category's ancestors + // (or itself) is attached to that budget. + $categoryMatchIds = $transaction->category_id + ? $this->tree->ancestorAndSelfIds($userId, $transaction->category_id) + : []; + + // Find budget periods that potentially match this transaction. + $budgetPeriods = BudgetPeriod::query() + ->whereHas('budget', function ($query) use ($categoryMatchIds, $transactionLabelIds, $userId) { + $query->where('user_id', $userId) + ->where(function ($q) use ($categoryMatchIds, $transactionLabelIds) { + $q->whereHas('categories', function ($cq) use ($categoryMatchIds) { + $cq->whereIn('categories.id', $categoryMatchIds); + }) + ->orWhereHas('labels', function ($lq) use ($transactionLabelIds) { + $lq->whereIn('labels.id', $transactionLabelIds); + }); + }); + }) + ->where('start_date', '<=', $transaction->transaction_date) + ->where('end_date', '>=', $transaction->transaction_date) + ->with('budget.categories:id', 'budget.labels:id') + ->get(); + + // Narrow down to periods whose budget actually matches the transaction. + $matchingPeriodIds = []; + + foreach ($budgetPeriods as $period) { + $budget = $period->budget; + + $matchesCategory = $categoryMatchIds !== [] + && $budget->categories->pluck('id')->intersect($categoryMatchIds)->isNotEmpty(); + $matchesLabel = $budget->labels + ->pluck('id') + ->intersect($transactionLabelIds) + ->isNotEmpty(); + + if ($matchesCategory || $matchesLabel) { + $matchingPeriodIds[] = $period->id; + } + } + + return $matchingPeriodIds; + } + /** * Catch-all budget periods that should absorb this expense. *