From e226861a3fab0b0903fca8b11078425fd8db791a Mon Sep 17 00:00:00 2001 From: Chen Asraf Date: Mon, 16 Mar 2026 22:33:32 +0200 Subject: [PATCH] fix: category read status after thread creation --- lib/Controller/ThreadController.php | 29 +++ lib/Db/ReadMarkerMapper.php | 21 +++ lib/Db/ThreadMapper.php | 24 +++ openapi-full.json | 7 +- .../unit/Controller/ThreadControllerTest.php | 177 ++++++++++++++++++ 5 files changed, 257 insertions(+), 1 deletion(-) diff --git a/lib/Controller/ThreadController.php b/lib/Controller/ThreadController.php index 8fe2537..0d72800 100644 --- a/lib/Controller/ThreadController.php +++ b/lib/Controller/ThreadController.php @@ -13,6 +13,7 @@ use OCA\Forum\Db\DraftMapper; use OCA\Forum\Db\ForumUserMapper; use OCA\Forum\Db\Post; use OCA\Forum\Db\PostMapper; +use OCA\Forum\Db\ReadMarkerMapper; use OCA\Forum\Db\Thread; use OCA\Forum\Db\ThreadMapper; use OCA\Forum\Db\ThreadSubscriptionMapper; @@ -42,6 +43,7 @@ class ThreadController extends OCSController { private ForumUserMapper $forumUserMapper, private ThreadSubscriptionMapper $threadSubscriptionMapper, private DraftMapper $draftMapper, + private ReadMarkerMapper $readMarkerMapper, private ThreadEnrichmentService $threadEnrichmentService, private UserPreferencesService $userPreferencesService, private UserService $userService, @@ -285,6 +287,23 @@ class ThreadController extends OCSController { return new DataResponse(['error' => 'User not authenticated'], Http::STATUS_UNAUTHORIZED); } + // Check if the category was already read before creating the thread + // (used later to decide whether to update the category read marker) + $wasCategoryRead = false; + try { + $lastActivity = $this->threadMapper->getLastActivityForCategory($categoryId); + if ($lastActivity === null) { + $wasCategoryRead = true; + } else { + $marker = $this->readMarkerMapper->findByUserAndCategory($user->getUID(), $categoryId); + $wasCategoryRead = $marker->getReadAt() >= $lastActivity; + } + } catch (DoesNotExistException $e) { + // No read marker means the category is unread + } catch (\Exception $e) { + $this->logger->warning('Failed to check category read state: ' . $e->getMessage()); + } + // Generate slug from title $slug = $this->generateSlug($title); @@ -369,6 +388,16 @@ class ThreadController extends OCSController { $this->logger->warning('Failed to send mention notifications: ' . $e->getMessage()); } + // Update category read marker so the category stays read, + // but only if it was not already unread before this thread was created + if ($wasCategoryRead) { + try { + $this->readMarkerMapper->createOrUpdateCategoryMarker($user->getUID(), $categoryId); + } catch (\Exception $e) { + $this->logger->warning('Failed to update category read marker: ' . $e->getMessage()); + } + } + // Delete any draft for this category now that the thread is created try { $this->draftMapper->deleteThreadDraft($user->getUID(), $categoryId); diff --git a/lib/Db/ReadMarkerMapper.php b/lib/Db/ReadMarkerMapper.php index 369f655..bd9d87c 100644 --- a/lib/Db/ReadMarkerMapper.php +++ b/lib/Db/ReadMarkerMapper.php @@ -136,6 +136,27 @@ class ReadMarkerMapper extends QBMapper { } } + /** + * Find a category read marker for a user + * + * @throws DoesNotExistException + */ + public function findByUserAndCategory(string $userId, int $categoryId): ReadMarker { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('marker_type', $qb->createNamedParameter(ReadMarker::TYPE_CATEGORY, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('entity_id', $qb->createNamedParameter($categoryId, IQueryBuilder::PARAM_INT)) + ); + return $this->findEntity($qb); + } + /** * Find all category read markers for a user * diff --git a/lib/Db/ThreadMapper.php b/lib/Db/ThreadMapper.php index 829589d..d193125 100644 --- a/lib/Db/ThreadMapper.php +++ b/lib/Db/ThreadMapper.php @@ -249,6 +249,30 @@ class ThreadMapper extends QBMapper { return $map; } + /** + * Get the last activity timestamp for a single category + * + * @return int|null The timestamp of the last post, or null if no activity + */ + public function getLastActivityForCategory(int $categoryId): ?int { + $postsTable = Application::tableName('forum_posts'); + + $qb = $this->db->getQueryBuilder(); + $qb->selectAlias($qb->func()->max('p.created_at'), 'last_activity') + ->from($this->getTableName(), 't') + ->innerJoin('t', $postsTable, 'p', $qb->expr()->eq('p.thread_id', 't.id')) + ->where($qb->expr()->eq('t.category_id', $qb->createNamedParameter($categoryId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->isNull('t.deleted_at')) + ->andWhere($qb->expr()->isNull('p.deleted_at')) + ->andWhere($qb->expr()->eq('t.is_hidden', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))); + + $result = $qb->executeQuery(); + $row = $result->fetch(); + $result->closeCursor(); + + return $row && $row['last_activity'] !== null ? (int)$row['last_activity'] : null; + } + /** * Find recent threads in specified categories * diff --git a/openapi-full.json b/openapi-full.json index 1cf5c9a..390fb84 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -7884,7 +7884,8 @@ "id", "displayName", "owner", - "ownerDisplayName" + "ownerDisplayName", + "memberCount" ], "properties": { "id": { @@ -7898,6 +7899,10 @@ }, "ownerDisplayName": { "type": "string" + }, + "memberCount": { + "type": "integer", + "format": "int64" } } } diff --git a/tests/unit/Controller/ThreadControllerTest.php b/tests/unit/Controller/ThreadControllerTest.php index 0e550d0..d57187b 100644 --- a/tests/unit/Controller/ThreadControllerTest.php +++ b/tests/unit/Controller/ThreadControllerTest.php @@ -13,6 +13,8 @@ use OCA\Forum\Db\ForumUser; use OCA\Forum\Db\ForumUserMapper; use OCA\Forum\Db\Post; use OCA\Forum\Db\PostMapper; +use OCA\Forum\Db\ReadMarker; +use OCA\Forum\Db\ReadMarkerMapper; use OCA\Forum\Db\Thread; use OCA\Forum\Db\ThreadMapper; use OCA\Forum\Db\ThreadSubscriptionMapper; @@ -51,6 +53,9 @@ class ThreadControllerTest extends TestCase { /** @var DraftMapper&MockObject */ private DraftMapper $draftMapper; + /** @var ReadMarkerMapper&MockObject */ + private ReadMarkerMapper $readMarkerMapper; + /** @var ThreadEnrichmentService&MockObject */ private ThreadEnrichmentService $threadEnrichmentService; @@ -83,6 +88,7 @@ class ThreadControllerTest extends TestCase { $this->forumUserMapper = $this->createMock(ForumUserMapper::class); $this->threadSubscriptionMapper = $this->createMock(ThreadSubscriptionMapper::class); $this->draftMapper = $this->createMock(DraftMapper::class); + $this->readMarkerMapper = $this->createMock(ReadMarkerMapper::class); $this->threadEnrichmentService = $this->createMock(ThreadEnrichmentService::class); $this->userPreferencesService = $this->createMock(UserPreferencesService::class); $this->userService = $this->createMock(UserService::class); @@ -111,6 +117,7 @@ class ThreadControllerTest extends TestCase { $this->forumUserMapper, $this->threadSubscriptionMapper, $this->draftMapper, + $this->readMarkerMapper, $this->threadEnrichmentService, $this->userPreferencesService, $this->userService, @@ -1243,6 +1250,176 @@ class ThreadControllerTest extends TestCase { $this->assertEquals(0, $data['pagination']['total']); } + public function testCreateThreadUpdatesCategoryReadMarkerWhenCategoryWasRead(): void { + $categoryId = 1; + $title = 'New Thread'; + $content = 'Content'; + $userId = 'user1'; + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($userId); + $this->userSession->method('getUser')->willReturn($user); + $this->userPreferencesService->method('getPreference')->willReturn(false); + + // Category had activity at time 1000, user read it at 1000 — category was read + $this->threadMapper->method('getLastActivityForCategory') + ->with($categoryId) + ->willReturn(1000); + + $categoryMarker = new ReadMarker(); + $categoryMarker->setReadAt(1000); + $this->readMarkerMapper->method('findByUserAndCategory') + ->with($userId, $categoryId) + ->willReturn($categoryMarker); + + // Expect category read marker to be updated + $this->readMarkerMapper->expects($this->once()) + ->method('createOrUpdateCategoryMarker') + ->with($userId, $categoryId); + + // Standard create mocks + $this->threadMapper->method('findBySlug')->willThrowException(new DoesNotExistException('')); + $thread = $this->createMockThread(1, $categoryId, $userId, $title); + $this->threadMapper->method('insert')->willReturn($thread); + $post = new Post(); + $post->setId(1); + $this->postMapper->method('insert')->willReturn($post); + $this->threadMapper->method('update')->willReturn($thread); + $category = new Category(); + $category->setThreadCount(0); + $category->setPostCount(0); + $this->categoryMapper->method('find')->willReturn($category); + $this->categoryMapper->method('update')->willReturn($category); + + $response = $this->controller->create($categoryId, $title, $content); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + + public function testCreateThreadDoesNotUpdateCategoryReadMarkerWhenCategoryWasUnread(): void { + $categoryId = 1; + $title = 'New Thread'; + $content = 'Content'; + $userId = 'user1'; + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($userId); + $this->userSession->method('getUser')->willReturn($user); + $this->userPreferencesService->method('getPreference')->willReturn(false); + + // Category had activity at time 2000, user read it at 1000 — category was unread + $this->threadMapper->method('getLastActivityForCategory') + ->with($categoryId) + ->willReturn(2000); + + $categoryMarker = new ReadMarker(); + $categoryMarker->setReadAt(1000); + $this->readMarkerMapper->method('findByUserAndCategory') + ->with($userId, $categoryId) + ->willReturn($categoryMarker); + + // Should NOT update category read marker + $this->readMarkerMapper->expects($this->never()) + ->method('createOrUpdateCategoryMarker'); + + // Standard create mocks + $this->threadMapper->method('findBySlug')->willThrowException(new DoesNotExistException('')); + $thread = $this->createMockThread(1, $categoryId, $userId, $title); + $this->threadMapper->method('insert')->willReturn($thread); + $post = new Post(); + $post->setId(1); + $this->postMapper->method('insert')->willReturn($post); + $this->threadMapper->method('update')->willReturn($thread); + $category = new Category(); + $category->setThreadCount(0); + $category->setPostCount(0); + $this->categoryMapper->method('find')->willReturn($category); + $this->categoryMapper->method('update')->willReturn($category); + + $response = $this->controller->create($categoryId, $title, $content); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + + public function testCreateThreadDoesNotUpdateCategoryReadMarkerWhenNoMarkerExists(): void { + $categoryId = 1; + $title = 'New Thread'; + $content = 'Content'; + $userId = 'user1'; + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($userId); + $this->userSession->method('getUser')->willReturn($user); + $this->userPreferencesService->method('getPreference')->willReturn(false); + + // Category has activity but user has no read marker — category is unread + $this->threadMapper->method('getLastActivityForCategory') + ->with($categoryId) + ->willReturn(1000); + + $this->readMarkerMapper->method('findByUserAndCategory') + ->with($userId, $categoryId) + ->willThrowException(new DoesNotExistException('')); + + // Should NOT update category read marker + $this->readMarkerMapper->expects($this->never()) + ->method('createOrUpdateCategoryMarker'); + + // Standard create mocks + $this->threadMapper->method('findBySlug')->willThrowException(new DoesNotExistException('')); + $thread = $this->createMockThread(1, $categoryId, $userId, $title); + $this->threadMapper->method('insert')->willReturn($thread); + $post = new Post(); + $post->setId(1); + $this->postMapper->method('insert')->willReturn($post); + $this->threadMapper->method('update')->willReturn($thread); + $category = new Category(); + $category->setThreadCount(0); + $category->setPostCount(0); + $this->categoryMapper->method('find')->willReturn($category); + $this->categoryMapper->method('update')->willReturn($category); + + $response = $this->controller->create($categoryId, $title, $content); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + + public function testCreateThreadUpdatesCategoryReadMarkerWhenCategoryHadNoActivity(): void { + $categoryId = 1; + $title = 'New Thread'; + $content = 'Content'; + $userId = 'user1'; + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($userId); + $this->userSession->method('getUser')->willReturn($user); + $this->userPreferencesService->method('getPreference')->willReturn(false); + + // No prior activity in category + $this->threadMapper->method('getLastActivityForCategory') + ->with($categoryId) + ->willReturn(null); + + // Should update category read marker (empty category stays read) + $this->readMarkerMapper->expects($this->once()) + ->method('createOrUpdateCategoryMarker') + ->with($userId, $categoryId); + + // Standard create mocks + $this->threadMapper->method('findBySlug')->willThrowException(new DoesNotExistException('')); + $thread = $this->createMockThread(1, $categoryId, $userId, $title); + $this->threadMapper->method('insert')->willReturn($thread); + $post = new Post(); + $post->setId(1); + $this->postMapper->method('insert')->willReturn($post); + $this->threadMapper->method('update')->willReturn($thread); + $category = new Category(); + $category->setThreadCount(0); + $category->setPostCount(0); + $this->categoryMapper->method('find')->willReturn($category); + $this->categoryMapper->method('update')->willReturn($category); + + $response = $this->controller->create($categoryId, $title, $content); + $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); + } + private function createMockThread(int $id, int $categoryId, string $authorId, string $title): Thread { $thread = new Thread(); $thread->setId($id);