From 9c4a9225af6f7fdcaf8494e8562197d831d25237 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 24 Oct 2025 14:22:53 +0100 Subject: [PATCH] Comments API: Addressed failing tests and static testing --- app/Activity/CommentRepo.php | 1 + app/Entities/Models/Entity.php | 1 + app/Http/ApiController.php | 2 +- app/Search/SearchRunner.php | 4 ++-- .../Activity/Models/CommentFactory.php | 5 ++++ tests/Activity/WatchTest.php | 4 ++-- tests/Entity/CommentDisplayTest.php | 6 ++--- tests/Entity/CommentStoreTest.php | 23 ++++++++++--------- 8 files changed, 27 insertions(+), 19 deletions(-) diff --git a/app/Activity/CommentRepo.php b/app/Activity/CommentRepo.php index 20513bcd4..13808903d 100644 --- a/app/Activity/CommentRepo.php +++ b/app/Activity/CommentRepo.php @@ -31,6 +31,7 @@ class CommentRepo /** * Start a query for comments visible to the user. + * @return Builder */ public function getQueryForVisible(): Builder { diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index c6839c15a..77393cbbc 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -237,6 +237,7 @@ abstract class Entity extends Model implements /** * Get the comments for an entity. + * @return MorphMany */ public function comments(bool $orderByCreated = true): MorphMany { diff --git a/app/Http/ApiController.php b/app/Http/ApiController.php index ac8844b81..8c0f206d0 100644 --- a/app/Http/ApiController.php +++ b/app/Http/ApiController.php @@ -12,7 +12,7 @@ abstract class ApiController extends Controller * The validation rules for this controller. * Can alternative be defined in a rules() method is they need to be dynamic. * - * @var array + * @var array> */ protected array $rules = []; diff --git a/app/Search/SearchRunner.php b/app/Search/SearchRunner.php index c14d1c642..a1ffeee50 100644 --- a/app/Search/SearchRunner.php +++ b/app/Search/SearchRunner.php @@ -461,9 +461,9 @@ class SearchRunner { $commentsTable = DB::getTablePrefix() . 'comments'; $morphClass = str_replace('\\', '\\\\', $model->getMorphClass()); - $commentQuery = DB::raw('(SELECT c1.entity_id, c1.entity_type, c1.created_at as last_commented FROM ' . $commentsTable . ' c1 LEFT JOIN ' . $commentsTable . ' c2 ON (c1.entity_id = c2.entity_id AND c1.entity_type = c2.entity_type AND c1.created_at < c2.created_at) WHERE c1.entity_type = \'' . $morphClass . '\' AND c2.created_at IS NULL) as comments'); + $commentQuery = DB::raw('(SELECT c1.commentable_id, c1.commentable_type, c1.created_at as last_commented FROM ' . $commentsTable . ' c1 LEFT JOIN ' . $commentsTable . ' c2 ON (c1.commentable_id = c2.commentable_id AND c1.commentable_type = c2.commentable_type AND c1.created_at < c2.created_at) WHERE c1.commentable_type = \'' . $morphClass . '\' AND c2.created_at IS NULL) as comments'); - $query->join($commentQuery, $model->getTable() . '.id', '=', DB::raw('comments.entity_id')) + $query->join($commentQuery, $model->getTable() . '.id', '=', DB::raw('comments.commentable_id')) ->orderBy('last_commented', $negated ? 'asc' : 'desc'); } } diff --git a/database/factories/Activity/Models/CommentFactory.php b/database/factories/Activity/Models/CommentFactory.php index 81022e0d4..2b7fb9ac9 100644 --- a/database/factories/Activity/Models/CommentFactory.php +++ b/database/factories/Activity/Models/CommentFactory.php @@ -2,6 +2,7 @@ namespace Database\Factories\Activity\Models; +use BookStack\Users\Models\User; use Illuminate\Database\Eloquent\Factories\Factory; class CommentFactory extends Factory @@ -29,12 +30,16 @@ class CommentFactory extends Factory $html = '

' . $text . '

'; $nextLocalId = static::$nextLocalId++; + $user = User::query()->first(); + return [ 'html' => $html, 'parent_id' => null, 'local_id' => $nextLocalId, 'content_ref' => '', 'archived' => false, + 'created_by' => $user ?? User::factory(), + 'updated_by' => $user ?? User::factory(), ]; } } diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php index c405b07ae..f6f3b71c1 100644 --- a/tests/Activity/WatchTest.php +++ b/tests/Activity/WatchTest.php @@ -340,8 +340,8 @@ class WatchTest extends TestCase ActivityType::PAGE_CREATE => $entities['page'], ActivityType::PAGE_UPDATE => $entities['page'], ActivityType::COMMENT_CREATE => Comment::factory()->make([ - 'entity_id' => $entities['page']->id, - 'entity_type' => $entities['page']->getMorphClass(), + 'commentable_id' => $entities['page']->id, + 'commentable_type' => $entities['page']->getMorphClass(), ]), ]; diff --git a/tests/Entity/CommentDisplayTest.php b/tests/Entity/CommentDisplayTest.php index bffe29fa9..80664890a 100644 --- a/tests/Entity/CommentDisplayTest.php +++ b/tests/Entity/CommentDisplayTest.php @@ -72,8 +72,8 @@ class CommentDisplayTest extends TestCase Comment::factory()->create([ 'created_by' => $editor->id, - 'entity_type' => 'page', - 'entity_id' => $page->id, + 'commentable_type' => 'page', + 'commentable_id' => $page->id, ]); $resp = $this->actingAs($editor)->get($page->getUrl()); @@ -84,7 +84,7 @@ class CommentDisplayTest extends TestCase public function test_comment_displays_relative_times() { $page = $this->entities->page(); - $comment = Comment::factory()->create(['entity_id' => $page->id, 'entity_type' => $page->getMorphClass()]); + $comment = Comment::factory()->create(['commentable_id' => $page->id, 'commentable_type' => $page->getMorphClass()]); $comment->created_at = now()->subWeek(); $comment->updated_at = now()->subDay(); $comment->save(); diff --git a/tests/Entity/CommentStoreTest.php b/tests/Entity/CommentStoreTest.php index de093a3a6..c4c959c29 100644 --- a/tests/Entity/CommentStoreTest.php +++ b/tests/Entity/CommentStoreTest.php @@ -14,6 +14,7 @@ class CommentStoreTest extends TestCase $this->asAdmin(); $page = $this->entities->page(); + Comment::factory()->create(['commentable_id' => $page->id, 'commentable_type' => 'page', 'local_id' => 2]); $comment = Comment::factory()->make(['parent_id' => 2]); $resp = $this->postJson("/comment/$page->id", $comment->getAttributes()); @@ -24,9 +25,9 @@ class CommentStoreTest extends TestCase $pageResp->assertSee($comment->html, false); $this->assertDatabaseHas('comments', [ - 'local_id' => 1, - 'entity_id' => $page->id, - 'entity_type' => Page::newModelInstance()->getMorphClass(), + 'local_id' => 3, + 'commentable_id' => $page->id, + 'commentable_type' => 'page', 'parent_id' => 2, ]); @@ -52,9 +53,9 @@ class CommentStoreTest extends TestCase ]); if ($valid) { - $this->assertDatabaseHas('comments', ['entity_id' => $page->id, 'content_ref' => $ref]); + $this->assertDatabaseHas('comments', ['commentable_id' => $page->id, 'content_ref' => $ref]); } else { - $this->assertDatabaseMissing('comments', ['entity_id' => $page->id, 'content_ref' => $ref]); + $this->assertDatabaseMissing('comments', ['commentable_id' => $page->id, 'content_ref' => $ref]); } } } @@ -79,7 +80,7 @@ class CommentStoreTest extends TestCase $this->assertDatabaseHas('comments', [ 'html' => $newHtml, - 'entity_id' => $page->id, + 'commentable_id' => $page->id, ]); $this->assertActivityExists(ActivityType::COMMENT_UPDATE); @@ -218,7 +219,7 @@ class CommentStoreTest extends TestCase $page = $this->entities->page(); Comment::factory()->create([ 'html' => '

scriptincommentest

', - 'entity_type' => 'page', 'entity_id' => $page + 'commentable_type' => 'page', 'commentable_id' => $page ]); $resp = $this->asAdmin()->get($page->getUrl()); @@ -236,8 +237,8 @@ class CommentStoreTest extends TestCase $resp = $this->asAdmin()->post("/comment/{$page->id}", ['html' => $input]); $resp->assertOk(); $this->assertDatabaseHas('comments', [ - 'entity_type' => 'page', - 'entity_id' => $page->id, + 'commentable_type' => 'page', + 'commentable_id' => $page->id, 'html' => $expected, ]); @@ -259,8 +260,8 @@ class CommentStoreTest extends TestCase $resp = $this->asAdmin()->post("/comment/{$page->id}", ['html' => $input]); $resp->assertOk(); $this->assertDatabaseHas('comments', [ - 'entity_type' => 'page', - 'entity_id' => $page->id, + 'commentable_type' => 'page', + 'commentable_id' => $page->id, 'html' => $expected, ]);