Extract duplicated getCategorySpending into CategorySpendingService

DashboardController::getCategorySpending and
DashboardAnalyticsController::getCategorySpending were the same rolled-up
expense-per-category query, differing only in whether they accepted a drill
parent (the dashboard always passed the top level). Copies of a financial
aggregation on two screens are a divergence risk.

Extract App\Services\CategorySpendingService::forPeriod() with an optional
drill parent and consume it from both controllers, dropping the now-unused
CategoryTree/DB/Collection/Category imports. Behaviour-preserving: the query,
roll-up and positive-amount filter are unchanged, and existing
DashboardAnalyticsTest / DashboardTest coverage stays green.
This commit is contained in:
Víctor Falcón 2026-07-04 20:23:52 +02:00
parent 5572a2d75a
commit 0f08c0c662
3 changed files with 58 additions and 75 deletions

View File

@ -7,18 +7,15 @@ use App\Enums\CategoryType;
use App\Http\Controllers\Controller;
use App\Models\Account;
use App\Models\AccountBalance;
use App\Models\Category;
use App\Models\Transaction;
use App\Services\AccountMetricsService;
use App\Services\BalanceLookup;
use App\Services\CategoryTree;
use App\Services\CategorySpendingService;
use App\Services\ExchangeRateService;
use App\Services\LoanAmortizationService;
use App\Services\PeriodComparator;
use Carbon\Carbon;
use Illuminate\Http\Request;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
class DashboardAnalyticsController extends Controller
{
@ -26,7 +23,7 @@ class DashboardAnalyticsController extends Controller
private ExchangeRateService $exchangeRateService,
private AccountMetricsService $accountMetricsService,
private LoanAmortizationService $loanAmortizationService,
private CategoryTree $tree,
private CategorySpendingService $categorySpendingService,
) {}
public function netWorth(Request $request)
@ -439,8 +436,8 @@ class DashboardAnalyticsController extends Controller
$previousPeriod = $period->previous();
$drillParentId = $validated['parent'] ?? null;
$currentSpending = $this->getCategorySpending($request->user()->id, $period->from, $period->to, $drillParentId);
$previousSpending = $this->getCategorySpending($request->user()->id, $previousPeriod->from, $previousPeriod->to, $drillParentId);
$currentSpending = $this->categorySpendingService->forPeriod($request->user()->id, $period->from, $period->to, $drillParentId);
$previousSpending = $this->categorySpendingService->forPeriod($request->user()->id, $previousPeriod->from, $previousPeriod->to, $drillParentId);
$totalAmount = $currentSpending->sum('amount');
@ -465,39 +462,6 @@ class DashboardAnalyticsController extends Controller
return response()->json($top);
}
/**
* Expense spending rolled up the category tree.
*
* Without a drill target, child amounts fold into their top-level ancestor
* so only parents are listed. With one, the parent's children become the
* rows (plus a direct node for transactions sitting on the parent itself).
*/
private function getCategorySpending(string $userId, Carbon $from, Carbon $to, ?string $drillParentId = null): Collection
{
$perCategory = Transaction::query()
->where('transactions.user_id', $userId)
->whereBetween('transactions.transaction_date', [$from, $to])
->join('categories', function ($join) {
$join->on('transactions.category_id', '=', 'categories.id')
->where('categories.type', '=', CategoryType::Expense)
->whereNull('categories.deleted_at');
})
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
->groupBy('transactions.category_id')
->get()
->map(fn ($item): array => [
'category_id' => $item->category_id,
'category' => null,
'amount' => (int) -$item->total_amount,
])
->values()
->all();
return collect($this->tree->rollUp($perCategory, $userId, $drillParentId))
->filter(fn (array $item): bool => $item['amount'] > 0)
->values();
}
private function calculateNetWorthAt(Carbon $date, string $userCurrency): int
{
$accounts = Account::where('user_id', request()->user()->id)->get();

View File

@ -7,11 +7,10 @@ use App\Models\Account;
use App\Models\Transaction;
use App\Services\AccountMetricsService;
use App\Services\CashflowSummaryService;
use App\Services\CategoryTree;
use App\Services\CategorySpendingService;
use App\Services\PeriodComparator;
use Carbon\Carbon;
use Illuminate\Http\Request;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Inertia\Inertia;
use Inertia\Response;
@ -20,7 +19,7 @@ class DashboardController extends Controller
{
public function __construct(
private AccountMetricsService $accountMetricsService,
private CategoryTree $tree,
private CategorySpendingService $categorySpendingService,
) {}
public function __invoke(Request $request): Response
@ -60,8 +59,8 @@ class DashboardController extends Controller
$period = new PeriodComparator($from, $to);
$previousPeriod = $period->previous();
$currentSpending = $this->getCategorySpending($user->id, $period->from, $period->to);
$previousSpending = $this->getCategorySpending($user->id, $previousPeriod->from, $previousPeriod->to);
$currentSpending = $this->categorySpendingService->forPeriod($user->id, $period->from, $period->to);
$previousSpending = $this->categorySpendingService->forPeriod($user->id, $previousPeriod->from, $previousPeriod->to);
$totalAmount = $currentSpending->sum('amount');
@ -101,36 +100,6 @@ class DashboardController extends Controller
];
}
/**
* Spending per top-level category: child category amounts roll up into
* their root ancestor so the dashboard only lists parents.
*/
private function getCategorySpending(string $userId, Carbon $from, Carbon $to): Collection
{
$perCategory = Transaction::query()
->where('transactions.user_id', $userId)
->whereBetween('transactions.transaction_date', [$from, $to])
->join('categories', function ($join) {
$join->on('transactions.category_id', '=', 'categories.id')
->where('categories.type', '=', CategoryType::Expense)
->whereNull('categories.deleted_at');
})
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
->groupBy('transactions.category_id')
->get()
->map(fn ($item): array => [
'category_id' => $item->category_id,
'category' => null,
'amount' => (int) -$item->total_amount,
])
->values()
->all();
return collect($this->tree->rollUp($perCategory, $userId, null))
->filter(fn (array $item): bool => $item['amount'] > 0)
->values();
}
private function calculateCashflowSummary(string $userId, Carbon $from, Carbon $to): array
{
$income = max(0, $this->getTransactionSum($userId, $from, $to, CategoryType::Income));

View File

@ -0,0 +1,50 @@
<?php
namespace App\Services;
use App\Enums\CategoryType;
use App\Models\Transaction;
use Carbon\Carbon;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
class CategorySpendingService
{
public function __construct(private CategoryTree $tree) {}
/**
* Expense spending rolled up the category tree.
*
* Without a drill target, child category amounts fold into their root
* ancestor so only parents are listed. With one, the parent's children
* become the rows (plus a direct node for transactions sitting on the
* parent itself). Soft-deleted categories are excluded.
*
* @return Collection<int, array<string, mixed>>
*/
public function forPeriod(string $userId, Carbon $from, Carbon $to, ?string $drillParentId = null): Collection
{
$perCategory = Transaction::query()
->where('transactions.user_id', $userId)
->whereBetween('transactions.transaction_date', [$from, $to])
->join('categories', function ($join) {
$join->on('transactions.category_id', '=', 'categories.id')
->where('categories.type', '=', CategoryType::Expense)
->whereNull('categories.deleted_at');
})
->select('transactions.category_id', DB::raw('sum(transactions.amount) as total_amount'))
->groupBy('transactions.category_id')
->get()
->map(fn ($item): array => [
'category_id' => $item->category_id,
'category' => null,
'amount' => (int) -$item->total_amount,
])
->values()
->all();
return collect($this->tree->rollUp($perCategory, $userId, $drillParentId))
->filter(fn (array $item): bool => $item['amount'] > 0)
->values();
}
}