From 304fae439a64394479ae08efd0ced0cc1abb68e6 Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 3 Feb 2026 05:42:50 +0100 Subject: [PATCH] Expand listeners and observers. --- app/Factory/TransactionJournalFactory.php | 160 +++++++++--------- .../ExchangeRate/ConversionParameters.php | 2 +- .../ConvertsAmountToPrimaryAmount.php | 13 +- app/Handlers/Observer/PiggyBankObserver.php | 22 ++- app/Handlers/Observer/TransactionObserver.php | 74 +++----- 5 files changed, 118 insertions(+), 153 deletions(-) diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index 828350097d..c8fcc2d560 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/app/Factory/TransactionJournalFactory.php @@ -56,7 +56,6 @@ use FireflyIII\Validation\AccountValidator; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; use JsonException; - use function Safe\json_encode; /** @@ -68,17 +67,17 @@ class TransactionJournalFactory { use JournalServiceTrait; - private AccountRepositoryInterface $accountRepository; - private AccountValidator $accountValidator; - private BillRepositoryInterface $billRepository; - private CurrencyRepositoryInterface $currencyRepository; - private bool $errorOnHash = false; - private array $fields; - private PiggyBankEventFactory $piggyEventFactory; - private PiggyBankRepositoryInterface $piggyRepository; + private AccountRepositoryInterface $accountRepository; + private AccountValidator $accountValidator; + private BillRepositoryInterface $billRepository; + private CurrencyRepositoryInterface $currencyRepository; + private bool $errorOnHash = false; + private array $fields; + private PiggyBankEventFactory $piggyEventFactory; + private PiggyBankRepositoryInterface $piggyRepository; private TransactionTypeRepositoryInterface $typeRepository; - private User $user; - private UserGroup $userGroup; + private User $user; + private UserGroup $userGroup; /** * Constructor. @@ -109,12 +108,8 @@ class TransactionJournalFactory public function create(array $data): Collection { Log::debug('Now in TransactionJournalFactory::create()'); - // convert to special object. - // $dataObject = new NullArrayObject($data); - - Log::debug('Start of TransactionJournalFactory::create()'); - $collection = new Collection(); - $transactions = $data['transactions'] ?? []; + $collection = new Collection(); + $transactions = $data['transactions'] ?? []; if (0 === count($transactions)) { Log::error('There are no transactions in the array, the TransactionJournalFactory cannot continue.'); @@ -127,7 +122,7 @@ class TransactionJournalFactory foreach ($transactions as $index => $row) { $row['batch_submission'] = $batchSubmission; Log::debug(sprintf('Now creating journal %d/%d', $index + 1, count($transactions))); - $journal = $this->createJournal(new NullArrayObject($row)); + $journal = $this->createJournal(new NullArrayObject($row)); if ($journal instanceof TransactionJournal) { $collection->push($journal); } @@ -171,18 +166,18 @@ class TransactionJournalFactory $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; + $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']); + $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 && $bill instanceof Bill ? $bill->id : null; - $description = (string) $row['description']; + $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 && $bill instanceof Bill ? $bill->id : null; + $description = (string)$row['description']; // Manipulate basic fields $carbon->setTimezone(config('app.timezone')); @@ -204,7 +199,7 @@ class TransactionJournalFactory } /** create or get source and destination accounts */ - $sourceInfo = [ + $sourceInfo = [ 'id' => $row['source_id'], 'name' => $row['source_name'], 'iban' => $row['source_iban'], @@ -213,7 +208,7 @@ class TransactionJournalFactory 'currency_id' => $currency->id, ]; - $destInfo = [ + $destInfo = [ 'id' => $row['destination_id'], 'name' => $row['destination_name'], 'iban' => $row['destination_iban'], @@ -223,8 +218,8 @@ class TransactionJournalFactory ]; Log::debug('Source info:', $sourceInfo); Log::debug('Destination info:', $destInfo); - $destinationAccount = null; - $sourceAccount = null; + $destinationAccount = null; + $sourceAccount = null; if (TransactionTypeEnum::DEPOSIT->value === $type->type) { Log::debug('Transaction type is deposit, start with destination first.'); $destinationAccount = $this->getAccount($type->type, 'destination', $destInfo); @@ -249,38 +244,38 @@ class TransactionJournalFactory [$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); Log::debug(sprintf( - 'Currency is #%d "%s", foreign currency is #%d "%s"', - $currency->id, - $currency->code, - $foreignCurrency?->id, - $foreignCurrency?->code - )); + 'Currency is #%d "%s", foreign currency is #%d "%s"', + $currency->id, + $currency->code, + $foreignCurrency?->id, + $foreignCurrency?->code + )); Log::debug(sprintf('Date: %s (%s)', $carbon->toW3cString(), $carbon->getTimezone()->getName())); /** Create a basic journal. */ - $journal = TransactionJournal::create([ - 'user_id' => $this->user->id, - 'user_group_id' => $this->userGroup->id, - 'transaction_type_id' => $type->id, - 'bill_id' => $billId, - 'transaction_currency_id' => $currency->id, - 'description' => substr($description, 0, 1000), - 'date' => $carbon, - 'date_tz' => $carbon->format('e'), - 'order' => $order, - 'tag_count' => 0, - 'completed' => !$row['batch_submission'], - ]); + $journal = TransactionJournal::create([ + 'user_id' => $this->user->id, + 'user_group_id' => $this->userGroup->id, + 'transaction_type_id' => $type->id, + 'bill_id' => $billId, + 'transaction_currency_id' => $currency->id, + 'description' => substr($description, 0, 1000), + 'date' => $carbon, + 'date_tz' => $carbon->format('e'), + 'order' => $order, + 'tag_count' => 0, + 'completed' => !$row['batch_submission'], + ]); 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); @@ -289,7 +284,7 @@ class TransactionJournalFactory $transactionFactory->setReconciled($row['reconciled'] ?? false); try { - $negative = $transactionFactory->createNegative((string) $row['amount'], (string) $row['foreign_amount']); + $negative = $transactionFactory->createNegative((string)$row['amount'], (string)$row['foreign_amount']); } catch (FireflyException $e) { Log::error(sprintf('Exception creating negative transaction: %s', $e->getMessage())); $this->forceDeleteOnError(new Collection()->push($journal)); @@ -298,7 +293,7 @@ class TransactionJournalFactory } /** @var TransactionFactory $transactionFactory */ - $transactionFactory = app(TransactionFactory::class); + $transactionFactory = app(TransactionFactory::class); $transactionFactory->setJournal($journal); $transactionFactory->setAccount($destinationAccount); $transactionFactory->setAccountInformation($destInfo); @@ -310,8 +305,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 ( $foreignCurrency instanceof TransactionCurrency && $foreignCurrency->id !== $currency->id @@ -319,8 +314,8 @@ class TransactionJournalFactory ) { $transactionFactory->setCurrency($foreignCurrency); $transactionFactory->setForeignCurrency($currency); - $amount = (string) $row['foreign_amount']; - $foreignAmount = (string) $row['amount']; + $amount = (string)$row['foreign_amount']; + $foreignAmount = (string)$row['amount']; Log::debug('Swap primary/foreign amounts in transfer for new save method.'); } @@ -379,18 +374,17 @@ class TransactionJournalFactory /** @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', 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', json_encode($hash, JSON_THROW_ON_ERROR)) + ->with(['transactionJournal', 'transactionJournal.transactionGroup']) + ->first(['journal_meta.*']); if (null !== $result) { 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; + $groupId = (int)$group?->id; throw new DuplicateTransactionException(sprintf('Duplicate of transaction #%d.', $groupId)); } @@ -402,18 +396,18 @@ class TransactionJournalFactory private function validateAccounts(NullArrayObject $data): void { Log::debug(sprintf('Now in %s', __METHOD__)); - $transactionType = $data['type'] ?? 'invalid'; + $transactionType = $data['type'] ?? 'invalid'; $this->accountValidator->setUser($this->user); $this->accountValidator->setTransactionType($transactionType); // validate source account. - $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, + $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) { @@ -422,11 +416,11 @@ class TransactionJournalFactory Log::debug('Source seems valid.'); // validate destination account - $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, - 'number' => null !== $data['destination_number'] ? (string) $data['destination_number'] : null, + $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, + 'number' => null !== $data['destination_number'] ? (string)$data['destination_number'] : null, ]; $validDestination = $this->accountValidator->validateDestination($array); @@ -511,7 +505,7 @@ class TransactionJournalFactory // return user's default: return Amount::getPrimaryCurrencyByUserGroup($this->user->userGroup); } - $result = $preference ?? $currency; + $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; @@ -581,7 +575,7 @@ class TransactionJournalFactory { Log::debug('Will now store piggy event.'); - $piggyBank = $this->piggyRepository->findPiggyBank((int) $data['piggy_bank_id'], $data['piggy_bank_name']); + $piggyBank = $this->piggyRepository->findPiggyBank((int)$data['piggy_bank_id'], $data['piggy_bank_name']); if ($piggyBank instanceof PiggyBank) { $this->piggyEventFactory->create($journal, $piggyBank); @@ -601,7 +595,7 @@ class TransactionJournalFactory protected function storeMeta(TransactionJournal $journal, array $data, string $field): void { - $set = ['journal' => $journal, 'name' => $field, 'data' => (string) ($data[$field] ?? '')]; + $set = ['journal' => $journal, 'name' => $field, 'data' => (string)($data[$field] ?? '')]; if (array_key_exists($field, $data) && $data[$field] instanceof Carbon) { $data[$field]->setTimezone(config('app.timezone')); Log::debug(sprintf('%s Date: %s (%s)', $field, $data[$field], $data[$field]->timezone->getName())); diff --git a/app/Handlers/ExchangeRate/ConversionParameters.php b/app/Handlers/ExchangeRate/ConversionParameters.php index e13cf041e0..a1ab87282d 100644 --- a/app/Handlers/ExchangeRate/ConversionParameters.php +++ b/app/Handlers/ExchangeRate/ConversionParameters.php @@ -30,7 +30,7 @@ class ConversionParameters { public User $user; public Model $model; - public TransactionCurrency $originalCurrency; + public ?TransactionCurrency $originalCurrency = null; public string $amountField; public string $primaryAmountField; public Carbon $date; diff --git a/app/Handlers/ExchangeRate/ConvertsAmountToPrimaryAmount.php b/app/Handlers/ExchangeRate/ConvertsAmountToPrimaryAmount.php index febe4d1d75..ddfcee79e5 100644 --- a/app/Handlers/ExchangeRate/ConvertsAmountToPrimaryAmount.php +++ b/app/Handlers/ExchangeRate/ConvertsAmountToPrimaryAmount.php @@ -29,8 +29,17 @@ class ConvertsAmountToPrimaryAmount { public static function convert(ConversionParameters $params): void { + $amountField = $params->amountField; + $primaryAmountField = $params->primaryAmountField; + if (!Amount::convertToPrimary($params->user)) { - Log::debug(sprintf('User does not want to do conversion, no need to convert %s and store it in field %s.', $amountField, $primaryAmountField)); + Log::debug(sprintf('User does not want to do conversion, no need to convert %s and store it in field %s for %s #%d.', $params->amountField, $params->primaryAmountField, get_class($params->model), $params->model->id)); + $params->model->$primaryAmountField = null; + $params->model->saveQuietly(); + return; + } + if(null === $params->originalCurrency) { + Log::debug(sprintf('Original currency field is empty, no need to convert %s and store it in field %s for %s #%d.', $params->amountField, $params->primaryAmountField, get_class($params->model), $params->model->id)); return; } $primaryCurrency = Amount::getPrimaryCurrencyByUserGroup($params->user->userGroup); @@ -39,8 +48,6 @@ class ConvertsAmountToPrimaryAmount Log::debug('Both currencies are the same, do nothing.'); return; } - $amountField = $params->amountField; - $primaryAmountField = $params->primaryAmountField; // field is empty or zero, do nothing. diff --git a/app/Handlers/Observer/PiggyBankObserver.php b/app/Handlers/Observer/PiggyBankObserver.php index 943775d6d4..4f6dc3e214 100644 --- a/app/Handlers/Observer/PiggyBankObserver.php +++ b/app/Handlers/Observer/PiggyBankObserver.php @@ -24,11 +24,11 @@ declare(strict_types=1); namespace FireflyIII\Handlers\Observer; +use FireflyIII\Handlers\ExchangeRate\ConversionParameters; +use FireflyIII\Handlers\ExchangeRate\ConvertsAmountToPrimaryAmount; use FireflyIII\Models\Attachment; use FireflyIII\Models\PiggyBank; use FireflyIII\Repositories\Attachment\AttachmentRepositoryInterface; -use FireflyIII\Support\Facades\Amount; -use FireflyIII\Support\Http\Api\ExchangeRateConverter; use Illuminate\Support\Facades\Log; /** @@ -77,15 +77,13 @@ class PiggyBankObserver return; } - $userCurrency = Amount::getPrimaryCurrencyByUserGroup($group); - $piggyBank->native_target_amount = null; - if ($piggyBank->transactionCurrency->id !== $userCurrency->id) { - $converter = new ExchangeRateConverter(); - $converter->setIgnoreSettings(true); - $converter->setUserGroup($group); - $piggyBank->native_target_amount = $converter->convert($piggyBank->transactionCurrency, $userCurrency, today(), $piggyBank->target_amount); - } - $piggyBank->saveQuietly(); - Log::debug('Piggy bank primary currency target amount is updated.'); + + $params = new ConversionParameters(); + $params->user = $piggyBank->accounts()->first()?->user; + $params->model = $piggyBank; + $params->originalCurrency = $piggyBank->transactionCurrency; + $params->amountField = 'target_amount'; + $params->primaryAmountField = 'native_target_amount'; + ConvertsAmountToPrimaryAmount::convert($params); } } diff --git a/app/Handlers/Observer/TransactionObserver.php b/app/Handlers/Observer/TransactionObserver.php index 5941cf9242..537c3cd9a2 100644 --- a/app/Handlers/Observer/TransactionObserver.php +++ b/app/Handlers/Observer/TransactionObserver.php @@ -23,11 +23,9 @@ declare(strict_types=1); namespace FireflyIII\Handlers\Observer; +use FireflyIII\Handlers\ExchangeRate\ConversionParameters; +use FireflyIII\Handlers\ExchangeRate\ConvertsAmountToPrimaryAmount; use FireflyIII\Models\Transaction; -use FireflyIII\Support\Facades\Amount; -use FireflyIII\Support\Facades\FireflyConfig; -use FireflyIII\Support\Http\Api\ExchangeRateConverter; -use FireflyIII\Support\Models\AccountBalanceCalculator; use Illuminate\Support\Facades\Log; /** @@ -39,8 +37,6 @@ class TransactionObserver public function created(Transaction $transaction): void { - return; - $this->updatePrimaryCurrencyAmount($transaction); } @@ -52,59 +48,29 @@ class TransactionObserver public function updated(Transaction $transaction): void { - // Log::debug('Observe "updated" of a transaction.'); - if ( - true === FireflyConfig::get('use_running_balance', config('firefly.feature_flags.running_balance_column'))->data - && self::$recalculate - && 1 === bccomp($transaction->amount, '0') - ) { - Log::debug('Trigger recalculateForJournal'); - AccountBalanceCalculator::recalculateForJournal($transaction->transactionJournal); - } $this->updatePrimaryCurrencyAmount($transaction); } private function updatePrimaryCurrencyAmount(Transaction $transaction): void { - if (!Amount::convertToPrimary($transaction->transactionJournal->user)) { - return; - } - $userCurrency = Amount::getPrimaryCurrencyByUserGroup($transaction->transactionJournal->user->userGroup); - $transaction->native_amount = null; - $transaction->native_foreign_amount = null; - // first normal amount - if ( - $transaction->transactionCurrency->id !== $userCurrency->id - && ( - null === $transaction->foreign_currency_id - || null !== $transaction->foreign_currency_id - && $transaction->foreign_currency_id !== $userCurrency->id - ) - ) { - $converter = new ExchangeRateConverter(); - $converter->setUserGroup($transaction->transactionJournal->user->userGroup); - $converter->setIgnoreSettings(true); - $transaction->native_amount = $converter->convert( - $transaction->transactionCurrency, - $userCurrency, - $transaction->transactionJournal->date, - $transaction->amount - ); - } - // then foreign amount - if ($transaction->foreignCurrency?->id !== $userCurrency->id && null !== $transaction->foreign_amount && null !== $transaction->foreignCurrency) { - $converter = new ExchangeRateConverter(); - $converter->setUserGroup($transaction->transactionJournal->user->userGroup); - $converter->setIgnoreSettings(true); - $transaction->native_foreign_amount = $converter->convert( - $transaction->foreignCurrency, - $userCurrency, - $transaction->transactionJournal->date, - $transaction->foreign_amount - ); - } + // convert "amount" to "native_amount" + $params = new ConversionParameters(); + $params->user = $transaction->transactionJournal->user; + $params->model = $transaction; + $params->originalCurrency = $transaction->transactionCurrency; + $params->amountField = 'amount'; + $params->date = $transaction->transactionJournal->date; + $params->primaryAmountField = 'native_amount'; + ConvertsAmountToPrimaryAmount::convert($params); - $transaction->saveQuietly(); - Log::debug(sprintf('Transaction #%d primary currency amounts are updated.', $transaction->id)); + // convert "foreign_amount" to "native_foreign_amount" + $params = new ConversionParameters(); + $params->user = $transaction->transactionJournal->user; + $params->model = $transaction; + $params->originalCurrency = $transaction->foreignCurrency; + $params->date = $transaction->transactionJournal->date; + $params->amountField = 'foreign_amount'; + $params->primaryAmountField = 'native_foreign_amount'; + ConvertsAmountToPrimaryAmount::convert($params); } }