From 6ff4a0b45c9ce10ffb9b4e7ef5047a13d1b96c62 Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 25 Jul 2019 14:19:49 +0200 Subject: [PATCH] Improve test coverage. --- app/Models/Note.php | 1 + app/Models/Preference.php | 1 - app/Models/Rule.php | 1 + app/Models/Transaction.php | 27 - app/Support/Binder/BudgetList.php | 6 +- app/Support/Binder/ImportProvider.php | 19 +- app/Support/Binder/TagList.php | 7 +- app/Support/Binder/TransactionGroup.php | 53 -- app/Support/Binder/UnfinishedJournal.php | 55 -- config/firefly.php | 6 - .../Controllers/AttachmentControllerTest.php | 10 +- .../Controllers/BillControllerTest.php | 24 +- .../Budget/DeleteControllerTest.php | 4 +- .../Controllers/Budget/EditControllerTest.php | 4 +- .../Budget/IndexControllerTest.php | 4 +- .../Controllers/Budget/ShowControllerTest.php | 2 +- .../Category/CreateControllerTest.php | 4 +- .../Category/DeleteControllerTest.php | 2 +- .../Category/EditControllerTest.php | 5 +- .../Category/NoCategoryControllerTest.php | 18 +- .../Category/ShowControllerTest.php | 8 +- .../Chart/AccountControllerTest.php | 41 +- .../Chart/BudgetControllerTest.php | 16 +- .../Chart/BudgetReportControllerTest.php | 7 +- .../Chart/CategoryReportControllerTest.php | 9 +- .../Chart/ExpenseReportControllerTest.php | 2 +- .../Chart/PiggyBankControllerTest.php | 2 +- .../Chart/TagReportControllerTest.php | 14 +- .../Controllers/CurrencyControllerTest.php | 38 +- .../Controllers/DebugControllerTest.php | 8 +- .../Import/PrerequisitesControllerTest.php | 65 +- .../Controllers/JavascriptControllerTest.php | 4 +- .../Controllers/Json/BoxControllerTest.php | 7 +- .../Json/ExchangeControllerTest.php | 5 +- .../Controllers/Json/IntroControllerTest.php | 2 +- .../Json/ReconcileControllerTest.php | 18 +- .../Controllers/Json/RuleControllerTest.php | 2 - .../Controllers/NewUserControllerTest.php | 1 - .../Controllers/PiggyBankControllerTest.php | 24 +- .../Popup/ReportControllerTest.php | 15 +- .../Controllers/PreferencesControllerTest.php | 4 +- .../Controllers/ProfileControllerTest.php | 17 +- .../Recurring/CreateControllerTest.php | 1 - .../Recurring/IndexControllerTest.php | 9 +- .../Controllers/ReportControllerTest.php | 34 +- .../Controllers/Rule/CreateControllerTest.php | 2 - .../Controllers/Rule/DeleteControllerTest.php | 9 +- .../Controllers/Rule/EditControllerTest.php | 18 +- .../Controllers/Rule/IndexControllerTest.php | 2 - .../Controllers/Rule/SelectControllerTest.php | 6 +- .../RuleGroup/CreateControllerTest.php | 4 +- .../RuleGroup/ExecutionControllerTest.php | 2 - .../Controllers/SearchControllerTest.php | 2 +- .../Controllers/System/CronControllerTest.php | 7 +- .../Feature/Controllers/TagControllerTest.php | 28 +- .../Transaction/ConvertControllerTest.php | 4 - .../Transaction/CreateControllerTest.php | 4 +- .../Transaction/IndexControllerTest.php | 12 +- .../Transaction/LinkControllerTest.php | 14 +- .../Transaction/MassControllerTest.php | 14 +- tests/Unit/Middleware/BinderTest.php | 687 ++++++++++++++---- 61 files changed, 822 insertions(+), 599 deletions(-) delete mode 100644 app/Support/Binder/TransactionGroup.php delete mode 100644 app/Support/Binder/UnfinishedJournal.php diff --git a/app/Models/Note.php b/app/Models/Note.php index 7526f1cf24..94732a1242 100644 --- a/app/Models/Note.php +++ b/app/Models/Note.php @@ -86,6 +86,7 @@ class Note extends Model /** * @param $value + * @codeCoverageIgnore */ public function setTextAttribute($value): void { diff --git a/app/Models/Preference.php b/app/Models/Preference.php index ac4c514b15..5e3853bc8f 100644 --- a/app/Models/Preference.php +++ b/app/Models/Preference.php @@ -38,7 +38,6 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; * @property int $id * @property User user * @property int $user_id - * @property-read \FireflyIII\User $user * @method static \Illuminate\Database\Eloquent\Builder|\FireflyIII\Models\Preference newModelQuery() * @method static \Illuminate\Database\Eloquent\Builder|\FireflyIII\Models\Preference newQuery() * @method static \Illuminate\Database\Eloquent\Builder|\FireflyIII\Models\Preference query() diff --git a/app/Models/Rule.php b/app/Models/Rule.php index 4bbd508263..e459eb4677 100644 --- a/app/Models/Rule.php +++ b/app/Models/Rule.php @@ -147,6 +147,7 @@ class Rule extends Model /** * @param $value + * @codeCoverageIgnore */ public function setDescriptionAttribute($value): void { diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index a599ebbb8f..8591b1f2c0 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -102,10 +102,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; * @property Carbon updated_at * @property string foreign_currency_code * @SuppressWarnings (PHPMD.TooManyPublicMethods) - * @property \Illuminate\Support\Carbon|null $created_at - * @property \Illuminate\Support\Carbon|null $updated_at * @property \Illuminate\Support\Carbon|null $deleted_at - * @property bool $reconciled * @property-read \Illuminate\Database\Eloquent\Collection|\FireflyIII\Models\Budget[] $budgets * @property-read \Illuminate\Database\Eloquent\Collection|\FireflyIII\Models\Category[] $categories * @method static \Illuminate\Database\Eloquent\Builder|\FireflyIII\Models\Transaction after(\Carbon\Carbon $date) @@ -185,30 +182,6 @@ class Transaction extends Model return false; } - /** - * Route binder. Converts the key in the URL to the specified object (or throw 404). - * - * @param string $value - * - * @return Transaction - * @throws NotFoundHttpException - */ - public static function routeBinder(string $value): Transaction - { - if (auth()->check()) { - $transactionId = (int)$value; - /** @var User $user */ - $user = auth()->user(); - /** @var Transaction $transaction */ - $transaction = $user->transactions()->where('transactions.id', $transactionId)->first(['transactions.*']); - if (null !== $transaction) { - return $transaction; - } - } - - throw new NotFoundHttpException; - } - /** * Get the account this object belongs to. diff --git a/app/Support/Binder/BudgetList.php b/app/Support/Binder/BudgetList.php index 67a2595c70..09f6188e9a 100644 --- a/app/Support/Binder/BudgetList.php +++ b/app/Support/Binder/BudgetList.php @@ -46,11 +46,13 @@ class BudgetList implements BinderInterface //Log::debug(sprintf('Now in BudgetList::routeBinder("%s")', $value)); if (auth()->check()) { $list = array_unique(array_map('\intval', explode(',', $value))); - //Log::debug('List is now', $list); + + // @codeCoverageIgnoreStart if (0 === count($list)) { Log::warning('Budget list count is zero, return 404.'); - throw new NotFoundHttpException; // @codeCoverageIgnore + throw new NotFoundHttpException; } + // @codeCoverageIgnoreEnd /** @var Collection $collection */ $collection = auth()->user()->budgets() diff --git a/app/Support/Binder/ImportProvider.php b/app/Support/Binder/ImportProvider.php index 4ee846d1e5..02cde65d27 100644 --- a/app/Support/Binder/ImportProvider.php +++ b/app/Support/Binder/ImportProvider.php @@ -44,8 +44,14 @@ class ImportProvider implements BinderInterface { $repository = app(UserRepositoryInterface::class); // get and filter all import routines: + + if (!auth()->check()) { + return []; + } + /** @var User $user */ $user = auth()->user(); + /** @var array $config */ $providerNames = array_keys(config('import.enabled')); $providers = []; @@ -55,13 +61,20 @@ class ImportProvider implements BinderInterface // only consider enabled providers $enabled = (bool)config(sprintf('import.enabled.%s', $providerName)); $allowedForUser = (bool)config(sprintf('import.allowed_for_user.%s', $providerName)); + $allowedForDemo = (bool)config(sprintf('import.allowed_for_demo.%s', $providerName)); if (false === $enabled) { continue; } - - if (false === $isDemoUser && false === $allowedForUser && false === $isDebug) { - continue; // @codeCoverageIgnore + if (false === $allowedForUser && !$isDemoUser) { + continue; } + if (false === $allowedForDemo && $isDemoUser) { + continue; + } + +// if (false === $isDemoUser && false === $allowedForUser && false === $isDebug) { +// continue; +// } $providers[$providerName] = [ 'has_prereq' => (bool)config('import.has_prereq.' . $providerName), diff --git a/app/Support/Binder/TagList.php b/app/Support/Binder/TagList.php index 150b072772..62d64dd568 100644 --- a/app/Support/Binder/TagList.php +++ b/app/Support/Binder/TagList.php @@ -46,17 +46,20 @@ class TagList implements BinderInterface if (auth()->check()) { $list = array_unique(array_map('\strtolower', explode(',', $value))); Log::debug('List of tags is', $list); + // @codeCoverageIgnoreStart if (0 === count($list)) { Log::error('Tag list is empty.'); - throw new NotFoundHttpException; // @codeCoverageIgnore + throw new NotFoundHttpException; } + // @codeCoverageIgnoreEnd + /** @var TagRepositoryInterface $repository */ $repository = app(TagRepositoryInterface::class); $repository->setUser(auth()->user()); $allTags = $repository->get(); $collection = $allTags->filter( - function (Tag $tag) use ($list) { + static function (Tag $tag) use ($list) { if (in_array(strtolower($tag->tag), $list, true)) { return true; } diff --git a/app/Support/Binder/TransactionGroup.php b/app/Support/Binder/TransactionGroup.php deleted file mode 100644 index abe4fe05e0..0000000000 --- a/app/Support/Binder/TransactionGroup.php +++ /dev/null @@ -1,53 +0,0 @@ -. - */ -declare(strict_types=1); - -namespace FireflyIII\Support\Binder; - -use FireflyIII\Models\TransactionJournal; -use Illuminate\Routing\Route; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; - -/** - * Class TransactionGroup. - */ -class TransactionGroup implements BinderInterface -{ - /** - * @param string $value - * @param Route $route - * - * @return TransactionGroup - * @throws NotFoundHttpException - */ - public static function routeBinder(string $value, Route $route): TransactionGroup - { - if (auth()->check()) { - $group = auth()->user()->transactionGroups() - ->find($value); - if (null !== $group) { - return $group; - } - } - - throw new NotFoundHttpException; - } -} diff --git a/app/Support/Binder/UnfinishedJournal.php b/app/Support/Binder/UnfinishedJournal.php deleted file mode 100644 index b255144d8a..0000000000 --- a/app/Support/Binder/UnfinishedJournal.php +++ /dev/null @@ -1,55 +0,0 @@ -. - */ -declare(strict_types=1); - -namespace FireflyIII\Support\Binder; - -use FireflyIII\Models\TransactionJournal; -use Illuminate\Routing\Route; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; - -/** - * Class Date. - */ -class UnfinishedJournal implements BinderInterface -{ - /** - * @param string $value - * @param Route $route - * - * @return TransactionJournal - * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException - */ - public static function routeBinder(string $value, Route $route): TransactionJournal - { - if (auth()->check()) { - $journal = auth()->user()->transactionJournals()->where('transaction_journals.id', $value) - ->leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') - ->where('completed', 0) - ->where('user_id', auth()->user()->id)->first(['transaction_journals.*']); - if (null !== $journal) { - return $journal; - } - } - - throw new NotFoundHttpException; - } -} diff --git a/config/firefly.php b/config/firefly.php index e062578248..ad3419778e 100644 --- a/config/firefly.php +++ b/config/firefly.php @@ -23,7 +23,6 @@ declare(strict_types=1); -use FireflyIII\Export\Exporter\CsvExporter; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\Attachment; @@ -32,7 +31,6 @@ use FireflyIII\Models\Bill; use FireflyIII\Models\Budget; use FireflyIII\Models\BudgetLimit; use FireflyIII\Models\Category; -use FireflyIII\Models\ExportJob; use FireflyIII\Models\ImportJob; use FireflyIII\Models\LinkType; use FireflyIII\Models\PiggyBank; @@ -41,7 +39,6 @@ use FireflyIII\Models\Recurrence; use FireflyIII\Models\Rule; use FireflyIII\Models\RuleGroup; use FireflyIII\Models\Tag; -use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionGroup; use FireflyIII\Models\TransactionJournal; @@ -60,7 +57,6 @@ use FireflyIII\Support\Binder\ImportProvider; use FireflyIII\Support\Binder\JournalList; use FireflyIII\Support\Binder\TagList; use FireflyIII\Support\Binder\TagOrId; -use FireflyIII\Support\Binder\UnfinishedJournal; use FireflyIII\TransactionRules\Actions\AddTag; use FireflyIII\TransactionRules\Actions\AppendDescription; use FireflyIII\TransactionRules\Actions\AppendNotes; @@ -377,7 +373,6 @@ return [ 'rule' => Rule::class, 'ruleGroup' => RuleGroup::class, 'importJob' => ImportJob::class, - 'transaction' => Transaction::class, 'transactionGroup' => TransactionGroup::class, 'user' => User::class, @@ -401,7 +396,6 @@ return [ // others 'fromCurrencyCode' => CurrencyCode::class, 'toCurrencyCode' => CurrencyCode::class, - 'unfinishedJournal' => UnfinishedJournal::class, 'cliToken' => CLIToken::class, 'tagOrId' => TagOrId::class, 'configName' => ConfigurationName::class, diff --git a/tests/Feature/Controllers/AttachmentControllerTest.php b/tests/Feature/Controllers/AttachmentControllerTest.php index 2bc51c5a17..9e762d2dab 100644 --- a/tests/Feature/Controllers/AttachmentControllerTest.php +++ b/tests/Feature/Controllers/AttachmentControllerTest.php @@ -23,9 +23,7 @@ declare(strict_types=1); namespace Tests\Feature\Controllers; use FireflyIII\Models\Attachment; -use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\Attachment\AttachmentRepositoryInterface; -use FireflyIII\Repositories\Journal\JournalRepositoryInterface; use FireflyIII\Repositories\User\UserRepositoryInterface; use Illuminate\Support\Collection; use Log; @@ -63,7 +61,7 @@ class AttachmentControllerTest extends TestCase // mock stuff $this->mock(AttachmentRepositoryInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->andReturn(true)->atLeast()->once(); @@ -122,8 +120,8 @@ class AttachmentControllerTest extends TestCase $this->mockDefaultSession(); // mock stuff - $attachment = $this->getRandomAttachment(); - $repository = $this->mock(AttachmentRepositoryInterface::class); + $attachment = $this->getRandomAttachment(); + $repository = $this->mock(AttachmentRepositoryInterface::class); $repository->shouldReceive('exists')->once()->andReturn(false); @@ -207,7 +205,7 @@ class AttachmentControllerTest extends TestCase $attachment = $this->getRandomAttachment(); $this->mockDefaultSession(); - $repository = $this->mock(AttachmentRepositoryInterface::class); + $repository = $this->mock(AttachmentRepositoryInterface::class); $repository->shouldReceive('exists')->once()->andReturn(true); $repository->shouldReceive('getContent')->once()->andReturn('This is attachment number one.'); diff --git a/tests/Feature/Controllers/BillControllerTest.php b/tests/Feature/Controllers/BillControllerTest.php index a460ed98ee..281b6645e1 100644 --- a/tests/Feature/Controllers/BillControllerTest.php +++ b/tests/Feature/Controllers/BillControllerTest.php @@ -160,10 +160,10 @@ class BillControllerTest extends TestCase // mock stuff $this->mock(AttachmentHelperInterface::class); - $bill = $this->getRandomBill(); - $repository = $this->mock(BillRepositoryInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); - $transformer = $this->mock(BillTransformer::class); + $bill = $this->getRandomBill(); + $repository = $this->mock(BillRepositoryInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); + $transformer = $this->mock(BillTransformer::class); $pref = new Preference; $pref->data = 50; @@ -172,8 +172,8 @@ class BillControllerTest extends TestCase $transformer->shouldReceive('setParameters')->atLeast()->once(); $transformer->shouldReceive('transform')->atLeast()->once()->andReturn( - ['id' => 5, 'active' => true, 'name' => 'x', 'next_expected_match' => '2018-01-01', - 'currency' => $this->getEuro(), + ['id' => 5, 'active' => true, 'name' => 'x', 'next_expected_match' => '2018-01-01', + 'currency' => $this->getEuro(), ] ); @@ -201,8 +201,8 @@ class BillControllerTest extends TestCase $this->mockDefaultSession(); // mock stuff - $rule = $this->getRandomRule(); - $repository = $this->mock(BillRepositoryInterface::class); + $rule = $this->getRandomRule(); + $repository = $this->mock(BillRepositoryInterface::class); $this->mock(AttachmentHelperInterface::class); @@ -414,8 +414,8 @@ class BillControllerTest extends TestCase $this->mockDefaultSession(); // mock stuff - $attachHelper = $this->mock(AttachmentHelperInterface::class); - $repository = $this->mock(BillRepositoryInterface::class); + $attachHelper = $this->mock(AttachmentHelperInterface::class); + $repository = $this->mock(BillRepositoryInterface::class); $repository->shouldReceive('store')->andReturn(new Bill); $attachHelper->shouldReceive('saveAttachmentsForModel'); @@ -450,8 +450,8 @@ class BillControllerTest extends TestCase $this->mockDefaultSession(); // mock stuff - $attachHelper = $this->mock(AttachmentHelperInterface::class); - $repository = $this->mock(BillRepositoryInterface::class); + $attachHelper = $this->mock(AttachmentHelperInterface::class); + $repository = $this->mock(BillRepositoryInterface::class); $repository->shouldReceive('update')->andReturn(new Bill); $attachHelper->shouldReceive('saveAttachmentsForModel'); diff --git a/tests/Feature/Controllers/Budget/DeleteControllerTest.php b/tests/Feature/Controllers/Budget/DeleteControllerTest.php index 21621106d4..08fa1dd524 100644 --- a/tests/Feature/Controllers/Budget/DeleteControllerTest.php +++ b/tests/Feature/Controllers/Budget/DeleteControllerTest.php @@ -57,7 +57,7 @@ class DeleteControllerTest extends TestCase // mock stuff $this->mock(BudgetRepositoryInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->andReturn(true)->atLeast()->once(); @@ -77,7 +77,7 @@ class DeleteControllerTest extends TestCase $budget = $this->getRandomBudget(); Log::debug('Now in testDestroy()'); // mock stuff - $repository = $this->mock(BudgetRepositoryInterface::class); + $repository = $this->mock(BudgetRepositoryInterface::class); $this->mock(UserRepositoryInterface::class); diff --git a/tests/Feature/Controllers/Budget/EditControllerTest.php b/tests/Feature/Controllers/Budget/EditControllerTest.php index 9cdfb28f5b..60082c81e5 100644 --- a/tests/Feature/Controllers/Budget/EditControllerTest.php +++ b/tests/Feature/Controllers/Budget/EditControllerTest.php @@ -54,7 +54,7 @@ class EditControllerTest extends TestCase Log::debug('Now in testEdit()'); // mock stuff $this->mock(BudgetRepositoryInterface::class); - $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->andReturn(true)->atLeast()->once(); $this->mockDefaultSession(); @@ -74,7 +74,7 @@ class EditControllerTest extends TestCase Log::debug('Now in testUpdate()'); // mock stuff $budget = $this->getRandomBudget(); - $repository = $this->mock(BudgetRepositoryInterface::class); + $repository = $this->mock(BudgetRepositoryInterface::class); $this->mock(UserRepositoryInterface::class); $repository->shouldReceive('findNull')->andReturn($budget); diff --git a/tests/Feature/Controllers/Budget/IndexControllerTest.php b/tests/Feature/Controllers/Budget/IndexControllerTest.php index ea01a0100e..cddfce6f07 100644 --- a/tests/Feature/Controllers/Budget/IndexControllerTest.php +++ b/tests/Feature/Controllers/Budget/IndexControllerTest.php @@ -65,7 +65,7 @@ class IndexControllerTest extends TestCase public function testIndex(string $range): void { // mock stuff - $budget = $this->getRandomBudget(); + $budget = $this->getRandomBudget(); $budgetLimit = $this->getRandomBudgetLimit(); $budgetLimit->start_date = Carbon::now()->startOfMonth(); $budgetLimit->end_date = Carbon::now()->endOfMonth(); @@ -102,7 +102,7 @@ class IndexControllerTest extends TestCase $this->be($this->user()); $this->changeDateRange($this->user(), $range); - $response = $this->get(route('budgets.index')); + $response = $this->get(route('budgets.index')); $response->assertStatus(200); // has bread crumb $response->assertSee('