From 0545826d57e8379c3ee4da4a6791b06d71ef0c17 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 07:09:48 +0200 Subject: [PATCH 1/8] Do not try to correct transactions between asset / liability with foreign amount info. --- .../Correction/CorrectsUnevenAmount.php | 92 +++++++++++++------ 1 file changed, 66 insertions(+), 26 deletions(-) diff --git a/app/Console/Commands/Correction/CorrectsUnevenAmount.php b/app/Console/Commands/Correction/CorrectsUnevenAmount.php index 895eb45b4d..6739139ce8 100644 --- a/app/Console/Commands/Correction/CorrectsUnevenAmount.php +++ b/app/Console/Commands/Correction/CorrectsUnevenAmount.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace FireflyIII\Console\Commands\Correction; use FireflyIII\Console\Commands\ShowsFriendlyMessages; +use FireflyIII\Enums\AccountTypeEnum; use FireflyIII\Enums\TransactionTypeEnum; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; @@ -37,8 +38,8 @@ class CorrectsUnevenAmount extends Command { use ShowsFriendlyMessages; - protected $description = 'Fix journals with uneven amounts.'; - protected $signature = 'correction:uneven-amounts'; + protected $description = 'Fix journals with uneven amounts.'; + protected $signature = 'correction:uneven-amounts'; private int $count; /** @@ -47,7 +48,10 @@ class CorrectsUnevenAmount extends Command public function handle(): int { $this->count = 0; + // convert transfers with foreign currency info where the amount is NOT uneven (it should be) $this->convertOldStyleTransfers(); + + $this->fixUnevenAmounts(); $this->matchCurrencies(); if (true === config('firefly.feature_flags.running_balance_column')) { @@ -64,12 +68,11 @@ class CorrectsUnevenAmount extends Command Log::debug('convertOldStyleTransfers()'); // select transactions with a foreign amount and a foreign currency. and it's a transfer. and they are different. $transactions = Transaction::distinct() - ->leftJoin('transaction_journals', 'transaction_journals.id', 'transactions.transaction_journal_id') - ->leftJoin('transaction_types', 'transaction_types.id', 'transaction_journals.transaction_type_id') - ->where('transaction_types.type', TransactionTypeEnum::TRANSFER->value) - ->whereNotNull('foreign_currency_id') - ->whereNotNull('foreign_amount')->get(['transactions.transaction_journal_id']) - ; + ->leftJoin('transaction_journals', 'transaction_journals.id', 'transactions.transaction_journal_id') + ->leftJoin('transaction_types', 'transaction_types.id', 'transaction_journals.transaction_type_id') + ->where('transaction_types.type', TransactionTypeEnum::TRANSFER->value) + ->whereNotNull('foreign_currency_id') + ->whereNotNull('foreign_amount')->get(['transactions.transaction_journal_id']); $count = 0; Log::debug(sprintf('Found %d potential journal(s)', $transactions->count())); @@ -77,7 +80,7 @@ class CorrectsUnevenAmount extends Command /** @var Transaction $transaction */ foreach ($transactions as $transaction) { /** @var null|TransactionJournal $journal */ - $journal = TransactionJournal::find($transaction->transaction_journal_id); + $journal = TransactionJournal::find($transaction->transaction_journal_id); if (null === $journal) { Log::debug('Found no journal, continue.'); @@ -94,7 +97,7 @@ class CorrectsUnevenAmount extends Command $destination = $journal->transactions()->where('amount', '>', 0)->first(); /** @var null|Transaction $source */ - $source = $journal->transactions()->where('amount', '<', 0)->first(); + $source = $journal->transactions()->where('amount', '<', 0)->first(); if (null === $destination || null === $source) { Log::debug('Source or destination transaction is NULL, continue.'); @@ -119,11 +122,11 @@ class CorrectsUnevenAmount extends Command private function fixUnevenAmounts(): void { + Log::debug('fixUnevenAmounts()'); $journals = DB::table('transactions') - ->groupBy('transaction_journal_id') - ->whereNull('deleted_at') - ->get(['transaction_journal_id', DB::raw('SUM(amount) AS the_sum')]) - ; + ->groupBy('transaction_journal_id') + ->whereNull('deleted_at') + ->get(['transaction_journal_id', DB::raw('SUM(amount) AS the_sum')]); /** @var \stdClass $entry */ foreach ($journals as $entry) { @@ -161,13 +164,13 @@ class CorrectsUnevenAmount extends Command private function fixJournal(int $param): void { // one of the transactions is bad. - $journal = TransactionJournal::find($param); + $journal = TransactionJournal::find($param); if (null === $journal) { return; } /** @var null|Transaction $source */ - $source = $journal->transactions()->where('amount', '<', 0)->first(); + $source = $journal->transactions()->where('amount', '<', 0)->first(); if (null === $source) { $this->friendlyError( @@ -184,11 +187,11 @@ class CorrectsUnevenAmount extends Command return; } - $amount = bcmul('-1', (string) $source->amount); + $amount = bcmul('-1', (string) $source->amount); // fix amount of destination: /** @var null|Transaction $destination */ - $destination = $journal->transactions()->where('amount', '>', 0)->first(); + $destination = $journal->transactions()->where('amount', '>', 0)->first(); if (null === $destination) { $this->friendlyError( @@ -207,13 +210,13 @@ class CorrectsUnevenAmount extends Command } // may still be able to salvage this journal if it is a transfer with foreign currency info - if ($this->isForeignCurrencyTransfer($journal)) { + if ($this->isForeignCurrencyTransfer($journal) || $this->isBetweenAssetAndLiability($journal)) { Log::debug(sprintf('Can skip foreign currency transfer #%d.', $journal->id)); return; } - $message = sprintf('Sum of journal #%d is not zero, journal is broken and now fixed.', $journal->id); + $message = sprintf('Sum of journal #%d is not zero, journal is broken and now fixed.', $journal->id); $this->friendlyWarning($message); app('log')->warning($message); @@ -221,7 +224,7 @@ class CorrectsUnevenAmount extends Command $destination->amount = $amount; $destination->save(); - $message = sprintf('Corrected amount in transaction journal #%d', $param); + $message = sprintf('Corrected amount in transaction journal #%d', $param); $this->friendlyInfo($message); ++$this->count; } @@ -236,7 +239,7 @@ class CorrectsUnevenAmount extends Command $destination = $journal->transactions()->where('amount', '>', 0)->first(); /** @var Transaction $source */ - $source = $journal->transactions()->where('amount', '<', 0)->first(); + $source = $journal->transactions()->where('amount', '<', 0)->first(); // safety catch on NULL should not be necessary, we just had that catch. // source amount = dest foreign amount @@ -263,11 +266,10 @@ class CorrectsUnevenAmount extends Command private function matchCurrencies(): void { $journals = TransactionJournal::leftJoin('transactions', 'transaction_journals.id', 'transactions.transaction_journal_id') - ->where('transactions.transaction_currency_id', '!=', DB::raw('transaction_journals.transaction_currency_id')) - ->get(['transaction_journals.*']) - ; + ->where('transactions.transaction_currency_id', '!=', DB::raw('transaction_journals.transaction_currency_id')) + ->get(['transaction_journals.*']); - $count = 0; + $count = 0; /** @var TransactionJournal $journal */ foreach ($journals as $journal) { @@ -285,4 +287,42 @@ class CorrectsUnevenAmount extends Command $this->friendlyPositive(sprintf('Fixed %d journal(s) with mismatched currencies.', $journals->count())); } + + private function isBetweenAssetAndLiability(TransactionJournal $journal): bool + { + /** @var Transaction $sourceTransaction */ + $sourceTransaction = $journal->transactions()->where('amount', '<', 0)->first(); + /** @var Transaction $destinationTransaction */ + $destinationTransaction = $journal->transactions()->where('amount', '>', 0)->first(); + if (null === $sourceTransaction || null === $destinationTransaction) { + Log::warning('Either transaction is false, stop.'); + return false; + } + if (null === $sourceTransaction->foreign_amount || null === $destinationTransaction->foreign_amount) { + Log::warning('Either foreign amount is false, stop.'); + return false; + } + + $source = $sourceTransaction->account; + $destination = $destinationTransaction->account; + + if (null === $source || null === $destination) { + Log::warning('Either is false, stop.'); + return false; + } + $sourceTypes = [AccountTypeEnum::LOAN->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::MORTGAGE->value]; + + // source is liability, destination is asset + if (in_array($source->accountType->type, $sourceTypes, true) && AccountTypeEnum::ASSET->value === $destination->accountType->type) { + Log::debug('Source is a liability account, destination is an asset account, return TRUE.'); + return true; + } + // source is asset, destination is liability + if (in_array($destination->accountType->type, $sourceTypes, true) && AccountTypeEnum::ASSET->value === $source->accountType->type) { + Log::debug('Destination is a liability account, source is an asset account, return TRUE.'); + return true; + } + Log::debug('Not between asset and liability, return FALSE'); + return false; + } } From f7baf5b2fd7cfc69b808a1ee1b929a23462b6bdc Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 07:11:00 +0200 Subject: [PATCH 2/8] Better debug info --- app/Console/Commands/Correction/CorrectsUnevenAmount.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Console/Commands/Correction/CorrectsUnevenAmount.php b/app/Console/Commands/Correction/CorrectsUnevenAmount.php index 6739139ce8..1fffee3fe3 100644 --- a/app/Console/Commands/Correction/CorrectsUnevenAmount.php +++ b/app/Console/Commands/Correction/CorrectsUnevenAmount.php @@ -211,7 +211,7 @@ class CorrectsUnevenAmount extends Command // may still be able to salvage this journal if it is a transfer with foreign currency info if ($this->isForeignCurrencyTransfer($journal) || $this->isBetweenAssetAndLiability($journal)) { - Log::debug(sprintf('Can skip foreign currency transfer #%d.', $journal->id)); + Log::debug(sprintf('Can skip foreign currency transfer / asset+liability transaction #%d.', $journal->id)); return; } From e61dadcbc61f85b2ab95d6db52f23293ecda578f Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 07:20:17 +0200 Subject: [PATCH 3/8] Implement correction method. --- .../Correction/CorrectsUnevenAmount.php | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/app/Console/Commands/Correction/CorrectsUnevenAmount.php b/app/Console/Commands/Correction/CorrectsUnevenAmount.php index 1fffee3fe3..981c8f6596 100644 --- a/app/Console/Commands/Correction/CorrectsUnevenAmount.php +++ b/app/Console/Commands/Correction/CorrectsUnevenAmount.php @@ -51,6 +51,8 @@ class CorrectsUnevenAmount extends Command // convert transfers with foreign currency info where the amount is NOT uneven (it should be) $this->convertOldStyleTransfers(); + // convert old-style transactions between assets and liabilities. + $this->convertOldStyleTransactions(); $this->fixUnevenAmounts(); $this->matchCurrencies(); @@ -325,4 +327,67 @@ class CorrectsUnevenAmount extends Command Log::debug('Not between asset and liability, return FALSE'); return false; } + + private function convertOldStyleTransactions(): void + { + Log::debug('convertOldStyleTransactions()'); + $count = 0; + $transactions = Transaction::distinct() + ->leftJoin('transaction_journals', 'transaction_journals.id', 'transactions.transaction_journal_id') + ->leftJoin('transaction_types', 'transaction_types.id', 'transaction_journals.transaction_type_id') + ->leftJoin('accounts', 'accounts.id', 'transactions.account_id') + ->leftJoin('account_types', 'account_types.id', 'accounts.account_type_id') + ->whereNot('transaction_types.type', TransactionTypeEnum::TRANSFER->value) + ->whereNotNull('foreign_currency_id') + ->whereNotNull('foreign_amount') + ->whereIn('account_types.type', [AccountTypeEnum::ASSET->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::MORTGAGE->value, AccountTypeEnum::LOAN->value]) + ->get(['transactions.transaction_journal_id']); + + Log::debug(sprintf('Found %d potential journal(s)', $transactions->count())); + + /** @var Transaction $transaction */ + foreach ($transactions as $transaction) { + /** @var null|TransactionJournal $journal */ + $journal = TransactionJournal::find($transaction->transaction_journal_id); + if (null === $journal) { + Log::debug('Found no journal, continue.'); + + continue; + } + if (!$this->isBetweenAssetAndLiability($journal)) { + Log::debug('Not between asset and liability, continue.'); + + continue; + } + Log::debug(sprintf('Potential fix for #%d', $journal->id)); + $source = $journal->transactions()->where('amount', '<', 0)->first(); + $destination = $journal->transactions()->where('amount', '>', 0)->first(); + if (null === $source || null === $destination) { + Log::debug('Either transaction is NULL, continue.'); + + continue; + } + if (0 === bccomp($source->amount, $source->foreign_amount) && 0 === bccomp($source->foreign_amount, $source->amount)) { + Log::debug('Already fixed, continue.'); + + continue; + } + Log::debug('Ready to swap data between transactions.'); + $destination->foreign_currency_id = $source->transaction_currency_id; + $destination->foreign_amount = app('steam')->positive($source->amount); + $destination->transaction_currency_id = $source->foreign_currency_id; + $destination->amount = app('steam')->positive($source->foreign_amount); + $destination->balance_dirty = true; + $source->balance_dirty = true; + $destination->save(); + $source->save(); + $this->friendlyWarning(sprintf('Corrected foreign amounts of transaction #%d.', $journal->id)); + ++$count; + } + if (0 === $count) { + return; + } + + $this->friendlyPositive(sprintf('Fixed %d journal(s) with unbalanced amounts.', $count)); + } } From 9b6314066bec8295c2a24c361972da4bdc14f65d Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 07:23:35 +0200 Subject: [PATCH 4/8] Clean up logs. --- app/Console/Commands/Correction/CorrectsUnevenAmount.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/Console/Commands/Correction/CorrectsUnevenAmount.php b/app/Console/Commands/Correction/CorrectsUnevenAmount.php index 981c8f6596..91e8d18d37 100644 --- a/app/Console/Commands/Correction/CorrectsUnevenAmount.php +++ b/app/Console/Commands/Correction/CorrectsUnevenAmount.php @@ -77,8 +77,6 @@ class CorrectsUnevenAmount extends Command ->whereNotNull('foreign_amount')->get(['transactions.transaction_journal_id']); $count = 0; - Log::debug(sprintf('Found %d potential journal(s)', $transactions->count())); - /** @var Transaction $transaction */ foreach ($transactions as $transaction) { /** @var null|TransactionJournal $journal */ @@ -120,6 +118,10 @@ class CorrectsUnevenAmount extends Command ++$count; } } + if (0 === $count) { + return; + } + $this->friendlyPositive(sprintf('Fixed %d transfer(s) with unbalanced amounts.', $count)); } private function fixUnevenAmounts(): void @@ -343,8 +345,6 @@ class CorrectsUnevenAmount extends Command ->whereIn('account_types.type', [AccountTypeEnum::ASSET->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::MORTGAGE->value, AccountTypeEnum::LOAN->value]) ->get(['transactions.transaction_journal_id']); - Log::debug(sprintf('Found %d potential journal(s)', $transactions->count())); - /** @var Transaction $transaction */ foreach ($transactions as $transaction) { /** @var null|TransactionJournal $journal */ @@ -359,7 +359,6 @@ class CorrectsUnevenAmount extends Command continue; } - Log::debug(sprintf('Potential fix for #%d', $journal->id)); $source = $journal->transactions()->where('amount', '<', 0)->first(); $destination = $journal->transactions()->where('amount', '>', 0)->first(); if (null === $source || null === $destination) { From 01502b456e20b4b9d4b35b27a92fbbf8b11cd44b Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 07:29:18 +0200 Subject: [PATCH 5/8] Disable fix for the moment. --- .../Correction/CorrectsUnevenAmount.php | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/app/Console/Commands/Correction/CorrectsUnevenAmount.php b/app/Console/Commands/Correction/CorrectsUnevenAmount.php index 91e8d18d37..0b58dbedb2 100644 --- a/app/Console/Commands/Correction/CorrectsUnevenAmount.php +++ b/app/Console/Commands/Correction/CorrectsUnevenAmount.php @@ -326,7 +326,6 @@ class CorrectsUnevenAmount extends Command Log::debug('Destination is a liability account, source is an asset account, return TRUE.'); return true; } - Log::debug('Not between asset and liability, return FALSE'); return false; } @@ -361,6 +360,8 @@ class CorrectsUnevenAmount extends Command } $source = $journal->transactions()->where('amount', '<', 0)->first(); $destination = $journal->transactions()->where('amount', '>', 0)->first(); + $sourceAccount = $source->account; + $destAccount = $destination->account; if (null === $source || null === $destination) { Log::debug('Either transaction is NULL, continue.'); @@ -372,15 +373,16 @@ class CorrectsUnevenAmount extends Command continue; } Log::debug('Ready to swap data between transactions.'); - $destination->foreign_currency_id = $source->transaction_currency_id; - $destination->foreign_amount = app('steam')->positive($source->amount); - $destination->transaction_currency_id = $source->foreign_currency_id; - $destination->amount = app('steam')->positive($source->foreign_amount); - $destination->balance_dirty = true; - $source->balance_dirty = true; - $destination->save(); - $source->save(); - $this->friendlyWarning(sprintf('Corrected foreign amounts of transaction #%d.', $journal->id)); +// // only fix the destination transaction +// $destination->foreign_currency_id = $source->transaction_currency_id; +// $destination->foreign_amount = app('steam')->positive($source->amount); +// $destination->transaction_currency_id = $source->foreign_currency_id; +// $destination->amount = app('steam')->positive($source->foreign_amount); +// $destination->balance_dirty = true; +// $source->balance_dirty = true; +// $destination->save(); +// $source->save(); +// $this->friendlyWarning(sprintf('Corrected foreign amounts of transaction #%d.', $journal->id)); ++$count; } if (0 === $count) { From 8cad430816fdbf936583945e1868ae8d8d2b2c20 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 07:35:20 +0200 Subject: [PATCH 6/8] Fix validation error in transaction processing. --- app/Validation/TransactionValidation.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php index 5b5a01a525..c6e4d09496 100644 --- a/app/Validation/TransactionValidation.php +++ b/app/Validation/TransactionValidation.php @@ -233,16 +233,16 @@ trait TransactionValidation return; } - Log::debug(sprintf('Source account expects %s', $sourceCurrency->code)); - Log::debug(sprintf('Destination account expects %s', $destinationCurrency->code)); + Log::debug(sprintf('Source account expects #%d: %s', $sourceCurrency->id, $sourceCurrency->code)); + Log::debug(sprintf('Destination account expects #%d: %s', $destinationCurrency->id, $destinationCurrency->code)); Log::debug(sprintf('Amount is %s', $transaction['amount'])); if (TransactionTypeEnum::DEPOSIT->value === ucfirst($transactionType)) { Log::debug(sprintf('Processing as a "%s"', $transactionType)); // use case: deposit from liability account to an asset account - // the foreign amount must be in the currency of the source - // the amount must be in the currency of the destination + // the amount must be in the currency of the SOURCE + // the foreign amount must be in the currency of the DESTINATION // no foreign currency information is present: if (!$this->hasForeignCurrencyInfo($transaction)) { @@ -255,7 +255,7 @@ trait TransactionValidation $foreignCurrencyCode = $transaction['foreign_currency_code'] ?? false; $foreignCurrencyId = (int) ($transaction['foreign_currency_id'] ?? 0); Log::debug(sprintf('Foreign currency code seems to be #%d "%s"', $foreignCurrencyId, $foreignCurrencyCode), $transaction); - if ($foreignCurrencyCode !== $sourceCurrency->code && $foreignCurrencyId !== $sourceCurrency->id) { + if ($foreignCurrencyCode !== $destinationCurrency->code && $foreignCurrencyId !== $destinationCurrency->id) { $validator->errors()->add(sprintf('transactions.%d.foreign_currency_code', $index), (string) trans('validation.require_foreign_src')); return; From fb3402acd4f61efdc468a17056552ed8cac0e3a1 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 08:00:38 +0200 Subject: [PATCH 7/8] Fix storage issue with transactions. --- app/Factory/TransactionJournalFactory.php | 200 +++++++++++------- app/Repositories/Bill/BillRepository.php | 2 +- .../Currency/CurrencyRepository.php | 54 ++--- .../PiggyBank/PiggyBankRepository.php | 2 +- .../Currency/CurrencyRepository.php | 46 ++-- app/Validation/AccountValidator.php | 2 +- app/Validation/TransactionValidation.php | 2 +- 7 files changed, 175 insertions(+), 133 deletions(-) diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index fe17fa054d..ec74a52d0c 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/app/Factory/TransactionJournalFactory.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace FireflyIII\Factory; use Carbon\Carbon; +use FireflyIII\Enums\AccountTypeEnum; use FireflyIII\Enums\TransactionTypeEnum; use FireflyIII\Exceptions\DuplicateTransactionException; use FireflyIII\Exceptions\FireflyException; @@ -101,15 +102,15 @@ class TransactionJournalFactory */ public function create(array $data): Collection { - app('log')->debug('Now in TransactionJournalFactory::create()'); + Log::debug('Now in TransactionJournalFactory::create()'); // convert to special object. - $dataObject = new NullArrayObject($data); + $dataObject = new NullArrayObject($data); - app('log')->debug('Start of TransactionJournalFactory::create()'); + Log::debug('Start of TransactionJournalFactory::create()'); $collection = new Collection(); $transactions = $dataObject['transactions'] ?? []; if (0 === count($transactions)) { - app('log')->error('There are no transactions in the array, the TransactionJournalFactory cannot continue.'); + Log::error('There are no transactions in the array, the TransactionJournalFactory cannot continue.'); return new Collection(); } @@ -117,26 +118,26 @@ class TransactionJournalFactory try { /** @var array $row */ foreach ($transactions as $index => $row) { - app('log')->debug(sprintf('Now creating journal %d/%d', $index + 1, count($transactions))); + Log::debug(sprintf('Now creating journal %d/%d', $index + 1, count($transactions))); $journal = $this->createJournal(new NullArrayObject($row)); if (null !== $journal) { $collection->push($journal); } if (null === $journal) { - app('log')->error('The createJournal() method returned NULL. This may indicate an error.'); + Log::error('The createJournal() method returned NULL. This may indicate an error.'); } } } catch (DuplicateTransactionException $e) { - app('log')->warning('TransactionJournalFactory::create() caught a duplicate journal in createJournal()'); - app('log')->error($e->getMessage()); - app('log')->error($e->getTraceAsString()); + Log::warning('TransactionJournalFactory::create() caught a duplicate journal in createJournal()'); + Log::error($e->getMessage()); + Log::error($e->getTraceAsString()); $this->forceDeleteOnError($collection); throw new DuplicateTransactionException($e->getMessage(), 0, $e); } catch (FireflyException $e) { - app('log')->warning('TransactionJournalFactory::create() caught an exception.'); - app('log')->error($e->getMessage()); - app('log')->error($e->getTraceAsString()); + Log::warning('TransactionJournalFactory::create() caught an exception.'); + Log::error($e->getMessage()); + Log::error($e->getTraceAsString()); $this->forceDeleteOnError($collection); throw new FireflyException($e->getMessage(), 0, $e); @@ -156,19 +157,23 @@ class TransactionJournalFactory */ private function createJournal(NullArrayObject $row): ?TransactionJournal { + Log::debug('Now in TransactionJournalFactory::createJournal()'); $row['import_hash_v2'] = $this->hashArray($row); $this->errorIfDuplicate($row['import_hash_v2']); // Some basic fields - $type = $this->typeRepository->findTransactionType(null, $row['type']); - $carbon = $row['date'] ?? today(config('app.timezone')); - $order = $row['order'] ?? 0; - $currency = $this->currencyRepository->findCurrency((int) $row['currency_id'], $row['currency_code']); - $foreignCurrency = $this->currencyRepository->findCurrencyNull($row['foreign_currency_id'], $row['foreign_currency_code']); - $bill = $this->billRepository->findBill((int) $row['bill_id'], $row['bill_name']); - $billId = TransactionTypeEnum::WITHDRAWAL->value === $type->type && null !== $bill ? $bill->id : null; - $description = (string) $row['description']; + $type = $this->typeRepository->findTransactionType(null, $row['type']); + $carbon = $row['date'] ?? today(config('app.timezone')); + $order = $row['order'] ?? 0; + + Log::debug('Find currency or return default.'); + $currency = $this->currencyRepository->findCurrency((int) $row['currency_id'], $row['currency_code']); + Log::debug('Find foreign currency or return NULL.'); + $foreignCurrency = $this->currencyRepository->findCurrencyNull($row['foreign_currency_id'], $row['foreign_currency_code']); + $bill = $this->billRepository->findBill((int) $row['bill_id'], $row['bill_name']); + $billId = TransactionTypeEnum::WITHDRAWAL->value === $type->type && null !== $bill ? $bill->id : null; + $description = (string) $row['description']; // Manipulate basic fields $carbon->setTimezone(config('app.timezone')); @@ -183,14 +188,14 @@ class TransactionJournalFactory // validate source and destination using a new Validator. $this->validateAccounts($row); } catch (FireflyException $e) { - app('log')->error('Could not validate source or destination.'); - app('log')->error($e->getMessage()); + Log::error('Could not validate source or destination.'); + Log::error($e->getMessage()); return null; } /** create or get source and destination accounts */ - $sourceInfo = [ + $sourceInfo = [ 'id' => $row['source_id'], 'name' => $row['source_name'], 'iban' => $row['source_iban'], @@ -199,7 +204,7 @@ class TransactionJournalFactory 'currency_id' => $currency->id, ]; - $destInfo = [ + $destInfo = [ 'id' => $row['destination_id'], 'name' => $row['destination_name'], 'iban' => $row['destination_iban'], @@ -207,26 +212,27 @@ class TransactionJournalFactory 'bic' => $row['destination_bic'], 'currency_id' => $currency->id, ]; - app('log')->debug('Source info:', $sourceInfo); - app('log')->debug('Destination info:', $destInfo); - $sourceAccount = $this->getAccount($type->type, 'source', $sourceInfo); - $destinationAccount = $this->getAccount($type->type, 'destination', $destInfo); - app('log')->debug('Done with getAccount(2x)'); + Log::debug('Source info:', $sourceInfo); + Log::debug('Destination info:', $destInfo); + $sourceAccount = $this->getAccount($type->type, 'source', $sourceInfo); + $destinationAccount = $this->getAccount($type->type, 'destination', $destInfo); + Log::debug('Done with getAccount(2x)'); // this is the moment for a reconciliation sanity check (again). if (TransactionTypeEnum::RECONCILIATION->value === $type->type) { [$sourceAccount, $destinationAccount] = $this->reconciliationSanityCheck($sourceAccount, $destinationAccount); } - $currency = $this->getCurrencyByAccount($type->type, $currency, $sourceAccount, $destinationAccount); - $foreignCurrency = $this->compareCurrencies($currency, $foreignCurrency); - $foreignCurrency = $this->getForeignByAccount($type->type, $foreignCurrency, $destinationAccount); - $description = $this->getDescription($description); + $currency = $this->getCurrencyByAccount($type->type, $currency, $sourceAccount, $destinationAccount); + $foreignCurrency = $this->compareCurrencies($currency, $foreignCurrency); + $foreignCurrency = $this->getForeignByAccount($type->type, $foreignCurrency, $destinationAccount); + $description = $this->getDescription($description); - app('log')->debug(sprintf('Date: %s (%s)', $carbon->toW3cString(), $carbon->getTimezone()->getName())); + Log::debug(sprintf('Currency is #%d "%s", foreign currency is #%d "%s"', $currency->id, $currency->code, $foreignCurrency?->id, $foreignCurrency)); + Log::debug(sprintf('Date: %s (%s)', $carbon->toW3cString(), $carbon->getTimezone()->getName())); /** Create a basic journal. */ - $journal = TransactionJournal::create( + $journal = TransactionJournal::create( [ 'user_id' => $this->user->id, 'user_group_id' => $this->userGroup->id, @@ -241,10 +247,10 @@ class TransactionJournalFactory 'completed' => 0, ] ); - app('log')->debug(sprintf('Created new journal #%d: "%s"', $journal->id, $journal->description)); + Log::debug(sprintf('Created new journal #%d: "%s"', $journal->id, $journal->description)); /** Create two transactions. */ - $transactionFactory = app(TransactionFactory::class); + $transactionFactory = app(TransactionFactory::class); $transactionFactory->setJournal($journal); $transactionFactory->setAccount($sourceAccount); $transactionFactory->setCurrency($currency); @@ -255,14 +261,14 @@ class TransactionJournalFactory try { $negative = $transactionFactory->createNegative((string) $row['amount'], (string) $row['foreign_amount']); } catch (FireflyException $e) { - app('log')->error(sprintf('Exception creating negative transaction: %s', $e->getMessage())); + Log::error(sprintf('Exception creating negative transaction: %s', $e->getMessage())); $this->forceDeleteOnError(new Collection([$journal])); throw new FireflyException($e->getMessage(), 0, $e); } /** @var TransactionFactory $transactionFactory */ - $transactionFactory = app(TransactionFactory::class); + $transactionFactory = app(TransactionFactory::class); $transactionFactory->setJournal($journal); $transactionFactory->setAccount($destinationAccount); $transactionFactory->setAccountInformation($destInfo); @@ -274,8 +280,8 @@ class TransactionJournalFactory // Firefly III will save the foreign currency information in such a way that both // asset accounts can look at the "amount" and "transaction_currency_id" column and // see the currency they expect to see. - $amount = (string) $row['amount']; - $foreignAmount = (string) $row['foreign_amount']; + $amount = (string) $row['amount']; + $foreignAmount = (string) $row['foreign_amount']; if (null !== $foreignCurrency && $foreignCurrency->id !== $currency->id && TransactionTypeEnum::TRANSFER->value === $type->type ) { @@ -289,13 +295,13 @@ class TransactionJournalFactory try { $transactionFactory->createPositive($amount, $foreignAmount); } catch (FireflyException $e) { - app('log')->error(sprintf('Exception creating positive transaction: %s', $e->getMessage())); + Log::error(sprintf('Exception creating positive transaction: %s', $e->getMessage())); $this->forceTrDelete($negative); $this->forceDeleteOnError(new Collection([$journal])); throw new FireflyException($e->getMessage(), 0, $e); } - $journal->completed = true; + $journal->completed = true; $journal->save(); $this->storeBudget($journal, $row); $this->storeCategory($journal, $row); @@ -317,11 +323,11 @@ class TransactionJournalFactory try { $json = \Safe\json_encode($dataRow, JSON_THROW_ON_ERROR); } catch (\JsonException $e) { - app('log')->error(sprintf('Could not encode dataRow: %s', $e->getMessage())); + Log::error(sprintf('Could not encode dataRow: %s', $e->getMessage())); $json = microtime(); } - $hash = hash('sha256', $json); - app('log')->debug(sprintf('The hash is: %s', $hash), $dataRow); + $hash = hash('sha256', $json); + Log::debug(sprintf('The hash is: %s', $hash), $dataRow); return $hash; } @@ -333,23 +339,22 @@ class TransactionJournalFactory */ private function errorIfDuplicate(string $hash): void { - app('log')->debug(sprintf('In errorIfDuplicate(%s)', $hash)); + Log::debug(sprintf('In errorIfDuplicate(%s)', $hash)); if (false === $this->errorOnHash) { return; } - app('log')->debug('Will verify duplicate!'); + Log::debug('Will verify duplicate!'); /** @var null|TransactionJournalMeta $result */ $result = TransactionJournalMeta::withTrashed() - ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'journal_meta.transaction_journal_id') - ->whereNotNull('transaction_journals.id') - ->where('transaction_journals.user_id', $this->user->id) - ->where('data', \Safe\json_encode($hash, JSON_THROW_ON_ERROR)) - ->with(['transactionJournal', 'transactionJournal.transactionGroup']) - ->first(['journal_meta.*']) - ; + ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'journal_meta.transaction_journal_id') + ->whereNotNull('transaction_journals.id') + ->where('transaction_journals.user_id', $this->user->id) + ->where('data', \Safe\json_encode($hash, JSON_THROW_ON_ERROR)) + ->with(['transactionJournal', 'transactionJournal.transactionGroup']) + ->first(['journal_meta.*']); if (null !== $result) { - app('log')->warning(sprintf('Found a duplicate in errorIfDuplicate because hash %s is not unique!', $hash)); + Log::warning(sprintf('Found a duplicate in errorIfDuplicate because hash %s is not unique!', $hash)); $journal = $result->transactionJournal()->withTrashed()->first(); $group = $journal?->transactionGroup()->withTrashed()->first(); $groupId = (int) $group?->id; @@ -363,28 +368,28 @@ class TransactionJournalFactory */ private function validateAccounts(NullArrayObject $data): void { - app('log')->debug(sprintf('Now in %s', __METHOD__)); - $transactionType = $data['type'] ?? 'invalid'; + Log::debug(sprintf('Now in %s', __METHOD__)); + $transactionType = $data['type'] ?? 'invalid'; $this->accountValidator->setUser($this->user); $this->accountValidator->setTransactionType($transactionType); // validate source account. - $array = [ + $array = [ 'id' => null !== $data['source_id'] ? (int) $data['source_id'] : null, 'name' => null !== $data['source_name'] ? (string) $data['source_name'] : null, 'iban' => null !== $data['source_iban'] ? (string) $data['source_iban'] : null, 'number' => null !== $data['source_number'] ? (string) $data['source_number'] : null, ]; - $validSource = $this->accountValidator->validateSource($array); + $validSource = $this->accountValidator->validateSource($array); // do something with result: if (false === $validSource) { throw new FireflyException(sprintf('Source: %s', $this->accountValidator->sourceError)); } - app('log')->debug('Source seems valid.'); + Log::debug('Source seems valid.'); // validate destination account - $array = [ + $array = [ 'id' => null !== $data['destination_id'] ? (int) $data['destination_id'] : null, 'name' => null !== $data['destination_name'] ? (string) $data['destination_name'] : null, 'iban' => null !== $data['destination_iban'] ? (string) $data['destination_iban'] : null, @@ -416,28 +421,28 @@ class TransactionJournalFactory private function reconciliationSanityCheck(?Account $sourceAccount, ?Account $destinationAccount): array { - app('log')->debug(sprintf('Now in %s', __METHOD__)); + Log::debug(sprintf('Now in %s', __METHOD__)); if (null !== $sourceAccount && null !== $destinationAccount) { - app('log')->debug('Both accounts exist, simply return them.'); + Log::debug('Both accounts exist, simply return them.'); return [$sourceAccount, $destinationAccount]; } if (null === $destinationAccount) { - app('log')->debug('Destination account is NULL, source account is not.'); + Log::debug('Destination account is NULL, source account is not.'); $account = $this->accountRepository->getReconciliation($sourceAccount); - app('log')->debug(sprintf('Will return account #%d ("%s") of type "%s"', $account->id, $account->name, $account->accountType->type)); + Log::debug(sprintf('Will return account #%d ("%s") of type "%s"', $account->id, $account->name, $account->accountType->type)); return [$sourceAccount, $account]; } if (null === $sourceAccount) { // @phpstan-ignore-line - app('log')->debug('Source account is NULL, destination account is not.'); + Log::debug('Source account is NULL, destination account is not.'); $account = $this->accountRepository->getReconciliation($destinationAccount); - app('log')->debug(sprintf('Will return account #%d ("%s") of type "%s"', $account->id, $account->name, $account->accountType->type)); + Log::debug(sprintf('Will return account #%d ("%s") of type "%s"', $account->id, $account->name, $account->accountType->type)); return [$account, $destinationAccount]; } - app('log')->debug('Unused fallback'); // @phpstan-ignore-line + Log::debug('Unused fallback'); // @phpstan-ignore-line return [$sourceAccount, $destinationAccount]; } @@ -447,7 +452,15 @@ class TransactionJournalFactory */ private function getCurrencyByAccount(string $type, ?TransactionCurrency $currency, Account $source, Account $destination): TransactionCurrency { - app('log')->debug('Now in getCurrencyByAccount()'); + Log::debug('Now in getCurrencyByAccount()'); + + /* + * Deze functie moet bij een transactie van liability naar asset wel degelijk de currency + * van de liability teruggeven en niet die van de destination. Fix voor #10265 + */ + if ($this->isBetweenAssetAndLiability($source, $destination) && TransactionTypeEnum::DEPOSIT->value === $type) { + return $this->getCurrency($currency, $source); + } return match ($type) { default => $this->getCurrency($currency, $source), @@ -460,7 +473,7 @@ class TransactionJournalFactory */ private function getCurrency(?TransactionCurrency $currency, Account $account): TransactionCurrency { - app('log')->debug('Now in getCurrency()'); + Log::debug(sprintf('Now in getCurrency(#%d, "%s")', $currency->id, $account->name)); /** @var null|TransactionCurrency $preference */ $preference = $this->accountRepository->getAccountCurrency($account); @@ -468,8 +481,8 @@ class TransactionJournalFactory // return user's default: return app('amount')->getNativeCurrencyByUserGroup($this->user->userGroup); } - $result = $preference ?? $currency; - app('log')->debug(sprintf('Currency is now #%d (%s) because of account #%d (%s)', $result->id, $result->code, $account->id, $account->name)); + $result = $preference ?? $currency; + Log::debug(sprintf('Currency is now #%d (%s) because of account #%d (%s)', $result->id, $result->code, $account->id, $account->name)); return $result; } @@ -479,6 +492,7 @@ class TransactionJournalFactory */ private function compareCurrencies(?TransactionCurrency $currency, ?TransactionCurrency $foreignCurrency): ?TransactionCurrency { + Log::debug(sprintf('Now in compareCurrencies("%s", "%s")', $currency?->code, $foreignCurrency?->code)); if (null === $currency) { return null; } @@ -494,6 +508,7 @@ class TransactionJournalFactory */ private function getForeignByAccount(string $type, ?TransactionCurrency $foreignCurrency, Account $destination): ?TransactionCurrency { + Log::debug(sprintf('Now in getForeignByAccount("%s", #%d, "%s")', $type, $foreignCurrency?->id, $destination->name)); if (TransactionTypeEnum::TRANSFER->value === $type) { return $this->getCurrency($foreignCurrency, $destination); } @@ -514,12 +529,12 @@ class TransactionJournalFactory */ private function forceDeleteOnError(Collection $collection): void { - app('log')->debug(sprintf('forceDeleteOnError on collection size %d item(s)', $collection->count())); + Log::debug(sprintf('forceDeleteOnError on collection size %d item(s)', $collection->count())); $service = app(JournalDestroyService::class); /** @var TransactionJournal $journal */ foreach ($collection as $journal) { - app('log')->debug(sprintf('forceDeleteOnError on journal #%d', $journal->id)); + Log::debug(sprintf('forceDeleteOnError on journal #%d', $journal->id)); $service->destroy($journal); } } @@ -534,17 +549,17 @@ class TransactionJournalFactory */ private function storePiggyEvent(TransactionJournal $journal, NullArrayObject $data): void { - app('log')->debug('Will now store piggy event.'); + Log::debug('Will now store piggy event.'); $piggyBank = $this->piggyRepository->findPiggyBank((int) $data['piggy_bank_id'], $data['piggy_bank_name']); if (null !== $piggyBank) { $this->piggyEventFactory->create($journal, $piggyBank); - app('log')->debug('Create piggy event.'); + Log::debug('Create piggy event.'); return; } - app('log')->debug('Create no piggy event'); + Log::debug('Create no piggy event'); } private function storeMetaFields(TransactionJournal $journal, NullArrayObject $transaction): void @@ -556,18 +571,18 @@ class TransactionJournalFactory protected function storeMeta(TransactionJournal $journal, NullArrayObject $data, string $field): void { - $set = [ + $set = [ 'journal' => $journal, 'name' => $field, 'data' => (string) ($data[$field] ?? ''), ]; if ($data[$field] instanceof Carbon) { $data[$field]->setTimezone(config('app.timezone')); - app('log')->debug(sprintf('%s Date: %s (%s)', $field, $data[$field], $data[$field]->timezone->getName())); + Log::debug(sprintf('%s Date: %s (%s)', $field, $data[$field], $data[$field]->timezone->getName())); $set['data'] = $data[$field]->format('Y-m-d H:i:s'); } - app('log')->debug(sprintf('Going to store meta-field "%s", with value "%s".', $set['name'], $set['data'])); + Log::debug(sprintf('Going to store meta-field "%s", with value "%s".', $set['name'], $set['data'])); /** @var TransactionJournalMetaFactory $factory */ $factory = app(TransactionJournalMetaFactory::class); @@ -590,7 +605,7 @@ class TransactionJournalFactory { $this->errorOnHash = $errorOnHash; if (true === $errorOnHash) { - app('log')->info('Will trigger duplication alert for this journal.'); + Log::info('Will trigger duplication alert for this journal.'); } } @@ -605,4 +620,25 @@ class TransactionJournalFactory $this->piggyRepository->setUserGroup($userGroup); $this->accountRepository->setUserGroup($userGroup); } + + private function isBetweenAssetAndLiability(Account $source, Account $destination): bool + { + if (null === $source || null === $destination) { + Log::warning('Either is false, stop.'); + return false; + } + $sourceTypes = [AccountTypeEnum::LOAN->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::MORTGAGE->value]; + + // source is liability, destination is asset + if (in_array($source->accountType->type, $sourceTypes, true) && AccountTypeEnum::ASSET->value === $destination->accountType->type) { + Log::debug('Source is a liability account, destination is an asset account, return TRUE.'); + return true; + } + // source is asset, destination is liability + if (in_array($destination->accountType->type, $sourceTypes, true) && AccountTypeEnum::ASSET->value === $source->accountType->type) { + Log::debug('Destination is a liability account, source is an asset account, return TRUE.'); + return true; + } + return false; + } } diff --git a/app/Repositories/Bill/BillRepository.php b/app/Repositories/Bill/BillRepository.php index 56094d1199..c08acbad8e 100644 --- a/app/Repositories/Bill/BillRepository.php +++ b/app/Repositories/Bill/BillRepository.php @@ -133,7 +133,7 @@ class BillRepository implements BillRepositoryInterface, UserGroupInterface return $searchResult; } } - app('log')->debug('Found nothing'); + app('log')->debug('Found no bill in findBill()'); return null; } diff --git a/app/Repositories/Currency/CurrencyRepository.php b/app/Repositories/Currency/CurrencyRepository.php index c6262f2b04..9676766b6c 100644 --- a/app/Repositories/Currency/CurrencyRepository.php +++ b/app/Repositories/Currency/CurrencyRepository.php @@ -65,17 +65,17 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf */ public function currencyInUseAt(TransactionCurrency $currency): ?string { - app('log')->debug(sprintf('Now in currencyInUse() for #%d ("%s")', $currency->id, $currency->code)); + Log::debug(sprintf('Now in currencyInUse() for #%d ("%s")', $currency->id, $currency->code)); $countJournals = $this->countJournals($currency); if ($countJournals > 0) { - app('log')->info(sprintf('Count journals is %d, return true.', $countJournals)); + Log::info(sprintf('Count journals is %d, return true.', $countJournals)); return 'journals'; } // is the only currency left if (1 === $this->getAll()->count()) { - app('log')->info('Is the last currency in the system, return true. '); + Log::info('Is the last currency in the system, return true. '); return 'last_left'; } @@ -83,7 +83,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // is being used in accounts: $meta = AccountMeta::where('name', 'currency_id')->where('data', \Safe\json_encode((string) $currency->id))->count(); if ($meta > 0) { - app('log')->info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); + Log::info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); return 'account_meta'; } @@ -91,7 +91,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // second search using integer check. $meta = AccountMeta::where('name', 'currency_id')->where('data', \Safe\json_encode((int) $currency->id))->count(); if ($meta > 0) { - app('log')->info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); + Log::info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); return 'account_meta'; } @@ -99,7 +99,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // is being used in bills: $bills = Bill::where('transaction_currency_id', $currency->id)->count(); if ($bills > 0) { - app('log')->info(sprintf('Used in %d bills as currency, return true. ', $bills)); + Log::info(sprintf('Used in %d bills as currency, return true. ', $bills)); return 'bills'; } @@ -109,7 +109,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf $recurringForeign = RecurrenceTransaction::where('foreign_currency_id', $currency->id)->count(); if ($recurringAmount > 0 || $recurringForeign > 0) { - app('log')->info(sprintf('Used in %d recurring transactions as (foreign) currency id, return true. ', $recurringAmount + $recurringForeign)); + Log::info(sprintf('Used in %d recurring transactions as (foreign) currency id, return true. ', $recurringAmount + $recurringForeign)); return 'recurring'; } @@ -120,7 +120,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf ->where('account_meta.name', 'currency_id')->where('account_meta.data', \Safe\json_encode($currency->id))->count() ; if ($meta > 0) { - app('log')->info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); + Log::info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); return 'account_meta'; } @@ -128,7 +128,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // is being used in available budgets $availableBudgets = AvailableBudget::where('transaction_currency_id', $currency->id)->count(); if ($availableBudgets > 0) { - app('log')->info(sprintf('Used in %d available budgets as currency, return true. ', $availableBudgets)); + Log::info(sprintf('Used in %d available budgets as currency, return true. ', $availableBudgets)); return 'available_budgets'; } @@ -136,7 +136,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // is being used in budget limits $budgetLimit = BudgetLimit::where('transaction_currency_id', $currency->id)->count(); if ($budgetLimit > 0) { - app('log')->info(sprintf('Used in %d budget limits as currency, return true. ', $budgetLimit)); + Log::info(sprintf('Used in %d budget limits as currency, return true. ', $budgetLimit)); return 'budget_limits'; } @@ -144,7 +144,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // is the default currency for the user or the system $count = $this->userGroup->currencies()->where('transaction_currencies.id', $currency->id)->wherePivot('group_default', 1)->count(); if ($count > 0) { - app('log')->info('Is the default currency of the user, return true.'); + Log::info('Is the default currency of the user, return true.'); return 'current_default'; } @@ -152,12 +152,12 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // is the default currency for the user or the system $count = $this->userGroup->currencies()->where('transaction_currencies.id', $currency->id)->wherePivot('group_default', 1)->count(); if ($count > 0) { - app('log')->info('Is the default currency of the user group, return true.'); + Log::info('Is the default currency of the user group, return true.'); return 'current_default'; } - app('log')->debug('Currency is not used, return false.'); + Log::debug('Currency is not used, return false.'); return null; } @@ -237,15 +237,15 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf $result = $this->findCurrencyNull($currencyId, $currencyCode); if (null === $result) { - app('log')->debug('Grabbing default currency for this user...'); + Log::debug('Grabbing default currency for this user...'); /** @var null|TransactionCurrency $result */ $result = app('amount')->getNativeCurrencyByUserGroup($this->user->userGroup); } - app('log')->debug(sprintf('Final result: %s', $result->code)); + Log::debug(sprintf('Final result: %s', $result->code)); if (false === $result->enabled) { - app('log')->debug(sprintf('Also enabled currency %s', $result->code)); + Log::debug(sprintf('Also enabled currency %s', $result->code)); $this->enable($result); } @@ -257,16 +257,22 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf */ public function findCurrencyNull(?int $currencyId, ?string $currencyCode): ?TransactionCurrency { - app('log')->debug('Now in findCurrencyNull()'); + Log::debug(sprintf('Now in findCurrencyNull(%s, "%s")', var_export($currencyId, true), $currencyCode)); $result = $this->find((int) $currencyId); + if (null !== $result) { + Log::debug(sprintf('Found currency by ID: %s', $result->code)); + + return $result; + } if (null === $result) { - app('log')->debug(sprintf('Searching for currency with code %s...', $currencyCode)); + Log::debug(sprintf('Searching for currency with code "%s"...', $currencyCode)); $result = $this->findByCode((string) $currencyCode); } if (null !== $result && false === $result->enabled) { - app('log')->debug(sprintf('Also enabled currency %s', $result->code)); + Log::debug(sprintf('Also enabled currency %s', $result->code)); $this->enable($result); } + Log::debug('Found no currency, returning NULL.'); return $result; } @@ -321,7 +327,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf ->where('date', $date->format('Y-m-d'))->first() ; if (null !== $rate) { - app('log')->debug(sprintf('Found cached exchange rate in database for %s to %s on %s', $fromCurrency->code, $toCurrency->code, $date->format('Y-m-d'))); + Log::debug(sprintf('Found cached exchange rate in database for %s to %s on %s', $fromCurrency->code, $toCurrency->code, $date->format('Y-m-d'))); return $rate; } @@ -380,7 +386,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf public function update(TransactionCurrency $currency, array $data): TransactionCurrency { - app('log')->debug('Now in update()'); + Log::debug('Now in update()'); // can be true, false, null $enabled = array_key_exists('enabled', $data) ? $data['enabled'] : null; // can be true, false, but method only responds to "true". @@ -396,12 +402,12 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf // currency is enabled, must be disabled. if (false === $enabled) { - app('log')->debug(sprintf('Disabled currency %s for user #%d', $currency->code, $this->userGroup->id)); + Log::debug(sprintf('Disabled currency %s for user #%d', $currency->code, $this->userGroup->id)); $this->userGroup->currencies()->detach($currency->id); } // currency must be enabled if (true === $enabled) { - app('log')->debug(sprintf('Enabled currency %s for user #%d', $currency->code, $this->userGroup->id)); + Log::debug(sprintf('Enabled currency %s for user #%d', $currency->code, $this->userGroup->id)); $this->userGroup->currencies()->detach($currency->id); $this->userGroup->currencies()->syncWithoutDetaching([$currency->id => ['group_default' => false]]); } @@ -420,7 +426,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface, UserGroupInterf public function makeDefault(TransactionCurrency $currency): void { $current = app('amount')->getNativeCurrencyByUserGroup($this->userGroup); - app('log')->debug(sprintf('Enabled + made default currency %s for user #%d', $currency->code, $this->userGroup->id)); + Log::debug(sprintf('Enabled + made default currency %s for user #%d', $currency->code, $this->userGroup->id)); $this->userGroup->currencies()->detach($currency->id); foreach ($this->userGroup->currencies()->get() as $item) { $this->userGroup->currencies()->updateExistingPivot($item->id, ['group_default' => false]); diff --git a/app/Repositories/PiggyBank/PiggyBankRepository.php b/app/Repositories/PiggyBank/PiggyBankRepository.php index 4118bee399..9296d3c977 100644 --- a/app/Repositories/PiggyBank/PiggyBankRepository.php +++ b/app/Repositories/PiggyBank/PiggyBankRepository.php @@ -82,7 +82,7 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface, UserGroupInte return $searchResult; } } - app('log')->debug('Found nothing'); + app('log')->debug('Found no piggy bank.'); return null; } diff --git a/app/Repositories/UserGroups/Currency/CurrencyRepository.php b/app/Repositories/UserGroups/Currency/CurrencyRepository.php index 813bdad772..092ef4b50c 100644 --- a/app/Repositories/UserGroups/Currency/CurrencyRepository.php +++ b/app/Repositories/UserGroups/Currency/CurrencyRepository.php @@ -65,17 +65,17 @@ class CurrencyRepository implements CurrencyRepositoryInterface */ public function currencyInUseAt(TransactionCurrency $currency): ?string { - app('log')->debug(sprintf('Now in currencyInUse() for #%d ("%s")', $currency->id, $currency->code)); + Log::debug(sprintf('Now in currencyInUse() for #%d ("%s")', $currency->id, $currency->code)); $countJournals = $this->countJournals($currency); if ($countJournals > 0) { - app('log')->info(sprintf('Count journals is %d, return true.', $countJournals)); + Log::info(sprintf('Count journals is %d, return true.', $countJournals)); return 'journals'; } // is the only currency left if (1 === $this->getAll()->count()) { - app('log')->info('Is the last currency in the system, return true. '); + Log::info('Is the last currency in the system, return true. '); return 'last_left'; } @@ -83,7 +83,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface // is being used in accounts: $meta = AccountMeta::where('name', 'currency_id')->where('data', \Safe\json_encode((string) $currency->id))->count(); if ($meta > 0) { - app('log')->info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); + Log::info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); return 'account_meta'; } @@ -91,7 +91,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface // second search using integer check. $meta = AccountMeta::where('name', 'currency_id')->where('data', \Safe\json_encode((int) $currency->id))->count(); if ($meta > 0) { - app('log')->info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); + Log::info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); return 'account_meta'; } @@ -99,7 +99,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface // is being used in bills: $bills = Bill::where('transaction_currency_id', $currency->id)->count(); if ($bills > 0) { - app('log')->info(sprintf('Used in %d bills as currency, return true. ', $bills)); + Log::info(sprintf('Used in %d bills as currency, return true. ', $bills)); return 'bills'; } @@ -109,7 +109,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface $recurringForeign = RecurrenceTransaction::where('foreign_currency_id', $currency->id)->count(); if ($recurringAmount > 0 || $recurringForeign > 0) { - app('log')->info(sprintf('Used in %d recurring transactions as (foreign) currency id, return true. ', $recurringAmount + $recurringForeign)); + Log::info(sprintf('Used in %d recurring transactions as (foreign) currency id, return true. ', $recurringAmount + $recurringForeign)); return 'recurring'; } @@ -120,7 +120,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface ->where('account_meta.name', 'currency_id')->where('account_meta.data', \Safe\json_encode($currency->id))->count() ; if ($meta > 0) { - app('log')->info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); + Log::info(sprintf('Used in %d accounts as currency_id, return true. ', $meta)); return 'account_meta'; } @@ -128,7 +128,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface // is being used in available budgets $availableBudgets = AvailableBudget::where('transaction_currency_id', $currency->id)->count(); if ($availableBudgets > 0) { - app('log')->info(sprintf('Used in %d available budgets as currency, return true. ', $availableBudgets)); + Log::info(sprintf('Used in %d available budgets as currency, return true. ', $availableBudgets)); return 'available_budgets'; } @@ -136,7 +136,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface // is being used in budget limits $budgetLimit = BudgetLimit::where('transaction_currency_id', $currency->id)->count(); if ($budgetLimit > 0) { - app('log')->info(sprintf('Used in %d budget limits as currency, return true. ', $budgetLimit)); + Log::info(sprintf('Used in %d budget limits as currency, return true. ', $budgetLimit)); return 'budget_limits'; } @@ -144,7 +144,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface // is the default currency for the user or the system $count = $this->userGroup->currencies()->where('transaction_currencies.id', $currency->id)->wherePivot('group_default', 1)->count(); if ($count > 0) { - app('log')->info('Is the default currency of the user, return true.'); + Log::info('Is the default currency of the user, return true.'); return 'current_default'; } @@ -152,12 +152,12 @@ class CurrencyRepository implements CurrencyRepositoryInterface // is the default currency for the user or the system $count = $this->userGroup->currencies()->where('transaction_currencies.id', $currency->id)->wherePivot('group_default', 1)->count(); if ($count > 0) { - app('log')->info('Is the default currency of the user group, return true.'); + Log::info('Is the default currency of the user group, return true.'); return 'current_default'; } - app('log')->debug('Currency is not used, return false.'); + Log::debug('Currency is not used, return false.'); return null; } @@ -240,15 +240,15 @@ class CurrencyRepository implements CurrencyRepositoryInterface $result = $this->findCurrencyNull($currencyId, $currencyCode); if (null === $result) { - app('log')->debug('Grabbing default currency for this user...'); + Log::debug('Grabbing default currency for this user...'); /** @var null|TransactionCurrency $result */ $result = app('amount')->getNativeCurrencyByUserGroup($this->user->userGroup); } - app('log')->debug(sprintf('Final result: %s', $result->code)); + Log::debug(sprintf('Final result: %s', $result->code)); if (false === $result->enabled) { - app('log')->debug(sprintf('Also enabled currency %s', $result->code)); + Log::debug(sprintf('Also enabled currency %s', $result->code)); $this->enable($result); } @@ -260,14 +260,14 @@ class CurrencyRepository implements CurrencyRepositoryInterface */ public function findCurrencyNull(?int $currencyId, ?string $currencyCode): ?TransactionCurrency { - app('log')->debug('Now in findCurrencyNull()'); + Log::debug(sprintf('Now in findCurrencyNull("%s", "%s")', $currencyId, $currencyCode)); $result = $this->find((int) $currencyId); if (null === $result) { - app('log')->debug(sprintf('Searching for currency with code %s...', $currencyCode)); + Log::debug(sprintf('Searching for currency with code "%s"...', $currencyCode)); $result = $this->findByCode((string) $currencyCode); } if (null !== $result && false === $result->enabled) { - app('log')->debug(sprintf('Also enabled currency %s', $result->code)); + Log::debug(sprintf('Also enabled currency %s', $result->code)); $this->enable($result); } @@ -335,7 +335,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface public function update(TransactionCurrency $currency, array $data): TransactionCurrency { - app('log')->debug('Now in update()'); + Log::debug('Now in update()'); // can be true, false, null $enabled = array_key_exists('enabled', $data) ? $data['enabled'] : null; // can be true, false, but method only responds to "true". @@ -351,12 +351,12 @@ class CurrencyRepository implements CurrencyRepositoryInterface // currency is enabled, must be disabled. if (false === $enabled) { - app('log')->debug(sprintf('Disabled currency %s for user #%d', $currency->code, $this->userGroup->id)); + Log::debug(sprintf('Disabled currency %s for user #%d', $currency->code, $this->userGroup->id)); $this->userGroup->currencies()->detach($currency->id); } // currency must be enabled if (true === $enabled) { - app('log')->debug(sprintf('Enabled currency %s for user #%d', $currency->code, $this->userGroup->id)); + Log::debug(sprintf('Enabled currency %s for user #%d', $currency->code, $this->userGroup->id)); $this->userGroup->currencies()->detach($currency->id); $this->userGroup->currencies()->syncWithoutDetaching([$currency->id => ['group_default' => false]]); } @@ -375,7 +375,7 @@ class CurrencyRepository implements CurrencyRepositoryInterface public function makeDefault(TransactionCurrency $currency): void { $current = app('amount')->getNativeCurrencyByUserGroup($this->userGroup); - app('log')->debug(sprintf('Enabled + made default currency %s for user #%d', $currency->code, $this->userGroup->id)); + Log::debug(sprintf('Enabled + made default currency %s for user #%d', $currency->code, $this->userGroup->id)); $this->userGroup->currencies()->detach($currency->id); foreach ($this->userGroup->currencies()->get() as $item) { $this->userGroup->currencies()->updateExistingPivot($item->id, ['group_default' => false]); diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 456d0df50b..d6de97ed25 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -305,7 +305,7 @@ class AccountValidator return $first; } } - app('log')->debug('Found nothing!'); + app('log')->debug('Found nothing in findExistingAccount()'); return null; } diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php index c6e4d09496..e1b7c573af 100644 --- a/app/Validation/TransactionValidation.php +++ b/app/Validation/TransactionValidation.php @@ -214,7 +214,7 @@ trait TransactionValidation $destination = $accountValidator->destination; Log::debug(sprintf('Source: #%d "%s (%s)"', $source->id, $source->name, $source->accountType->type)); - Log::debug(sprintf('Destination: #%d "%s" (%s)', $destination->id, $destination->name, $source->accountType->type)); + Log::debug(sprintf('Destination: #%d "%s" (%s)', $destination->id, $destination->name, $destination->accountType->type)); if (!$this->isLiabilityOrAsset($source) || !$this->isLiabilityOrAsset($destination)) { Log::debug('Any account must be liability or asset account to continue.'); From 4c8ed784cd1ad340358b36b692ad0512e447b466 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 11 May 2025 08:29:31 +0200 Subject: [PATCH 8/8] Final check for transaction types --- .../Correction/CorrectsUnevenAmount.php | 60 ++++++++++++++++--- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/app/Console/Commands/Correction/CorrectsUnevenAmount.php b/app/Console/Commands/Correction/CorrectsUnevenAmount.php index 0b58dbedb2..e6390c793a 100644 --- a/app/Console/Commands/Correction/CorrectsUnevenAmount.php +++ b/app/Console/Commands/Correction/CorrectsUnevenAmount.php @@ -29,6 +29,7 @@ use FireflyIII\Enums\AccountTypeEnum; use FireflyIII\Enums\TransactionTypeEnum; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; +use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Support\Models\AccountBalanceCalculator; use Illuminate\Console\Command; use Illuminate\Support\Facades\DB; @@ -277,13 +278,13 @@ class CorrectsUnevenAmount extends Command /** @var TransactionJournal $journal */ foreach ($journals as $journal) { - if (!$this->isForeignCurrencyTransfer($journal)) { + if (!$this->isForeignCurrencyTransfer($journal) && !$this->isBetweenAssetAndLiability($journal)) { Transaction::where('transaction_journal_id', $journal->id)->update(['transaction_currency_id' => $journal->transaction_currency_id]); ++$count; continue; } - Log::debug(sprintf('Can skip foreign currency transfer #%d.', $journal->id)); + Log::debug(sprintf('Can skip foreign currency transfer or transaction between asset and liability #%d.', $journal->id)); } if (0 === $count) { return; @@ -331,6 +332,9 @@ class CorrectsUnevenAmount extends Command private function convertOldStyleTransactions(): void { + + /** @var AccountRepositoryInterface $repository */ + $repository = app(AccountRepositoryInterface::class); Log::debug('convertOldStyleTransactions()'); $count = 0; $transactions = Transaction::distinct() @@ -348,6 +352,7 @@ class CorrectsUnevenAmount extends Command foreach ($transactions as $transaction) { /** @var null|TransactionJournal $journal */ $journal = TransactionJournal::find($transaction->transaction_journal_id); + $repository->setUser($journal->user); if (null === $journal) { Log::debug('Found no journal, continue.'); @@ -358,10 +363,12 @@ class CorrectsUnevenAmount extends Command continue; } - $source = $journal->transactions()->where('amount', '<', 0)->first(); - $destination = $journal->transactions()->where('amount', '>', 0)->first(); - $sourceAccount = $source->account; - $destAccount = $destination->account; + $source = $journal->transactions()->where('amount', '<', 0)->first(); + $destination = $journal->transactions()->where('amount', '>', 0)->first(); + $sourceAccount = $source->account; + $destAccount = $destination->account; + $sourceCurrency = $repository->getAccountCurrency($sourceAccount); + $destCurrency = $repository->getAccountCurrency($destAccount); if (null === $source || null === $destination) { Log::debug('Either transaction is NULL, continue.'); @@ -372,7 +379,45 @@ class CorrectsUnevenAmount extends Command continue; } - Log::debug('Ready to swap data between transactions.'); + // source transaction. Switch info when does not match. + if ((int) $source->transaction_currency_id !== (int) $sourceCurrency->id) { + Log::debug(sprintf('Ready to swap data in transaction #%d.', $source->id)); + // swap amounts. + $amount = $source->amount; + $currency = $source->transaction_currency_id; + $source->amount = $source->foreign_amount; + $source->transaction_currency_id = $source->foreign_currency_id; + $source->foreign_amount = $amount; + $source->foreign_currency_id = $currency; + $source->saveQuietly(); + $source->refresh(); +// Log::debug(sprintf('source->amount = %s', $source->amount)); +// Log::debug(sprintf('source->transaction_currency_id = %s', $source->transaction_currency_id)); +// Log::debug(sprintf('source->foreign_amount = %s', $source->foreign_amount)); +// Log::debug(sprintf('source->foreign_currency_id = %s', $source->foreign_currency_id)); + ++$count; + } + // same but for destination + if ((int) $destination->transaction_currency_id !== (int) $destCurrency->id) { + ++$count; + Log::debug(sprintf('Ready to swap data in transaction #%d.', $destination->id)); + // swap amounts. + $amount = $destination->amount; + $currency = $destination->transaction_currency_id; + $destination->amount = $destination->foreign_amount; + $destination->transaction_currency_id = $destination->foreign_currency_id; + $destination->foreign_amount = $amount; + $destination->foreign_currency_id = $currency; + $destination->balance_dirty = true; + $destination->saveQuietly(); + $destination->refresh(); +// Log::debug(sprintf('destination->amount = %s', $destination->amount)); +// Log::debug(sprintf('destination->transaction_currency_id = %s', $destination->transaction_currency_id)); +// Log::debug(sprintf('destination->foreign_amount = %s', $destination->foreign_amount)); +// Log::debug(sprintf('destination->foreign_currency_id = %s', $destination->foreign_currency_id)); + } + + // // only fix the destination transaction // $destination->foreign_currency_id = $source->transaction_currency_id; // $destination->foreign_amount = app('steam')->positive($source->amount); @@ -383,7 +428,6 @@ class CorrectsUnevenAmount extends Command // $destination->save(); // $source->save(); // $this->friendlyWarning(sprintf('Corrected foreign amounts of transaction #%d.', $journal->id)); - ++$count; } if (0 === $count) { return;