From 344cbe4fb7434e74ebf8262985750908a4bb8f43 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 Dec 2022 07:29:06 +0100 Subject: [PATCH] Pass on the journal and update rules. --- app/Factory/PiggyBankEventFactory.php | 5 +- app/Handlers/Events/PiggyBankEventHandler.php | 14 ++++ .../PiggyBank/ModifiesPiggyBanks.php | 17 ++--- .../PiggyBankRepositoryInterface.php | 18 ++--- .../Actions/UpdatePiggybank.php | 65 ++++++++++--------- 5 files changed, 67 insertions(+), 52 deletions(-) diff --git a/app/Factory/PiggyBankEventFactory.php b/app/Factory/PiggyBankEventFactory.php index 158bd3801b..008f5348ed 100644 --- a/app/Factory/PiggyBankEventFactory.php +++ b/app/Factory/PiggyBankEventFactory.php @@ -74,8 +74,7 @@ class PiggyBankEventFactory return; } - // amount can be negative. - - $piggyRepos->addAmountToRepetition($repetition, $amount); + // amount can be negative here + $piggyRepos->addAmountToRepetition($repetition, $amount, $journal); } } diff --git a/app/Handlers/Events/PiggyBankEventHandler.php b/app/Handlers/Events/PiggyBankEventHandler.php index 646d363198..7c97c29b19 100644 --- a/app/Handlers/Events/PiggyBankEventHandler.php +++ b/app/Handlers/Events/PiggyBankEventHandler.php @@ -24,6 +24,8 @@ namespace FireflyIII\Handlers\Events; use Carbon\Carbon; use FireflyIII\Events\ChangedPiggyBankAmount; use FireflyIII\Models\PiggyBankEvent; +use Illuminate\Support\Facades\Log; + /** * Class PiggyBankEventHandler @@ -42,6 +44,18 @@ class PiggyBankEventHandler $journal = $event->transactionGroup->transactionJournals()->first(); } $date = $journal?->date ?? Carbon::now(); + + // sanity check: event must not already exist for this journal and piggy bank. + if (null !== $journal) { + $exists = PiggyBankEvent::where('piggy_bank_id', $event->piggyBank->id) + ->where('transaction_journal_id', $journal->id) + ->exists(); + if($exists) { + Log::warning('Already have event for this journal and piggy, will not create another.'); + return; + } + } + PiggyBankEvent::create( [ 'piggy_bank_id' => $event->piggyBank->id, diff --git a/app/Repositories/PiggyBank/ModifiesPiggyBanks.php b/app/Repositories/PiggyBank/ModifiesPiggyBanks.php index f68e6d3b44..0c2be397a5 100644 --- a/app/Repositories/PiggyBank/ModifiesPiggyBanks.php +++ b/app/Repositories/PiggyBank/ModifiesPiggyBanks.php @@ -30,6 +30,7 @@ use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Note; use FireflyIII\Models\PiggyBank; use FireflyIII\Models\PiggyBankRepetition; +use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\ObjectGroup\CreatesObjectGroups; use Illuminate\Database\QueryException; use Illuminate\Support\Facades\Log; @@ -44,10 +45,10 @@ trait ModifiesPiggyBanks /** * @param PiggyBank $piggyBank * @param string $amount - * + * @param TransactionJournal|null $journal * @return bool */ - public function addAmount(PiggyBank $piggyBank, string $amount): bool + public function addAmount(PiggyBank $piggyBank, string $amount, ?TransactionJournal $journal = null): bool { $repetition = $this->getRepetition($piggyBank); if (null === $repetition) { @@ -58,7 +59,7 @@ trait ModifiesPiggyBanks $repetition->save(); Log::debug('addAmount: Trigger change for positive amount.'); - event(new ChangedPiggyBankAmount($piggyBank, $amount, null, null)); + event(new ChangedPiggyBankAmount($piggyBank, $amount, $journal, null)); return true; } @@ -69,16 +70,16 @@ trait ModifiesPiggyBanks * * @return void */ - public function addAmountToRepetition(PiggyBankRepetition $repetition, string $amount): void + public function addAmountToRepetition(PiggyBankRepetition $repetition, string $amount, TransactionJournal $journal): void { Log::debug(sprintf('addAmountToRepetition: %s', $amount)); if (-1 === bccomp($amount, '0')) { Log::debug('Remove amount.'); - $this->removeAmount($repetition->piggyBank, bcmul($amount, '-1')); + $this->removeAmount($repetition->piggyBank, bcmul($amount, '-1'), $journal); } if (1 === bccomp($amount, '0')) { Log::debug('Add amount.'); - $this->addAmount($repetition->piggyBank, $amount); + $this->addAmount($repetition->piggyBank, $amount, $journal); } } @@ -149,7 +150,7 @@ trait ModifiesPiggyBanks * * @return bool */ - public function removeAmount(PiggyBank $piggyBank, string $amount): bool + public function removeAmount(PiggyBank $piggyBank, string $amount, ?TransactionJournal $journal = null): bool { $repetition = $this->getRepetition($piggyBank); if (null === $repetition) { @@ -159,7 +160,7 @@ trait ModifiesPiggyBanks $repetition->save(); Log::debug('addAmount: Trigger change for negative amount.'); - event(new ChangedPiggyBankAmount($piggyBank, bcmul($amount, '-1'), null, null)); + event(new ChangedPiggyBankAmount($piggyBank, bcmul($amount, '-1'), $journal, null)); return true; } diff --git a/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php b/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php index 63e2743a71..e3b83132b2 100644 --- a/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php +++ b/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php @@ -37,18 +37,18 @@ use Illuminate\Support\Collection; interface PiggyBankRepositoryInterface { /** - * @param PiggyBank $piggyBank - * @param string $amount - * + * @param PiggyBank $piggyBank + * @param string $amount + * @param TransactionJournal|null $journal * @return bool */ - public function addAmount(PiggyBank $piggyBank, string $amount): bool; + public function addAmount(PiggyBank $piggyBank, string $amount, ?TransactionJournal $journal = null): bool; /** * @param PiggyBankRepetition $repetition * @param string $amount */ - public function addAmountToRepetition(PiggyBankRepetition $repetition, string $amount): void; + public function addAmountToRepetition(PiggyBankRepetition $repetition, string $amount, TransactionJournal $journal): void; /** * @param PiggyBank $piggyBank @@ -197,12 +197,12 @@ interface PiggyBankRepositoryInterface public function leftOnAccount(PiggyBank $piggyBank, Carbon $date): string; /** - * @param PiggyBank $piggyBank - * @param string $amount - * + * @param PiggyBank $piggyBank + * @param string $amount + * @param TransactionJournal|null $journal * @return bool */ - public function removeAmount(PiggyBank $piggyBank, string $amount): bool; + public function removeAmount(PiggyBank $piggyBank, string $amount, ?TransactionJournal $journal = null): bool; /** * @param PiggyBank $piggyBank diff --git a/app/TransactionRules/Actions/UpdatePiggybank.php b/app/TransactionRules/Actions/UpdatePiggybank.php index 26c232362c..f11220d799 100644 --- a/app/TransactionRules/Actions/UpdatePiggybank.php +++ b/app/TransactionRules/Actions/UpdatePiggybank.php @@ -27,6 +27,7 @@ namespace FireflyIII\TransactionRules\Actions; use FireflyIII\Models\PiggyBank; use FireflyIII\Models\RuleAction; use FireflyIII\Models\Transaction; +use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use FireflyIII\User; @@ -37,14 +38,12 @@ use Log; */ class UpdatePiggybank implements ActionInterface { - - /** @var RuleAction The rule action */ - private $action; + private RuleAction $action; /** * TriggerInterface constructor. * - * @param RuleAction $action + * @param RuleAction $action */ public function __construct(RuleAction $action) { @@ -59,9 +58,10 @@ class UpdatePiggybank implements ActionInterface Log::debug(sprintf('Triggered rule action UpdatePiggybank on journal #%d', $journal['transaction_journal_id'])); // refresh the transaction type. - $user = User::find($journal['user_id']); + $user = User::find($journal['user_id']); + /** @var TransactionJournal $journalObj */ $journalObj = $user->transactionJournals()->find($journal['transaction_journal_id']); - $type = TransactionType::find((int) $journalObj->transaction_type_id); + $type = TransactionType::find((int)$journalObj->transaction_type_id); $journal['transaction_type_type'] = $type->type; if (TransactionType::TRANSFER !== $journal['transaction_type_type']) { @@ -70,10 +70,10 @@ class UpdatePiggybank implements ActionInterface return false; } - $piggyBank = $this->findPiggybank($user); + $piggyBank = $this->findPiggyBank($user); if (null === $piggyBank) { Log::info( - sprintf('No piggy bank names "%s", cant execute action #%d of rule #%d', $this->action->action_value, $this->action->id, $this->action->rule_id) + sprintf('No piggy bank named "%s", cant execute action #%d of rule #%d', $this->action->action_value, $this->action->id, $this->action->rule_id) ); return false; @@ -82,25 +82,27 @@ class UpdatePiggybank implements ActionInterface Log::debug(sprintf('Found piggy bank #%d ("%s")', $piggyBank->id, $piggyBank->name)); /** @var Transaction $source */ - $source = Transaction::where('transaction_journal_id', $journal['transaction_journal_id'])->where('amount', '<', 0)->first(); /** @var Transaction $destination */ - $destination = Transaction::where('transaction_journal_id', $journal['transaction_journal_id'])->where('amount', '>', 0)->first(); + $source = $journalObj->transactions()->where('amount', '<', 0)->first(); + $destination = $journalObj->transactions()->where('amount', '>', 0)->first(); - if ((int) $source->account_id === (int) $piggyBank->account_id) { - Log::debug('Piggy bank account is linked to source, so remove amount.'); + if ((int)$source->account_id === (int)$piggyBank->account_id) { + Log::debug('Piggy bank account is linked to source, so remove amount from piggy bank.'); $this->removeAmount($journal, $piggyBank, $destination->amount); return true; } - if ((int) $destination->account_id === (int) $piggyBank->account_id) { - Log::debug('Piggy bank account is linked to source, so add amount.'); + if ((int)$destination->account_id === (int)$piggyBank->account_id) { + Log::debug('Piggy bank account is linked to source, so add amount to piggy bank.'); $this->addAmount($journal, $piggyBank, $destination->amount); return true; } Log::info( sprintf( - 'Piggy bank is not linked to source ("#%d") or destination ("#%d"), so no action will be taken.', $source->account_id, $destination->account_id + 'Piggy bank is not linked to source ("#%d") or destination ("#%d"), so no action will be taken.', + $source->account_id, + $destination->account_id ) ); @@ -108,30 +110,30 @@ class UpdatePiggybank implements ActionInterface } /** - * @param User $user + * @param User $user * * @return PiggyBank|null */ - private function findPiggybank(User $user): ?PiggyBank + private function findPiggyBank(User $user): ?PiggyBank { return $user->piggyBanks()->where('piggy_banks.name', $this->action->action_value)->first(); } /** - * @param array $journalArray - * @param PiggyBank $piggyBank - * @param string $amount + * @param PiggyBank $piggyBank + * @param TransactionJournal $journal + * @param string $amount + * @return void */ - private function removeAmount(array $journalArray, PiggyBank $piggyBank, string $amount): void + private function removeAmount(PiggyBank $piggyBank, TransactionJournal $journal, string $amount): void { - $user = User::find($journalArray['user_id']); - $journal = $user->transactionJournals()->find($journalArray['transaction_journal_id']); $repository = app(PiggyBankRepositoryInterface::class); $repository->setUser($journal->user); - // how much can we remove from piggy bank? + // how much can we remove from this piggy bank? $toRemove = $repository->getCurrentAmount($piggyBank); Log::debug(sprintf('Amount is %s, max to remove is %s', $amount, $toRemove)); + // if $amount is bigger than $toRemove, shrink it. $amount = -1 === bccomp($amount, $toRemove) ? $amount : $toRemove; Log::debug(sprintf('Amount is now %s', $amount)); @@ -151,18 +153,17 @@ class UpdatePiggybank implements ActionInterface } Log::debug(sprintf('Will now remove %s from piggy bank.', $amount)); - $repository->removeAmount($piggyBank, $amount); + $repository->removeAmount($piggyBank, $amount, $journal); } /** - * @param array $journalArray - * @param PiggyBank $piggyBank - * @param string $amount + * @param PiggyBank $piggyBank + * @param TransactionJournal $journal + * @param string $amount + * @return void */ - private function addAmount(array $journalArray, PiggyBank $piggyBank, string $amount): void + private function addAmount(PiggyBank $piggyBank, TransactionJournal $journal, string $amount): void { - $user = User::find($journalArray['user_id']); - $journal = $user->transactionJournals()->find($journalArray['transaction_journal_id']); $repository = app(PiggyBankRepositoryInterface::class); $repository->setUser($journal->user); @@ -189,6 +190,6 @@ class UpdatePiggybank implements ActionInterface } Log::debug(sprintf('Will now add %s to piggy bank.', $amount)); - $repository->addAmount($piggyBank, $amount); + $repository->addAmount($piggyBank, $amount, $journal); } }