diff --git a/app/TransactionRules/Actions/ConvertToDeposit.php b/app/TransactionRules/Actions/ConvertToDeposit.php index 99dd4d3e83..c4f774d543 100644 --- a/app/TransactionRules/Actions/ConvertToDeposit.php +++ b/app/TransactionRules/Actions/ConvertToDeposit.php @@ -27,14 +27,15 @@ use DB; use FireflyIII\Events\TriggeredAuditLog; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Factory\AccountFactory; +use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\RuleAction; +use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; -use FireflyIII\User; -use JsonException; use Illuminate\Support\Facades\Log; +use JsonException; /** * @@ -56,11 +57,16 @@ class ConvertToDeposit implements ActionInterface /** * @inheritDoc - * @throws FireflyException - * @throws JsonException */ public function actOnArray(array $journal): bool { + // make object from array (so the data is fresh). + /** @var TransactionJournal|null $object */ + $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + if (null === $object) { + Log::error(sprintf('Cannot find journal #%d, cannot convert to deposit.', $journal['transaction_journal_id'])); + return false; + } $groupCount = TransactionJournal::where('transaction_group_id', $journal['transaction_group_id'])->count(); if ($groupCount > 1) { Log::error(sprintf('Group #%d has more than one transaction in it, cannot convert to deposit.', $journal['transaction_group_id'])); @@ -68,7 +74,7 @@ class ConvertToDeposit implements ActionInterface } Log::debug(sprintf('Convert journal #%d to deposit.', $journal['transaction_journal_id'])); - $type = $journal['transaction_type_type']; + $type = $object->transactionType->type; if (TransactionType::DEPOSIT === $type) { Log::error(sprintf('Journal #%d is already a deposit (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id)); @@ -77,17 +83,32 @@ class ConvertToDeposit implements ActionInterface if (TransactionType::WITHDRAWAL === $type) { Log::debug('Going to transform a withdrawal to a deposit.'); - $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + + try { + $res = $this->convertWithdrawalArray($object); + } catch (JsonException|FireflyException $e) { + Log::debug('Could not convert withdrawal to deposit.'); + Log::error($e->getMessage()); + return false; + } + event(new TriggeredAuditLog($this->action->rule, $object, 'update_transaction_type', TransactionType::WITHDRAWAL, TransactionType::DEPOSIT)); - return $this->convertWithdrawalArray($journal); + return $res; } if (TransactionType::TRANSFER === $type) { - $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); - event(new TriggeredAuditLog($this->action->rule, $object, 'update_transaction_type', TransactionType::TRANSFER, TransactionType::DEPOSIT)); Log::debug('Going to transform a transfer to a deposit.'); - return $this->convertTransferArray($journal); + try { + $res = $this->convertTransferArray($object); + } catch (JsonException|FireflyException $e) { + Log::debug('Could not convert transfer to deposit.'); + Log::error($e->getMessage()); + return false; + } + event(new TriggeredAuditLog($this->action->rule, $object, 'update_transaction_type', TransactionType::TRANSFER, TransactionType::DEPOSIT)); + + return $res; } return false; @@ -97,15 +118,15 @@ class ConvertToDeposit implements ActionInterface * Input is a withdrawal from A to B * Is converted to a deposit from C to A. * - * @param array $journal + * @param TransactionJournal $journal * * @return bool * @throws FireflyException * @throws JsonException */ - private function convertWithdrawalArray(array $journal): bool + private function convertWithdrawalArray(TransactionJournal $journal): bool { - $user = User::find($journal['user_id']); + $user = $journal->user; // find or create revenue account. /** @var AccountFactory $factory */ $factory = app(AccountFactory::class); @@ -114,35 +135,38 @@ class ConvertToDeposit implements ActionInterface $repository = app(AccountRepositoryInterface::class); $repository->setUser($user); + $destAccount = $this->getDestinationAccount($journal); + $sourceAccount = $this->getSourceAccount($journal); + // get the action value, or use the original destination name in case the action value is empty: // this becomes a new or existing (revenue) account, which is the source of the new deposit. - $opposingName = '' === $this->action->action_value ? $journal['destination_account_name'] : $this->action->action_value; + $opposingName = '' === $this->action->action_value ? $destAccount->name : $this->action->action_value; // we check all possible source account types if one exists: - $validTypes = config('firefly.expected_source_types.source.Deposit'); - $opposingAccount = $repository->findByName($opposingName, $validTypes); + $validTypes = config('firefly.expected_source_types.source.Deposit'); + $opposingAccount = $repository->findByName($opposingName, $validTypes); if (null === $opposingAccount) { $opposingAccount = $factory->findOrCreate($opposingName, AccountType::REVENUE); } - Log::debug(sprintf('ConvertToDeposit. Action value is "%s", new opposing name is "%s"', $this->action->action_value, $journal['destination_account_name'])); + Log::debug(sprintf('ConvertToDeposit. Action value is "%s", new opposing name is "%s"', $this->action->action_value, $opposingAccount->name)); // update the source transaction and put in the new revenue ID. DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', ) ->where('amount', '<', 0) ->update(['account_id' => $opposingAccount->id]); // update the destination transaction and put in the original source account ID. DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', $journal->id) ->where('amount', '>', 0) - ->update(['account_id' => $journal['source_account_id']]); + ->update(['account_id' => $sourceAccount->id]); // change transaction type of journal: $newType = TransactionType::whereType(TransactionType::DEPOSIT)->first(); DB::table('transaction_journals') - ->where('id', '=', $journal['transaction_journal_id']) + ->where('id', '=', $journal->id) ->update(['transaction_type_id' => $newType->id, 'bill_id' => null]); Log::debug('Converted withdrawal to deposit.'); @@ -155,15 +179,15 @@ class ConvertToDeposit implements ActionInterface * Output is a deposit from C to B. * The source account is replaced. * - * @param array $journal + * @param TransactionJournal $journal * * @return bool * @throws FireflyException * @throws JsonException */ - private function convertTransferArray(array $journal): bool + private function convertTransferArray(TransactionJournal $journal): bool { - $user = User::find($journal['user_id']); + $user = $journal->user; // find or create revenue account. /** @var AccountFactory $factory */ $factory = app(AccountFactory::class); @@ -172,21 +196,23 @@ class ConvertToDeposit implements ActionInterface $repository = app(AccountRepositoryInterface::class); $repository->setUser($user); + $sourceAccount = $this->getSourceAccount($journal); + // get the action value, or use the original source name in case the action value is empty: // this becomes a new or existing (revenue) account, which is the source of the new deposit. - $opposingName = '' === $this->action->action_value ? $journal['source_account_name'] : $this->action->action_value; + $opposingName = '' === $this->action->action_value ? $sourceAccount->name : $this->action->action_value; // we check all possible source account types if one exists: - $validTypes = config('firefly.expected_source_types.source.Deposit'); - $opposingAccount = $repository->findByName($opposingName, $validTypes); + $validTypes = config('firefly.expected_source_types.source.Deposit'); + $opposingAccount = $repository->findByName($opposingName, $validTypes); if (null === $opposingAccount) { $opposingAccount = $factory->findOrCreate($opposingName, AccountType::REVENUE); } - Log::debug(sprintf('ConvertToDeposit. Action value is "%s", revenue name is "%s"', $this->action->action_value, $journal['source_account_name'])); + Log::debug(sprintf('ConvertToDeposit. Action value is "%s", revenue name is "%s"', $this->action->action_value, $opposingAccount->name)); // update source transaction(s) to be revenue account DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', $journal->id) ->where('amount', '<', 0) ->update(['account_id' => $opposingAccount->id]); @@ -194,11 +220,41 @@ class ConvertToDeposit implements ActionInterface $newType = TransactionType::whereType(TransactionType::DEPOSIT)->first(); DB::table('transaction_journals') - ->where('id', '=', $journal['transaction_journal_id']) + ->where('id', '=', $journal->id) ->update(['transaction_type_id' => $newType->id, 'bill_id' => null]); Log::debug('Converted transfer to deposit.'); return true; } + + /** + * @param TransactionJournal $journal + * @return Account + * @throws FireflyException + */ + private function getSourceAccount(TransactionJournal $journal): Account + { + /** @var Transaction|null $sourceTransaction */ + $sourceTransaction = $journal->transactions()->where('amount', '<', 0)->first(); + if (null === $sourceTransaction) { + throw new FireflyException(sprintf('Cannot find source transaction for journal #%d', $journal->id)); + } + return $sourceTransaction->account; + } + + /** + * @param TransactionJournal $journal + * @return Account + * @throws FireflyException + */ + private function getDestinationAccount(TransactionJournal $journal): Account + { + /** @var Transaction|null $destAccount */ + $destAccount = $journal->transactions()->where('amount', '>', 0)->first(); + if (null === $destAccount) { + throw new FireflyException(sprintf('Cannot find destination transaction for journal #%d', $journal->id)); + } + return $destAccount->account; + } } diff --git a/app/TransactionRules/Actions/ConvertToTransfer.php b/app/TransactionRules/Actions/ConvertToTransfer.php index 30baedc4d1..28dddc06a2 100644 --- a/app/TransactionRules/Actions/ConvertToTransfer.php +++ b/app/TransactionRules/Actions/ConvertToTransfer.php @@ -25,12 +25,13 @@ namespace FireflyIII\TransactionRules\Actions; use DB; use FireflyIII\Events\TriggeredAuditLog; +use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Account; use FireflyIII\Models\RuleAction; +use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; -use FireflyIII\User; use Illuminate\Support\Facades\Log; /** @@ -56,18 +57,25 @@ class ConvertToTransfer implements ActionInterface */ public function actOnArray(array $journal): bool { + // make object from array (so the data is fresh). + /** @var TransactionJournal|null $object */ + $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + if (null === $object) { + Log::error(sprintf('Cannot find journal #%d, cannot convert to transfer.', $journal['transaction_journal_id'])); + return false; + } $groupCount = TransactionJournal::where('transaction_group_id', $journal['transaction_group_id'])->count(); if ($groupCount > 1) { Log::error(sprintf('Group #%d has more than one transaction in it, cannot convert to transfer.', $journal['transaction_group_id'])); return false; } - - $type = $journal['transaction_type_type']; - $user = User::find($journal['user_id']); + $type = $object->transactionType->type; + $user = $object->user; + $journalId = (int)$object->id; if (TransactionType::TRANSFER === $type) { Log::error( - sprintf('Journal #%d is already a transfer so cannot be converted (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id) + sprintf('Journal #%d is already a transfer so cannot be converted (rule #%d).', $object->id, $this->action->rule_id) ); return false; @@ -77,14 +85,14 @@ class ConvertToTransfer implements ActionInterface /** @var AccountRepositoryInterface $repository */ $repository = app(AccountRepositoryInterface::class); $repository->setUser($user); - $opposing = null; + $opposing = null; $expectedType = null; if (TransactionType::WITHDRAWAL === $type) { - $expectedType = $this->getSourceType($journal['transaction_journal_id']); + $expectedType = $this->getSourceType($journalId); // Withdrawal? Replace destination with account with same type as source. } if (TransactionType::DEPOSIT === $type) { - $expectedType = $this->getDestinationType($journal['transaction_journal_id']); + $expectedType = $this->getDestinationType($journalId); // Deposit? Replace source with account with same type as destination. } $opposing = $repository->findByName($this->action->action_value, [$expectedType]); @@ -94,7 +102,7 @@ class ConvertToTransfer implements ActionInterface sprintf( 'Journal #%d cannot be converted because no valid %s account with name "%s" exists (rule #%d).', $expectedType, - $journal['transaction_journal_id'], + $journalId, $this->action->action_value, $this->action->rule_id ) @@ -102,20 +110,30 @@ class ConvertToTransfer implements ActionInterface return false; } + if (TransactionType::WITHDRAWAL === $type) { Log::debug('Going to transform a withdrawal to a transfer.'); - $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + try { + $res = $this->convertWithdrawalArray($object, $opposing); + } catch (FireflyException $e) { + Log::debug('Could not convert withdrawal to transfer.'); + Log::error($e->getMessage()); + return false; + } event(new TriggeredAuditLog($this->action->rule, $object, 'update_transaction_type', TransactionType::WITHDRAWAL, TransactionType::TRANSFER)); - - return $this->convertWithdrawalArray($journal, $opposing); + return $res; } if (TransactionType::DEPOSIT === $type) { Log::debug('Going to transform a deposit to a transfer.'); - - $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + try { + $res = $this->convertDepositArray($object, $opposing); + } catch (FireflyException $e) { + Log::debug('Could not convert deposit to transfer.'); + Log::error($e->getMessage()); + return false; + } event(new TriggeredAuditLog($this->action->rule, $object, 'update_transaction_type', TransactionType::DEPOSIT, TransactionType::TRANSFER)); - - return $this->convertDepositArray($journal, $opposing); + return $res; } return false; @@ -126,18 +144,20 @@ class ConvertToTransfer implements ActionInterface * We replace the Expense with another asset. * So this replaces the destination * - * @param array $journal + * @param TransactionJournal $journal * @param Account $opposing * * @return bool + * @throws FireflyException */ - private function convertWithdrawalArray(array $journal, Account $opposing): bool + private function convertWithdrawalArray(TransactionJournal $journal, Account $opposing): bool { - if ($journal['source_account_id'] === $opposing->id) { + $sourceAccount = $this->getSourceAccount($journal); + if ((int)$sourceAccount->id === (int)$opposing->id) { Log::error( vsprintf( 'Journal #%d has already has "%s" as a source asset. ConvertToTransfer failed. (rule #%d).', - [$journal['transaction_journal_id'], $opposing->name, $this->action->rule_id] + [$journal->id, $opposing->name, $this->action->rule_id] ) ); @@ -146,7 +166,7 @@ class ConvertToTransfer implements ActionInterface // update destination transaction: DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', $journal->id) ->where('amount', '>', 0) ->update(['account_id' => $opposing->id]); @@ -154,7 +174,7 @@ class ConvertToTransfer implements ActionInterface $newType = TransactionType::whereType(TransactionType::TRANSFER)->first(); DB::table('transaction_journals') - ->where('id', '=', $journal['transaction_journal_id']) + ->where('id', '=', $journal->id) ->update(['transaction_type_id' => $newType->id, 'bill_id' => null]); Log::debug('Converted withdrawal to transfer.'); @@ -166,18 +186,20 @@ class ConvertToTransfer implements ActionInterface * A deposit is from Revenue to Asset. * We replace the Revenue with another asset. * - * @param array $journal + * @param TransactionJournal $journal * @param Account $opposing * * @return bool + * @throws FireflyException */ - private function convertDepositArray(array $journal, Account $opposing): bool + private function convertDepositArray(TransactionJournal $journal, Account $opposing): bool { - if ($journal['destination_account_id'] === $opposing->id) { + $destAccount = $this->getDestinationAccount($journal); + if ((int)$destAccount->id === (int)$opposing->id) { Log::error( vsprintf( 'Journal #%d has already has "%s" as a destination asset. ConvertToTransfer failed. (rule #%d).', - [$journal['transaction_journal_id'], $opposing->name, $this->action->rule_id] + [$journal->id, $opposing->name, $this->action->rule_id] ) ); @@ -186,7 +208,7 @@ class ConvertToTransfer implements ActionInterface // update source transaction: DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', $journal->id) ->where('amount', '<', 0) ->update(['account_id' => $opposing->id]); @@ -194,7 +216,7 @@ class ConvertToTransfer implements ActionInterface $newType = TransactionType::whereType(TransactionType::TRANSFER)->first(); DB::table('transaction_journals') - ->where('id', '=', $journal['transaction_journal_id']) + ->where('id', '=', $journal->id) ->update(['transaction_type_id' => $newType->id, 'bill_id' => null]); Log::debug('Converted deposit to transfer.'); @@ -202,6 +224,36 @@ class ConvertToTransfer implements ActionInterface return true; } + /** + * @param TransactionJournal $journal + * @return Account + * @throws FireflyException + */ + private function getSourceAccount(TransactionJournal $journal): Account + { + /** @var Transaction|null $sourceTransaction */ + $sourceTransaction = $journal->transactions()->where('amount', '<', 0)->first(); + if (null === $sourceTransaction) { + throw new FireflyException(sprintf('Cannot find source transaction for journal #%d', $journal->id)); + } + return $sourceTransaction->account; + } + + /** + * @param TransactionJournal $journal + * @return Account + * @throws FireflyException + */ + private function getDestinationAccount(TransactionJournal $journal): Account + { + /** @var Transaction|null $destAccount */ + $destAccount = $journal->transactions()->where('amount', '>', 0)->first(); + if (null === $destAccount) { + throw new FireflyException(sprintf('Cannot find destination transaction for journal #%d', $journal->id)); + } + return $destAccount->account; + } + /** * @param int $journalId * @return string diff --git a/app/TransactionRules/Actions/ConvertToWithdrawal.php b/app/TransactionRules/Actions/ConvertToWithdrawal.php index 9b0a79672f..c2a414bc7e 100644 --- a/app/TransactionRules/Actions/ConvertToWithdrawal.php +++ b/app/TransactionRules/Actions/ConvertToWithdrawal.php @@ -27,14 +27,16 @@ use DB; use FireflyIII\Events\TriggeredAuditLog; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Factory\AccountFactory; +use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\RuleAction; +use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\User; -use JsonException; use Illuminate\Support\Facades\Log; +use JsonException; /** * @@ -59,13 +61,20 @@ class ConvertToWithdrawal implements ActionInterface */ public function actOnArray(array $journal): bool { + // make object from array (so the data is fresh). + /** @var TransactionJournal|null $object */ + $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + if (null === $object) { + Log::error(sprintf('Cannot find journal #%d, cannot convert to withdrawal.', $journal['transaction_journal_id'])); + return false; + } $groupCount = TransactionJournal::where('transaction_group_id', $journal['transaction_group_id'])->count(); if ($groupCount > 1) { Log::error(sprintf('Group #%d has more than one transaction in it, cannot convert to withdrawal.', $journal['transaction_group_id'])); return false; } - $type = $journal['transaction_type_type']; + $type = $object->transactionType->type; if (TransactionType::WITHDRAWAL === $type) { Log::error(sprintf('Journal #%d is already a withdrawal (rule #%d).', $journal['transaction_journal_id'], $this->action->rule_id)); @@ -74,31 +83,44 @@ class ConvertToWithdrawal implements ActionInterface if (TransactionType::DEPOSIT === $type) { Log::debug('Going to transform a deposit to a withdrawal.'); - $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + try { + $res = $this->convertDepositArray($object); + } catch (JsonException|FireflyException $e) { + Log::debug('Could not convert transfer to deposit.'); + Log::error($e->getMessage()); + return false; + } event(new TriggeredAuditLog($this->action->rule, $object, 'update_transaction_type', TransactionType::DEPOSIT, TransactionType::WITHDRAWAL)); - return $this->convertDepositArray($journal); + return $res; } if (TransactionType::TRANSFER === $type) { Log::debug('Going to transform a transfer to a withdrawal.'); - $object = TransactionJournal::where('user_id', $journal['user_id'])->find($journal['transaction_journal_id']); + + try { + $res = $this->convertTransferArray($object); + } catch (JsonException|FireflyException $e) { + Log::debug('Could not convert transfer to deposit.'); + Log::error($e->getMessage()); + return false; + } event(new TriggeredAuditLog($this->action->rule, $object, 'update_transaction_type', TransactionType::TRANSFER, TransactionType::WITHDRAWAL)); - return $this->convertTransferArray($journal); + return $res; } return false; } /** - * @param array $journal + * @param TransactionJournal $journal * @return bool * @throws FireflyException * @throws JsonException */ - private function convertDepositArray(array $journal): bool + private function convertDepositArray(TransactionJournal $journal): bool { - $user = User::find($journal['user_id']); + $user = $journal->user; /** @var AccountFactory $factory */ $factory = app(AccountFactory::class); $factory->setUser($user); @@ -106,35 +128,37 @@ class ConvertToWithdrawal implements ActionInterface $repository = app(AccountRepositoryInterface::class); $repository->setUser($user); + $sourceAccount = $this->getSourceAccount($journal); + $destAccount = $this->getDestinationAccount($journal); + // get the action value, or use the original source name in case the action value is empty: // this becomes a new or existing (expense) account, which is the destination of the new withdrawal. - $opposingName = '' === $this->action->action_value ? $journal['source_account_name'] : $this->action->action_value; + $opposingName = '' === $this->action->action_value ? $sourceAccount->name : $this->action->action_value; // we check all possible source account types if one exists: - $validTypes = config('firefly.expected_source_types.destination.Withdrawal'); - $opposingAccount = $repository->findByName($opposingName, $validTypes); + $validTypes = config('firefly.expected_source_types.destination.Withdrawal'); + $opposingAccount = $repository->findByName($opposingName, $validTypes); if (null === $opposingAccount) { $opposingAccount = $factory->findOrCreate($opposingName, AccountType::EXPENSE); } - $destinationId = $journal['destination_account_id']; Log::debug(sprintf('ConvertToWithdrawal. Action value is "%s", expense name is "%s"', $this->action->action_value, $opposingName)); // update source transaction(s) to be the original destination account DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', $journal->id) ->where('amount', '<', 0) - ->update(['account_id' => $destinationId]); + ->update(['account_id' => $destAccount->id]); // update destination transaction(s) to be new expense account. DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', $journal->id) ->where('amount', '>', 0) ->update(['account_id' => $opposingAccount->id]); // change transaction type of journal: $newType = TransactionType::whereType(TransactionType::WITHDRAWAL)->first(); DB::table('transaction_journals') - ->where('id', '=', $journal['transaction_journal_id']) + ->where('id', '=', $journal->id) ->update(['transaction_type_id' => $newType->id]); Log::debug('Converted deposit to withdrawal.'); @@ -146,16 +170,16 @@ class ConvertToWithdrawal implements ActionInterface * Input is a transfer from A to B. * Output is a withdrawal from A to C. * - * @param array $journal + * @param TransactionJournal $journal * * @return bool * @throws FireflyException * @throws JsonException */ - private function convertTransferArray(array $journal): bool + private function convertTransferArray(TransactionJournal $journal): bool { // find or create expense account. - $user = User::find($journal['user_id']); + $user = $journal->user; /** @var AccountFactory $factory */ $factory = app(AccountFactory::class); $factory->setUser($user); @@ -163,12 +187,14 @@ class ConvertToWithdrawal implements ActionInterface $repository = app(AccountRepositoryInterface::class); $repository->setUser($user); + $destAccount = $this->getDestinationAccount($journal); + // get the action value, or use the original source name in case the action value is empty: // this becomes a new or existing (expense) account, which is the destination of the new withdrawal. - $opposingName = '' === $this->action->action_value ? $journal['destination_account_name'] : $this->action->action_value; + $opposingName = '' === $this->action->action_value ? $destAccount->name : $this->action->action_value; // we check all possible source account types if one exists: - $validTypes = config('firefly.expected_source_types.destination.Withdrawal'); - $opposingAccount = $repository->findByName($opposingName, $validTypes); + $validTypes = config('firefly.expected_source_types.destination.Withdrawal'); + $opposingAccount = $repository->findByName($opposingName, $validTypes); if (null === $opposingAccount) { $opposingAccount = $factory->findOrCreate($opposingName, AccountType::EXPENSE); } @@ -177,18 +203,48 @@ class ConvertToWithdrawal implements ActionInterface // update destination transaction(s) to be new expense account. DB::table('transactions') - ->where('transaction_journal_id', '=', $journal['transaction_journal_id']) + ->where('transaction_journal_id', '=', $journal->id) ->where('amount', '>', 0) ->update(['account_id' => $opposingAccount->id]); // change transaction type of journal: $newType = TransactionType::whereType(TransactionType::WITHDRAWAL)->first(); DB::table('transaction_journals') - ->where('id', '=', $journal['transaction_journal_id']) + ->where('id', '=', $journal->id) ->update(['transaction_type_id' => $newType->id]); Log::debug('Converted transfer to withdrawal.'); return true; } + + /** + * @param TransactionJournal $journal + * @return Account + * @throws FireflyException + */ + private function getSourceAccount(TransactionJournal $journal): Account + { + /** @var Transaction|null $sourceTransaction */ + $sourceTransaction = $journal->transactions()->where('amount', '<', 0)->first(); + if (null === $sourceTransaction) { + throw new FireflyException(sprintf('Cannot find source transaction for journal #%d', $journal->id)); + } + return $sourceTransaction->account; + } + + /** + * @param TransactionJournal $journal + * @return Account + * @throws FireflyException + */ + private function getDestinationAccount(TransactionJournal $journal): Account + { + /** @var Transaction|null $destAccount */ + $destAccount = $journal->transactions()->where('amount', '>', 0)->first(); + if (null === $destAccount) { + throw new FireflyException(sprintf('Cannot find destination transaction for journal #%d', $journal->id)); + } + return $destAccount->account; + } }