Skip to content

Commit 11ef858

Browse files
authored
Merge pull request Expensify#68662 from dmkt9/fix/68563
Fix/68563 - Share displays a different avatar than the one set on the account
2 parents 1ef115b + 227ec59 commit 11ef858

2 files changed

Lines changed: 29 additions & 15 deletions

File tree

src/libs/UserUtils.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -111,17 +111,7 @@ function generateAccountID(searchValue: string): number {
111111
return hashText(searchValue, 2 ** 32);
112112
}
113113

114-
/**
115-
* Helper method to return the default avatar associated with the given accountID
116-
*/
117-
function getDefaultAvatar(accountID = -1, avatarURL?: string): IconAsset | undefined {
118-
if (accountID === CONST.ACCOUNT_ID.CONCIERGE) {
119-
return ConciergeAvatar;
120-
}
121-
if (accountID === CONST.ACCOUNT_ID.NOTIFICATIONS) {
122-
return NotificationsAvatar;
123-
}
124-
114+
function getAccountIDHashBucket(accountID = -1, avatarURL?: string) {
125115
// There are 24 possible default avatars, so we choose which one this user has based
126116
// on a simple modulo operation of their login number. Note that Avatar count starts at 1.
127117

@@ -135,7 +125,21 @@ function getDefaultAvatar(accountID = -1, avatarURL?: string): IconAsset | undef
135125
} else if (accountID > 0) {
136126
accountIDHashBucket = ((accountID % CONST.DEFAULT_AVATAR_COUNT) + 1) as AvatarRange;
137127
}
128+
return accountIDHashBucket;
129+
}
130+
131+
/**
132+
* Helper method to return the default avatar associated with the given accountID
133+
*/
134+
function getDefaultAvatar(accountID = -1, avatarURL?: string): IconAsset | undefined {
135+
if (accountID === CONST.ACCOUNT_ID.CONCIERGE) {
136+
return ConciergeAvatar;
137+
}
138+
if (accountID === CONST.ACCOUNT_ID.NOTIFICATIONS) {
139+
return NotificationsAvatar;
140+
}
138141

142+
const accountIDHashBucket = getAccountIDHashBucket(accountID, avatarURL);
139143
if (!accountIDHashBucket) {
140144
return;
141145
}
@@ -146,13 +150,12 @@ function getDefaultAvatar(accountID = -1, avatarURL?: string): IconAsset | undef
146150
/**
147151
* Helper method to return default avatar URL associated with the accountID
148152
*/
149-
function getDefaultAvatarURL(accountID: string | number = ''): string {
153+
function getDefaultAvatarURL(accountID: string | number = '', avatarURL?: string): string {
150154
if (Number(accountID) === CONST.ACCOUNT_ID.CONCIERGE) {
151155
return CONST.CONCIERGE_ICON_URL;
152156
}
153157

154-
// Note that Avatar count starts at 1 which is why 1 has to be added to the result (or else 0 would result in a broken avatar link)
155-
const accountIDHashBucket = (Number(accountID) % CONST.DEFAULT_AVATAR_COUNT) + 1;
158+
const accountIDHashBucket = getAccountIDHashBucket(Number(accountID) || -1, avatarURL);
156159
const avatarPrefix = `default-avatar`;
157160

158161
return `${CONST.CLOUDFRONT_URL}/images/avatars/${avatarPrefix}_${accountIDHashBucket}.png`;
@@ -196,7 +199,7 @@ function getAvatar(avatarSource?: AvatarSource, accountID?: number): AvatarSourc
196199
* @param accountID - the accountID of the user
197200
*/
198201
function getAvatarUrl(avatarSource: AvatarSource | undefined, accountID: number): AvatarSource {
199-
return isDefaultAvatar(avatarSource) ? getDefaultAvatarURL(accountID) : avatarSource;
202+
return isDefaultAvatar(avatarSource) ? getDefaultAvatarURL(accountID, avatarSource) : avatarSource;
200203
}
201204

202205
/**

tests/unit/UserUtilsTest.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as defaultAvatars from '@components/Icon/DefaultAvatars';
12
import * as UserUtils from '@src/libs/UserUtils';
23

34
describe('UserUtils', () => {
@@ -6,12 +7,22 @@ describe('UserUtils', () => {
67
const defaultAvatar = UserUtils.getAvatar(avatarURL, 1);
78

89
expect(typeof defaultAvatar).toBe('function');
10+
11+
const defaultAvatarUrl = UserUtils.getAvatarUrl(avatarURL, 1);
12+
13+
expect(defaultAvatarUrl).toBe('https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/default-avatar_7.png');
14+
// Both defaultAvatar and defaultAvatarUrl must be `defaultAvatars.Avatar7`
15+
expect(defaultAvatar === defaultAvatars.Avatar7).toBeTruthy();
916
});
1017

1118
it('should return the same url if url is not for default avatar', () => {
1219
const avatarURL = 'https://test.com/images/some_avatar.png';
1320
const avatar = UserUtils.getAvatar(avatarURL, 1);
1421

1522
expect(avatar).toEqual('https://test.com/images/some_avatar.png');
23+
24+
const avatarUrl = UserUtils.getAvatarUrl(avatarURL, 1);
25+
26+
expect(avatarUrl).toEqual('https://test.com/images/some_avatar.png');
1627
});
1728
});

0 commit comments

Comments
 (0)