Fixed error on image deletion

Also Added tests to cover image upload and deletion.
Fixes #136.

diff --git a/app/Http/Controllers/ImageController.php b/app/Http/Controllers/ImageController.php
index 2e5d5f3034180dd3b2d6f40483a664ff677f6f6c..621c23e85d576e1cf4a777c80eb07c1dc5b55ee6 100644 (file)
--- a/app/Http/Controllers/ImageController.php
+++ b/app/Http/Controllers/ImageController.php
@@ -51,9 +51,9 @@ class ImageController extends Controller
         $this->validate($request, [
             'term' => 'required|string'
         ]);
-        
+
         $searchTerm = $request->get('term');
-        $imgData = $this->imageRepo->searchPaginatedByType($type, $page,24, $searchTerm);
+        $imgData = $this->imageRepo->searchPaginatedByType($type, $page, 24, $searchTerm);
         return response()->json($imgData);
     }
 
@@ -99,7 +99,7 @@ class ImageController extends Controller
     {
         $this->checkPermission('image-create-all');
         $this->validate($request, [
-            'file' => 'image|mimes:jpeg,gif,png'
+            'file' => 'is_image'
         ]);
 
         $imageUpload = $request->file('file');

diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php
index 8bcbcbdad2c800cd75ea263a2a6487e0af2a8ee9..f214c9141aa2b5835de735b8baf8e6f01b0ff6b5 100644 (file)
--- a/app/Providers/AppServiceProvider.php
+++ b/app/Providers/AppServiceProvider.php
@@ -15,7 +15,12 @@ class AppServiceProvider extends ServiceProvider
      */
     public function boot()
     {
-        //
+        // Custom validation methods
+        \Validator::extend('is_image', function($attribute, $value, $parameters, $validator) {
+            $imageMimes = ['image/png', 'image/bmp', 'image/gif', 'image/jpeg', 'image/jpg', 'image/tiff', 'image/webp'];
+            return in_array($value->getMimeType(), $imageMimes);
+        });
+
     }
 
     /**

diff --git a/app/Services/PermissionService.php b/app/Services/PermissionService.php
index 218cb30a578f7483bdde8ea61cf29fba7d959dd7..0fffe60f29d523153516fc33b12431af8f9a5660 100644 (file)
--- a/app/Services/PermissionService.php
+++ b/app/Services/PermissionService.php
@@ -4,6 +4,7 @@ use BookStack\Book;
 use BookStack\Chapter;
 use BookStack\Entity;
 use BookStack\JointPermission;
+use BookStack\Ownable;
 use BookStack\Page;
 use BookStack\Role;
 use BookStack\User;
@@ -307,16 +308,16 @@ class PermissionService
 
     /**
      * Checks if an entity has a restriction set upon it.
-     * @param Entity $entity
+     * @param Ownable $ownable
      * @param $permission
      * @return bool
      */
-    public function checkEntityUserAccess(Entity $entity, $permission)
+    public function checkOwnableUserAccess(Ownable $ownable, $permission)
     {
         if ($this->isAdmin) return true;
         $explodedPermission = explode('-', $permission);
 
-        $baseQuery = $entity->where('id', '=', $entity->id);
+        $baseQuery = $ownable->where('id', '=', $ownable->id);
         $action = end($explodedPermission);
         $this->currentAction = $action;
 
@@ -327,7 +328,7 @@ class PermissionService
             $allPermission = $this->currentUser && $this->currentUser->can($permission . '-all');
             $ownPermission = $this->currentUser && $this->currentUser->can($permission . '-own');
             $this->currentAction = 'view';
-            $isOwner = $this->currentUser && $this->currentUser->id === $entity->created_by;
+            $isOwner = $this->currentUser && $this->currentUser->id === $ownable->created_by;
             return ($allPermission || ($isOwner && $ownPermission));
         }
 

diff --git a/app/helpers.php b/app/helpers.php
index b8f61d94e3fde57f757c115b5f45c4fa0dc84080..42e4c189476c8669a2d8bd336a7c9b84a7223df2 100644 (file)
--- a/app/helpers.php
+++ b/app/helpers.php
@@ -1,5 +1,7 @@
 <?php
 
+use BookStack\Ownable;
+
 if (!function_exists('versioned_asset')) {
     /**
      * Get the path to a versioned file.
@@ -34,18 +36,18 @@ if (!function_exists('versioned_asset')) {
  * If an ownable element is passed in the jointPermissions are checked against
  * that particular item.
  * @param $permission
- * @param \BookStack\Ownable $ownable
+ * @param Ownable $ownable
  * @return mixed
  */
-function userCan($permission, \BookStack\Ownable $ownable = null)
+function userCan($permission, Ownable $ownable = null)
 {
     if ($ownable === null) {
         return auth()->user() && auth()->user()->can($permission);
     }
 
     // Check permission on ownable item
-    $permissionService = app('BookStack\Services\PermissionService');
-    return $permissionService->checkEntityUserAccess($ownable, $permission);
+    $permissionService = app(\BookStack\Services\PermissionService::class);
+    return $permissionService->checkOwnableUserAccess($ownable, $permission);
 }
 
 /**

diff --git a/tests/ImageTest.php b/tests/ImageTest.php
new file mode 100644 (file)
index 0000000..806a36a
--- /dev/null
+++ b/tests/ImageTest.php
@@ -0,0 +1,95 @@
+<?php
+
+class ImageTest extends TestCase
+{
+
+    /**
+     * Get a test image that can be uploaded
+     * @param $fileName
+     * @return \Illuminate\Http\UploadedFile
+     */
+    protected function getTestImage($fileName)
+    {
+        return new \Illuminate\Http\UploadedFile(base_path('tests/test-image.jpg'), $fileName, 'image/jpeg', 5238);
+    }
+
+    /**
+     * Get the path for a test image.
+     * @param $type
+     * @param $fileName
+     * @return string
+     */
+    protected function getTestImagePath($type, $fileName)
+    {
+        return '/uploads/images/' . $type . '/' . Date('Y-m-M') . '/' . $fileName;
+    }
+
+    /**
+     * Uploads an image with the given name.
+     * @param $name
+     * @param int $uploadedTo
+     * @return string
+     */
+    protected function uploadImage($name, $uploadedTo = 0)
+    {
+        $file = $this->getTestImage($name);
+        $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []);
+        return $this->getTestImagePath('gallery', $name);
+    }
+
+    /**
+     * Delete an uploaded image.
+     * @param $relPath
+     */
+    protected function deleteImage($relPath)
+    {
+        unlink(public_path($relPath));
+    }
+
+
+    public function test_image_upload()
+    {
+        $page = \BookStack\Page::first();
+        $this->asAdmin();
+        $admin = $this->getAdmin();
+        $imageName = 'first-image.jpg';
+
+        $relPath = $this->uploadImage($imageName, $page->id);
+        $this->assertResponseOk();
+
+        $this->assertTrue(file_exists(public_path($relPath)), 'Uploaded image exists');
+
+        $this->seeInDatabase('images', [
+            'url' => $relPath,
+            'type' => 'gallery',
+            'uploaded_to' => $page->id,
+            'path' => $relPath,
+            'created_by' => $admin->id,
+            'updated_by' => $admin->id,
+            'name' => $imageName
+        ]);
+        
+        $this->deleteImage($relPath);
+    }
+
+    public function test_image_delete()
+    {
+        $page = \BookStack\Page::first();
+        $this->asAdmin();
+        $imageName = 'first-image.jpg';
+
+        $relPath = $this->uploadImage($imageName, $page->id);
+        $image = \BookStack\Image::first();
+
+        $this->call('DELETE', '/images/' . $image->id);
+        $this->assertResponseOk();
+
+        $this->dontSeeInDatabase('images', [
+            'url' => $relPath,
+            'type' => 'gallery'
+        ]);
+
+        $this->assertFalse(file_exists(public_path($relPath)), 'Uploaded image has been deleted');
+    }
+
+}
\ No newline at end of file

diff --git a/tests/TestCase.php b/tests/TestCase.php
index 4c2893f4e7d43ce8f0c85850ba078cce81658bc0..6a8c2d732b65dc123de08569c6f4c2da7c2ac3bc 100644 (file)
--- a/tests/TestCase.php
+++ b/tests/TestCase.php
@@ -39,11 +39,19 @@ class TestCase extends Illuminate\Foundation\Testing\TestCase
      */
     public function asAdmin()
     {
+        return $this->actingAs($this->getAdmin());
+    }
+
+    /**
+     * Get the current admin user.
+     * @return mixed
+     */
+    public function getAdmin() {
         if($this->admin === null) {
             $adminRole = \BookStack\Role::getRole('admin');
             $this->admin = $adminRole->users->first();
         }
-        return $this->actingAs($this->admin);
+        return $this->admin;
     }
 
     /**

diff --git a/tests/test-image.jpg b/tests/test-image.jpg
new file mode 100644 (file)
index 0000000..fb8da91
Binary files /dev/null and b/tests/test-image.jpg differ