Fixed error on image deletion
Also Added tests to cover image upload and deletion. Fixes #136.
Showing
7 changed files
with
123 additions
and
12 deletions
| ... | @@ -53,7 +53,7 @@ class ImageController extends Controller | ... | @@ -53,7 +53,7 @@ class ImageController extends Controller |
| 53 | ]); | 53 | ]); |
| 54 | 54 | ||
| 55 | $searchTerm = $request->get('term'); | 55 | $searchTerm = $request->get('term'); |
| 56 | - $imgData = $this->imageRepo->searchPaginatedByType($type, $page,24, $searchTerm); | 56 | + $imgData = $this->imageRepo->searchPaginatedByType($type, $page, 24, $searchTerm); |
| 57 | return response()->json($imgData); | 57 | return response()->json($imgData); |
| 58 | } | 58 | } |
| 59 | 59 | ||
| ... | @@ -99,7 +99,7 @@ class ImageController extends Controller | ... | @@ -99,7 +99,7 @@ class ImageController extends Controller |
| 99 | { | 99 | { |
| 100 | $this->checkPermission('image-create-all'); | 100 | $this->checkPermission('image-create-all'); |
| 101 | $this->validate($request, [ | 101 | $this->validate($request, [ |
| 102 | - 'file' => 'image|mimes:jpeg,gif,png' | 102 | + 'file' => 'is_image' |
| 103 | ]); | 103 | ]); |
| 104 | 104 | ||
| 105 | $imageUpload = $request->file('file'); | 105 | $imageUpload = $request->file('file'); | ... | ... |
| ... | @@ -15,7 +15,12 @@ class AppServiceProvider extends ServiceProvider | ... | @@ -15,7 +15,12 @@ class AppServiceProvider extends ServiceProvider |
| 15 | */ | 15 | */ |
| 16 | public function boot() | 16 | public function boot() |
| 17 | { | 17 | { |
| 18 | - // | 18 | + // Custom validation methods |
| 19 | + \Validator::extend('is_image', function($attribute, $value, $parameters, $validator) { | ||
| 20 | + $imageMimes = ['image/png', 'image/bmp', 'image/gif', 'image/jpeg', 'image/jpg', 'image/tiff', 'image/webp']; | ||
| 21 | + return in_array($value->getMimeType(), $imageMimes); | ||
| 22 | + }); | ||
| 23 | + | ||
| 19 | } | 24 | } |
| 20 | 25 | ||
| 21 | /** | 26 | /** | ... | ... |
| ... | @@ -4,6 +4,7 @@ use BookStack\Book; | ... | @@ -4,6 +4,7 @@ use BookStack\Book; |
| 4 | use BookStack\Chapter; | 4 | use BookStack\Chapter; |
| 5 | use BookStack\Entity; | 5 | use BookStack\Entity; |
| 6 | use BookStack\JointPermission; | 6 | use BookStack\JointPermission; |
| 7 | +use BookStack\Ownable; | ||
| 7 | use BookStack\Page; | 8 | use BookStack\Page; |
| 8 | use BookStack\Role; | 9 | use BookStack\Role; |
| 9 | use BookStack\User; | 10 | use BookStack\User; |
| ... | @@ -307,16 +308,16 @@ class PermissionService | ... | @@ -307,16 +308,16 @@ class PermissionService |
| 307 | 308 | ||
| 308 | /** | 309 | /** |
| 309 | * Checks if an entity has a restriction set upon it. | 310 | * Checks if an entity has a restriction set upon it. |
| 310 | - * @param Entity $entity | 311 | + * @param Ownable $ownable |
| 311 | * @param $permission | 312 | * @param $permission |
| 312 | * @return bool | 313 | * @return bool |
| 313 | */ | 314 | */ |
| 314 | - public function checkEntityUserAccess(Entity $entity, $permission) | 315 | + public function checkOwnableUserAccess(Ownable $ownable, $permission) |
| 315 | { | 316 | { |
| 316 | if ($this->isAdmin) return true; | 317 | if ($this->isAdmin) return true; |
| 317 | $explodedPermission = explode('-', $permission); | 318 | $explodedPermission = explode('-', $permission); |
| 318 | 319 | ||
| 319 | - $baseQuery = $entity->where('id', '=', $entity->id); | 320 | + $baseQuery = $ownable->where('id', '=', $ownable->id); |
| 320 | $action = end($explodedPermission); | 321 | $action = end($explodedPermission); |
| 321 | $this->currentAction = $action; | 322 | $this->currentAction = $action; |
| 322 | 323 | ||
| ... | @@ -327,7 +328,7 @@ class PermissionService | ... | @@ -327,7 +328,7 @@ class PermissionService |
| 327 | $allPermission = $this->currentUser && $this->currentUser->can($permission . '-all'); | 328 | $allPermission = $this->currentUser && $this->currentUser->can($permission . '-all'); |
| 328 | $ownPermission = $this->currentUser && $this->currentUser->can($permission . '-own'); | 329 | $ownPermission = $this->currentUser && $this->currentUser->can($permission . '-own'); |
| 329 | $this->currentAction = 'view'; | 330 | $this->currentAction = 'view'; |
| 330 | - $isOwner = $this->currentUser && $this->currentUser->id === $entity->created_by; | 331 | + $isOwner = $this->currentUser && $this->currentUser->id === $ownable->created_by; |
| 331 | return ($allPermission || ($isOwner && $ownPermission)); | 332 | return ($allPermission || ($isOwner && $ownPermission)); |
| 332 | } | 333 | } |
| 333 | 334 | ... | ... |
| 1 | <?php | 1 | <?php |
| 2 | 2 | ||
| 3 | +use BookStack\Ownable; | ||
| 4 | + | ||
| 3 | if (!function_exists('versioned_asset')) { | 5 | if (!function_exists('versioned_asset')) { |
| 4 | /** | 6 | /** |
| 5 | * Get the path to a versioned file. | 7 | * Get the path to a versioned file. |
| ... | @@ -34,18 +36,18 @@ if (!function_exists('versioned_asset')) { | ... | @@ -34,18 +36,18 @@ if (!function_exists('versioned_asset')) { |
| 34 | * If an ownable element is passed in the jointPermissions are checked against | 36 | * If an ownable element is passed in the jointPermissions are checked against |
| 35 | * that particular item. | 37 | * that particular item. |
| 36 | * @param $permission | 38 | * @param $permission |
| 37 | - * @param \BookStack\Ownable $ownable | 39 | + * @param Ownable $ownable |
| 38 | * @return mixed | 40 | * @return mixed |
| 39 | */ | 41 | */ |
| 40 | -function userCan($permission, \BookStack\Ownable $ownable = null) | 42 | +function userCan($permission, Ownable $ownable = null) |
| 41 | { | 43 | { |
| 42 | if ($ownable === null) { | 44 | if ($ownable === null) { |
| 43 | return auth()->user() && auth()->user()->can($permission); | 45 | return auth()->user() && auth()->user()->can($permission); |
| 44 | } | 46 | } |
| 45 | 47 | ||
| 46 | // Check permission on ownable item | 48 | // Check permission on ownable item |
| 47 | - $permissionService = app('BookStack\Services\PermissionService'); | 49 | + $permissionService = app(\BookStack\Services\PermissionService::class); |
| 48 | - return $permissionService->checkEntityUserAccess($ownable, $permission); | 50 | + return $permissionService->checkOwnableUserAccess($ownable, $permission); |
| 49 | } | 51 | } |
| 50 | 52 | ||
| 51 | /** | 53 | /** | ... | ... |
tests/ImageTest.php
0 → 100644
| 1 | +<?php | ||
| 2 | + | ||
| 3 | +class ImageTest extends TestCase | ||
| 4 | +{ | ||
| 5 | + | ||
| 6 | + /** | ||
| 7 | + * Get a test image that can be uploaded | ||
| 8 | + * @param $fileName | ||
| 9 | + * @return \Illuminate\Http\UploadedFile | ||
| 10 | + */ | ||
| 11 | + protected function getTestImage($fileName) | ||
| 12 | + { | ||
| 13 | + return new \Illuminate\Http\UploadedFile(base_path('tests/test-image.jpg'), $fileName, 'image/jpeg', 5238); | ||
| 14 | + } | ||
| 15 | + | ||
| 16 | + /** | ||
| 17 | + * Get the path for a test image. | ||
| 18 | + * @param $type | ||
| 19 | + * @param $fileName | ||
| 20 | + * @return string | ||
| 21 | + */ | ||
| 22 | + protected function getTestImagePath($type, $fileName) | ||
| 23 | + { | ||
| 24 | + return '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/' . $fileName; | ||
| 25 | + } | ||
| 26 | + | ||
| 27 | + /** | ||
| 28 | + * Uploads an image with the given name. | ||
| 29 | + * @param $name | ||
| 30 | + * @param int $uploadedTo | ||
| 31 | + * @return string | ||
| 32 | + */ | ||
| 33 | + protected function uploadImage($name, $uploadedTo = 0) | ||
| 34 | + { | ||
| 35 | + $file = $this->getTestImage($name); | ||
| 36 | + $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []); | ||
| 37 | + return $this->getTestImagePath('gallery', $name); | ||
| 38 | + } | ||
| 39 | + | ||
| 40 | + /** | ||
| 41 | + * Delete an uploaded image. | ||
| 42 | + * @param $relPath | ||
| 43 | + */ | ||
| 44 | + protected function deleteImage($relPath) | ||
| 45 | + { | ||
| 46 | + unlink(public_path($relPath)); | ||
| 47 | + } | ||
| 48 | + | ||
| 49 | + | ||
| 50 | + public function test_image_upload() | ||
| 51 | + { | ||
| 52 | + $page = \BookStack\Page::first(); | ||
| 53 | + $this->asAdmin(); | ||
| 54 | + $admin = $this->getAdmin(); | ||
| 55 | + $imageName = 'first-image.jpg'; | ||
| 56 | + | ||
| 57 | + $relPath = $this->uploadImage($imageName, $page->id); | ||
| 58 | + $this->assertResponseOk(); | ||
| 59 | + | ||
| 60 | + $this->assertTrue(file_exists(public_path($relPath)), 'Uploaded image exists'); | ||
| 61 | + | ||
| 62 | + $this->seeInDatabase('images', [ | ||
| 63 | + 'url' => $relPath, | ||
| 64 | + 'type' => 'gallery', | ||
| 65 | + 'uploaded_to' => $page->id, | ||
| 66 | + 'path' => $relPath, | ||
| 67 | + 'created_by' => $admin->id, | ||
| 68 | + 'updated_by' => $admin->id, | ||
| 69 | + 'name' => $imageName | ||
| 70 | + ]); | ||
| 71 | + | ||
| 72 | + $this->deleteImage($relPath); | ||
| 73 | + } | ||
| 74 | + | ||
| 75 | + public function test_image_delete() | ||
| 76 | + { | ||
| 77 | + $page = \BookStack\Page::first(); | ||
| 78 | + $this->asAdmin(); | ||
| 79 | + $imageName = 'first-image.jpg'; | ||
| 80 | + | ||
| 81 | + $relPath = $this->uploadImage($imageName, $page->id); | ||
| 82 | + $image = \BookStack\Image::first(); | ||
| 83 | + | ||
| 84 | + $this->call('DELETE', '/images/' . $image->id); | ||
| 85 | + $this->assertResponseOk(); | ||
| 86 | + | ||
| 87 | + $this->dontSeeInDatabase('images', [ | ||
| 88 | + 'url' => $relPath, | ||
| 89 | + 'type' => 'gallery' | ||
| 90 | + ]); | ||
| 91 | + | ||
| 92 | + $this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has been deleted'); | ||
| 93 | + } | ||
| 94 | + | ||
| 95 | +} | ||
| ... | \ No newline at end of file | ... | \ No newline at end of file |
| ... | @@ -39,11 +39,19 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase | ... | @@ -39,11 +39,19 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase |
| 39 | */ | 39 | */ |
| 40 | public function asAdmin() | 40 | public function asAdmin() |
| 41 | { | 41 | { |
| 42 | + return $this->actingAs($this->getAdmin()); | ||
| 43 | + } | ||
| 44 | + | ||
| 45 | + /** | ||
| 46 | + * Get the current admin user. | ||
| 47 | + * @return mixed | ||
| 48 | + */ | ||
| 49 | + public function getAdmin() { | ||
| 42 | if($this->admin === null) { | 50 | if($this->admin === null) { |
| 43 | $adminRole = \BookStack\Role::getRole('admin'); | 51 | $adminRole = \BookStack\Role::getRole('admin'); |
| 44 | $this->admin = $adminRole->users->first(); | 52 | $this->admin = $adminRole->users->first(); |
| 45 | } | 53 | } |
| 46 | - return $this->actingAs($this->admin); | 54 | + return $this->admin; |
| 47 | } | 55 | } |
| 48 | 56 | ||
| 49 | /** | 57 | /** | ... | ... |
tests/test-image.jpg
0 → 100644
5.12 KB
-
Please register or sign in to post a comment