From 3924781797cbe8007964d77ffcef3d536b3e85de Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 2 May 2025 06:45:34 +0200 Subject: [PATCH] Make sure running balance also updates when transactions are removed. --- .../Events/DestroyedGroupEventHandler.php | 20 +++++- .../Events/UpdatedGroupEventHandler.php | 65 +++++++++++-------- app/Providers/EventServiceProvider.php | 2 +- .../Internal/Update/GroupUpdateService.php | 45 ++++++------- .../Models/AccountBalanceCalculator.php | 52 +++++++-------- 5 files changed, 105 insertions(+), 79 deletions(-) diff --git a/app/Handlers/Events/DestroyedGroupEventHandler.php b/app/Handlers/Events/DestroyedGroupEventHandler.php index 0b28a7fb10..5b55bcd66b 100644 --- a/app/Handlers/Events/DestroyedGroupEventHandler.php +++ b/app/Handlers/Events/DestroyedGroupEventHandler.php @@ -27,15 +27,24 @@ namespace FireflyIII\Handlers\Events; use FireflyIII\Enums\WebhookTrigger; use FireflyIII\Events\DestroyedTransactionGroup; use FireflyIII\Events\RequestedSendWebhookMessages; +use FireflyIII\Events\UpdatedTransactionGroup; use FireflyIII\Generator\Webhook\MessageGeneratorInterface; +use FireflyIII\Support\Models\AccountBalanceCalculator; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Log; /** * Class DestroyedGroupEventHandler */ class DestroyedGroupEventHandler { - public function triggerWebhooks(DestroyedTransactionGroup $destroyedGroupEvent): void + public function runAllHandlers(DestroyedTransactionGroup $event): void + { + $this->triggerWebhooks($event); + $this->updateRunningBalance($event); + } + + private function triggerWebhooks(DestroyedTransactionGroup $destroyedGroupEvent): void { app('log')->debug('DestroyedTransactionGroup:triggerWebhooks'); $group = $destroyedGroupEvent->transactionGroup; @@ -50,4 +59,13 @@ class DestroyedGroupEventHandler event(new RequestedSendWebhookMessages()); } + + private function updateRunningBalance(DestroyedTransactionGroup $event): void + { + Log::debug(__METHOD__); + $group = $event->transactionGroup; + foreach ($group->transactionJournals as $journal) { + AccountBalanceCalculator::recalculateForJournal($journal); + } + } } diff --git a/app/Handlers/Events/UpdatedGroupEventHandler.php b/app/Handlers/Events/UpdatedGroupEventHandler.php index 723cb9743a..88198aa520 100644 --- a/app/Handlers/Events/UpdatedGroupEventHandler.php +++ b/app/Handlers/Events/UpdatedGroupEventHandler.php @@ -33,8 +33,10 @@ use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\RuleGroup\RuleGroupRepositoryInterface; use FireflyIII\Services\Internal\Support\CreditRecalculateService; +use FireflyIII\Support\Models\AccountBalanceCalculator; use FireflyIII\TransactionRules\Engine\RuleEngineInterface; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Log; /** * Class UpdatedGroupEventHandler @@ -47,6 +49,7 @@ class UpdatedGroupEventHandler $this->processRules($event); $this->recalculateCredit($event); $this->triggerWebhooks($event); + $this->updateRunningBalance($event); } @@ -56,29 +59,29 @@ class UpdatedGroupEventHandler private function processRules(UpdatedTransactionGroup $updatedGroupEvent): void { if (false === $updatedGroupEvent->applyRules) { - app('log')->info(sprintf('Will not run rules on group #%d', $updatedGroupEvent->transactionGroup->id)); + Log::info(sprintf('Will not run rules on group #%d', $updatedGroupEvent->transactionGroup->id)); return; } - $journals = $updatedGroupEvent->transactionGroup->transactionJournals; - $array = []; + $journals = $updatedGroupEvent->transactionGroup->transactionJournals; + $array = []; /** @var TransactionJournal $journal */ foreach ($journals as $journal) { $array[] = $journal->id; } - $journalIds = implode(',', $array); - app('log')->debug(sprintf('Add local operator for journal(s): %s', $journalIds)); + $journalIds = implode(',', $array); + Log::debug(sprintf('Add local operator for journal(s): %s', $journalIds)); // collect rules: $ruleGroupRepository = app(RuleGroupRepositoryInterface::class); $ruleGroupRepository->setUser($updatedGroupEvent->transactionGroup->user); - $groups = $ruleGroupRepository->getRuleGroupsWithRules('update-journal'); + $groups = $ruleGroupRepository->getRuleGroupsWithRules('update-journal'); // file rule engine. - $newRuleEngine = app(RuleEngineInterface::class); + $newRuleEngine = app(RuleEngineInterface::class); $newRuleEngine->setUser($updatedGroupEvent->transactionGroup->user); $newRuleEngine->addOperator(['type' => 'journal_id', 'value' => $journalIds]); $newRuleEngine->setRuleGroups($groups); @@ -87,7 +90,7 @@ class UpdatedGroupEventHandler private function recalculateCredit(UpdatedTransactionGroup $event): void { - $group = $event->transactionGroup; + $group = $event->transactionGroup; /** @var CreditRecalculateService $object */ $object = app(CreditRecalculateService::class); @@ -97,14 +100,14 @@ class UpdatedGroupEventHandler private function triggerWebhooks(UpdatedTransactionGroup $updatedGroupEvent): void { - app('log')->debug(__METHOD__); - $group = $updatedGroupEvent->transactionGroup; + Log::debug(__METHOD__); + $group = $updatedGroupEvent->transactionGroup; if (false === $updatedGroupEvent->fireWebhooks) { - app('log')->info(sprintf('Will not fire webhooks for transaction group #%d', $group->id)); + Log::info(sprintf('Will not fire webhooks for transaction group #%d', $group->id)); return; } - $user = $group->user; + $user = $group->user; /** @var MessageGeneratorInterface $engine */ $engine = app(MessageGeneratorInterface::class); @@ -121,47 +124,53 @@ class UpdatedGroupEventHandler */ public function unifyAccounts(UpdatedTransactionGroup $updatedGroupEvent): void { - $group = $updatedGroupEvent->transactionGroup; + $group = $updatedGroupEvent->transactionGroup; if (1 === $group->transactionJournals->count()) { return; } // first journal: /** @var null|TransactionJournal $first */ - $first = $group->transactionJournals() - ->orderBy('transaction_journals.date', 'DESC') - ->orderBy('transaction_journals.order', 'ASC') - ->orderBy('transaction_journals.id', 'DESC') - ->orderBy('transaction_journals.description', 'DESC') - ->first() - ; + $first = $group->transactionJournals() + ->orderBy('transaction_journals.date', 'DESC') + ->orderBy('transaction_journals.order', 'ASC') + ->orderBy('transaction_journals.id', 'DESC') + ->orderBy('transaction_journals.description', 'DESC') + ->first(); if (null === $first) { - app('log')->warning(sprintf('Group #%d has no transaction journals.', $group->id)); + Log::warning(sprintf('Group #%d has no transaction journals.', $group->id)); return; } - $all = $group->transactionJournals()->get()->pluck('id')->toArray(); + $all = $group->transactionJournals()->get()->pluck('id')->toArray(); /** @var Account $sourceAccount */ $sourceAccount = $first->transactions()->where('amount', '<', '0')->first()->account; /** @var Account $destAccount */ - $destAccount = $first->transactions()->where('amount', '>', '0')->first()->account; + $destAccount = $first->transactions()->where('amount', '>', '0')->first()->account; - $type = $first->transactionType->type; + $type = $first->transactionType->type; if (TransactionTypeEnum::TRANSFER->value === $type || TransactionTypeEnum::WITHDRAWAL->value === $type) { // set all source transactions to source account: Transaction::whereIn('transaction_journal_id', $all) - ->where('amount', '<', 0)->update(['account_id' => $sourceAccount->id]) - ; + ->where('amount', '<', 0)->update(['account_id' => $sourceAccount->id]); } if (TransactionTypeEnum::TRANSFER->value === $type || TransactionTypeEnum::DEPOSIT->value === $type) { // set all destination transactions to destination account: Transaction::whereIn('transaction_journal_id', $all) - ->where('amount', '>', 0)->update(['account_id' => $destAccount->id]) - ; + ->where('amount', '>', 0)->update(['account_id' => $destAccount->id]); + } + } + + private function updateRunningBalance(UpdatedTransactionGroup $event): void + { + Log::debug(__METHOD__); + $group = $event->transactionGroup; + foreach ($group->transactionJournals as $journal) { + AccountBalanceCalculator::recalculateForJournal($journal); } } } diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php index 1e9d655b0d..fe465411ea 100644 --- a/app/Providers/EventServiceProvider.php +++ b/app/Providers/EventServiceProvider.php @@ -176,7 +176,7 @@ class EventServiceProvider extends ServiceProvider 'FireflyIII\Handlers\Events\UpdatedGroupEventHandler@runAllHandlers', ], DestroyedTransactionGroup::class => [ - 'FireflyIII\Handlers\Events\DestroyedGroupEventHandler@triggerWebhooks', + 'FireflyIII\Handlers\Events\DestroyedGroupEventHandler@runAllHandlers', ], // API related events: AccessTokenCreated::class => [ diff --git a/app/Services/Internal/Update/GroupUpdateService.php b/app/Services/Internal/Update/GroupUpdateService.php index 2d30599e54..45bcf06d82 100644 --- a/app/Services/Internal/Update/GroupUpdateService.php +++ b/app/Services/Internal/Update/GroupUpdateService.php @@ -31,6 +31,7 @@ use FireflyIII\Factory\TransactionJournalFactory; use FireflyIII\Models\TransactionGroup; use FireflyIII\Models\TransactionJournal; use FireflyIII\Services\Internal\Destroy\JournalDestroyService; +use Illuminate\Support\Facades\Log; /** * Class GroupUpdateService @@ -45,14 +46,14 @@ class GroupUpdateService */ public function update(TransactionGroup $transactionGroup, array $data): TransactionGroup { - app('log')->debug(sprintf('Now in %s', __METHOD__)); - app('log')->debug('Now in group update service', $data); + Log::debug(sprintf('Now in %s', __METHOD__)); + Log::debug('Now in group update service', $data); /** @var array $transactions */ $transactions = $data['transactions'] ?? []; // update group name. if (array_key_exists('group_title', $data)) { - app('log')->debug(sprintf('Update transaction group #%d title.', $transactionGroup->id)); + Log::debug(sprintf('Update transaction group #%d title.', $transactionGroup->id)); $oldTitle = $transactionGroup->title; $transactionGroup->title = $data['group_title']; $transactionGroup->save(); @@ -68,7 +69,7 @@ class GroupUpdateService } if (0 === count($transactions)) { - app('log')->debug('No transactions submitted, do nothing.'); + Log::debug('No transactions submitted, do nothing.'); return $transactionGroup; } @@ -76,7 +77,7 @@ class GroupUpdateService if (1 === count($transactions) && 1 === $transactionGroup->transactionJournals()->count()) { /** @var TransactionJournal $first */ $first = $transactionGroup->transactionJournals()->first(); - app('log')->debug( + Log::debug( sprintf('Will now update journal #%d (only journal in group #%d)', $first->id, $transactionGroup->id) ); $this->updateTransactionJournal($transactionGroup, $first, reset($transactions)); @@ -87,14 +88,14 @@ class GroupUpdateService return $transactionGroup; } - app('log')->debug('Going to update split group.'); + Log::debug('Going to update split group.'); $existing = $transactionGroup->transactionJournals->pluck('id')->toArray(); $updated = $this->updateTransactions($transactionGroup, $transactions); - app('log')->debug('Array of updated IDs: ', $updated); + Log::debug('Array of updated IDs: ', $updated); if (0 === count($updated)) { - app('log')->error('There were no transactions updated or created. Will not delete anything.'); + Log::error('There were no transactions updated or created. Will not delete anything.'); $transactionGroup->touch(); $transactionGroup->refresh(); app('preferences')->mark(); @@ -103,7 +104,7 @@ class GroupUpdateService } $result = array_diff($existing, $updated); - app('log')->debug('Result of DIFF: ', $result); + Log::debug('Result of DIFF: ', $result); if (count($result) > 0) { /** @var string $deletedId */ foreach ($result as $deletedId) { @@ -131,7 +132,7 @@ class GroupUpdateService TransactionJournal $journal, array $data ): void { - app('log')->debug(sprintf('Now in %s', __METHOD__)); + Log::debug(sprintf('Now in %s', __METHOD__)); if (0 === count($data)) { return; } @@ -153,7 +154,7 @@ class GroupUpdateService */ private function updateTransactions(TransactionGroup $transactionGroup, array $transactions): array { - app('log')->debug(sprintf('Now in %s', __METHOD__)); + Log::debug(sprintf('Now in %s', __METHOD__)); // updated or created transaction journals: $updated = []; @@ -162,17 +163,17 @@ class GroupUpdateService * @var array $transaction */ foreach ($transactions as $index => $transaction) { - app('log')->debug(sprintf('Now at #%d of %d', $index + 1, count($transactions)), $transaction); + Log::debug(sprintf('Now at #%d of %d', $index + 1, count($transactions)), $transaction); $journalId = (int) ($transaction['transaction_journal_id'] ?? 0); /** @var null|TransactionJournal $journal */ $journal = $transactionGroup->transactionJournals()->find($journalId); if (null === $journal) { - app('log')->debug('This entry has no existing journal: make a new split.'); + Log::debug('This entry has no existing journal: make a new split.'); // force the transaction type on the transaction data. // by plucking it from another journal in the group: if (!array_key_exists('type', $transaction)) { - app('log')->debug('No transaction type is indicated.'); + Log::debug('No transaction type is indicated.'); /** @var null|TransactionJournal $randomJournal */ $randomJournal = $transactionGroup->transactionJournals()->inRandomOrder()->with( @@ -180,24 +181,24 @@ class GroupUpdateService )->first(); if (null !== $randomJournal) { $transaction['type'] = $randomJournal->transactionType->type; - app('log')->debug(sprintf('Transaction type set to %s.', $transaction['type'])); + Log::debug(sprintf('Transaction type set to %s.', $transaction['type'])); } } - app('log')->debug('Call createTransactionJournal'); + Log::debug('Call createTransactionJournal'); $newJournal = $this->createTransactionJournal($transactionGroup, $transaction); - app('log')->debug('Done calling createTransactionJournal'); + Log::debug('Done calling createTransactionJournal'); if (null !== $newJournal) { $updated[] = $newJournal->id; } if (null === $newJournal) { - app('log')->error('createTransactionJournal returned NULL, indicating something went wrong.'); + Log::error('createTransactionJournal returned NULL, indicating something went wrong.'); } } if (null !== $journal) { - app('log')->debug('Call updateTransactionJournal'); + Log::debug('Call updateTransactionJournal'); $this->updateTransactionJournal($transactionGroup, $journal, $transaction); $updated[] = $journal->id; - app('log')->debug('Done calling updateTransactionJournal'); + Log::debug('Done calling updateTransactionJournal'); } } @@ -223,8 +224,8 @@ class GroupUpdateService try { $collection = $factory->create($submission); } catch (FireflyException $e) { - app('log')->error($e->getMessage()); - app('log')->error($e->getTraceAsString()); + Log::error($e->getMessage()); + Log::error($e->getTraceAsString()); throw new FireflyException( sprintf('Could not create new transaction journal: %s', $e->getMessage()), diff --git a/app/Support/Models/AccountBalanceCalculator.php b/app/Support/Models/AccountBalanceCalculator.php index b51af3d7f7..11ee28f04b 100644 --- a/app/Support/Models/AccountBalanceCalculator.php +++ b/app/Support/Models/AccountBalanceCalculator.php @@ -71,15 +71,14 @@ class AccountBalanceCalculator $balances = []; $count = 0; $query = Transaction::leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') - ->whereNull('transactions.deleted_at') - ->whereNull('transaction_journals.deleted_at') + ->whereNull('transactions.deleted_at') + ->whereNull('transaction_journals.deleted_at') // this order is the same as GroupCollector, but in the exact reverse. - ->orderBy('transaction_journals.date', 'asc') - ->orderBy('transaction_journals.order', 'desc') - ->orderBy('transaction_journals.id', 'asc') - ->orderBy('transaction_journals.description', 'asc') - ->orderBy('transactions.amount', 'asc') - ; + ->orderBy('transaction_journals.date', 'asc') + ->orderBy('transaction_journals.order', 'desc') + ->orderBy('transaction_journals.id', 'asc') + ->orderBy('transaction_journals.description', 'asc') + ->orderBy('transactions.amount', 'asc'); if ($accounts->count() > 0) { $query->whereIn('transactions.account_id', $accounts->pluck('id')->toArray()); } @@ -88,7 +87,7 @@ class AccountBalanceCalculator $query->where('transaction_journals.date', '>=', $notBefore); } - $set = $query->get(['transactions.id', 'transactions.balance_dirty', 'transactions.transaction_currency_id', 'transaction_journals.date', 'transactions.account_id', 'transactions.amount']); + $set = $query->get(['transactions.id', 'transactions.balance_dirty', 'transactions.transaction_currency_id', 'transaction_journals.date', 'transactions.account_id', 'transactions.amount']); Log::debug(sprintf('Counted %d transaction(s)', $set->count())); // the balance value is an array. @@ -101,8 +100,8 @@ class AccountBalanceCalculator $balances[$entry->account_id][$entry->transaction_currency_id] ??= [$this->getLatestBalance($entry->account_id, $entry->transaction_currency_id, $notBefore), null]; // before and after are easy: - $before = $balances[$entry->account_id][$entry->transaction_currency_id][0]; - $after = bcadd($before, $entry->amount); + $before = $balances[$entry->account_id][$entry->transaction_currency_id][0]; + $after = bcadd($before, $entry->amount); if (true === $entry->balance_dirty || $accounts->count() > 0) { // update the transaction: $entry->balance_before = $before; @@ -128,18 +127,17 @@ class AccountBalanceCalculator return '0'; } Log::debug(sprintf('getLatestBalance: notBefore date is "%s", calculating', $notBefore->format('Y-m-d'))); - $query = Transaction::leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') - ->whereNull('transactions.deleted_at') - ->where('transaction_journals.transaction_currency_id', $currencyId) - ->whereNull('transaction_journals.deleted_at') + $query = Transaction::leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id') + ->whereNull('transactions.deleted_at') + ->where('transaction_journals.transaction_currency_id', $currencyId) + ->whereNull('transaction_journals.deleted_at') // this order is the same as GroupCollector - ->orderBy('transaction_journals.date', 'DESC') - ->orderBy('transaction_journals.order', 'ASC') - ->orderBy('transaction_journals.id', 'DESC') - ->orderBy('transaction_journals.description', 'DESC') - ->orderBy('transactions.amount', 'DESC') - ->where('transactions.account_id', $accountId) - ; + ->orderBy('transaction_journals.date', 'DESC') + ->orderBy('transaction_journals.order', 'ASC') + ->orderBy('transaction_journals.id', 'DESC') + ->orderBy('transaction_journals.description', 'DESC') + ->orderBy('transactions.amount', 'DESC') + ->where('transactions.account_id', $accountId); $notBefore->startOfDay(); $query->where('transaction_journals.date', '<', $notBefore); @@ -171,7 +169,7 @@ class AccountBalanceCalculator */ foreach ($currencies as $currencyId => $balance) { /** @var null|TransactionCurrency $currency */ - $currency = TransactionCurrency::find($currencyId); + $currency = TransactionCurrency::find($currencyId); if (null === $currency) { Log::error(sprintf('Could not find currency #%d, will not save account balance.', $currencyId)); @@ -199,13 +197,13 @@ class AccountBalanceCalculator public static function recalculateForJournal(TransactionJournal $transactionJournal): void { Log::debug(__METHOD__); - $object = new self(); + $object = new self(); - // recalculate the involved accounts: - $accounts = new Collection(); + $set = []; foreach ($transactionJournal->transactions as $transaction) { - $accounts->push($transaction->account); + $set[$transaction->account_id] = $transaction->account; } + $accounts = new Collection($set); $object->optimizedCalculation($accounts, $transactionJournal->date); } }