From a3ec741d670ea6f912ce01cc52b32b5c115819c5 Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 25 Dec 2017 09:44:46 +0100 Subject: [PATCH] Updated code coverage, improved binder code. --- app/Support/Binder/AccountList.php | 54 ++++------ app/Support/Binder/BinderInterface.php | 8 +- app/Support/Binder/BudgetList.php | 9 +- app/Support/Binder/CategoryList.php | 9 +- app/Support/Binder/CurrencyCode.php | 11 +- app/Support/Binder/Date.php | 9 +- app/Support/Binder/JournalList.php | 7 +- app/Support/Binder/TagList.php | 9 +- app/Support/Binder/UnfinishedJournal.php | 21 ++-- app/Support/Facades/Amount.php | 1 + app/Support/Facades/ExpandedForm.php | 3 +- app/Support/Facades/FireflyConfig.php | 1 + app/Support/Facades/Navigation.php | 1 + app/Support/Facades/Preferences.php | 1 + app/Support/Facades/Steam.php | 1 + tests/Unit/Middleware/BinderTest.php | 131 ++++++++++++++++++++++- 16 files changed, 205 insertions(+), 71 deletions(-) diff --git a/app/Support/Binder/AccountList.php b/app/Support/Binder/AccountList.php index d49cac9f0c..78be7c6e38 100644 --- a/app/Support/Binder/AccountList.php +++ b/app/Support/Binder/AccountList.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Support\Binder; use FireflyIII\Models\Account; +use Illuminate\Routing\Route; use Illuminate\Support\Collection; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -31,52 +32,41 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; */ class AccountList implements BinderInterface { + /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * * @return Collection */ - public static function routeBinder($value, $route): Collection + public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { - $ids = explode(',', $value); - // filter ids: - $ids = self::filterIds($ids); + $list = []; + $incoming = explode(',', $value); + foreach ($incoming as $entry) { + $list[] = intval($entry); + } + $list = array_unique($list); + if (count($list) === 0) { + throw new NotFoundHttpException; // @codeCoverageIgnore + } - /** @var \Illuminate\Support\Collection $object */ - $object = Account::leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id') - ->whereIn('accounts.id', $ids) - ->where('user_id', auth()->user()->id) - ->get(['accounts.*']); - if ($object->count() > 0) { - $object = $object->sortBy( + /** @var \Illuminate\Support\Collection $collection */ + $collection = auth()->user()->accounts() + ->leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id') + ->whereIn('accounts.id', $list) + ->get(['accounts.*']); + if ($collection->count() > 0) { + $collection = $collection->sortBy( function (Account $account) { return $account->name; } ); - return $object; + return $collection; } } throw new NotFoundHttpException; } - - /** - * @param array $ids - * - * @return array - */ - protected static function filterIds(array $ids): array - { - $new = []; - foreach ($ids as $id) { - if (intval($id) > 0) { - $new[] = $id; - } - } - $new = array_unique($new); - - return $new; - } } diff --git a/app/Support/Binder/BinderInterface.php b/app/Support/Binder/BinderInterface.php index 6055ec4bac..3488867ba1 100644 --- a/app/Support/Binder/BinderInterface.php +++ b/app/Support/Binder/BinderInterface.php @@ -22,16 +22,18 @@ declare(strict_types=1); namespace FireflyIII\Support\Binder; +use Illuminate\Routing\Route; + /** * Interface BinderInterface. */ interface BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * * @return mixed */ - public static function routeBinder($value, $route); + public static function routeBinder(string $value, Route $route); } diff --git a/app/Support/Binder/BudgetList.php b/app/Support/Binder/BudgetList.php index bc9a939bb9..fd6031596d 100644 --- a/app/Support/Binder/BudgetList.php +++ b/app/Support/Binder/BudgetList.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Support\Binder; use FireflyIII\Models\Budget; +use Illuminate\Routing\Route; use Illuminate\Support\Collection; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -32,12 +33,12 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class BudgetList implements BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * - * @return mixed + * @return Collection */ - public static function routeBinder($value, $route): Collection + public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { $ids = explode(',', $value); diff --git a/app/Support/Binder/CategoryList.php b/app/Support/Binder/CategoryList.php index 9d10c9b33d..a62c43d6c7 100644 --- a/app/Support/Binder/CategoryList.php +++ b/app/Support/Binder/CategoryList.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Support\Binder; use FireflyIII\Models\Category; +use Illuminate\Routing\Route; use Illuminate\Support\Collection; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -32,12 +33,12 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class CategoryList implements BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * - * @return mixed + * @return Collection */ - public static function routeBinder($value, $route): Collection + public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { $ids = explode(',', $value); diff --git a/app/Support/Binder/CurrencyCode.php b/app/Support/Binder/CurrencyCode.php index a8887c6466..e142ee83a6 100644 --- a/app/Support/Binder/CurrencyCode.php +++ b/app/Support/Binder/CurrencyCode.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Support\Binder; use FireflyIII\Models\TransactionCurrency; +use Illuminate\Routing\Route; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** @@ -31,15 +32,15 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class CurrencyCode implements BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * - * @return mixed + * @return TransactionCurrency */ - public static function routeBinder($value, $route) + public static function routeBinder(string $value, Route $route): TransactionCurrency { if (auth()->check()) { - $currency = TransactionCurrency::where('code', $value)->first(); + $currency = TransactionCurrency::where('code', trim($value))->first(); if (null !== $currency) { return $currency; } diff --git a/app/Support/Binder/Date.php b/app/Support/Binder/Date.php index 7c99cb304e..06d86a3ce2 100644 --- a/app/Support/Binder/Date.php +++ b/app/Support/Binder/Date.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support\Binder; use Carbon\Carbon; use Exception; use FireflyIII\Helpers\FiscalHelper; +use Illuminate\Routing\Route; use Log; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -34,12 +35,12 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class Date implements BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * - * @return mixed + * @return Carbon */ - public static function routeBinder($value, $route): Carbon + public static function routeBinder(string $value, Route $route): Carbon { $fiscalHelper = new FiscalHelper; diff --git a/app/Support/Binder/JournalList.php b/app/Support/Binder/JournalList.php index df6db30d7a..1c44c8d5d4 100644 --- a/app/Support/Binder/JournalList.php +++ b/app/Support/Binder/JournalList.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Support\Binder; use FireflyIII\Models\TransactionJournal; +use Illuminate\Routing\Route; use Illuminate\Support\Collection; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -32,12 +33,12 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class JournalList implements BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * * @return mixed */ - public static function routeBinder($value, $route): Collection + public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { $ids = explode(',', $value); diff --git a/app/Support/Binder/TagList.php b/app/Support/Binder/TagList.php index 7571e58cd6..0cc154c961 100644 --- a/app/Support/Binder/TagList.php +++ b/app/Support/Binder/TagList.php @@ -24,6 +24,7 @@ namespace FireflyIII\Support\Binder; use FireflyIII\Models\Tag; use FireflyIII\Repositories\Tag\TagRepositoryInterface; +use Illuminate\Routing\Route; use Illuminate\Support\Collection; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -33,12 +34,12 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class TagList implements BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * - * @return mixed + * @return Collection */ - public static function routeBinder($value, $route): Collection + public static function routeBinder(string $value, Route $route): Collection { if (auth()->check()) { $tags = explode(',', $value); diff --git a/app/Support/Binder/UnfinishedJournal.php b/app/Support/Binder/UnfinishedJournal.php index 66804dfcbf..92d03ac75f 100644 --- a/app/Support/Binder/UnfinishedJournal.php +++ b/app/Support/Binder/UnfinishedJournal.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Support\Binder; use FireflyIII\Models\TransactionJournal; +use Illuminate\Routing\Route; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** @@ -31,20 +32,20 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class UnfinishedJournal implements BinderInterface { /** - * @param $value - * @param $route + * @param string $value + * @param Route $route * - * @return mixed + * @return TransactionJournal */ - public static function routeBinder($value, $route): TransactionJournal + public static function routeBinder(string $value, Route $route): TransactionJournal { if (auth()->check()) { - $object = TransactionJournal::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 ($object) { - return $object; + $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 (!is_null($journal)) { + return $journal; } } diff --git a/app/Support/Facades/Amount.php b/app/Support/Facades/Amount.php index 58e1e75091..24bb3d89c6 100644 --- a/app/Support/Facades/Amount.php +++ b/app/Support/Facades/Amount.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support\Facades; use Illuminate\Support\Facades\Facade; /** + * @codeCoverageIgnore * Class Amount. */ class Amount extends Facade diff --git a/app/Support/Facades/ExpandedForm.php b/app/Support/Facades/ExpandedForm.php index bd47a39ffc..909313f951 100644 --- a/app/Support/Facades/ExpandedForm.php +++ b/app/Support/Facades/ExpandedForm.php @@ -25,7 +25,8 @@ namespace FireflyIII\Support\Facades; use Illuminate\Support\Facades\Facade; /** - * Class Amount. + * @codeCoverageIgnore + * Class ExpandedForm. */ class ExpandedForm extends Facade { diff --git a/app/Support/Facades/FireflyConfig.php b/app/Support/Facades/FireflyConfig.php index 3c305252d8..98fdaf7201 100644 --- a/app/Support/Facades/FireflyConfig.php +++ b/app/Support/Facades/FireflyConfig.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support\Facades; use Illuminate\Support\Facades\Facade; /** + * @codeCoverageIgnore * Class FireflyConfig. */ class FireflyConfig extends Facade diff --git a/app/Support/Facades/Navigation.php b/app/Support/Facades/Navigation.php index 18f3f8aa84..f54c32e414 100644 --- a/app/Support/Facades/Navigation.php +++ b/app/Support/Facades/Navigation.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support\Facades; use Illuminate\Support\Facades\Facade; /** + * @codeCoverageIgnore * Class Navigation. */ class Navigation extends Facade diff --git a/app/Support/Facades/Preferences.php b/app/Support/Facades/Preferences.php index 8b09a72c0d..a166989cbb 100644 --- a/app/Support/Facades/Preferences.php +++ b/app/Support/Facades/Preferences.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support\Facades; use Illuminate\Support\Facades\Facade; /** + * @codeCoverageIgnore * Class Preferences. */ class Preferences extends Facade diff --git a/app/Support/Facades/Steam.php b/app/Support/Facades/Steam.php index d4854a9eae..d59b93b9b8 100644 --- a/app/Support/Facades/Steam.php +++ b/app/Support/Facades/Steam.php @@ -25,6 +25,7 @@ namespace FireflyIII\Support\Facades; use Illuminate\Support\Facades\Facade; /** + * @codeCoverageIgnore * Class Steam. */ class Steam extends Facade diff --git a/tests/Unit/Middleware/BinderTest.php b/tests/Unit/Middleware/BinderTest.php index d9a394c28a..753c3995c5 100644 --- a/tests/Unit/Middleware/BinderTest.php +++ b/tests/Unit/Middleware/BinderTest.php @@ -25,6 +25,7 @@ namespace Tests\Unit\Helpers; use FireflyIII\Http\Middleware\Binder; +use Illuminate\Support\Collection; use Route; use Symfony\Component\HttpFoundation\Response; use Tests\TestCase; @@ -55,6 +56,80 @@ class BinderTest extends TestCase $this->assertEquals(Response::HTTP_OK, $response->getStatusCode()); } + /** + * @covers \FireflyIII\Http\Middleware\Binder::handle + * @covers \FireflyIII\Http\Middleware\Binder::__construct + * @covers \FireflyIII\Http\Middleware\Binder::performBinding + * @covers \FireflyIII\Support\Binder\AccountList::routeBinder + */ + public function testAccountList() + { + Route::middleware(Binder::class)->any( + '/_test/binder/{accountList}', function (Collection $accounts) { + return 'count: ' . $accounts->count(); + } + ); + $this->be($this->user()); + $response = $this->get('/_test/binder/1,2'); + $this->assertEquals(Response::HTTP_OK, $response->getStatusCode()); + $response->assertSee('count: 2'); + } + + /** + * @covers \FireflyIII\Http\Middleware\Binder::handle + * @covers \FireflyIII\Http\Middleware\Binder::__construct + * @covers \FireflyIII\Http\Middleware\Binder::performBinding + * @covers \FireflyIII\Support\Binder\AccountList::routeBinder + */ + public function testAccountListEmpty() + { + Route::middleware(Binder::class)->any( + '/_test/binder/{accountList}', function (Collection $accounts) { + return 'count: ' . $accounts->count(); + } + ); + $this->be($this->user()); + $response = $this->get('/_test/binder/'); + $this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode()); + } + + /** + * @covers \FireflyIII\Http\Middleware\Binder::handle + * @covers \FireflyIII\Http\Middleware\Binder::__construct + * @covers \FireflyIII\Http\Middleware\Binder::performBinding + * @covers \FireflyIII\Support\Binder\AccountList::routeBinder + */ + public function testAccountListInvalid() + { + Route::middleware(Binder::class)->any( + '/_test/binder/{accountList}', function (Collection $accounts) { + return 'count: ' . $accounts->count(); + } + ); + $this->be($this->user()); + $response = $this->get('/_test/binder/0,1,2'); + $this->assertEquals(Response::HTTP_OK, $response->getStatusCode()); + $response->assertSee('count: 2'); + } + + /** + * @covers \FireflyIII\Http\Middleware\Binder::handle + * @covers \FireflyIII\Http\Middleware\Binder::__construct + * @covers \FireflyIII\Http\Middleware\Binder::performBinding + * @covers \FireflyIII\Support\Binder\AccountList::routeBinder + */ + public function testAccountListNotLoggedIn() + { + Route::middleware(Binder::class)->any( + '/_test/binder/{accountList}', function (Collection $accounts) { + return 'count: ' . $accounts->count(); + } + ); + $response = $this->get('/_test/binder/1,2'); + $this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode()); + } + + /** * @covers \FireflyIII\Http\Middleware\Binder::handle * @covers \FireflyIII\Http\Middleware\Binder::__construct @@ -92,7 +167,6 @@ class BinderTest extends TestCase $this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode()); } - /** * @covers \FireflyIII\Http\Middleware\Binder::handle * @covers \FireflyIII\Http\Middleware\Binder::__construct @@ -1063,5 +1137,60 @@ class BinderTest extends TestCase $this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode()); } + /** + * @covers \FireflyIII\Http\Middleware\Binder::handle + * @covers \FireflyIII\Http\Middleware\Binder::__construct + * @covers \FireflyIII\Http\Middleware\Binder::performBinding + * @covers \FireflyIII\Support\Binder\UnfinishedJournal::routeBinder + */ + public function testUnfinishedJournal() + { + $journal = $this->user()->transactionJournals()->where('completed', 0)->first(); + Route::middleware(Binder::class)->any( + '/_test/binder/{unfinishedJournal}', function () { + return 'OK'; + } + ); + $this->be($this->user()); + $response = $this->get('/_test/binder/' . $journal->id); + $this->assertEquals(Response::HTTP_OK, $response->getStatusCode()); + } + + /** + * @covers \FireflyIII\Http\Middleware\Binder::handle + * @covers \FireflyIII\Http\Middleware\Binder::__construct + * @covers \FireflyIII\Http\Middleware\Binder::performBinding + * @covers \FireflyIII\Support\Binder\UnfinishedJournal::routeBinder + */ + public function testUnfinishedJournalFinished() + { + $journal = $this->user()->transactionJournals()->where('completed', 1)->first(); + Route::middleware(Binder::class)->any( + '/_test/binder/{unfinishedJournal}', function () { + return 'OK'; + } + ); + $response = $this->get('/_test/binder/' . $journal->id); + $this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode()); + } + + /** + * @covers \FireflyIII\Http\Middleware\Binder::handle + * @covers \FireflyIII\Http\Middleware\Binder::__construct + * @covers \FireflyIII\Http\Middleware\Binder::performBinding + * @covers \FireflyIII\Support\Binder\UnfinishedJournal::routeBinder + */ + public function testUnfinishedJournalNotLoggedIn() + { + $journal = $this->user()->transactionJournals()->where('completed', 0)->first(); + Route::middleware(Binder::class)->any( + '/_test/binder/{unfinishedJournal}', function () { + return 'OK'; + } + ); + $response = $this->get('/_test/binder/' . $journal->id); + $this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode()); + } + } \ No newline at end of file