diff --git a/app/TransactionRules/Actions/UpdatePiggybank.php b/app/TransactionRules/Actions/UpdatePiggybank.php index 37f4db13a4..b2774a5630 100644 --- a/app/TransactionRules/Actions/UpdatePiggybank.php +++ b/app/TransactionRules/Actions/UpdatePiggybank.php @@ -26,13 +26,14 @@ namespace FireflyIII\TransactionRules\Actions; use FireflyIII\Events\Model\Rule\RuleActionFailedOnArray; use FireflyIII\Events\TriggeredAuditLog; -use FireflyIII\Exceptions\FireflyException; +use FireflyIII\Models\Account; use FireflyIII\Models\PiggyBank; use FireflyIII\Models\RuleAction; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use FireflyIII\User; +use Illuminate\Support\Facades\Log; /** * Class UpdatePiggybank @@ -53,18 +54,18 @@ class UpdatePiggybank implements ActionInterface { $actionValue = $this->action->getValue($journal); - app('log')->debug(sprintf('Triggered rule action UpdatePiggybank on journal #%d', $journal['transaction_journal_id'])); + Log::debug(sprintf('Triggered rule action UpdatePiggybank on journal #%d', $journal['transaction_journal_id'])); // refresh the transaction type. /** @var User $user */ - $user = User::find($journal['user_id']); + $user = User::find($journal['user_id']); /** @var TransactionJournal $journalObj */ - $journalObj = $user->transactionJournals()->find($journal['transaction_journal_id']); + $journalObj = $user->transactionJournals()->find($journal['transaction_journal_id']); - $piggyBank = $this->findPiggyBank($user, $actionValue); + $piggyBank = $this->findPiggyBank($user, $actionValue); if (null === $piggyBank) { - app('log')->info( + Log::info( sprintf('No piggy bank named "%s", cant execute action #%d of rule #%d', $actionValue, $this->action->id, $this->action->rule_id) ); event(new RuleActionFailedOnArray($this->action, $journal, trans('rules.cannot_find_piggy', ['name' => $actionValue]))); @@ -72,20 +73,19 @@ class UpdatePiggybank implements ActionInterface return false; } - app('log')->debug(sprintf('Found piggy bank #%d ("%s")', $piggyBank->id, $piggyBank->name)); - - /** @var Transaction $source */ - $source = $journalObj->transactions()->where('amount', '<', 0)->first(); + Log::debug(sprintf('Found piggy bank #%d ("%s")', $piggyBank->id, $piggyBank->name)); /** @var Transaction $destination */ $destination = $journalObj->transactions()->where('amount', '>', 0)->first(); - if ($source->account_id === $piggyBank->account_id) { - app('log')->debug('Piggy bank account is linked to source, so remove amount from piggy bank.'); - - throw new FireflyException('Reference the correct account here.'); - $this->removeAmount($piggyBank, $journal, $journalObj, $destination->amount); + $accounts = $this->getAccounts($journalObj); + Log::debug(sprintf('Source account is #%d: "%s"', $accounts['source']->id, $accounts['source']->name)); + Log::debug(sprintf('Destination account is #%d: "%s"', $accounts['destination']->id, $accounts['source']->name)); + // if connected to source but not to destination, needs to be removed from source account connected to piggy bank. + if ($this->isConnected($piggyBank, $accounts['source']) && !$this->isConnected($piggyBank, $accounts['destination'])) { + Log::debug('Piggy bank account is linked to source, so remove amount from piggy bank.'); + $this->removeAmount($piggyBank, $journal, $journalObj, $accounts['source'], $destination->amount); event( new TriggeredAuditLog( $this->action->rule, @@ -103,9 +103,11 @@ class UpdatePiggybank implements ActionInterface return true; } - if ($destination->account_id === $piggyBank->account_id) { - app('log')->debug('Piggy bank account is linked to source, so add amount to piggy bank.'); - $this->addAmount($piggyBank, $journal, $journalObj, $destination->amount); + + // if connected to destination but not to source, needs to be removed from source account connected to piggy bank. + if (!$this->isConnected($piggyBank, $accounts['source']) && $this->isConnected($piggyBank, $accounts['destination'])) { + Log::debug('Piggy bank account is linked to source, so add amount to piggy bank.'); + $this->addAmount($piggyBank, $journal, $journalObj, $accounts['destination'], $destination->amount); event( new TriggeredAuditLog( @@ -125,13 +127,12 @@ class UpdatePiggybank implements ActionInterface return true; } - app('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 - ) - ); + if ($this->isConnected($piggyBank, $accounts['source']) && $this->isConnected($piggyBank, $accounts['destination'])) { + Log::info(sprintf('Piggy bank is linked to BOTH source ("#%d") and destination ("#%d"), so no action will be taken.', $accounts['source']->id, $accounts['destination']->id)); + event(new RuleActionFailedOnArray($this->action, $journal, trans('rules.no_link_piggy', ['name' => $actionValue]))); + return false; + } + Log::info(sprintf('Piggy bank is not linked to source ("#%d") or destination ("#%d"), so no action will be taken.', $accounts['source']->id, $accounts['destination']->id)); event(new RuleActionFailedOnArray($this->action, $journal, trans('rules.no_link_piggy', ['name' => $actionValue]))); return false; @@ -139,80 +140,101 @@ class UpdatePiggybank implements ActionInterface private function findPiggyBank(User $user, string $name): ?PiggyBank { - return $user->piggyBanks()->where('piggy_banks.name', $name)->first(); + /** @var PiggyBankRepositoryInterface $repository */ + $repository = app(PiggyBankRepositoryInterface::class); + $repository->setUser($user); + return $repository->findByName($name); } - private function removeAmount(PiggyBank $piggyBank, array $array, TransactionJournal $journal, string $amount): void + private function removeAmount(PiggyBank $piggyBank, array $array, TransactionJournal $journal, Account $account, string $amount): void { $repository = app(PiggyBankRepositoryInterface::class); $repository->setUser($journal->user); // how much can we remove from this piggy bank? - $toRemove = $repository->getCurrentAmount($piggyBank); - app('log')->debug(sprintf('Amount is %s, max to remove is %s', $amount, $toRemove)); + $toRemove = $repository->getCurrentAmount($piggyBank, $account); + 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; - app('log')->debug(sprintf('Amount is now %s', $amount)); + $amount = -1 === bccomp($amount, $toRemove) ? $amount : $toRemove; + Log::debug(sprintf('Amount is now %s', $amount)); // if amount is zero, stop. if (0 === bccomp('0', $amount)) { - app('log')->warning('Amount left is zero, stop.'); + Log::warning('Amount left is zero, stop.'); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_remove_zero_piggy', ['name' => $piggyBank->name]))); return; } - // make sure we can remove amount: - throw new FireflyException('Reference the correct account here.'); - if (false === $repository->canRemoveAmount($piggyBank, $amount)) { - app('log')->warning(sprintf('Cannot remove %s from piggy bank.', $amount)); + if (false === $repository->canRemoveAmount($piggyBank, $account, $amount)) { + Log::warning(sprintf('Cannot remove %s from piggy bank.', $amount)); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_remove_from_piggy', ['amount' => $amount, 'name' => $piggyBank->name]))); return; } - app('log')->debug(sprintf('Will now remove %s from piggy bank.', $amount)); + Log::debug(sprintf('Will now remove %s from piggy bank.', $amount)); - throw new FireflyException('Reference the correct account here.'); - $repository->removeAmount($piggyBank, $amount, $journal); + $repository->removeAmount($piggyBank, $account, $amount, $journal); } - private function addAmount(PiggyBank $piggyBank, array $array, TransactionJournal $journal, string $amount): void + private function addAmount(PiggyBank $piggyBank, array $array, TransactionJournal $journal, Account $account, string $amount): void { $repository = app(PiggyBankRepositoryInterface::class); $repository->setUser($journal->user); // how much can we add to the piggy bank? if (0 !== bccomp($piggyBank->target_amount, '0')) { - $toAdd = bcsub($piggyBank->target_amount, $repository->getCurrentAmount($piggyBank)); - app('log')->debug(sprintf('Max amount to add to piggy bank is %s, amount is %s', $toAdd, $amount)); + $toAdd = bcsub($piggyBank->target_amount, $repository->getCurrentAmount($piggyBank, $account)); + Log::debug(sprintf('Max amount to add to piggy bank is %s, amount is %s', $toAdd, $amount)); // update amount to fit: $amount = -1 === bccomp($amount, $toAdd) ? $amount : $toAdd; - app('log')->debug(sprintf('Amount is now %s', $amount)); + Log::debug(sprintf('Amount is now %s', $amount)); } if (0 === bccomp($piggyBank->target_amount, '0')) { - app('log')->debug('Target amount is zero, can add anything.'); + Log::debug('Target amount is zero, can add anything.'); } // if amount is zero, stop. if (0 === bccomp('0', $amount)) { - app('log')->warning('Amount left is zero, stop.'); + Log::warning('Amount left is zero, stop.'); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_add_zero_piggy', ['name' => $piggyBank->name]))); return; } - // make sure we can add amount: - throw new FireflyException('Reference the correct account here.'); - if (false === $repository->canAddAmount($piggyBank, $amount)) { - app('log')->warning(sprintf('Cannot add %s to piggy bank.', $amount)); + if (false === $repository->canAddAmount($piggyBank, $account, $amount)) { + Log::warning(sprintf('Cannot add %s to piggy bank.', $amount)); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_add_to_piggy', ['amount' => $amount, 'name' => $piggyBank->name]))); return; } - app('log')->debug(sprintf('Will now add %s to piggy bank.', $amount)); + Log::debug(sprintf('Will now add %s to piggy bank.', $amount)); - $repository->addAmount($piggyBank, $amount, $journal); + $repository->addAmount($piggyBank, $account, $amount, $journal); } + + private function getAccounts(TransactionJournal $journal): array + { + return [ + 'source' => $journal->transactions()->where('amount', '<', '0')->first()?->account, + 'destination' => $journal->transactions()->where('amount', '>', '0')->first()?->account, + ]; + } + + private function isConnected(PiggyBank $piggyBank, ?Account $link): bool + { + if (null === $link) { + return false; + } + foreach ($piggyBank->accounts as $account) { + if ($account->id === $link->id) { + return true; + } + } + Log::debug(sprintf('Piggy bank is not connected to account #%d "%s"', $account->id, $account->name)); + return false; + } + } diff --git a/resources/lang/en_US/firefly.php b/resources/lang/en_US/firefly.php index bd38b2404d..09db945ef0 100644 --- a/resources/lang/en_US/firefly.php +++ b/resources/lang/en_US/firefly.php @@ -217,9 +217,9 @@ return [ 'button_register' => 'Register', 'authorization' => 'Authorization', 'active_bills_only' => 'active subscription only', - 'active_bills_only_total' => 'all active subscription', - 'active_exp_bills_only' => 'active and expected subscription only', - 'active_exp_bills_only_total' => 'all active expected subscription only', + 'active_bills_only_total' => 'all active subscriptions', + 'active_exp_bills_only' => 'active and expected subscriptions only', + 'active_exp_bills_only_total' => 'all active expected subscriptions only', 'per_period_sum_1D' => 'Expected daily costs', 'per_period_sum_1W' => 'Expected weekly costs', 'per_period_sum_1M' => 'Expected monthly costs', @@ -1328,7 +1328,7 @@ return [ 'rule_for_bill_title' => 'Auto-generated rule for subscription ":name"', 'rule_for_bill_description' => 'This rule is auto-generated to try to match subscription ":name".', 'create_rule_for_bill' => 'Create a new rule for subscription ":name"', - 'create_rule_for_bill_txt' => 'You have just created a new subscription called ":name", congratulations!Firefly III can automagically match new withdrawals to this subscription. For example, whenever you pay your rent, the subscription "rent" will be linked to the expense. This way, Firefly III can accurately show you which subscriptions are due and which ones aren\'t. In order to do so, a new rule must be created. Firefly III has filled in some sensible defaults for you. Please make sure these are correct. If these values are correct, Firefly III will automatically link the correct withdrawal to the correct subscription. Please check out the triggers to see if they are correct, and add some if they\'re wrong.', + 'create_rule_for_bill_txt' => 'You have just created a new subscription called ":name", congratulations! Firefly III can automagically match new withdrawals to this subscription. For example, whenever you pay your rent, the subscription "rent" will be linked to the expense. This way, Firefly III can accurately show you which subscriptions are due and which ones aren\'t. In order to do so, a new rule must be created. Firefly III has filled in some sensible defaults for you. Please make sure these are correct. If these values are correct, Firefly III will automatically link the correct withdrawal to the correct subscription. Please check out the triggers to see if they are correct, and add some if they\'re wrong.', 'new_rule_for_bill_title' => 'Rule for subscription ":name"', 'new_rule_for_bill_description' => 'This rule marks transactions for subscription ":name".', diff --git a/resources/lang/en_US/rules.php b/resources/lang/en_US/rules.php index 4b1324d2ef..909f45cd7c 100644 --- a/resources/lang/en_US/rules.php +++ b/resources/lang/en_US/rules.php @@ -66,6 +66,7 @@ return [ 'cannot_find_destination_transaction_account' => 'Firefly III can\'t find the destination transaction account', 'cannot_find_piggy' => 'Firefly III can\'t find a piggy bank named ":name"', 'no_link_piggy' => 'This transaction\'s accounts are not linked to the piggy bank, so no action will be taken', + 'both_link_piggy' => 'This transaction\'s accounts are both linked to the piggy bank, so no action will be taken', 'cannot_unlink_tag' => 'Tag ":tag" isn\'t linked to this transaction', 'cannot_find_budget' => 'Firefly III can\'t find budget ":name"', 'cannot_find_category' => 'Firefly III can\'t find category ":name"',