From 6fe319702d9ec3948a95b3d21719880ba6832f3d Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 22 Mar 2017 17:02:15 +0100 Subject: [PATCH] Expand test coverage. Remove else-statement. --- app/Http/Controllers/PiggyBankController.php | 52 ++++++--------- .../Controllers/PreferencesController.php | 3 +- .../PiggyBank/PiggyBankRepository.php | 66 +++++++++++++++++++ .../PiggyBankRepositoryInterface.php | 32 +++++++++ .../Controllers/PiggyBankControllerTest.php | 61 +++++++++++++---- .../Controllers/PreferencesControllerTest.php | 1 + 6 files changed, 167 insertions(+), 48 deletions(-) diff --git a/app/Http/Controllers/PiggyBankController.php b/app/Http/Controllers/PiggyBankController.php index 213fcd8350..54185731c6 100644 --- a/app/Http/Controllers/PiggyBankController.php +++ b/app/Http/Controllers/PiggyBankController.php @@ -273,32 +273,16 @@ class PiggyBankController extends Controller public function postAdd(Request $request, PiggyBankRepositoryInterface $repository, PiggyBank $piggyBank) { $amount = $request->get('amount'); - Log::debug(sprintf('Found amount is %s', $amount)); - /** @var Carbon $date */ - $date = session('end', Carbon::now()->endOfMonth()); - $leftOnAccount = $piggyBank->leftOnAccount($date); - $savedSoFar = strval($piggyBank->currentRelevantRep()->currentamount); - $leftToSave = bcsub($piggyBank->targetamount, $savedSoFar); - $maxAmount = strval(min(round($leftOnAccount, 12), round($leftToSave, 12))); - if (bccomp($amount, $maxAmount) <= 0) { - $repetition = $piggyBank->currentRelevantRep(); - $currentAmount = $repetition->currentamount ?? '0'; - $repetition->currentamount = bcadd($currentAmount, $amount); - $repetition->save(); - - // create event - $repository->createEvent($piggyBank, $amount); - - Session::flash( - 'success', strval(trans('firefly.added_amount_to_piggy', ['amount' => Amount::format($amount, false), 'name' => $piggyBank->name])) - ); + if ($repository->canAddAmount($piggyBank, $amount)) { + $repository->addAmount($piggyBank, $amount); + Session::flash('success', strval(trans('firefly.added_amount_to_piggy', ['amount' => Amount::format($amount, false), 'name' => $piggyBank->name]))); Preferences::mark(); return redirect(route('piggy-banks.index')); } - Log::error('Cannot add ' . $amount . ' because max amount is ' . $maxAmount . ' (left on account is ' . $leftOnAccount . ')'); + Log::error('Cannot add ' . $amount . ' because canAddAmount returned false.'); Session::flash('error', strval(trans('firefly.cannot_add_amount_piggy', ['amount' => Amount::format($amount, false), 'name' => e($piggyBank->name)]))); return redirect(route('piggy-banks.index')); @@ -313,26 +297,24 @@ class PiggyBankController extends Controller */ public function postRemove(Request $request, PiggyBankRepositoryInterface $repository, PiggyBank $piggyBank) { - $amount = strval(round($request->get('amount'), 12)); - - $savedSoFar = $piggyBank->currentRelevantRep()->currentamount; - - if (bccomp($amount, $savedSoFar) <= 0) { - $repetition = $piggyBank->currentRelevantRep(); - $repetition->currentamount = bcsub($repetition->currentamount, $amount); - $repetition->save(); - - // create event - $repository->createEvent($piggyBank, bcmul($amount, '-1')); - + $amount = $request->get('amount'); + if ($repository->canRemoveAmount($piggyBank, $amount)) { + $repository->removeAmount($piggyBank, $amount); Session::flash( - 'success', strval(trans('firefly.removed_amount_from_piggy', ['amount' => Amount::format($amount, false), 'name' => e($piggyBank->name)])) + 'success', strval(trans('firefly.removed_amount_from_piggy', ['amount' => Amount::format($amount, false), 'name' => $piggyBank->name])) ); Preferences::mark(); return redirect(route('piggy-banks.index')); } + + $amount = strval(round($request->get('amount'), 12)); + $savedSoFar = $piggyBank->currentRelevantRep()->currentamount; + + if (bccomp($amount, $savedSoFar) <= 0) { + } + Session::flash('error', strval(trans('firefly.cannot_remove_from_piggy', ['amount' => Amount::format($amount, false), 'name' => e($piggyBank->name)]))); return redirect(route('piggy-banks.index')); @@ -392,9 +374,11 @@ class PiggyBankController extends Controller Preferences::mark(); if (intval($request->get('create_another')) === 1) { + // @codeCoverageIgnoreStart Session::put('piggy-banks.create.fromStore', true); return redirect(route('piggy-banks.create'))->withInput(); + // @codeCoverageIgnoreEnd } return redirect($this->getPreviousUri('piggy-banks.edit.uri')); @@ -416,9 +400,11 @@ class PiggyBankController extends Controller Preferences::mark(); if (intval($request->get('return_to_edit')) === 1) { + // @codeCoverageIgnoreStart Session::put('piggy-banks.edit.fromUpdate', true); return redirect(route('piggy-banks.edit', [$piggyBank->id])); + // @codeCoverageIgnoreEnd } return redirect($this->getPreviousUri('piggy-banks.edit.uri')); diff --git a/app/Http/Controllers/PreferencesController.php b/app/Http/Controllers/PreferencesController.php index 4373d61204..529993a8df 100644 --- a/app/Http/Controllers/PreferencesController.php +++ b/app/Http/Controllers/PreferencesController.php @@ -160,11 +160,10 @@ class PreferencesController extends Controller Preferences::set('showDepositsFrontpage', $showDepositsFrontpage); // save page size: + Preferences::set('transactionPageSize', 50); $transactionPageSize = intval($request->get('transactionPageSize')); if ($transactionPageSize > 0 && $transactionPageSize < 1337) { Preferences::set('transactionPageSize', $transactionPageSize); - } else { - Preferences::set('transactionPageSize', 50); } $twoFactorAuthEnabled = false; diff --git a/app/Repositories/PiggyBank/PiggyBankRepository.php b/app/Repositories/PiggyBank/PiggyBankRepository.php index 96226b81e3..86f05c8832 100644 --- a/app/Repositories/PiggyBank/PiggyBankRepository.php +++ b/app/Repositories/PiggyBank/PiggyBankRepository.php @@ -32,6 +32,54 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface /** @var User */ private $user; + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function addAmount(PiggyBank $piggyBank, string $amount): bool + { + $repetition = $piggyBank->currentRelevantRep(); + $currentAmount = $repetition->currentamount ?? '0'; + $repetition->currentamount = bcadd($currentAmount, $amount); + $repetition->save(); + + // create event + $this->createEvent($piggyBank, $amount); + + return true; + } + + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function canAddAmount(PiggyBank $piggyBank, string $amount): bool + { + $leftOnAccount = $piggyBank->leftOnAccount(new Carbon); + $savedSoFar = strval($piggyBank->currentRelevantRep()->currentamount); + $leftToSave = bcsub($piggyBank->targetamount, $savedSoFar); + $maxAmount = strval(min(round($leftOnAccount, 12), round($leftToSave, 12))); + + return bccomp($amount, $maxAmount) <= 0; + } + + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function canRemoveAmount(PiggyBank $piggyBank, string $amount): bool + { + $savedSoFar = $piggyBank->currentRelevantRep()->currentamount; + + return bccomp($amount, $savedSoFar) <= 0; + } + /** * @param PiggyBank $piggyBank * @param string $amount @@ -119,6 +167,24 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface return $set; } + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function removeAmount(PiggyBank $piggyBank, string $amount): bool + { + $repetition = $piggyBank->currentRelevantRep(); + $repetition->currentamount = bcsub($repetition->currentamount, $amount); + $repetition->save(); + + // create event + $this->createEvent($piggyBank, bcmul($amount, '-1')); + + return true; + } + /** * Set all piggy banks to order 0. * diff --git a/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php b/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php index a2da326d86..69fb317c93 100644 --- a/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php +++ b/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php @@ -26,6 +26,30 @@ use Illuminate\Support\Collection; interface PiggyBankRepositoryInterface { + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function addAmount(PiggyBank $piggyBank, string $amount): bool; + + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function canAddAmount(PiggyBank $piggyBank, string $amount): bool; + + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function canRemoveAmount(PiggyBank $piggyBank, string $amount): bool; + /** * Create a new event. * @@ -82,6 +106,14 @@ interface PiggyBankRepositoryInterface */ public function getPiggyBanksWithAmount(): Collection; + /** + * @param PiggyBank $piggyBank + * @param string $amount + * + * @return bool + */ + public function removeAmount(PiggyBank $piggyBank, string $amount): bool; + /** * Set all piggy banks to order 0. * diff --git a/tests/Feature/Controllers/PiggyBankControllerTest.php b/tests/Feature/Controllers/PiggyBankControllerTest.php index 1004d41efd..a185801ede 100644 --- a/tests/Feature/Controllers/PiggyBankControllerTest.php +++ b/tests/Feature/Controllers/PiggyBankControllerTest.php @@ -78,6 +78,25 @@ class PiggyBankControllerTest extends TestCase $response->assertSee('
    '); } + /** + * @covers \FireflyIII\Http\Controllers\PiggyBankController::create + */ + public function testCreateEmpty() + { + // mock stuff + $accountRepos = $this->mock(AccountRepositoryInterface::class); + $journalRepos = $this->mock(JournalRepositoryInterface::class); + $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); + $accountRepos->shouldReceive('getAccountsByType') + ->withArgs([[AccountType::DEFAULT, AccountType::ASSET]])->andReturn(new Collection())->once(); + + $this->be($this->user()); + $response = $this->get(route('piggy-banks.create')); + $response->assertStatus(302); + $response->assertRedirect(route('new-user.index')); + $response->assertSessionHas('error'); + } + /** * @covers \FireflyIII\Http\Controllers\PiggyBankController::delete */ @@ -180,7 +199,8 @@ class PiggyBankControllerTest extends TestCase $repository = $this->mock(PiggyBankRepositoryInterface::class); $journalRepos = $this->mock(JournalRepositoryInterface::class); $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); - $repository->shouldReceive('createEvent')->once(); + $repository->shouldReceive('canAddAmount')->once()->andReturn(true); + $repository->shouldReceive('addAmount')->once()->andReturn(true); $data = ['amount' => '1.123']; $this->be($this->user()); @@ -191,30 +211,25 @@ class PiggyBankControllerTest extends TestCase } /** - * Add the exact amount to fill a piggy bank + * Add way too much * * @covers \FireflyIII\Http\Controllers\PiggyBankController::postAdd */ - public function testPostAddExact() + public function testPostAddTooMuch() { // mock stuff $repository = $this->mock(PiggyBankRepositoryInterface::class); $journalRepos = $this->mock(JournalRepositoryInterface::class); $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); - $repository->shouldReceive('createEvent')->once(); - - // find a piggy with current amount = 0. - $piggy = PiggyBank::leftJoin('piggy_bank_repetitions', 'piggy_bank_repetitions.piggy_bank_id', '=', 'piggy_banks.id') - ->where('currentamount', 0) - ->first(['piggy_banks.id', 'targetamount']); + $repository->shouldReceive('canAddAmount')->once()->andReturn(false); - $data = ['amount' => strval($piggy->targetamount)]; + $data = ['amount' => '1000']; $this->be($this->user()); - $response = $this->post(route('piggy-banks.add', [$piggy->id]), $data); + $response = $this->post(route('piggy-banks.add', [1]), $data); $response->assertStatus(302); $response->assertRedirect(route('piggy-banks.index')); - $response->assertSessionHas('success'); + $response->assertSessionHas('error'); } /** @@ -226,7 +241,8 @@ class PiggyBankControllerTest extends TestCase $repository = $this->mock(PiggyBankRepositoryInterface::class); $journalRepos = $this->mock(JournalRepositoryInterface::class); $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); - $repository->shouldReceive('createEvent')->once(); + $repository->shouldReceive('canRemoveAmount')->once()->andReturn(true); + $repository->shouldReceive('removeAmount')->once()->andReturn(true); $data = ['amount' => '1.123']; $this->be($this->user()); @@ -236,6 +252,25 @@ class PiggyBankControllerTest extends TestCase $response->assertSessionHas('success'); } + /** + * @covers \FireflyIII\Http\Controllers\PiggyBankController::postRemove + */ + public function testPostRemoveTooMuch() + { + // mock stuff + $repository = $this->mock(PiggyBankRepositoryInterface::class); + $journalRepos = $this->mock(JournalRepositoryInterface::class); + $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); + $repository->shouldReceive('canRemoveAmount')->once()->andReturn(false); + + $data = ['amount' => '1.123']; + $this->be($this->user()); + $response = $this->post(route('piggy-banks.remove', [1]), $data); + $response->assertStatus(302); + $response->assertRedirect(route('piggy-banks.index')); + $response->assertSessionHas('error'); + } + /** * @covers \FireflyIII\Http\Controllers\PiggyBankController::remove */ diff --git a/tests/Feature/Controllers/PreferencesControllerTest.php b/tests/Feature/Controllers/PreferencesControllerTest.php index b64e916edb..bb23853f60 100644 --- a/tests/Feature/Controllers/PreferencesControllerTest.php +++ b/tests/Feature/Controllers/PreferencesControllerTest.php @@ -29,6 +29,7 @@ class PreferencesControllerTest extends TestCase /** * @covers \FireflyIII\Http\Controllers\PreferencesController::code + * @covers \FireflyIII\Http\Controllers\PreferencesController::getDomain */ public function testCode() {