Started testing work for recycle bin implementation

app/Entities/Entity.php

@@ -295,7 +295,7 @@ class Entity extends Ownable
     public function getParent(): ?Entity
     {
         if ($this->isA('page')) {
-            return $this->chapter_id ? $this->chapter()->withTrashed()->first() : $this->book->withTrashed()->first();
+            return $this->chapter_id ? $this->chapter()->withTrashed()->first() : $this->book()->withTrashed()->first();
         }
         if ($this->isA('chapter')) {
             return $this->book->withTrashed()->first();

app/Entities/Managers/TrashCan.php

@@ -90,7 +90,7 @@ class TrashCan
      * Remove a bookshelf from the system.
      * @throws Exception
      */
-    public function destroyShelf(Bookshelf $shelf): int
+    protected function destroyShelf(Bookshelf $shelf): int
     {
         $this->destroyCommonRelations($shelf);
         $shelf->forceDelete();
@@ -102,7 +102,7 @@ class TrashCan
      * Destroys any child chapters and pages.
      * @throws Exception
      */
-    public function destroyBook(Book $book): int
+    protected function destroyBook(Book $book): int
     {
         $count = 0;
         $pages = $book->pages()->withTrashed()->get();
@@ -127,7 +127,7 @@ class TrashCan
      * Destroys all pages within.
      * @throws Exception
      */
-    public function destroyChapter(Chapter $chapter): int
+    protected function destroyChapter(Chapter $chapter): int
     {
         $count = 0;
         $pages = $chapter->pages()->withTrashed()->get();
@@ -147,7 +147,7 @@ class TrashCan
      * Remove a page from the system.
      * @throws Exception
      */
-    public function destroyPage(Page $page): int
+    protected function destroyPage(Page $page): int
     {
         $this->destroyCommonRelations($page);
 
@@ -182,7 +182,7 @@ class TrashCan
      * Destroy all items that have pending deletions.
      * @throws Exception
      */
-    public function destroyFromAllDeletions(): int
+    public function empty(): int
     {
         $deletions = Deletion::all();
         $deleteCount = 0;

app/Http/Controllers/AuditLogController.php

@@ -23,7 +23,12 @@ class AuditLogController extends Controller
         ];
 
         $query = Activity::query()
-            ->with(['entity', 'user'])
+            ->with([
+                'entity' => function ($query) {
+                    $query->withTrashed();
+                },
+                'user'
+            ])
             ->orderBy($listDetails['sort'], $listDetails['order']);
 
         if ($listDetails['event']) {

app/Http/Controllers/BookController.php

@@ -181,14 +181,13 @@ class BookController extends Controller
     /**
      * Remove the specified book from the system.
      * @throws Throwable
-     * @throws NotifyException
      */
     public function destroy(string $bookSlug)
     {
         $book = $this->bookRepo->getBySlug($bookSlug);
         $this->checkOwnablePermission('book-delete', $book);
 
-        Activity::addMessage('book_delete', $book->name);
+        Activity::add($book, 'book_delete', $book->id);
         $this->bookRepo->destroy($book);
 
         return redirect('/books');

app/Http/Controllers/BookshelfController.php

@@ -182,7 +182,7 @@ class BookshelfController extends Controller
         $shelf = $this->bookshelfRepo->getBySlug($slug);
         $this->checkOwnablePermission('bookshelf-delete', $shelf);
 
-        Activity::addMessage('bookshelf_delete', $shelf->name);
+        Activity::add($shelf, 'bookshelf_delete');
         $this->bookshelfRepo->destroy($shelf);
 
         return redirect('/shelves');

app/Http/Controllers/ChapterController.php

@@ -128,7 +128,7 @@ class ChapterController extends Controller
         $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug);
         $this->checkOwnablePermission('chapter-delete', $chapter);
 
-        Activity::addMessage('chapter_delete', $chapter->name, $chapter->book->id);
+        Activity::add($chapter, 'chapter_delete', $chapter->book->id);
         $this->chapterRepo->destroy($chapter);
 
         return redirect($chapter->book->getUrl());

app/Http/Controllers/PageController.php

@@ -308,9 +308,8 @@ class PageController extends Controller
         $book = $page->book;
         $parent = $page->chapter ?? $book;
         $this->pageRepo->destroy($page);
-        Activity::addMessage('page_delete', $page->name, $book->id);
+        Activity::add($page, 'page_delete', $page->book_id);
 
-        $this->showSuccessNotification(trans('entities.pages_delete_success'));
         return redirect($parent->getUrl());
     }
 

app/Http/Controllers/RecycleBinController.php

@@ -14,7 +14,6 @@ class RecycleBinController extends Controller
      */
     public function __construct()
     {
-        // TODO - Check this is enforced.
         $this->middleware(function ($request, $next) {
             $this->checkPermission('settings-manage');
             $this->checkPermission('restrictions-manage-all');
@@ -96,7 +95,7 @@ class RecycleBinController extends Controller
      */
     public function empty()
     {
-        $deleteCount = (new TrashCan())->destroyFromAllDeletions();
+        $deleteCount = (new TrashCan())->empty();
 
         $this->showSuccessNotification(trans('settings.recycle_bin_destroy_notification', ['count' => $deleteCount]));
         return redirect($this->recycleBinBaseUrl);

tests/AuditLogTest.php

@@ -3,6 +3,7 @@
 use BookStack\Actions\Activity;
 use BookStack\Actions\ActivityService;
 use BookStack\Auth\UserRepo;
+use BookStack\Entities\Managers\TrashCan;
 use BookStack\Entities\Page;
 use BookStack\Entities\Repos\PageRepo;
 use Carbon\Carbon;
@@ -40,7 +41,7 @@ class AuditLogTest extends TestCase
         $resp->assertSeeText($page->name);
         $resp->assertSeeText('page_create');
         $resp->assertSeeText($activity->created_at->toDateTimeString());
-        $resp->assertElementContains('.audit-log-user', $admin->name);
+        $resp->assertElementContains('.table-user-item', $admin->name);
     }
 
     public function test_shows_name_for_deleted_items()
@@ -51,6 +52,7 @@ class AuditLogTest extends TestCase
         app(ActivityService::class)->add($page, 'page_create', $page->book->id);
 
         app(PageRepo::class)->destroy($page);
+        app(TrashCan::class)->empty();
 
         $resp = $this->get('settings/audit');
         $resp->assertSeeText('Deleted Item');

tests/Entity/BookShelfTest.php

@@ -222,16 +222,25 @@ class BookShelfTest extends TestCase
 
     public function test_shelf_delete()
     {
-        $shelf = Bookshelf::first();
-        $resp = $this->asEditor()->get($shelf->getUrl('/delete'));
-        $resp->assertSeeText('Delete Bookshelf');
-        $resp->assertSee("action=\"{$shelf->getUrl()}\"");
+        $shelf = Bookshelf::query()->whereHas('books')->first();
+        $this->assertNull($shelf->deleted_at);
+        $bookCount = $shelf->books()->count();
 
-        $resp = $this->delete($shelf->getUrl());
-        $resp->assertRedirect('/shelves');
-        $this->assertDatabaseMissing('bookshelves', ['id' => $shelf->id]);
-        $this->assertDatabaseMissing('bookshelves_books', ['bookshelf_id' => $shelf->id]);
-        $this->assertSessionHas('success');
+        $deleteViewReq = $this->asEditor()->get($shelf->getUrl('/delete'));
+        $deleteViewReq->assertSeeText('Are you sure you want to delete this bookshelf?');
+
+        $deleteReq = $this->delete($shelf->getUrl());
+        $deleteReq->assertRedirect(url('/shelves'));
+        $this->assertActivityExists('bookshelf_delete', $shelf);
+
+        $shelf->refresh();
+        $this->assertNotNull($shelf->deleted_at);
+
+        $this->assertTrue($shelf->books()->count() === $bookCount);
+        $this->assertTrue($shelf->deletions()->count() === 1);
+
+        $redirectReq = $this->get($deleteReq->baseResponse->headers->get('location'));
+        $redirectReq->assertNotificationContains('Bookshelf Successfully Deleted');
     }
 
     public function test_shelf_copy_permissions()

tests/Entity/BookTest.php

@@ -0,0 +1,34 @@
+<?php namespace Tests\Entity;
+
+use BookStack\Entities\Book;
+use Tests\TestCase;
+
+class BookTest extends TestCase
+{
+    public function test_book_delete()
+    {
+        $book = Book::query()->whereHas('pages')->whereHas('chapters')->first();
+        $this->assertNull($book->deleted_at);
+        $pageCount = $book->pages()->count();
+        $chapterCount = $book->chapters()->count();
+
+        $deleteViewReq = $this->asEditor()->get($book->getUrl('/delete'));
+        $deleteViewReq->assertSeeText('Are you sure you want to delete this book?');
+
+        $deleteReq = $this->delete($book->getUrl());
+        $deleteReq->assertRedirect(url('/books'));
+        $this->assertActivityExists('book_delete', $book);
+
+        $book->refresh();
+        $this->assertNotNull($book->deleted_at);
+
+        $this->assertTrue($book->pages()->count() === 0);
+        $this->assertTrue($book->chapters()->count() === 0);
+        $this->assertTrue($book->pages()->withTrashed()->count() === $pageCount);
+        $this->assertTrue($book->chapters()->withTrashed()->count() === $chapterCount);
+        $this->assertTrue($book->deletions()->count() === 1);
+
+        $redirectReq = $this->get($deleteReq->baseResponse->headers->get('location'));
+        $redirectReq->assertNotificationContains('Book Successfully Deleted');
+    }
+}

tests/Entity/ChapterTest.php

@@ -0,0 +1,31 @@
+<?php namespace Tests\Entity;
+
+use BookStack\Entities\Chapter;
+use Tests\TestCase;
+
+class ChapterTest extends TestCase
+{
+    public function test_chapter_delete()
+    {
+        $chapter = Chapter::query()->whereHas('pages')->first();
+        $this->assertNull($chapter->deleted_at);
+        $pageCount = $chapter->pages()->count();
+
+        $deleteViewReq = $this->asEditor()->get($chapter->getUrl('/delete'));
+        $deleteViewReq->assertSeeText('Are you sure you want to delete this chapter?');
+
+        $deleteReq = $this->delete($chapter->getUrl());
+        $deleteReq->assertRedirect($chapter->getParent()->getUrl());
+        $this->assertActivityExists('chapter_delete', $chapter);
+
+        $chapter->refresh();
+        $this->assertNotNull($chapter->deleted_at);
+
+        $this->assertTrue($chapter->pages()->count() === 0);
+        $this->assertTrue($chapter->pages()->withTrashed()->count() === $pageCount);
+        $this->assertTrue($chapter->deletions()->count() === 1);
+
+        $redirectReq = $this->get($deleteReq->baseResponse->headers->get('location'));
+        $redirectReq->assertNotificationContains('Chapter Successfully Deleted');
+    }
+}

tests/Entity/EntityTest.php

@@ -7,7 +7,6 @@ use BookStack\Entities\Page;
 use BookStack\Auth\UserRepo;
 use BookStack\Entities\Repos\PageRepo;
 use Carbon\Carbon;
-use Illuminate\Support\Facades\DB;
 use Tests\BrowserKitTest;
 
 class EntityTest extends BrowserKitTest
@@ -18,27 +17,10 @@ class EntityTest extends BrowserKitTest
         // Test Creation
         $book = $this->bookCreation();
         $chapter = $this->chapterCreation($book);
-        $page = $this->pageCreation($chapter);
+        $this->pageCreation($chapter);
 
         // Test Updating
-        $book = $this->bookUpdate($book);
-
-        // Test Deletion
-        $this->bookDelete($book);
-    }
-
-    public function bookDelete(Book $book)
-    {
-        $this->asAdmin()
-            ->visit($book->getUrl())
-            // Check link works correctly
-            ->click('Delete')
-            ->seePageIs($book->getUrl() . '/delete')
-            // Ensure the book name is show to user
-            ->see($book->name)
-            ->press('Confirm')
-            ->seePageIs('/books')
-            ->notSeeInDatabase('books', ['id' => $book->id]);
+        $this->bookUpdate($book);
     }
 
     public function bookUpdate(Book $book)
@@ -332,34 +314,4 @@ class EntityTest extends BrowserKitTest
             ->seePageIs($chapter->getUrl());
     }
 
-    public function test_page_delete_removes_entity_from_its_activity()
-    {
-        $page = Page::query()->first();
-
-        $this->asEditor()->put($page->getUrl(), [
-            'name' => 'My updated page',
-            'html' => '<p>updated content</p>',
-        ]);
-        $page->refresh();
-
-        $this->seeInDatabase('activities', [
-            'entity_id' => $page->id,
-            'entity_type' => $page->getMorphClass(),
-        ]);
-
-        $resp = $this->delete($page->getUrl());
-        $resp->assertResponseStatus(302);
-
-        $this->dontSeeInDatabase('activities', [
-            'entity_id' => $page->id,
-            'entity_type' => $page->getMorphClass(),
-        ]);
-
-        $this->seeInDatabase('activities', [
-            'extra' => 'My updated page',
-            'entity_id' => 0,
-            'entity_type' => '',
-        ]);
-    }
-
 }

tests/Entity/PageTest.php

@@ -0,0 +1,27 @@
+<?php namespace Tests\Entity;
+
+use BookStack\Entities\Page;
+use Tests\TestCase;
+
+class PageTest extends TestCase
+{
+    public function test_page_delete()
+    {
+        $page = Page::query()->first();
+        $this->assertNull($page->deleted_at);
+
+        $deleteViewReq = $this->asEditor()->get($page->getUrl('/delete'));
+        $deleteViewReq->assertSeeText('Are you sure you want to delete this page?');
+
+        $deleteReq = $this->delete($page->getUrl());
+        $deleteReq->assertRedirect($page->getParent()->getUrl());
+        $this->assertActivityExists('page_delete', $page);
+
+        $page->refresh();
+        $this->assertNotNull($page->deleted_at);
+        $this->assertTrue($page->deletions()->count() === 1);
+
+        $redirectReq = $this->get($deleteReq->baseResponse->headers->get('location'));
+        $redirectReq->assertNotificationContains('Page Successfully Deleted');
+    }
+}

tests/RecycleBinTest.php

@@ -0,0 +1,68 @@
+<?php namespace Tests;
+
+use BookStack\Entities\Book;
+use BookStack\Entities\Deletion;
+use BookStack\Entities\Page;
+
+class RecycleBinTest extends TestCase
+{
+    // TODO - Test activity updating on destroy
+
+    public function test_recycle_bin_routes_permissions()
+    {
+        $page = Page::query()->first();
+        $editor = $this->getEditor();
+        $this->actingAs($editor)->delete($page->getUrl());
+        $deletion = Deletion::query()->firstOrFail();
+
+        $routes = [
+            'GET:/settings/recycle-bin',
+            'POST:/settings/recycle-bin/empty',
+            "GET:/settings/recycle-bin/{$deletion->id}/destroy",
+            "GET:/settings/recycle-bin/{$deletion->id}/restore",
+            "POST:/settings/recycle-bin/{$deletion->id}/restore",
+            "DELETE:/settings/recycle-bin/{$deletion->id}",
+        ];
+
+        foreach($routes as $route) {
+            [$method, $url] = explode(':', $route);
+            $resp = $this->call($method, $url);
+            $this->assertPermissionError($resp);
+        }
+
+        $this->giveUserPermissions($editor, ['restrictions-manage-all']);
+
+        foreach($routes as $route) {
+            [$method, $url] = explode(':', $route);
+            $resp = $this->call($method, $url);
+            $this->assertPermissionError($resp);
+        }
+
+        $this->giveUserPermissions($editor, ['settings-manage']);
+
+        foreach($routes as $route) {
+            \DB::beginTransaction();
+            [$method, $url] = explode(':', $route);
+            $resp = $this->call($method, $url);
+            $this->assertNotPermissionError($resp);
+            \DB::rollBack();
+        }
+
+    }
+
+    public function test_recycle_bin_view()
+    {
+        $page = Page::query()->first();
+        $book = Book::query()->whereHas('pages')->whereHas('chapters')->withCount(['pages', 'chapters'])->first();
+        $editor = $this->getEditor();
+        $this->actingAs($editor)->delete($page->getUrl());
+        $this->actingAs($editor)->delete($book->getUrl());
+
+        $viewReq = $this->asAdmin()->get('/settings/recycle-bin');
+        $viewReq->assertElementContains('table.table', $page->name);
+        $viewReq->assertElementContains('table.table', $editor->name);
+        $viewReq->assertElementContains('table.table', $book->name);
+        $viewReq->assertElementContains('table.table', $book->pages_count . ' Pages');
+        $viewReq->assertElementContains('table.table', $book->chapters_count . ' Chapters');
+    }
+}

tests/SharedTestHelpers.php

@@ -15,12 +15,14 @@ use BookStack\Auth\Permissions\PermissionService;
 use BookStack\Entities\Repos\PageRepo;
 use BookStack\Settings\SettingService;
 use BookStack\Uploads\HttpFetcher;
+use Illuminate\Http\Response;
 use Illuminate\Support\Env;
 use Illuminate\Support\Facades\Log;
 use Mockery;
 use Monolog\Handler\TestHandler;
 use Monolog\Logger;
 use Throwable;
+use Illuminate\Foundation\Testing\Assert as PHPUnit;
 
 trait SharedTestHelpers
 {
@@ -270,14 +272,25 @@ trait SharedTestHelpers
      */
     protected function assertPermissionError($response)
     {
-        if ($response instanceof BrowserKitTest) {
-            $response = \Illuminate\Foundation\Testing\TestResponse::fromBaseResponse($response->response);
-        }
+        PHPUnit::assertTrue($this->isPermissionError($response->baseResponse ?? $response->response), "Failed asserting the response contains a permission error.");
+    }
 
-        $response->assertRedirect('/');
-        $this->assertSessionHas('error');
-        $error = session()->pull('error');
-        $this->assertStringStartsWith('You do not have permission to access', $error);
+    /**
+     * Assert a permission error has occurred.
+     */
+    protected function assertNotPermissionError($response)
+    {
+        PHPUnit::assertFalse($this->isPermissionError($response->baseResponse ?? $response->response), "Failed asserting the response does not contain a permission error.");
+    }
+
+    /**
+     * Check if the given response is a permission error.
+     */
+    private function isPermissionError($response): bool
+    {
+        return $response->status() === 302
+            && $response->headers->get('Location') === url('/')
+            && strpos(session()->pull('error', ''), 'You do not have permission to access') === 0;
     }
 
     /**

tests/TestResponse.php

@@ -15,9 +15,8 @@ class TestResponse extends BaseTestResponse {
 
     /**
      * Get the DOM Crawler for the response content.
-     * @return Crawler
      */
-    protected function crawler()
+    protected function crawler(): Crawler
     {
         if (!is_object($this->crawlerInstance)) {
             $this->crawlerInstance = new Crawler($this->getContent());
@@ -27,7 +26,6 @@ class TestResponse extends BaseTestResponse {
 
     /**
      * Assert the response contains the specified element.
-     * @param string $selector
      * @return $this
      */
     public function assertElementExists(string $selector)
@@ -45,7 +43,6 @@ class TestResponse extends BaseTestResponse {
 
     /**
      * Assert the response does not contain the specified element.
-     * @param string $selector
      * @return $this
      */
     public function assertElementNotExists(string $selector)
@@ -63,8 +60,6 @@ class TestResponse extends BaseTestResponse {
 
     /**
      * Assert the response includes a specific element containing the given text.
-     * @param string $selector
-     * @param string $text
      * @return $this
      */
     public function assertElementContains(string $selector, string $text)
@@ -95,8 +90,6 @@ class TestResponse extends BaseTestResponse {
 
     /**
      * Assert the response does not include a specific element containing the given text.
-     * @param string $selector
-     * @param string $text
      * @return $this
      */
     public function assertElementNotContains(string $selector, string $text)
@@ -125,12 +118,20 @@ class TestResponse extends BaseTestResponse {
         return $this;
     }
 
+    /**
+     * Assert there's a notification within the view containing the given text.
+     * @return $this
+     */
+    public function assertNotificationContains(string $text)
+    {
+        return $this->assertElementContains('[notification]', $text);
+    }
+
     /**
      * Get the escaped text pattern for the constraint.
-     * @param  string  $text
      * @return string
      */
-    protected function getEscapedPattern($text)
+    protected function getEscapedPattern(string $text)
     {
         $rawPattern = preg_quote($text, '/');
         $escapedPattern = preg_quote(e($text), '/');

tests/Uploads/AttachmentTest.php

@@ -1,5 +1,7 @@
 <?php namespace Tests\Uploads;
 
+use BookStack\Entities\Managers\TrashCan;
+use BookStack\Entities\Repos\PageRepo;
 use BookStack\Uploads\Attachment;
 use BookStack\Entities\Page;
 use BookStack\Auth\Permissions\PermissionService;
@@ -208,7 +210,8 @@ class AttachmentTest extends TestCase
             'name' => $fileName
         ]);
 
-        $this->call('DELETE', $page->getUrl());
+        app(PageRepo::class)->destroy($page);
+        app(TrashCan::class)->empty();
 
         $this->assertDatabaseMissing('attachments', [
             'name' => $fileName