From 94b8bb8f663d1fca705f9f7acd4cc5cfae4611d6 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 10 Aug 2019 08:42:46 +0200 Subject: [PATCH] Cleanup journal repository --- .../Journal/JournalRepository.php | 257 ------------------ .../Journal/JournalRepositoryInterface.php | 127 ++------- .../Triggers/ToAccountContains.php | 1 + 3 files changed, 31 insertions(+), 354 deletions(-) diff --git a/app/Repositories/Journal/JournalRepository.php b/app/Repositories/Journal/JournalRepository.php index df607ffa80..6cbb046064 100644 --- a/app/Repositories/Journal/JournalRepository.php +++ b/app/Repositories/Journal/JournalRepository.php @@ -86,73 +86,6 @@ class JournalRepository implements JournalRepositoryInterface return $query->get(); } - /** @noinspection MoreThanThreeArgumentsInspection */ - - /** - * @param TransactionJournal $journal - * @param TransactionType $type - * @param Account $source - * @param Account $destination - * - * @return MessageBag - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) - */ - public function convert(TransactionJournal $journal, TransactionType $type, Account $source, Account $destination): MessageBag - { - if ($source->id === $destination->id || null === $source->id || null === $destination->id) { - // default message bag that shows errors for everything. - $messages = new MessageBag; - $messages->add('source_account_revenue', (string)trans('firefly.invalid_convert_selection')); - $messages->add('destination_account_asset', (string)trans('firefly.invalid_convert_selection')); - $messages->add('destination_account_expense', (string)trans('firefly.invalid_convert_selection')); - $messages->add('source_account_asset', (string)trans('firefly.invalid_convert_selection')); - - return $messages; - } - - $srcTransaction = $journal->transactions()->where('amount', '<', 0)->first(); - $dstTransaction = $journal->transactions()->where('amount', '>', 0)->first(); - if (null === $srcTransaction || null === $dstTransaction) { - // default message bag that shows errors for everything. - - $messages = new MessageBag; - $messages->add('source_account_revenue', (string)trans('firefly.source_or_dest_invalid')); - $messages->add('destination_account_asset', (string)trans('firefly.source_or_dest_invalid')); - $messages->add('destination_account_expense', (string)trans('firefly.source_or_dest_invalid')); - $messages->add('source_account_asset', (string)trans('firefly.source_or_dest_invalid')); - - return $messages; - } - // update transactions, and update journal: - - $srcTransaction->account_id = $source->id; - $dstTransaction->account_id = $destination->id; - $journal->transaction_type_id = $type->id; - $dstTransaction->save(); - $srcTransaction->save(); - $journal->save(); - - // if journal is a transfer now, remove budget: - if (TransactionType::TRANSFER === $type->type) { - - $journal->budgets()->detach(); - // also from transactions: - foreach ($journal->transactions as $transaction) { - $transaction->budgets()->detach(); - } - } - // if journal is not a withdrawal, remove the bill ID. - if (TransactionType::WITHDRAWAL !== $type->type) { - $journal->bill_id = null; - $journal->save(); - } - - app('preferences')->mark(); - - return new MessageBag; - } - /** * @param TransactionGroup $transactionGroup * @@ -245,23 +178,6 @@ class JournalRepository implements JournalRepositoryInterface return $result; } - /** - * @param TransactionJournal $journal - * - * @return Transaction|null - */ - public function getAssetTransaction(TransactionJournal $journal): ?Transaction - { - /** @var Transaction $transaction */ - foreach ($journal->transactions as $transaction) { - if (AccountType::ASSET === $transaction->account->accountType->type) { - return $transaction; - } - } - - return null; - } - /** * Return all attachments for journal. * @@ -287,18 +203,6 @@ class JournalRepository implements JournalRepositoryInterface throw new NotImplementedException; } - /** - * Returns the first positive transaction for the journal. Useful when editing journals. - * - * @param TransactionJournal $journal - * - * @return Transaction - */ - public function getFirstPosTransaction(TransactionJournal $journal): Transaction - { - return $journal->transactions()->where('amount', '>', 0)->first(); - } - /** * Return the ID of the budget linked to the journal (if any) or the transactions (if any). * @@ -343,65 +247,6 @@ class JournalRepository implements JournalRepositoryInterface return 0; } - /** - * Return the name of the category linked to the journal (if any) or to the transactions (if any). - * - * @param TransactionJournal $journal - * - * @return string - */ - public function getJournalCategoryName(TransactionJournal $journal): string - { - $category = $journal->categories()->first(); - if (null !== $category) { - return $category->name; - } - /** @noinspection NullPointerExceptionInspection */ - $category = $journal->transactions()->first()->categories()->first(); - if (null !== $category) { - return $category->name; - } - - return ''; - } - - /** - * Return requested date as string. When it's a NULL return the date of journal, - * otherwise look for meta field and return that one. - * - * @param TransactionJournal $journal - * @param null|string $field - * - * @return string - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) - */ - public function getJournalDate(TransactionJournal $journal, ?string $field): string - { - if (null === $field) { - return $journal->date->format('Y-m-d'); - } - /** @noinspection NotOptimalIfConditionsInspection */ - if (null !== $journal->$field && $journal->$field instanceof Carbon) { - // make field NULL - $carbon = clone $journal->$field; - $journal->$field = null; - $journal->save(); - - // create meta entry - $this->setMetaDate($journal, $field, $carbon); - - // return that one instead. - return $carbon->format('Y-m-d'); - } - $metaField = $this->getMetaDate($journal, $field); - if (null !== $metaField) { - return $metaField->format('Y-m-d'); - } - - return ''; - } - /** * Return Carbon value of a meta field (or NULL). * @@ -538,24 +383,6 @@ class JournalRepository implements JournalRepositoryInterface return ''; } - /** - * Return string value of a meta date (or NULL). - * - * @param TransactionJournal $journal - * @param string $field - * - * @return null|string - */ - public function getMetaDateString(TransactionJournal $journal, string $field): ?string - { - $date = $this->getMetaDate($journal, $field); - if (null === $date) { - return null; - } - - return $date->format('Y-m-d'); - } - /** * Return value of a meta field (or NULL) as a string. * @@ -676,36 +503,6 @@ class JournalRepository implements JournalRepositoryInterface return $journal->tags()->get()->pluck('tag')->toArray(); } - /** - * Return the transaction type of the journal. - * - * @param TransactionJournal $journal - * - * @return string - */ - public function getTransactionType(TransactionJournal $journal): string - { - return $journal->transactionType->type; - } - - /** - * Will tell you if journal is reconciled or not. - * - * @param TransactionJournal $journal - * - * @return bool - */ - public function isJournalReconciled(TransactionJournal $journal): bool - { - foreach ($journal->transactions as $transaction) { - if ($transaction->reconciled) { - return true; - } - } - - return false; - } - /** * @param int $transactionId */ @@ -718,45 +515,6 @@ class JournalRepository implements JournalRepositoryInterface } } - /** - * @param Transaction $transaction - * - * @return bool - */ - public function reconcile(Transaction $transaction): bool - { - Log::debug(sprintf('Going to reconcile transaction #%d', $transaction->id)); - $opposing = $this->findOpposingTransaction($transaction); - - if (null === $opposing) { - Log::debug('Opposing transaction is NULL. Cannot reconcile.'); - - return false; - } - Log::debug(sprintf('Opposing transaction ID is #%d', $opposing->id)); - - $transaction->reconciled = true; - $opposing->reconciled = true; - $transaction->save(); - $opposing->save(); - - return true; - } - - /** - * @param TransactionJournal $journal - * @param int $order - * - * @return bool - */ - public function setOrder(TransactionJournal $journal, int $order): bool - { - $journal->order = $order; - $journal->save(); - - return true; - } - /** * @param User $user */ @@ -853,21 +611,6 @@ class JournalRepository implements JournalRepositoryInterface ->get(['transaction_journals.*']); } - /** - * Get all transaction journals with a specific type, for the logged in user. - * - * @param array $types - * @return Collection - */ - public function getJournals(array $types): Collection - { - return $this->user->transactionJournals() - ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') - ->whereIn('transaction_types.type', $types) - ->with(['user', 'transactionType', 'transactionCurrency', 'transactions', 'transactions.account']) - ->get(['transaction_journals.*']); - } - /** * Return Carbon value of a meta field (or NULL). * diff --git a/app/Repositories/Journal/JournalRepositoryInterface.php b/app/Repositories/Journal/JournalRepositoryInterface.php index 8586eec220..64db01974f 100644 --- a/app/Repositories/Journal/JournalRepositoryInterface.php +++ b/app/Repositories/Journal/JournalRepositoryInterface.php @@ -42,6 +42,8 @@ interface JournalRepositoryInterface { /** + * TODO maybe create JSON repository? + * * Search in journal descriptions. * * @param string $search @@ -50,6 +52,8 @@ interface JournalRepositoryInterface public function searchJournalDescriptions(string $search): Collection; /** + * TODO maybe create command line repository? + * * Get all transaction journals with a specific type, regardless of user. * * @param array $types @@ -57,24 +61,6 @@ interface JournalRepositoryInterface */ public function getAllJournals(array $types): Collection; - /** - * Get all transaction journals with a specific type, for the logged in user. - * - * @param array $types - * @return Collection - */ - public function getJournals(array $types): Collection; - - /** - * @param TransactionJournal $journal - * @param TransactionType $type - * @param Account $source - * @param Account $destination - * - * @return MessageBag - */ - public function convert(TransactionJournal $journal, TransactionType $type, Account $source, Account $destination): MessageBag; - /** * Deletes a transaction group. * @@ -90,9 +76,9 @@ interface JournalRepositoryInterface public function destroyJournal(TransactionJournal $journal): void; - /** @noinspection MoreThanThreeArgumentsInspection */ - /** + * TODO move to import repository. + * * Find a journal by its hash. * * @param string $hash @@ -102,6 +88,7 @@ interface JournalRepositoryInterface public function findByHash(string $hash): ?TransactionJournalMeta; /** + * TODO Refactor to "find". * Find a specific journal. * * @param int $journalId @@ -111,6 +98,8 @@ interface JournalRepositoryInterface public function findNull(int $journalId): ?TransactionJournal; /** + * TODO maybe create API repository? + * * @param int $transactionid * * @return Transaction|null @@ -125,13 +114,8 @@ interface JournalRepositoryInterface public function firstNull(): ?TransactionJournal; /** - * @param TransactionJournal $journal + * TODO maybe create API repository? * - * @return Transaction|null - */ - public function getAssetTransaction(TransactionJournal $journal): ?Transaction; - - /** * Return all attachments for journal. * * @param TransactionJournal $journal @@ -141,15 +125,8 @@ interface JournalRepositoryInterface public function getAttachments(TransactionJournal $journal): Collection; /** - * Returns the first positive transaction for the journal. Useful when editing journals. + * TODO console repository? * - * @param TransactionJournal $journal - * - * @return Transaction - */ - public function getFirstPosTransaction(TransactionJournal $journal): Transaction; - - /** * Return the ID of the budget linked to the journal (if any) or the transactions (if any). * * @param TransactionJournal $journal @@ -159,6 +136,8 @@ interface JournalRepositoryInterface public function getJournalBudgetId(TransactionJournal $journal): int; /** + * TODO console repository? + * * Return the ID of the category linked to the journal (if any) or to the transactions (if any). * * @param TransactionJournal $journal @@ -168,39 +147,21 @@ interface JournalRepositoryInterface public function getJournalCategoryId(TransactionJournal $journal): int; /** - * Return the name of the category linked to the journal (if any) or to the transactions (if any). - * - * @param TransactionJournal $journal - * - * @return string - */ - public function getJournalCategoryName(TransactionJournal $journal): string; - - /** - * Return requested date as string. When it's a NULL return the date of journal, - * otherwise look for meta field and return that one. - * - * @param TransactionJournal $journal - * @param null|string $field - * - * @return string - */ - public function getJournalDate(TransactionJournal $journal, ?string $field): string; - - /** + * TODO this method is no longer well-fitted in 4.8.0. Should be refactored and/or removed. * Return a list of all destination accounts related to journal. * * @param TransactionJournal $journal - * + * @deprecated * @return Collection */ public function getJournalDestinationAccounts(TransactionJournal $journal): Collection; /** + * TODO this method is no longer well-fitted in 4.8.0. Should be refactored and/or removed. * Return a list of all source accounts related to journal. * * @param TransactionJournal $journal - * + * @deprecated * @return Collection */ public function getJournalSourceAccounts(TransactionJournal $journal): Collection; @@ -215,6 +176,7 @@ interface JournalRepositoryInterface public function getJournalTotal(TransactionJournal $journal): string; /** + * TODO only used on command line. * Return all journals without a group, used in an upgrade routine. * * @return array @@ -222,6 +184,7 @@ interface JournalRepositoryInterface public function getJournalsWithoutGroup(): array; /** + * TODO used only in transformer, so only for API use. * @param TransactionJournalLink $link * * @return string @@ -229,6 +192,7 @@ interface JournalRepositoryInterface public function getLinkNoteText(TransactionJournalLink $link): string; /** + * TODO used only on console * Return Carbon value of a meta field (or NULL). * * @param TransactionJournal $journal @@ -249,16 +213,8 @@ interface JournalRepositoryInterface public function getMetaDateById(int $journalId, string $field): ?Carbon; /** - * Return string value of a meta date (or NULL). + * TODO used only on the console. * - * @param TransactionJournal $journal - * @param string $field - * - * @return null|string - */ - public function getMetaDateString(TransactionJournal $journal, string $field): ?string; - - /** * Return value of a meta field (or NULL). * * @param TransactionJournal $journal @@ -269,6 +225,8 @@ interface JournalRepositoryInterface public function getMetaField(TransactionJournal $journal, string $field): ?string; /** + * TODO used only on the console. + * * Return text of a note attached to journal, or NULL * * @param TransactionJournal $journal @@ -278,6 +236,8 @@ interface JournalRepositoryInterface public function getNoteText(TransactionJournal $journal): ?string; /** + * TODO used only in the API + * * @param TransactionJournal $journal * * @return Collection @@ -285,6 +245,8 @@ interface JournalRepositoryInterface public function getPiggyBankEvents(TransactionJournal $journal): Collection; /** + * TODO used only on the console. + * * Returns all journals with more than 2 transactions. Should only return empty collections * in Firefly III > v4.8.0. * @@ -293,6 +255,8 @@ interface JournalRepositoryInterface public function getSplitJournals(): Collection; /** + * TODO used only on the console. + * * Return all tags as strings in an array. * * @param TransactionJournal $journal @@ -302,43 +266,12 @@ interface JournalRepositoryInterface public function getTags(TransactionJournal $journal): array; /** - * Return the transaction type of the journal. + * TODO maybe move to account repository? * - * @param TransactionJournal $journal - * - * @return string - */ - public function getTransactionType(TransactionJournal $journal): string; - - /** - * Will tell you if journal is reconciled or not. - * - * @param TransactionJournal $journal - * - * @return bool - */ - public function isJournalReconciled(TransactionJournal $journal): bool; - - /** - * @param Transaction $transaction - * - * @return bool - */ - public function reconcile(Transaction $transaction): bool; - - /** * @param int $journalId */ public function reconcileById(int $journalId): void; - /** - * @param TransactionJournal $journal - * @param int $order - * - * @return bool - */ - public function setOrder(TransactionJournal $journal, int $order): bool; - /** * @param User $user */ diff --git a/app/TransactionRules/Triggers/ToAccountContains.php b/app/TransactionRules/Triggers/ToAccountContains.php index 99730c70c2..0206db1afb 100644 --- a/app/TransactionRules/Triggers/ToAccountContains.php +++ b/app/TransactionRules/Triggers/ToAccountContains.php @@ -65,6 +65,7 @@ final class ToAccountContains extends AbstractTrigger implements TriggerInterfac /** * Returns true when to-account contains X + * TODO * * @param TransactionJournal $journal *