Fixed name retrieval on missing users and added tests to cover along with some test helper methods
Showing
9 changed files
with
88 additions
and
12 deletions
| ... | @@ -159,16 +159,14 @@ class UserController extends Controller | ... | @@ -159,16 +159,14 @@ class UserController extends Controller |
| 159 | $this->checkPermissionOr('user-delete', function () use ($id) { | 159 | $this->checkPermissionOr('user-delete', function () use ($id) { |
| 160 | return $this->currentUser->id == $id; | 160 | return $this->currentUser->id == $id; |
| 161 | }); | 161 | }); |
| 162 | - $user = $this->userRepo->getById($id); | ||
| 163 | 162 | ||
| 164 | - // Delete social accounts | 163 | + $user = $this->userRepo->getById($id); |
| 165 | if ($this->userRepo->isOnlyAdmin($user)) { | 164 | if ($this->userRepo->isOnlyAdmin($user)) { |
| 166 | session()->flash('error', 'You cannot delete the only admin'); | 165 | session()->flash('error', 'You cannot delete the only admin'); |
| 167 | return redirect($user->getEditUrl()); | 166 | return redirect($user->getEditUrl()); |
| 168 | } | 167 | } |
| 168 | + $this->userRepo->destroy($user); | ||
| 169 | 169 | ||
| 170 | - $user->socialAccounts()->delete(); | ||
| 171 | - $user->delete(); | ||
| 172 | return redirect('/users'); | 170 | return redirect('/users'); |
| 173 | } | 171 | } |
| 174 | } | 172 | } | ... | ... |
| ... | @@ -46,14 +46,19 @@ class UserRepo | ... | @@ -46,14 +46,19 @@ class UserRepo |
| 46 | public function registerNew(array $data) | 46 | public function registerNew(array $data) |
| 47 | { | 47 | { |
| 48 | $user = $this->create($data); | 48 | $user = $this->create($data); |
| 49 | - $roleId = \Setting::get('registration-role'); | 49 | + $this->attachDefaultRole($user); |
| 50 | - | 50 | + return $user; |
| 51 | - if ($roleId === false) { | ||
| 52 | - $roleId = $this->role->getDefault()->id; | ||
| 53 | } | 51 | } |
| 54 | 52 | ||
| 53 | + /** | ||
| 54 | + * Give a user the default role. Used when creating a new user. | ||
| 55 | + * @param $user | ||
| 56 | + */ | ||
| 57 | + public function attachDefaultRole($user) | ||
| 58 | + { | ||
| 59 | + $roleId = \Setting::get('registration-role'); | ||
| 60 | + if ($roleId === false) $roleId = $this->role->getDefault()->id; | ||
| 55 | $user->attachRoleId($roleId); | 61 | $user->attachRoleId($roleId); |
| 56 | - return $user; | ||
| 57 | } | 62 | } |
| 58 | 63 | ||
| 59 | /** | 64 | /** |
| ... | @@ -88,4 +93,14 @@ class UserRepo | ... | @@ -88,4 +93,14 @@ class UserRepo |
| 88 | 'password' => bcrypt($data['password']) | 93 | 'password' => bcrypt($data['password']) |
| 89 | ]); | 94 | ]); |
| 90 | } | 95 | } |
| 96 | + | ||
| 97 | + /** | ||
| 98 | + * Remove the given user from storage, Delete all related content. | ||
| 99 | + * @param User $user | ||
| 100 | + */ | ||
| 101 | + public function destroy(User $user) | ||
| 102 | + { | ||
| 103 | + $user->socialAccounts()->delete(); | ||
| 104 | + $user->delete(); | ||
| 105 | + } | ||
| 91 | } | 106 | } |
| ... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
| ... | @@ -32,6 +32,8 @@ body.dragging, body.dragging * { | ... | @@ -32,6 +32,8 @@ body.dragging, body.dragging * { |
| 32 | .avatar { | 32 | .avatar { |
| 33 | border-radius: 100%; | 33 | border-radius: 100%; |
| 34 | background-color: #EEE; | 34 | background-color: #EEE; |
| 35 | + width: 30px; | ||
| 36 | + height: 30px; | ||
| 35 | &.med { | 37 | &.med { |
| 36 | width: 40px; | 38 | width: 40px; |
| 37 | height: 40px; | 39 | height: 40px; | ... | ... |
| ... | @@ -58,7 +58,7 @@ | ... | @@ -58,7 +58,7 @@ |
| 58 | <p class="text-muted small"> | 58 | <p class="text-muted small"> |
| 59 | Created {{$book->created_at->diffForHumans()}} @if($book->createdBy) by {{$book->createdBy->name}} @endif | 59 | Created {{$book->created_at->diffForHumans()}} @if($book->createdBy) by {{$book->createdBy->name}} @endif |
| 60 | <br> | 60 | <br> |
| 61 | - Last Updated {{$book->updated_at->diffForHumans()}} @if($book->createdBy) by {{$book->updatedBy->name}} @endif | 61 | + Last Updated {{$book->updated_at->diffForHumans()}} @if($book->updatedBy) by {{$book->updatedBy->name}} @endif |
| 62 | </p> | 62 | </p> |
| 63 | </div> | 63 | </div> |
| 64 | </div> | 64 | </div> | ... | ... |
| ... | @@ -56,7 +56,7 @@ | ... | @@ -56,7 +56,7 @@ |
| 56 | <p class="text-muted small"> | 56 | <p class="text-muted small"> |
| 57 | Created {{$chapter->created_at->diffForHumans()}} @if($chapter->createdBy) by {{$chapter->createdBy->name}} @endif | 57 | Created {{$chapter->created_at->diffForHumans()}} @if($chapter->createdBy) by {{$chapter->createdBy->name}} @endif |
| 58 | <br> | 58 | <br> |
| 59 | - Last Updated {{$chapter->updated_at->diffForHumans()}} @if($chapter->createdBy) by {{$chapter->updatedBy->name}} @endif | 59 | + Last Updated {{$chapter->updated_at->diffForHumans()}} @if($chapter->updatedBy) by {{$chapter->updatedBy->name}} @endif |
| 60 | </p> | 60 | </p> |
| 61 | </div> | 61 | </div> |
| 62 | <div class="col-md-3 col-md-offset-1"> | 62 | <div class="col-md-3 col-md-offset-1"> | ... | ... |
| ... | @@ -53,7 +53,7 @@ | ... | @@ -53,7 +53,7 @@ |
| 53 | <p class="text-muted small"> | 53 | <p class="text-muted small"> |
| 54 | Created {{$page->created_at->diffForHumans()}} @if($page->createdBy) by {{$page->createdBy->name}} @endif | 54 | Created {{$page->created_at->diffForHumans()}} @if($page->createdBy) by {{$page->createdBy->name}} @endif |
| 55 | <br> | 55 | <br> |
| 56 | - Last Updated {{$page->updated_at->diffForHumans()}} @if($page->createdBy) by {{$page->updatedBy->name}} @endif | 56 | + Last Updated {{$page->updated_at->diffForHumans()}} @if($page->updatedBy) by {{$page->updatedBy->name}} @endif |
| 57 | </p> | 57 | </p> |
| 58 | 58 | ||
| 59 | </div> | 59 | </div> | ... | ... |
| ... | @@ -10,6 +10,8 @@ | ... | @@ -10,6 +10,8 @@ |
| 10 | <div class="right"> | 10 | <div class="right"> |
| 11 | @if($activity->user) | 11 | @if($activity->user) |
| 12 | {{$activity->user->name}} | 12 | {{$activity->user->name}} |
| 13 | + @else | ||
| 14 | + A deleted user | ||
| 13 | @endif | 15 | @endif |
| 14 | 16 | ||
| 15 | {{ $activity->getText() }} | 17 | {{ $activity->getText() }} | ... | ... |
| ... | @@ -171,4 +171,29 @@ class EntityTest extends TestCase | ... | @@ -171,4 +171,29 @@ class EntityTest extends TestCase |
| 171 | } | 171 | } |
| 172 | 172 | ||
| 173 | 173 | ||
| 174 | + public function testEntitiesViewableAfterCreatorDeletion() | ||
| 175 | + { | ||
| 176 | + $creator = $this->getNewUser(); | ||
| 177 | + $updater = $this->getNewUser(); | ||
| 178 | + $entities = $this->createEntityChainBelongingToUser($creator, $updater); | ||
| 179 | + app('BookStack\Repos\UserRepo')->destroy($creator); | ||
| 180 | + | ||
| 181 | + $this->asAdmin()->visit($entities['book']->getUrl())->seeStatusCode(200) | ||
| 182 | + ->visit($entities['chapter']->getUrl())->seeStatusCode(200) | ||
| 183 | + ->visit($entities['page']->getUrl())->seeStatusCode(200); | ||
| 184 | + } | ||
| 185 | + | ||
| 186 | + public function testEntitiesViewableAfterUpdaterDeletion() | ||
| 187 | + { | ||
| 188 | + $creator = $this->getNewUser(); | ||
| 189 | + $updater = $this->getNewUser(); | ||
| 190 | + $entities = $this->createEntityChainBelongingToUser($creator, $updater); | ||
| 191 | + app('BookStack\Repos\UserRepo')->destroy($updater); | ||
| 192 | + | ||
| 193 | + $this->asAdmin()->visit($entities['book']->getUrl())->seeStatusCode(200) | ||
| 194 | + ->visit($entities['chapter']->getUrl())->seeStatusCode(200) | ||
| 195 | + ->visit($entities['page']->getUrl())->seeStatusCode(200); | ||
| 196 | + } | ||
| 197 | + | ||
| 198 | + | ||
| 174 | } | 199 | } | ... | ... |
| ... | @@ -50,6 +50,40 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase | ... | @@ -50,6 +50,40 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase |
| 50 | } | 50 | } |
| 51 | 51 | ||
| 52 | /** | 52 | /** |
| 53 | + * Create a group of entities that belong to a specific user. | ||
| 54 | + * @param $creatorUser | ||
| 55 | + * @param $updaterUser | ||
| 56 | + * @return array | ||
| 57 | + */ | ||
| 58 | + protected function createEntityChainBelongingToUser($creatorUser, $updaterUser = false) | ||
| 59 | + { | ||
| 60 | + if ($updaterUser === false) $updaterUser = $creatorUser; | ||
| 61 | + $book = factory(BookStack\Book::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id]); | ||
| 62 | + $chapter = factory(BookStack\Chapter::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id]); | ||
| 63 | + $page = factory(BookStack\Page::class)->create(['created_by' => $creatorUser->id, 'updated_by' => $updaterUser->id, 'book_id' => $book->id]); | ||
| 64 | + $book->chapters()->saveMany([$chapter]); | ||
| 65 | + $chapter->pages()->saveMany([$page]); | ||
| 66 | + return [ | ||
| 67 | + 'book' => $book, | ||
| 68 | + 'chapter' => $chapter, | ||
| 69 | + 'page' => $page | ||
| 70 | + ]; | ||
| 71 | + } | ||
| 72 | + | ||
| 73 | + /** | ||
| 74 | + * Quick way to create a new user | ||
| 75 | + * @param array $attributes | ||
| 76 | + * @return mixed | ||
| 77 | + */ | ||
| 78 | + protected function getNewUser($attributes = []) | ||
| 79 | + { | ||
| 80 | + $user = factory(\BookStack\User::class)->create($attributes); | ||
| 81 | + $userRepo = app('BookStack\Repos\UserRepo'); | ||
| 82 | + $userRepo->attachDefaultRole($user); | ||
| 83 | + return $user; | ||
| 84 | + } | ||
| 85 | + | ||
| 86 | + /** | ||
| 53 | * Assert that a given string is seen inside an element. | 87 | * Assert that a given string is seen inside an element. |
| 54 | * | 88 | * |
| 55 | * @param bool|string|null $element | 89 | * @param bool|string|null $element | ... | ... |
-
Please register or sign in to post a comment