From 0638d109d09b14c60a8ca89c04327396e26bbafb Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 13 May 2024 20:31:52 +0200 Subject: [PATCH] Clean up code. --- .../Correction/CorrectAccountBalance.php | 6 - app/Handlers/Observer/TransactionObserver.php | 10 +- .../Models/AccountBalanceCalculator.php | 211 +++++++++++++----- 3 files changed, 152 insertions(+), 75 deletions(-) diff --git a/app/Console/Commands/Correction/CorrectAccountBalance.php b/app/Console/Commands/Correction/CorrectAccountBalance.php index 0c3e4185c2..e62068e7a2 100644 --- a/app/Console/Commands/Correction/CorrectAccountBalance.php +++ b/app/Console/Commands/Correction/CorrectAccountBalance.php @@ -30,11 +30,5 @@ class CorrectAccountBalance extends Command private function correctBalanceAmounts(): void { AccountBalanceCalculator::recalculate(null, null); - foreach (TransactionJournal::all() as $journal) { - Log::debug(sprintf('Recalculating account balances for journal #%d', $journal->id)); - foreach ($journal->transactions as $transaction) { - AccountBalanceCalculator::recalculate($transaction->account, $journal); - } - } } } diff --git a/app/Handlers/Observer/TransactionObserver.php b/app/Handlers/Observer/TransactionObserver.php index 6413a4b3f4..cca0afa2ad 100644 --- a/app/Handlers/Observer/TransactionObserver.php +++ b/app/Handlers/Observer/TransactionObserver.php @@ -40,18 +40,12 @@ class TransactionObserver public function updated(Transaction $transaction): void { app('log')->debug('Observe "updated" of a transaction.'); - AccountBalanceCalculator::recalculate($transaction->account, null); - if ((float)$transaction->amount > 0) { - AccountBalanceCalculator::recalculateByJournal($transaction->transactionJournal); - } + AccountBalanceCalculator::recalculateForAccount($transaction->account); } public function created(Transaction $transaction): void { app('log')->debug('Observe "created" of a transaction.'); - AccountBalanceCalculator::recalculate($transaction->account, null); - if ((float)$transaction->amount > 0) { - AccountBalanceCalculator::recalculateByJournal($transaction->transactionJournal); - } + AccountBalanceCalculator::recalculateForAccount($transaction->account); } } diff --git a/app/Support/Models/AccountBalanceCalculator.php b/app/Support/Models/AccountBalanceCalculator.php index d06adfdd48..2a2e9cb74f 100644 --- a/app/Support/Models/AccountBalanceCalculator.php +++ b/app/Support/Models/AccountBalanceCalculator.php @@ -28,98 +28,138 @@ use FireflyIII\Models\AccountBalance; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use Illuminate\Support\Facades\Log; +use stdClass; class AccountBalanceCalculator { - public static function recalculate(?Account $account, ?TransactionJournal $transactionJournal): void + + /** + * Recalculate all balances for a given account. + * + * Je moet toch altijd wel alles doen want je weet niet waar een transaction journal invloed op heeft. + * Dus dit aantikken per transaction journal is zinloos, beide accounts moeten gedaan worden. + * + * + * @param Account $account + * + * @return void + */ + public static function recalculateForAccount(Account $account): void { - // first collect normal amounts (in whatever currency), and set them. + self::recalculate($account); + } - // select account_id, transaction_currency_id, foreign_currency_id, sum(amount), sum(foreign_amount) from transactions group by account_id, transaction_currency_id, foreign_currency_id - $query = Transaction::groupBy(['transactions.account_id', 'transactions.transaction_currency_id', 'transactions.foreign_currency_id']); - $title = 'balance'; - if (null !== $account) { - $query->where('transactions.account_id', $account->id); - } - if (null !== $transactionJournal) { - $query->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id'); - $query->where('transaction_journals.date', '<=', $transactionJournal->date); - $title = 'balance_after_journal'; - } - - $result = $query->get(['transactions.account_id', 'transactions.transaction_currency_id', 'transactions.foreign_currency_id', \DB::raw('SUM(transactions.amount) as sum_amount'), \DB::raw('SUM(transactions.foreign_amount) as sum_foreign_amount')]); - - // reset account balances: - self::resetAccountBalances($title, $account, $transactionJournal); - - /** @var \stdClass $row */ - foreach ($result as $row) { - $account = (int) $row->account_id; - $transactionCurrency = (int) $row->transaction_currency_id; - $foreignCurrency = (int) $row->foreign_currency_id; - $sumAmount = $row->sum_amount; - $sumForeignAmount = $row->sum_foreign_amount; - - // first create for normal currency: - $entry = self::getBalance($title, $account, $transactionCurrency, $transactionJournal?->id); - $entry->balance = bcadd($entry->balance, $sumAmount); - $entry->transaction_journal_id = $transactionJournal?->id; - $entry->save(); - Log::debug(sprintf('Set balance entry "%s" #%d to amount %s', $title, $entry->id, $entry->balance)); - - // then do foreign amount, if present: - if ($foreignCurrency > 0) { - $entry = self::getBalance($title, $account, $foreignCurrency, $transactionJournal?->id); - $entry->balance = bcadd($entry->balance, $sumForeignAmount); - $entry->transaction_journal_id = $transactionJournal?->id; - $entry->save(); - Log::debug(sprintf('Set balance entry "%s" #%d to amount %s', $title, $entry->id, $entry->balance)); - } + public static function recalculateForTransactionJournal(TransactionJournal $transactionJournal): void + { + foreach ($transactionJournal->transactions as $transaction) { + self::recalculateForAccount($transaction->account); } } - private static function getBalance(string $title, int $account, int $currency, ?int $journal): AccountBalance + /** + * select account_id, transaction_currency_id, foreign_currency_id, sum(amount), sum(foreign_amount) from + * transactions group by account_id, transaction_currency_id, foreign_currency_id + * + * @param Account|null $account + * + * @return void + */ + public static function recalculate(?Account $account): void { - $query = AccountBalance::where('title', $title)->where('account_id', $account)->where('transaction_currency_id', $currency); + self::recalculateLatest($account); + // loop all transaction journals and set those amounts too. + self::recalculateJournals($account); - if (null !== $journal) { - $query->where('transaction_journal_id', $journal); - } + // loop all dates and set those amounts too. - $entry = $query->first(); + } + + private static function getAccountBalanceByAccount(int $account, int $currency): AccountBalance + { + $query = AccountBalance::where('title', 'balance')->where('account_id', $account)->where('transaction_currency_id', $currency); + + $entry = $query->first(); if (null !== $entry) { - Log::debug(sprintf('Found account balance "%s" for account #%d and currency #%d: %s', $title, $account, $currency, $entry->balance)); + //Log::debug(sprintf('Found account balance "balance" for account #%d and currency #%d: %s', $account, $currency, $entry->balance)); return $entry; } $entry = new AccountBalance(); + $entry->title = 'balance'; + $entry->account_id = $account; + $entry->transaction_currency_id = $currency; + $entry->balance = '0'; + $entry->save(); + //Log::debug(sprintf('Created new account balance for account #%d and currency #%d: %s', $account, $currency, $entry->balance)); + + return $entry; + } + + private static function getAccountBalanceByJournal(string $title, int $account, int $journal, int $currency): AccountBalance + { + $query = AccountBalance::where('title', $title)->where('account_id', $account)->where('transaction_journal_id', $journal)->where('transaction_currency_id', $currency); + + $entry = $query->first(); + if (null !== $entry) { + return $entry; + } + $entry = new AccountBalance(); $entry->title = $title; $entry->account_id = $account; - $entry->transaction_currency_id = $currency; $entry->transaction_journal_id = $journal; + $entry->transaction_currency_id = $currency; $entry->balance = '0'; $entry->save(); - Log::debug(sprintf('Created new account balance for account #%d and currency #%d: %s', $account, $currency, $entry->balance)); return $entry; } - private static function resetAccountBalances(string $title, ?Account $account, ?TransactionJournal $transactionJournal): void + private static function recalculateLatest(?Account $account): void { - if (null === $account && null === $transactionJournal) { + $query = Transaction::groupBy(['transactions.account_id', 'transactions.transaction_currency_id', 'transactions.foreign_currency_id']); + + if (null !== $account) { + $query->where('transactions.account_id', $account->id); + } + $result = $query->get(['transactions.account_id', 'transactions.transaction_currency_id', 'transactions.foreign_currency_id', \DB::raw('SUM(transactions.amount) as sum_amount'), \DB::raw('SUM(transactions.foreign_amount) as sum_foreign_amount')]); + + // reset account balances: + self::resetAccountBalancesByAccount('balance', $account); + + /** @var stdClass $row */ + foreach ($result as $row) { + $account = (int) $row->account_id; + $transactionCurrency = (int) $row->transaction_currency_id; + $foreignCurrency = (int) $row->foreign_currency_id; + $sumAmount = $row->sum_amount; + $sumForeignAmount = $row->sum_foreign_amount; + + // first create for normal currency: + $entry = self::getAccountBalanceByAccount($account, $transactionCurrency); + $entry->balance = bcadd($entry->balance, $sumAmount); + $entry->save(); +// Log::debug(sprintf('Set balance entry #%d ("balance") to amount %s', $entry->id, $entry->balance)); + + // then do foreign amount, if present: + if ($foreignCurrency > 0) { + $entry = self::getAccountBalanceByAccount($account, $foreignCurrency); + $entry->balance = bcadd($entry->balance, $sumForeignAmount); + $entry->save(); +// Log::debug(sprintf('Set balance entry #%d ("balance") to amount %s', $entry->id, $entry->balance)); + } + } + } + + private static function resetAccountBalancesByAccount(string $title, ?Account $account): void + { + if (null === $account) { AccountBalance::whereNotNull('updated_at')->where('title', $title)->update(['balance' => '0']); Log::debug('Set ALL balances to zero.'); return; } - if (null !== $account && null === $transactionJournal) { - AccountBalance::where('account_id', $account->id)->where('title', $title)->update(['balance' => '0']); - Log::debug(sprintf('Set balances of account #%d to zero.', $account->id)); - - return; - } - AccountBalance::where('account_id', $account->id)->where('transaction_journal_id', $transactionJournal->id)->where('title', $title)->update(['balance' => '0']); - Log::debug(sprintf('Set balances of account #%d + journal #%d to zero.', $account->id, $transactionJournal->id)); + AccountBalance::where('account_id', $account->id)->where('title', $title)->update(['balance' => '0']); + Log::debug(sprintf('Set balances of account #%d to zero.', $account->id)); } public static function recalculateByJournal(TransactionJournal $transactionJournal): void @@ -130,4 +170,53 @@ class AccountBalanceCalculator self::recalculate($transaction->account, $transactionJournal); } } + + private static function recalculateJournals(?Account $account): void + { + $query = Transaction::groupBy(['transactions.account_id', 'transaction_journals.id', 'transactions.transaction_currency_id', 'transactions.foreign_currency_id']); + $query->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id'); + $query->orderBy('transaction_journals.date', 'asc'); + if (null !== $account) { + $query->where('transactions.account_id', $account->id); + } + $result = $query->get(['transactions.account_id', 'transaction_journals.id', 'transactions.transaction_currency_id', 'transactions.foreign_currency_id', \DB::raw('SUM(transactions.amount) as sum_amount'), \DB::raw('SUM(transactions.foreign_amount) as sum_foreign_amount')]); + $amounts = []; + + /** @var stdClass $row */ + foreach ($result as $row) { + $account = (int) $row->account_id; + $transactionCurrency = (int) $row->transaction_currency_id; + $foreignCurrency = (int) $row->foreign_currency_id; + $sumAmount = $row->sum_amount; + $sumForeignAmount = $row->sum_foreign_amount; + $journalId = (int) $row->id; + + // new amounts: + $amounts[$account][$transactionCurrency] = bcadd($amounts[$account][$transactionCurrency] ?? '0', $sumAmount ?? '0'); + $amounts[$account][$foreignCurrency] = bcadd($amounts[$account][$foreignCurrency] ?? '0', $sumForeignAmount ?? '0'); + + // first create for normal currency: + $entry = self::getAccountBalanceByJournal('balance_after_journal', $account, $journalId, $transactionCurrency); + $entry->balance = $amounts[$account][$transactionCurrency]; + $entry->save(); + + // then do foreign amount, if present: + if ($foreignCurrency > 0) { + $entry = self::getAccountBalanceByJournal('balance_after_journal', $account, $journalId, $foreignCurrency); + $entry->balance = $amounts[$account][$foreignCurrency]; + $entry->save(); + } + } + + // select transactions.account_id, transaction_journals.id, transactions.transaction_currency_id, transactions.foreign_currency_id, sum(transactions.amount), sum(transactions.foreign_amount) + // + //from transactions + // + //left join transaction_journals ON transaction_journals.id = transactions.transaction_journal_id + // + //group by account_id, transaction_journals.id, transaction_currency_id, foreign_currency_id + //order by transaction_journals.date desc + + + } }