diff --git a/app/Actions/Activity.php b/app/Actions/Activity.php index 05f0129dd..035a9cc75 100644 --- a/app/Actions/Activity.php +++ b/app/Actions/Activity.php @@ -50,10 +50,8 @@ class Activity extends Model /** * Checks if another Activity matches the general information of another. - * @param $activityB - * @return bool */ - public function isSimilarTo($activityB) + public function isSimilarTo(Activity $activityB): bool { return [$this->key, $this->entity_type, $this->entity_id] === [$activityB->key, $activityB->entity_type, $activityB->entity_id]; } diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index f56f1ca57..a92447cff 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -1,8 +1,9 @@ newActivityForUser($activityKey, $bookId); $entity->activity()->save($activity); @@ -37,11 +33,8 @@ class ActivityService /** * Adds a activity history with a message, without binding to a entity. - * @param string $activityKey - * @param string $message - * @param int $bookId */ - public function addMessage(string $activityKey, string $message, int $bookId = null) + public function addMessage(string $activityKey, string $message, ?int $bookId = null) { $this->newActivityForUser($activityKey, $bookId)->forceFill([ 'extra' => $message @@ -52,11 +45,8 @@ class ActivityService /** * Get a new activity instance for the current user. - * @param string $key - * @param int|null $bookId - * @return Activity */ - protected function newActivityForUser(string $key, int $bookId = null) + protected function newActivityForUser(string $key, ?int $bookId = null): Activity { return $this->activity->newInstance()->forceFill([ 'key' => strtolower($key), @@ -69,34 +59,27 @@ class ActivityService * Removes the entity attachment from each of its activities * and instead uses the 'extra' field with the entities name. * Used when an entity is deleted. - * @param \BookStack\Entities\Entity $entity - * @return mixed */ - public function removeEntity(Entity $entity) + public function removeEntity(Entity $entity): Collection { - // TODO - Rewrite to db query. - $activities = $entity->activity; - foreach ($activities as $activity) { - $activity->extra = $entity->name; - $activity->entity_id = 0; - $activity->entity_type = null; - $activity->save(); - } + $activities = $entity->activity()->get(); + $entity->activity()->update([ + 'extra' => $entity->name, + 'entity_id' => 0, + 'entity_type' => '', + ]); return $activities; } /** * Gets the latest activity. - * @param int $count - * @param int $page - * @return array */ - public function latest($count = 20, $page = 0) + public function latest(int $count = 20, int $page = 0): array { $activityList = $this->permissionService ->filterRestrictedEntityRelations($this->activity, 'activities', 'entity_id', 'entity_type') ->orderBy('created_at', 'desc') - ->with('user', 'entity') + ->with(['user', 'entity']) ->skip($count * $page) ->take($count) ->get(); @@ -107,17 +90,13 @@ class ActivityService /** * Gets the latest activity for an entity, Filtering out similar * items to prevent a message activity list. - * @param \BookStack\Entities\Entity $entity - * @param int $count - * @param int $page - * @return array */ - public function entityActivity($entity, $count = 20, $page = 1) + public function entityActivity(Entity $entity, int $count = 20, int $page = 1): array { if ($entity->isA('book')) { - $query = $this->activity->where('book_id', '=', $entity->id); + $query = $this->activity->newQuery()->where('book_id', '=', $entity->id); } else { - $query = $this->activity->where('entity_type', '=', $entity->getMorphClass()) + $query = $this->activity->newQuery()->where('entity_type', '=', $entity->getMorphClass()) ->where('entity_id', '=', $entity->id); } @@ -133,18 +112,18 @@ class ActivityService } /** - * Get latest activity for a user, Filtering out similar - * items. - * @param $user - * @param int $count - * @param int $page - * @return array + * Get latest activity for a user, Filtering out similar items. */ - public function userActivity($user, $count = 20, $page = 0) + public function userActivity(User $user, int $count = 20, int $page = 0): array { $activityList = $this->permissionService ->filterRestrictedEntityRelations($this->activity, 'activities', 'entity_id', 'entity_type') - ->orderBy('created_at', 'desc')->where('user_id', '=', $user->id)->skip($count * $page)->take($count)->get(); + ->orderBy('created_at', 'desc') + ->where('user_id', '=', $user->id) + ->skip($count * $page) + ->take($count) + ->get()->toArray(); + return $this->filterSimilar($activityList); } @@ -153,29 +132,26 @@ class ActivityService * @param Activity[] $activities * @return array */ - protected function filterSimilar($activities) + protected function filterSimilar(iterable $activities): array { $newActivity = []; - $previousItem = false; + $previousItem = null; + foreach ($activities as $activityItem) { - if ($previousItem === false) { - $previousItem = $activityItem; - $newActivity[] = $activityItem; - continue; - } - if (!$activityItem->isSimilarTo($previousItem)) { + if (!$previousItem || !$activityItem->isSimilarTo($previousItem)) { $newActivity[] = $activityItem; } + $previousItem = $activityItem; } + return $newActivity; } /** * Flashes a notification message to the session if an appropriate message is available. - * @param $activityKey */ - protected function setNotification($activityKey) + protected function setNotification(string $activityKey) { $notificationTextKey = 'activities.' . $activityKey . '_notification'; if (trans()->has($notificationTextKey)) { diff --git a/tests/Entity/EntityTest.php b/tests/Entity/EntityTest.php index 6d71d6090..d7e4ec61c 100644 --- a/tests/Entity/EntityTest.php +++ b/tests/Entity/EntityTest.php @@ -7,6 +7,7 @@ use BookStack\Entities\Page; use BookStack\Auth\UserRepo; use BookStack\Entities\Repos\PageRepo; use Carbon\Carbon; +use Illuminate\Support\Facades\DB; use Tests\BrowserKitTest; class EntityTest extends BrowserKitTest @@ -326,4 +327,34 @@ class EntityTest extends BrowserKitTest ->seePageIs($chapter->getUrl()); } + public function test_page_delete_removes_entity_from_its_activity() + { + $page = Page::query()->first(); + + $this->asEditor()->put($page->getUrl(), [ + 'name' => 'My updated page', + 'html' => '

updated content

', + ]); + $page->refresh(); + + $this->seeInDatabase('activities', [ + 'entity_id' => $page->id, + 'entity_type' => $page->getMorphClass(), + ]); + + $resp = $this->delete($page->getUrl()); + $resp->assertResponseStatus(302); + + $this->dontSeeInDatabase('activities', [ + 'entity_id' => $page->id, + 'entity_type' => $page->getMorphClass(), + ]); + + $this->seeInDatabase('activities', [ + 'extra' => 'My updated page', + 'entity_id' => 0, + 'entity_type' => '', + ]); + } + }