Skip to content

Commit

Permalink
fix(dashboard): Dashboard does not show mentions from federated conve…
Browse files Browse the repository at this point in the history 
…rsations

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
@nickvergessen
nickvergessen committed Apr 19, 2024
1 parent ac17e26 commit 79adeb2
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 29 deletions.
77 changes: 51 additions & 26 deletions lib/Dashboard/TalkWidget.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,13 @@
use OCA\Talk\Config;
use OCA\Talk\Manager;
use OCA\Talk\Model\BreakoutRoom;
use OCA\Talk\Model\Message;
use OCA\Talk\Participant;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCA\Talk\Service\ParticipantService;
use OCA\Talk\Service\ProxyCacheMessageService;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Comments\IComment;
use OCP\Dashboard\IAPIWidget;
Expand Down Expand Up @@ -64,6 +67,7 @@ public function __construct(
protected ParticipantService $participantService,
protected MessageParser $messageParser,
protected ChatManager $chatManager,
protected ProxyCacheMessageService $pcmService,
protected ITimeFactory $timeFactory,
) {
}
Expand Down Expand Up @@ -156,13 +160,13 @@ public function getItems(string $userId, ?string $since = null, int $limit = 7):
$participant = $this->participantService->getParticipant($room, $userId);
$attendee = $participant->getAttendee();

if ($attendee->getLastMentionMessage() > $attendee->getLastReadMessage()) {
if (($room->isFederatedConversation() && $attendee->getLastMentionMessage())
|| (!$room->isFederatedConversation() && $attendee->getLastMentionMessage() > $attendee->getLastReadMessage())) {
return true;
}

return ($room->getType() === Room::TYPE_ONE_TO_ONE || $room->getType() === Room::TYPE_ONE_TO_ONE_FORMER)
&& $room->getLastMessage()
&& $room->getLastMessage()->getId() > $attendee->getLastReadMessage()
&& $room->getLastMessageId() > $attendee->getLastReadMessage()
&& $this->chatManager->getUnreadCount($room, $attendee->getLastReadMessage()) > 0;
});

Expand Down Expand Up @@ -200,14 +204,15 @@ public function getItemsV2(string $userId, ?string $since = null, int $limit = 7
continue;
}

if ($attendee->getLastMentionMessage() > $attendee->getLastReadMessage()) {
if (($room->isFederatedConversation() && $attendee->getLastMentionMessage())
|| (!$room->isFederatedConversation() && $attendee->getLastMentionMessage() > $attendee->getLastReadMessage())) {
// Really mentioned
$mentions[] = $room;
continue;
}

if (($room->getType() === Room::TYPE_ONE_TO_ONE || $room->getType() === Room::TYPE_ONE_TO_ONE_FORMER)
&& $room->getLastMessage()?->getId() > $attendee->getLastReadMessage()) {
&& $room->getLastMessageId() > $attendee->getLastReadMessage()) {
// If there are "unread" messages in one-to-one or former one-to-one
// we check if they are actual messages or system messages not
// considered by the read-marker
Expand Down Expand Up @@ -242,33 +247,29 @@ protected function prepareRoom(Room $room, string $userId): WidgetItem {
$participant = $this->participantService->getParticipant($room, $userId);
$subtitle = '';

$lastMessage = $room->getLastMessage();
if ($lastMessage instanceof IComment) {
if ($room->getLastMessageId() && $room->isFederatedConversation()) {
try {
$cachedMessage = $this->pcmService->findByRemote(
$room->getRemoteServer(),
$room->getRemoteToken(),
$room->getLastMessageId(),
);
$message = $this->messageParser->createMessageFromProxyCache($room, $participant, $cachedMessage, $this->l10n);
$subtitle = $this->getSubtitleFromMessage($message);
} catch (DoesNotExistException $e) {
// Fallback to empty subtitle
}
} else if ($room->getLastMessageId() && !$room->isFederatedConversation()) {
$message = $this->messageParser->createMessage($room, $participant, $room->getLastMessage(), $this->l10n);
$this->messageParser->parseMessage($message);

$now = $this->timeFactory->getDateTime();
$expireDate = $message->getComment()->getExpireDate();
if ((!$expireDate instanceof \DateTime || $expireDate >= $now)
&& $message->getVisibility()) {
$placeholders = $replacements = [];

foreach ($message->getMessageParameters() as $placeholder => $parameter) {
$placeholders[] = '{' . $placeholder . '}';
if ($parameter['type'] === 'user' || $parameter['type'] === 'guest') {
$replacements[] = '@' . $parameter['name'];
} else {
$replacements[] = $parameter['name'];
}
}

$subtitle = str_replace($placeholders, $replacements, $message->getMessage());
}
$subtitle = $this->getSubtitleFromMessage($message);
}

$attendee = $participant->getAttendee();
if ($room->getCallFlag() !== Participant::FLAG_DISCONNECTED) {
$subtitle = $this->l10n->t('Call in progress');
} elseif ($participant->getAttendee()->getLastMentionMessage() > $participant->getAttendee()->getLastReadMessage()) {
} elseif (($room->isFederatedConversation() && $attendee->getLastMentionMessage())
|| (!$room->isFederatedConversation() && $attendee->getLastMentionMessage() > $attendee->getLastReadMessage())) {
$subtitle = $this->l10n->t('You were mentioned');
}

Expand All @@ -280,6 +281,30 @@ protected function prepareRoom(Room $room, string $userId): WidgetItem {
);
}

protected function getSubtitleFromMessage(Message $message): string {
$expireDate = $message->getExpirationDateTime();
if ($expireDate instanceof \DateTimeInterface
&& $expireDate <= $this->timeFactory->getDateTime()) {
return '';
}

if (!$message->getVisibility()) {
return '';
}

$placeholders = $replacements = [];
foreach ($message->getMessageParameters() as $placeholder => $parameter) {
$placeholders[] = '{' . $placeholder . '}';
if ($parameter['type'] === 'user' || $parameter['type'] === 'guest') {
$replacements[] = '@' . $parameter['name'];
} else {
$replacements[] = $parameter['name'];
}
}

return str_replace($placeholders, $replacements, $message->getMessage());
}

protected function sortRooms(Room $roomA, Room $roomB): int {
if ($roomA->getCallFlag() !== $roomB->getCallFlag()) {
return $roomA->getCallFlag() !== Participant::FLAG_DISCONNECTED ? -1 : 1;
Expand Down
6 changes: 5 additions & 1 deletion lib/Manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1326,7 +1326,11 @@ public function isGuestUser(string $userId): bool {
}

protected function loadLastMessageInfo(IQueryBuilder $query): void {
$query->leftJoin('r', 'comments', 'c', $query->expr()->eq('r.last_message', 'c.id'));
$query->leftJoin('r', 'comments', 'c', $query->expr()->andX(
$query->expr()->eq('r.last_message', 'c.id'),
$query->expr()->emptyString('r.remote_server'),
));
// $query->leftJoin('r', 'comments', 'c', $query->expr()->eq('r.last_message', 'c.id'));
$query->selectAlias('c.id', 'comment_id');
$query->selectAlias('c.parent_id', 'comment_parent_id');
$query->selectAlias('c.topmost_parent_id', 'comment_topmost_parent_id');
Expand Down
6 changes: 5 additions & 1 deletion lib/Room.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,11 @@ public function setLastMessageId(int $lastMessageId): void {
}

public function getLastMessage(): ?IComment {
if ($this->lastMessageId && $this->lastMessage === null && !$this->isFederatedConversation()) {
if ($this->isFederatedConversation()) {
return null;
}

if ($this->lastMessageId && $this->lastMessage === null) {
$this->lastMessage = $this->manager->loadLastCommentInfo($this->lastMessageId);
if ($this->lastMessage === null) {
$this->lastMessageId = 0;
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/features/bootstrap/FeatureContext.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2357,12 +2357,14 @@ public function userGetsDashboardWidgetItems($user, $widgetId, $apiVersion = 'v1
$actualItems = $data[$widgetId]['items'];
}

$actualItems = array_filter($actualItems, static fn ($item) => $item['title'] !== 'Note to self' && $item['title'] !== 'Talk updates ✅');

if (empty($expectedItems)) {
Assert::assertEmpty($actualItems);
return;
}

Assert::assertCount(count($expectedItems), $actualItems);
Assert::assertCount(count($expectedItems), $actualItems, json_encode($actualItems, JSON_PRETTY_PRINT));

foreach ($expectedItems as $key => $item) {
$token = self::$identifierToToken[$item['link']];
Expand Down
11 changes: 11 additions & 0 deletions tests/integration/features/federation/chat.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,18 @@ Feature: federation/chat
Given user "participant2" joins room "LOCAL::room" with 200 (v4)
Given user "participant2" leaves room "LOCAL::room" with 200 (v4)
And user "participant2" sends message "Message 1" to room "LOCAL::room" with 201
Then user "participant2" sees the following entries for dashboard widgets "spreed" (v1)
| title | subtitle | link | iconUrl | sinceId | overlayIconUrl |
Then user "participant2" sees the following entries for dashboard widgets "spreed" (v2)
| title | subtitle | link | iconUrl | sinceId | overlayIconUrl |
| room | Message 1 | LOCAL::room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar{version} | | |
When user "participant1" sends reply "Message 1-1" on message "Message 1" to room "room" with 201
Then user "participant2" sees the following entries for dashboard widgets "spreed" (v1)
| title | subtitle | link | iconUrl | sinceId | overlayIconUrl |
| room | You were mentioned | LOCAL::room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar{version} | | |
Then user "participant2" sees the following entries for dashboard widgets "spreed" (v2)
| title | subtitle | link | iconUrl | sinceId | overlayIconUrl |
| room | You were mentioned | LOCAL::room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar{version} | | |
And user "participant1" sends message 'Hi @"federated_user/participant2@{$REMOTE_URL}" bye' to room "room" with 201
And user "participant1" sends message 'Hi @all bye' to room "room" with 201
Then user "participant2" has the following notifications
Expand Down

0 comments on commit 79adeb2

Please sign in to comment.