From 483cb41665c9d5994b47c762670d5fd98188cf14 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 6 Nov 2020 12:54:39 +0000 Subject: [PATCH] Started testing work for recycle bin implementation --- app/Entities/Entity.php | 2 +- app/Entities/Managers/TrashCan.php | 10 +-- app/Http/Controllers/AuditLogController.php | 7 +- app/Http/Controllers/BookController.php | 3 +- app/Http/Controllers/BookshelfController.php | 2 +- app/Http/Controllers/ChapterController.php | 2 +- app/Http/Controllers/PageController.php | 3 +- app/Http/Controllers/RecycleBinController.php | 3 +- tests/AuditLogTest.php | 4 +- tests/Entity/BookShelfTest.php | 27 +++++--- tests/Entity/BookTest.php | 34 ++++++++++ tests/Entity/ChapterTest.php | 31 +++++++++ tests/Entity/EntityTest.php | 52 +------------- tests/Entity/PageTest.php | 27 ++++++++ tests/RecycleBinTest.php | 68 +++++++++++++++++++ tests/SharedTestHelpers.php | 27 ++++++-- tests/TestResponse.php | 21 +++--- tests/Uploads/AttachmentTest.php | 5 +- 18 files changed, 235 insertions(+), 93 deletions(-) create mode 100644 tests/Entity/BookTest.php create mode 100644 tests/Entity/ChapterTest.php create mode 100644 tests/Entity/PageTest.php create mode 100644 tests/RecycleBinTest.php diff --git a/app/Entities/Entity.php b/app/Entities/Entity.php index ed3040929..34cdb4b8c 100644 --- a/app/Entities/Entity.php +++ b/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(); diff --git a/app/Entities/Managers/TrashCan.php b/app/Entities/Managers/TrashCan.php index c567edaf3..686918ce2 100644 --- a/app/Entities/Managers/TrashCan.php +++ b/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; diff --git a/app/Http/Controllers/AuditLogController.php b/app/Http/Controllers/AuditLogController.php index a3ef01baa..fad4e8d38 100644 --- a/app/Http/Controllers/AuditLogController.php +++ b/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']) { diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index 1643c62f9..25dc65194 100644 --- a/app/Http/Controllers/BookController.php +++ b/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'); diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php index f2cc11c7b..efe280235 100644 --- a/app/Http/Controllers/BookshelfController.php +++ b/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'); diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index 135597910..5d8631154 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/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()); diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index ee998996f..6396da23e 100644 --- a/app/Http/Controllers/PageController.php +++ b/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()); } diff --git a/app/Http/Controllers/RecycleBinController.php b/app/Http/Controllers/RecycleBinController.php index 64459da23..459dbb39d 100644 --- a/app/Http/Controllers/RecycleBinController.php +++ b/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); diff --git a/tests/AuditLogTest.php b/tests/AuditLogTest.php index a2cdc33ff..94eb02599 100644 --- a/tests/AuditLogTest.php +++ b/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'); diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php index cb3acfb1e..c1748281e 100644 --- a/tests/Entity/BookShelfTest.php +++ b/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() diff --git a/tests/Entity/BookTest.php b/tests/Entity/BookTest.php new file mode 100644 index 000000000..b502bdcc5 --- /dev/null +++ b/tests/Entity/BookTest.php @@ -0,0 +1,34 @@ +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'); + } +} \ No newline at end of file diff --git a/tests/Entity/ChapterTest.php b/tests/Entity/ChapterTest.php new file mode 100644 index 000000000..d072f8d8b --- /dev/null +++ b/tests/Entity/ChapterTest.php @@ -0,0 +1,31 @@ +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'); + } +} \ No newline at end of file diff --git a/tests/Entity/EntityTest.php b/tests/Entity/EntityTest.php index de1e025ad..4aad6622f 100644 --- a/tests/Entity/EntityTest.php +++ b/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' => '

updated content

', - ]); - $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' => '', - ]); - } - } diff --git a/tests/Entity/PageTest.php b/tests/Entity/PageTest.php new file mode 100644 index 000000000..742fd1151 --- /dev/null +++ b/tests/Entity/PageTest.php @@ -0,0 +1,27 @@ +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'); + } +} \ No newline at end of file diff --git a/tests/RecycleBinTest.php b/tests/RecycleBinTest.php new file mode 100644 index 000000000..086f63679 --- /dev/null +++ b/tests/RecycleBinTest.php @@ -0,0 +1,68 @@ +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'); + } +} \ No newline at end of file diff --git a/tests/SharedTestHelpers.php b/tests/SharedTestHelpers.php index c7659a02d..1ba474d76 100644 --- a/tests/SharedTestHelpers.php +++ b/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; } /** diff --git a/tests/TestResponse.php b/tests/TestResponse.php index a68a5783f..9c6b78782 100644 --- a/tests/TestResponse.php +++ b/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), '/'); diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index a7efe08ab..4614c8e22 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -1,5 +1,7 @@ $fileName ]); - $this->call('DELETE', $page->getUrl()); + app(PageRepo::class)->destroy($page); + app(TrashCan::class)->empty(); $this->assertDatabaseMissing('attachments', [ 'name' => $fileName