Skip to content

fix(threads): add threadId to objectId for notifications #15880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nickvergessen merged 3 commits into from
Sep 17, 2025
Merged

fix(threads): add threadId to objectId for notifications #15880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9340f5
fix(threads): add threadId to objectId for notifications
miaulalala Sep 10, 2025
63e7441
fix(threads): Correctly build parameter list
nickvergessen Sep 17, 2025
4dbd3b5
fix(threads): Don't use thread id in notification when not a thread
nickvergessen Sep 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions lib/Chat/ChatManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,20 @@ public function addSystemMessage(
// Update last_message
$this->roomService->setLastMessage($chat, $comment);
$this->unreadCountCache->clear($chat->getId() . '-');

if ($threadId !== 0) {
$isThread = $this->threadService->updateLastMessageInfoAfterReply($threadId, (int)$comment->getId());
if ($isThread && $actorType === Attendee::ACTOR_USERS) {
try {
// Add to subscribed threads list
$participant = $this->participantService->getParticipant($chat, $actorId);
$this->threadService->ensureIsThreadAttendee($participant->getAttendee(), $threadId);
} catch (ParticipantNotFoundException) {
}
} elseif (!$isThread) {
$threadId = 0;
}
}
}

if ($sendNotifications) {
Expand All @@ -220,7 +234,7 @@ public function addSystemMessage(
$federatedUsersDirectlyMentioned = $this->notifier->getMentionedCloudIds($captionComment);
}
if ($replyTo instanceof IComment) {
$alreadyNotifiedUsers = $this->notifier->notifyReplyToAuthor($chat, $comment, $replyTo, $silent);
$alreadyNotifiedUsers = $this->notifier->notifyReplyToAuthor($chat, $comment, $replyTo, $silent, $threadId);
if ($replyTo->getActorType() === Attendee::ACTOR_USERS) {
$usersDirectlyMentioned[] = $replyTo->getActorId();
} elseif ($replyTo->getActorType() === Attendee::ACTOR_FEDERATED_USERS) {
Expand All @@ -229,7 +243,7 @@ public function addSystemMessage(
}
}

$alreadyNotifiedUsers = $this->notifier->notifyMentionedUsers($chat, $captionComment ?? $comment, $alreadyNotifiedUsers, $silent);
$alreadyNotifiedUsers = $this->notifier->notifyMentionedUsers($chat, $captionComment ?? $comment, $alreadyNotifiedUsers, $silent, threadId: $threadId);
if (!empty($alreadyNotifiedUsers)) {
$userIds = array_column($alreadyNotifiedUsers, 'id');
$this->participantService->markUsersAsMentioned($chat, Attendee::ACTOR_USERS, $userIds, (int)$comment->getId(), $usersDirectlyMentioned);
Expand Down Expand Up @@ -429,8 +443,10 @@ public function sendMessage(
$messageId = (int)$comment->getId();
$threadId = (int)$comment->getTopmostParentId();
if ($threadId !== 0) {
$this->threadService->updateLastMessageInfoAfterReply($threadId, $messageId);
if ($participant instanceof Participant) {
$isThread = $this->threadService->updateLastMessageInfoAfterReply($threadId, $messageId);
if (!$isThread) {
$threadId = 0;
} elseif ($participant instanceof Participant) {
// Add to subscribed threads list
$this->threadService->ensureIsThreadAttendee($participant->getAttendee(), $threadId);
}
Expand All @@ -454,15 +470,15 @@ public function sendMessage(
$usersDirectlyMentioned = $this->notifier->getMentionedUserIds($comment);
$federatedUsersDirectlyMentioned = $this->notifier->getMentionedCloudIds($comment);
if ($replyTo instanceof IComment) {
$alreadyNotifiedUsers = $this->notifier->notifyReplyToAuthor($chat, $comment, $replyTo, $silent);
$alreadyNotifiedUsers = $this->notifier->notifyReplyToAuthor($chat, $comment, $replyTo, $silent, $threadId);
if ($replyTo->getActorType() === Attendee::ACTOR_USERS) {
$usersDirectlyMentioned[] = $replyTo->getActorId();
} elseif ($replyTo->getActorType() === Attendee::ACTOR_FEDERATED_USERS) {
$federatedUsersDirectlyMentioned[] = $replyTo->getActorId();
}
}

$alreadyNotifiedUsers = $this->notifier->notifyMentionedUsers($chat, $comment, $alreadyNotifiedUsers, $silent, $participant);
$alreadyNotifiedUsers = $this->notifier->notifyMentionedUsers($chat, $comment, $alreadyNotifiedUsers, $silent, $participant, threadId: $threadId);
if (!empty($alreadyNotifiedUsers)) {
$userIds = array_column($alreadyNotifiedUsers, 'id');
$this->participantService->markUsersAsMentioned($chat, Attendee::ACTOR_USERS, $userIds, (int)$comment->getId(), $usersDirectlyMentioned);
Expand Down
24 changes: 15 additions & 9 deletions lib/Chat/Notifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function __construct(
* @return string[] Users that were mentioned
* @psalm-return array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}>
*/
public function notifyMentionedUsers(Room $chat, IComment $comment, array $alreadyNotifiedUsers, bool $silent, ?Participant $participant = null): array {
public function notifyMentionedUsers(Room $chat, IComment $comment, array $alreadyNotifiedUsers, bool $silent, ?Participant $participant = null, ?int $threadId = null): array {
$usersToNotify = $this->getUsersToNotify($chat, $comment, $alreadyNotifiedUsers, $participant);

if (!$usersToNotify) {
Expand All @@ -78,7 +78,7 @@ public function notifyMentionedUsers(Room $chat, IComment $comment, array $alrea

$shouldFlush = false;
if (!$silent) {
$notification = $this->createNotification($chat, $comment, 'mention');
$notification = $this->createNotification($chat, $comment, 'mention', threadId: $threadId);
$parameters = $notification->getSubjectParameters();
$shouldFlush = $this->notificationManager->defer();
}
Expand Down Expand Up @@ -203,7 +203,7 @@ private function addMentionAllToList(Room $chat, array $list, ?Participant $part
* @return array[] Actor that was replied to
* @psalm-return array<int, array{id: string, type: string, reason: string}>
*/
public function notifyReplyToAuthor(Room $chat, IComment $comment, IComment $replyTo, bool $silent): array {
public function notifyReplyToAuthor(Room $chat, IComment $comment, IComment $replyTo, bool $silent, ?int $threadId = null): array {
if ($replyTo->getActorType() !== Attendee::ACTOR_USERS && $replyTo->getActorType() !== Attendee::ACTOR_FEDERATED_USERS) {
// No reply notification when the replyTo-author was not a user or federated user
return [];
Expand All @@ -225,7 +225,7 @@ public function notifyReplyToAuthor(Room $chat, IComment $comment, IComment $rep
}

if (!$silent) {
$notification = $this->createNotification($chat, $comment, 'reply');
$notification = $this->createNotification($chat, $comment, 'reply', threadId: $threadId);
$notification->setUser($replyTo->getActorId());
$notification->setPriorityNotification($shouldMentionedUserBeNotified === self::PRIORITY_IMPORTANT);
$this->notificationManager->notify($notification);
Expand Down Expand Up @@ -283,7 +283,7 @@ public function notifyOtherParticipant(Room $chat, IComment $comment, array $alr
}
}

$notification = $this->createNotification($chat, $comment, 'chat');
$notification = $this->createNotification($chat, $comment, 'chat', threadId: $threadId);
foreach ($participants as $participant) {
$attendeeId = $participant->getAttendee()->getId();
$shouldParticipantBeNotified = $this->shouldParticipantBeNotified($participant, $comment, $alreadyNotifiedUsers);
Expand Down Expand Up @@ -634,18 +634,24 @@ private function getMentionedTeamMembers(Room $chat, IComment $comment, array $l
* Creates a notification for the given chat message comment and mentioned
* user ID.
*/
private function createNotification(Room $chat, IComment $comment, string $subject, array $subjectData = [], ?IComment $reaction = null): INotification {
private function createNotification(Room $chat, IComment $comment, string $subject, array $subjectData = [], ?IComment $reaction = null, ?int $threadId = null): INotification {
$subjectData['userType'] = $reaction ? $reaction->getActorType() : $comment->getActorType();
$subjectData['userId'] = $reaction ? $reaction->getActorId() : $comment->getActorId();

$messageData = [
'commentId' => $comment->getId(),
];

if ($threadId !== null && $threadId !== 0) {
$messageData['threadId'] = $threadId;
}

$notification = $this->notificationManager->createNotification();
$notification
->setApp('spreed')
->setObject('chat', $chat->getToken())
->setSubject($subject, $subjectData)
->setMessage($comment->getVerb(), [
'commentId' => $comment->getId(),
])
->setMessage($comment->getVerb(), $messageData)
->setDateTime($reaction ? $reaction->getCreationDateTime() : $comment->getCreationDateTime());

return $notification;
Expand Down
13 changes: 0 additions & 13 deletions lib/Chat/SystemMessage/Listener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,19 +443,6 @@ protected function fixMimeTypeOfVoiceMessage(ShareCreatedEvent|BeforeDuplicateSh
silent: true,
parent: $comment,
);
} else {
$threadId = (int)$comment->getTopmostParentId();
if ($threadId !== 0) {
$isThread = $this->threadService->updateLastMessageInfoAfterReply($threadId, $messageId);
if ($isThread) {
try {
// Add to subscribed threads list
$participant = $this->participantService->getParticipant($room, $this->getUserId());
$this->threadService->ensureIsThreadAttendee($participant->getAttendee(), $threadId);
} catch (ParticipantNotFoundException) {
}
}
}
}
}

Expand Down
12 changes: 11 additions & 1 deletion lib/Notification/Notifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,14 @@ protected function parseChatMessage(INotification $notification, Room $room, Par
];

// Set the link to the specific message
$notification->setLink($this->url->linkToRouteAbsolute('spreed.Page.showCall', ['token' => $room->getToken()]) . '#message_' . $message->getMessageId());
$urlParams = [
'token' => $room->getToken(),
'_fragment' => 'message_' . $message->getMessageId(),
];
if (isset($messageParameters['threadId'])) {
$urlParams['threadId'] = $messageParameters['threadId'];
}
$notification->setLink($this->url->linkToRouteAbsolute('spreed.Page.showCall', $urlParams));

$now = $this->timeFactory->getDateTime();
$expireDate = $message->getExpirationDateTime();
Expand Down Expand Up @@ -632,6 +639,9 @@ protected function parseChatMessage(INotification $notification, Room $room, Par

// Forward the message ID as well to the clients, so they can quote the message on replies
$notification->setObject($notification->getObjectType(), $notification->getObjectId() . '/' . $message->getMessageId());
if (isset($messageParameters['threadId'])) {
$notification->setObject($notification->getObjectType(), $notification->getObjectId() . '/' . $messageParameters['threadId']);
}
}

$richSubjectParameters = [
Expand Down
10 changes: 7 additions & 3 deletions lib/Search/MessageSearch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,18 @@ protected function commentToSearchResultEntry(Room $room, IUser $user, IComment
}
}

$urlParams = [
'token' => $room->getToken(),
'_fragment' => 'message_' . $id,
];
$threadId = (int)$comment->getTopmostParentId() ?: (int)$comment->getId();
try {
$thread = $this->threadService->findByThreadId($room->getId(), $threadId);
$urlParams['threadId'] = $thread->getId();
} catch (DoesNotExistException) {
$thread = null;
}

$entry = new SearchResultEntry(
$iconUrl,
str_replace(
Expand All @@ -299,9 +305,7 @@ protected function commentToSearchResultEntry(Room $room, IUser $user, IComment
$subline
),
$messageStr,
$this->url->linkToRouteAbsolute('spreed.Page.showCall', ['token' => $room->getToken()])
. ($thread !== null ? '?threadId=' . $thread->getId() : '')
. '#message_' . $comment->getId(),
$this->url->linkToRouteAbsolute('spreed.Page.showCall', $urlParams),
'icon-talk', // $iconClass,
true
);
Expand Down
15 changes: 9 additions & 6 deletions tests/integration/features/bootstrap/FeatureContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3493,14 +3493,17 @@ private function assertNotifications(array $notifications, TableNode $formData):
$data = [];
if (isset($expectedNotification['object_id'])) {
if (str_contains($notification['object_id'], '/')) {
[$roomToken, $message] = explode('/', $notification['object_id']);
$messageText = self::$messageIdToText[$message] ?? 'UNKNOWN_MESSAGE';

$messageText = str_replace($this->localServerUrl, '{$LOCAL_URL}', $messageText);
$messageText = str_replace($this->remoteServerUrl, '{$REMOTE_URL}', $messageText);
$parts = explode('/', $notification['object_id']);
$roomToken = $parts[0];
$message = $parts[1];

$messageText = self::$messageIdToText[$message] ?? 'UNKNOWN_MESSAGE';
$messageText = str_replace([$this->localServerUrl, $this->remoteServerUrl], ['{$LOCAL_URL}', '{$REMOTE_URL}'], $messageText);
$data['object_id'] = self::$tokenToIdentifier[$roomToken] . '/' . $messageText;
} elseif (strpos($expectedNotification['object_id'], 'INVITE_ID') !== false) {
if (isset($parts[2])) {
$data['object_id'] .= '/' . self::$threadIdToTitle[$parts[2]];
}
} elseif (str_contains($expectedNotification['object_id'], 'INVITE_ID')) {
$data['object_id'] = 'INVITE_ID(' . self::$inviteIdToRemote[$notification['object_id']] . ')';
} else {
[$roomToken,] = explode('/', $notification['object_id']);
Expand Down
16 changes: 11 additions & 5 deletions tests/integration/features/chat-4/threads.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,21 @@ Feature: chat-4/threads
And user "participant2" sends reply "Message 1-2" on message "Message 1" to room "room" with 201
And user "participant1" has the following notifications
| app | object_type | object_id | subject |
| spreed | chat | room/Message 1-2 | participant2-displayname replied to your message in conversation room |
| spreed | chat | room/Message 1-2/Thread 1 | participant2-displayname replied to your message in conversation room |
And user "participant1" subscribes to thread "Message 1" in room "room" with notification level 1 with 200
| t.id | t.title | t.numReplies | t.lastMessage | a.notificationLevel | firstMessage | lastMessage |
| Message 1 | Thread 1 | 2 | Message 1-2 | 1 | Message 1 | Message 1-2 |
And user "participant2" sends reply "Message 1-3" on thread "Thread 1" to room "room" with 201
And user "participant1" has the following notifications
| app | object_type | object_id | subject |
| spreed | chat | room/Message 1-3 | participant2-displayname sent a message in conversation room |
| spreed | chat | room/Message 1-2 | participant2-displayname replied to your message in conversation room |
| app | object_type | object_id | subject |
| spreed | chat | room/Message 1-3/Thread 1 | participant2-displayname sent a message in conversation room |
| spreed | chat | room/Message 1-2/Thread 1 | participant2-displayname replied to your message in conversation room |
When user "participant2" sends reply "@participant1" on thread "Thread 1" to room "room" with 201
Then user "participant1" has the following notifications
| app | object_type | object_id | subject |
| spreed | chat | room/@participant1/Thread 1 | participant2-displayname mentioned you in conversation room |
| spreed | chat | room/Message 1-3/Thread 1 | participant2-displayname sent a message in conversation room |
| spreed | chat | room/Message 1-2/Thread 1 | participant2-displayname replied to your message in conversation room |

Scenario: Thread titles are trimmed
Given user "participant1" creates room "room" (v4)
Expand Down Expand Up @@ -214,7 +220,7 @@ Feature: chat-4/threads
| Message 1 | room1 | Thread 1 | 1 | Message 2 | 0 | Message 1 | Message 2 |
And user "participant1" has the following notifications
| app | object_type | object_id | subject |
| spreed | chat | room1/Message 2 | participant2-displayname replied to your message in conversation room1 |
| spreed | chat | room1/Message 2/Thread 1 | participant2-displayname replied to your message in conversation room1 |
Then user "participant2" sees the following subscribed threads
| t.id | t.token | t.title | t.numReplies | t.lastMessage | a.notificationLevel | firstMessage | lastMessage |
| Message 1 | room1 | Thread 1 | 1 | Message 2 | 0 | Message 1 | Message 2 |
Expand Down
Loading