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 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Support\Facades\DB;
+
+class SimplifyActivitiesTable extends Migration
+{
+ /**
+ * Run the migrations.
+ *
+ * @return void
+ */
+ public function up()
+ {
+ Schema::table('activities', function (Blueprint $table) {
+ $table->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 @@
<button refs="dropdown@toggle" aria-haspopup="true" aria-expanded="false" aria-label="{{ trans('common.sort_options') }}" class="input-base text-left">{{ $listDetails['event'] ?: trans('settings.audit_event_filter_no_filter') }}</button>
<ul refs="dropdown@menu" class="dropdown-menu">
<li @if($listDetails['event'] === '') class="active" @endif><a href="{{ sortUrl('/settings/audit', $listDetails, ['event' => '']) }}">{{ trans('settings.audit_event_filter_no_filter') }}</a></li>
- @foreach($activityKeys as $key)
- <li @if($key === $listDetails['event']) class="active" @endif><a href="{{ sortUrl('/settings/audit', $listDetails, ['event' => $key]) }}">{{ $key }}</a></li>
+ @foreach($activityTypes as $type)
+ <li @if($type === $listDetails['event']) class="active" @endif><a href="{{ sortUrl('/settings/audit', $listDetails, ['event' => $type]) }}">{{ $type }}</a></li>
@endforeach
</ul>
</div>
@@ -62,7 +62,7 @@
<td>
@include('partials.table-user', ['user' => $activity->user, 'user_id' => $activity->user_id])
</td>
- <td>{{ $activity->key }}</td>
+ <td>{{ $activity->type }}</td>
<td>
@if($activity->entity)
<a href="{{ $activity->entity->getUrl() }}" class="table-entity-item">
@@ -71,10 +71,10 @@
{{ $activity->entity->name }}
</div>
</a>
- @elseif($activity->extra)
+ @elseif($activity->detail)
<div class="px-m">
{{ trans('settings.audit_deleted_item') }} <br>
- {{ trans('settings.audit_deleted_item_name', ['name' => $activity->extra]) }}
+ {{ trans('settings.audit_deleted_item_name', ['name' => $activity->detail]) }}
</div>
@endif
</td>
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();