From af331563697b30912cffdfbb6691f6d6ace03312 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 15 Dec 2015 19:27:36 +0000 Subject: [PATCH] Fixed name retrieval on missing users and added tests to cover along with some test helper methods --- app/Http/Controllers/UserController.php | 6 ++-- app/Repos/UserRepo.php | 29 ++++++++++++---- resources/assets/sass/styles.scss | 2 ++ resources/views/books/show.blade.php | 2 +- resources/views/chapters/show.blade.php | 2 +- resources/views/pages/show.blade.php | 2 +- .../views/partials/activity-item.blade.php | 2 ++ tests/EntityTest.php | 25 ++++++++++++++ tests/TestCase.php | 34 +++++++++++++++++++ 9 files changed, 90 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 46b6bf22e..3f41b2d0e 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -159,16 +159,14 @@ class UserController extends Controller $this->checkPermissionOr('user-delete', function () use ($id) { return $this->currentUser->id == $id; }); - $user = $this->userRepo->getById($id); - // Delete social accounts + $user = $this->userRepo->getById($id); if ($this->userRepo->isOnlyAdmin($user)) { session()->flash('error', 'You cannot delete the only admin'); return redirect($user->getEditUrl()); } + $this->userRepo->destroy($user); - $user->socialAccounts()->delete(); - $user->delete(); return redirect('/users'); } } diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php index 5122aac77..fecd7c88b 100644 --- a/app/Repos/UserRepo.php +++ b/app/Repos/UserRepo.php @@ -46,16 +46,21 @@ class UserRepo public function registerNew(array $data) { $user = $this->create($data); - $roleId = \Setting::get('registration-role'); - - if ($roleId === false) { - $roleId = $this->role->getDefault()->id; - } - - $user->attachRoleId($roleId); + $this->attachDefaultRole($user); return $user; } + /** + * Give a user the default role. Used when creating a new user. + * @param $user + */ + public function attachDefaultRole($user) + { + $roleId = \Setting::get('registration-role'); + if ($roleId === false) $roleId = $this->role->getDefault()->id; + $user->attachRoleId($roleId); + } + /** * Checks if the give user is the only admin. * @param User $user @@ -88,4 +93,14 @@ class UserRepo 'password' => bcrypt($data['password']) ]); } + + /** + * Remove the given user from storage, Delete all related content. + * @param User $user + */ + public function destroy(User $user) + { + $user->socialAccounts()->delete(); + $user->delete(); + } } \ No newline at end of file diff --git a/resources/assets/sass/styles.scss b/resources/assets/sass/styles.scss index 174c90669..c048aa9fe 100644 --- a/resources/assets/sass/styles.scss +++ b/resources/assets/sass/styles.scss @@ -32,6 +32,8 @@ body.dragging, body.dragging * { .avatar { border-radius: 100%; background-color: #EEE; + width: 30px; + height: 30px; &.med { width: 40px; height: 40px; diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index a9f02aaed..9a965ccbc 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -58,7 +58,7 @@

Created {{$book->created_at->diffForHumans()}} @if($book->createdBy) by {{$book->createdBy->name}} @endif
- Last Updated {{$book->updated_at->diffForHumans()}} @if($book->createdBy) by {{$book->updatedBy->name}} @endif + Last Updated {{$book->updated_at->diffForHumans()}} @if($book->updatedBy) by {{$book->updatedBy->name}} @endif

diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index 75ae6bf65..9421bbe18 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -56,7 +56,7 @@

Created {{$chapter->created_at->diffForHumans()}} @if($chapter->createdBy) by {{$chapter->createdBy->name}} @endif
- Last Updated {{$chapter->updated_at->diffForHumans()}} @if($chapter->createdBy) by {{$chapter->updatedBy->name}} @endif + Last Updated {{$chapter->updated_at->diffForHumans()}} @if($chapter->updatedBy) by {{$chapter->updatedBy->name}} @endif

diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index db5e51681..d60ee0034 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -53,7 +53,7 @@

Created {{$page->created_at->diffForHumans()}} @if($page->createdBy) by {{$page->createdBy->name}} @endif
- Last Updated {{$page->updated_at->diffForHumans()}} @if($page->createdBy) by {{$page->updatedBy->name}} @endif + Last Updated {{$page->updated_at->diffForHumans()}} @if($page->updatedBy) by {{$page->updatedBy->name}} @endif

diff --git a/resources/views/partials/activity-item.blade.php b/resources/views/partials/activity-item.blade.php index 78168ea96..2302eab68 100644 --- a/resources/views/partials/activity-item.blade.php +++ b/resources/views/partials/activity-item.blade.php @@ -10,6 +10,8 @@
@if($activity->user) {{$activity->user->name}} + @else + A deleted user @endif {{ $activity->getText() }} diff --git a/tests/EntityTest.php b/tests/EntityTest.php index 7cc2d640f..02924c123 100644 --- a/tests/EntityTest.php +++ b/tests/EntityTest.php @@ -171,4 +171,29 @@ class EntityTest extends TestCase } + public function testEntitiesViewableAfterCreatorDeletion() + { + $creator = $this->getNewUser(); + $updater = $this->getNewUser(); + $entities = $this->createEntityChainBelongingToUser($creator, $updater); + app('BookStack\Repos\UserRepo')->destroy($creator); + + $this->asAdmin()->visit($entities['book']->getUrl())->seeStatusCode(200) + ->visit($entities['chapter']->getUrl())->seeStatusCode(200) + ->visit($entities['page']->getUrl())->seeStatusCode(200); + } + + public function testEntitiesViewableAfterUpdaterDeletion() + { + $creator = $this->getNewUser(); + $updater = $this->getNewUser(); + $entities = $this->createEntityChainBelongingToUser($creator, $updater); + app('BookStack\Repos\UserRepo')->destroy($updater); + + $this->asAdmin()->visit($entities['book']->getUrl())->seeStatusCode(200) + ->visit($entities['chapter']->getUrl())->seeStatusCode(200) + ->visit($entities['page']->getUrl())->seeStatusCode(200); + } + + } diff --git a/tests/TestCase.php b/tests/TestCase.php index 3a0967c8e..24685321f 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -49,6 +49,40 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase } } + /** + * Create a group of entities that belong to a specific user. + * @param $creatorUser + * @param $updaterUser + * @return array + */ + protected function createEntityChainBelongingToUser($creatorUser, $updaterUser = false) + { + if ($updaterUser === false) $updaterUser = $creatorUser; + $book = factory(BookStack\Book::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id]); + $chapter = factory(BookStack\Chapter::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id]); + $page = factory(BookStack\Page::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id, 'book_id' => $book->id]); + $book->chapters()->saveMany([$chapter]); + $chapter->pages()->saveMany([$page]); + return [ + 'book' => $book, + 'chapter' => $chapter, + 'page' => $page + ]; + } + + /** + * Quick way to create a new user + * @param array $attributes + * @return mixed + */ + protected function getNewUser($attributes = []) + { + $user = factory(\BookStack\User::class)->create($attributes); + $userRepo = app('BookStack\Repos\UserRepo'); + $userRepo->attachDefaultRole($user); + return $user; + } + /** * Assert that a given string is seen inside an element. *