From c157dc3490c39aa236e22714019a649ebeb11f13 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 7 Nov 2020 22:37:27 +0000 Subject: [PATCH 1/7] Organised activity types and moved most to repos Repos are generally better since otherwise we end up duplicating things between front-end and API. Types moved to by CONST values within a class for better visibilty of usage and listing of types. --- app/Actions/ActivityService.php | 21 +++--------------- app/Actions/ActivityType.php | 22 +++++++++++++++++++ app/Actions/CommentRepo.php | 1 + app/Entities/Repos/BookRepo.php | 6 +++++ app/Entities/Repos/BookshelfRepo.php | 7 +++++- app/Entities/Repos/ChapterRepo.php | 8 +++++++ app/Entities/Repos/PageRepo.php | 15 ++++++++++--- .../Controllers/Api/BookApiController.php | 6 +---- .../Api/BookshelfApiController.php | 7 +----- .../Controllers/Api/ChapterApiController.php | 7 +----- app/Http/Controllers/BookController.php | 11 ++-------- app/Http/Controllers/BookSortController.php | 3 ++- app/Http/Controllers/BookshelfController.php | 3 --- app/Http/Controllers/ChapterController.php | 6 ----- app/Http/Controllers/CommentController.php | 2 +- app/Http/Controllers/PageController.php | 10 ++------- .../Controllers/PageRevisionController.php | 2 +- tests/CommandsTest.php | 3 ++- tests/User/UserProfileTest.php | 9 ++++---- 19 files changed, 76 insertions(+), 73 deletions(-) create mode 100644 app/Actions/ActivityType.php diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index 4e5b1f365..e1046db88 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -13,9 +13,6 @@ class ActivityService protected $user; protected $permissionService; - /** - * ActivityService constructor. - */ public function __construct(Activity $activity, PermissionService $permissionService) { $this->activity = $activity; @@ -26,23 +23,11 @@ class ActivityService /** * Add activity data to database. */ - public function add(Entity $entity, string $activityKey, ?int $bookId = null) + public function add(Entity $entity, string $type, ?int $bookId = null) { - $activity = $this->newActivityForUser($activityKey, $bookId); + $activity = $this->newActivityForUser($type, $bookId); $entity->activity()->save($activity); - $this->setNotification($activityKey); - } - - /** - * Adds a activity history with a message, without binding to a entity. - */ - public function addMessage(string $activityKey, string $message, ?int $bookId = null) - { - $this->newActivityForUser($activityKey, $bookId)->forceFill([ - 'extra' => $message - ])->save(); - - $this->setNotification($activityKey); + $this->setNotification($type); } /** diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php new file mode 100644 index 000000000..5404d884c --- /dev/null +++ b/app/Actions/ActivityType.php @@ -0,0 +1,22 @@ +parent_id = $parent_id; $entity->comments()->save($comment); + Activity::add($entity, ActivityType::COMMENTED_ON, $entity->book->id); return $comment; } diff --git a/app/Entities/Repos/BookRepo.php b/app/Entities/Repos/BookRepo.php index b0ea7cb87..c2a613f71 100644 --- a/app/Entities/Repos/BookRepo.php +++ b/app/Entities/Repos/BookRepo.php @@ -1,11 +1,13 @@ baseRepo->create($book, $input); + Activity::add($book, ActivityType::BOOK_CREATE, $book->id); return $book; } @@ -100,6 +103,7 @@ class BookRepo public function update(Book $book, array $input): Book { $this->baseRepo->update($book, $input); + Activity::add($book, ActivityType::BOOK_UPDATE, $book->id); return $book; } @@ -129,6 +133,8 @@ class BookRepo { $trashCan = new TrashCan(); $trashCan->softDestroyBook($book); + Activity::add($book, ActivityType::BOOK_DELETE, $book->id); + $trashCan->autoClearOld(); } } diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index 49fb75f4c..2c80b2a7d 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -1,10 +1,12 @@ baseRepo->create($shelf, $input); $this->updateBooks($shelf, $bookIds); + Activity::add($shelf, ActivityType::BOOKSHELF_CREATE); return $shelf; } /** - * Create a new shelf in the system. + * Update an existing shelf in the system using the given input. */ public function update(Bookshelf $shelf, array $input, ?array $bookIds): Bookshelf { @@ -101,6 +104,7 @@ class BookshelfRepo $this->updateBooks($shelf, $bookIds); } + Activity::add($shelf, ActivityType::BOOKSHELF_UPDATE); return $shelf; } @@ -175,6 +179,7 @@ class BookshelfRepo { $trashCan = new TrashCan(); $trashCan->softDestroyShelf($shelf); + Activity::add($shelf, ActivityType::BOOKSHELF_DELETE); $trashCan->autoClearOld(); } } diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php index 60599eac8..581da1fa3 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -1,11 +1,13 @@ book_id = $parentBook->id; $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1; $this->baseRepo->create($chapter, $input); + Activity::add($chapter, ActivityType::CHAPTER_CREATE, $parentBook->id); return $chapter; } @@ -55,6 +58,7 @@ class ChapterRepo public function update(Chapter $chapter, array $input): Chapter { $this->baseRepo->update($chapter, $input); + Activity::add($chapter, ActivityType::CHAPTER_UPDATE, $chapter->book->id); return $chapter; } @@ -74,6 +78,7 @@ class ChapterRepo { $trashCan = new TrashCan(); $trashCan->softDestroyChapter($chapter); + Activity::add($chapter, ActivityType::CHAPTER_DELETE, $chapter->book->id); $trashCan->autoClearOld(); } @@ -93,6 +98,7 @@ class ChapterRepo throw new MoveOperationException('Chapters can only be moved into books'); } + /** @var Book $parent */ $parent = Book::visible()->where('id', '=', $entityId)->first(); if ($parent === null) { throw new MoveOperationException('Book to move chapter into not found'); @@ -100,6 +106,8 @@ class ChapterRepo $chapter->changeBook($parent->id); $chapter->rebuildPermissions(); + Activity::add($chapter, ActivityType::CHAPTER_MOVE, $parent->id); + return $parent; } } diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index 80c8afe98..065f1606f 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -1,5 +1,6 @@ savePageRevision($draft, trans('entities.pages_initial_revision')); $draft->indexForSearch(); - return $draft->refresh(); + $draft->refresh(); + + Activity::add($draft, ActivityType::PAGE_CREATE, $draft->book->id); + return $draft; } /** @@ -203,6 +208,7 @@ class PageRepo $this->savePageRevision($page, $summary); } + Activity::add($page, ActivityType::PAGE_UPDATE, $page->book->id); return $page; } @@ -266,6 +272,7 @@ class PageRepo { $trashCan = new TrashCan(); $trashCan->softDestroyPage($page); + Activity::add($page, ActivityType::PAGE_DELETE, $page->book_id); $trashCan->autoClearOld(); } @@ -286,6 +293,7 @@ class PageRepo $page->save(); $page->indexForSearch(); + Activity::add($page, ActivityType::PAGE_RESTORE, $page->book->id); return $page; } @@ -296,7 +304,7 @@ class PageRepo * @throws MoveOperationException * @throws PermissionsException */ - public function move(Page $page, string $parentIdentifier): Book + public function move(Page $page, string $parentIdentifier): Entity { $parent = $this->findParentByIdentifier($parentIdentifier); if ($parent === null) { @@ -311,7 +319,8 @@ class PageRepo $page->changeBook($parent instanceof Book ? $parent->id : $parent->book->id); $page->rebuildPermissions(); - return ($parent instanceof Book ? $parent : $parent->book); + Activity::add($page, ActivityType::PAGE_MOVE, $page->book->id); + return $parent; } /** diff --git a/app/Http/Controllers/Api/BookApiController.php b/app/Http/Controllers/Api/BookApiController.php index 8333eba3a..8ec9fdc06 100644 --- a/app/Http/Controllers/Api/BookApiController.php +++ b/app/Http/Controllers/Api/BookApiController.php @@ -1,5 +1,6 @@ validate($request, $this->rules['create']); $book = $this->bookRepo->create($requestData); - Activity::add($book, 'book_create', $book->id); - return response()->json($book); } @@ -80,7 +79,6 @@ class BookApiController extends ApiController $requestData = $this->validate($request, $this->rules['update']); $book = $this->bookRepo->update($book, $requestData); - Activity::add($book, 'book_update', $book->id); return response()->json($book); } @@ -96,8 +94,6 @@ class BookApiController extends ApiController $this->checkOwnablePermission('book-delete', $book); $this->bookRepo->destroy($book); - Activity::addMessage('book_delete', $book->name); - return response('', 204); } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/BookshelfApiController.php b/app/Http/Controllers/Api/BookshelfApiController.php index 14b5e053b..4650e1dde 100644 --- a/app/Http/Controllers/Api/BookshelfApiController.php +++ b/app/Http/Controllers/Api/BookshelfApiController.php @@ -1,5 +1,6 @@ get('books', []); $shelf = $this->bookshelfRepo->create($requestData, $bookIds); - Activity::add($shelf, 'bookshelf_create', $shelf->id); return response()->json($shelf); } @@ -94,12 +94,9 @@ class BookshelfApiController extends ApiController $this->checkOwnablePermission('bookshelf-update', $shelf); $requestData = $this->validate($request, $this->rules['update']); - $bookIds = $request->get('books', null); $shelf = $this->bookshelfRepo->update($shelf, $requestData, $bookIds); - Activity::add($shelf, 'bookshelf_update', $shelf->id); - return response()->json($shelf); } @@ -115,8 +112,6 @@ class BookshelfApiController extends ApiController $this->checkOwnablePermission('bookshelf-delete', $shelf); $this->bookshelfRepo->destroy($shelf); - Activity::addMessage('bookshelf_delete', $shelf->name); - return response('', 204); } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/ChapterApiController.php b/app/Http/Controllers/Api/ChapterApiController.php index 50aa8834e..60e0f0131 100644 --- a/app/Http/Controllers/Api/ChapterApiController.php +++ b/app/Http/Controllers/Api/ChapterApiController.php @@ -1,5 +1,6 @@ checkOwnablePermission('chapter-create', $book); $chapter = $this->chapterRepo->create($request->all(), $book); - Activity::add($chapter, 'chapter_create', $book->id); - return response()->json($chapter->load(['tags'])); } @@ -83,8 +82,6 @@ class ChapterApiController extends ApiController $this->checkOwnablePermission('chapter-update', $chapter); $updatedChapter = $this->chapterRepo->update($chapter, $request->all()); - Activity::add($chapter, 'chapter_update', $chapter->book->id); - return response()->json($updatedChapter->load(['tags'])); } @@ -97,8 +94,6 @@ class ChapterApiController extends ApiController $this->checkOwnablePermission('chapter-delete', $chapter); $this->chapterRepo->destroy($chapter); - Activity::addMessage('chapter_delete', $chapter->name, $chapter->book->id); - return response('', 204); } } diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index 25dc65194..5a42daddb 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -1,12 +1,12 @@ bookRepo = $bookRepo; @@ -97,11 +94,10 @@ class BookController extends Controller $book = $this->bookRepo->create($request->all()); $this->bookRepo->updateCoverImage($book, $request->file('image', null)); - Activity::add($book, 'book_create', $book->id); if ($bookshelf) { $bookshelf->appendBook($book); - Activity::add($bookshelf, 'bookshelf_update'); + Activity::add($bookshelf, ActivityType::BOOKSHELF_UPDATE); } return redirect($book->getUrl()); @@ -162,8 +158,6 @@ class BookController extends Controller $resetCover = $request->has('image_reset'); $this->bookRepo->updateCoverImage($book, $request->file('image', null), $resetCover); - Activity::add($book, 'book_update', $book->id); - return redirect($book->getUrl()); } @@ -187,7 +181,6 @@ class BookController extends Controller $book = $this->bookRepo->getBySlug($bookSlug); $this->checkOwnablePermission('book-delete', $book); - Activity::add($book, 'book_delete', $book->id); $this->bookRepo->destroy($book); return redirect('/books'); diff --git a/app/Http/Controllers/BookSortController.php b/app/Http/Controllers/BookSortController.php index f5fb6f255..e94e4ecce 100644 --- a/app/Http/Controllers/BookSortController.php +++ b/app/Http/Controllers/BookSortController.php @@ -2,6 +2,7 @@ namespace BookStack\Http\Controllers; +use BookStack\Actions\ActivityType; use BookStack\Entities\Book; use BookStack\Entities\Managers\BookContents; use BookStack\Entities\Repos\BookRepo; @@ -74,7 +75,7 @@ class BookSortController extends Controller // Rebuild permissions and add activity for involved books. $booksInvolved->each(function (Book $book) { - Activity::add($book, 'book_sort', $book->id); + Activity::add($book, ActivityType::BOOK_SORT, $book->id); }); return redirect($book->getUrl()); diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php index efe280235..8d2eec348 100644 --- a/app/Http/Controllers/BookshelfController.php +++ b/app/Http/Controllers/BookshelfController.php @@ -92,7 +92,6 @@ class BookshelfController extends Controller $shelf = $this->bookshelfRepo->create($request->all(), $bookIds); $this->bookshelfRepo->updateCoverImage($shelf, $request->file('image', null)); - Activity::add($shelf, 'bookshelf_create'); return redirect($shelf->getUrl()); } @@ -156,7 +155,6 @@ class BookshelfController extends Controller $shelf = $this->bookshelfRepo->update($shelf, $request->all(), $bookIds); $resetCover = $request->has('image_reset'); $this->bookshelfRepo->updateCoverImage($shelf, $request->file('image', null), $resetCover); - Activity::add($shelf, 'bookshelf_update'); return redirect($shelf->getUrl()); } @@ -182,7 +180,6 @@ class BookshelfController extends Controller $shelf = $this->bookshelfRepo->getBySlug($slug); $this->checkOwnablePermission('bookshelf-delete', $shelf); - 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 5d8631154..8eba43e21 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -1,6 +1,5 @@ checkOwnablePermission('chapter-create', $book); $chapter = $this->chapterRepo->create($request->all(), $book); - Activity::add($chapter, 'chapter_create', $book->id); return redirect($chapter->getUrl()); } @@ -100,7 +98,6 @@ class ChapterController extends Controller $this->checkOwnablePermission('chapter-update', $chapter); $this->chapterRepo->update($chapter, $request->all()); - Activity::add($chapter, 'chapter_update', $chapter->book->id); return redirect($chapter->getUrl()); } @@ -128,7 +125,6 @@ class ChapterController extends Controller $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug); $this->checkOwnablePermission('chapter-delete', $chapter); - Activity::add($chapter, 'chapter_delete', $chapter->book->id); $this->chapterRepo->destroy($chapter); return redirect($chapter->book->getUrl()); @@ -173,8 +169,6 @@ class ChapterController extends Controller return redirect()->back(); } - Activity::add($chapter, 'chapter_move', $newBook->id); - $this->showSuccessNotification(trans('entities.chapter_move_success', ['bookName' => $newBook->name])); return redirect($chapter->getUrl()); } diff --git a/app/Http/Controllers/CommentController.php b/app/Http/Controllers/CommentController.php index 4eb56a4b0..2dc1a4de4 100644 --- a/app/Http/Controllers/CommentController.php +++ b/app/Http/Controllers/CommentController.php @@ -1,6 +1,7 @@ checkPermission('comment-create-all'); $comment = $this->commentRepo->create($page, $request->get('text'), $request->get('parent_id')); - Activity::add($page, 'commented_on', $page->book->id); return view('comments.comment', ['comment' => $comment]); } diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index 6396da23e..862ba3d7f 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -1,6 +1,7 @@ checkOwnablePermission('page-create', $draftPage->getParent()); $page = $this->pageRepo->publishDraft($draftPage, $request->all()); - Activity::add($page, 'page_create', $draftPage->book->id); return redirect($page->getUrl()); } @@ -224,7 +224,6 @@ class PageController extends Controller $this->checkOwnablePermission('page-update', $page); $this->pageRepo->update($page, $request->all()); - Activity::add($page, 'page_update', $page->book->id); return redirect($page->getUrl()); } @@ -304,11 +303,9 @@ class PageController extends Controller { $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug); $this->checkOwnablePermission('page-delete', $page); + $parent = $page->getParent(); - $book = $page->book; - $parent = $page->chapter ?? $book; $this->pageRepo->destroy($page); - Activity::add($page, 'page_delete', $page->book_id); return redirect($parent->getUrl()); } @@ -393,7 +390,6 @@ class PageController extends Controller return redirect()->back(); } - Activity::add($page, 'page_move', $page->book->id); $this->showSuccessNotification(trans('entities.pages_move_success', ['parentName' => $parent->name])); return redirect($page->getUrl()); } @@ -438,8 +434,6 @@ class PageController extends Controller return redirect()->back(); } - Activity::add($pageCopy, 'page_create', $pageCopy->book->id); - $this->showSuccessNotification(trans('entities.pages_copy_success')); return redirect($pageCopy->getUrl()); } diff --git a/app/Http/Controllers/PageRevisionController.php b/app/Http/Controllers/PageRevisionController.php index 797f5db8f..b56235d5b 100644 --- a/app/Http/Controllers/PageRevisionController.php +++ b/app/Http/Controllers/PageRevisionController.php @@ -1,5 +1,6 @@ pageRepo->restoreRevision($page, $revisionId); - Activity::add($page, 'page_restore', $page->book->id); return redirect($page->getUrl()); } diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index bfc0ac0eb..7e931ee96 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -1,5 +1,6 @@ asEditor(); $page = Page::first(); - \Activity::add($page, 'page_update', $page->book->id); + \Activity::add($page, ActivityType::PAGE_UPDATE, $page->book->id); $this->assertDatabaseHas('activities', [ 'key' => 'page_update', diff --git a/tests/User/UserProfileTest.php b/tests/User/UserProfileTest.php index b564ed8c2..cc39c0d8e 100644 --- a/tests/User/UserProfileTest.php +++ b/tests/User/UserProfileTest.php @@ -1,6 +1,7 @@ getNewBlankUser(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); - Activity::add($entities['book'], 'book_update', $entities['book']->id); - Activity::add($entities['page'], 'page_create', $entities['book']->id); + Activity::add($entities['book'], ActivityType::BOOK_UPDATE, $entities['book']->id); + Activity::add($entities['page'], ActivityType::PAGE_CREATE, $entities['book']->id); $this->asAdmin()->visit('/user/' . $newUser->id) ->seeInElement('#recent-user-activity', 'updated book') @@ -74,8 +75,8 @@ class UserProfileTest extends BrowserKitTest $newUser = $this->getNewBlankUser(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); - Activity::add($entities['book'], 'book_update', $entities['book']->id); - Activity::add($entities['page'], 'page_create', $entities['book']->id); + Activity::add($entities['book'], ActivityType::BOOK_UPDATE, $entities['book']->id); + Activity::add($entities['page'], ActivityType::PAGE_CREATE, $entities['book']->id); $this->asAdmin()->visit('/')->clickInElement('#recent-activity', $newUser->name) ->seePageIs('/user/' . $newUser->id) From ee7e1122d325b1807f993e4f0cacc40fbbc806fb Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 7 Nov 2020 23:15:13 +0000 Subject: [PATCH 2/7] Removed use of book_id in activity --- app/Actions/ActivityService.php | 44 +++++++++++++-------- app/Actions/CommentRepo.php | 3 +- app/Entities/Repos/BookRepo.php | 6 +-- app/Entities/Repos/BookshelfRepo.php | 6 +-- app/Entities/Repos/ChapterRepo.php | 8 ++-- app/Entities/Repos/PageRepo.php | 10 ++--- app/Http/Controllers/BookController.php | 2 +- app/Http/Controllers/BookSortController.php | 2 +- tests/CommandsTest.php | 2 +- tests/User/UserProfileTest.php | 8 ++-- 10 files changed, 51 insertions(+), 40 deletions(-) diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index e1046db88..be9f656c3 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -2,9 +2,11 @@ use BookStack\Auth\Permissions\PermissionService; use BookStack\Auth\User; +use BookStack\Entities\Chapter; use BookStack\Entities\Entity; +use BookStack\Entities\Page; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\Relation; -use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; class ActivityService @@ -21,11 +23,11 @@ class ActivityService } /** - * Add activity data to database. + * Add activity data to database for an entity. */ - public function add(Entity $entity, string $type, ?int $bookId = null) + public function addForEntity(Entity $entity, string $type) { - $activity = $this->newActivityForUser($type, $bookId); + $activity = $this->newActivityForUser($type); $entity->activity()->save($activity); $this->setNotification($type); } @@ -33,12 +35,11 @@ class ActivityService /** * Get a new activity instance for the current user. */ - protected function newActivityForUser(string $key, ?int $bookId = null): Activity + protected function newActivityForUser(string $type): Activity { return $this->activity->newInstance()->forceFill([ - 'key' => strtolower($key), + 'key' => strtolower($type), 'user_id' => $this->user->id, - 'book_id' => $bookId ?? 0, ]); } @@ -47,15 +48,13 @@ class ActivityService * and instead uses the 'extra' field with the entities name. * Used when an entity is deleted. */ - public function removeEntity(Entity $entity): Collection + public function removeEntity(Entity $entity) { - $activities = $entity->activity()->get(); $entity->activity()->update([ 'extra' => $entity->name, 'entity_id' => 0, 'entity_type' => '', ]); - return $activities; } /** @@ -80,16 +79,27 @@ class ActivityService */ public function entityActivity(Entity $entity, int $count = 20, int $page = 1): array { + /** @var [string => int[]] $queryIds */ + $queryIds = [$entity->getMorphClass() => [$entity->id]]; + if ($entity->isA('book')) { - $query = $this->activity->newQuery()->where('book_id', '=', $entity->id); - } else { - $query = $this->activity->newQuery()->where('entity_type', '=', $entity->getMorphClass()) - ->where('entity_id', '=', $entity->id); + $queryIds[(new Chapter)->getMorphClass()] = $entity->chapters()->visible()->pluck('id'); + } + if ($entity->isA('book') || $entity->isA('chapter')) { + $queryIds[(new Page)->getMorphClass()] = $entity->pages()->visible()->pluck('id'); } - $activity = $this->permissionService - ->filterRestrictedEntityRelations($query, 'activities', 'entity_id', 'entity_type') - ->orderBy('created_at', 'desc') + $query = $this->activity->newQuery(); + $query->where(function (Builder $query) use ($queryIds) { + foreach ($queryIds as $morphClass => $idArr) { + $query->orWhere(function (Builder $innerQuery) use ($morphClass, $idArr) { + $innerQuery->where('entity_type', '=', $morphClass) + ->whereIn('entity_id', $idArr); + }); + } + }); + + $activity = $query->orderBy('created_at', 'desc') ->with(['entity' => function (Relation $query) { $query->withTrashed(); }, 'user.avatar']) diff --git a/app/Actions/CommentRepo.php b/app/Actions/CommentRepo.php index c0f008137..1d46e781c 100644 --- a/app/Actions/CommentRepo.php +++ b/app/Actions/CommentRepo.php @@ -2,6 +2,7 @@ use BookStack\Entities\Entity; use League\CommonMark\CommonMarkConverter; +use BookStack\Facades\Activity as ActivityService; /** * Class CommentRepo @@ -44,7 +45,7 @@ class CommentRepo $comment->parent_id = $parent_id; $entity->comments()->save($comment); - Activity::add($entity, ActivityType::COMMENTED_ON, $entity->book->id); + ActivityService::addForEntity($entity, ActivityType::COMMENTED_ON); return $comment; } diff --git a/app/Entities/Repos/BookRepo.php b/app/Entities/Repos/BookRepo.php index c2a613f71..008a9080c 100644 --- a/app/Entities/Repos/BookRepo.php +++ b/app/Entities/Repos/BookRepo.php @@ -93,7 +93,7 @@ class BookRepo { $book = new Book(); $this->baseRepo->create($book, $input); - Activity::add($book, ActivityType::BOOK_CREATE, $book->id); + Activity::addForEntity($book, ActivityType::BOOK_CREATE); return $book; } @@ -103,7 +103,7 @@ class BookRepo public function update(Book $book, array $input): Book { $this->baseRepo->update($book, $input); - Activity::add($book, ActivityType::BOOK_UPDATE, $book->id); + Activity::addForEntity($book, ActivityType::BOOK_UPDATE); return $book; } @@ -133,7 +133,7 @@ class BookRepo { $trashCan = new TrashCan(); $trashCan->softDestroyBook($book); - Activity::add($book, ActivityType::BOOK_DELETE, $book->id); + Activity::addForEntity($book, ActivityType::BOOK_DELETE); $trashCan->autoClearOld(); } diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index 2c80b2a7d..7cd333ff3 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -89,7 +89,7 @@ class BookshelfRepo $shelf = new Bookshelf(); $this->baseRepo->create($shelf, $input); $this->updateBooks($shelf, $bookIds); - Activity::add($shelf, ActivityType::BOOKSHELF_CREATE); + Activity::addForEntity($shelf, ActivityType::BOOKSHELF_CREATE); return $shelf; } @@ -104,7 +104,7 @@ class BookshelfRepo $this->updateBooks($shelf, $bookIds); } - Activity::add($shelf, ActivityType::BOOKSHELF_UPDATE); + Activity::addForEntity($shelf, ActivityType::BOOKSHELF_UPDATE); return $shelf; } @@ -179,7 +179,7 @@ class BookshelfRepo { $trashCan = new TrashCan(); $trashCan->softDestroyShelf($shelf); - Activity::add($shelf, ActivityType::BOOKSHELF_DELETE); + Activity::addForEntity($shelf, ActivityType::BOOKSHELF_DELETE); $trashCan->autoClearOld(); } } diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php index 581da1fa3..312cb69e5 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -48,7 +48,7 @@ class ChapterRepo $chapter->book_id = $parentBook->id; $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1; $this->baseRepo->create($chapter, $input); - Activity::add($chapter, ActivityType::CHAPTER_CREATE, $parentBook->id); + Activity::addForEntity($chapter, ActivityType::CHAPTER_CREATE); return $chapter; } @@ -58,7 +58,7 @@ class ChapterRepo public function update(Chapter $chapter, array $input): Chapter { $this->baseRepo->update($chapter, $input); - Activity::add($chapter, ActivityType::CHAPTER_UPDATE, $chapter->book->id); + Activity::addForEntity($chapter, ActivityType::CHAPTER_UPDATE); return $chapter; } @@ -78,7 +78,7 @@ class ChapterRepo { $trashCan = new TrashCan(); $trashCan->softDestroyChapter($chapter); - Activity::add($chapter, ActivityType::CHAPTER_DELETE, $chapter->book->id); + Activity::addForEntity($chapter, ActivityType::CHAPTER_DELETE); $trashCan->autoClearOld(); } @@ -106,7 +106,7 @@ class ChapterRepo $chapter->changeBook($parent->id); $chapter->rebuildPermissions(); - Activity::add($chapter, ActivityType::CHAPTER_MOVE, $parent->id); + Activity::addForEntity($chapter, ActivityType::CHAPTER_MOVE); return $parent; } diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index 065f1606f..6b593d85e 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -169,7 +169,7 @@ class PageRepo $draft->indexForSearch(); $draft->refresh(); - Activity::add($draft, ActivityType::PAGE_CREATE, $draft->book->id); + Activity::addForEntity($draft, ActivityType::PAGE_CREATE); return $draft; } @@ -208,7 +208,7 @@ class PageRepo $this->savePageRevision($page, $summary); } - Activity::add($page, ActivityType::PAGE_UPDATE, $page->book->id); + Activity::addForEntity($page, ActivityType::PAGE_UPDATE); return $page; } @@ -272,7 +272,7 @@ class PageRepo { $trashCan = new TrashCan(); $trashCan->softDestroyPage($page); - Activity::add($page, ActivityType::PAGE_DELETE, $page->book_id); + Activity::addForEntity($page, ActivityType::PAGE_DELETE); $trashCan->autoClearOld(); } @@ -293,7 +293,7 @@ class PageRepo $page->save(); $page->indexForSearch(); - Activity::add($page, ActivityType::PAGE_RESTORE, $page->book->id); + Activity::addForEntity($page, ActivityType::PAGE_RESTORE); return $page; } @@ -319,7 +319,7 @@ class PageRepo $page->changeBook($parent instanceof Book ? $parent->id : $parent->book->id); $page->rebuildPermissions(); - Activity::add($page, ActivityType::PAGE_MOVE, $page->book->id); + Activity::addForEntity($page, ActivityType::PAGE_MOVE); return $parent; } diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index 5a42daddb..74f9586aa 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -97,7 +97,7 @@ class BookController extends Controller if ($bookshelf) { $bookshelf->appendBook($book); - Activity::add($bookshelf, ActivityType::BOOKSHELF_UPDATE); + Activity::addForEntity($bookshelf, ActivityType::BOOKSHELF_UPDATE); } return redirect($book->getUrl()); diff --git a/app/Http/Controllers/BookSortController.php b/app/Http/Controllers/BookSortController.php index e94e4ecce..fb9308b46 100644 --- a/app/Http/Controllers/BookSortController.php +++ b/app/Http/Controllers/BookSortController.php @@ -75,7 +75,7 @@ class BookSortController extends Controller // Rebuild permissions and add activity for involved books. $booksInvolved->each(function (Book $book) { - Activity::add($book, ActivityType::BOOK_SORT, $book->id); + Activity::addForEntity($book, ActivityType::BOOK_SORT); }); return redirect($book->getUrl()); diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index 7e931ee96..69233e287 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -38,7 +38,7 @@ class CommandsTest extends TestCase { $this->asEditor(); $page = Page::first(); - \Activity::add($page, ActivityType::PAGE_UPDATE, $page->book->id); + \Activity::addForEntity($page, ActivityType::PAGE_UPDATE); $this->assertDatabaseHas('activities', [ 'key' => 'page_update', diff --git a/tests/User/UserProfileTest.php b/tests/User/UserProfileTest.php index cc39c0d8e..f0c36454e 100644 --- a/tests/User/UserProfileTest.php +++ b/tests/User/UserProfileTest.php @@ -61,8 +61,8 @@ class UserProfileTest extends BrowserKitTest $newUser = $this->getNewBlankUser(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); - Activity::add($entities['book'], ActivityType::BOOK_UPDATE, $entities['book']->id); - Activity::add($entities['page'], ActivityType::PAGE_CREATE, $entities['book']->id); + Activity::addForEntity($entities['book'], ActivityType::BOOK_UPDATE); + Activity::addForEntity($entities['page'], ActivityType::PAGE_CREATE); $this->asAdmin()->visit('/user/' . $newUser->id) ->seeInElement('#recent-user-activity', 'updated book') @@ -75,8 +75,8 @@ class UserProfileTest extends BrowserKitTest $newUser = $this->getNewBlankUser(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); - Activity::add($entities['book'], ActivityType::BOOK_UPDATE, $entities['book']->id); - Activity::add($entities['page'], ActivityType::PAGE_CREATE, $entities['book']->id); + Activity::addForEntity($entities['book'], ActivityType::BOOK_UPDATE); + Activity::addForEntity($entities['page'], ActivityType::PAGE_CREATE); $this->asAdmin()->visit('/')->clickInElement('#recent-activity', $newUser->name) ->seePageIs('/user/' . $newUser->id) From 712ccd23c4738e6a59a10f31ff654743fbc61879 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 8 Nov 2020 00:03:19 +0000 Subject: [PATCH 3/7] Updated activities table format Renamed some columns to be more generic and applicable. Removed now redundant book_id column. Allowed nullable entity morph columns for non-entity activity. Ran tests and made required changes. --- app/Actions/Activity.php | 18 +++--- app/Actions/ActivityService.php | 16 +++-- app/Entities/BookChild.php | 3 - app/Http/Controllers/AuditLogController.php | 6 +- app/Http/Controllers/BookSortController.php | 4 -- ...11_07_232321_simplify_activities_table.php | 58 +++++++++++++++++++ resources/views/settings/audit.blade.php | 10 ++-- tests/AuditLogTest.php | 19 ++++-- tests/CommandsTest.php | 4 +- tests/Entity/SortTest.php | 2 +- tests/RecycleBinTest.php | 12 ++-- tests/TestCase.php | 4 +- 12 files changed, 106 insertions(+), 50 deletions(-) create mode 100644 database/migrations/2020_11_07_232321_simplify_activities_table.php diff --git a/app/Actions/Activity.php b/app/Actions/Activity.php index 035a9cc75..63eda5917 100644 --- a/app/Actions/Activity.php +++ b/app/Actions/Activity.php @@ -5,16 +5,16 @@ namespace BookStack\Actions; use BookStack\Auth\User; use BookStack\Entities\Entity; use BookStack\Model; +use Illuminate\Database\Eloquent\Relations\BelongsTo; /** - * @property string $key + * @property string $type * @property User $user * @property Entity $entity - * @property string $extra + * @property string $detail * @property string $entity_type * @property int $entity_id * @property int $user_id - * @property int $book_id */ class Activity extends Model { @@ -32,20 +32,18 @@ class Activity extends Model /** * Get the user this activity relates to. - * @return \Illuminate\Database\Eloquent\Relations\BelongsTo */ - public function user() + public function user(): BelongsTo { return $this->belongsTo(User::class); } /** - * Returns text from the language files, Looks up by using the - * activity key. + * Returns text from the language files, Looks up by using the activity key. */ - public function getText() + public function getText(): string { - return trans('activities.' . $this->key); + return trans('activities.' . $this->type); } /** @@ -53,6 +51,6 @@ class Activity extends Model */ public function isSimilarTo(Activity $activityB): bool { - return [$this->key, $this->entity_type, $this->entity_id] === [$activityB->key, $activityB->entity_type, $activityB->entity_id]; + return [$this->type, $this->entity_type, $this->entity_id] === [$activityB->type, $activityB->entity_type, $activityB->entity_id]; } } diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index be9f656c3..fb4a739cd 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -12,14 +12,12 @@ use Illuminate\Support\Facades\Log; class ActivityService { protected $activity; - protected $user; protected $permissionService; public function __construct(Activity $activity, PermissionService $permissionService) { $this->activity = $activity; $this->permissionService = $permissionService; - $this->user = user(); } /** @@ -38,8 +36,8 @@ class ActivityService protected function newActivityForUser(string $type): Activity { return $this->activity->newInstance()->forceFill([ - 'key' => strtolower($type), - 'user_id' => $this->user->id, + 'type' => strtolower($type), + 'user_id' => user()->id, ]); } @@ -51,9 +49,9 @@ class ActivityService public function removeEntity(Entity $entity) { $entity->activity()->update([ - 'extra' => $entity->name, - 'entity_id' => 0, - 'entity_type' => '', + 'detail' => $entity->name, + 'entity_id' => null, + 'entity_type' => null, ]); } @@ -150,9 +148,9 @@ class ActivityService /** * Flashes a notification message to the session if an appropriate message is available. */ - protected function setNotification(string $activityKey) + protected function setNotification(string $type) { - $notificationTextKey = 'activities.' . $activityKey . '_notification'; + $notificationTextKey = 'activities.' . $type . '_notification'; if (trans()->has($notificationTextKey)) { $message = trans($notificationTextKey); session()->flash('success', $message); diff --git a/app/Entities/BookChild.php b/app/Entities/BookChild.php index 6eac4375d..042b56e28 100644 --- a/app/Entities/BookChild.php +++ b/app/Entities/BookChild.php @@ -45,9 +45,6 @@ class BookChild extends Entity $this->save(); $this->refresh(); - // Update related activity - $this->activity()->update(['book_id' => $newBookId]); - // Update all child pages if a chapter if ($this instanceof Chapter) { foreach ($this->pages as $page) { diff --git a/app/Http/Controllers/AuditLogController.php b/app/Http/Controllers/AuditLogController.php index fad4e8d38..eb6eecc94 100644 --- a/app/Http/Controllers/AuditLogController.php +++ b/app/Http/Controllers/AuditLogController.php @@ -32,7 +32,7 @@ class AuditLogController extends Controller ->orderBy($listDetails['sort'], $listDetails['order']); if ($listDetails['event']) { - $query->where('key', '=', $listDetails['event']); + $query->where('type', '=', $listDetails['event']); } if ($listDetails['date_from']) { @@ -45,12 +45,12 @@ class AuditLogController extends Controller $activities = $query->paginate(100); $activities->appends($listDetails); - $keys = DB::table('activities')->select('key')->distinct()->pluck('key'); + $types = DB::table('activities')->select('type')->distinct()->pluck('type'); $this->setPageTitle(trans('settings.audit')); return view('settings.audit', [ 'activities' => $activities, 'listDetails' => $listDetails, - 'activityKeys' => $keys, + 'activityTypes' => $types, ]); } } diff --git a/app/Http/Controllers/BookSortController.php b/app/Http/Controllers/BookSortController.php index fb9308b46..9375b618a 100644 --- a/app/Http/Controllers/BookSortController.php +++ b/app/Http/Controllers/BookSortController.php @@ -15,10 +15,6 @@ class BookSortController extends Controller protected $bookRepo; - /** - * BookSortController constructor. - * @param $bookRepo - */ public function __construct(BookRepo $bookRepo) { $this->bookRepo = $bookRepo; diff --git a/database/migrations/2020_11_07_232321_simplify_activities_table.php b/database/migrations/2020_11_07_232321_simplify_activities_table.php new file mode 100644 index 000000000..828dbc656 --- /dev/null +++ b/database/migrations/2020_11_07_232321_simplify_activities_table.php @@ -0,0 +1,58 @@ +renameColumn('key', 'type'); + $table->renameColumn('extra', 'detail'); + $table->dropColumn('book_id'); + $table->integer('entity_id')->nullable()->change(); + $table->string('entity_type', 191)->nullable()->change(); + }); + + DB::table('activities') + ->where('entity_id', '=', 0) + ->update([ + 'entity_id' => null, + 'entity_type' => null, + ]); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + DB::table('activities') + ->whereNull('entity_id') + ->update([ + 'entity_id' => 0, + 'entity_type' => '', + ]); + + Schema::table('activities', function (Blueprint $table) { + $table->renameColumn('type', 'key'); + $table->renameColumn('detail', 'extra'); + $table->integer('book_id'); + + $table->integer('entity_id')->change(); + $table->string('entity_type', 191)->change(); + + $table->index('book_id'); + }); + } +} diff --git a/resources/views/settings/audit.blade.php b/resources/views/settings/audit.blade.php index 47a2355d1..7bbf0ed1a 100644 --- a/resources/views/settings/audit.blade.php +++ b/resources/views/settings/audit.blade.php @@ -19,8 +19,8 @@ @@ -62,7 +62,7 @@ @include('partials.table-user', ['user' => $activity->user, 'user_id' => $activity->user_id]) - {{ $activity->key }} + {{ $activity->type }} @if($activity->entity) @@ -71,10 +71,10 @@ {{ $activity->entity->name }} - @elseif($activity->extra) + @elseif($activity->detail)
{{ trans('settings.audit_deleted_item') }}
- {{ trans('settings.audit_deleted_item_name', ['name' => $activity->extra]) }} + {{ trans('settings.audit_deleted_item_name', ['name' => $activity->detail]) }}
@endif diff --git a/tests/AuditLogTest.php b/tests/AuditLogTest.php index 94eb02599..efe842aa1 100644 --- a/tests/AuditLogTest.php +++ b/tests/AuditLogTest.php @@ -2,6 +2,7 @@ use BookStack\Actions\Activity; use BookStack\Actions\ActivityService; +use BookStack\Actions\ActivityType; use BookStack\Auth\UserRepo; use BookStack\Entities\Managers\TrashCan; use BookStack\Entities\Page; @@ -10,6 +11,14 @@ use Carbon\Carbon; class AuditLogTest extends TestCase { + /** @var ActivityService */ + protected $activityService; + + public function setUp(): void + { + parent::setUp(); + $this->activityService = app(ActivityService::class); + } public function test_only_accessible_with_right_permissions() { @@ -34,7 +43,7 @@ class AuditLogTest extends TestCase $admin = $this->getAdmin(); $this->actingAs($admin); $page = Page::query()->first(); - app(ActivityService::class)->add($page, 'page_create', $page->book->id); + $this->activityService->addForEntity($page, ActivityType::PAGE_CREATE); $activity = Activity::query()->orderBy('id', 'desc')->first(); $resp = $this->get('settings/audit'); @@ -49,7 +58,7 @@ class AuditLogTest extends TestCase $this->actingAs( $this->getAdmin()); $page = Page::query()->first(); $pageName = $page->name; - app(ActivityService::class)->add($page, 'page_create', $page->book->id); + $this->activityService->addForEntity($page, ActivityType::PAGE_CREATE); app(PageRepo::class)->destroy($page); app(TrashCan::class)->empty(); @@ -64,7 +73,7 @@ class AuditLogTest extends TestCase $viewer = $this->getViewer(); $this->actingAs($viewer); $page = Page::query()->first(); - app(ActivityService::class)->add($page, 'page_create', $page->book->id); + $this->activityService->addForEntity($page, ActivityType::PAGE_CREATE); $this->actingAs($this->getAdmin()); app(UserRepo::class)->destroy($viewer); @@ -77,7 +86,7 @@ class AuditLogTest extends TestCase { $this->actingAs($this->getAdmin()); $page = Page::query()->first(); - app(ActivityService::class)->add($page, 'page_create', $page->book->id); + $this->activityService->addForEntity($page, ActivityType::PAGE_CREATE); $resp = $this->get('settings/audit'); $resp->assertSeeText($page->name); @@ -90,7 +99,7 @@ class AuditLogTest extends TestCase { $this->actingAs($this->getAdmin()); $page = Page::query()->first(); - app(ActivityService::class)->add($page, 'page_create', $page->book->id); + $this->activityService->addForEntity($page, ActivityType::PAGE_CREATE); $yesterday = (Carbon::now()->subDay()->format('Y-m-d')); $tomorrow = (Carbon::now()->addDay()->format('Y-m-d')); diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index 69233e287..ca90bf055 100644 --- a/tests/CommandsTest.php +++ b/tests/CommandsTest.php @@ -41,7 +41,7 @@ class CommandsTest extends TestCase \Activity::addForEntity($page, ActivityType::PAGE_UPDATE); $this->assertDatabaseHas('activities', [ - 'key' => 'page_update', + 'type' => 'page_update', 'entity_id' => $page->id, 'user_id' => $this->getEditor()->id ]); @@ -51,7 +51,7 @@ class CommandsTest extends TestCase $this->assertDatabaseMissing('activities', [ - 'key' => 'page_update' + 'type' => 'page_update' ]); } diff --git a/tests/Entity/SortTest.php b/tests/Entity/SortTest.php index 28c3adf31..d510a20ca 100644 --- a/tests/Entity/SortTest.php +++ b/tests/Entity/SortTest.php @@ -79,7 +79,7 @@ class SortTest extends TestCase $movePageResp = $this->actingAs($this->getEditor())->put($page->getUrl('/move'), [ 'entity_selection' => 'book:' . $newBook->id ]); - $page = Page::find($page->id); + $page->refresh(); $movePageResp->assertRedirect($page->getUrl()); $this->assertTrue($page->book->id == $newBook->id, 'Page parent is now the new book'); diff --git a/tests/RecycleBinTest.php b/tests/RecycleBinTest.php index b6a9dc791..505ee6939 100644 --- a/tests/RecycleBinTest.php +++ b/tests/RecycleBinTest.php @@ -136,7 +136,7 @@ class RecycleBinTest extends TestCase $deletion = $page->deletions()->firstOrFail(); $this->assertDatabaseHas('activities', [ - 'key' => 'page_delete', + 'type' => 'page_delete', 'entity_id' => $page->id, 'entity_type' => $page->getMorphClass(), ]); @@ -144,16 +144,16 @@ class RecycleBinTest extends TestCase $this->asAdmin()->delete("/settings/recycle-bin/{$deletion->id}"); $this->assertDatabaseMissing('activities', [ - 'key' => 'page_delete', + 'type' => 'page_delete', 'entity_id' => $page->id, 'entity_type' => $page->getMorphClass(), ]); $this->assertDatabaseHas('activities', [ - 'key' => 'page_delete', - 'entity_id' => 0, - 'entity_type' => '', - 'extra' => $page->name, + 'type' => 'page_delete', + 'entity_id' => null, + 'entity_type' => null, + 'detail' => $page->name, ]); } diff --git a/tests/TestCase.php b/tests/TestCase.php index 1f1d5ece7..b5c773037 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -53,9 +53,9 @@ abstract class TestCase extends BaseTestCase * Assert that an activity entry exists of the given key. * Checks the activity belongs to the given entity if provided. */ - protected function assertActivityExists(string $key, Entity $entity = null) + protected function assertActivityExists(string $type, Entity $entity = null) { - $detailsToCheck = ['key' => $key]; + $detailsToCheck = ['type' => $type]; if ($entity) { $detailsToCheck['entity_type'] = $entity->getMorphClass(); From 3f7180fa99cbca96f83fcc8d95b3be08adfbb65f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 18 Nov 2020 23:38:44 +0000 Subject: [PATCH 4/7] Started widening of activity logging In progress, Need to implement much of the logging in controllers. Also cleaned up base controller along the way. --- app/Actions/ActivityService.php | 17 ++++ app/Actions/ActivityType.php | 32 +++++++- app/Entities/Deletion.php | 8 +- app/Entities/Repos/BaseRepo.php | 8 +- .../Api/BookshelfApiController.php | 2 - app/Http/Controllers/Controller.php | 82 ++++++++----------- .../Controllers/MaintenanceController.php | 3 + app/Http/Controllers/RecycleBinController.php | 4 + app/Http/Controllers/SettingController.php | 7 +- app/Interfaces/Loggable.php | 11 +++ resources/lang/en/activities.php | 1 + 11 files changed, 115 insertions(+), 60 deletions(-) create mode 100644 app/Interfaces/Loggable.php diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index fb4a739cd..0b3b0f0bc 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -5,6 +5,7 @@ use BookStack\Auth\User; use BookStack\Entities\Chapter; use BookStack\Entities\Entity; use BookStack\Entities\Page; +use BookStack\Interfaces\Loggable; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Support\Facades\Log; @@ -30,6 +31,22 @@ class ActivityService $this->setNotification($type); } + /** + * Add a generic activity event to the database. + * @param string|Loggable $detail + */ + public function add(string $type, $detail = '') + { + if ($detail instanceof Loggable) { + $detail = $detail->logDescriptor(); + } + + $activity = $this->newActivityForUser($type); + $activity->detail = $detail; + $activity->save(); + $this->setNotification($type); + } + /** * Get a new activity instance for the current user. */ diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php index 5404d884c..9b4e86a16 100644 --- a/app/Actions/ActivityType.php +++ b/app/Actions/ActivityType.php @@ -7,16 +7,46 @@ class ActivityType const PAGE_DELETE = 'page_delete'; const PAGE_RESTORE = 'page_restore'; const PAGE_MOVE = 'page_move'; - const COMMENTED_ON = 'commented_on'; + const CHAPTER_CREATE = 'chapter_create'; const CHAPTER_UPDATE = 'chapter_update'; const CHAPTER_DELETE = 'chapter_delete'; const CHAPTER_MOVE = 'chapter_move'; + const BOOK_CREATE = 'book_create'; const BOOK_UPDATE = 'book_update'; const BOOK_DELETE = 'book_delete'; const BOOK_SORT = 'book_sort'; + const BOOKSHELF_CREATE = 'bookshelf_create'; const BOOKSHELF_UPDATE = 'bookshelf_update'; const BOOKSHELF_DELETE = 'bookshelf_delete'; + + const COMMENTED_ON = 'commented_on'; + const PERMISSIONS_UPDATE = 'permissions_update'; + + const SETTINGS_UPDATE = 'settings_update'; + const MAINTENANCE_ACTION_RUN = 'maintenance_action_run'; + + const RECYCLE_BIN_EMPTY = 'recycle_bin_empty'; + const RECYCLE_BIN_RESTORE = 'recycle_bin_restore'; + const RECYCLE_BIN_DESTROY = 'recycle_bin_destroy'; + + // TODO - Implement all below + const USER_CREATE = 'user_create'; + const USER_UPDATE = 'user_update'; + const USER_DELETE = 'user_delete'; + + const API_TOKEN_CREATE = 'api_token_create'; + const API_TOKEN_UPDATE = 'api_token_update'; + const API_TOKEN_DELETE = 'api_token_delete'; + + const ROLE_CREATE = 'role_create'; + const ROLE_UPDATE = 'role_update'; + const ROLE_DELETE = 'role_delete'; + + const ACCESS_PASSWORD_RESET = 'access_password_reset_request'; + const ACCESS_PASSWORD_RESET_UPDATE = 'access_password_reset_update'; + const ACCESS_LOGIN = 'access_login'; + const ACCESS_FAILED_LOGIN = 'access_failed_login'; } \ No newline at end of file diff --git a/app/Entities/Deletion.php b/app/Entities/Deletion.php index 576862caa..dab44d4ad 100644 --- a/app/Entities/Deletion.php +++ b/app/Entities/Deletion.php @@ -1,11 +1,12 @@ deletable()->first(); + return "Deletion ({$this->id}) for {$deletable->getType()} ({$deletable->id}) {$deletable->name}"; + } } diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index 7c25e4981..a9d599eb2 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -2,11 +2,12 @@ namespace BookStack\Entities\Repos; +use BookStack\Actions\ActivityType; use BookStack\Actions\TagRepo; -use BookStack\Entities\Book; use BookStack\Entities\Entity; use BookStack\Entities\HasCoverImage; use BookStack\Exceptions\ImageUploadException; +use BookStack\Facades\Activity; use BookStack\Uploads\ImageRepo; use Illuminate\Http\UploadedFile; use Illuminate\Support\Collection; @@ -18,10 +19,6 @@ class BaseRepo protected $imageRepo; - /** - * BaseRepo constructor. - * @param $tagRepo - */ public function __construct(TagRepo $tagRepo, ImageRepo $imageRepo) { $this->tagRepo = $tagRepo; @@ -115,5 +112,6 @@ class BaseRepo $entity->save(); $entity->rebuildPermissions(); + Activity::addForEntity($entity, ActivityType::PERMISSIONS_UPDATE); } } diff --git a/app/Http/Controllers/Api/BookshelfApiController.php b/app/Http/Controllers/Api/BookshelfApiController.php index 4650e1dde..6792f9f45 100644 --- a/app/Http/Controllers/Api/BookshelfApiController.php +++ b/app/Http/Controllers/Api/BookshelfApiController.php @@ -1,7 +1,5 @@ share('pageTitle', $title); } @@ -67,79 +65,59 @@ abstract class Controller extends BaseController } /** - * Checks for a permission. - * @param string $permissionName - * @return bool|\Illuminate\Http\RedirectResponse + * Checks that the current user has the given permission otherwise throw an exception. */ - protected function checkPermission($permissionName) + protected function checkPermission(string $permission): void { - if (!user() || !user()->can($permissionName)) { + if (!user() || !user()->can($permission)) { $this->showPermissionError(); } - return true; } /** - * Check the current user's permissions against an ownable item. - * @param $permission - * @param Ownable $ownable - * @return bool + * Check the current user's permissions against an ownable item otherwise throw an exception. */ - protected function checkOwnablePermission($permission, Ownable $ownable) + protected function checkOwnablePermission(string $permission, Ownable $ownable): void { - if (userCan($permission, $ownable)) { - return true; + if (!userCan($permission, $ownable)) { + $this->showPermissionError(); } - return $this->showPermissionError(); } /** - * Check if a user has a permission or bypass if the callback is true. - * @param $permissionName - * @param $callback - * @return bool + * Check if a user has a permission or bypass the permission + * check if the given callback resolves true. */ - protected function checkPermissionOr($permissionName, $callback) + protected function checkPermissionOr(string $permission, callable $callback): void { - $callbackResult = $callback(); - if ($callbackResult === false) { - $this->checkPermission($permissionName); + if ($callback() !== true) { + $this->checkPermission($permission); } - return true; } /** * Check if the current user has a permission or bypass if the provided user * id matches the current user. - * @param string $permissionName - * @param int $userId - * @return bool */ - protected function checkPermissionOrCurrentUser(string $permissionName, int $userId) + protected function checkPermissionOrCurrentUser(string $permission, int $userId): void { - return $this->checkPermissionOr($permissionName, function () use ($userId) { + $this->checkPermissionOr($permission, function () use ($userId) { return $userId === user()->id; }); } /** * Send back a json error message. - * @param string $messageText - * @param int $statusCode - * @return mixed */ - protected function jsonError($messageText = "", $statusCode = 500) + protected function jsonError(string $messageText = "", int $statusCode = 500): JsonResponse { return response()->json(['message' => $messageText, 'status' => 'error'], $statusCode); } /** * Create a response that forces a download in the browser. - * @param string $content - * @param string $fileName - * @return \Illuminate\Http\Response */ - protected function downloadResponse(string $content, string $fileName) + protected function downloadResponse(string $content, string $fileName): Response { return response()->make($content, 200, [ 'Content-Type' => 'application/octet-stream', @@ -149,31 +127,37 @@ abstract class Controller extends BaseController /** * Show a positive, successful notification to the user on next view load. - * @param string $message */ - protected function showSuccessNotification(string $message) + protected function showSuccessNotification(string $message): void { session()->flash('success', $message); } /** * Show a warning notification to the user on next view load. - * @param string $message */ - protected function showWarningNotification(string $message) + protected function showWarningNotification(string $message): void { session()->flash('warning', $message); } /** * Show an error notification to the user on next view load. - * @param string $message */ - protected function showErrorNotification(string $message) + protected function showErrorNotification(string $message): void { session()->flash('error', $message); } + /** + * Log an activity in the system. + * @param string|Loggable + */ + protected function logActivity(string $type, $detail = ''): void + { + Activity::add($type, $detail); + } + /** * Get the validation rules for image files. */ diff --git a/app/Http/Controllers/MaintenanceController.php b/app/Http/Controllers/MaintenanceController.php index 0d6265f90..a638b69cd 100644 --- a/app/Http/Controllers/MaintenanceController.php +++ b/app/Http/Controllers/MaintenanceController.php @@ -2,6 +2,7 @@ namespace BookStack\Http\Controllers; +use BookStack\Actions\ActivityType; use BookStack\Entities\Managers\TrashCan; use BookStack\Notifications\TestEmail; use BookStack\Uploads\ImageService; @@ -35,6 +36,7 @@ class MaintenanceController extends Controller public function cleanupImages(Request $request, ImageService $imageService) { $this->checkPermission('settings-manage'); + $this->logActivity(ActivityType::MAINTENANCE_ACTION_RUN, 'cleanup-images'); $checkRevisions = !($request->get('ignore_revisions', 'false') === 'true'); $dryRun = !($request->has('confirm')); @@ -61,6 +63,7 @@ class MaintenanceController extends Controller public function sendTestEmail() { $this->checkPermission('settings-manage'); + $this->logActivity(ActivityType::MAINTENANCE_ACTION_RUN, 'send-test-email'); try { user()->notify(new TestEmail()); diff --git a/app/Http/Controllers/RecycleBinController.php b/app/Http/Controllers/RecycleBinController.php index 459dbb39d..57038046a 100644 --- a/app/Http/Controllers/RecycleBinController.php +++ b/app/Http/Controllers/RecycleBinController.php @@ -1,5 +1,6 @@ findOrFail($id); + $this->logActivity(ActivityType::RECYCLE_BIN_RESTORE, $deletion); $restoreCount = (new TrashCan())->restoreFromDeletion($deletion); $this->showSuccessNotification(trans('settings.recycle_bin_restore_notification', ['count' => $restoreCount])); @@ -85,6 +87,7 @@ class RecycleBinController extends Controller $deletion = Deletion::query()->findOrFail($id); $deleteCount = (new TrashCan())->destroyFromDeletion($deletion); + $this->logActivity(ActivityType::RECYCLE_BIN_DESTROY, $deletion); $this->showSuccessNotification(trans('settings.recycle_bin_destroy_notification', ['count' => $deleteCount])); return redirect($this->recycleBinBaseUrl); } @@ -97,6 +100,7 @@ class RecycleBinController extends Controller { $deleteCount = (new TrashCan())->empty(); + $this->logActivity(ActivityType::RECYCLE_BIN_EMPTY); $this->showSuccessNotification(trans('settings.recycle_bin_destroy_notification', ['count' => $deleteCount])); return redirect($this->recycleBinBaseUrl); } diff --git a/app/Http/Controllers/SettingController.php b/app/Http/Controllers/SettingController.php index 50d91d388..92435f924 100644 --- a/app/Http/Controllers/SettingController.php +++ b/app/Http/Controllers/SettingController.php @@ -1,5 +1,6 @@ all() as $name => $value) { + $key = str_replace('setting-', '', trim($name)); if (strpos($name, 'setting-') !== 0) { continue; } - $key = str_replace('setting-', '', trim($name)); setting()->put($key, $value); } @@ -68,8 +69,10 @@ class SettingController extends Controller setting()->remove('app-logo'); } + $section = $request->get('section', ''); + $this->logActivity(ActivityType::SETTINGS_UPDATE, $section); $this->showSuccessNotification(trans('settings.settings_save_success')); - $redirectLocation = '/settings#' . $request->get('section', ''); + $redirectLocation = '/settings#' . $section; return redirect(rtrim($redirectLocation, '#')); } } diff --git a/app/Interfaces/Loggable.php b/app/Interfaces/Loggable.php new file mode 100644 index 000000000..33e1d7c95 --- /dev/null +++ b/app/Interfaces/Loggable.php @@ -0,0 +1,11 @@ + 'commented on', + 'permissions_update' => 'updated permissions', ]; From da37700ac23b0a3789c7da1ed3517d1900d5894d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 20 Nov 2020 18:53:01 +0000 Subject: [PATCH 5/7] Implemented user, api_tokem & role activity logging Also refactored some role content, primarily updating the permission controller to be RoleController since it only dealt with roles. --- app/Actions/ActivityType.php | 2 +- app/Api/ApiToken.php | 21 ++++- app/Auth/Permissions/PermissionsRepo.php | 8 +- app/Auth/Role.php | 11 ++- app/Auth/User.php | 11 ++- app/Http/Controllers/RecycleBinController.php | 2 +- ...ssionController.php => RoleController.php} | 16 ++-- .../Controllers/UserApiTokenController.php | 7 +- app/Http/Controllers/UserController.php | 4 + routes/web.php | 14 ++-- tests/Permissions/RolesTest.php | 77 ++++++++++--------- tests/User/UserApiTokenTest.php | 4 + 12 files changed, 118 insertions(+), 59 deletions(-) rename app/Http/Controllers/{PermissionController.php => RoleController.php} (90%) diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php index 9b4e86a16..376312cbb 100644 --- a/app/Actions/ActivityType.php +++ b/app/Actions/ActivityType.php @@ -32,7 +32,6 @@ class ActivityType const RECYCLE_BIN_RESTORE = 'recycle_bin_restore'; const RECYCLE_BIN_DESTROY = 'recycle_bin_destroy'; - // TODO - Implement all below const USER_CREATE = 'user_create'; const USER_UPDATE = 'user_update'; const USER_DELETE = 'user_delete'; @@ -45,6 +44,7 @@ class ActivityType const ROLE_UPDATE = 'role_update'; const ROLE_DELETE = 'role_delete'; + // TODO - Implement all below const ACCESS_PASSWORD_RESET = 'access_password_reset_request'; const ACCESS_PASSWORD_RESET_UPDATE = 'access_password_reset_update'; const ACCESS_LOGIN = 'access_login'; diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php index 523c3b8b8..91c407fd8 100644 --- a/app/Api/ApiToken.php +++ b/app/Api/ApiToken.php @@ -1,11 +1,22 @@ addYears(100)->format('Y-m-d'); } + + /** + * @inheritdoc + */ + public function logDescriptor(): string + { + return "({$this->id}) {$this->name}; User: {$this->user->logDescriptor()}"; + } } diff --git a/app/Auth/Permissions/PermissionsRepo.php b/app/Auth/Permissions/PermissionsRepo.php index ce61093cc..f54612a43 100644 --- a/app/Auth/Permissions/PermissionsRepo.php +++ b/app/Auth/Permissions/PermissionsRepo.php @@ -1,10 +1,11 @@ assignRolePermissions($role, $permissions); $this->permissionService->buildJointPermissionForRole($role); + Activity::add(ActivityType::ROLE_CREATE, $role); return $role; } @@ -88,12 +90,13 @@ class PermissionsRepo $role->fill($roleData); $role->save(); $this->permissionService->buildJointPermissionForRole($role); + Activity::add(ActivityType::ROLE_UPDATE, $role); } /** * Assign an list of permission names to an role. */ - public function assignRolePermissions(Role $role, array $permissionNameArray = []) + protected function assignRolePermissions(Role $role, array $permissionNameArray = []) { $permissions = []; $permissionNameArray = array_values($permissionNameArray); @@ -137,6 +140,7 @@ class PermissionsRepo } $this->permissionService->deleteJointPermissionsForRole($role); + Activity::add(ActivityType::ROLE_DELETE, $role); $role->delete(); } } diff --git a/app/Auth/Role.php b/app/Auth/Role.php index 13ec6df16..255158afb 100644 --- a/app/Auth/Role.php +++ b/app/Auth/Role.php @@ -2,6 +2,7 @@ use BookStack\Auth\Permissions\JointPermission; use BookStack\Auth\Permissions\RolePermission; +use BookStack\Interfaces\Loggable; use BookStack\Model; use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Relations\HasMany; @@ -14,7 +15,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany; * @property string $external_auth_id * @property string $system_name */ -class Role extends Model +class Role extends Model implements Loggable { protected $fillable = ['display_name', 'description', 'external_auth_id']; @@ -104,4 +105,12 @@ class Role extends Model { return static::query()->where('system_name', '!=', 'admin')->get(); } + + /** + * @inheritdoc + */ + public function logDescriptor(): string + { + return "({$this->id}) {$this->display_name}"; + } } diff --git a/app/Auth/User.php b/app/Auth/User.php index f65ef5316..6bf656be3 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -1,6 +1,7 @@ notify(new ResetPassword($token)); } + + /** + * @inheritdoc + */ + public function logDescriptor(): string + { + return "({$this->id}) {$this->name}"; + } } diff --git a/app/Http/Controllers/RecycleBinController.php b/app/Http/Controllers/RecycleBinController.php index 57038046a..928310779 100644 --- a/app/Http/Controllers/RecycleBinController.php +++ b/app/Http/Controllers/RecycleBinController.php @@ -85,9 +85,9 @@ class RecycleBinController extends Controller { /** @var Deletion $deletion */ $deletion = Deletion::query()->findOrFail($id); + $this->logActivity(ActivityType::RECYCLE_BIN_DESTROY, $deletion); $deleteCount = (new TrashCan())->destroyFromDeletion($deletion); - $this->logActivity(ActivityType::RECYCLE_BIN_DESTROY, $deletion); $this->showSuccessNotification(trans('settings.recycle_bin_destroy_notification', ['count' => $deleteCount])); return redirect($this->recycleBinBaseUrl); } diff --git a/app/Http/Controllers/PermissionController.php b/app/Http/Controllers/RoleController.php similarity index 90% rename from app/Http/Controllers/PermissionController.php rename to app/Http/Controllers/RoleController.php index 1200d44ab..6e3d5afdc 100644 --- a/app/Http/Controllers/PermissionController.php +++ b/app/Http/Controllers/RoleController.php @@ -6,7 +6,7 @@ use Exception; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; -class PermissionController extends Controller +class RoleController extends Controller { protected $permissionsRepo; @@ -23,7 +23,7 @@ class PermissionController extends Controller /** * Show a listing of the roles in the system. */ - public function listRoles() + public function list() { $this->checkPermission('user-roles-manage'); $roles = $this->permissionsRepo->getAllRoles(); @@ -33,7 +33,7 @@ class PermissionController extends Controller /** * Show the form to create a new role */ - public function createRole() + public function create() { $this->checkPermission('user-roles-manage'); return view('settings.roles.create'); @@ -42,7 +42,7 @@ class PermissionController extends Controller /** * Store a new role in the system. */ - public function storeRole(Request $request) + public function store(Request $request) { $this->checkPermission('user-roles-manage'); $this->validate($request, [ @@ -59,7 +59,7 @@ class PermissionController extends Controller * Show the form for editing a user role. * @throws PermissionsException */ - public function editRole(string $id) + public function edit(string $id) { $this->checkPermission('user-roles-manage'); $role = $this->permissionsRepo->getRoleById($id); @@ -73,7 +73,7 @@ class PermissionController extends Controller * Updates a user role. * @throws ValidationException */ - public function updateRole(Request $request, string $id) + public function update(Request $request, string $id) { $this->checkPermission('user-roles-manage'); $this->validate($request, [ @@ -90,7 +90,7 @@ class PermissionController extends Controller * Show the view to delete a role. * Offers the chance to migrate users. */ - public function showDeleteRole(string $id) + public function showDelete(string $id) { $this->checkPermission('user-roles-manage'); $role = $this->permissionsRepo->getRoleById($id); @@ -105,7 +105,7 @@ class PermissionController extends Controller * Migrate from a previous role if set. * @throws Exception */ - public function deleteRole(Request $request, string $id) + public function delete(Request $request, string $id) { $this->checkPermission('user-roles-manage'); diff --git a/app/Http/Controllers/UserApiTokenController.php b/app/Http/Controllers/UserApiTokenController.php index 55675233c..ab0e9069e 100644 --- a/app/Http/Controllers/UserApiTokenController.php +++ b/app/Http/Controllers/UserApiTokenController.php @@ -1,9 +1,9 @@ flash('api-token-secret:' . $token->id, $secret); $this->showSuccessNotification(trans('settings.user_api_token_create_success')); + $this->logActivity(ActivityType::API_TOKEN_CREATE, $token); + return redirect($user->getEditUrl('/api-tokens/' . $token->id)); } @@ -93,6 +95,7 @@ class UserApiTokenController extends Controller ])->save(); $this->showSuccessNotification(trans('settings.user_api_token_update_success')); + $this->logActivity(ActivityType::API_TOKEN_UPDATE, $token); return redirect($user->getEditUrl('/api-tokens/' . $token->id)); } @@ -117,6 +120,8 @@ class UserApiTokenController extends Controller $token->delete(); $this->showSuccessNotification(trans('settings.user_api_token_delete_success')); + $this->logActivity(ActivityType::API_TOKEN_DELETE, $token); + return redirect($user->getEditUrl('#api_tokens')); } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 651dedc0d..b1b7e5c44 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -1,5 +1,6 @@ userRepo->downloadAndAssignUserAvatar($user); + $this->logActivity(ActivityType::USER_CREATE, $user); return redirect('/settings/users'); } @@ -194,6 +196,7 @@ class UserController extends Controller $user->save(); $this->showSuccessNotification(trans('settings.users_edit_success')); + $this->logActivity(ActivityType::USER_UPDATE, $user); $redirectUrl = userCan('users-manage') ? '/settings/users' : ('/settings/users/' . $user->id); return redirect($redirectUrl); @@ -234,6 +237,7 @@ class UserController extends Controller $this->userRepo->destroy($user); $this->showSuccessNotification(trans('settings.users_delete_success')); + $this->logActivity(ActivityType::USER_DELETE, $user); return redirect('/settings/users'); } diff --git a/routes/web.php b/routes/web.php index b87355105..afefcb99e 100644 --- a/routes/web.php +++ b/routes/web.php @@ -201,13 +201,13 @@ Route::group(['middleware' => 'auth'], function () { Route::delete('/users/{userId}/api-tokens/{tokenId}', 'UserApiTokenController@destroy'); // Roles - Route::get('/roles', 'PermissionController@listRoles'); - Route::get('/roles/new', 'PermissionController@createRole'); - Route::post('/roles/new', 'PermissionController@storeRole'); - Route::get('/roles/delete/{id}', 'PermissionController@showDeleteRole'); - Route::delete('/roles/delete/{id}', 'PermissionController@deleteRole'); - Route::get('/roles/{id}', 'PermissionController@editRole'); - Route::put('/roles/{id}', 'PermissionController@updateRole'); + Route::get('/roles', 'RoleController@list'); + Route::get('/roles/new', 'RoleController@create'); + Route::post('/roles/new', 'RoleController@store'); + Route::get('/roles/delete/{id}', 'RoleController@showDelete'); + Route::delete('/roles/delete/{id}', 'RoleController@delete'); + Route::get('/roles/{id}', 'RoleController@edit'); + Route::put('/roles/{id}', 'RoleController@update'); }); }); diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php index 73060c834..6bfa8067f 100644 --- a/tests/Permissions/RolesTest.php +++ b/tests/Permissions/RolesTest.php @@ -1,8 +1,13 @@ id; $this->asAdmin()->visit($deletePageUrl) ->press('Confirm') @@ -195,7 +200,7 @@ class RolesTest extends BrowserKitTest public function test_restrictions_manage_all_permission() { - $page = \BookStack\Entities\Page::take(1)->get()->first(); + $page = Page::take(1)->get()->first(); $this->actingAs($this->user)->visit($page->getUrl()) ->dontSee('Permissions') ->visit($page->getUrl() . '/permissions') @@ -209,7 +214,7 @@ class RolesTest extends BrowserKitTest public function test_restrictions_manage_own_permission() { - $otherUsersPage = \BookStack\Entities\Page::first(); + $otherUsersPage = Page::first(); $content = $this->createEntityChainBelongingToUser($this->user); // Check can't restrict other's content $this->actingAs($this->user)->visit($otherUsersPage->getUrl()) @@ -301,7 +306,7 @@ class RolesTest extends BrowserKitTest public function test_bookshelves_edit_all_permission() { - $otherShelf = \BookStack\Entities\Bookshelf::first(); + $otherShelf = Bookshelf::first(); $this->checkAccessPermission('bookshelf-update-all', [ $otherShelf->getUrl('/edit') ], [ @@ -312,7 +317,7 @@ class RolesTest extends BrowserKitTest public function test_bookshelves_delete_own_permission() { $this->giveUserPermissions($this->user, ['bookshelf-update-all']); - $otherShelf = \BookStack\Entities\Bookshelf::first(); + $otherShelf = Bookshelf::first(); $ownShelf = $this->newShelf(['name' => 'test-shelf', 'slug' => 'test-shelf']); $ownShelf->forceFill(['created_by' => $this->user->id, 'updated_by' => $this->user->id])->save(); $this->regenEntityPermissions($ownShelf); @@ -336,7 +341,7 @@ class RolesTest extends BrowserKitTest public function test_bookshelves_delete_all_permission() { $this->giveUserPermissions($this->user, ['bookshelf-update-all']); - $otherShelf = \BookStack\Entities\Bookshelf::first(); + $otherShelf = Bookshelf::first(); $this->checkAccessPermission('bookshelf-delete-all', [ $otherShelf->getUrl('/delete') ], [ @@ -366,7 +371,7 @@ class RolesTest extends BrowserKitTest public function test_books_edit_own_permission() { - $otherBook = \BookStack\Entities\Book::take(1)->get()->first(); + $otherBook = Book::take(1)->get()->first(); $ownBook = $this->createEntityChainBelongingToUser($this->user)['book']; $this->checkAccessPermission('book-update-own', [ $ownBook->getUrl() . '/edit' @@ -382,7 +387,7 @@ class RolesTest extends BrowserKitTest public function test_books_edit_all_permission() { - $otherBook = \BookStack\Entities\Book::take(1)->get()->first(); + $otherBook = Book::take(1)->get()->first(); $this->checkAccessPermission('book-update-all', [ $otherBook->getUrl() . '/edit' ], [ @@ -393,7 +398,7 @@ class RolesTest extends BrowserKitTest public function test_books_delete_own_permission() { $this->giveUserPermissions($this->user, ['book-update-all']); - $otherBook = \BookStack\Entities\Book::take(1)->get()->first(); + $otherBook = Book::take(1)->get()->first(); $ownBook = $this->createEntityChainBelongingToUser($this->user)['book']; $this->checkAccessPermission('book-delete-own', [ $ownBook->getUrl() . '/delete' @@ -414,7 +419,7 @@ class RolesTest extends BrowserKitTest public function test_books_delete_all_permission() { $this->giveUserPermissions($this->user, ['book-update-all']); - $otherBook = \BookStack\Entities\Book::take(1)->get()->first(); + $otherBook = Book::take(1)->get()->first(); $this->checkAccessPermission('book-delete-all', [ $otherBook->getUrl() . '/delete' ], [ @@ -429,7 +434,7 @@ class RolesTest extends BrowserKitTest public function test_chapter_create_own_permissions() { - $book = \BookStack\Entities\Book::take(1)->get()->first(); + $book = Book::take(1)->get()->first(); $ownBook = $this->createEntityChainBelongingToUser($this->user)['book']; $this->checkAccessPermission('chapter-create-own', [ $ownBook->getUrl('/create-chapter') @@ -451,7 +456,7 @@ class RolesTest extends BrowserKitTest public function test_chapter_create_all_permissions() { - $book = \BookStack\Entities\Book::take(1)->get()->first(); + $book = Book::take(1)->get()->first(); $this->checkAccessPermission('chapter-create-all', [ $book->getUrl('/create-chapter') ], [ @@ -467,7 +472,7 @@ class RolesTest extends BrowserKitTest public function test_chapter_edit_own_permission() { - $otherChapter = \BookStack\Entities\Chapter::take(1)->get()->first(); + $otherChapter = Chapter::take(1)->get()->first(); $ownChapter = $this->createEntityChainBelongingToUser($this->user)['chapter']; $this->checkAccessPermission('chapter-update-own', [ $ownChapter->getUrl() . '/edit' @@ -483,7 +488,7 @@ class RolesTest extends BrowserKitTest public function test_chapter_edit_all_permission() { - $otherChapter = \BookStack\Entities\Chapter::take(1)->get()->first(); + $otherChapter = Chapter::take(1)->get()->first(); $this->checkAccessPermission('chapter-update-all', [ $otherChapter->getUrl() . '/edit' ], [ @@ -494,7 +499,7 @@ class RolesTest extends BrowserKitTest public function test_chapter_delete_own_permission() { $this->giveUserPermissions($this->user, ['chapter-update-all']); - $otherChapter = \BookStack\Entities\Chapter::take(1)->get()->first(); + $otherChapter = Chapter::take(1)->get()->first(); $ownChapter = $this->createEntityChainBelongingToUser($this->user)['chapter']; $this->checkAccessPermission('chapter-delete-own', [ $ownChapter->getUrl() . '/delete' @@ -516,7 +521,7 @@ class RolesTest extends BrowserKitTest public function test_chapter_delete_all_permission() { $this->giveUserPermissions($this->user, ['chapter-update-all']); - $otherChapter = \BookStack\Entities\Chapter::take(1)->get()->first(); + $otherChapter = Chapter::take(1)->get()->first(); $this->checkAccessPermission('chapter-delete-all', [ $otherChapter->getUrl() . '/delete' ], [ @@ -532,8 +537,8 @@ class RolesTest extends BrowserKitTest public function test_page_create_own_permissions() { - $book = \BookStack\Entities\Book::first(); - $chapter = \BookStack\Entities\Chapter::first(); + $book = Book::first(); + $chapter = Chapter::first(); $entities = $this->createEntityChainBelongingToUser($this->user); $ownBook = $entities['book']; @@ -557,7 +562,7 @@ class RolesTest extends BrowserKitTest foreach ($accessUrls as $index => $url) { $this->actingAs($this->user)->visit($url); - $expectedUrl = \BookStack\Entities\Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl(); + $expectedUrl = Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl(); $this->seePageIs($expectedUrl); } @@ -579,8 +584,8 @@ class RolesTest extends BrowserKitTest public function test_page_create_all_permissions() { - $book = \BookStack\Entities\Book::take(1)->get()->first(); - $chapter = \BookStack\Entities\Chapter::take(1)->get()->first(); + $book = Book::take(1)->get()->first(); + $chapter = Chapter::take(1)->get()->first(); $baseUrl = $book->getUrl() . '/page'; $createUrl = $book->getUrl('/create-page'); @@ -601,7 +606,7 @@ class RolesTest extends BrowserKitTest foreach ($accessUrls as $index => $url) { $this->actingAs($this->user)->visit($url); - $expectedUrl = \BookStack\Entities\Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl(); + $expectedUrl = Page::where('draft', '=', true)->orderBy('id', 'desc')->first()->getUrl(); $this->seePageIs($expectedUrl); } @@ -620,7 +625,7 @@ class RolesTest extends BrowserKitTest public function test_page_edit_own_permission() { - $otherPage = \BookStack\Entities\Page::take(1)->get()->first(); + $otherPage = Page::take(1)->get()->first(); $ownPage = $this->createEntityChainBelongingToUser($this->user)['page']; $this->checkAccessPermission('page-update-own', [ $ownPage->getUrl() . '/edit' @@ -636,7 +641,7 @@ class RolesTest extends BrowserKitTest public function test_page_edit_all_permission() { - $otherPage = \BookStack\Entities\Page::take(1)->get()->first(); + $otherPage = Page::take(1)->get()->first(); $this->checkAccessPermission('page-update-all', [ $otherPage->getUrl() . '/edit' ], [ @@ -647,7 +652,7 @@ class RolesTest extends BrowserKitTest public function test_page_delete_own_permission() { $this->giveUserPermissions($this->user, ['page-update-all']); - $otherPage = \BookStack\Entities\Page::take(1)->get()->first(); + $otherPage = Page::take(1)->get()->first(); $ownPage = $this->createEntityChainBelongingToUser($this->user)['page']; $this->checkAccessPermission('page-delete-own', [ $ownPage->getUrl() . '/delete' @@ -669,7 +674,7 @@ class RolesTest extends BrowserKitTest public function test_page_delete_all_permission() { $this->giveUserPermissions($this->user, ['page-update-all']); - $otherPage = \BookStack\Entities\Page::take(1)->get()->first(); + $otherPage = Page::take(1)->get()->first(); $this->checkAccessPermission('page-delete-all', [ $otherPage->getUrl() . '/delete' ], [ @@ -685,7 +690,7 @@ class RolesTest extends BrowserKitTest public function test_public_role_visible_in_user_edit_screen() { - $user = \BookStack\Auth\User::first(); + $user = User::first(); $adminRole = Role::getSystemRole('admin'); $publicRole = Role::getSystemRole('public'); $this->asAdmin()->visit('/settings/users/' . $user->id) @@ -721,8 +726,8 @@ class RolesTest extends BrowserKitTest public function test_image_delete_own_permission() { $this->giveUserPermissions($this->user, ['image-update-all']); - $page = \BookStack\Entities\Page::first(); - $image = factory(\BookStack\Uploads\Image::class)->create(['uploaded_to' => $page->id, 'created_by' => $this->user->id, 'updated_by' => $this->user->id]); + $page = Page::first(); + $image = factory(Image::class)->create(['uploaded_to' => $page->id, 'created_by' => $this->user->id, 'updated_by' => $this->user->id]); $this->actingAs($this->user)->json('delete', '/images/' . $image->id) ->seeStatusCode(403); @@ -738,8 +743,8 @@ class RolesTest extends BrowserKitTest { $this->giveUserPermissions($this->user, ['image-update-all']); $admin = $this->getAdmin(); - $page = \BookStack\Entities\Page::first(); - $image = factory(\BookStack\Uploads\Image::class)->create(['uploaded_to' => $page->id, 'created_by' => $admin->id, 'updated_by' => $admin->id]); + $page = Page::first(); + $image = factory(Image::class)->create(['uploaded_to' => $page->id, 'created_by' => $admin->id, 'updated_by' => $admin->id]); $this->actingAs($this->user)->json('delete', '/images/' . $image->id) ->seeStatusCode(403); @@ -760,7 +765,7 @@ class RolesTest extends BrowserKitTest { // To cover issue fixed in f99c8ff99aee9beb8c692f36d4b84dc6e651e50a. $page = Page::first(); - $viewerRole = \BookStack\Auth\Role::getRole('viewer'); + $viewerRole = Role::getRole('viewer'); $viewer = $this->getViewer(); $this->actingAs($viewer)->visit($page->getUrl())->assertResponseStatus(200); @@ -778,14 +783,14 @@ class RolesTest extends BrowserKitTest { $admin = $this->getAdmin(); // Book links - $book = factory(\BookStack\Entities\Book::class)->create(['created_by' => $admin->id, 'updated_by' => $admin->id]); + $book = factory(Book::class)->create(['created_by' => $admin->id, 'updated_by' => $admin->id]); $this->updateEntityPermissions($book); $this->actingAs($this->getViewer())->visit($book->getUrl()) ->dontSee('Create a new page') ->dontSee('Add a chapter'); // Chapter links - $chapter = factory(\BookStack\Entities\Chapter::class)->create(['created_by' => $admin->id, 'updated_by' => $admin->id, 'book_id' => $book->id]); + $chapter = factory(Chapter::class)->create(['created_by' => $admin->id, 'updated_by' => $admin->id, 'book_id' => $book->id]); $this->updateEntityPermissions($chapter); $this->actingAs($this->getViewer())->visit($chapter->getUrl()) ->dontSee('Create a new page') @@ -869,7 +874,7 @@ class RolesTest extends BrowserKitTest } private function addComment($page) { - $comment = factory(\BookStack\Actions\Comment::class)->make(); + $comment = factory(Comment::class)->make(); $url = "/comment/$page->id"; $request = [ 'text' => $comment->text, @@ -882,7 +887,7 @@ class RolesTest extends BrowserKitTest } private function updateComment($commentId) { - $comment = factory(\BookStack\Actions\Comment::class)->make(); + $comment = factory(Comment::class)->make(); $url = "/comment/$commentId"; $request = [ 'text' => $comment->text, diff --git a/tests/User/UserApiTokenTest.php b/tests/User/UserApiTokenTest.php index c89a590f0..df686dd77 100644 --- a/tests/User/UserApiTokenTest.php +++ b/tests/User/UserApiTokenTest.php @@ -1,5 +1,6 @@ assertTrue(strlen($secret) === 32); $this->assertSessionHas('success'); + $this->assertActivityExists(ActivityType::API_TOKEN_CREATE); } public function test_create_with_no_expiry_sets_expiry_hundred_years_away() @@ -124,6 +126,7 @@ class UserApiTokenTest extends TestCase $this->assertDatabaseHas('api_tokens', array_merge($updateData, ['id' => $token->id])); $this->assertSessionHas('success'); + $this->assertActivityExists(ActivityType::API_TOKEN_UPDATE); } public function test_token_update_with_blank_expiry_sets_to_hundred_years_away() @@ -162,6 +165,7 @@ class UserApiTokenTest extends TestCase $resp = $this->delete($tokenUrl); $resp->assertRedirect($editor->getEditUrl('#api_tokens')); $this->assertDatabaseMissing('api_tokens', ['id' => $token->id]); + $this->assertActivityExists(ActivityType::API_TOKEN_DELETE); } public function test_user_manage_can_delete_token_without_api_permission_themselves() From bd6a1a66d14151ce50505a338a45a2c3be260bcf Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 20 Nov 2020 19:33:11 +0000 Subject: [PATCH 6/7] Implemented remainder of activity types Also fixed audit log to work for non-entity items. --- app/Actions/Activity.php | 11 +++++++++++ app/Actions/ActivityType.php | 9 ++++----- app/Auth/Access/RegistrationService.php | 4 ++++ app/Auth/Access/Saml2Service.php | 3 +++ app/Auth/Access/SocialAuthService.php | 3 +++ app/Auth/SocialAccount.php | 17 ++++++++++++++++- .../Auth/ForgotPasswordController.php | 5 +++++ app/Http/Controllers/Auth/LoginController.php | 3 ++- .../Auth/ResetPasswordController.php | 2 ++ resources/lang/en/settings.php | 2 +- resources/views/settings/audit.blade.php | 6 ++++-- 11 files changed, 55 insertions(+), 10 deletions(-) diff --git a/app/Actions/Activity.php b/app/Actions/Activity.php index 63eda5917..42cc95613 100644 --- a/app/Actions/Activity.php +++ b/app/Actions/Activity.php @@ -6,6 +6,7 @@ use BookStack\Auth\User; use BookStack\Entities\Entity; use BookStack\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Support\Str; /** * @property string $type @@ -46,6 +47,16 @@ class Activity extends Model return trans('activities.' . $this->type); } + /** + * Check if this activity is intended to be for an entity. + */ + public function isForEntity(): bool + { + return Str::startsWith($this->type, [ + 'page_', 'chapter_', 'book_', 'bookshelf_' + ]); + } + /** * Checks if another Activity matches the general information of another. */ diff --git a/app/Actions/ActivityType.php b/app/Actions/ActivityType.php index 376312cbb..216f61249 100644 --- a/app/Actions/ActivityType.php +++ b/app/Actions/ActivityType.php @@ -44,9 +44,8 @@ class ActivityType const ROLE_UPDATE = 'role_update'; const ROLE_DELETE = 'role_delete'; - // TODO - Implement all below - const ACCESS_PASSWORD_RESET = 'access_password_reset_request'; - const ACCESS_PASSWORD_RESET_UPDATE = 'access_password_reset_update'; - const ACCESS_LOGIN = 'access_login'; - const ACCESS_FAILED_LOGIN = 'access_failed_login'; + const AUTH_PASSWORD_RESET = 'auth_password_reset_request'; + const AUTH_PASSWORD_RESET_UPDATE = 'auth_password_reset_update'; + const AUTH_LOGIN = 'auth_login'; + const AUTH_REGISTER = 'auth_register'; } \ No newline at end of file diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index ecc92c117..2aff6c37d 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -1,9 +1,11 @@ socialAccounts()->save($socialAccount); } + Activity::add(ActivityType::AUTH_REGISTER, $socialAccount ?? $newUser); + // Start email confirmation flow if required if ($this->emailConfirmationService->confirmationRequired() && !$emailConfirmed) { $newUser->save(); diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index 89ddd0011..0316ff976 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -1,9 +1,11 @@ login($user); + Activity::add(ActivityType::AUTH_LOGIN, "saml2; {$user->logDescriptor()}"); return $user; } } diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index 657aae3f3..b0383a938 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -1,10 +1,12 @@ login($socialAccount->user); + Activity::add(ActivityType::AUTH_LOGIN, $socialAccount); return redirect()->intended('/'); } diff --git a/app/Auth/SocialAccount.php b/app/Auth/SocialAccount.php index 804dbe629..1c83980cb 100644 --- a/app/Auth/SocialAccount.php +++ b/app/Auth/SocialAccount.php @@ -1,8 +1,15 @@ belongsTo(User::class); } + + /** + * @inheritDoc + */ + public function logDescriptor(): string + { + return "{$this->driver}; {$this->user->logDescriptor()}"; + } } diff --git a/app/Http/Controllers/Auth/ForgotPasswordController.php b/app/Http/Controllers/Auth/ForgotPasswordController.php index fadac641e..31e6d848b 100644 --- a/app/Http/Controllers/Auth/ForgotPasswordController.php +++ b/app/Http/Controllers/Auth/ForgotPasswordController.php @@ -2,6 +2,7 @@ namespace BookStack\Http\Controllers\Auth; +use BookStack\Actions\ActivityType; use BookStack\Http\Controllers\Controller; use Illuminate\Foundation\Auth\SendsPasswordResetEmails; use Illuminate\Http\Request; @@ -52,6 +53,10 @@ class ForgotPasswordController extends Controller $request->only('email') ); + if ($response === Password::RESET_LINK_SENT) { + $this->logActivity(ActivityType::AUTH_PASSWORD_RESET, $request->get('email')); + } + if ($response === Password::RESET_LINK_SENT || $response === Password::INVALID_USER) { $message = trans('auth.reset_password_sent', ['email' => $request->get('email')]); $this->showSuccessNotification($message); diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 8084ce1a5..3890da4b0 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -3,10 +3,10 @@ namespace BookStack\Http\Controllers\Auth; use Activity; +use BookStack\Actions\ActivityType; use BookStack\Auth\Access\SocialAuthService; use BookStack\Exceptions\LoginAttemptEmailNeededException; use BookStack\Exceptions\LoginAttemptException; -use BookStack\Exceptions\UserRegistrationException; use BookStack\Http\Controllers\Controller; use Illuminate\Foundation\Auth\AuthenticatesUsers; use Illuminate\Http\Request; @@ -151,6 +151,7 @@ class LoginController extends Controller } } + $this->logActivity(ActivityType::AUTH_LOGIN, $user); return redirect()->intended($this->redirectPath()); } diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index efdf00159..96f05db26 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -2,6 +2,7 @@ namespace BookStack\Http\Controllers\Auth; +use BookStack\Actions\ActivityType; use BookStack\Http\Controllers\Controller; use Illuminate\Foundation\Auth\ResetsPasswords; use Illuminate\Http\Request; @@ -47,6 +48,7 @@ class ResetPasswordController extends Controller { $message = trans('auth.reset_password_success'); $this->showSuccessNotification($message); + $this->logActivity(ActivityType::AUTH_PASSWORD_RESET_UPDATE, user()); return redirect($this->redirectPath()) ->with('status', trans($response)); } diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 269c775ba..52919d44d 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -111,7 +111,7 @@ return [ 'audit_deleted_item_name' => 'Name: :name', 'audit_table_user' => 'User', 'audit_table_event' => 'Event', - 'audit_table_item' => 'Related Item', + 'audit_table_related' => 'Related Item or Detail', 'audit_table_date' => 'Activity Date', 'audit_date_from' => 'Date Range From', 'audit_date_to' => 'Date Range To', diff --git a/resources/views/settings/audit.blade.php b/resources/views/settings/audit.blade.php index 7bbf0ed1a..1996e1c21 100644 --- a/resources/views/settings/audit.blade.php +++ b/resources/views/settings/audit.blade.php @@ -53,7 +53,7 @@ {{ trans('settings.audit_table_event') }} - {{ trans('settings.audit_table_item') }} + {{ trans('settings.audit_table_related') }} {{ trans('settings.audit_table_date') }} @@ -71,11 +71,13 @@ {{ $activity->entity->name }} - @elseif($activity->detail) + @elseif($activity->detail && $activity->isForEntity())
{{ trans('settings.audit_deleted_item') }}
{{ trans('settings.audit_deleted_item_name', ['name' => $activity->detail]) }}
+ @elseif($activity->detail) +
{{ $activity->detail }}
@endif {{ $activity->created_at }} From c0680d5717a202d0f1ef55bc7e6a428220f94443 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 20 Nov 2020 20:10:18 +0000 Subject: [PATCH 7/7] Added latest activity into users list view --- app/Auth/User.php | 10 ++++++++++ app/Auth/UserRepo.php | 17 ++++++++++++----- resources/lang/en/settings.php | 1 + resources/views/users/index.blade.php | 9 ++++++++- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/app/Auth/User.php b/app/Auth/User.php index 6bf656be3..5feb269e2 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -1,5 +1,6 @@ hasMany(ApiToken::class); } + /** + * Get the latest activity instance for this user. + */ + public function latestActivity(): HasOne + { + return $this->hasOne(Activity::class)->latest(); + } + /** * Get the url for editing this user. */ diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index fdb8c0923..5ae2ed2a2 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -10,6 +10,7 @@ use BookStack\Exceptions\UserUpdateException; use BookStack\Uploads\Image; use Exception; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Pagination\LengthAwarePaginator; use Images; use Log; @@ -56,13 +57,19 @@ class UserRepo /** * Get all the users with their permissions in a paginated format. - * @param int $count - * @param $sortData - * @return Builder|static */ - public function getAllUsersPaginatedAndSorted($count, $sortData) + public function getAllUsersPaginatedAndSorted(int $count, array $sortData): LengthAwarePaginator { - $query = $this->user->with('roles', 'avatar')->orderBy($sortData['sort'], $sortData['order']); + $sort = $sortData['sort']; + if ($sort === 'latest_activity') { + $sort = \BookStack\Actions\Activity::query()->select('created_at') + ->whereColumn('activities.user_id', 'users.id') + ->latest() + ->take(1); + } + + $query = $this->user->with(['roles', 'avatar', 'latestActivity']) + ->orderBy($sort, $sortData['order']); if ($sortData['search']) { $term = '%' . $sortData['search'] . '%'; diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 52919d44d..dce345426 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -157,6 +157,7 @@ return [ 'user_profile' => 'User Profile', 'users_add_new' => 'Add New User', 'users_search' => 'Search Users', + 'users_latest_activity' => 'Latest Activity', 'users_details' => 'User Details', 'users_details_desc' => 'Set a display name and an email address for this user. The email address will be used for logging into the application.', 'users_details_desc_no_email' => 'Set a display name for this user so others can recognise them.', diff --git a/resources/views/users/index.blade.php b/resources/views/users/index.blade.php index da373c161..4b5bad0fd 100644 --- a/resources/views/users/index.blade.php +++ b/resources/views/users/index.blade.php @@ -27,7 +27,6 @@ - {{--TODO - Add last login--}} @@ -37,6 +36,9 @@ {{ trans('auth.email') }} + @foreach($users as $user) @@ -55,6 +57,11 @@ id}") }}">{{$role->display_name}}@if($index !== count($user->roles) -1),@endif @endforeach + @endforeach
{{ trans('settings.role_user_roles') }} + {{ trans('settings.users_latest_activity') }} +
+ @if($user->latestActivity) + {{ $user->latestActivity->created_at->diffForHumans() }} + @endif +