Merge pull request #9423 from nextcloud/bugfix/9320/expose-if-is-custom-avatar

Expose if the room has a custom avatar

docs/avatar.md

@@ -24,6 +24,10 @@
 
 ## Delete conversations avatar
 
+!!! note
+    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`
 * Endpoint: `/room/{token}/avatar`

docs/conversation.md

@@ -103,7 +103,8 @@
 | `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)                                                                                                                                                                                                                                              |
 

lib/Service/AvatarService.php

@@ -164,46 +164,29 @@ 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) {
-				$file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/changelog.svg'));
-			} elseif ($room->getObjectType() === 'file') {
-				$file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-text-' . $colorTone . '.svg'));
-			} elseif ($room->getObjectType() === 'share:password') {
-				$file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-password-' . $colorTone . '.svg'));
-			} elseif ($room->getObjectType() === 'emails') {
-				$file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-mail-' . $colorTone . '.svg'));
-			} elseif ($room->getType() === Room::TYPE_PUBLIC) {
-				$file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-public-' . $colorTone . '.svg'));
-			} elseif ($room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) {
-				$file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-user-' . $colorTone . '.svg'));
-			} else {
-				$file = new InMemoryFile($token, file_get_contents(__DIR__ . '/../../img/icon-conversation-group-' . $colorTone . '.svg'));
 			}
 		}
-		return $file;
+		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}',
@@ -245,6 +228,35 @@ class AvatarService {
 		return '';
 	}
 
+	public function isCustomAvatar(Room $room): bool {
+		return $room->getAvatar() !== '';
+	}
+
+	private function getAvatarPath(Room $room, bool $darkTheme = false): string {
+		$colorTone = $darkTheme ? 'dark' : 'bright';
+		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
+			|| $room->getType() === Room::TYPE_ONE_TO_ONE
+		) {
+			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');
@@ -270,7 +282,14 @@ class AvatarService {
 
 	public function getAvatarVersion(Room $room): string {
 		$avatarVersion = $room->getAvatar();
-		[$version] = explode('.', $avatarVersion);
-		return $version;
+		if ($avatarVersion) {
+			[$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);
 	}
 }

lib/Service/RoomFormatter.php

@@ -136,6 +136,7 @@ class RoomFormatter {
 			'callFlag' => Participant::FLAG_DISCONNECTED,
 			'messageExpiration' => 0,
 			'avatarVersion' => $this->avatarService->getAvatarVersion($room),
+			'isCustomAvatar' => $this->avatarService->isCustomAvatar($room),
 			'breakoutRoomMode' => BreakoutRoom::MODE_NOT_CONFIGURED,
 			'breakoutRoomStatus' => BreakoutRoom::STATUS_STOPPED,
 		];

tests/integration/features/bootstrap/FeatureContext.php

@@ -1197,9 +1197,13 @@ 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') {
+					Assert::assertNotEmpty($actual[$attribute]);
+					continue;
+				}
 				Assert::assertEquals($expectedValue, $actual[$attribute]);
 			}
 		}
@@ -1942,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('/(?<version>\?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);
 		}
 	}
@@ -3222,11 +3230,51 @@ 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);
 	}
 
+	/**
+	 * @When /^the room "([^"]*)" has an svg as avatar with (\d+)(?: \((v1)\))?$/
+	 */
+	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 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);
+			$actualIsSvg = true;
+		} catch (\Throwable $th) {
+			$actualIsSvg = false;
+		}
+		if ($expectedToBeSvg) {
+			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');
+		}
+	}
+
+	/**
+	 * @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');
+		$content = $this->response->getBody()->getContents();
+		try {
+			simplexml_load_string($content);
+		} catch (\Throwable $th) {
+			throw new Exception('The avatar needs to be a XML');
+		}
+		Assert::stringContains($content, $string);
+	}
+
 	/**
 	 * @When /^user "([^"]*)" delete the avatar of room "([^"]*)" with (\d+)(?: \((v1)\))?$/
 	 */
@@ -3485,4 +3533,5 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 			Assert::assertEquals($statusCode, $response->getStatusCode(), $message);
 		}
 	}
+
 }

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 |

tests/integration/features/conversation/avatar.feature

@@ -16,35 +16,74 @@ Feature: conversation/avatar
       | roomType | 3 |
       | 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
+    Then user "participant1" gets room "room2" with 200 (v4)
+      | avatarVersion | NOT_EMPTY |
+      | isCustomAvatar | 1 |
+    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     |
       | 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
 
   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: Conversation that the name start with emoji dont need to have custom 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)
+    Then user "participant1" gets room "room1" with 200 (v4)
+      | avatarVersion | NOT_EMPTY |
+      | isCustomAvatar | 0 |
+      | displayName | 💙room2 |
+    And the room "room1" has an svg as avatar with 200
+    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 |
+      | 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)

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} |         |

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'],
 		];