From 93e98343907525d7ada2c956cef082a24ba344a3 Mon Sep 17 00:00:00 2001 From: Martin Date: Wed, 16 Sep 2020 13:24:11 -0300 Subject: [PATCH 1/4] [FIX] Wrong avatar urls when using providers --- client/components/basic/avatar/UserAvatar.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/components/basic/avatar/UserAvatar.js b/client/components/basic/avatar/UserAvatar.js index 17301a4e9e592..99615325f6e93 100644 --- a/client/components/basic/avatar/UserAvatar.js +++ b/client/components/basic/avatar/UserAvatar.js @@ -1,13 +1,14 @@ import React from 'react'; import BaseAvatar from './BaseAvatar'; +import { getUserAvatarURL as getAvatarUrl } from '../../../../app/utils/lib/getUserAvatarURL'; function UserAvatar({ url, username, etag, ...props }) { // NOW, `username` and `etag` props are enough to determine the whole state of // this component, but it must be as performatic as possible as it will be // rendered many times; and some of the state can be derived at the ancestors. // Ideally, it should be a purely visual component. - const avatarUrl = url || `/avatar/${ username }${ etag ? `?etag=${ etag }` : '' }`; + const avatarUrl = getAvatarUrl(username) || url || `/avatar/${ username }${ etag ? `?etag=${ etag }` : '' }`; return ; } From 5b33235b07799eddbd77b5c4889b44453dbc3dee Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 28 Sep 2020 15:16:23 -0300 Subject: [PATCH 2/4] add new setting for room avatar providers --- app/lib/server/startup/settings.js | 5 +++++ app/utils/lib/getRoomAvatarURL.js | 2 +- client/components/basic/avatar/UserAvatar.js | 8 ++++++-- packages/rocketchat-i18n/i18n/en.i18n.json | 2 ++ 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/lib/server/startup/settings.js b/app/lib/server/startup/settings.js index af11aabf4b33e..7d793e5f2c0e6 100644 --- a/app/lib/server/startup/settings.js +++ b/app/lib/server/startup/settings.js @@ -522,6 +522,11 @@ settings.addGroup('Accounts', function() { public: true, }); + this.add('Accounts_RoomAvatarExternalProviderUrl', '', { + type: 'string', + public: true, + }); + this.add('Accounts_AvatarCacheTime', 3600, { type: 'int', i18nDescription: 'Accounts_AvatarCacheTime_description', diff --git a/app/utils/lib/getRoomAvatarURL.js b/app/utils/lib/getRoomAvatarURL.js index 83c8d21086046..3130ebd6742ce 100644 --- a/app/utils/lib/getRoomAvatarURL.js +++ b/app/utils/lib/getRoomAvatarURL.js @@ -2,7 +2,7 @@ import { getAvatarURL } from './getAvatarURL'; import { settings } from '../../settings'; export const getRoomAvatarURL = function(roomId, etag) { - const externalSource = (settings.get('Accounts_AvatarExternalProviderUrl') || '').trim().replace(/\/$/, ''); + const externalSource = (settings.get('Accounts_RoomAvatarExternalProviderUrl') || '').trim().replace(/\/$/, ''); if (externalSource !== '') { return externalSource.replace('{roomId}', roomId); } diff --git a/client/components/basic/avatar/UserAvatar.js b/client/components/basic/avatar/UserAvatar.js index 99615325f6e93..938d487d2b9cf 100644 --- a/client/components/basic/avatar/UserAvatar.js +++ b/client/components/basic/avatar/UserAvatar.js @@ -1,14 +1,18 @@ import React from 'react'; import BaseAvatar from './BaseAvatar'; -import { getUserAvatarURL as getAvatarUrl } from '../../../../app/utils/lib/getUserAvatarURL'; +import { settings } from '../../../../app/settings'; function UserAvatar({ url, username, etag, ...props }) { // NOW, `username` and `etag` props are enough to determine the whole state of // this component, but it must be as performatic as possible as it will be // rendered many times; and some of the state can be derived at the ancestors. // Ideally, it should be a purely visual component. - const avatarUrl = getAvatarUrl(username) || url || `/avatar/${ username }${ etag ? `?etag=${ etag }` : '' }`; + + let externalSource = (settings.get('Accounts_AvatarExternalProviderUrl') || '').trim().replace(/\/$/, ''); + externalSource = externalSource !== '' && externalSource.replace('{username}', username); + + const avatarUrl = externalSource || url || `/avatar/${ username }${ etag ? `?etag=${ etag }` : '' }`; return ; } diff --git a/packages/rocketchat-i18n/i18n/en.i18n.json b/packages/rocketchat-i18n/i18n/en.i18n.json index 115efd951e896..2f9d2c3c5c6e6 100644 --- a/packages/rocketchat-i18n/i18n/en.i18n.json +++ b/packages/rocketchat-i18n/i18n/en.i18n.json @@ -54,6 +54,8 @@ "Accounts_AvatarSize": "Avatar Size", "Accounts_AvatarExternalProviderUrl": "Avatar External Provider URL", "Accounts_AvatarExternalProviderUrl_Description": "Example: `https://acme.com/api/v1/{username}`", + "Accounts_RoomAvatarExternalProviderUrl": "Room Avatar External Provider URL", + "Accounts_RoomAvatarExternalProviderUrl_Description": "Example: `https://acme.com/api/v1/{roomId}`", "Accounts_BlockedDomainsList": "Blocked Domains List", "Accounts_BlockedDomainsList_Description": "Comma-separated list of blocked domains", "Accounts_Verify_Email_For_External_Accounts": "Verify Email for External Accounts", From 3eb1f589065cfb82444fd5cd8187b8309b518a9f Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 13 Oct 2020 16:05:56 -0300 Subject: [PATCH 3/4] useSetting --- client/components/basic/avatar/UserAvatar.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/components/basic/avatar/UserAvatar.js b/client/components/basic/avatar/UserAvatar.js index 938d487d2b9cf..481b2463318df 100644 --- a/client/components/basic/avatar/UserAvatar.js +++ b/client/components/basic/avatar/UserAvatar.js @@ -2,14 +2,16 @@ import React from 'react'; import BaseAvatar from './BaseAvatar'; import { settings } from '../../../../app/settings'; +import { useSetting } from '../../../contexts/SettingsContext'; function UserAvatar({ url, username, etag, ...props }) { // NOW, `username` and `etag` props are enough to determine the whole state of // this component, but it must be as performatic as possible as it will be // rendered many times; and some of the state can be derived at the ancestors. // Ideally, it should be a purely visual component. + const externalProviderUrl = useSetting('Accounts_AvatarExternalProviderUrl'); - let externalSource = (settings.get('Accounts_AvatarExternalProviderUrl') || '').trim().replace(/\/$/, ''); + let externalSource = (externalProviderUrl || '').trim().replace(/\/$/, ''); externalSource = externalSource !== '' && externalSource.replace('{username}', username); const avatarUrl = externalSource || url || `/avatar/${ username }${ etag ? `?etag=${ etag }` : '' }`; From 5d32b41eac88ce98bf36b333cd4eeb4199e50775 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 13 Oct 2020 16:10:40 -0300 Subject: [PATCH 4/4] lint --- client/components/basic/avatar/UserAvatar.js | 1 - 1 file changed, 1 deletion(-) diff --git a/client/components/basic/avatar/UserAvatar.js b/client/components/basic/avatar/UserAvatar.js index 481b2463318df..b157944510e1c 100644 --- a/client/components/basic/avatar/UserAvatar.js +++ b/client/components/basic/avatar/UserAvatar.js @@ -1,7 +1,6 @@ import React from 'react'; import BaseAvatar from './BaseAvatar'; -import { settings } from '../../../../app/settings'; import { useSetting } from '../../../contexts/SettingsContext'; function UserAvatar({ url, username, etag, ...props }) {