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.

diff --git a/app/Actions/Activity.php b/app/Actions/Activity.php
index 035a9cc750ef16618ee81b8f48e0981c2442f8da..63eda591783c8d14a2fc2f478ee79b99f0bb0be8 100644 (file)
--- 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 be9f656c3f097093a85f5b5e651475f5be743706..fb4a739cd27dbcedcef17e23b759f0747f3ab3ce 100644 (file)
--- 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 6eac4375ddce6c271a06669a8eaa108b774d55e2..042b56e288989b141a5c5f44e8ccb1f058776aa3 100644 (file)
--- 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 fad4e8d38a0db99ef3802ea64c2500529b3a4713..eb6eecc944ec0ba29567b2147b5d88af4f39d437 100644 (file)
--- 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 fb9308b4668ec19d760b64cd7b2acc9ed432dc27..9375b618afaefc3c97f91d86848e55d3127b932e 100644 (file)
--- 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 (file)
index 0000000..828dbc6
--- /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 47a2355d1eb13d5d15185353ac2e2f43954c4d32..7bbf0ed1ac6db4f04cf62739a44f3dba745f30d3 100644 (file)
--- 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 94eb02599ca45b85e8f2e2b6524bb45ebb44ef01..efe842aa1421c5a77f2127ad44995fbd37b8b448 100644 (file)
--- 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 69233e287379221fdb9cf4d5a4449cf7c1c3f971..ca90bf055998325626e4c578924088518ca9ede1 100644 (file)
--- 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 28c3adf312682fa84783a78634eb946681c98e70..d510a20ca1f1dd077b475e228ec02b7ad0fb5d4e 100644 (file)
--- 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 b6a9dc791fa009ad8a398a7a37944a7175315f83..505ee6939a976642f2c0549806424da6fa296457 100644 (file)
--- 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 1f1d5ece7288e88575b49975848812bac5915173..b5c7730373fa0b2641b86dd238890ce90af489c2 100644 (file)
--- 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();