diff options
author | Joas Schilling <coding@schilljs.com> | 2024-05-03 11:25:07 +0200 |
---|---|---|
committer | Joas Schilling <coding@schilljs.com> | 2024-05-16 16:58:34 +0200 |
commit | dc2682e5321414d5ad4b1768d90d852d13d20556 (patch) | |
tree | 13b1b0daee14e6de99ab26cd27efddc152ac7ba6 | |
parent | 3dd943836a9eb56157b2d7fb3552221efdfba2ff (diff) |
fix(chat): Better handling of captioned messages in federated conversations
Signed-off-by: Joas Schilling <coding@schilljs.com>
-rw-r--r-- | lib/Chat/Parser/SystemMessage.php | 19 | ||||
-rw-r--r-- | tests/integration/features/sharing-1/delete.feature | 7 | ||||
-rw-r--r-- | tests/php/Chat/Parser/SystemMessageTest.php | 42 |
3 files changed, 39 insertions, 29 deletions
diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php index 8d7ae3288..a5d4d68e1 100644 --- a/lib/Chat/Parser/SystemMessage.php +++ b/lib/Chat/Parser/SystemMessage.php @@ -485,7 +485,7 @@ class SystemMessage implements IEventListener { } } elseif ($message === 'file_shared') { try { - $parsedParameters['file'] = $this->getFileFromShare($participant, $parameters['share']); + $parsedParameters['file'] = $this->getFileFromShare($room, $participant, $parameters['share']); $parsedMessage = '{file}'; $metaData = $parameters['metaData'] ?? []; if (isset($metaData['messageType'])) { @@ -505,10 +505,19 @@ class SystemMessage implements IEventListener { if (isset($metaData['caption']) && $metaData['caption'] !== '') { $parsedMessage = $metaData['caption']; } - } catch (\Exception $e) { + } catch (\Exception) { + $chatMessage->setMessageType(ChatManager::VERB_MESSAGE); $parsedMessage = $this->l->t('{actor} shared a file which is no longer available'); if ($currentUserIsActor) { $parsedMessage = $this->l->t('You shared a file which is no longer available'); + } elseif ($currentActorType === Attendee::ACTOR_FEDERATED_USERS) { + $parsedMessage = $this->l->t('File shares are currently not supported in federated conversations'); + } + $parsedMessage = '*' . $parsedMessage . '*'; + + $metaData = $parameters['metaData'] ?? []; + if (isset($metaData['caption']) && $metaData['caption'] !== '') { + $parsedMessage .= "\n\n" . $metaData['caption']; } } } elseif ($message === 'object_shared') { @@ -726,10 +735,10 @@ class SystemMessage implements IEventListener { * @throws NotFoundException * @throws ShareNotFound */ - protected function getFileFromShare(?Participant $participant, string $shareId): array { + protected function getFileFromShare(Room $room, ?Participant $participant, string $shareId): array { $share = $this->shareProvider->getShareById((int) $shareId); - if ($participant && !$participant->isGuest()) { + if ($participant && $participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) { if ($share->getShareOwner() !== $participant->getAttendee()->getActorId()) { $userFolder = $this->rootFolder->getUserFolder($participant->getAttendee()->getActorId()); if ($userFolder instanceof Node) { @@ -769,6 +778,8 @@ class SystemMessage implements IEventListener { $url = $this->url->linkToRouteAbsolute('files.viewcontroller.showFile', [ 'fileid' => $node->getId(), ]); + } elseif ($participant && $room->getType() !== Room::TYPE_PUBLIC && $participant->getAttendee()->getActorType() === Attendee::ACTOR_FEDERATED_USERS) { + throw new ShareNotFound(); } else { $node = $share->getNode(); $name = $node->getName(); diff --git a/tests/integration/features/sharing-1/delete.feature b/tests/integration/features/sharing-1/delete.feature index d5d472001..21ae2a589 100644 --- a/tests/integration/features/sharing-1/delete.feature +++ b/tests/integration/features/sharing-1/delete.feature @@ -384,7 +384,6 @@ Feature: delete | poll | 0 | | voice | 0 | | recording | 0 | - And user "participant1" sees the following system messages in room "public room" with 200 - | room | actorType | actorId | actorDisplayName | systemMessage | - | public room | users | participant1 | participant1-displayname | file_shared | - | public room | users | participant1 | participant1-displayname | conversation_created | + And user "participant1" sees the following messages in room "public room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | public room | users | participant1 | participant1-displayname | *You shared a file which is no longer available* | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | diff --git a/tests/php/Chat/Parser/SystemMessageTest.php b/tests/php/Chat/Parser/SystemMessageTest.php index 57921a0db..f44a4955f 100644 --- a/tests/php/Chat/Parser/SystemMessageTest.php +++ b/tests/php/Chat/Parser/SystemMessageTest.php @@ -356,11 +356,11 @@ class SystemMessageTest extends TestCase { ['actor' => ['id' => 'actor', 'type' => 'user'], 'file' => ['id' => 'file-from-share']], ], ['file_shared', ['share' => InvalidPathException::class], 'recipient', - '{actor} shared a file which is no longer available', + '*{actor} shared a file which is no longer available*', ['actor' => ['id' => 'actor', 'type' => 'user']], ], ['file_shared', ['share' => NotFoundException::class], 'actor', - 'You shared a file which is no longer available', + '*You shared a file which is no longer available*', ['actor' => ['id' => 'actor', 'type' => 'user']], ], ['read_only', [], 'recipient', @@ -525,12 +525,12 @@ class SystemMessageTest extends TestCase { if (is_subclass_of($parameters['share'], \Exception::class)) { $parser->expects($this->once()) ->method('getFileFromShare') - ->with($participant, $parameters['share']) + ->with($room, $participant, $parameters['share']) ->willThrowException(new $parameters['share']()); } else { $parser->expects($this->once()) ->method('getFileFromShare') - ->with($participant, $parameters['share']) + ->with($room, $participant, $parameters['share']) ->willReturn(['id' => 'file-from-share']); } } else { @@ -588,6 +588,7 @@ class SystemMessageTest extends TestCase { } public function testGetFileFromShareForGuest(): void { + $room = $this->createMock(Room::class); $node = $this->createMock(Node::class); $node->expects($this->once()) ->method('getId') @@ -639,9 +640,6 @@ class SystemMessageTest extends TestCase { ->willReturn(['width' => 1234, 'height' => 4567]); $participant = $this->createMock(Participant::class); - $participant->expects($this->once()) - ->method('isGuest') - ->willReturn(true); $parser = $this->getParser(); $this->assertSame([ @@ -657,10 +655,11 @@ class SystemMessageTest extends TestCase { 'preview-available' => 'yes', 'width' => '1234', 'height' => '4567', - ], self::invokePrivate($parser, 'getFileFromShare', [$participant, '23'])); + ], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23'])); } public function testGetFileFromShareForOwner(): void { + $room = $this->createMock(Room::class); $node = $this->createMock(Node::class); $node->expects($this->exactly(2)) ->method('getId') @@ -713,9 +712,6 @@ class SystemMessageTest extends TestCase { ->method('getMetadataPhotosSizeForFileId'); $participant = $this->createMock(Participant::class); - $participant->expects($this->once()) - ->method('isGuest') - ->willReturn(false); $attendee = Attendee::fromRow([ 'actor_type' => 'users', 'actor_id' => 'owner', @@ -736,19 +732,17 @@ class SystemMessageTest extends TestCase { 'permissions' => '27', 'mimetype' => 'httpd/unix-directory', 'preview-available' => 'no', - ], self::invokePrivate($parser, 'getFileFromShare', [$participant, '23'])); + ], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23'])); } public function testGetFileFromShareForRecipient(): void { + $room = $this->createMock(Room::class); $share = $this->createMock(IShare::class); $share->expects($this->any()) ->method('getNodeId') ->willReturn(54); $participant = $this->createMock(Participant::class); - $participant->expects($this->once()) - ->method('isGuest') - ->willReturn(false); $attendee = Attendee::fromRow([ 'actor_type' => 'users', 'actor_id' => 'user', @@ -823,19 +817,17 @@ class SystemMessageTest extends TestCase { 'permissions' => '27', 'mimetype' => 'application/octet-stream', 'preview-available' => 'no', - ], self::invokePrivate($parser, 'getFileFromShare', [$participant, '23'])); + ], self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23'])); } public function testGetFileFromShareForRecipientThrows(): void { + $room = $this->createMock(Room::class); $share = $this->createMock(IShare::class); $share->expects($this->any()) ->method('getNodeId') ->willReturn(54); $participant = $this->createMock(Participant::class); - $participant->expects($this->once()) - ->method('isGuest') - ->willReturn(false); $attendee = Attendee::fromRow([ 'actor_type' => 'users', 'actor_id' => 'user', @@ -865,19 +857,27 @@ class SystemMessageTest extends TestCase { $parser = $this->getParser(); $this->expectException(NotFoundException::class); - self::invokePrivate($parser, 'getFileFromShare', [$participant, '23']); + self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']); } public function testGetFileFromShareThrows(): void { + $room = $this->createMock(Room::class); $this->shareProvider->expects($this->once()) ->method('getShareById') ->with('23') ->willThrowException(new ShareNotFound()); $participant = $this->createMock(Participant::class); + $attendee = Attendee::fromRow([ + 'actor_type' => 'users', + 'actor_id' => 'user', + ]); + $participant->expects($this->any()) + ->method('getAttendee') + ->willReturn($attendee); $parser = $this->getParser(); $this->expectException(ShareNotFound::class); - self::invokePrivate($parser, 'getFileFromShare', [$participant, '23']); + self::invokePrivate($parser, 'getFileFromShare', [$room, $participant, '23']); } public static function dataGetActor(): array { |