Expand test coverage. Remove else-statement.

2024-12-29 10:21:25 -06:00 · 2017-03-22 17:02:15 +01:00 · 2017-03-22 17:02:15 +01:00 · 6fe319702d
commit 6fe319702d
parent 32c89f9a98
6 changed files with 167 additions and 48 deletions
--- 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'));
--- 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;
--- 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.
     *
--- 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.
     *
--- a/tests/Feature/Controllers/PiggyBankControllerTest.php
+++ b/tests/Feature/Controllers/PiggyBankControllerTest.php
@ -78,6 +78,25 @@ class PiggyBankControllerTest extends TestCase
        $response->assertSee('<ol class="breadcrumb">');
    }

+    /**
+     * @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
     */
--- 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()
    {