From 6f974fe2850c9ea635b572965972490efbf9b0c3 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 15 Apr 2018 19:20:24 +0200 Subject: [PATCH] Improve API and test coverage. --- app/Api/V1/Controllers/BillController.php | 41 +-- app/Api/V1/Controllers/CurrencyController.php | 68 ++--- app/Api/V1/Requests/BillRequest.php | 46 ++- app/Api/V1/Requests/CurrencyRequest.php | 3 - .../Api/V1/Controllers/BillControllerTest.php | 3 +- .../V1/Controllers/CurrencyControllerTest.php | 267 ++++++++++++++++++ 6 files changed, 342 insertions(+), 86 deletions(-) create mode 100644 tests/Api/V1/Controllers/CurrencyControllerTest.php diff --git a/app/Api/V1/Controllers/BillController.php b/app/Api/V1/Controllers/BillController.php index a6aeeda7b9..bbf38c444d 100644 --- a/app/Api/V1/Controllers/BillController.php +++ b/app/Api/V1/Controllers/BillController.php @@ -27,6 +27,7 @@ use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Bill; use FireflyIII\Repositories\Bill\BillRepositoryInterface; use FireflyIII\Transformers\BillTransformer; +use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Collection; use League\Fractal\Manager; @@ -47,7 +48,7 @@ class BillController extends Controller /** * BillController constructor. * - * @throws \FireflyIII\Exceptions\FireflyException + * @throws FireflyException */ public function __construct() { @@ -66,11 +67,11 @@ class BillController extends Controller /** * Remove the specified resource from storage. * - * @param \FireflyIII\Models\Bill $bill + * @param Bill $bill * - * @return \Illuminate\Http\Response + * @return JsonResponse */ - public function delete(Bill $bill) + public function delete(Bill $bill): JsonResponse { $this->repository->destroy($bill); @@ -82,9 +83,9 @@ class BillController extends Controller * * @param Request $request * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse */ - public function index(Request $request) + public function index(Request $request): JsonResponse { $pageSize = (int)Preferences::getForUser(auth()->user(), 'listPageSize', 50)->data; $paginator = $this->repository->getPaginator($pageSize); @@ -106,9 +107,9 @@ class BillController extends Controller * @param Request $request * @param Bill $bill * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse */ - public function show(Request $request, Bill $bill) + public function show(Request $request, Bill $bill): JsonResponse { $manager = new Manager(); // add include parameter: @@ -126,22 +127,22 @@ class BillController extends Controller /** * @param BillRequest $request * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse * @throws FireflyException */ - public function store(BillRequest $request) + public function store(BillRequest $request): JsonResponse { - $bill = $this->repository->store($request->getAll()); - if(null !== $bill) { - $manager = new Manager(); - $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; - $manager->setSerializer(new JsonApiSerializer($baseUrl)); + $bill = $this->repository->store($request->getAll()); + if (null !== $bill) { + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $manager->setSerializer(new JsonApiSerializer($baseUrl)); - $resource = new Item($bill, new BillTransformer($this->parameters), 'bills'); + $resource = new Item($bill, new BillTransformer($this->parameters), 'bills'); - return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); + return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } - throw new FireflyException('Could not store new bill.'); + throw new FireflyException('Could not store new bill.'); // @codeCoverageIgnore } @@ -150,9 +151,9 @@ class BillController extends Controller * @param BillRequest $request * @param Bill $bill * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse */ - public function update(BillRequest $request, Bill $bill) + public function update(BillRequest $request, Bill $bill): JsonResponse { $data = $request->getAll(); $bill = $this->repository->update($bill, $data); diff --git a/app/Api/V1/Controllers/CurrencyController.php b/app/Api/V1/Controllers/CurrencyController.php index 2302f3b4be..176986afb8 100644 --- a/app/Api/V1/Controllers/CurrencyController.php +++ b/app/Api/V1/Controllers/CurrencyController.php @@ -20,25 +20,7 @@ */ declare(strict_types=1); -/** - * CurrencyController.php - * Copyright (c) 2018 thegrumpydictator@gmail.com - * - * This file is part of Firefly III. - * - * Firefly III is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Firefly III is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Firefly III. If not, see . - */ + namespace FireflyIII\Api\V1\Controllers; @@ -49,8 +31,11 @@ use FireflyIII\Models\TransactionCurrency; use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; use FireflyIII\Repositories\User\UserRepositoryInterface; use FireflyIII\Transformers\CurrencyTransformer; +use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; +use Illuminate\Pagination\LengthAwarePaginator; use League\Fractal\Manager; +use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; use League\Fractal\Resource\Item; use League\Fractal\Serializer\JsonApiSerializer; @@ -69,7 +54,7 @@ class CurrencyController extends Controller /** * CurrencyRepository constructor. * - * @throws \FireflyIII\Exceptions\FireflyException + * @throws FireflyException */ public function __construct() { @@ -91,10 +76,10 @@ class CurrencyController extends Controller * * @param TransactionCurrency $currency * - * @return \Illuminate\Http\Response + * @return JsonResponse * @throws FireflyException */ - public function delete(TransactionCurrency $currency) + public function delete(TransactionCurrency $currency): JsonResponse { if (!$this->userRepository->hasRole(auth()->user(), 'owner')) { // access denied: @@ -113,18 +98,27 @@ class CurrencyController extends Controller * * @param Request $request * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse */ - public function index(Request $request) + public function index(Request $request): JsonResponse { + $pageSize = (int)Preferences::getForUser(auth()->user(), 'listPageSize', 50)->data; $collection = $this->repository->get(); - $manager = new Manager(); - $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $count = $collection->count(); + // slice them: + $currencies = $collection->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); + $paginator = new LengthAwarePaginator($currencies, $count, $pageSize, $this->parameters->get('page')); + $paginator->setPath(route('api.v1.currencies.index') . $this->buildParams()); + + + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); $defaultCurrency = app('amount')->getDefaultCurrencyByUser(auth()->user()); $this->parameters->set('defaultCurrency', $defaultCurrency); - $resource = new FractalCollection($collection, new CurrencyTransformer($this->parameters), 'currencies'); + $resource = new FractalCollection($currencies, new CurrencyTransformer($this->parameters), 'currencies'); + $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } @@ -134,9 +128,9 @@ class CurrencyController extends Controller * @param Request $request * @param TransactionCurrency $currency * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse */ - public function show(Request $request, TransactionCurrency $currency) + public function show(Request $request, TransactionCurrency $currency): JsonResponse { $manager = new Manager(); // add include parameter: @@ -156,10 +150,10 @@ class CurrencyController extends Controller /** * @param CurrencyRequest $request * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse * @throws FireflyException */ - public function store(CurrencyRequest $request) + public function store(CurrencyRequest $request): JsonResponse { $currency = $this->repository->store($request->getAll()); @@ -178,18 +172,18 @@ class CurrencyController extends Controller return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } - throw new FireflyException('Could not store new currency.'); + throw new FireflyException('Could not store new currency.'); // @codeCoverageIgnore } /** - * @param BillRequest $request + * @param CurrencyRequest $request * @param TransactionCurrency $currency * - * @return \Illuminate\Http\JsonResponse + * @return JsonResponse */ - public function update(CurrencyRequest $request, TransactionCurrency $currency) + public function update(CurrencyRequest $request, TransactionCurrency $currency): JsonResponse { $data = $request->getAll(); $currency = $this->repository->update($currency, $data); @@ -199,8 +193,8 @@ class CurrencyController extends Controller Preferences::mark(); } - $manager = new Manager(); - $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); $defaultCurrency = app('amount')->getDefaultCurrencyByUser(auth()->user()); diff --git a/app/Api/V1/Requests/BillRequest.php b/app/Api/V1/Requests/BillRequest.php index 9cc6a848ed..64f64127d6 100644 --- a/app/Api/V1/Requests/BillRequest.php +++ b/app/Api/V1/Requests/BillRequest.php @@ -46,18 +46,17 @@ class BillRequest extends Request public function getAll(): array { $data = [ - 'name' => $this->string('name'), - 'match' => $this->string('match'), - 'amount_min' => $this->string('amount_min'), - 'amount_max' => $this->string('amount_max'), - //'currency_id' => $this->integer('currency_id'), - //'currency_code' => $this->string('currency_code'), - 'date' => $this->date('date'), - 'repeat_freq' => $this->string('repeat_freq'), - 'skip' => $this->integer('skip'), - 'automatch' => $this->boolean('automatch'), - 'active' => $this->boolean('active'), - 'notes' => $this->string('notes'), + 'name' => $this->string('name'), + 'amount_min' => $this->string('amount_min'), + 'amount_max' => $this->string('amount_max'), + 'currency_id' => $this->integer('currency_id'), + 'currency_code' => $this->string('currency_code'), + 'date' => $this->date('date'), + 'repeat_freq' => $this->string('repeat_freq'), + 'skip' => $this->integer('skip'), + 'automatch' => $this->boolean('automatch'), + 'active' => $this->boolean('active'), + 'notes' => $this->string('notes'), ]; return $data; @@ -69,18 +68,17 @@ class BillRequest extends Request public function rules(): array { $rules = [ - 'name' => 'required|between:1,255|uniqueObjectForUser:bills,name', - 'match' => 'required|between:1,255|uniqueObjectForUser:bills,match', - 'amount_min' => 'required|numeric|more:0', - 'amount_max' => 'required|numeric|more:0', - //'currency_id' => 'numeric|exists:transaction_currencies,id|required_without:currency_code', - //'currency_code' => 'min:3|max:3|exists:transaction_currencies,code|required_without:currency_id', - 'date' => 'required|date', - 'repeat_freq' => 'required|in:weekly,monthly,quarterly,half-year,yearly', - 'skip' => 'required|between:0,31', - 'automatch' => 'required|boolean', - 'active' => 'required|boolean', - 'notes' => 'between:1,65536', + 'name' => 'required|between:1,255|uniqueObjectForUser:bills,name', + 'amount_min' => 'required|numeric|more:0', + 'amount_max' => 'required|numeric|more:0', + 'currency_id' => 'numeric|exists:transaction_currencies,id|required_without:currency_code', + 'currency_code' => 'min:3|max:3|exists:transaction_currencies,code|required_without:currency_id', + 'date' => 'required|date', + 'repeat_freq' => 'required|in:weekly,monthly,quarterly,half-year,yearly', + 'skip' => 'required|between:0,31', + 'automatch' => 'required|boolean', + 'active' => 'required|boolean', + 'notes' => 'between:1,65536', ]; switch ($this->method()) { default: diff --git a/app/Api/V1/Requests/CurrencyRequest.php b/app/Api/V1/Requests/CurrencyRequest.php index 1a3c3c020c..72666a3a77 100644 --- a/app/Api/V1/Requests/CurrencyRequest.php +++ b/app/Api/V1/Requests/CurrencyRequest.php @@ -74,9 +74,6 @@ class CurrencyRequest extends Request $rules['name'] = 'required|between:1,255|unique:transaction_currencies,name,' . $currency->id; $rules['code'] = 'required|between:1,255|unique:transaction_currencies,code,' . $currency->id; $rules['symbol'] = 'required|between:1,255|unique:transaction_currencies,symbol,' . $currency->id; - //$bill = $this->route()->parameter('bill'); - //$rules['name'] .= ',' . $bill->id; - //$rules['match'] .= ',' . $bill->id; break; } diff --git a/tests/Api/V1/Controllers/BillControllerTest.php b/tests/Api/V1/Controllers/BillControllerTest.php index ebb886fede..22a4b6cdf0 100644 --- a/tests/Api/V1/Controllers/BillControllerTest.php +++ b/tests/Api/V1/Controllers/BillControllerTest.php @@ -50,8 +50,7 @@ class BillControllerTest extends TestCase /** * Send delete * - * @covers \FireflyIII\Api\V1\Controllers\BillController::delete - * @covers \FireflyIII\Api\V1\Controllers\BillController::__construct + * @covers \FireflyIII\Api\V1\Controllers\BillController */ public function testDelete() { diff --git a/tests/Api/V1/Controllers/CurrencyControllerTest.php b/tests/Api/V1/Controllers/CurrencyControllerTest.php new file mode 100644 index 0000000000..70c18e13a6 --- /dev/null +++ b/tests/Api/V1/Controllers/CurrencyControllerTest.php @@ -0,0 +1,267 @@ +. + */ + +declare(strict_types=1); + +namespace Tests\Api\V1\Controllers; + + +use FireflyIII\Models\Preference; +use FireflyIII\Models\TransactionCurrency; +use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; +use FireflyIII\Repositories\User\UserRepositoryInterface; +use Laravel\Passport\Passport; +use Log; +use Mockery; +use Preferences; +use Tests\TestCase; + +/** + * Class CurrencyControllerTest + */ +class CurrencyControllerTest extends TestCase +{ + /** + * + */ + public function setUp() + { + parent::setUp(); + Passport::actingAs($this->user()); + Log::debug(sprintf('Now in %s.', get_class($this))); + + } + + /** + * Send delete + * + * @covers \FireflyIII\Api\V1\Controllers\CurrencyController + */ + public function testDelete(): void + { + // mock stuff: + $repository = $this->mock(CurrencyRepositoryInterface::class); + $userRepos = $this->mock(UserRepositoryInterface::class); + + // mock calls: + $repository->shouldReceive('setUser')->once(); + //$userRepos->shouldReceive('setUser')->once(); + + $userRepos->shouldReceive('hasRole')->once()->withArgs([Mockery::any(), 'owner'])->andReturn(true); + $repository->shouldReceive('canDeleteCurrency')->once()->andReturn(true); + + $repository->shouldReceive('destroy')->once()->andReturn(true); + + // get a currency + $currency = TransactionCurrency::first(); + + // call API + $response = $this->delete('/api/v1/currencies/' . $currency->id); + $response->assertStatus(204); + } + + /** + * Show index. + * + * @covers \FireflyIII\Api\V1\Controllers\CurrencyController + */ + public function testIndex(): void + { + $collection = TransactionCurrency::get(); + // mock stuff: + $repository = $this->mock(CurrencyRepositoryInterface::class); + + // mock calls: + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('get')->withNoArgs()->andReturn($collection)->once(); + + // test API + $response = $this->get('/api/v1/currencies'); + $response->assertStatus(200); + $response->assertJson(['data' => [],]); + $response->assertJson( + ['meta' => ['pagination' => ['total' => $collection->count(), 'count' => $collection->count(), 'per_page' => 50, 'current_page' => 1, + 'total_pages' => 1]],] + ); + $response->assertJson( + ['links' => ['self' => true, 'first' => true, 'last' => true,],] + ); + $response->assertHeader('Content-Type', 'application/vnd.api+json'); + } + + /** + * Test show of a currency. + * + * @covers \FireflyIII\Api\V1\Controllers\CurrencyController + */ + public function testShow(): void + { + // create stuff + $currency = TransactionCurrency::first(); + $repository = $this->mock(CurrencyRepositoryInterface::class); + + // mock calls: + $repository->shouldReceive('setUser')->once(); + + // test API + $response = $this->get('/api/v1/currencies/' . $currency->id); + $response->assertStatus(200); + $response->assertJson( + ['data' => [ + 'type' => 'currencies', + 'id' => $currency->id, + ],] + ); + $response->assertHeader('Content-Type', 'application/vnd.api+json'); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\CurrencyController + * @covers \FireflyIII\Api\V1\Requests\CurrencyRequest + */ + public function testStore(): void + { + + $currency = TransactionCurrency::first(); + $repository = $this->mock(CurrencyRepositoryInterface::class); + + // mock calls: + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('store')->andReturn($currency); + + // data to submit: + $data = [ + 'name' => 'New currency', + 'code' => 'ABC', + 'symbol' => 'A', + 'decimal_places' => 2, + 'default' => 'false', + ]; + + // test API + $response = $this->post('/api/v1/currencies', $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson(['data' => ['type' => 'currencies', 'links' => true],]); + $response->assertHeader('Content-Type', 'application/vnd.api+json'); + $response->assertSee($currency->name); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\CurrencyController + * @covers \FireflyIII\Api\V1\Requests\CurrencyRequest + */ + public function testStoreWithDefault(): void + { + $currency = TransactionCurrency::first(); + $repository = $this->mock(CurrencyRepositoryInterface::class); + $preference = new Preference; + $preference->data = 'EUR'; + // mock calls: + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('store')->andReturn($currency); + Preferences::shouldReceive('set')->withArgs(['currencyPreference', 'EUR'])->once(); + Preferences::shouldReceive('mark')->once(); + Preferences::shouldReceive('lastActivity')->once(); + Preferences::shouldReceive('getForUser')->once()->andReturn($preference); + + // data to submit: + $data = [ + 'name' => 'New currency', + 'code' => 'ABC', + 'symbol' => 'A', + 'decimal_places' => 2, + 'default' => 'true', + ]; + + // test API + $response = $this->post('/api/v1/currencies', $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson(['data' => ['type' => 'currencies', 'links' => true],]); + $response->assertHeader('Content-Type', 'application/vnd.api+json'); + $response->assertSee($currency->name); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\CurrencyController + * @covers \FireflyIII\Api\V1\Requests\CurrencyRequest + */ + public function testUpdate(): void + { + $currency = TransactionCurrency::first(); + $repository = $this->mock(CurrencyRepositoryInterface::class); + + // mock calls: + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('update')->andReturn($currency); + + // data to submit: + $data = [ + 'name' => 'Updated currency', + 'code' => 'ABC', + 'symbol' => '$E', + 'decimal_places' => '2', + 'default' => 'false', + ]; + + // test API + $response = $this->put('/api/v1/currencies/' . $currency->id, $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson(['data' => ['type' => 'currencies', 'links' => true],]); + $response->assertHeader('Content-Type', 'application/vnd.api+json'); + $response->assertSee($currency->name); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\CurrencyController + * @covers \FireflyIII\Api\V1\Requests\CurrencyRequest + */ + public function testUpdateWithDefault(): void + { + $currency = TransactionCurrency::first(); + $repository = $this->mock(CurrencyRepositoryInterface::class); + $preference = new Preference; + $preference->data = 'EUR'; + + // mock calls: + $repository->shouldReceive('setUser')->once(); + $repository->shouldReceive('update')->andReturn($currency); + Preferences::shouldReceive('set')->withArgs(['currencyPreference', 'EUR'])->once(); + Preferences::shouldReceive('mark')->once(); + Preferences::shouldReceive('lastActivity')->once(); + Preferences::shouldReceive('getForUser')->once()->andReturn($preference); + + // data to submit: + $data = [ + 'name' => 'Updated currency', + 'code' => 'ABC', + 'symbol' => '$E', + 'decimal_places' => '2', + 'default' => 'true', + ]; + + // test API + $response = $this->put('/api/v1/currencies/' . $currency->id, $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson(['data' => ['type' => 'currencies', 'links' => true],]); + $response->assertHeader('Content-Type', 'application/vnd.api+json'); + $response->assertSee($currency->name); + } +} \ No newline at end of file