From 38c1d332e2754477a1c846d8f476499a234bcc2a Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 22 Feb 2018 20:13:00 +0100 Subject: [PATCH] Removed a lot of old spaghetti code. Now have to rewrite it --- app/Models/PiggyBankEvent.php | 1 + .../Journal/CreateJournalsTrait.php | 240 --------------- .../Journal/JournalRepository.php | 6 +- app/Repositories/Journal/JournalTasker.php | 3 +- .../Journal/SupportJournalsTrait.php | 287 ------------------ .../Journal/UpdateJournalsTrait.php | 122 -------- 6 files changed, 5 insertions(+), 654 deletions(-) delete mode 100644 app/Repositories/Journal/CreateJournalsTrait.php delete mode 100644 app/Repositories/Journal/SupportJournalsTrait.php delete mode 100644 app/Repositories/Journal/UpdateJournalsTrait.php diff --git a/app/Models/PiggyBankEvent.php b/app/Models/PiggyBankEvent.php index 4bd7753aa9..d937815739 100644 --- a/app/Models/PiggyBankEvent.php +++ b/app/Models/PiggyBankEvent.php @@ -26,6 +26,7 @@ use Illuminate\Database\Eloquent\Model; /** * Class PiggyBankEvent. + * @property $piggyBank */ class PiggyBankEvent extends Model { diff --git a/app/Repositories/Journal/CreateJournalsTrait.php b/app/Repositories/Journal/CreateJournalsTrait.php deleted file mode 100644 index 897d145a1c..0000000000 --- a/app/Repositories/Journal/CreateJournalsTrait.php +++ /dev/null @@ -1,240 +0,0 @@ -. - */ -declare(strict_types=1); - -namespace FireflyIII\Repositories\Journal; - -use FireflyIII\Models\Budget; -use FireflyIII\Models\Category; -use FireflyIII\Models\Note; -use FireflyIII\Models\Tag; -use FireflyIII\Models\Transaction; -use FireflyIII\Models\TransactionJournal; -use FireflyIII\Models\TransactionType; -use FireflyIII\Repositories\Tag\TagRepositoryInterface; -use FireflyIII\User; -use Illuminate\Support\Collection; -use Log; - -/** - * @property User $user - * - * Trait CreateJournalsTrait - */ -trait CreateJournalsTrait -{ - /** - * Store accounts found in parent class. - * - * @param User $user - * @param TransactionType $type - * @param array $data - * - * @return array - */ - abstract public function storeAccounts(User $user, TransactionType $type, array $data): array; - - /** - * * Remember: a balancingAct takes at most one expense and one transfer. - * an advancePayment takes at most one expense, infinite deposits and NO transfers. - * - * @param TransactionJournal $journal - * @param array $array - * - * @return bool - */ - protected function saveTags(TransactionJournal $journal, array $array): bool - { - /** @var TagRepositoryInterface $tagRepository */ - $tagRepository = app(TagRepositoryInterface::class); - - foreach ($array as $name) { - if (strlen(trim($name)) > 0) { - $tag = Tag::firstOrCreateEncrypted(['tag' => $name, 'user_id' => $journal->user_id]); - if (null !== $tag) { - Log::debug(sprintf('Will try to connect tag #%d to journal #%d.', $tag->id, $journal->id)); - $tagRepository->connect($journal, $tag); - } - } - } - - return true; - } - - /** - * Store a budget if found. - * - * @param Transaction $transaction - * @param int $budgetId - */ - protected function storeBudgetWithTransaction(Transaction $transaction, int $budgetId) - { - if (intval($budgetId) > 0 && TransactionType::TRANSFER !== $transaction->transactionJournal->transactionType->type) { - /** @var \FireflyIII\Models\Budget $budget */ - $budget = Budget::find($budgetId); - $transaction->budgets()->save($budget); - } - } - - /** - * Store category if found. - * - * @param Transaction $transaction - * @param string $category - */ - protected function storeCategoryWithTransaction(Transaction $transaction, string $category) - { - if (strlen($category) > 0) { - $category = Category::firstOrCreateEncrypted(['name' => $category, 'user_id' => $transaction->transactionJournal->user_id]); - $transaction->categories()->save($category); - } - } - - /** - * The reference to storeAccounts() in this function is an indication of spagetti code but alas, - * I leave it as it is. - * - * @param TransactionJournal $journal - * @param array $transaction - * @param int $identifier - * - * @return Collection - */ - protected function storeSplitTransaction(TransactionJournal $journal, array $transaction, int $identifier): Collection - { - // store source and destination accounts (depends on type) - $accounts = $this->storeAccounts($this->user, $journal->transactionType, $transaction); - - // store transaction one way: - $amount = bcmul(strval($transaction['amount']), '-1'); - $foreignAmount = null === $transaction['foreign_amount'] ? null : bcmul(strval($transaction['foreign_amount']), '-1'); - $one = $this->storeTransaction( - [ - 'journal' => $journal, - 'account' => $accounts['source'], - 'amount' => $amount, - 'transaction_currency_id' => $transaction['transaction_currency_id'], - 'foreign_amount' => $foreignAmount, - 'foreign_currency_id' => $transaction['foreign_currency_id'], - 'description' => $transaction['description'], - 'category' => null, - 'budget' => null, - 'identifier' => $identifier, - ] - ); - $this->storeCategoryWithTransaction($one, $transaction['category']); - $this->storeBudgetWithTransaction($one, $transaction['budget_id']); - - // and the other way: - $amount = strval($transaction['amount']); - $foreignAmount = null === $transaction['foreign_amount'] ? null : strval($transaction['foreign_amount']); - $two = $this->storeTransaction( - [ - 'journal' => $journal, - 'account' => $accounts['destination'], - 'amount' => $amount, - 'transaction_currency_id' => $transaction['transaction_currency_id'], - 'foreign_amount' => $foreignAmount, - 'foreign_currency_id' => $transaction['foreign_currency_id'], - 'description' => $transaction['description'], - 'category' => null, - 'budget' => null, - 'identifier' => $identifier, - ] - ); - $this->storeCategoryWithTransaction($two, $transaction['category']); - $this->storeBudgetWithTransaction($two, $transaction['budget_id']); - - return new Collection([$one, $two]); - } - - /** - * Store a transaction. - * - * @param array $data - * - * @return Transaction - */ - protected function storeTransaction(array $data): Transaction - { - $fields = [ - 'transaction_journal_id' => $data['journal']->id, - 'account_id' => $data['account']->id, - 'amount' => $data['amount'], - 'foreign_amount' => $data['foreign_amount'], - 'transaction_currency_id' => $data['transaction_currency_id'], - 'foreign_currency_id' => $data['foreign_currency_id'], - 'description' => $data['description'], - 'identifier' => $data['identifier'], - ]; - - if (null === $data['foreign_currency_id']) { - unset($fields['foreign_currency_id']); - } - if (null === $data['foreign_amount']) { - unset($fields['foreign_amount']); - } - - /** @var Transaction $transaction */ - $transaction = Transaction::create($fields); - - Log::debug(sprintf('Transaction stored with ID: %s', $transaction->id)); - - if (null !== $data['category']) { - $transaction->categories()->save($data['category']); - } - - if (null !== $data['budget']) { - $transaction->categories()->save($data['budget']); - } - - return $transaction; - } - - /** - * Update note for journal. - * - * @param TransactionJournal $journal - * @param string $note - * - * @return bool - */ - protected function updateNote(TransactionJournal $journal, string $note): bool - { - if (0 === strlen($note)) { - $dbNote = $journal->notes()->first(); - if (null !== $dbNote) { - $dbNote->delete(); - } - - return true; - } - $dbNote = $journal->notes()->first(); - if (null === $dbNote) { - $dbNote = new Note(); - $dbNote->noteable()->associate($journal); - } - $dbNote->text = trim($note); - $dbNote->save(); - - return true; - } -} diff --git a/app/Repositories/Journal/JournalRepository.php b/app/Repositories/Journal/JournalRepository.php index a8c0bbeac0..3f4a481357 100644 --- a/app/Repositories/Journal/JournalRepository.php +++ b/app/Repositories/Journal/JournalRepository.php @@ -43,12 +43,8 @@ use Preferences; */ class JournalRepository implements JournalRepositoryInterface { - use CreateJournalsTrait, UpdateJournalsTrait, SupportJournalsTrait; - /** @var User */ private $user; - /** @var array */ - private $validMetaFields = ['interest_date', 'book_date', 'process_date', 'due_date', 'payment_date', 'invoice_date', 'internal_reference']; /** * @param TransactionJournal $journal @@ -123,6 +119,7 @@ class JournalRepository implements JournalRepositoryInterface */ public function find(int $journalId): TransactionJournal { + /** @var TransactionJournal $journal */ $journal = $this->user->transactionJournals()->where('id', $journalId)->first(); if (null === $journal) { return new TransactionJournal; @@ -170,6 +167,7 @@ class JournalRepository implements JournalRepositoryInterface */ public function first(): TransactionJournal { + /** @var TransactionJournal $entry */ $entry = $this->user->transactionJournals()->orderBy('date', 'ASC')->first(['transaction_journals.*']); if (null === $entry) { diff --git a/app/Repositories/Journal/JournalTasker.php b/app/Repositories/Journal/JournalTasker.php index b51181fdf4..cdf5726868 100644 --- a/app/Repositories/Journal/JournalTasker.php +++ b/app/Repositories/Journal/JournalTasker.php @@ -64,6 +64,7 @@ class JournalTasker implements JournalTaskerInterface * that shows a transaction (transaction/show/xx). * * @param TransactionJournal $journal + * @deprecated * * @return array */ @@ -186,7 +187,7 @@ class JournalTasker implements JournalTaskerInterface * the order of transactions within the journal. So the query is pretty complex:. * * @param int $transactionId - * + * @deprecated * @return string */ private function getBalance(int $transactionId): string diff --git a/app/Repositories/Journal/SupportJournalsTrait.php b/app/Repositories/Journal/SupportJournalsTrait.php deleted file mode 100644 index 0ea65efd29..0000000000 --- a/app/Repositories/Journal/SupportJournalsTrait.php +++ /dev/null @@ -1,287 +0,0 @@ -. - */ -declare(strict_types=1); - -namespace FireflyIII\Repositories\Journal; - -use FireflyIII\Exceptions\FireflyException; -use FireflyIII\Models\Account; -use FireflyIII\Models\AccountType; -use FireflyIII\Models\Budget; -use FireflyIII\Models\Category; -use FireflyIII\Models\TransactionJournal; -use FireflyIII\Models\TransactionType; -use FireflyIII\User; -use Log; - -/** - * Trait SupportJournalsTrait. - */ -trait SupportJournalsTrait -{ - /** - * @param User $user - * @param TransactionType $type - * @param array $data - * - * @return array - * - * @throws FireflyException - */ - protected function storeAccounts(User $user, TransactionType $type, array $data): array - { - $accounts = [ - 'source' => null, - 'destination' => null, - ]; - - Log::debug(sprintf('Going to store accounts for type %s', $type->type)); - switch ($type->type) { - case TransactionType::WITHDRAWAL: - $accounts = $this->storeWithdrawalAccounts($user, $data); - break; - - case TransactionType::DEPOSIT: - $accounts = $this->storeDepositAccounts($user, $data); - - break; - case TransactionType::TRANSFER: - $accounts['source'] = Account::where('user_id', $user->id)->where('id', $data['source_account_id'])->first(); - $accounts['destination'] = Account::where('user_id', $user->id)->where('id', $data['destination_account_id'])->first(); - break; - case TransactionType::RECONCILIATION: - $accounts['source'] = $data['source']; - $accounts['destination'] = $data['destination']; - unset($data['source'], $data['destination']); - break; - default: - throw new FireflyException(sprintf('Did not recognise transaction type "%s".', $type->type)); - } - - if (null === $accounts['source']) { - Log::error('"source"-account is null, so we cannot continue!', ['data' => $data]); - throw new FireflyException('"source"-account is null, so we cannot continue!'); - } - - if (null === $accounts['destination']) { - Log::error('"destination"-account is null, so we cannot continue!', ['data' => $data]); - throw new FireflyException('"destination"-account is null, so we cannot continue!'); - } - - return $accounts; - } - - /** - * @param TransactionJournal $journal - * @param int $budgetId - */ - protected function storeBudgetWithJournal(TransactionJournal $journal, int $budgetId) - { - if (intval($budgetId) > 0 && TransactionType::WITHDRAWAL === $journal->transactionType->type) { - /** @var \FireflyIII\Models\Budget $budget */ - $budget = Budget::find($budgetId); - $journal->budgets()->save($budget); - } - $journal->touch(); - } - - /** - * @param TransactionJournal $journal - * @param string $category - */ - protected function storeCategoryWithJournal(TransactionJournal $journal, string $category) - { - if (strlen($category) > 0) { - $category = Category::firstOrCreateEncrypted(['name' => $category, 'user_id' => $journal->user_id]); - $journal->categories()->save($category); - } - $journal->touch(); - } - - /** - * @param User $user - * @param array $data - * - * @return array - * - * @throws FireflyException - * @throws FireflyException - */ - protected function storeDepositAccounts(User $user, array $data): array - { - Log::debug('Now in storeDepositAccounts().'); - $destinationAccount = Account::where('user_id', $user->id)->where('id', $data['destination_account_id'])->first(['accounts.*']); - - Log::debug(sprintf('Destination account is #%d ("%s")', $destinationAccount->id, $destinationAccount->name)); - - if (strlen($data['source_account_name']) > 0) { - $sourceType = AccountType::where('type', 'Revenue account')->first(); - $sourceAccount = Account::firstOrCreateEncrypted( - ['user_id' => $user->id, 'account_type_id' => $sourceType->id, 'name' => $data['source_account_name']] - ); - // always make account active - $sourceAccount->active = true; - $sourceAccount->save(); - - Log::debug(sprintf('source account name is "%s", account is %d', $data['source_account_name'], $sourceAccount->id)); - - return [ - 'source' => $sourceAccount, - 'destination' => $destinationAccount, - ]; - } - - Log::debug('source_account_name is empty, so default to cash account!'); - - $sourceType = AccountType::where('type', AccountType::CASH)->first(); - $sourceAccount = Account::firstOrCreateEncrypted( - ['user_id' => $user->id, 'account_type_id' => $sourceType->id, 'name' => 'Cash account'] - ); - // always make account active - $sourceAccount->active = true; - $sourceAccount->save(); - - return [ - 'source' => $sourceAccount, - 'destination' => $destinationAccount, - ]; - } - - /** - * @param User $user - * @param array $data - * - * @return array - * - * @throws FireflyException - * @throws FireflyException - */ - protected function storeWithdrawalAccounts(User $user, array $data): array - { - Log::debug('Now in storeWithdrawalAccounts().'); - $sourceAccount = Account::where('user_id', $user->id)->where('id', $data['source_account_id'])->first(['accounts.*']); - - Log::debug(sprintf('Source account is #%d ("%s")', $sourceAccount->id, $sourceAccount->name)); - - if (strlen($data['destination_account_name']) > 0) { - $destinationType = AccountType::where('type', AccountType::EXPENSE)->first(); - $destinationAccount = Account::firstOrCreateEncrypted( - [ - 'user_id' => $user->id, - 'account_type_id' => $destinationType->id, - 'name' => $data['destination_account_name'], - ] - ); - - // always make account active - $destinationAccount->active = true; - $destinationAccount->save(); - - Log::debug(sprintf('destination account name is "%s", account is %d', $data['destination_account_name'], $destinationAccount->id)); - - return [ - 'source' => $sourceAccount, - 'destination' => $destinationAccount, - ]; - } - Log::debug('destination_account_name is empty, so default to cash account!'); - $destinationType = AccountType::where('type', AccountType::CASH)->first(); - $destinationAccount = Account::firstOrCreateEncrypted( - ['user_id' => $user->id, 'account_type_id' => $destinationType->id, 'name' => 'Cash account'] - ); - // always make account active - $destinationAccount->active = true; - $destinationAccount->save(); - - return [ - 'source' => $sourceAccount, - 'destination' => $destinationAccount, - ]; - } - - /** - * This method checks the data array and the given accounts to verify that the native amount, currency - * and possible the foreign currency and amount are properly saved. - * - * @param array $data - * @param array $accounts - * - * @return array - * - * @throws FireflyException - */ - protected function verifyNativeAmount(array $data, array $accounts): array - { - /** @var TransactionType $transactionType */ - $transactionType = TransactionType::where('type', ucfirst($data['type']))->first(); - $submittedCurrencyId = $data['currency_id']; - $data['foreign_amount'] = null; - $data['foreign_currency_id'] = null; - - // which account to check for what the native currency is? - $check = 'source'; - if (TransactionType::DEPOSIT === $transactionType->type) { - $check = 'destination'; - } - switch ($transactionType->type) { - case TransactionType::RECONCILIATION: - // do nothing. - break; - case TransactionType::DEPOSIT: - case TransactionType::WITHDRAWAL: - // continue: - $nativeCurrencyId = intval($accounts[$check]->getMeta('currency_id')); - if ($nativeCurrencyId === 0) { - // fall back to given ID (not everybody upgrades nicely). - $nativeCurrencyId = $submittedCurrencyId; - } - - // does not match? Then user has submitted amount in a foreign currency: - if ($nativeCurrencyId !== $submittedCurrencyId) { - // store amount and submitted currency in "foreign currency" fields: - $data['foreign_amount'] = $data['amount']; - $data['foreign_currency_id'] = $submittedCurrencyId; - - // overrule the amount and currency ID fields to be the original again: - $data['amount'] = strval($data['native_amount']); - $data['currency_id'] = $nativeCurrencyId; - } - break; - case TransactionType::TRANSFER: - $sourceCurrencyId = intval($accounts['source']->getMeta('currency_id')); - $destinationCurrencyId = intval($accounts['destination']->getMeta('currency_id')); - $data['amount'] = strval($data['source_amount']); - $data['currency_id'] = intval($accounts['source']->getMeta('currency_id')); - - if ($sourceCurrencyId !== $destinationCurrencyId) { - // accounts have different id's, save this info: - $data['foreign_amount'] = strval($data['destination_amount']); - $data['foreign_currency_id'] = $destinationCurrencyId; - } - - break; - default: - throw new FireflyException(sprintf('Cannot handle %s in verifyNativeAmount()', $transactionType->type)); - } - - return $data; - } -} diff --git a/app/Repositories/Journal/UpdateJournalsTrait.php b/app/Repositories/Journal/UpdateJournalsTrait.php deleted file mode 100644 index cd2c3991af..0000000000 --- a/app/Repositories/Journal/UpdateJournalsTrait.php +++ /dev/null @@ -1,122 +0,0 @@ -. - */ -declare(strict_types=1); - -namespace FireflyIII\Repositories\Journal; - -use FireflyIII\Exceptions\FireflyException; -use FireflyIII\Models\Account; -use FireflyIII\Models\Transaction; -use FireflyIII\Models\TransactionJournal; -use FireflyIII\Models\TransactionType; - -/** - * Trait UpdateJournalsTrait. - */ -trait UpdateJournalsTrait -{ - - /** - * When the user edits a split journal, each line is missing crucial data:. - * - * - Withdrawal lines are missing the source account ID - * - Deposit lines are missing the destination account ID - * - Transfers are missing both. - * - * We need to append the array. - * - * @param array $transaction - * @param array $data - * - * @return array - */ - protected function appendTransactionData(array $transaction, array $data): array - { - switch ($data['what']) { - case strtolower(TransactionType::TRANSFER): - case strtolower(TransactionType::WITHDRAWAL): - $transaction['source_account_id'] = intval($data['journal_source_account_id']); - break; - } - - switch ($data['what']) { - case strtolower(TransactionType::TRANSFER): - case strtolower(TransactionType::DEPOSIT): - $transaction['destination_account_id'] = intval($data['journal_destination_account_id']); - break; - } - - return $transaction; - } - - /** - * Update destination transaction. - * - * @param TransactionJournal $journal - * @param Account $account - * @param array $data - * - * @throws FireflyException - */ - protected function updateDestinationTransaction(TransactionJournal $journal, Account $account, array $data) - { - $set = $journal->transactions()->where('amount', '>', 0)->get(); - if (1 !== $set->count()) { - throw new FireflyException(sprintf('Journal #%d has %d transactions with an amount more than zero.', $journal->id, $set->count())); - } - /** @var Transaction $transaction */ - $transaction = $set->first(); - $transaction->amount = app('steam')->positive($data['amount']); - $transaction->transaction_currency_id = $data['currency_id']; - $transaction->foreign_amount = null === $data['foreign_amount'] ? null : app('steam')->positive($data['foreign_amount']); - $transaction->foreign_currency_id = $data['foreign_currency_id']; - $transaction->account_id = $account->id; - $transaction->save(); - } - - /** - * Update source transaction. - * - * @param TransactionJournal $journal - * @param Account $account - * @param array $data - * - * @throws FireflyException - */ - protected function updateSourceTransaction(TransactionJournal $journal, Account $account, array $data) - { - // should be one: - $set = $journal->transactions()->where('amount', '<', 0)->get(); - if (1 !== $set->count()) { - throw new FireflyException(sprintf('Journal #%d has %d transactions with an amount more than zero.', $journal->id, $set->count())); - } - /** @var Transaction $transaction */ - $transaction = $set->first(); - $transaction->amount = bcmul(app('steam')->positive($data['amount']), '-1'); - $transaction->transaction_currency_id = $data['currency_id']; - $transaction->foreign_amount = null === $data['foreign_amount'] ? null : bcmul(app('steam')->positive($data['foreign_amount']), '-1'); - $transaction->foreign_currency_id = $data['foreign_currency_id']; - $transaction->account_id = $account->id; - $transaction->save(); - } - - -}