Fix piggy bank rule action

This commit is contained in:
James Cole
2025-01-01 16:43:31 +01:00
parent 6fde693e7a
commit ed92cbd4b8
3 changed files with 78 additions and 55 deletions

View File

@@ -26,13 +26,14 @@ namespace FireflyIII\TransactionRules\Actions;
use FireflyIII\Events\Model\Rule\RuleActionFailedOnArray; use FireflyIII\Events\Model\Rule\RuleActionFailedOnArray;
use FireflyIII\Events\TriggeredAuditLog; use FireflyIII\Events\TriggeredAuditLog;
use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Account;
use FireflyIII\Models\PiggyBank; use FireflyIII\Models\PiggyBank;
use FireflyIII\Models\RuleAction; use FireflyIII\Models\RuleAction;
use FireflyIII\Models\Transaction; use FireflyIII\Models\Transaction;
use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionJournal;
use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface;
use FireflyIII\User; use FireflyIII\User;
use Illuminate\Support\Facades\Log;
/** /**
* Class UpdatePiggybank * Class UpdatePiggybank
@@ -53,18 +54,18 @@ class UpdatePiggybank implements ActionInterface
{ {
$actionValue = $this->action->getValue($journal); $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. // refresh the transaction type.
/** @var User $user */ /** @var User $user */
$user = User::find($journal['user_id']); $user = User::find($journal['user_id']);
/** @var TransactionJournal $journalObj */ /** @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) { 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) 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]))); event(new RuleActionFailedOnArray($this->action, $journal, trans('rules.cannot_find_piggy', ['name' => $actionValue])));
@@ -72,20 +73,19 @@ class UpdatePiggybank implements ActionInterface
return false; return false;
} }
app('log')->debug(sprintf('Found piggy bank #%d ("%s")', $piggyBank->id, $piggyBank->name)); Log::debug(sprintf('Found piggy bank #%d ("%s")', $piggyBank->id, $piggyBank->name));
/** @var Transaction $source */
$source = $journalObj->transactions()->where('amount', '<', 0)->first();
/** @var Transaction $destination */ /** @var Transaction $destination */
$destination = $journalObj->transactions()->where('amount', '>', 0)->first(); $destination = $journalObj->transactions()->where('amount', '>', 0)->first();
if ($source->account_id === $piggyBank->account_id) { $accounts = $this->getAccounts($journalObj);
app('log')->debug('Piggy bank account is linked to source, so remove amount from piggy bank.'); 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));
throw new FireflyException('Reference the correct account here.');
$this->removeAmount($piggyBank, $journal, $journalObj, $destination->amount);
// 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( event(
new TriggeredAuditLog( new TriggeredAuditLog(
$this->action->rule, $this->action->rule,
@@ -103,9 +103,11 @@ class UpdatePiggybank implements ActionInterface
return true; 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.'); // if connected to destination but not to source, needs to be removed from source account connected to piggy bank.
$this->addAmount($piggyBank, $journal, $journalObj, $destination->amount); 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( event(
new TriggeredAuditLog( new TriggeredAuditLog(
@@ -125,13 +127,12 @@ class UpdatePiggybank implements ActionInterface
return true; return true;
} }
app('log')->info( if ($this->isConnected($piggyBank, $accounts['source']) && $this->isConnected($piggyBank, $accounts['destination'])) {
sprintf( 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));
'Piggy bank is not linked to source ("#%d") or destination ("#%d"), so no action will be taken.', event(new RuleActionFailedOnArray($this->action, $journal, trans('rules.no_link_piggy', ['name' => $actionValue])));
$source->account_id, return false;
$destination->account_id }
) 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]))); event(new RuleActionFailedOnArray($this->action, $journal, trans('rules.no_link_piggy', ['name' => $actionValue])));
return false; return false;
@@ -139,80 +140,101 @@ class UpdatePiggybank implements ActionInterface
private function findPiggyBank(User $user, string $name): ?PiggyBank 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 = app(PiggyBankRepositoryInterface::class);
$repository->setUser($journal->user); $repository->setUser($journal->user);
// how much can we remove from this piggy bank? // how much can we remove from this piggy bank?
$toRemove = $repository->getCurrentAmount($piggyBank); $toRemove = $repository->getCurrentAmount($piggyBank, $account);
app('log')->debug(sprintf('Amount is %s, max to remove is %s', $amount, $toRemove)); Log::debug(sprintf('Amount is %s, max to remove is %s', $amount, $toRemove));
// if $amount is bigger than $toRemove, shrink it. // if $amount is bigger than $toRemove, shrink it.
$amount = -1 === bccomp($amount, $toRemove) ? $amount : $toRemove; $amount = -1 === bccomp($amount, $toRemove) ? $amount : $toRemove;
app('log')->debug(sprintf('Amount is now %s', $amount)); Log::debug(sprintf('Amount is now %s', $amount));
// if amount is zero, stop. // if amount is zero, stop.
if (0 === bccomp('0', $amount)) { 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]))); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_remove_zero_piggy', ['name' => $piggyBank->name])));
return; return;
} }
// make sure we can remove amount: if (false === $repository->canRemoveAmount($piggyBank, $account, $amount)) {
throw new FireflyException('Reference the correct account here.'); Log::warning(sprintf('Cannot remove %s from piggy bank.', $amount));
if (false === $repository->canRemoveAmount($piggyBank, $amount)) {
app('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]))); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_remove_from_piggy', ['amount' => $amount, 'name' => $piggyBank->name])));
return; 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, $account, $amount, $journal);
$repository->removeAmount($piggyBank, $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 = app(PiggyBankRepositoryInterface::class);
$repository->setUser($journal->user); $repository->setUser($journal->user);
// how much can we add to the piggy bank? // how much can we add to the piggy bank?
if (0 !== bccomp($piggyBank->target_amount, '0')) { if (0 !== bccomp($piggyBank->target_amount, '0')) {
$toAdd = bcsub($piggyBank->target_amount, $repository->getCurrentAmount($piggyBank)); $toAdd = bcsub($piggyBank->target_amount, $repository->getCurrentAmount($piggyBank, $account));
app('log')->debug(sprintf('Max amount to add to piggy bank is %s, amount is %s', $toAdd, $amount)); Log::debug(sprintf('Max amount to add to piggy bank is %s, amount is %s', $toAdd, $amount));
// update amount to fit: // update amount to fit:
$amount = -1 === bccomp($amount, $toAdd) ? $amount : $toAdd; $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')) { 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 amount is zero, stop.
if (0 === bccomp('0', $amount)) { 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]))); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_add_zero_piggy', ['name' => $piggyBank->name])));
return; return;
} }
// make sure we can add amount: if (false === $repository->canAddAmount($piggyBank, $account, $amount)) {
throw new FireflyException('Reference the correct account here.'); Log::warning(sprintf('Cannot add %s to piggy bank.', $amount));
if (false === $repository->canAddAmount($piggyBank, $amount)) {
app('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]))); event(new RuleActionFailedOnArray($this->action, $array, trans('rules.cannot_add_to_piggy', ['amount' => $amount, 'name' => $piggyBank->name])));
return; 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;
}
} }

View File

@@ -217,9 +217,9 @@ return [
'button_register' => 'Register', 'button_register' => 'Register',
'authorization' => 'Authorization', 'authorization' => 'Authorization',
'active_bills_only' => 'active subscription only', 'active_bills_only' => 'active subscription only',
'active_bills_only_total' => 'all active subscription', 'active_bills_only_total' => 'all active subscriptions',
'active_exp_bills_only' => 'active and expected subscription only', 'active_exp_bills_only' => 'active and expected subscriptions only',
'active_exp_bills_only_total' => 'all active expected subscription only', 'active_exp_bills_only_total' => 'all active expected subscriptions only',
'per_period_sum_1D' => 'Expected daily costs', 'per_period_sum_1D' => 'Expected daily costs',
'per_period_sum_1W' => 'Expected weekly costs', 'per_period_sum_1W' => 'Expected weekly costs',
'per_period_sum_1M' => 'Expected monthly 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_title' => 'Auto-generated rule for subscription ":name"',
'rule_for_bill_description' => 'This rule is auto-generated to try to match 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' => '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_title' => 'Rule for subscription ":name"',
'new_rule_for_bill_description' => 'This rule marks transactions for subscription ":name".', 'new_rule_for_bill_description' => 'This rule marks transactions for subscription ":name".',

View File

@@ -66,6 +66,7 @@ return [
'cannot_find_destination_transaction_account' => 'Firefly III can\'t find the destination transaction account', '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"', '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', '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_unlink_tag' => 'Tag ":tag" isn\'t linked to this transaction',
'cannot_find_budget' => 'Firefly III can\'t find budget ":name"', 'cannot_find_budget' => 'Firefly III can\'t find budget ":name"',
'cannot_find_category' => 'Firefly III can\'t find category ":name"', 'cannot_find_category' => 'Firefly III can\'t find category ":name"',