From 947b83cbd131266fd69597244b4b7c77db780c63 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 29 Jun 2019 19:47:31 +0200 Subject: [PATCH] Improve test coverage. --- .../Json/AutoCompleteController.php | 231 +----------- .../Controllers/Json/ReconcileController.php | 9 +- .../Controllers/Json/RecurrenceController.php | 1 + .../Controllers/AutoCompleteCollector.php | 155 -------- app/Support/Twig/AmountFormat.php | 16 +- app/Validation/AccountValidator.php | 1 + phpunit.coverage.specific.xml | 2 + phpunit.coverage.xml | 2 + phpunit.xml | 2 + .../V1/Controllers/BudgetControllerTest.php | 2 +- .../Controllers/BudgetLimitControllerTest.php | 2 +- .../V1/Controllers/CategoryControllerTest.php | 2 +- .../V1/Controllers/CurrencyControllerTest.php | 2 +- .../TransactionLinkControllerTest.php | 2 +- .../Account/IndexControllerTest.php | 5 +- .../Controllers/BillControllerTest.php | 2 +- .../Category/NoCategoryControllerTest.php | 4 +- .../Category/ShowControllerTest.php | 4 +- .../Chart/AccountControllerTest.php | 2 +- .../Chart/BudgetReportControllerTest.php | 8 +- .../Chart/CategoryReportControllerTest.php | 10 +- .../Chart/ExpenseReportControllerTest.php | 2 +- .../Controllers/HomeControllerTest.php | 2 +- .../Json/AutoCompleteControllerTest.php | 346 +----------------- .../Controllers/Json/BoxControllerTest.php | 61 +-- .../Json/ExchangeControllerTest.php | 3 + .../Json/FrontpageControllerTest.php | 2 + .../Controllers/Json/IntroControllerTest.php | 12 + .../Json/ReconcileControllerTest.php | 132 +++---- .../Json/RecurrenceControllerTest.php | 12 +- .../Popup/ReportControllerTest.php | 84 +++-- .../Report/ExpenseControllerTest.php | 2 +- .../Feature/Controllers/TagControllerTest.php | 2 +- .../Transaction/MassControllerTest.php | 2 +- .../Transaction/SingleControllerTest.php | 2 +- .../Transaction/SplitControllerTest.php | 2 +- .../Controllers/TransactionControllerTest.php | 4 +- tests/TestCase.php | 5 +- .../Import/Storage/ImportArrayStorageTest.php | 6 +- 39 files changed, 238 insertions(+), 907 deletions(-) delete mode 100644 app/Support/Http/Controllers/AutoCompleteCollector.php diff --git a/app/Http/Controllers/Json/AutoCompleteController.php b/app/Http/Controllers/Json/AutoCompleteController.php index 2cca2c370a..e5f87f3b68 100644 --- a/app/Http/Controllers/Json/AutoCompleteController.php +++ b/app/Http/Controllers/Json/AutoCompleteController.php @@ -22,21 +22,16 @@ declare(strict_types=1); namespace FireflyIII\Http\Controllers\Json; -use FireflyIII\Exceptions\FireflyException; -use FireflyIII\Helpers\Collector\GroupCollectorInterface; use FireflyIII\Http\Controllers\Controller; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\TransactionCurrency; -use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; use FireflyIII\Repositories\Category\CategoryRepositoryInterface; use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use FireflyIII\Repositories\Tag\TagRepositoryInterface; -use FireflyIII\Support\CacheProperties; -use FireflyIII\Support\Http\Controllers\AutoCompleteCollector; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Log; @@ -44,11 +39,12 @@ use Log; /** * Class AutoCompleteController. * + * TODO autocomplete for transaction types. + * * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class AutoCompleteController extends Controller { - use AutoCompleteCollector; /** * @param Request $request @@ -95,119 +91,9 @@ class AutoCompleteController extends Controller return response()->json($return); } - /** - * List of all journals. - * - * @param Request $request - * @return JsonResponse - */ - public function allTransactionJournals(Request $request): JsonResponse - { - $search = (string)$request->get('search'); - $cache = new CacheProperties; - $cache->addProperty('ac-all-journals'); - // very unlikely a user will actually search for this string. - $key = '' === $search ? 'skjf0893j89fj2398hd89dh289h2398hr7isd8900828u209ujnxs88929282u' : $search; - $cache->addProperty($key); - if ($cache->has()) { - return response()->json($cache->get()); // @codeCoverageIgnore - } - // find everything: - /** @var GroupCollectorInterface $collector */ - $collector = app(GroupCollectorInterface::class); - $collector->setLimit(250)->setPage(1); - - $journals = $collector->getExtractedJournals(); - $return = []; - /** @var array $journal */ - foreach ($journals as $journal) { - if (strlen($journal['group_title']) > 0) { - $return[] = $journal['group_title']; - } - if (strlen($journal['description']) > 0) { - $return[] = $journal['description']; - } - } - $return = array_unique($return); - - if ('' !== $search) { - $return = array_values( - array_unique( - array_filter( - $return, function (string $value) use ($search) { - return !(false === stripos($value, $search)); - }, ARRAY_FILTER_USE_BOTH - ) - ) - ); - } - $cache->store($return); - - return response()->json($return); - } - - /** - * @param Request $request - * @param string $subject - * - * @return JsonResponse - * @throws FireflyException - */ - public function autoComplete(Request $request, string $subject): JsonResponse - { - $search = (string)$request->get('search'); - $unfiltered = null; - $filtered = null; - - switch ($subject) { - default: - break; - case 'all-accounts': - $unfiltered = $this->getAccounts( - [AccountType::REVENUE, AccountType::EXPENSE, AccountType::BENEFICIARY, AccountType::DEFAULT, AccountType::ASSET, AccountType::LOAN, - AccountType::DEBT, AccountType::MORTGAGE] - ); - break; - case 'expense-accounts': - $unfiltered = $this->getAccounts([AccountType::EXPENSE, AccountType::BENEFICIARY]); - break; - case 'revenue-accounts': - $unfiltered = $this->getAccounts([AccountType::REVENUE]); - break; - case 'asset-accounts': - $unfiltered = $this->getAccounts([AccountType::ASSET, AccountType::DEFAULT]); - break; - case 'categories': - $unfiltered = $this->getCategories(); - break; - case 'budgets': - $unfiltered = $this->getBudgets(); - break; - case 'tags': - $unfiltered = $this->getTags(); - break; - case 'bills': - $unfiltered = $this->getBills(); - break; - case 'currency-names': - $unfiltered = $this->getCurrencyNames(); - break; - case 'transaction-types': - case 'transaction_types': - $unfiltered = $this->getTransactionTypes(); - break; - } - $filtered = $this->filterResult($unfiltered, $search); - - if (null === $filtered) { - throw new FireflyException(sprintf('Auto complete handler cannot handle "%s"', $subject)); // @codeCoverageIgnore - } - - return response()->json($filtered); - } - /** * @return JsonResponse + * @codeCoverageIgnore */ public function budgets(): JsonResponse { @@ -221,6 +107,7 @@ class AutoCompleteController extends Controller * @param Request $request * * @return JsonResponse + * @codeCoverageIgnore */ public function categories(Request $request): JsonResponse { @@ -233,11 +120,10 @@ class AutoCompleteController extends Controller } /** - * @param Request $request - * * @return JsonResponse + * @codeCoverageIgnore */ - public function currencies(Request $request): JsonResponse + public function currencies(): JsonResponse { /** @var CurrencyRepositoryInterface $repository */ $repository = app(CurrencyRepositoryInterface::class); @@ -259,61 +145,9 @@ class AutoCompleteController extends Controller return response()->json($return); } - /** - * List of journals with their ID. - * - * @param Request $request - * @param TransactionJournal $except - * - * @return JsonResponse - */ - public function journalsWithId(Request $request, TransactionJournal $except): JsonResponse - { - $search = (string)$request->get('search'); - $cache = new CacheProperties; - $cache->addProperty('ac-expense-accounts'); - // very unlikely a user will actually search for this string. - $key = '' === $search ? 'skjf0893j89fj2398hd89dh289h2398hr7isd8900828u209ujnxs88929282u' : $search; - $cache->addProperty($key); - if ($cache->has()) { - return response()->json($cache->get()); // @codeCoverageIgnore - } - // find everything: - /** @var GroupCollectorInterface $collector */ - $collector = app(GroupCollectorInterface::class); - $collector->setLimit(400)->setPage(1); - $set = $collector->getExtractedJournals(); - $return = []; - foreach ($set as $journal) { - $id = (int)$journal['transaction_journal_id']; - if ($id !== $except->id) { - $return[] = [ - 'id' => $id, - 'name' => $id . ': ' . $journal['description'], - ]; - } - } - - sort($return); - - if ('' !== $search) { - $return = array_filter( - $return, function (array $array) use ($search) { - $haystack = $array['name']; - $result = stripos($haystack, $search); - - return !(false === $result); - } - ); - - } - $cache->store($return); - - return response()->json($return); - } - /** * @return JsonResponse + * @codeCoverageIgnore */ public function piggyBanks(): JsonResponse { @@ -324,7 +158,9 @@ class AutoCompleteController extends Controller } /** + * @param Request $request * @return JsonResponse + * @codeCoverageIgnore */ public function tags(Request $request): JsonResponse { @@ -337,53 +173,4 @@ class AutoCompleteController extends Controller return response()->json($result->toArray()); } - /** - * List of journals by type. - * - * @param Request $request - * @param string $what - * - * @return JsonResponse - */ - public function transactionJournals(Request $request, string $what): JsonResponse - { - $search = (string)$request->get('search'); - $cache = new CacheProperties; - $cache->addProperty('ac-journals'); - // very unlikely a user will actually search for this string. - $key = '' === $search ? 'skjf0893j89fj2398hd89dh289h2398hr7isd8900828u209ujnxs88929282u' : $search; - $cache->addProperty($key); - if ($cache->has()) { - return response()->json($cache->get()); // @codeCoverageIgnore - } - // find everything: - $type = config('firefly.transactionTypesByWhat.' . $what); - $types = [$type]; - - /** @var GroupCollectorInterface $collector */ - $collector = app(GroupCollectorInterface::class); - $collector->setTypes($types)->setLimit(250)->setPage(1); - $result = $collector->getExtractedJournals(); - $return = []; - foreach ($result as $journal) { - $return[] = $journal['description']; - } - $return = array_unique($return); - sort($return); - - if ('' !== $search) { - $return = array_values( - array_unique( - array_filter( - $return, function (string $value) use ($search) { - return !(false === stripos($value, $search)); - }, ARRAY_FILTER_USE_BOTH - ) - ) - ); - } - $cache->store($return); - - return response()->json($return); - } } diff --git a/app/Http/Controllers/Json/ReconcileController.php b/app/Http/Controllers/Json/ReconcileController.php index d111379fdf..25cd2e07e0 100644 --- a/app/Http/Controllers/Json/ReconcileController.php +++ b/app/Http/Controllers/Json/ReconcileController.php @@ -145,7 +145,7 @@ class ReconcileController extends Controller // @codeCoverageIgnoreStart } catch (Throwable $e) { Log::debug(sprintf('View error: %s', $e->getMessage())); - $view = 'Could not render accounts.reconcile.overview'; + $view = sprintf('Could not render accounts.reconcile.overview: %s', $e->getMessage()); } // @codeCoverageIgnoreEnd @@ -196,6 +196,7 @@ class ReconcileController extends Controller /** @var array $journal */ foreach ($array as $journal) { $inverse = false; + // @codeCoverageIgnoreStart if (TransactionType::DEPOSIT === $journal['transaction_type_type']) { $inverse = true; } @@ -211,6 +212,7 @@ class ReconcileController extends Controller $journal['foreign_amount'] = app('steam')->positive($journal['foreign_amount']); } } + // @codeCoverageIgnoreEnd $journals[] = $journal; } @@ -240,6 +242,9 @@ class ReconcileController extends Controller { $toAdd = '0'; Log::debug(sprintf('User submitted %s #%d: "%s"', $journal['transaction_type_type'], $journal['transaction_journal_id'], $journal['description'])); + + // not much magic below we need to cover using tests. + // @codeCoverageIgnoreStart if ($account->id === $journal['source_account_id']) { if ($currency->id === $journal['currency_id']) { $toAdd = $journal['amount']; @@ -256,6 +261,8 @@ class ReconcileController extends Controller $toAdd = bcmul($journal['foreign_amount'], '-1'); } } + // @codeCoverageIgnoreEnd + Log::debug(sprintf('Going to add %s to %s', $toAdd, $amount)); $amount = bcadd($amount, $toAdd); Log::debug(sprintf('Result is %s', $amount)); diff --git a/app/Http/Controllers/Json/RecurrenceController.php b/app/Http/Controllers/Json/RecurrenceController.php index d672b01311..77ef359ae4 100644 --- a/app/Http/Controllers/Json/RecurrenceController.php +++ b/app/Http/Controllers/Json/RecurrenceController.php @@ -42,6 +42,7 @@ class RecurrenceController extends Controller /** * RecurrenceController constructor. + * @codeCoverageIgnore */ public function __construct() { diff --git a/app/Support/Http/Controllers/AutoCompleteCollector.php b/app/Support/Http/Controllers/AutoCompleteCollector.php deleted file mode 100644 index 719640f082..0000000000 --- a/app/Support/Http/Controllers/AutoCompleteCollector.php +++ /dev/null @@ -1,155 +0,0 @@ -. - */ - -declare(strict_types=1); - -namespace FireflyIII\Support\Http\Controllers; - -use FireflyIII\Models\Account; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; -use FireflyIII\Repositories\Bill\BillRepositoryInterface; -use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; -use FireflyIII\Repositories\Category\CategoryRepositoryInterface; -use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; -use FireflyIII\Repositories\Journal\JournalRepositoryInterface; -use FireflyIII\Repositories\Tag\TagRepositoryInterface; -use Illuminate\Support\Collection; - -/** - * Trait AutoCompleteCollector - */ -trait AutoCompleteCollector -{ - - /** - * @param array $unfiltered - * @param string $query - * - * @return array|null - */ - protected function filterResult(?array $unfiltered, string $query): ?array - { - if (null === $unfiltered) { - return null; // @codeCoverageIgnore - } - if ('' === $query) { - sort($unfiltered); - - return $unfiltered; - } - $return = []; - if ('' !== $query) { - $return = array_values( - array_filter( - $unfiltered, function (string $value) use ($query) { - return !(false === stripos($value, $query)); - }, ARRAY_FILTER_USE_BOTH - ) - ); - } - sort($return); - - - return $return; - } - - /** - * @param array $types - * - * @return array - */ - protected function getAccounts(array $types): array - { - $repository = app(AccountRepositoryInterface::class); - // find everything: - /** @var Collection $collection */ - $collection = $repository->getAccountsByType($types); - $filtered = $collection->filter( - function (Account $account) { - return true === $account->active; - } - ); - - return array_values(array_unique($filtered->pluck('name')->toArray())); - } - - /** - * @return array - */ - protected function getBills(): array - { - $repository = app(BillRepositoryInterface::class); - - return array_unique($repository->getActiveBills()->pluck('name')->toArray()); - } - - /** - * @return array - */ - protected function getBudgets(): array - { - $repository = app(BudgetRepositoryInterface::class); - - return array_unique($repository->getBudgets()->pluck('name')->toArray()); - } - - /** - * @return array - */ - protected function getCategories(): array - { - $repository = app(CategoryRepositoryInterface::class); - - return array_unique($repository->getCategories()->pluck('name')->toArray()); - } - - /** - * @return array - */ - protected function getCurrencyNames(): array - { - /** @var CurrencyRepositoryInterface $repository */ - $repository = app(CurrencyRepositoryInterface::class); - - return $repository->get()->pluck('name')->toArray(); - } - - /** - * @return array - */ - protected function getTags(): array - { - /** @var TagRepositoryInterface $repository */ - $repository = app(TagRepositoryInterface::class); - - return array_unique($repository->get()->pluck('tag')->toArray()); - } - - /** - * @return array - */ - protected function getTransactionTypes(): array - { - $repository = app(JournalRepositoryInterface::class); - - return array_unique($repository->getTransactionTypes()->pluck('type')->toArray()); - } -} diff --git a/app/Support/Twig/AmountFormat.php b/app/Support/Twig/AmountFormat.php index c9be6b5258..ec8b8b3d61 100644 --- a/app/Support/Twig/AmountFormat.php +++ b/app/Support/Twig/AmountFormat.php @@ -25,7 +25,6 @@ namespace FireflyIII\Support\Twig; use FireflyIII\Models\Account as AccountModel; use FireflyIII\Models\TransactionCurrency; use FireflyIII\Repositories\Account\AccountRepositoryInterface; -use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; use Log; use Twig_Extension; use Twig_SimpleFilter; @@ -87,23 +86,14 @@ class AmountFormat extends Twig_Extension static function (AccountModel $account, string $amount, bool $coloured = null): string { if ('testing' === config('app.env')) { - Log::warning('AmountFormat::formatAmountByAccount should NOT be called in the TEST environment!'); + Log::warning('Twig AmountFormat::formatAmountByAccount should NOT be called in the TEST environment!'); + Log::warning('Make sure AccountRepos and Amount::getDefaultCurrency are mocked.'); } $coloured = $coloured ?? true; /** @var AccountRepositoryInterface $accountRepos */ $accountRepos = app(AccountRepositoryInterface::class); - /** @var CurrencyRepositoryInterface $currencyRepos */ - $currencyRepos = app(CurrencyRepositoryInterface::class); - $currency = app('amount')->getDefaultCurrency(); - $currencyId = (int)$accountRepos->getMetaValue($account, 'currency_id'); - $accountCurrency = null; - if (0 !== $currencyId) { - $accountCurrency = $currencyRepos->findNull($currencyId); - } - if (null !== $accountCurrency) { - $currency = $accountCurrency; - } + $currency = $accountRepos->getAccountCurrency($account) ?? app('amount')->getDefaultCurrency(); return app('amount')->formatAnything($currency, $amount, $coloured); }, diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index e91e90ef19..6e87415335 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -554,6 +554,7 @@ class AccountValidator } // don't expect to end up here: + return false; } diff --git a/phpunit.coverage.specific.xml b/phpunit.coverage.specific.xml index 2665e0bf58..d9a53a5f60 100644 --- a/phpunit.coverage.specific.xml +++ b/phpunit.coverage.specific.xml @@ -51,6 +51,8 @@ ./tests/Feature/Controllers/Category ./tests/Feature/Controllers/Chart ./tests/Feature/Controllers/Import + ./tests/Feature/Controllers/Json + ./tests/Feature/Controllers/Popup diff --git a/phpunit.coverage.xml b/phpunit.coverage.xml index 2499add904..d676ea64f1 100644 --- a/phpunit.coverage.xml +++ b/phpunit.coverage.xml @@ -51,6 +51,8 @@ ./tests/Feature/Controllers/Category ./tests/Feature/Controllers/Chart ./tests/Feature/Controllers/Import + ./tests/Feature/Controllers/Json + ./tests/Feature/Controllers/Popup diff --git a/phpunit.xml b/phpunit.xml index 6ad538ee36..8000be2857 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -51,6 +51,8 @@ ./tests/Feature/Controllers/Category ./tests/Feature/Controllers/Chart ./tests/Feature/Controllers/Import + ./tests/Feature/Controllers/Json + ./tests/Feature/Controllers/Popup