From ec08485c2b3841ad67aa5a5555a53c47328c1dc0 Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 8 Oct 2025 06:44:48 +0200 Subject: [PATCH] Clean up some code surrounding account balances. --- app/Helpers/Report/NetWorth.php | 51 ------ app/Helpers/Report/NetWorthInterface.php | 7 - app/Http/Controllers/Json/BoxController.php | 5 +- app/Support/Steam.php | 189 ++++++++++---------- 4 files changed, 95 insertions(+), 157 deletions(-) diff --git a/app/Helpers/Report/NetWorth.php b/app/Helpers/Report/NetWorth.php index 0ea1ce6cfe..80c0171ad6 100644 --- a/app/Helpers/Report/NetWorth.php +++ b/app/Helpers/Report/NetWorth.php @@ -130,55 +130,4 @@ class NetWorth implements NetWorthInterface $this->accountRepository->setUserGroup($userGroup); } - - #[Deprecated] - public function sumNetWorthByCurrency(Carbon $date): array - { - /** - * Collect accounts - */ - $accounts = $this->getAccounts(); - $return = []; - Log::debug(sprintf('SumNetWorth: accountsBalancesOptimized("%s")', $date->format('Y-m-d H:i:s'))); - $balances = Steam::accountsBalancesOptimized($accounts, $date); - foreach ($accounts as $account) { - $currency = $this->accountRepository->getAccountCurrency($account); - $balance = $balances[$account->id]['balance'] ?? '0'; - - // always subtract virtual balance. - $virtualBalance = $account->virtual_balance; - if ('' !== $virtualBalance) { - $balance = bcsub($balance, (string) $virtualBalance); - } - - $return[$currency->id] ??= [ - 'id' => (string) $currency->id, - 'name' => $currency->name, - 'symbol' => $currency->symbol, - 'code' => $currency->code, - 'decimal_places' => $currency->decimal_places, - 'sum' => '0', - ]; - $return[$currency->id]['sum'] = bcadd($return[$currency->id]['sum'], (string) $balance); - } - - return $return; - } - - private function getAccounts(): Collection - { - $accounts = $this->accountRepository->getAccountsByType( - [AccountTypeEnum::ASSET->value, AccountTypeEnum::DEFAULT->value, AccountTypeEnum::LOAN->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::MORTGAGE->value] - ); - $filtered = new Collection(); - - /** @var Account $account */ - foreach ($accounts as $account) { - if (1 === (int) $this->accountRepository->getMetaValue($account, 'include_net_worth')) { - $filtered->push($account); - } - } - - return $filtered; - } } diff --git a/app/Helpers/Report/NetWorthInterface.php b/app/Helpers/Report/NetWorthInterface.php index 421caa9a74..8660ab4e4c 100644 --- a/app/Helpers/Report/NetWorthInterface.php +++ b/app/Helpers/Report/NetWorthInterface.php @@ -50,11 +50,4 @@ interface NetWorthInterface public function setUserGroup(UserGroup $userGroup): void; - /** - * TODO move to repository - * - * Same as above but cleaner function with less dependencies. - */ - #[Deprecated] - public function sumNetWorthByCurrency(Carbon $date): array; } diff --git a/app/Http/Controllers/Json/BoxController.php b/app/Http/Controllers/Json/BoxController.php index d202a80af0..04d6a6b108 100644 --- a/app/Http/Controllers/Json/BoxController.php +++ b/app/Http/Controllers/Json/BoxController.php @@ -38,6 +38,7 @@ use FireflyIII\Support\Facades\Amount; use FireflyIII\Support\Facades\Steam; use FireflyIII\Support\Http\Controllers\DateCalculation; use Illuminate\Http\JsonResponse; +use Illuminate\Support\Facades\Log; /** * Class BoxController. @@ -165,7 +166,7 @@ class BoxController extends Controller $allAccounts = $accountRepository->getActiveAccountsByType( [AccountTypeEnum::DEFAULT->value, AccountTypeEnum::ASSET->value, AccountTypeEnum::LOAN->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::MORTGAGE->value] ); - app('log')->debug(sprintf('Found %d accounts.', $allAccounts->count())); + Log::debug(sprintf('Found %d accounts.', $allAccounts->count())); // filter list on preference of being included. $filtered = $allAccounts->filter( @@ -173,7 +174,7 @@ class BoxController extends Controller $includeNetWorth = $accountRepository->getMetaValue($account, 'include_net_worth'); $result = null === $includeNetWorth || '1' === $includeNetWorth; if (false === $result) { - app('log')->debug(sprintf('Will not include "%s" in net worth charts.', $account->name)); + Log::debug(sprintf('Will not include "%s" in net worth charts.', $account->name)); } return $result; diff --git a/app/Support/Steam.php b/app/Support/Steam.php index 0e4ddd661b..df270a2a3c 100644 --- a/app/Support/Steam.php +++ b/app/Support/Steam.php @@ -40,7 +40,6 @@ use Illuminate\Support\Str; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; use ValueError; - use function Safe\parse_url; use function Safe\preg_replace; @@ -51,47 +50,45 @@ class Steam { public function accountsBalancesOptimized(Collection $accounts, Carbon $date, ?TransactionCurrency $primary = null, ?bool $convertToPrimary = null, bool $inclusive = true): array { - Log::debug(sprintf('accountsBalancesOptimized: Called for %d account(s) with date/time "%s"', $accounts->count(), $date->toIso8601String())); - $result = []; + Log::debug(sprintf('accountsBalancesOptimized: Called for %d account(s) with date/time "%s" (inclusive: %s)', $accounts->count(), $date->toIso8601String(), var_export($inclusive, true))); + $result = []; $convertToPrimary ??= Amount::convertToPrimary(); $primary ??= Amount::getPrimaryCurrency(); - $currencies = $this->getCurrencies($accounts); + $currencies = $this->getCurrencies($accounts); // balance(s) in all currencies for ALL accounts. $arrayOfSums = Transaction::whereIn('account_id', $accounts->pluck('id')->toArray()) - ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') - ->leftJoin('transaction_currencies', 'transaction_currencies.id', '=', 'transactions.transaction_currency_id') - ->where('transaction_journals.date', $inclusive ? '<=': '<', $date->format('Y-m-d H:i:s')) - ->groupBy(['transactions.account_id', 'transaction_currencies.code']) - ->get(['transactions.account_id', 'transaction_currencies.code', DB::raw('SUM(transactions.amount) as sum_of_amount')])->toArray() - ; + ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') + ->leftJoin('transaction_currencies', 'transaction_currencies.id', '=', 'transactions.transaction_currency_id') + ->where('transaction_journals.date', $inclusive ? '<=' : '<', $date->format('Y-m-d H:i:s')) + ->groupBy(['transactions.account_id', 'transaction_currencies.code']) + ->get(['transactions.account_id', 'transaction_currencies.code', DB::raw('SUM(transactions.amount) as sum_of_amount')])->toArray(); /** @var Account $account */ foreach ($accounts as $account) { - // this array is PER account, so we wait a bit before we change code here. - $return = [ + $return = [ 'pc_balance' => '0', 'balance' => '0', // this key is overwritten right away, but I must remember it is always created. ]; - $currency = $currencies[$account->id]; + $currency = $currencies[$account->id]; // second array - $accountSum = array_filter($arrayOfSums, fn ($entry) => $entry['account_id'] === $account->id); + $accountSum = array_filter($arrayOfSums, fn($entry) => $entry['account_id'] === $account->id); if (0 === count($accountSum)) { $result[$account->id] = $return; continue; } - $accountSum = array_values($accountSum)[0]; - $sumOfAmount = (string)$accountSum['sum_of_amount']; - $sumOfAmount = $this->floatalize('' === $sumOfAmount ? '0' : $sumOfAmount); - $sumsByCode = [ + $accountSum = array_values($accountSum)[0]; + $sumOfAmount = (string)$accountSum['sum_of_amount']; + $sumOfAmount = $this->floatalize('' === $sumOfAmount ? '0' : $sumOfAmount); + $sumsByCode = [ $accountSum['code'] => $sumOfAmount, ]; // Log::debug('All balances are (joined)', $others); // if there is no request to convert, take this as "balance" and "pc_balance". - $return['balance'] = $sumsByCode[$currency->code] ?? '0'; + $return['balance'] = $sumsByCode[$currency->code] ?? '0'; if (!$convertToPrimary) { unset($return['pc_balance']); // Log::debug(sprintf('Set balance to %s, unset pc_balance', $return['balance'])); @@ -103,7 +100,7 @@ class Steam } // either way, the balance is always combined with the virtual balance: - $virtualBalance = (string)('' === (string)$account->virtual_balance ? '0' : $account->virtual_balance); + $virtualBalance = (string)('' === (string)$account->virtual_balance ? '0' : $account->virtual_balance); if ($convertToPrimary) { // the primary currency balance is combined with a converted virtual_balance: @@ -152,10 +149,10 @@ class Steam // Log::debug(sprintf('Trying bcround("%s",%d)', $number, $precision)); if (str_contains($number, '.')) { if ('-' !== $number[0]) { - return bcadd($number, '0.'.str_repeat('0', $precision).'5', $precision); + return bcadd($number, '0.' . str_repeat('0', $precision) . '5', $precision); } - return bcsub($number, '0.'.str_repeat('0', $precision).'5', $precision); + return bcsub($number, '0.' . str_repeat('0', $precision) . '5', $precision); } return $number; @@ -284,7 +281,8 @@ class Steam } /** - * Returns smaller than or equal to, so be careful with END OF DAY. + * By default this method returns "smaller than or equal to", so be careful with END OF DAY. + * If you need end of day balance, use "inclusive = false". * * Returns the balance of an account at exact moment given. Array with at least one value. * Always returns: @@ -296,18 +294,17 @@ class Steam * --> "pc_balance": balance in the user's primary currency, with all amounts converted to the primary currency. * "EUR": balance in EUR (or whatever currencies the account has balance in) */ - public function finalAccountBalance(Account $account, Carbon $date, ?TransactionCurrency $primary = null, ?bool $convertToPrimary = null): array + public function finalAccountBalance(Account $account, Carbon $date, ?TransactionCurrency $primary = null, ?bool $convertToPrimary = null, bool $inclusive = true): array { - $cache = new CacheProperties(); + $cache = new CacheProperties(); $cache->addProperty($account->id); $cache->addProperty($date); if ($cache->has()) { - Log::debug(sprintf('CACHED finalAccountBalance(#%d, %s)', $account->id, $date->format('Y-m-d H:i:s'))); - - // return $cache->get(); + Log::debug(sprintf('CACHED finalAccountBalance(#%d, %s, inclusive:%s)', $account->id, $date->format('Y-m-d H:i:s'), var_export($inclusive, true))); + return $cache->get(); } - // Log::debug(sprintf('finalAccountBalance(#%d, %s)', $account->id, $date->format('Y-m-d H:i:s'))); + Log::debug(sprintf('finalAccountBalance(#%d, %s)', $account->id, $date->format('Y-m-d H:i:s'))); if (null === $convertToPrimary) { $convertToPrimary = Amount::convertToPrimary($account->user); } @@ -315,7 +312,7 @@ class Steam $primary = Amount::getPrimaryCurrencyByUserGroup($account->user->userGroup); } // account balance thing. - $currencyPresent = isset($account->meta) && array_key_exists('currency', $account->meta) && null !== $account->meta['currency']; + $currencyPresent = isset($account->meta) && array_key_exists('currency', $account->meta) && null !== $account->meta['currency']; if ($currencyPresent) { $accountCurrency = $account->meta['currency']; } @@ -323,20 +320,19 @@ class Steam $accountCurrency = $this->getAccountCurrency($account); } - $hasCurrency = null !== $accountCurrency; - $currency = $hasCurrency ? $accountCurrency : $primary; - $return = [ + $hasCurrency = null !== $accountCurrency; + $currency = $hasCurrency ? $accountCurrency : $primary; + $return = [ 'pc_balance' => '0', 'balance' => '0', // this key is overwritten right away, but I must remember it is always created. ]; // balance(s) in all currencies. - $array = $account->transactions() - ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') - ->leftJoin('transaction_currencies', 'transaction_currencies.id', '=', 'transactions.transaction_currency_id') - ->where('transaction_journals.date', '<=', $date->format('Y-m-d H:i:s')) - ->get(['transaction_currencies.code', 'transactions.amount'])->toArray() - ; - $others = $this->groupAndSumTransactions($array, 'code', 'amount'); + $array = $account->transactions() + ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') + ->leftJoin('transaction_currencies', 'transaction_currencies.id', '=', 'transactions.transaction_currency_id') + ->where('transaction_journals.date', $inclusive ? '<=': '<', $date->format('Y-m-d H:i:s')) + ->get(['transaction_currencies.code', 'transactions.amount'])->toArray(); + $others = $this->groupAndSumTransactions($array, 'code', 'amount'); // Log::debug('All balances are (joined)', $others); // if there is no request to convert, take this as "balance" and "pc_balance". $return['balance'] = $others[$currency->code] ?? '0'; @@ -351,7 +347,7 @@ class Steam } // either way, the balance is always combined with the virtual balance: - $virtualBalance = (string)('' === (string)$account->virtual_balance ? '0' : $account->virtual_balance); + $virtualBalance = (string)('' === (string)$account->virtual_balance ? '0' : $account->virtual_balance); if ($convertToPrimary) { // the primary currency balance is combined with a converted virtual_balance: @@ -365,7 +361,7 @@ class Steam $return['balance'] = bcadd($return['balance'], $virtualBalance); // Log::debug(sprintf('Virtual balance makes the (primary currency) total %s', $return['balance'])); } - $final = array_merge($return, $others); + $final = array_merge($return, $others); // Log::debug('Final balance is', $final); $cache->store($final); @@ -380,7 +376,7 @@ class Steam Log::debug(sprintf('finalAccountBalanceInRange(#%d, %s, %s)', $account->id, $start->format('Y-m-d H:i:s'), $end->format('Y-m-d H:i:s'))); // set up cache - $cache = new CacheProperties(); + $cache = new CacheProperties(); $cache->addProperty($account->id); $cache->addProperty('final-balance-in-range'); $cache->addProperty($start); @@ -390,22 +386,22 @@ class Steam return $cache->get(); } - $balances = []; - $formatted = $start->format('Y-m-d'); + $balances = []; + $formatted = $start->format('Y-m-d'); /* * To make sure the start balance is correct, we need to get the balance at the exact end of the previous day. * Since we just did "startOfDay" we can do subDay()->endOfDay() to get the correct moment. * THAT will be the start balance. */ - $request = clone $start; + $request = clone $start; $request->subDay()->endOfDay(); Log::debug('Get first balance to start.'); Log::debug(sprintf('finalAccountBalanceInRange: Call finalAccountBalance with date/time "%s"', $request->toIso8601String())); - $startBalance = $this->finalAccountBalance($account, $request); - $primaryCurrency = Amount::getPrimaryCurrencyByUserGroup($account->user->userGroup); - $accountCurrency = $this->getAccountCurrency($account); - $hasCurrency = $accountCurrency instanceof TransactionCurrency; - $currency = $accountCurrency ?? $primaryCurrency; + $startBalance = $this->finalAccountBalance($account, $request); + $primaryCurrency = Amount::getPrimaryCurrencyByUserGroup($account->user->userGroup); + $accountCurrency = $this->getAccountCurrency($account); + $hasCurrency = $accountCurrency instanceof TransactionCurrency; + $currency = $accountCurrency ?? $primaryCurrency; Log::debug(sprintf('Currency is %s', $currency->code)); @@ -418,7 +414,7 @@ class Steam Log::debug(sprintf('Also set start balance in %s', $primaryCurrency->code)); $startBalance[$primaryCurrency->code] ??= '0'; } - $currencies = [ + $currencies = [ $currency->id => $currency, $primaryCurrency->id => $primaryCurrency, ]; @@ -428,48 +424,47 @@ class Steam // sums up the balance changes per day. Log::debug(sprintf('Date >= %s and <= %s', $start->format('Y-m-d H:i:s'), $end->format('Y-m-d H:i:s'))); - $set = $account->transactions() - ->leftJoin('transaction_journals', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') - ->where('transaction_journals.date', '>=', $start->format('Y-m-d H:i:s')) - ->where('transaction_journals.date', '<=', $end->format('Y-m-d H:i:s')) - ->groupBy('transaction_journals.date') - ->groupBy('transactions.transaction_currency_id') - ->orderBy('transaction_journals.date', 'ASC') - ->whereNull('transaction_journals.deleted_at') - ->get( - [ // @phpstan-ignore-line - 'transaction_journals.date', - 'transactions.transaction_currency_id', - DB::raw('SUM(transactions.amount) AS sum_of_day'), - ] - ) - ; + $set = $account->transactions() + ->leftJoin('transaction_journals', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->where('transaction_journals.date', '>=', $start->format('Y-m-d H:i:s')) + ->where('transaction_journals.date', '<=', $end->format('Y-m-d H:i:s')) + ->groupBy('transaction_journals.date') + ->groupBy('transactions.transaction_currency_id') + ->orderBy('transaction_journals.date', 'ASC') + ->whereNull('transaction_journals.deleted_at') + ->get( + [ // @phpstan-ignore-line + 'transaction_journals.date', + 'transactions.transaction_currency_id', + DB::raw('SUM(transactions.amount) AS sum_of_day'), + ] + ); - $currentBalance = $startBalance; - $converter = new ExchangeRateConverter(); + $currentBalance = $startBalance; + $converter = new ExchangeRateConverter(); /** @var Transaction $entry */ foreach ($set as $entry) { // get date object - $carbon = new Carbon($entry->date, $entry->date_tz); - $carbonKey = $carbon->format('Y-m-d'); + $carbon = new Carbon($entry->date, $entry->date_tz); + $carbonKey = $carbon->format('Y-m-d'); // make sure sum is a string: - $sumOfDay = (string)($entry->sum_of_day ?? '0'); + $sumOfDay = (string)($entry->sum_of_day ?? '0'); // #10426 make sure sum is not in scientific notation. - $sumOfDay = $this->floatalize($sumOfDay); + $sumOfDay = $this->floatalize($sumOfDay); // find currency of this entry, does not have to exist. $currencies[$entry->transaction_currency_id] ??= Amount::getTransactionCurrencyById($entry->transaction_currency_id); // make sure this $entry has its own $entryCurrency /** @var TransactionCurrency $entryCurrency */ - $entryCurrency = $currencies[$entry->transaction_currency_id]; + $entryCurrency = $currencies[$entry->transaction_currency_id]; Log::debug(sprintf('Processing transaction(s) on moment %s', $carbon->format('Y-m-d H:i:s'))); // add amount to current balance in currency code. - $currentBalance[$entryCurrency->code] ??= '0'; + $currentBalance[$entryCurrency->code] ??= '0'; $currentBalance[$entryCurrency->code] = bcadd($sumOfDay, (string)$currentBalance[$entryCurrency->code]); // if not requested to convert to primary currency, add the amount to "balance", do nothing else. @@ -487,7 +482,7 @@ class Steam } } // add to final array. - $balances[$carbonKey] = $currentBalance; + $balances[$carbonKey] = $currentBalance; Log::debug(sprintf('Updated entry [%s]', $carbonKey), $currentBalance); } $cache->store($balances); @@ -504,7 +499,7 @@ class Steam */ public function floatalize(string $value): string { - $value = strtoupper($value); + $value = strtoupper($value); if (!str_contains($value, 'E')) { return $value; } @@ -528,8 +523,8 @@ class Steam public function getAccountCurrency(Account $account): ?TransactionCurrency { - $type = $account->accountType->type; - $list = config('firefly.valid_currency_account_types'); + $type = $account->accountType->type; + $list = config('firefly.valid_currency_account_types'); // return null if not in this list. if (!in_array($type, $list, true)) { @@ -550,7 +545,7 @@ class Steam try { $hostName = gethostbyaddr($ipAddress); } catch (Exception $e) { - app('log')->error($e->getMessage()); + Log::error($e->getMessage()); $hostName = $ipAddress; } @@ -582,15 +577,15 @@ class Steam { $list = []; - $set = auth()->user()->transactions() - ->whereIn('transactions.account_id', $accounts) - ->groupBy(['transactions.account_id', 'transaction_journals.user_id']) - ->get(['transactions.account_id', DB::raw('MAX(transaction_journals.date) AS max_date')]) // @phpstan-ignore-line + $set = auth()->user()->transactions() + ->whereIn('transactions.account_id', $accounts) + ->groupBy(['transactions.account_id', 'transaction_journals.user_id']) + ->get(['transactions.account_id', DB::raw('MAX(transaction_journals.date) AS max_date')]) // @phpstan-ignore-line ; /** @var Transaction $entry */ foreach ($set as $entry) { - $date = new Carbon($entry->max_date, config('app.timezone')); + $date = new Carbon($entry->max_date, config('app.timezone')); $date->setTimezone(config('app.timezone')); $list[(int)$entry->account_id] = $date; } @@ -608,14 +603,14 @@ class Steam if (null !== $cached) { return $cached; } - $locale = app('preferences')->get('locale', config('firefly.default_locale', 'equal'))->data; + $locale = app('preferences')->get('locale', config('firefly.default_locale', 'equal'))->data; if (is_array($locale)) { $locale = 'equal'; } if ('equal' === $locale) { $locale = $this->getLanguage(); } - $locale = (string)$locale; + $locale = (string)$locale; // Check for Windows to replace the locale correctly. if ('WIN' === strtoupper(substr(PHP_OS, 0, 3))) { @@ -656,9 +651,9 @@ class Steam public function getSafeUrl(string $unknownUrl, string $safeUrl): string { // Log::debug(sprintf('getSafeUrl(%s, %s)', $unknownUrl, $safeUrl)); - $returnUrl = $safeUrl; - $unknownHost = parse_url($unknownUrl, PHP_URL_HOST); - $safeHost = parse_url($safeUrl, PHP_URL_HOST); + $returnUrl = $safeUrl; + $unknownHost = parse_url($unknownUrl, PHP_URL_HOST); + $safeHost = parse_url($safeUrl, PHP_URL_HOST); if (null !== $unknownHost && $unknownHost === $safeHost) { $returnUrl = $unknownUrl; @@ -760,12 +755,12 @@ class Steam if (null === $preference) { $singleton->setPreference($key, $currency); } - $current = $amount; + $current = $amount; if ($currency->id !== $primary->id) { $current = $converter->convert($currency, $primary, $date, $amount); Log::debug(sprintf('Convert %s %s to %s %s', $currency->code, $amount, $primary->code, $current)); } - $total = bcadd((string) $current, $total); + $total = bcadd((string)$current, $total); } return $total; @@ -779,8 +774,8 @@ class Steam $primary = Amount::getPrimaryCurrency(); $currencies[$primary->id] = $primary; - $ids = $accounts->pluck('id')->toArray(); - $result = AccountMeta::whereIn('account_id', $ids)->where('name', 'currency_id')->get(); + $ids = $accounts->pluck('id')->toArray(); + $result = AccountMeta::whereIn('account_id', $ids)->where('name', 'currency_id')->get(); /** @var AccountMeta $item */ foreach ($result as $item) { @@ -790,7 +785,7 @@ class Steam } } // collect those currencies, skip primary because we already have it. - $set = TransactionCurrency::whereIn('id', $accountPreferences)->where('id', '!=', $primary->id)->get(); + $set = TransactionCurrency::whereIn('id', $accountPreferences)->where('id', '!=', $primary->id)->get(); foreach ($set as $item) { $currencies[$item->id] = $item; } @@ -801,7 +796,7 @@ class Steam $currencyPresent = isset($account->meta) && array_key_exists('currency', $account->meta) && null !== $account->meta['currency']; if ($currencyPresent) { $currencyId = $account->meta['currency']->id; - $currencies[$currencyId] ??= $account->meta['currency']; + $currencies[$currencyId] ??= $account->meta['currency']; $accountCurrencies[$accountId] = $account->meta['currency']; } if (!$currencyPresent && !array_key_exists($accountId, $accountPreferences)) {