From 48fa86cc54b28853345cfd6b42b87b6c9df9123e Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 21 Jan 2018 18:06:57 +0100 Subject: [PATCH] Improve some test coverage. --- app/Handlers/Events/UserEventHandler.php | 14 +- app/Repositories/User/UserRepository.php | 22 +++ .../User/UserRepositoryInterface.php | 17 +++ .../Handlers/Events/UserEventHandlerTest.php | 127 ++++++++++++++++++ tests/Unit/Helpers/AttachmentHelperTest.php | 16 +++ 5 files changed, 189 insertions(+), 7 deletions(-) diff --git a/app/Handlers/Events/UserEventHandler.php b/app/Handlers/Events/UserEventHandler.php index 23f09fbfeb..e89d04aa57 100644 --- a/app/Handlers/Events/UserEventHandler.php +++ b/app/Handlers/Events/UserEventHandler.php @@ -25,11 +25,11 @@ namespace FireflyIII\Handlers\Events; use FireflyIII\Events\RegisteredUser; use FireflyIII\Events\RequestedNewPassword; use FireflyIII\Events\UserChangedEmail; +use FireflyIII\Factories\RoleFactory; use FireflyIII\Mail\ConfirmEmailChangeMail; use FireflyIII\Mail\RegisteredUser as RegisteredUserMail; use FireflyIII\Mail\RequestedNewPassword as RequestedNewPasswordMail; use FireflyIII\Mail\UndoEmailChangeMail; -use FireflyIII\Models\Role; use FireflyIII\Repositories\User\UserRepositoryInterface; use FireflyIII\User; use Illuminate\Auth\Events\Login; @@ -74,11 +74,12 @@ class UserEventHandler */ public function checkSingleUserIsAdmin(Login $event): bool { - Log::debug('In checkSingleUserIsAdmin'); + /** @var UserRepositoryInterface $repository */ + $repository = app(UserRepositoryInterface::class); /** @var User $user */ $user = $event->user; - $count = User::count(); + $count = $repository->count(); if ($count > 1) { // if more than one user, do nothing. @@ -93,17 +94,16 @@ class UserEventHandler return true; } // user is the only user but does not have role "owner". - $role = Role::where('name', 'owner')->first(); + $role = $repository->getRole('owner'); if (is_null($role)) { // create role, does not exist. Very strange situation so let's raise a big fuss about it. - $role = Role::create(['name' => 'owner', 'display_name' => 'Site Owner', 'description' => 'User runs this instance of FF3']); + $role = $repository->createRole('owner', 'Site Owner', 'User runs this instance of FF3'); Log::error('Could not find role "owner". This is weird.'); } Log::info(sprintf('Gave user #%d role #%d ("%s")', $user->id, $role->id, $role->name)); // give user the role - $user->attachRole($role); - $user->save(); + $repository->attachRole($user, 'owner'); return true; } diff --git a/app/Repositories/User/UserRepository.php b/app/Repositories/User/UserRepository.php index 5f7c793f15..95aa678992 100644 --- a/app/Repositories/User/UserRepository.php +++ b/app/Repositories/User/UserRepository.php @@ -128,6 +128,18 @@ class UserRepository implements UserRepositoryInterface return $this->all()->count(); } + /** + * @param string $name + * @param string $displayName + * @param string $description + * + * @return Role + */ + public function createRole(string $name, string $displayName, string $description): Role + { + return Role::create(['name' => $name, 'display_name' => $displayName, 'description' => $description]); + } + /** * @param User $user * @@ -178,6 +190,16 @@ class UserRepository implements UserRepositoryInterface return User::first(); } + /** + * @param string $role + * + * @return Role|null + */ + public function getRole(string $role): ?Role + { + return Role::where('name', $role)->first(); + } + /** * Return basic user information. * diff --git a/app/Repositories/User/UserRepositoryInterface.php b/app/Repositories/User/UserRepositoryInterface.php index f3a76efcd4..13568f754b 100644 --- a/app/Repositories/User/UserRepositoryInterface.php +++ b/app/Repositories/User/UserRepositoryInterface.php @@ -22,6 +22,7 @@ declare(strict_types=1); namespace FireflyIII\Repositories\User; +use FireflyIII\Models\Role; use FireflyIII\User; use Illuminate\Support\Collection; @@ -30,6 +31,15 @@ use Illuminate\Support\Collection; */ interface UserRepositoryInterface { + + /** + * @param string $name + * @param string $displayName + * @param string $description + * + * @return Role + */ + public function createRole(string $name, string $displayName, string $description): Role; /** * Returns a collection of all users. * @@ -37,6 +47,13 @@ interface UserRepositoryInterface */ public function all(): Collection; + /** + * @param string $role + * + * @return Role|null + */ + public function getRole(string $role): ?Role; + /** * Gives a user a role. * diff --git a/tests/Unit/Handlers/Events/UserEventHandlerTest.php b/tests/Unit/Handlers/Events/UserEventHandlerTest.php index 7175df3a69..9cc3d59360 100644 --- a/tests/Unit/Handlers/Events/UserEventHandlerTest.php +++ b/tests/Unit/Handlers/Events/UserEventHandlerTest.php @@ -24,11 +24,17 @@ namespace Tests\Unit\Handlers\Events; use FireflyIII\Events\RegisteredUser; use FireflyIII\Events\RequestedNewPassword; +use FireflyIII\Events\UserChangedEmail; use FireflyIII\Handlers\Events\UserEventHandler; +use FireflyIII\Mail\ConfirmEmailChangeMail; use FireflyIII\Mail\RegisteredUser as RegisteredUserMail; use FireflyIII\Mail\RequestedNewPassword as RequestedNewPasswordMail; +use FireflyIII\Mail\UndoEmailChangeMail; +use FireflyIII\Models\Role; use FireflyIII\Repositories\User\UserRepositoryInterface; +use Illuminate\Auth\Events\Login; use Illuminate\Support\Facades\Mail; +use Mockery; use Tests\TestCase; /** @@ -57,6 +63,127 @@ class UserEventHandlerTest extends TestCase $this->assertTrue(true); } + /** + * @covers \FireflyIII\Handlers\Events\UserEventHandler::checkSingleUserIsAdmin + */ + public function testCheckSingleUserIsAdminMulti() + { + $repository = $this->mock(UserRepositoryInterface::class); + $user = $this->user(); + $event = new Login($user, true); + $listener = new UserEventHandler(); + + // mock stuff + $repository->shouldReceive('count')->once()->andReturn(2); + + + $listener->checkSingleUserIsAdmin($event); + $this->assertTrue(true); + } + + /** + * @covers \FireflyIII\Handlers\Events\UserEventHandler::checkSingleUserIsAdmin + */ + public function testCheckSingleUserIsAdminNoRole() + { + $repository = $this->mock(UserRepositoryInterface::class); + $user = $this->emptyUser(); + $event = new Login($user, true); + $listener = new UserEventHandler(); + + // mock stuff + $repository->shouldReceive('count')->once()->andReturn(1); + $repository->shouldReceive('getRole')->once()->andReturn(null); + $repository->shouldReceive('attachRole')->once()->withArgs([Mockery::any(), 'owner']); + $repository->shouldReceive('createRole')->once()->withArgs(['owner', 'Site Owner', 'User runs this instance of FF3'])->andReturn(new Role); + + $listener->checkSingleUserIsAdmin($event); + $this->assertTrue(true); + } + + /** + * @covers \FireflyIII\Handlers\Events\UserEventHandler::checkSingleUserIsAdmin + */ + public function testCheckSingleUserIsAdminNotAdmin() + { + $repository = $this->mock(UserRepositoryInterface::class); + $user = $this->emptyUser(); + $event = new Login($user, true); + $listener = new UserEventHandler(); + + // mock stuff + $repository->shouldReceive('count')->once()->andReturn(1); + $repository->shouldReceive('getRole')->once()->andReturn(new Role); + $repository->shouldReceive('attachRole')->once()->withArgs([Mockery::any(), 'owner']); + + $listener->checkSingleUserIsAdmin($event); + $this->assertTrue(true); + } + + /** + * @covers \FireflyIII\Handlers\Events\UserEventHandler::checkSingleUserIsAdmin + */ + public function testCheckSingleUserIsAdminSingle() + { + $repository = $this->mock(UserRepositoryInterface::class); + $user = $this->user(); + $event = new Login($user, true); + $listener = new UserEventHandler(); + + // mock stuff + $repository->shouldReceive('count')->once()->andReturn(1); + + $listener->checkSingleUserIsAdmin($event); + $this->assertTrue(true); + } + + /** + * @covers \FireflyIII\Handlers\Events\UserEventHandler::sendEmailChangeConfirmMail + * @covers \FireflyIII\Events\UserChangedEmail + */ + public function testSendEmailChangeConfirmMail() + { + Mail::fake(); + $user = $this->emptyUser(); + $event = new UserChangedEmail($user, 'new@new', 'old@old', '127.0.0.1'); + $listener = new UserEventHandler; + $listener->sendEmailChangeConfirmMail($event); + + // must send user an email: + + Mail::assertSent( + ConfirmEmailChangeMail::class, function ($mail) { + return $mail->hasTo('new@new') && '127.0.0.1' === $mail->ipAddress; + } + ); + + $this->assertTrue(true); + } + + /** + * @covers \FireflyIII\Handlers\Events\UserEventHandler::sendEmailChangeUndoMail + * @covers \FireflyIII\Events\UserChangedEmail + */ + public function testSendEmailChangeUndoMail() + { + Mail::fake(); + $user = $this->emptyUser(); + $event = new UserChangedEmail($user, 'new@new', 'old@old', '127.0.0.1'); + $listener = new UserEventHandler; + $listener->sendEmailChangeUndoMail($event); + + // must send user an email: + + Mail::assertSent( + UndoEmailChangeMail::class, function ($mail) { + return $mail->hasTo('old@old') && '127.0.0.1' === $mail->ipAddress; + } + ); + + $this->assertTrue(true); + } + + /** * @covers \FireflyIII\Handlers\Events\UserEventHandler::sendNewPassword * @covers \FireflyIII\Events\RequestedNewPassword diff --git a/tests/Unit/Helpers/AttachmentHelperTest.php b/tests/Unit/Helpers/AttachmentHelperTest.php index 5f31313eff..f40d677aa2 100644 --- a/tests/Unit/Helpers/AttachmentHelperTest.php +++ b/tests/Unit/Helpers/AttachmentHelperTest.php @@ -112,6 +112,22 @@ class AttachmentHelperTest extends TestCase Storage::disk('upload')->assertExists(sprintf('at-%d.data', $attachments->first()->id)); } + /** + * Test double file upload. Needs to be after testSave. + * + * @covers \FireflyIII\Helpers\Attachments\AttachmentHelper::__construct + * @covers \FireflyIII\Helpers\Attachments\AttachmentHelper::saveAttachmentsForModel + */ + public function testSaveEmpty() + { + $journal = TransactionJournal::first(); + $helper = new AttachmentHelper; + + $res = $helper->saveAttachmentsForModel($journal, null); + $this->assertTrue($res); + } + + /** * Test double file upload. Needs to be after testSave. *