Cleaned up the activity service

- Added test to ensure activity on entity delete works as expected.
This commit is contained in:
Dan Brown 2020-04-10 20:55:33 +01:00
parent 7b8fe5fbc6
commit d4df18098f
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
3 changed files with 64 additions and 59 deletions

View File

@ -50,10 +50,8 @@ class Activity extends Model
/** /**
* Checks if another Activity matches the general information of another. * 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]; return [$this->key, $this->entity_type, $this->entity_id] === [$activityB->key, $activityB->entity_type, $activityB->entity_id];
} }

View File

@ -1,8 +1,9 @@
<?php namespace BookStack\Actions; <?php namespace BookStack\Actions;
use BookStack\Auth\Permissions\PermissionService; use BookStack\Auth\Permissions\PermissionService;
use BookStack\Entities\Book; use BookStack\Auth\User;
use BookStack\Entities\Entity; use BookStack\Entities\Entity;
use Illuminate\Support\Collection;
class ActivityService class ActivityService
{ {
@ -12,8 +13,6 @@ class ActivityService
/** /**
* ActivityService constructor. * ActivityService constructor.
* @param Activity $activity
* @param PermissionService $permissionService
*/ */
public function __construct(Activity $activity, PermissionService $permissionService) public function __construct(Activity $activity, PermissionService $permissionService)
{ {
@ -24,11 +23,8 @@ class ActivityService
/** /**
* Add activity data to database. * Add activity data to database.
* @param \BookStack\Entities\Entity $entity
* @param string $activityKey
* @param int $bookId
*/ */
public function add(Entity $entity, string $activityKey, int $bookId = null) public function add(Entity $entity, string $activityKey, ?int $bookId = null)
{ {
$activity = $this->newActivityForUser($activityKey, $bookId); $activity = $this->newActivityForUser($activityKey, $bookId);
$entity->activity()->save($activity); $entity->activity()->save($activity);
@ -37,11 +33,8 @@ class ActivityService
/** /**
* Adds a activity history with a message, without binding to a entity. * 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([ $this->newActivityForUser($activityKey, $bookId)->forceFill([
'extra' => $message 'extra' => $message
@ -52,11 +45,8 @@ class ActivityService
/** /**
* Get a new activity instance for the current user. * 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([ return $this->activity->newInstance()->forceFill([
'key' => strtolower($key), 'key' => strtolower($key),
@ -69,34 +59,27 @@ class ActivityService
* Removes the entity attachment from each of its activities * Removes the entity attachment from each of its activities
* and instead uses the 'extra' field with the entities name. * and instead uses the 'extra' field with the entities name.
* Used when an entity is deleted. * 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()->get();
$activities = $entity->activity; $entity->activity()->update([
foreach ($activities as $activity) { 'extra' => $entity->name,
$activity->extra = $entity->name; 'entity_id' => 0,
$activity->entity_id = 0; 'entity_type' => '',
$activity->entity_type = null; ]);
$activity->save();
}
return $activities; return $activities;
} }
/** /**
* Gets the latest activity. * 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 $activityList = $this->permissionService
->filterRestrictedEntityRelations($this->activity, 'activities', 'entity_id', 'entity_type') ->filterRestrictedEntityRelations($this->activity, 'activities', 'entity_id', 'entity_type')
->orderBy('created_at', 'desc') ->orderBy('created_at', 'desc')
->with('user', 'entity') ->with(['user', 'entity'])
->skip($count * $page) ->skip($count * $page)
->take($count) ->take($count)
->get(); ->get();
@ -107,17 +90,13 @@ class ActivityService
/** /**
* Gets the latest activity for an entity, Filtering out similar * Gets the latest activity for an entity, Filtering out similar
* items to prevent a message activity list. * 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')) { if ($entity->isA('book')) {
$query = $this->activity->where('book_id', '=', $entity->id); $query = $this->activity->newQuery()->where('book_id', '=', $entity->id);
} else { } else {
$query = $this->activity->where('entity_type', '=', $entity->getMorphClass()) $query = $this->activity->newQuery()->where('entity_type', '=', $entity->getMorphClass())
->where('entity_id', '=', $entity->id); ->where('entity_id', '=', $entity->id);
} }
@ -133,18 +112,18 @@ class ActivityService
} }
/** /**
* Get latest activity for a user, Filtering out similar * Get latest activity for a user, Filtering out similar items.
* items.
* @param $user
* @param int $count
* @param int $page
* @return array
*/ */
public function userActivity($user, $count = 20, $page = 0) public function userActivity(User $user, int $count = 20, int $page = 0): array
{ {
$activityList = $this->permissionService $activityList = $this->permissionService
->filterRestrictedEntityRelations($this->activity, 'activities', 'entity_id', 'entity_type') ->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); return $this->filterSimilar($activityList);
} }
@ -153,29 +132,26 @@ class ActivityService
* @param Activity[] $activities * @param Activity[] $activities
* @return array * @return array
*/ */
protected function filterSimilar($activities) protected function filterSimilar(iterable $activities): array
{ {
$newActivity = []; $newActivity = [];
$previousItem = false; $previousItem = null;
foreach ($activities as $activityItem) { foreach ($activities as $activityItem) {
if ($previousItem === false) { if (!$previousItem || !$activityItem->isSimilarTo($previousItem)) {
$previousItem = $activityItem;
$newActivity[] = $activityItem;
continue;
}
if (!$activityItem->isSimilarTo($previousItem)) {
$newActivity[] = $activityItem; $newActivity[] = $activityItem;
} }
$previousItem = $activityItem; $previousItem = $activityItem;
} }
return $newActivity; return $newActivity;
} }
/** /**
* Flashes a notification message to the session if an appropriate message is available. * 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'; $notificationTextKey = 'activities.' . $activityKey . '_notification';
if (trans()->has($notificationTextKey)) { if (trans()->has($notificationTextKey)) {

View File

@ -7,6 +7,7 @@ use BookStack\Entities\Page;
use BookStack\Auth\UserRepo; use BookStack\Auth\UserRepo;
use BookStack\Entities\Repos\PageRepo; use BookStack\Entities\Repos\PageRepo;
use Carbon\Carbon; use Carbon\Carbon;
use Illuminate\Support\Facades\DB;
use Tests\BrowserKitTest; use Tests\BrowserKitTest;
class EntityTest extends BrowserKitTest class EntityTest extends BrowserKitTest
@ -326,4 +327,34 @@ class EntityTest extends BrowserKitTest
->seePageIs($chapter->getUrl()); ->seePageIs($chapter->getUrl());
} }
public function test_page_delete_removes_entity_from_its_activity()
{
$page = Page::query()->first();
$this->asEditor()->put($page->getUrl(), [
'name' => 'My updated page',
'html' => '<p>updated content</p>',
]);
$page->refresh();
$this->seeInDatabase('activities', [
'entity_id' => $page->id,
'entity_type' => $page->getMorphClass(),
]);
$resp = $this->delete($page->getUrl());
$resp->assertResponseStatus(302);
$this->dontSeeInDatabase('activities', [
'entity_id' => $page->id,
'entity_type' => $page->getMorphClass(),
]);
$this->seeInDatabase('activities', [
'extra' => 'My updated page',
'entity_id' => 0,
'entity_type' => '',
]);
}
} }