New way to store transfers

This commit is contained in:
James Cole
2024-07-30 18:04:39 +02:00
parent ebaebb09d1
commit 7e37d10016
4 changed files with 108 additions and 24 deletions

View File

@@ -69,7 +69,7 @@ class UpdateController extends Controller
*/ */
public function update(UpdateRequest $request, TransactionGroup $transactionGroup): JsonResponse public function update(UpdateRequest $request, TransactionGroup $transactionGroup): JsonResponse
{ {
app('log')->debug('Now in update routine for transaction group!'); app('log')->debug('Now in update routine for transaction group');
$data = $request->getAll(); $data = $request->getAll();
// Fixes 8750. // Fixes 8750.

View File

@@ -26,6 +26,8 @@ namespace FireflyIII\Console\Commands\Correction;
use FireflyIII\Console\Commands\ShowsFriendlyMessages; use FireflyIII\Console\Commands\ShowsFriendlyMessages;
use FireflyIII\Models\Transaction; use FireflyIII\Models\Transaction;
use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionJournal;
use FireflyIII\Models\TransactionType;
use FireflyIII\Support\Facades\Steam;
use Illuminate\Console\Command; use Illuminate\Console\Command;
use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Log;
@@ -38,12 +40,14 @@ class FixUnevenAmount extends Command
protected $description = 'Fix journals with uneven amounts.'; protected $description = 'Fix journals with uneven amounts.';
protected $signature = 'firefly-iii:fix-uneven-amount'; protected $signature = 'firefly-iii:fix-uneven-amount';
private int $count;
/** /**
* Execute the console command. * Execute the console command.
*/ */
public function handle(): int public function handle(): int
{ {
$this->count = 0;
$this->fixUnevenAmounts(); $this->fixUnevenAmounts();
$this->matchCurrencies(); $this->matchCurrencies();
@@ -71,7 +75,7 @@ class FixUnevenAmount extends Command
); );
Transaction::where('transaction_journal_id', $journal->id ?? 0)->forceDelete(); Transaction::where('transaction_journal_id', $journal->id ?? 0)->forceDelete();
TransactionJournal::where('id', $journal->id ?? 0)->forceDelete(); TransactionJournal::where('id', $journal->id ?? 0)->forceDelete();
++$this->count;
return; return;
} }
@@ -92,20 +96,31 @@ class FixUnevenAmount extends Command
Transaction::where('transaction_journal_id', $journal->id ?? 0)->forceDelete(); Transaction::where('transaction_journal_id', $journal->id ?? 0)->forceDelete();
TransactionJournal::where('id', $journal->id ?? 0)->forceDelete(); TransactionJournal::where('id', $journal->id ?? 0)->forceDelete();
++$this->count;
return; return;
} }
// may still be able to salvage this journal if it is a transfer with foreign currency info
if($this->isForeignCurrencyTransfer($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);
$this->friendlyWarning($message);
app('log')->warning($message);
$destination->amount = $amount; $destination->amount = $amount;
$destination->save(); $destination->save();
$message = sprintf('Corrected amount in transaction journal #%d', $param); $message = sprintf('Corrected amount in transaction journal #%d', $param);
$this->friendlyInfo($message); $this->friendlyInfo($message);
++$this->count;
} }
private function fixUnevenAmounts(): void private function fixUnevenAmounts(): void
{ {
$count = 0;
$journals = \DB::table('transactions') $journals = \DB::table('transactions')
->groupBy('transaction_journal_id') ->groupBy('transaction_journal_id')
->whereNull('deleted_at') ->whereNull('deleted_at')
@@ -126,7 +141,7 @@ class FixUnevenAmount extends Command
); );
$this->friendlyWarning($message); $this->friendlyWarning($message);
app('log')->warning($message); app('log')->warning($message);
++$count; ++$this->count;
continue; continue;
} }
@@ -140,18 +155,10 @@ class FixUnevenAmount extends Command
Log::error($e->getTraceAsString()); Log::error($e->getTraceAsString());
} }
if (0 !== $res) { if (0 !== $res) {
$message = sprintf(
'Sum of journal #%d is %s instead of zero.',
$entry->transaction_journal_id,
$entry->the_sum
);
$this->friendlyWarning($message);
app('log')->warning($message);
$this->fixJournal($entry->transaction_journal_id); $this->fixJournal($entry->transaction_journal_id);
++$count;
} }
} }
if (0 === $count) { if (0 === $this->count) {
$this->friendlyPositive('Database amount integrity is OK'); $this->friendlyPositive('Database amount integrity is OK');
} }
} }
@@ -160,18 +167,56 @@ class FixUnevenAmount extends Command
{ {
$journals = TransactionJournal::leftJoin('transactions', 'transaction_journals.id', 'transactions.transaction_journal_id') $journals = TransactionJournal::leftJoin('transactions', 'transaction_journals.id', 'transactions.transaction_journal_id')
->where('transactions.transaction_currency_id', '!=', \DB::raw('transaction_journals.transaction_currency_id')) ->where('transactions.transaction_currency_id', '!=', \DB::raw('transaction_journals.transaction_currency_id'))
->get(['transaction_journals.*']) ->get(['transaction_journals.*']);
;
if (0 === $journals->count()) { $count = 0;
/** @var TransactionJournal $journal */
foreach ($journals as $journal) {
if(!$this->isForeignCurrencyTransfer($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));
}
if (0 === $count) {
$this->friendlyPositive('Journal currency integrity is OK'); $this->friendlyPositive('Journal currency integrity is OK');
return; return;
} }
/** @var TransactionJournal $journal */
foreach ($journals as $journal) {
Transaction::where('transaction_journal_id', $journal->id)->update(['transaction_currency_id' => $journal->transaction_currency_id]);
}
$this->friendlyPositive(sprintf('Fixed %d journal(s) with mismatched currencies.', $journals->count())); $this->friendlyPositive(sprintf('Fixed %d journal(s) with mismatched currencies.', $journals->count()));
} }
private function isForeignCurrencyTransfer(TransactionJournal $journal): bool
{
if(TransactionType::TRANSFER !== $journal->transactionType->type) {
return false;
}
/** @var Transaction $destination */
$destination = $journal->transactions()->where('amount', '>', 0)->first();
/** @var Transaction $source */
$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
// source currency = dest foreign currency
// dest amount = source foreign currency
// dest currency = source foreign currency
// Log::debug(sprintf('[a] %s', bccomp(app('steam')->positive($source->amount), app('steam')->positive($destination->foreign_amount))));
// Log::debug(sprintf('[b] %s', bccomp(app('steam')->positive($destination->amount), app('steam')->positive($source->foreign_amount))));
// Log::debug(sprintf('[c] %s', var_export($source->transaction_currency_id === $destination->foreign_currency_id,true)));
// Log::debug(sprintf('[d] %s', var_export((int) $destination->transaction_currency_id ===(int) $source->foreign_currency_id, true)));
if(0 === bccomp(app('steam')->positive($source->amount), app('steam')->positive($destination->foreign_amount)) &&
$source->transaction_currency_id === $destination->foreign_currency_id &&
0 === bccomp(app('steam')->positive($destination->amount), app('steam')->positive($source->foreign_amount)) &&
(int) $destination->transaction_currency_id === (int) $source->foreign_currency_id
) {
return true;
}
return false;
}
} }

View File

@@ -47,6 +47,7 @@ use FireflyIII\Support\NullArrayObject;
use FireflyIII\User; use FireflyIII\User;
use FireflyIII\Validation\AccountValidator; use FireflyIII\Validation\AccountValidator;
use Illuminate\Support\Collection; use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Log;
/** /**
* Class TransactionJournalFactory * Class TransactionJournalFactory
@@ -261,8 +262,24 @@ class TransactionJournalFactory
$transactionFactory->setForeignCurrency($foreignCurrency); $transactionFactory->setForeignCurrency($foreignCurrency);
$transactionFactory->setReconciled($row['reconciled'] ?? false); $transactionFactory->setReconciled($row['reconciled'] ?? false);
// if the foreign currency is set and is different, and the transaction type is a transfer,
// 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'];
if(null !== $foreignCurrency && $foreignCurrency->id !== $currency->id &&
TransactionType::TRANSFER === $type->type
) {
$transactionFactory->setCurrency($foreignCurrency);
$transactionFactory->setForeignCurrency($currency);
$amount = (string)$row['foreign_amount'];
$foreignAmount = (string)$row['amount'];
Log::debug('Swap native/foreign amounts in transfer for new save method.');
}
try { try {
$transactionFactory->createPositive((string)$row['amount'], (string)$row['foreign_amount']); $transactionFactory->createPositive($amount, $foreignAmount);
} catch (FireflyException $e) { } catch (FireflyException $e) {
app('log')->error(sprintf('Exception creating positive transaction: %s', $e->getMessage())); app('log')->error(sprintf('Exception creating positive transaction: %s', $e->getMessage()));
$this->forceTrDelete($negative); $this->forceTrDelete($negative);

View File

@@ -33,6 +33,7 @@ use FireflyIII\Factory\TransactionJournalMetaFactory;
use FireflyIII\Factory\TransactionTypeFactory; use FireflyIII\Factory\TransactionTypeFactory;
use FireflyIII\Models\Account; use FireflyIII\Models\Account;
use FireflyIII\Models\Transaction; use FireflyIII\Models\Transaction;
use FireflyIII\Models\TransactionCurrency;
use FireflyIII\Models\TransactionGroup; use FireflyIII\Models\TransactionGroup;
use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionJournal;
use FireflyIII\Models\TransactionType; use FireflyIII\Models\TransactionType;
@@ -44,6 +45,7 @@ use FireflyIII\Repositories\UserGroups\Currency\CurrencyRepositoryInterface;
use FireflyIII\Services\Internal\Support\JournalServiceTrait; use FireflyIII\Services\Internal\Support\JournalServiceTrait;
use FireflyIII\Support\NullArrayObject; use FireflyIII\Support\NullArrayObject;
use FireflyIII\Validation\AccountValidator; use FireflyIII\Validation\AccountValidator;
use Illuminate\Support\Facades\Log;
/** /**
* Class to centralise code that updates a journal given the input by system. * Class to centralise code that updates a journal given the input by system.
@@ -698,8 +700,23 @@ class JournalUpdateService
$source->foreign_currency_id = $foreignCurrency->id; $source->foreign_currency_id = $foreignCurrency->id;
$source->foreign_amount = app('steam')->negative($foreignAmount); $source->foreign_amount = app('steam')->negative($foreignAmount);
$source->save(); $source->save();
$dest->foreign_currency_id = $foreignCurrency->id;
$dest->foreign_amount = app('steam')->positive($foreignAmount); // if the transaction is a TRANSFER, and the foreign amount and currency are set (like they seem to be)
// the correct fields to update in the destination transaction are NOT the foreign amount and currency
// but rather the normal amount and currency. This is new behavior.
if(TransactionType::TRANSFER === $this->transactionJournal->transactionType->type) {
Log::debug('Switch amounts, store in amount and not foreign_amount');
$dest->transaction_currency_id = $foreignCurrency->id;
$dest->amount = app('steam')->positive($foreignAmount);
}
if(TransactionType::TRANSFER !== $this->transactionJournal->transactionType->type) {
$dest->foreign_currency_id = $foreignCurrency->id;
$dest->foreign_amount = app('steam')->positive($foreignAmount);
}
$dest->save(); $dest->save();
app('log')->debug( app('log')->debug(
@@ -733,4 +750,9 @@ class JournalUpdateService
$this->sourceTransaction->refresh(); $this->sourceTransaction->refresh();
$this->destinationTransaction->refresh(); $this->destinationTransaction->refresh();
} }
private function collectCurrency(): TransactionCurrency
{
}
} }