diff --git a/app/Services/Internal/Support/CreditRecalculateService.php b/app/Services/Internal/Support/CreditRecalculateService.php index c37dbf181d..365209035d 100644 --- a/app/Services/Internal/Support/CreditRecalculateService.php +++ b/app/Services/Internal/Support/CreditRecalculateService.php @@ -202,6 +202,8 @@ class CreditRecalculateService /** @var AccountMetaFactory $factory */ $factory = app(AccountMetaFactory::class); + + // amount is positive or negative, doesn't matter. $factory->crud($account, 'start_of_debt', $startOfDebt); // get direction of liability: @@ -209,6 +211,8 @@ class CreditRecalculateService // now loop all transactions (except opening balance and credit thing) $transactions = $account->transactions()->get(); + Log::debug(sprintf('Going to process %d transaction(s)', $transactions->count())); + Log::debug(sprintf('Account currency is #%d (%s)', $account->id, $this->repository->getAccountCurrency($account)?->code)); /** @var Transaction $transaction */ foreach ($transactions as $transaction) { $leftOfDebt = $this->processTransaction($account, $direction, $transaction, $leftOfDebt); @@ -226,32 +230,39 @@ class CreditRecalculateService * * @return string */ - private function processTransaction(Account $account, string $direction, Transaction $transaction, string $amount): string + private function processTransaction(Account $account, string $direction, Transaction $transaction, string $leftOfDebt): string { - Log::debug(sprintf('Now in %s(#%d, %s)', __METHOD__, $transaction->id, $amount)); + Log::debug(sprintf('Now in processTransaction(#%d, %s)', $transaction->id, $leftOfDebt)); $journal = $transaction->transactionJournal; $foreignCurrency = $transaction->foreignCurrency; $accountCurrency = $this->repository->getAccountCurrency($account); $groupId = $journal->transaction_group_id; $type = $journal->transactionType->type; - - Log::debug(sprintf('Account currency is #%d (%s)', $accountCurrency->id, $accountCurrency->code)); + /** @var Transaction $destTransaction */ + $destTransaction = $journal->transactions()->where('amount', '>', '0')->first(); + /** @var Transaction $sourceTransaction */ + $sourceTransaction = $journal->transactions()->where('amount', '<', '0')->first(); if ('' === $direction) { Log::debug('Since direction is "", do nothing.'); - return $amount; + return $leftOfDebt; } + if (TransactionType::LIABILITY_CREDIT === $type || TransactionType::OPENING_BALANCE === $type) { + Log::debug(sprintf('Skip group #%d, journal #%d of type "%s"', $journal->id, $groupId, $type)); + return $leftOfDebt; + } + // amount to use depends on the currency: $usedAmount = $transaction->amount; if (null !== $foreignCurrency && $foreignCurrency->id === $accountCurrency->id) { $usedAmount = $transaction->foreign_amount; - Log::debug('Will now use foreign amount!'); + Log::debug('Will use foreign amount to match account currency.'); } - Log::debug(sprintf('Processing group #%d, journal #%d of type "%s"', $journal->id, $groupId, $type)); + // Case 1 // it's a withdrawal into this liability (from asset). // if it's a credit ("I am owed"), this increases the amount due, // because we're lending person X more money @@ -261,13 +272,33 @@ class CreditRecalculateService && 1 === bccomp($usedAmount, '0') && 'credit' === $direction ) { - $amount = bcadd($amount, app('steam')->positive($usedAmount)); - Log::debug( - sprintf('Is withdrawal (%s) into credit liability #%d, will increase amount due to %s.', $transaction->account_id, $usedAmount, $amount) - ); - return $amount; + $newLeftOfDebt = bcadd($leftOfDebt, app('steam')->positive($usedAmount)); + Log::debug(sprintf('[1] Is withdrawal (%s) into liability, left of debt = %s.', $usedAmount, $newLeftOfDebt)); + return $newLeftOfDebt; } + // Case 2 + // it's a withdrawal away from this liability (into expense account). + // if it's a credit ("I am owed"), this decreases the amount due, + // because we're sending money away from the loan (like loan forgiveness) + if ( + $type === TransactionType::WITHDRAWAL + && (int)$account->id === (int)$sourceTransaction->account_id + && -1 === bccomp($usedAmount, '0') + && 'credit' === $direction + ) { + $newLeftOfDebt = bcsub($leftOfDebt, app('steam')->positive($usedAmount)); + Log::debug( + sprintf( + '[2] Is withdrawal (%s) away from liability, left of debt = %s.', + $usedAmount, + $newLeftOfDebt + ) + ); + return $newLeftOfDebt; + } + + // case 3 // it's a deposit out of this liability (to asset). // if it's a credit ("I am owed") this decreases the amount due. // because the person is paying us back. @@ -277,17 +308,21 @@ class CreditRecalculateService && -1 === bccomp($usedAmount, '0') && 'credit' === $direction ) { - $amount = bcsub($amount, app('steam')->positive($usedAmount)); - Log::debug(sprintf('Is deposit (%s) from credit liability #%d, will decrease amount due to %s.', $transaction->account_id, $usedAmount, $amount)); - return $amount; + $newLeftOfDebt = bcsub($leftOfDebt, app('steam')->positive($usedAmount)); + Log::debug(sprintf('[3] Is deposit (%s) away from liability, left of debt = %s.', $usedAmount, $newLeftOfDebt)); + return $newLeftOfDebt; } + // in any other case, remove amount from left of debt. if (in_array($type, [TransactionType::WITHDRAWAL, TransactionType::DEPOSIT, TransactionType::TRANSFER], true)) { - $amount = bcadd($amount, bcmul($usedAmount, '-1')); + $newLeftOfDebt = bcadd($leftOfDebt, bcmul($usedAmount, '-1')); + Log::debug(sprintf('[4] transaction is withdrawal/deposit/transfer, remove amount %s from left of debt, = %s.', $usedAmount, $newLeftOfDebt)); + return $newLeftOfDebt; } - Log::debug(sprintf('Amount is now %s', $amount)); - return $amount; + Log::warning(sprintf('[5] Catch-all, should not happen. Left of debt = %s', $leftOfDebt)); + + return $leftOfDebt; } /** diff --git a/app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php b/app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php index 85c823ea1e..f73add54e0 100644 --- a/app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php +++ b/app/Validation/Api/Data/Bulk/ValidatesBulkTransactionQuery.php @@ -71,7 +71,7 @@ trait ValidatesBulkTransactionQuery // must have same currency: // some account types (like expenses) do not have currency, so they have to be omitted $sourceCurrency = $repository->getAccountCurrency($source); - $destCurrency = $repository->getAccountCurrency($dest); + $destCurrency = $repository->getAccountCurrency($dest); if ( $sourceCurrency !== null && $destCurrency !== null diff --git a/changelog.md b/changelog.md index 580409fe2f..ad9e2b2a64 100644 --- a/changelog.md +++ b/changelog.md @@ -6,7 +6,7 @@ Alpha 2 - Data import: when you submit a transaction but give the ID of an account of the wrong type, Firefly III will try to create an account of the right type. For example: you submit a deposit but the source account is an expense account: Firefly III will try to create a revenue account instead. -- Security: blocked users can access API, users can unblock themselves using the API. +- Security: blocked users can access API, users can unblock themselves using the API. Recurrent Nymph CVE-2023-0298 - New language: catalan ## 5.8.0-alpha.1 - 2023-01-08