From 5b8adbfd0c88ae41e9052cafdaf66b3cccb62933 Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 28 Feb 2018 21:32:59 +0100 Subject: [PATCH] Repository and test clean up. --- app/Helpers/Attachments/AttachmentHelper.php | 3 +- app/Helpers/Chart/MetaPieChart.php | 3 ++ app/Http/Requests/SplitJournalFormRequest.php | 36 ----------------- app/Models/PiggyBank.php | 5 ++- .../Account/AccountRepository.php | 2 + app/Repositories/Bill/BillRepository.php | 3 +- app/Repositories/Budget/BudgetRepository.php | 1 - .../Currency/CurrencyRepository.php | 7 ++++ .../ImportJobRepositoryInterface.php | 4 +- .../Journal/JournalRepository.php | 40 ++++++++----------- .../Journal/JournalRepositoryInterface.php | 25 ++++-------- app/Repositories/Journal/JournalTasker.php | 5 +++ .../RuleGroupRepositoryInterface.php | 2 + .../TransactionTypeRepository.php | 2 +- app/Rules/BelongsUser.php | 2 +- app/Support/Binder/AccountList.php | 3 ++ app/Support/Binder/TagList.php | 3 ++ .../Models/TransactionJournalTrait.php | 8 ++++ .../Transaction/SplitControllerTest.php | 1 - 19 files changed, 67 insertions(+), 88 deletions(-) diff --git a/app/Helpers/Attachments/AttachmentHelper.php b/app/Helpers/Attachments/AttachmentHelper.php index 1e6e0e6986..cdc3df2c85 100644 --- a/app/Helpers/Attachments/AttachmentHelper.php +++ b/app/Helpers/Attachments/AttachmentHelper.php @@ -50,8 +50,9 @@ class AttachmentHelper implements AttachmentHelperInterface /** @var \Illuminate\Contracts\Filesystem\Filesystem */ protected $uploadDisk; + /** - * + * AttachmentHelper constructor. */ public function __construct() { diff --git a/app/Helpers/Chart/MetaPieChart.php b/app/Helpers/Chart/MetaPieChart.php index 8fccc9d81f..91e28f0ee0 100644 --- a/app/Helpers/Chart/MetaPieChart.php +++ b/app/Helpers/Chart/MetaPieChart.php @@ -79,6 +79,9 @@ class MetaPieChart implements MetaPieChartInterface /** @var User */ protected $user; + /** + * MetaPieChart constructor. + */ public function __construct() { $this->accounts = new Collection; diff --git a/app/Http/Requests/SplitJournalFormRequest.php b/app/Http/Requests/SplitJournalFormRequest.php index a15a9ac203..303c0d7bf8 100644 --- a/app/Http/Requests/SplitJournalFormRequest.php +++ b/app/Http/Requests/SplitJournalFormRequest.php @@ -77,7 +77,6 @@ class SplitJournalFormRequest extends Request break; } $foreignAmount = $transaction['foreign_amount'] ?? null; - $foreignCurrency = isset($transaction['foreign_currency_id']) ? intval($transaction['foreign_currency_id']) : null; $set = [ 'source_id' => $sourceId, 'source_name' => $sourceName, @@ -129,39 +128,4 @@ class SplitJournalFormRequest extends Request ]; } - /** - * @return array - */ - private function getTransactionData(): array - { - $descriptions = $this->getArray('description', 'string'); - $categories = $this->getArray('category', 'string'); - $amounts = $this->getArray('amount', 'float'); - $budgets = $this->getArray('amount', 'integer'); - $srcAccountIds = $this->getArray('source_account_id', 'integer'); - $srcAccountNames = $this->getArray('source_account_name', 'string'); - $dstAccountIds = $this->getArray('destination_account_id', 'integer'); - $dstAccountNames = $this->getArray('destination_account_name', 'string'); - $piggyBankIds = $this->getArray('piggy_bank_id', 'integer'); - - $return = []; - // description is leading because it is one of the mandatory fields. - foreach ($descriptions as $index => $description) { - $category = $categories[$index] ?? ''; - $transaction = [ - 'description' => $description, - 'amount' => Steam::positive($amounts[$index]), - 'budget_id' => $budgets[$index] ?? 0, - 'category' => $category, - 'source_account_id' => $srcAccountIds[$index] ?? $this->get('journal_source_account_id'), - 'source_account_name' => $srcAccountNames[$index] ?? '', - 'piggy_bank_id' => $piggyBankIds[$index] ?? 0, - 'destination_account_id' => $dstAccountIds[$index] ?? $this->get('journal_destination_account_id'), - 'destination_account_name' => $dstAccountNames[$index] ?? '', - ]; - $return[] = $transaction; - } - - return $return; - } } diff --git a/app/Models/PiggyBank.php b/app/Models/PiggyBank.php index d5e4cd3194..8706a4b302 100644 --- a/app/Models/PiggyBank.php +++ b/app/Models/PiggyBank.php @@ -90,7 +90,7 @@ class PiggyBank extends Model /** * Grabs the PiggyBankRepetition that's currently relevant / active. - * + * @deprecated * @returns PiggyBankRepetition */ public function currentRelevantRep(): PiggyBankRepetition @@ -126,6 +126,7 @@ class PiggyBank extends Model } /** + * @deprecated * @return string */ public function getSuggestedMonthlyAmount(): string @@ -152,7 +153,7 @@ class PiggyBank extends Model /** * @param Carbon $date - * + * @deprecated * @return string */ public function leftOnAccount(Carbon $date): string diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 601e2b4473..6c6d5348ff 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -111,6 +111,8 @@ class AccountRepository implements AccountRepositoryInterface /** * Returns the amount of the opening balance for this account. * + * @param Account $account + * * @return string */ public function getOpeningBalanceAmount(Account $account): ?string diff --git a/app/Repositories/Bill/BillRepository.php b/app/Repositories/Bill/BillRepository.php index 834b69b9b2..368ee78f1f 100644 --- a/app/Repositories/Bill/BillRepository.php +++ b/app/Repositories/Bill/BillRepository.php @@ -410,7 +410,6 @@ class BillRepository implements BillRepositoryInterface * @param Carbon $date * * @return \Carbon\Carbon - * @throws \FireflyIII\Exceptions\FireflyException */ public function nextDateMatch(Bill $bill, Carbon $date): Carbon { @@ -575,6 +574,7 @@ class BillRepository implements BillRepositoryInterface /** * TODO refactor + * * @param float $amount * @param float $min * @param float $max @@ -592,6 +592,7 @@ class BillRepository implements BillRepositoryInterface /** * TODO refactor + * * @param array $matches * @param $description * diff --git a/app/Repositories/Budget/BudgetRepository.php b/app/Repositories/Budget/BudgetRepository.php index 473ff7e234..98e91fc5af 100644 --- a/app/Repositories/Budget/BudgetRepository.php +++ b/app/Repositories/Budget/BudgetRepository.php @@ -23,7 +23,6 @@ declare(strict_types=1); namespace FireflyIII\Repositories\Budget; use Carbon\Carbon; -use DB; use FireflyIII\Helpers\Collector\JournalCollectorInterface; use FireflyIII\Models\AccountType; use FireflyIII\Models\AvailableBudget; diff --git a/app/Repositories/Currency/CurrencyRepository.php b/app/Repositories/Currency/CurrencyRepository.php index 822f9dee22..37af7d2f9b 100644 --- a/app/Repositories/Currency/CurrencyRepository.php +++ b/app/Repositories/Currency/CurrencyRepository.php @@ -39,6 +39,13 @@ class CurrencyRepository implements CurrencyRepositoryInterface /** @var User */ private $user; + /** + * CurrencyRepository constructor. + */ + public function __construct() + { + } + /** * @param TransactionCurrency $currency * diff --git a/app/Repositories/ImportJob/ImportJobRepositoryInterface.php b/app/Repositories/ImportJob/ImportJobRepositoryInterface.php index 2c0efe97f2..5bb3a905cb 100644 --- a/app/Repositories/ImportJob/ImportJobRepositoryInterface.php +++ b/app/Repositories/ImportJob/ImportJobRepositoryInterface.php @@ -133,7 +133,7 @@ interface ImportJobRepositoryInterface * @param ImportJob $job * @param array $array * - * @return void + * @return ImportJob */ public function setExtendedStatus(ImportJob $job, array $array): ImportJob; @@ -147,7 +147,7 @@ interface ImportJobRepositoryInterface /** * @param ImportJob $job - * @param int $count + * @param int $steps * * @return ImportJob */ diff --git a/app/Repositories/Journal/JournalRepository.php b/app/Repositories/Journal/JournalRepository.php index a6c1658fb7..1ce51efde1 100644 --- a/app/Repositories/Journal/JournalRepository.php +++ b/app/Repositories/Journal/JournalRepository.php @@ -197,18 +197,6 @@ class JournalRepository implements JournalRepositoryInterface return null; } - /** - * Get account of transaction that is more than zero. Only works with unsplit journals. - * - * @param TransactionJournal $journal - * - * @return Account - */ - public function getDestinationAccount(TransactionJournal $journal): Account - { - return $journal->transactions()->where('amount', '<', 0)->first()->account; - } - /** * Returns the first positive transaction for the journal. Useful when editing journals. * @@ -438,18 +426,6 @@ class JournalRepository implements JournalRepositoryInterface return $note->text; } - /** - * Get account of transaction that is less than zero. Only works with unsplit journals. - * - * @param TransactionJournal $journal - * - * @return Account - */ - public function getSourceAccount(TransactionJournal $journal): Account - { - return $journal->transactions()->where('amount', '>', 0)->first()->account; - } - /** * Return all tags as strings in an array. * @@ -542,6 +518,22 @@ class JournalRepository implements JournalRepositoryInterface return true; } + /** + * @param int $transactionId + * + * @return bool + */ + public function reconcileById(int $transactionId): bool + { + /** @var Transaction $transaction */ + $transaction = $this->user->transactions()->find($transactionId); + if (!is_null($transaction)) { + return $this->reconcile($transaction); + } + + return false; + } + /** * @param TransactionJournal $journal * @param int $order diff --git a/app/Repositories/Journal/JournalRepositoryInterface.php b/app/Repositories/Journal/JournalRepositoryInterface.php index ac183c1741..3d71181414 100644 --- a/app/Repositories/Journal/JournalRepositoryInterface.php +++ b/app/Repositories/Journal/JournalRepositoryInterface.php @@ -99,15 +99,6 @@ interface JournalRepositoryInterface */ public function getAssetTransaction(TransactionJournal $journal): ?Transaction; - /** - * Get account of transaction that is more than zero. Only works with unsplit journals. - * - * @param TransactionJournal $journal - * - * @return Account - */ - public function getDestinationAccount(TransactionJournal $journal): Account; - /** * Returns the first positive transaction for the journal. Useful when editing journals. * @@ -199,15 +190,6 @@ interface JournalRepositoryInterface */ public function getNoteText(TransactionJournal $journal): string; - /** - * Get account of transaction that is less than zero. Only works with unsplit journals. - * - * @param TransactionJournal $journal - * - * @return Account - */ - public function getSourceAccount(TransactionJournal $journal): Account; - /** * Return all tags as strings in an array. * @@ -254,6 +236,13 @@ interface JournalRepositoryInterface */ public function reconcile(Transaction $transaction): bool; + /** + * @param int $transactionId + * + * @return bool + */ + public function reconcileById(int $transactionId): bool; + /** * @param TransactionJournal $journal * @param int $order diff --git a/app/Repositories/Journal/JournalTasker.php b/app/Repositories/Journal/JournalTasker.php index cdf5726868..d360490e0f 100644 --- a/app/Repositories/Journal/JournalTasker.php +++ b/app/Repositories/Journal/JournalTasker.php @@ -42,6 +42,8 @@ class JournalTasker implements JournalTaskerInterface private $user; /** + * @deprecated + * * @param TransactionJournal $journal * * @return Collection @@ -64,6 +66,7 @@ class JournalTasker implements JournalTaskerInterface * that shows a transaction (transaction/show/xx). * * @param TransactionJournal $journal + * * @deprecated * * @return array @@ -187,6 +190,8 @@ class JournalTasker implements JournalTaskerInterface * the order of transactions within the journal. So the query is pretty complex:. * * @param int $transactionId + * + * @deprecated * @deprecated * @return string */ diff --git a/app/Repositories/RuleGroup/RuleGroupRepositoryInterface.php b/app/Repositories/RuleGroup/RuleGroupRepositoryInterface.php index c45524d31f..2082a78012 100644 --- a/app/Repositories/RuleGroup/RuleGroupRepositoryInterface.php +++ b/app/Repositories/RuleGroup/RuleGroupRepositoryInterface.php @@ -52,6 +52,8 @@ interface RuleGroupRepositoryInterface public function find(int $ruleGroupId): RuleGroup; /** + * Get all rule groups. + * * @return Collection */ public function get(): Collection; diff --git a/app/Repositories/TransactionType/TransactionTypeRepository.php b/app/Repositories/TransactionType/TransactionTypeRepository.php index a2538c453f..47b5d27354 100644 --- a/app/Repositories/TransactionType/TransactionTypeRepository.php +++ b/app/Repositories/TransactionType/TransactionTypeRepository.php @@ -22,6 +22,7 @@ declare(strict_types=1); namespace FireflyIII\Repositories\TransactionType; + use FireflyIII\Models\TransactionType; /** @@ -29,7 +30,6 @@ use FireflyIII\Models\TransactionType; */ class TransactionTypeRepository implements TransactionTypeRepositoryInterface { - /** * Find a transaction type or return NULL. * diff --git a/app/Rules/BelongsUser.php b/app/Rules/BelongsUser.php index 57aa963e11..7967117327 100644 --- a/app/Rules/BelongsUser.php +++ b/app/Rules/BelongsUser.php @@ -58,7 +58,7 @@ class BelongsUser implements Rule /** * Determine if the validation rule passes. - * + * TODO use repositories? * @param string $attribute * @param mixed $value * diff --git a/app/Support/Binder/AccountList.php b/app/Support/Binder/AccountList.php index 78be7c6e38..e667ac9a9e 100644 --- a/app/Support/Binder/AccountList.php +++ b/app/Support/Binder/AccountList.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support\Binder; use FireflyIII\Models\Account; use Illuminate\Routing\Route; use Illuminate\Support\Collection; +use Log; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** @@ -49,6 +50,7 @@ class AccountList implements BinderInterface } $list = array_unique($list); if (count($list) === 0) { + Log::error('Account list is empty.'); throw new NotFoundHttpException; // @codeCoverageIgnore } @@ -67,6 +69,7 @@ class AccountList implements BinderInterface return $collection; } } + Log::error('User is not logged in.'); throw new NotFoundHttpException; } } diff --git a/app/Support/Binder/TagList.php b/app/Support/Binder/TagList.php index ced7578e00..60eb79420e 100644 --- a/app/Support/Binder/TagList.php +++ b/app/Support/Binder/TagList.php @@ -26,6 +26,7 @@ use FireflyIII\Models\Tag; use FireflyIII\Repositories\Tag\TagRepositoryInterface; use Illuminate\Routing\Route; use Illuminate\Support\Collection; +use Log; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** @@ -49,6 +50,7 @@ class TagList implements BinderInterface } $list = array_unique($list); if (count($list) === 0) { + Log::error('Tag list is empty.'); throw new NotFoundHttpException; // @codeCoverageIgnore } /** @var TagRepositoryInterface $repository */ @@ -66,6 +68,7 @@ class TagList implements BinderInterface return $collection; } } + Log::error('TagList: user is not logged in.'); throw new NotFoundHttpException; } } diff --git a/app/Support/Models/TransactionJournalTrait.php b/app/Support/Models/TransactionJournalTrait.php index aad8de504f..006288e478 100644 --- a/app/Support/Models/TransactionJournalTrait.php +++ b/app/Support/Models/TransactionJournalTrait.php @@ -74,6 +74,7 @@ trait TransactionJournalTrait abstract public function categories(): BelongsToMany; /** + * @deprecated * @return Collection */ public function destinationAccountList(): Collection @@ -98,6 +99,7 @@ trait TransactionJournalTrait } /** + * @deprecated * @return Collection */ public function destinationTransactionList(): Collection @@ -116,6 +118,7 @@ trait TransactionJournalTrait } /** + * * @param string $name * * @return string @@ -148,6 +151,7 @@ trait TransactionJournalTrait abstract public function piggyBankEvents(): HasMany; /** + * @deprecated * @return int */ public function piggyBankId(): int @@ -160,6 +164,7 @@ trait TransactionJournalTrait } /** + * @deprecated * @return Transaction */ public function positiveTransaction(): Transaction @@ -185,6 +190,7 @@ trait TransactionJournalTrait abstract public function setMeta(string $name, $value): TransactionJournalMeta; /** + * @deprecated * @return Collection */ public function sourceAccountList(): Collection @@ -209,6 +215,7 @@ trait TransactionJournalTrait } /** + * @deprecated * @return Collection */ public function sourceTransactionList(): Collection @@ -227,6 +234,7 @@ trait TransactionJournalTrait } /** + * @deprecated * @return string */ public function transactionTypeStr(): string diff --git a/tests/Feature/Controllers/Transaction/SplitControllerTest.php b/tests/Feature/Controllers/Transaction/SplitControllerTest.php index 64eaf5a597..122321a9ca 100644 --- a/tests/Feature/Controllers/Transaction/SplitControllerTest.php +++ b/tests/Feature/Controllers/Transaction/SplitControllerTest.php @@ -258,7 +258,6 @@ class SplitControllerTest extends TestCase /** * @covers \FireflyIII\Http\Controllers\Transaction\SplitController::update - * @covers \FireflyIII\Http\Controllers\Transaction\SplitController::arrayFromInput * @covers \FireflyIII\Http\Controllers\Transaction\SplitController::getTransactionDataFromRequest */ public function testUpdate()