From cb3fa784d49afd72587130ec7fe9b3b71bb12a13 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Mon, 1 May 2023 14:31:28 -0300 Subject: [PATCH 01/19] Fixes to be able identify if have a custom avatar * Added the header X-NC-IsCustomAvatar * Return every time a non empty string as avatarVersion Signed-off-by: Vitor Mattos --- lib/Controller/AvatarController.php | 1 + lib/Service/AvatarService.php | 34 ++++++++++++++----- .../features/bootstrap/FeatureContext.php | 27 +++++++++++++++ .../features/conversation/avatar.feature | 8 +++++ tests/php/Service/AvatarServiceTest.php | 11 +++--- 5 files changed, 68 insertions(+), 13 deletions(-) diff --git a/lib/Controller/AvatarController.php b/lib/Controller/AvatarController.php index 7edebf977d..876f0dc22b 100644 --- a/lib/Controller/AvatarController.php +++ b/lib/Controller/AvatarController.php @@ -86,6 +86,7 @@ class AvatarController extends AEnvironmentAwareController { $response = new FileDisplayResponse($file); $response->addHeader('Content-Type', $file->getMimeType()); + $response->addHeader('X-NC-IsCustomAvatar', $this->avatarService->isCustomAvatar($this->getRoom()) ? '1' : '0'); // Cache for 1 day $response->cacheFor(60 * 60 * 24, false, true); return $response; diff --git a/lib/Service/AvatarService.php b/lib/Service/AvatarService.php index 5dd942fc47..2a05137612 100644 --- a/lib/Service/AvatarService.php +++ b/lib/Service/AvatarService.php @@ -185,19 +185,26 @@ class AvatarService { } elseif ($this->emojiHelper->isValidSingleEmoji(mb_substr($room->getName(), 0, 1))) { $file = new InMemoryFile($token, $this->getEmojiAvatar($room->getName(), $darkTheme)); } elseif ($room->getType() === Room::TYPE_CHANGELOG) { - $file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/changelog.svg')); + $fileName = 'changelog.svg'; + $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } elseif ($room->getObjectType() === 'file') { - $file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-text-' . $colorTone . '.svg')); + $fileName = 'icon-conversation-text-' . $colorTone . '.svg'; + $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } elseif ($room->getObjectType() === 'share:password') { - $file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-password-' . $colorTone . '.svg')); + $fileName = 'icon-conversation-password-' . $colorTone . '.svg'; + $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } elseif ($room->getObjectType() === 'emails') { - $file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-mail-' . $colorTone . '.svg')); + $fileName = 'icon-conversation-mail-' . $colorTone . '.svg'; + $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } elseif ($room->getType() === Room::TYPE_PUBLIC) { - $file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-public-' . $colorTone . '.svg')); + $fileName = 'icon-conversation-public-' . $colorTone . '.svg'; + $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } elseif ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) { - $file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-user-' . $colorTone . '.svg')); + $fileName = 'icon-conversation-user-' . $colorTone . '.svg'; + $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } else { - $file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-group-' . $colorTone . '.svg')); + $fileName = 'icon-conversation-group-' . $colorTone . '.svg'; + $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } } return $file; @@ -245,6 +252,11 @@ class AvatarService { return ''; } + public function isCustomAvatar(Room $room): bool { + return $room->getAvatar() !== '' + || $this->getFirstCombinedEmoji($room->getName()); + } + public function deleteAvatar(Room $room): void { try { $folder = $this->appData->getFolder('room-avatar'); @@ -270,7 +282,11 @@ class AvatarService { public function getAvatarVersion(Room $room): string { $avatarVersion = $room->getAvatar(); - [$version] = explode('.', $avatarVersion); - return $version; + if ($avatarVersion) { + [$version] = explode('.', $avatarVersion); + return $version; + } + $file = $this->getAvatar($room, null); + return substr(md5($file->getName()), 0, 8); } } diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 6276e1bbaf..924dac174c 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -1200,6 +1200,10 @@ class FeatureContext implements Context, SnippetAcceptingContext { $xpectedAttributes = $formData->getColumnsHash()[0]; $actual = $this->getDataFromResponse($this->response); foreach ($xpectedAttributes as $attribute => $expectedValue) { + if ($expectedValue === 'NOT_EMPTY') { + Assert::assertNotEmpty($actual[$attribute]); + continue; + } Assert::assertEquals($expectedValue, $actual[$attribute]); } } @@ -3485,4 +3489,27 @@ class FeatureContext implements Context, SnippetAcceptingContext { Assert::assertEquals($statusCode, $response->getStatusCode(), $message); } } + + /** + * @Then The following headers should be set + * @param TableNode $table + * @throws \Exception + */ + public function theFollowingHeadersShouldBeSet(TableNode $table) { + foreach ($table->getTable() as $header) { + $headerName = $header[0]; + $expectedHeaderValue = $header[1]; + $returnedHeader = $this->response->getHeader($headerName)[0]; + if ($returnedHeader !== $expectedHeaderValue) { + throw new \Exception( + sprintf( + "Expected value '%s' for header '%s', got '%s'", + $expectedHeaderValue, + $headerName, + $returnedHeader + ) + ); + } + } + } } diff --git a/tests/integration/features/conversation/avatar.feature b/tests/integration/features/conversation/avatar.feature index 00965c6e59..605eecbff0 100644 --- a/tests/integration/features/conversation/avatar.feature +++ b/tests/integration/features/conversation/avatar.feature @@ -17,6 +17,8 @@ Feature: conversation/avatar | roomName | room2 | When user "participant1" uploads file "/img/favicon.png" as avatar of room "room2" with 200 Then the room "room2" has an avatar with 200 + And The following headers should be set + | X-NC-IsCustomAvatar | 1 | And user "participant1" sees the following system messages in room "room2" with 200 | room | actorType | actorId | systemMessage | message | | room2 | users | participant1 | avatar_set | You set the conversation picture | @@ -27,6 +29,12 @@ Feature: conversation/avatar | room2 | users | participant1 | avatar_removed | You removed the conversation picture | | room2 | users | participant1 | avatar_set | You set the conversation picture | | room2 | users | participant1 | conversation_created | You created the conversation | + And user "participant1" gets room "room2" with 200 (v4) + | avatarVersion | + | NOT_EMPTY | + Then the room "room2" has an avatar with 200 + And The following headers should be set + | X-NC-IsCustomAvatar | 0 | Scenario: Get avatar of conversation without custom avatar (fallback) Given user "participant1" creates room "room3" (v4) diff --git a/tests/php/Service/AvatarServiceTest.php b/tests/php/Service/AvatarServiceTest.php index 97cfc80feb..44fa822aee 100644 --- a/tests/php/Service/AvatarServiceTest.php +++ b/tests/php/Service/AvatarServiceTest.php @@ -83,16 +83,19 @@ class AvatarServiceTest extends TestCase { public function testGetAvatarVersion(string $avatar, string $expected): void { /** @var Room|MockObject $room */ $room = $this->createMock(Room::class); - $room->expects($this->once()) - ->method('getAvatar') + $room->method('getAvatar') ->willReturn($avatar); $actual = $this->service->getAvatarVersion($room); - $this->assertEquals($expected, $actual); + if ($expected === 'STRING WITH 8 CHARS') { + $this->assertEquals(8, strlen($actual)); + } else { + $this->assertEquals($expected, $actual); + } } public function dataGetAvatarVersion(): array { return [ - ['', ''], + ['', 'STRING WITH 8 CHARS'], ['1', '1'], ['1.png', '1'], ]; From abe35b37a514d15fab482f640a542beb6a4bd27f Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Mon, 1 May 2023 14:45:12 -0300 Subject: [PATCH 02/19] Document the new header Signed-off-by: Vitor Mattos --- docs/avatar.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/avatar.md b/docs/avatar.md index bfeb9e63d9..8b748e38fd 100644 --- a/docs/avatar.md +++ b/docs/avatar.md @@ -46,6 +46,12 @@ - Status code: + `200 OK` + `404 Not Found` When the conversation could not be found for the participant + - Header: + +| field | type | Description | +| ------------------- | ------ | ----------------------------------------------------------------- | +| X-NC-IsCustomAvatar | string | Return 1 when is custom avatar and 0 when is not a custom a vatar | + - Body: the image file ## Get dark mode conversations avatar (binary) From 54176a0ea63260953e2d1c51b83f32144b01a738 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 06:35:32 -0300 Subject: [PATCH 03/19] Make possible check more than one property from room object Signed-off-by: Vitor Mattos --- tests/integration/features/bootstrap/FeatureContext.php | 2 +- tests/integration/features/chat/one-to-one.feature | 9 +++------ tests/integration/features/conversation/avatar.feature | 3 +-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 924dac174c..33235ba3ba 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -1197,7 +1197,7 @@ class FeatureContext implements Context, SnippetAcceptingContext { $this->assertStatusCode($this->response, $statusCode); if ($formData instanceof TableNode) { - $xpectedAttributes = $formData->getColumnsHash()[0]; + $xpectedAttributes = $formData->getRowsHash(); $actual = $this->getDataFromResponse($this->response); foreach ($xpectedAttributes as $attribute => $expectedValue) { if ($expectedValue === 'NOT_EMPTY') { diff --git a/tests/integration/features/chat/one-to-one.feature b/tests/integration/features/chat/one-to-one.feature index 2d1782d897..ef3baf12a7 100644 --- a/tests/integration/features/chat/one-to-one.feature +++ b/tests/integration/features/chat/one-to-one.feature @@ -74,13 +74,10 @@ Feature: chat/one-to-one | invite | participant2 | When user "participant2" set status to "online" with 200 (v1) Then user "participant1" gets room "one-to-one room" with 200 (v4) - | status | - | online | + | status | online | When user "participant2" set status to "offline" with 200 (v1) Then user "participant1" gets room "one-to-one room" with 200 (v4) - | status | - | offline | + | status | offline | Then user "participant2" set status to "away" with 200 (v1) Then user "participant1" gets room "one-to-one room" with 200 (v4) - | status | - | away | + | status | away | diff --git a/tests/integration/features/conversation/avatar.feature b/tests/integration/features/conversation/avatar.feature index 605eecbff0..4b52e32bc0 100644 --- a/tests/integration/features/conversation/avatar.feature +++ b/tests/integration/features/conversation/avatar.feature @@ -30,8 +30,7 @@ Feature: conversation/avatar | room2 | users | participant1 | avatar_set | You set the conversation picture | | room2 | users | participant1 | conversation_created | You created the conversation | And user "participant1" gets room "room2" with 200 (v4) - | avatarVersion | - | NOT_EMPTY | + | avatarVersion | NOT_EMPTY | Then the room "room2" has an avatar with 200 And The following headers should be set | X-NC-IsCustomAvatar | 0 | From 7f50f59e0ca7996001d9c330315c2f8ad5712aa8 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 08:03:56 -0300 Subject: [PATCH 04/19] Reduce IO When we retrieve the room avatar sometimes is necessary to check if the file exists and in other places we only need to get the avatar file name. With this refactor I separated the logic to get the path of file in a different method. Signed-off-by: Vitor Mattos --- lib/Service/AvatarService.php | 91 +++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 37 deletions(-) diff --git a/lib/Service/AvatarService.php b/lib/Service/AvatarService.php index 2a05137612..aada07c227 100644 --- a/lib/Service/AvatarService.php +++ b/lib/Service/AvatarService.php @@ -164,50 +164,41 @@ class AvatarService { try { $folder = $this->appData->getFolder('room-avatar'); if ($folder->fileExists($token)) { - $file = $folder->getFolder($token)->getFile($avatar); + return $folder->getFolder($token)->getFile($avatar); } } catch (NotFoundException $e) { } } // Fallback - if (!isset($file)) { - $colorTone = $darkTheme ? 'dark' : 'bright'; - - if ($room->getType() === Room::TYPE_ONE_TO_ONE) { - $users = json_decode($room->getName(), true); - foreach ($users as $participantId) { - if ($user instanceof IUser && $participantId !== $user->getUID()) { - $avatar = $this->avatarManager->getAvatar($participantId); - $file = $avatar->getFile(512, $darkTheme); - } + if ($room->getType() === Room::TYPE_ONE_TO_ONE) { + $users = json_decode($room->getName(), true); + foreach ($users as $participantId) { + if ($user instanceof IUser && $participantId !== $user->getUID()) { + $avatar = $this->avatarManager->getAvatar($participantId); + return $avatar->getFile(512, $darkTheme); } - } elseif ($this->emojiHelper->isValidSingleEmoji(mb_substr($room->getName(), 0, 1))) { - $file = new InMemoryFile($token, $this->getEmojiAvatar($room->getName(), $darkTheme)); - } elseif ($room->getType() === Room::TYPE_CHANGELOG) { - $fileName = 'changelog.svg'; - $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); - } elseif ($room->getObjectType() === 'file') { - $fileName = 'icon-conversation-text-' . $colorTone . '.svg'; - $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); - } elseif ($room->getObjectType() === 'share:password') { - $fileName = 'icon-conversation-password-' . $colorTone . '.svg'; - $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); - } elseif ($room->getObjectType() === 'emails') { - $fileName = 'icon-conversation-mail-' . $colorTone . '.svg'; - $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); - } elseif ($room->getType() === Room::TYPE_PUBLIC) { - $fileName = 'icon-conversation-public-' . $colorTone . '.svg'; - $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); - } elseif ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) { - $fileName = 'icon-conversation-user-' . $colorTone . '.svg'; - $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); - } else { - $fileName = 'icon-conversation-group-' . $colorTone . '.svg'; - $file = new InMemoryFile($fileName, file_get_contents(__DIR__ . '/../../img/' . $fileName)); } } - return $file; + if ($room->getType() === Room::TYPE_CHANGELOG) { + return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); + } + if ($room->getObjectType() === 'file') { + return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); + } + if ($room->getObjectType() === 'share:password') { + return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); + } + if ($room->getObjectType() === 'emails') { + return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); + } + if ($room->getType() === Room::TYPE_PUBLIC) { + return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); + } + if ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) { + return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); + } + return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); } protected function getEmojiAvatar(string $roomName, bool $darkTheme): string { @@ -257,6 +248,32 @@ class AvatarService { || $this->getFirstCombinedEmoji($room->getName()); } + private function getAvatarPath(Room $room, bool $darkTheme = false): string { + $colorTone = $darkTheme ? 'dark' : 'bright'; + if ($room->getType() === Room::TYPE_ONE_TO_ONE) { + return ''; + } + if ($room->getType() === Room::TYPE_CHANGELOG) { + return __DIR__ . '/../../img/changelog.svg'; + } + if ($room->getObjectType() === 'file') { + return __DIR__ . '/../../img/icon-conversation-text-' . $colorTone . '.svg'; + } + if ($room->getObjectType() === 'share:password') { + return __DIR__ . '/../../img/icon-conversation-password-' . $colorTone . '.svg'; + } + if ($room->getObjectType() === 'emails') { + return __DIR__ . '/../../img/icon-conversation-mail-' . $colorTone . '.svg'; + } + if ($room->getType() === Room::TYPE_PUBLIC) { + return __DIR__ . '/../../img/icon-conversation-public-' . $colorTone . '.svg'; + } + if ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) { + return __DIR__ . '/../../img/icon-conversation-user-' . $colorTone . '.svg'; + } + return __DIR__ . '/../../img/icon-conversation-group-' . $colorTone . '.svg'; + } + public function deleteAvatar(Room $room): void { try { $folder = $this->appData->getFolder('room-avatar'); @@ -286,7 +303,7 @@ class AvatarService { [$version] = explode('.', $avatarVersion); return $version; } - $file = $this->getAvatar($room, null); - return substr(md5($file->getName()), 0, 8); + $avatarPath = $this->getAvatarPath($room); + return substr(md5($avatarPath), 0, 8); } } From 7a0ac60c920d7138a97e2a2c015fe52801dff0e2 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 09:33:34 -0300 Subject: [PATCH 05/19] Replace the header X-NC-IsCustomAvatar by conversation property isCustomAvatar With the header isn't possible to check if `isCustomAvatar` without retrieve the avatar to verify the header. Because this, isCUstomAvatar was moved to a conversation property. Signed-off-by: Vitor Mattos --- docs/avatar.md | 8 ++------ docs/conversation.md | 1 + lib/Controller/AvatarController.php | 1 - lib/Service/RoomFormatter.php | 1 + .../features/conversation/avatar.feature | 17 +++++++++++++---- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/docs/avatar.md b/docs/avatar.md index 8b748e38fd..d2288aeb19 100644 --- a/docs/avatar.md +++ b/docs/avatar.md @@ -24,6 +24,8 @@ ## Delete conversations avatar +> **PS**: To check if need to delete when is customized, you need to verify the property `isCustomAvatar` from [conversation](conversation.md). + * Required capability: `avatar` * Method: `DELETE` * Endpoint: `/room/{token}/avatar` @@ -46,12 +48,6 @@ - Status code: + `200 OK` + `404 Not Found` When the conversation could not be found for the participant - - Header: - -| field | type | Description | -| ------------------- | ------ | ----------------------------------------------------------------- | -| X-NC-IsCustomAvatar | string | Return 1 when is custom avatar and 0 when is not a custom a vatar | - - Body: the image file ## Get dark mode conversations avatar (binary) diff --git a/docs/conversation.md b/docs/conversation.md index 61369ae624..6e64db5914 100644 --- a/docs/conversation.md +++ b/docs/conversation.md @@ -104,6 +104,7 @@ | `participants` | array | v1 | v2 | **Removed** | | `guestList` | string | v1 | v2 | **Removed** | | `avatarVersion` | string | v4 | | Version of conversation avatar used to easier expiration of the avatar in case a moderator updates it, since the avatar endpoint should be cached for 24 hours. | +| `isCustomAvatar` | bool | v4 | | Flag if the conversation have a custom avatar | | `callStartTime` | int | v4 | | Timestamp when the call was started (only available with `recording-v1` capability) | | `callRecording` | int | v4 | | Type of call recording (see [Constants - Call recording status](constants.md#call-recording-status)) (only available with `recording-v1` capability) | diff --git a/lib/Controller/AvatarController.php b/lib/Controller/AvatarController.php index 876f0dc22b..7edebf977d 100644 --- a/lib/Controller/AvatarController.php +++ b/lib/Controller/AvatarController.php @@ -86,7 +86,6 @@ class AvatarController extends AEnvironmentAwareController { $response = new FileDisplayResponse($file); $response->addHeader('Content-Type', $file->getMimeType()); - $response->addHeader('X-NC-IsCustomAvatar', $this->avatarService->isCustomAvatar($this->getRoom()) ? '1' : '0'); // Cache for 1 day $response->cacheFor(60 * 60 * 24, false, true); return $response; diff --git a/lib/Service/RoomFormatter.php b/lib/Service/RoomFormatter.php index 1b8f4ecda1..741b6f0c2a 100644 --- a/lib/Service/RoomFormatter.php +++ b/lib/Service/RoomFormatter.php @@ -136,6 +136,7 @@ class RoomFormatter { 'callFlag' => Participant::FLAG_DISCONNECTED, 'messageExpiration' => 0, 'avatarVersion' => $this->avatarService->getAvatarVersion($room), + 'isCustomAvatar' => $room->getAvatar() ? true : false, 'breakoutRoomMode' => BreakoutRoom::MODE_NOT_CONFIGURED, 'breakoutRoomStatus' => BreakoutRoom::STATUS_STOPPED, ]; diff --git a/tests/integration/features/conversation/avatar.feature b/tests/integration/features/conversation/avatar.feature index 4b52e32bc0..91219eec8b 100644 --- a/tests/integration/features/conversation/avatar.feature +++ b/tests/integration/features/conversation/avatar.feature @@ -16,9 +16,10 @@ Feature: conversation/avatar | roomType | 3 | | roomName | room2 | When user "participant1" uploads file "/img/favicon.png" as avatar of room "room2" with 200 + And user "participant1" gets room "room2" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 1 | Then the room "room2" has an avatar with 200 - And The following headers should be set - | X-NC-IsCustomAvatar | 1 | And user "participant1" sees the following system messages in room "room2" with 200 | room | actorType | actorId | systemMessage | message | | room2 | users | participant1 | avatar_set | You set the conversation picture | @@ -31,27 +32,35 @@ Feature: conversation/avatar | room2 | users | participant1 | conversation_created | You created the conversation | And user "participant1" gets room "room2" with 200 (v4) | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 0 | Then the room "room2" has an avatar with 200 - And The following headers should be set - | X-NC-IsCustomAvatar | 0 | Scenario: Get avatar of conversation without custom avatar (fallback) Given user "participant1" creates room "room3" (v4) | roomType | 3 | | roomName | room3 | Then the room "room3" has an avatar with 200 + And user "participant1" gets room "room3" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 0 | Scenario: Get avatar of one2one without custom avatar (fallback) When user "participant1" creates room "one2one" (v4) | roomType | 1 | | invite | participant2 | Then the room "one2one" has an avatar with 200 + And user "participant1" gets room "one2one" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 0 | Scenario: Try to change avatar of one2one without success When user "participant1" creates room "one2one" (v4) | roomType | 1 | | invite | participant2 | Then user "participant1" uploads file "/img/favicon.png" as avatar of room "one2one" with 400 + And user "participant1" gets room "one2one" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 0 | Scenario: User should receive the room avatar when see a rich object at media tab Given user "participant1" creates room "public room" (v4) From 92fe810ca26105b1b4dbfb89fdb4ccf4cf086794 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 09:46:53 -0300 Subject: [PATCH 06/19] Remove unused step definition Signed-off-by: Vitor Mattos --- .../features/bootstrap/FeatureContext.php | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 33235ba3ba..541e940d68 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -3490,26 +3490,4 @@ class FeatureContext implements Context, SnippetAcceptingContext { } } - /** - * @Then The following headers should be set - * @param TableNode $table - * @throws \Exception - */ - public function theFollowingHeadersShouldBeSet(TableNode $table) { - foreach ($table->getTable() as $header) { - $headerName = $header[0]; - $expectedHeaderValue = $header[1]; - $returnedHeader = $this->response->getHeader($headerName)[0]; - if ($returnedHeader !== $expectedHeaderValue) { - throw new \Exception( - sprintf( - "Expected value '%s' for header '%s', got '%s'", - $expectedHeaderValue, - $headerName, - $returnedHeader - ) - ); - } - } - } } From d102271134026c75eaa0f10f22ef1587e0fd7b51 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 09:52:18 -0300 Subject: [PATCH 07/19] Simplify code after refactor Signed-off-by: Vitor Mattos --- lib/Service/AvatarService.php | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/lib/Service/AvatarService.php b/lib/Service/AvatarService.php index aada07c227..b10b4ceb75 100644 --- a/lib/Service/AvatarService.php +++ b/lib/Service/AvatarService.php @@ -180,24 +180,6 @@ class AvatarService { } } } - if ($room->getType() === Room::TYPE_CHANGELOG) { - return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); - } - if ($room->getObjectType() === 'file') { - return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); - } - if ($room->getObjectType() === 'share:password') { - return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); - } - if ($room->getObjectType() === 'emails') { - return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); - } - if ($room->getType() === Room::TYPE_PUBLIC) { - return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); - } - if ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) { - return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); - } return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); } From 28348080453aac27cb57df7bd4015054178f034d Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 12:49:12 -0300 Subject: [PATCH 08/19] Change text after code review https://github.com/nextcloud/spreed/pull/9423#discussion_r1182704162 Signed-off-by: Vitor Mattos --- docs/avatar.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/avatar.md b/docs/avatar.md index d2288aeb19..a64428e2f8 100644 --- a/docs/avatar.md +++ b/docs/avatar.md @@ -24,7 +24,7 @@ ## Delete conversations avatar -> **PS**: To check if need to delete when is customized, you need to verify the property `isCustomAvatar` from [conversation](conversation.md). +> **Note**: To determine if a delete option should be presented to the user, it's recommended to check the `isCustomAvatar` property from [conversation](conversation.md). * Required capability: `avatar` * Method: `DELETE` From bd40421b1ce0be33e16a8644906445dcf25a3d28 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 13:12:13 -0300 Subject: [PATCH 09/19] Prevent future problems with empty return Signed-off-by: Vitor Mattos --- lib/Service/AvatarService.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/Service/AvatarService.php b/lib/Service/AvatarService.php index b10b4ceb75..dbbaa9c6d5 100644 --- a/lib/Service/AvatarService.php +++ b/lib/Service/AvatarService.php @@ -232,9 +232,6 @@ class AvatarService { private function getAvatarPath(Room $room, bool $darkTheme = false): string { $colorTone = $darkTheme ? 'dark' : 'bright'; - if ($room->getType() === Room::TYPE_ONE_TO_ONE) { - return ''; - } if ($room->getType() === Room::TYPE_CHANGELOG) { return __DIR__ . '/../../img/changelog.svg'; } @@ -250,7 +247,9 @@ class AvatarService { if ($room->getType() === Room::TYPE_PUBLIC) { return __DIR__ . '/../../img/icon-conversation-public-' . $colorTone . '.svg'; } - if ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) { + if ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER + || $room->getType() === Room::TYPE_ONE_TO_ONE + ) { return __DIR__ . '/../../img/icon-conversation-user-' . $colorTone . '.svg'; } return __DIR__ . '/../../img/icon-conversation-group-' . $colorTone . '.svg'; From 030fdbfd544d40c7f6268e98bfacf1197ebc2bc1 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 2 May 2023 13:20:48 -0300 Subject: [PATCH 10/19] Changes after code reivew https://github.com/nextcloud/spreed/pull/9423#discussion_r1182750772 https://github.com/nextcloud/spreed/pull/9423#discussion_r1182758481 Signed-off-by: Vitor Mattos --- docs/avatar.md | 3 ++- docs/conversation.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/avatar.md b/docs/avatar.md index a64428e2f8..1cffbf2a22 100644 --- a/docs/avatar.md +++ b/docs/avatar.md @@ -24,7 +24,8 @@ ## Delete conversations avatar -> **Note**: To determine if a delete option should be presented to the user, it's recommended to check the `isCustomAvatar` property from [conversation](conversation.md). +!!! note + To determine if a delete option should be presented to the user, it's recommended to check the `isCustomAvatar` property from [conversation](conversation.md). * Required capability: `avatar` * Method: `DELETE` diff --git a/docs/conversation.md b/docs/conversation.md index 6e64db5914..a4031a4b4d 100644 --- a/docs/conversation.md +++ b/docs/conversation.md @@ -104,7 +104,7 @@ | `participants` | array | v1 | v2 | **Removed** | | `guestList` | string | v1 | v2 | **Removed** | | `avatarVersion` | string | v4 | | Version of conversation avatar used to easier expiration of the avatar in case a moderator updates it, since the avatar endpoint should be cached for 24 hours. | -| `isCustomAvatar` | bool | v4 | | Flag if the conversation have a custom avatar | +| `isCustomAvatar` | bool | v4 | | Flag if the conversation has a custom avatar | | `callStartTime` | int | v4 | | Timestamp when the call was started (only available with `recording-v1` capability) | | `callRecording` | int | v4 | | Type of call recording (see [Constants - Call recording status](constants.md#call-recording-status)) (only available with `recording-v1` capability) | From 21bc57c6492dda5712dd7f30877403b358ab8643 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Wed, 3 May 2023 04:50:14 -0300 Subject: [PATCH 11/19] Text fixes after code review https://github.com/nextcloud/spreed/pull/9423#discussion_r1182886226 https://github.com/nextcloud/spreed/pull/9423#discussion_r1182886827 Signed-off-by: Vitor Mattos --- docs/avatar.md | 3 ++- docs/conversation.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/avatar.md b/docs/avatar.md index 1cffbf2a22..891cbe7178 100644 --- a/docs/avatar.md +++ b/docs/avatar.md @@ -25,7 +25,8 @@ ## Delete conversations avatar !!! note - To determine if a delete option should be presented to the user, it's recommended to check the `isCustomAvatar` property from [conversation](conversation.md). + To determine if the delete option should be presented to the user, it's recommended to check the `isCustomAvatar` property of the [Get user´s conversations](conversation.md#get-user-s-conversations) API. + * Required capability: `avatar` * Method: `DELETE` diff --git a/docs/conversation.md b/docs/conversation.md index a4031a4b4d..b7e172b49e 100644 --- a/docs/conversation.md +++ b/docs/conversation.md @@ -104,7 +104,7 @@ | `participants` | array | v1 | v2 | **Removed** | | `guestList` | string | v1 | v2 | **Removed** | | `avatarVersion` | string | v4 | | Version of conversation avatar used to easier expiration of the avatar in case a moderator updates it, since the avatar endpoint should be cached for 24 hours. | -| `isCustomAvatar` | bool | v4 | | Flag if the conversation has a custom avatar | +| `isCustomAvatar` | bool | v4 | | Flag if the conversation has a custom avatar (only available with `avatar` capability) | | `callStartTime` | int | v4 | | Timestamp when the call was started (only available with `recording-v1` capability) | | `callRecording` | int | v4 | | Type of call recording (see [Constants - Call recording status](constants.md#call-recording-status)) (only available with `recording-v1` capability) | From 0ce23663e3aa5870b5cd0cd2db4e012dc811d40b Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Wed, 3 May 2023 06:26:40 -0300 Subject: [PATCH 12/19] Return isCustomAvatar=true when the name of room start with an emoji Signed-off-by: Vitor Mattos --- lib/Service/RoomFormatter.php | 2 +- .../features/conversation/avatar.feature | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/Service/RoomFormatter.php b/lib/Service/RoomFormatter.php index 741b6f0c2a..f0b8341314 100644 --- a/lib/Service/RoomFormatter.php +++ b/lib/Service/RoomFormatter.php @@ -136,7 +136,7 @@ class RoomFormatter { 'callFlag' => Participant::FLAG_DISCONNECTED, 'messageExpiration' => 0, 'avatarVersion' => $this->avatarService->getAvatarVersion($room), - 'isCustomAvatar' => $room->getAvatar() ? true : false, + 'isCustomAvatar' => $this->avatarService->isCustomAvatar($room), 'breakoutRoomMode' => BreakoutRoom::MODE_NOT_CONFIGURED, 'breakoutRoomStatus' => BreakoutRoom::STATUS_STOPPED, ]; diff --git a/tests/integration/features/conversation/avatar.feature b/tests/integration/features/conversation/avatar.feature index 91219eec8b..27e56f3545 100644 --- a/tests/integration/features/conversation/avatar.feature +++ b/tests/integration/features/conversation/avatar.feature @@ -62,6 +62,32 @@ Feature: conversation/avatar | avatarVersion | NOT_EMPTY | | isCustomAvatar | 0 | + Scenario: Conversation that the name start with emoji need to have custom avatar + Given user "participant1" creates room "room1" (v4) + | roomType | 3 | + | roomName | room1 | + And user "participant1" gets room "room1" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 0 | + | displayName | room1 | + And user "participant1" renames room "room1" to "room2" with 200 (v4) + And user "participant1" gets room "room1" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 0 | + | displayName | room2 | + Then the room "room1" has an avatar with 200 + And user "participant1" renames room "room1" to "💙room2" with 200 (v4) + And user "participant1" gets room "room1" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 1 | + | displayName | 💙room2 | + Then the room "room1" has an avatar with 200 + And user "participant1" renames room "room1" to "room1" with 200 (v4) + And user "participant1" gets room "room1" with 200 (v4) + | avatarVersion | NOT_EMPTY | + | isCustomAvatar | 0 | + | displayName | room1 | + Scenario: User should receive the room avatar when see a rich object at media tab Given user "participant1" creates room "public room" (v4) | roomType | 3 | From ee52c47523ade345991e71b730f437cc3ba5abad Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Wed, 3 May 2023 06:56:43 -0300 Subject: [PATCH 13/19] Return isCustomAvatar=false when the name of room start with an avatar Didn't make sense to return true because this property is only used to display the trash icon to remove the customized avatar. Signed-off-by: Vitor Mattos --- lib/Service/AvatarService.php | 3 +-- tests/integration/features/conversation/avatar.feature | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/Service/AvatarService.php b/lib/Service/AvatarService.php index dbbaa9c6d5..0769a1ad00 100644 --- a/lib/Service/AvatarService.php +++ b/lib/Service/AvatarService.php @@ -226,8 +226,7 @@ class AvatarService { } public function isCustomAvatar(Room $room): bool { - return $room->getAvatar() !== '' - || $this->getFirstCombinedEmoji($room->getName()); + return $room->getAvatar() !== ''; } private function getAvatarPath(Room $room, bool $darkTheme = false): string { diff --git a/tests/integration/features/conversation/avatar.feature b/tests/integration/features/conversation/avatar.feature index 27e56f3545..861f75795c 100644 --- a/tests/integration/features/conversation/avatar.feature +++ b/tests/integration/features/conversation/avatar.feature @@ -62,7 +62,7 @@ Feature: conversation/avatar | avatarVersion | NOT_EMPTY | | isCustomAvatar | 0 | - Scenario: Conversation that the name start with emoji need to have custom avatar + Scenario: Conversation that the name start with emoji dont need to have custom avatar Given user "participant1" creates room "room1" (v4) | roomType | 3 | | roomName | room1 | @@ -79,7 +79,7 @@ Feature: conversation/avatar And user "participant1" renames room "room1" to "💙room2" with 200 (v4) And user "participant1" gets room "room1" with 200 (v4) | avatarVersion | NOT_EMPTY | - | isCustomAvatar | 1 | + | isCustomAvatar | 0 | | displayName | 💙room2 | Then the room "room1" has an avatar with 200 And user "participant1" renames room "room1" to "room1" with 200 (v4) From c92f66c01d7a1d522e3f4a6ff69ec9955fee7322 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Wed, 3 May 2023 09:06:11 -0300 Subject: [PATCH 14/19] Cover with integration tests the room avatar with emoji Signed-off-by: Vitor Mattos --- lib/Service/AvatarService.php | 10 ++++- .../features/bootstrap/FeatureContext.php | 40 +++++++++++++++++++ .../features/conversation/avatar.feature | 25 +++++------- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/lib/Service/AvatarService.php b/lib/Service/AvatarService.php index 0769a1ad00..b6a727dc43 100644 --- a/lib/Service/AvatarService.php +++ b/lib/Service/AvatarService.php @@ -180,10 +180,13 @@ class AvatarService { } } } - return new InMemoryFile($token, $this->getAvatarPath($room, $darkTheme)); + if ($this->emojiHelper->isValidSingleEmoji(mb_substr($room->getName(), 0, 1))) { + return new InMemoryFile($token, $this->getEmojiAvatar($room->getName(), $darkTheme)); + } + return new InMemoryFile($token, file_get_contents($this->getAvatarPath($room, $darkTheme))); } - protected function getEmojiAvatar(string $roomName, bool $darkTheme): string { + protected function getEmojiAvatar(string $roomName, bool $darkTheme = false): string { return str_replace([ '{letter}', '{fill}', @@ -283,6 +286,9 @@ class AvatarService { [$version] = explode('.', $avatarVersion); return $version; } + if ($this->emojiHelper->isValidSingleEmoji(mb_substr($room->getName(), 0, 1))) { + return substr(md5($this->getEmojiAvatar($room->getName())), 0, 8); + } $avatarPath = $this->getAvatarPath($room); return substr(md5($avatarPath), 0, 8); } diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 541e940d68..71084f8fb3 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -3231,6 +3231,46 @@ class FeatureContext implements Context, SnippetAcceptingContext { $this->assertStatusCode($this->response, $statusCode); } + /** + * @When /^the room "([^"]*)" has an svg as avatar with (\d+)(?: \((v1)\))?$/ + */ + public function theRoomNeedToHaveAnAsvAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1'): void { + $this->theRoomNeedToHavetAnAsvAvatarWithStatusCode($identifier, $statusCode, $apiVersion, true); + } + + /** + * @When /^the room "([^"]*)" has not an svg as avatar with (\d+)(?: \((v1)\))?$/ + */ + public function theRoomNeedToHavetAnAsvAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1', bool $expectedToBeSvg = false): void { + $this->theRoomNeedToHaveAnAvatarWithStatusCode($identifier, $statusCode, $apiVersion); + $content = $this->response->getBody()->getContents(); + try { + simplexml_load_string($content); + $actualIsSvg = true; + } catch (\Throwable $th) { + $actualIsSvg = false; + } + if ($expectedToBeSvg) { + Assert::assertEquals($expectedToBeSvg, $actualIsSvg, 'The room avatar need to be a XML file'); + } else { + Assert::assertEquals($expectedToBeSvg, $actualIsSvg, 'The room avatar can not be a XML file'); + } + } + + /** + * @When /^the avatar svg of room "([^"]*)" contais the string "([^"]*)"(?: \((v1)\))?$/ + */ + public function theAvatarSvgOfRoomContainsTheString(string $identifier, string $string, string $apiVersion = 'v1'): void { + $this->sendRequest('GET', '/apps/spreed/api/' . $apiVersion . '/room/' . self::$identifierToToken[$identifier] . '/avatar'); + $content = $this->response->getBody()->getContents(); + try { + simplexml_load_string($content); + } catch (\Throwable $th) { + throw new Exception('The avatar need to be a XML'); + } + Assert::stringContains($content, $string); + } + /** * @When /^user "([^"]*)" delete the avatar of room "([^"]*)" with (\d+)(?: \((v1)\))?$/ */ diff --git a/tests/integration/features/conversation/avatar.feature b/tests/integration/features/conversation/avatar.feature index 861f75795c..b327af66c7 100644 --- a/tests/integration/features/conversation/avatar.feature +++ b/tests/integration/features/conversation/avatar.feature @@ -16,16 +16,16 @@ Feature: conversation/avatar | roomType | 3 | | roomName | room2 | When user "participant1" uploads file "/img/favicon.png" as avatar of room "room2" with 200 - And user "participant1" gets room "room2" with 200 (v4) + Then user "participant1" gets room "room2" with 200 (v4) | avatarVersion | NOT_EMPTY | | isCustomAvatar | 1 | - Then the room "room2" has an avatar with 200 + And the room "room2" has not an svg as avatar with 200 And user "participant1" sees the following system messages in room "room2" with 200 | room | actorType | actorId | systemMessage | message | | room2 | users | participant1 | avatar_set | You set the conversation picture | | room2 | users | participant1 | conversation_created | You created the conversation | - And user "participant1" delete the avatar of room "room2" with 200 - And user "participant1" sees the following system messages in room "room2" with 200 + When user "participant1" delete the avatar of room "room2" with 200 + Then user "participant1" sees the following system messages in room "room2" with 200 | room | actorType | actorId | systemMessage | message | | room2 | users | participant1 | avatar_removed | You removed the conversation picture | | room2 | users | participant1 | avatar_set | You set the conversation picture | @@ -66,27 +66,24 @@ Feature: conversation/avatar Given user "participant1" creates room "room1" (v4) | roomType | 3 | | roomName | room1 | + And the room "room1" has an svg as avatar with 200 And user "participant1" gets room "room1" with 200 (v4) | avatarVersion | NOT_EMPTY | | isCustomAvatar | 0 | | displayName | room1 | - And user "participant1" renames room "room1" to "room2" with 200 (v4) - And user "participant1" gets room "room1" with 200 (v4) - | avatarVersion | NOT_EMPTY | - | isCustomAvatar | 0 | - | displayName | room2 | - Then the room "room1" has an avatar with 200 And user "participant1" renames room "room1" to "💙room2" with 200 (v4) - And user "participant1" gets room "room1" with 200 (v4) + Then user "participant1" gets room "room1" with 200 (v4) | avatarVersion | NOT_EMPTY | | isCustomAvatar | 0 | | displayName | 💙room2 | - Then the room "room1" has an avatar with 200 - And user "participant1" renames room "room1" to "room1" with 200 (v4) - And user "participant1" gets room "room1" with 200 (v4) + And the room "room1" has an svg as avatar with 200 + And the avatar svg of room "room1" contais the string "💙" + When user "participant1" renames room "room1" to "room1" with 200 (v4) + Then user "participant1" gets room "room1" with 200 (v4) | avatarVersion | NOT_EMPTY | | isCustomAvatar | 0 | | displayName | room1 | + And the room "room1" has an svg as avatar with 200 Scenario: User should receive the room avatar when see a rich object at media tab Given user "participant1" creates room "public room" (v4) From 895730434debccd5e0a3dec0d1926b95023c098a Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Wed, 3 May 2023 09:46:34 -0300 Subject: [PATCH 15/19] Fix typo https://github.com/nextcloud/spreed/pull/9423#discussion_r1183635637 Signed-off-by: Vitor Mattos --- tests/integration/features/bootstrap/FeatureContext.php | 2 +- tests/integration/features/conversation/avatar.feature | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 71084f8fb3..7b5aab4a19 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -3258,7 +3258,7 @@ class FeatureContext implements Context, SnippetAcceptingContext { } /** - * @When /^the avatar svg of room "([^"]*)" contais the string "([^"]*)"(?: \((v1)\))?$/ + * @When /^the avatar svg of room "([^"]*)" contains the string "([^"]*)"(?: \((v1)\))?$/ */ public function theAvatarSvgOfRoomContainsTheString(string $identifier, string $string, string $apiVersion = 'v1'): void { $this->sendRequest('GET', '/apps/spreed/api/' . $apiVersion . '/room/' . self::$identifierToToken[$identifier] . '/avatar'); diff --git a/tests/integration/features/conversation/avatar.feature b/tests/integration/features/conversation/avatar.feature index b327af66c7..81030f2795 100644 --- a/tests/integration/features/conversation/avatar.feature +++ b/tests/integration/features/conversation/avatar.feature @@ -77,7 +77,7 @@ Feature: conversation/avatar | isCustomAvatar | 0 | | displayName | 💙room2 | And the room "room1" has an svg as avatar with 200 - And the avatar svg of room "room1" contais the string "💙" + And the avatar svg of room "room1" contains the string "💙" When user "participant1" renames room "room1" to "room1" with 200 (v4) Then user "participant1" gets room "room1" with 200 (v4) | avatarVersion | NOT_EMPTY | From 9e4753354e73ee05b525e2a27afdb2df9bf1deea Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Wed, 3 May 2023 09:50:46 -0300 Subject: [PATCH 16/19] Add capability requrement https://github.com/nextcloud/spreed/pull/9423#discussion_r1183640175 Signed-off-by: Vitor Mattos --- docs/conversation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conversation.md b/docs/conversation.md index b7e172b49e..3bfdf3dcb3 100644 --- a/docs/conversation.md +++ b/docs/conversation.md @@ -103,7 +103,7 @@ | `statusClearAt` | ?int | v4 | | Optional: Only available for one-to-one conversations, when `includeStatus=true` is set and the user has a status, can still be null even with a status | | `participants` | array | v1 | v2 | **Removed** | | `guestList` | string | v1 | v2 | **Removed** | -| `avatarVersion` | string | v4 | | Version of conversation avatar used to easier expiration of the avatar in case a moderator updates it, since the avatar endpoint should be cached for 24 hours. | +| `avatarVersion` | string | v4 | | Version of conversation avatar used to easier expiration of the avatar in case a moderator updates it, since the avatar endpoint should be cached for 24 hours. (only available with `avatar` capability) | | `isCustomAvatar` | bool | v4 | | Flag if the conversation has a custom avatar (only available with `avatar` capability) | | `callStartTime` | int | v4 | | Timestamp when the call was started (only available with `recording-v1` capability) | | `callRecording` | int | v4 | | Type of call recording (see [Constants - Call recording status](constants.md#call-recording-status)) (only available with `recording-v1` capability) | From 6659f1b1e1f642ce116dfc3ad5170c0d1b4e5a6f Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Wed, 3 May 2023 10:17:22 -0300 Subject: [PATCH 17/19] Fix texts Signed-off-by: Vitor Mattos --- .../integration/features/bootstrap/FeatureContext.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 7b5aab4a19..23c3fb826c 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -3234,14 +3234,14 @@ class FeatureContext implements Context, SnippetAcceptingContext { /** * @When /^the room "([^"]*)" has an svg as avatar with (\d+)(?: \((v1)\))?$/ */ - public function theRoomNeedToHaveAnAsvAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1'): void { - $this->theRoomNeedToHavetAnAsvAvatarWithStatusCode($identifier, $statusCode, $apiVersion, true); + public function theRoomNeedsToHaveASvgAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1'): void { + $this->theRoomNeedsToHaveASvgAvatarWithStatusCode($identifier, $statusCode, $apiVersion, true); } /** * @When /^the room "([^"]*)" has not an svg as avatar with (\d+)(?: \((v1)\))?$/ */ - public function theRoomNeedToHavetAnAsvAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1', bool $expectedToBeSvg = false): void { + public function theRoomNeedsToHaveASvgAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1', bool $expectedToBeSvg = false): void { $this->theRoomNeedToHaveAnAvatarWithStatusCode($identifier, $statusCode, $apiVersion); $content = $this->response->getBody()->getContents(); try { @@ -3251,7 +3251,7 @@ class FeatureContext implements Context, SnippetAcceptingContext { $actualIsSvg = false; } if ($expectedToBeSvg) { - Assert::assertEquals($expectedToBeSvg, $actualIsSvg, 'The room avatar need to be a XML file'); + Assert::assertEquals($expectedToBeSvg, $actualIsSvg, 'The room avatar needs to be a XML file'); } else { Assert::assertEquals($expectedToBeSvg, $actualIsSvg, 'The room avatar can not be a XML file'); } @@ -3266,7 +3266,7 @@ class FeatureContext implements Context, SnippetAcceptingContext { try { simplexml_load_string($content); } catch (\Throwable $th) { - throw new Exception('The avatar need to be a XML'); + throw new Exception('The avatar needs to be a XML'); } Assert::stringContains($content, $string); } From 8bc090611b60f333c76a181ee5b335f3e618a210 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 3 May 2023 22:38:36 +0200 Subject: [PATCH 18/19] fix(tests): Adjust function names to be unique Signed-off-by: Joas Schilling --- .../integration/features/bootstrap/FeatureContext.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 23c3fb826c..d4963e47f0 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -3226,7 +3226,7 @@ class FeatureContext implements Context, SnippetAcceptingContext { /** * @When /^the room "([^"]*)" has an avatar with (\d+)(?: \((v1)\))?$/ */ - public function theRoomNeedToHaveAnAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1'): void { + public function theRoomHasAnAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1'): void { $this->sendRequest('GET', '/apps/spreed/api/' . $apiVersion . '/room/' . self::$identifierToToken[$identifier] . '/avatar'); $this->assertStatusCode($this->response, $statusCode); } @@ -3234,15 +3234,15 @@ class FeatureContext implements Context, SnippetAcceptingContext { /** * @When /^the room "([^"]*)" has an svg as avatar with (\d+)(?: \((v1)\))?$/ */ - public function theRoomNeedsToHaveASvgAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1'): void { - $this->theRoomNeedsToHaveASvgAvatarWithStatusCode($identifier, $statusCode, $apiVersion, true); + public function theRoomHasASvgAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1'): void { + $this->theRoomHasNoSvgAvatarWithStatusCode($identifier, $statusCode, $apiVersion, true); } /** * @When /^the room "([^"]*)" has not an svg as avatar with (\d+)(?: \((v1)\))?$/ */ - public function theRoomNeedsToHaveASvgAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1', bool $expectedToBeSvg = false): void { - $this->theRoomNeedToHaveAnAvatarWithStatusCode($identifier, $statusCode, $apiVersion); + public function theRoomHasNoSvgAvatarWithStatusCode(string $identifier, int $statusCode, string $apiVersion = 'v1', bool $expectedToBeSvg = false): void { + $this->theRoomHasAnAvatarWithStatusCode($identifier, $statusCode, $apiVersion); $content = $this->response->getBody()->getContents(); try { simplexml_load_string($content); From 602ff9625547118a6963b132e08e42c030a8a5bc Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Thu, 4 May 2023 04:54:54 -0300 Subject: [PATCH 19/19] Fix integration test I prefered to put a placeholder at URL from step to make splicit that the URL have a version at the place of placeholder. Signed-off-by: Vitor Mattos --- tests/integration/features/bootstrap/FeatureContext.php | 4 ++++ tests/integration/features/integration/dashboard.feature | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index d4963e47f0..27537c1c3b 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -1946,6 +1946,10 @@ class FeatureContext implements Context, SnippetAcceptingContext { $item['iconUrl'] = str_replace('{$BASE_URL}', $this->baseUrl, $item['iconUrl']); $item['iconUrl'] = str_replace('{token}', $token, $item['iconUrl']); + Assert::assertMatchesRegularExpression('/\?v=\w{8}$/', $data[$widgetId][$key]['iconUrl']); + preg_match('/(?\?v=\w{8})$/', $data[$widgetId][$key]['iconUrl'], $matches); + $item['iconUrl'] = str_replace('{version}', $matches['version'], $item['iconUrl']); + Assert::assertEquals($item, $data[$widgetId][$key], 'Wrong details for item #' . $key); } } diff --git a/tests/integration/features/integration/dashboard.feature b/tests/integration/features/integration/dashboard.feature index 2120bbbef5..8730bedd04 100644 --- a/tests/integration/features/integration/dashboard.feature +++ b/tests/integration/features/integration/dashboard.feature @@ -37,6 +37,6 @@ Feature: integration/dashboard And user "participant2" broadcasts message "@participant1 hello" to room "breakout room parent" with 201 (v1) Then user "participant1" sees the following entries for dashboard widgets "spreed" (v1) | title | subtitle | link | iconUrl | sinceId | - | call room | Call in progress | call room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar | | - | group room | You were mentioned | group room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar | | - | participant2-displayname | Hello | one-to-one room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar | | + | call room | Call in progress | call room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar{version} | | + | group room | You were mentioned | group room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar{version} | | + | participant2-displayname | Hello | one-to-one room | {$BASE_URL}ocs/v2.php/apps/spreed/api/v1/room/{token}/avatar{version} | |