Skip to content
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

feat(chat): Edit message mention support #11365

Merged
merged 2 commits into from
Jan 18, 2024
Merged

feat(chat): Edit message mention support #11365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d956f20
feat(tests): Add first mention notification test
nickvergessen Dec 20, 2023
6ed6950
fix(chat): Diff mentions when editing messages
nickvergessen Dec 20, 2023
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
36 changes: 35 additions & 1 deletion lib/Chat/ChatManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,11 +508,38 @@ public function editMessage(Room $chat, IComment $comment, Participant $particip
$metaData['last_edited_by_id'] = $participant->getAttendee()->getActorId();
$metaData['last_edited_time'] = $editTime->getTimestamp();
$comment->setMetaData($metaData);

$mentionsBefore = $comment->getMentions();
$usersDirectlyMentionedBefore = $this->notifier->getMentionedUserIds($comment);
$usersToNotifyBefore = $this->notifier->getUsersToNotify($chat, $comment, []);
$comment->setMessage($message, self::MAX_CHAT_LENGTH);
$mentionsAfter = $comment->getMentions();

$this->commentsManager->save($comment);
$this->referenceManager->invalidateCache($chat->getToken());

// TODO update mentions/notifications
$removedMentions = empty($mentionsAfter) ? $mentionsBefore : array_udiff($mentionsBefore, $mentionsAfter, [$this, 'compareMention']);
$addedMentions = empty($mentionsBefore) ? $mentionsAfter : array_udiff($mentionsAfter, $mentionsBefore, [$this, 'compareMention']);

// FIXME Not needed when it was silent, once it's stored in metadata
if (!empty($removedMentions)) {
$usersToNotifyAfter = $this->notifier->getUsersToNotify($chat, $comment, []);
$removedUsersMentioned = array_udiff($usersToNotifyBefore, $usersToNotifyAfter, [$this, 'compareMention']);
$userIds = array_column($removedUsersMentioned, 'id');
$this->notifier->removeMentionNotificationAfterEdit($chat, $comment, $userIds);
}

// FIXME silent support, once it's stored in metadata
if (!empty($addedMentions)) {
$usersDirectlyMentionedAfter = $this->notifier->getMentionedUserIds($comment);
$addedUsersDirectMentioned = array_diff($usersDirectlyMentionedAfter, $usersDirectlyMentionedBefore);

$alreadyNotifiedUsers = $this->notifier->notifyMentionedUsers($chat, $comment, $usersToNotifyBefore, silent: false);
if (!empty($alreadyNotifiedUsers)) {
$userIds = array_column($alreadyNotifiedUsers, 'id');
$this->participantService->markUsersAsMentioned($chat, $userIds, (int) $comment->getId(), $addedUsersDirectMentioned);
}
}

return $this->addSystemMessage(
$chat,
Expand All @@ -527,6 +554,13 @@ public function editMessage(Room $chat, IComment $comment, Participant $particip
);
}

protected static function compareMention(array $mention1, array $mention2): int {
if ($mention1['type'] === $mention2['type']) {
return $mention1['id'] <=> $mention2['id'];
}
return $mention1['type'] <=> $mention2['type'];
}

public function clearHistory(Room $chat, string $actorType, string $actorId): IComment {
$this->commentsManager->deleteCommentsAtObject('chat', (string) $chat->getId());

Expand Down
44 changes: 37 additions & 7 deletions lib/Chat/Notifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function __construct(
* are able to participate in the room.
*
* @param array[] $alreadyNotifiedUsers
* @psalm-param array<int, array{id: string, type: string, reason: string}> $alreadyNotifiedUsers
* @psalm-param array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}> $alreadyNotifiedUsers
* @return string[] Users that were mentioned
* @psalm-return array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}>
*/
Expand Down Expand Up @@ -117,11 +117,11 @@ public function notifyMentionedUsers(Room $chat, IComment $comment, array $alrea
* @param Room $chat
* @param IComment $comment
* @param array $alreadyNotifiedUsers
* @psalm-param array<int, array{id: string, type: string, reason?: string}> $alreadyNotifiedUsers
* @psalm-param array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}> $alreadyNotifiedUsers
* @return array
* @psalm-return array<int, array{id: string, type: string, reason?: string, sourceId?: string, attendee?: Attendee}>
* @psalm-return array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}>
*/
private function getUsersToNotify(Room $chat, IComment $comment, array $alreadyNotifiedUsers): array {
public function getUsersToNotify(Room $chat, IComment $comment, array $alreadyNotifiedUsers): array {
$usersToNotify = $this->getMentionedUsers($comment);
$usersToNotify = $this->getMentionedGroupMembers($chat, $comment, $usersToNotify);
$usersToNotify = $this->addMentionAllToList($chat, $usersToNotify);
Expand All @@ -132,11 +132,11 @@ private function getUsersToNotify(Room $chat, IComment $comment, array $alreadyN

/**
* @param array $usersToNotify
* @psalm-param array<int, array{id: string, type: string, reason?: string, sourceId?: string, attendee?: Attendee}> $usersToNotify
* @psalm-param array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}> $usersToNotify
* @param array $alreadyNotifiedUsers
* @psalm-param array<int, array{id: string, type: string, reason?: string}> $alreadyNotifiedUsers
* @psalm-param array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}> $alreadyNotifiedUsers
* @return array
* @psalm-return array<int, array{id: string, type: string, reason?: string, sourceId?: string, attendee?: Attendee}>
* @psalm-return array<int, array{id: string, type: string, reason: string, sourceId?: string, attendee?: Attendee}>
*/
private function removeAlreadyNotifiedUsers(array $usersToNotify, array $alreadyNotifiedUsers): array {
return array_filter($usersToNotify, static function (array $userToNotify) use ($alreadyNotifiedUsers): bool {
Expand Down Expand Up @@ -364,6 +364,36 @@ public function markMentionNotificationsRead(Room $chat, ?string $userId): void
}
}

/**
* Remove all mention notifications of users that got their mention removed
*
* @param list<string> $userIds
*/
public function removeMentionNotificationAfterEdit(Room $chat, IComment $comment, array $userIds): void {
$shouldFlush = $this->notificationManager->defer();
$notification = $this->notificationManager->createNotification();

$notification
->setApp('spreed')
->setObject('chat', $chat->getToken())
// FIXME message_parameters are not handled by notification app, so this removes all notifications :(
->setMessage('comment', [
'commentId' => $comment->getId(),
]);

foreach (['mention_all', 'mention_direct'] as $subject) {
$notification->setSubject($subject);
foreach ($userIds as $userId) {
$notification->setUser($userId);
$this->notificationManager->markProcessed($notification);
}
}

if ($shouldFlush) {
$this->notificationManager->flush();
}
}

/**
* Returns the IDs of the users mentioned in the given comment.
*
Expand Down
6 changes: 4 additions & 2 deletions lib/Service/ParticipantService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,7 +1267,8 @@ public function markUsersAsMentioned(Room $room, array $userIds, int $messageId,
->set('last_mention_message', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT))
->where($update->expr()->eq('room_id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)))
->andWhere($update->expr()->eq('actor_type', $update->createNamedParameter(Attendee::ACTOR_USERS)))
->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY)));
->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY)))
->andWhere($update->expr()->lt('last_mention_message', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT)));
$update->executeStatement();

if (!empty($usersDirectlyMentioned)) {
Expand All @@ -1276,7 +1277,8 @@ public function markUsersAsMentioned(Room $room, array $userIds, int $messageId,
->set('last_mention_direct', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT))
->where($update->expr()->eq('room_id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)))
->andWhere($update->expr()->eq('actor_type', $update->createNamedParameter(Attendee::ACTOR_USERS)))
->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($usersDirectlyMentioned, IQueryBuilder::PARAM_STR_ARRAY)));
->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($usersDirectlyMentioned, IQueryBuilder::PARAM_STR_ARRAY)))
->andWhere($update->expr()->lt('last_mention_direct', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT)));
$update->executeStatement();
}
}
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/features/bootstrap/FeatureContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3140,7 +3140,7 @@ public function userNotifications(string $user, TableNode $body = null): void {

if ($body === null) {
self::$lastNotifications = [];
Assert::assertCount(0, $data);
Assert::assertCount(0, $data, json_encode($data, JSON_PRETTY_PRINT));
return;
}

Expand All @@ -3149,7 +3149,7 @@ public function userNotifications(string $user, TableNode $body = null): void {
}

private function assertNotifications($notifications, TableNode $formData) {
Assert::assertCount(count($formData->getHash()), $notifications, 'Notifications count does not match');
Assert::assertCount(count($formData->getHash()), $notifications, 'Notifications count does not match:' . "\n" . json_encode($notifications, JSON_PRETTY_PRINT));
Assert::assertEquals($formData->getHash(), array_map(function ($notification, $expectedNotification) {
$data = [];
if (isset($expectedNotification['object_id'])) {
Expand Down Expand Up @@ -3181,7 +3181,7 @@ private function assertNotifications($notifications, TableNode $formData) {
}

return $data;
}, $notifications, $formData->getHash()));
}, $notifications, $formData->getHash()), json_encode($notifications, JSON_PRETTY_PRINT));
}

/**
Expand Down
Loading
Loading