Fixed name retrieval on missing users and added tests to cover along with some test...

diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php
index 46b6bf22ec4abb27cc2017100a1e519d789ee9a5..3f41b2d0e91e6a58a1c3edf36a8a3874f414e795 100644 (file)
--- 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 5122aac77765f36f9911147dafc8c9f70c59d39f..fecd7c88bf8754e0584e6500df69eac6b704b961 100644 (file)
--- a/app/Repos/UserRepo.php
+++ b/app/Repos/UserRepo.php
@@ -46,14 +46,19 @@ class UserRepo
     public function registerNew(array $data)
     {
         $user = $this->create($data);
-        $roleId = \Setting::get('registration-role');
-
-        if ($roleId === false) {
-            $roleId = $this->role->getDefault()->id;
-        }
+        $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);
-        return $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 174c90669e010dbcc8061ad42aef29882d97f711..c048aa9fe4268e50c99448bda2b7402f690b6bf5 100644 (file)
--- 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 a9f02aaed714edc6addd0f6910cdeb5459fbe8e9..9a965ccbc8ada02d488594987d311920d9532e68 100644 (file)
--- a/resources/views/books/show.blade.php
+++ b/resources/views/books/show.blade.php
@@ -58,7 +58,7 @@
                         <p class="text-muted small">
                             Created {{$book->created_at->diffForHumans()}} @if($book->createdBy) by {{$book->createdBy->name}} @endif
                             <br>
-                            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
                         </p>
                     </div>
                 </div>

diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php
index 75ae6bf65d938dea6914c35db589b3ec2b128ed4..9421bbe18bcce6668a108c31865841db576d27cb 100644 (file)
--- a/resources/views/chapters/show.blade.php
+++ b/resources/views/chapters/show.blade.php
@@ -56,7 +56,7 @@
                 <p class="text-muted small">
                     Created {{$chapter->created_at->diffForHumans()}} @if($chapter->createdBy) by {{$chapter->createdBy->name}} @endif
                     <br>
-                    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
                 </p>
             </div>
             <div class="col-md-3 col-md-offset-1">

diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php
index db5e5168167fbd8de2e0656c2a5999946c72d564..d60ee003484a659831159455bd9bab0214891c37 100644 (file)
--- a/resources/views/pages/show.blade.php
+++ b/resources/views/pages/show.blade.php
@@ -53,7 +53,7 @@
                     <p class="text-muted small">
                         Created {{$page->created_at->diffForHumans()}} @if($page->createdBy) by {{$page->createdBy->name}} @endif
                         <br>
-                        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
                     </p>
 
                 </div>

diff --git a/resources/views/partials/activity-item.blade.php b/resources/views/partials/activity-item.blade.php
index 78168ea9653ca35973acd63c905f9d25d2a7afa8..2302eab6871c2aa7abb6f12bc2819584e6e1cb22 100644 (file)
--- a/resources/views/partials/activity-item.blade.php
+++ b/resources/views/partials/activity-item.blade.php
@@ -10,6 +10,8 @@
 <div class="right">
     @if($activity->user)
         {{$activity->user->name}}
+    @else
+        A deleted user
     @endif
 
     {{ $activity->getText() }}

diff --git a/tests/EntityTest.php b/tests/EntityTest.php
index 7cc2d640f77250715f0049579a77c290e6a9cc10..02924c1231a5304e5cf679f68b4a707f53071be4 100644 (file)
--- 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 3a0967c8e626e4867021e53b22718776d7a323ba..24685321fcf84bac451c9e91fa1d96b8e2e59f92 100644 (file)
--- 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.
      *