From 067cce1ac0fdda3888ac067939df24d173e4149b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 26 Oct 2023 09:57:21 -0300 Subject: [PATCH 1/2] Address review comments --- src/libs/actions/PersonalDetails.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/PersonalDetails.ts b/src/libs/actions/PersonalDetails.ts index 6600945b8601..e2dbef071f9b 100644 --- a/src/libs/actions/PersonalDetails.ts +++ b/src/libs/actions/PersonalDetails.ts @@ -70,11 +70,17 @@ function getDisplayNameForTypingIndicator(userAccountIDOrLogin: string, defaultD // so Number(string) is NaN. Search for personalDetails by login to get the display name. if (Number.isNaN(accountID)) { const detailsByLogin = Object.entries(allPersonalDetails ?? {}).find(([, value]) => value?.login === userAccountIDOrLogin)?.[1]; - return detailsByLogin?.displayName ?? userAccountIDOrLogin; + + // It's possible for displayName to be empty string, so we must fallback to userAccountIDOrLogin. + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + return detailsByLogin?.displayName || userAccountIDOrLogin; } const detailsByAccountID = allPersonalDetails?.[accountID]; - return detailsByAccountID?.displayName ?? detailsByAccountID?.login ?? defaultDisplayName; + + // It's possible for displayName to be empty string, so we must fallback to login or defaultDisplayName. + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + return detailsByAccountID?.displayName || detailsByAccountID?.login || defaultDisplayName; } /** @@ -83,7 +89,9 @@ function getDisplayNameForTypingIndicator(userAccountIDOrLogin: string, defaultD * so we return empty strings instead. */ function extractFirstAndLastNameFromAvailableDetails({login, displayName, firstName, lastName}: PersonalDetails): FirstAndLastName { - if (firstName ?? lastName) { + // It's possible for firstName to be empty string, so we must consider lastName instead. + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + if (firstName || lastName) { return {firstName: firstName ?? '', lastName: lastName ?? ''}; } if (login && Str.removeSMSDomain(login) === displayName) { From 58eb8f3e8d100cf88a3a866346b7a9f866e2a9fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 27 Oct 2023 09:33:28 -0300 Subject: [PATCH 2/2] Improve comments --- src/libs/actions/PersonalDetails.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/PersonalDetails.ts b/src/libs/actions/PersonalDetails.ts index e2dbef071f9b..90a53a8de8df 100644 --- a/src/libs/actions/PersonalDetails.ts +++ b/src/libs/actions/PersonalDetails.ts @@ -55,6 +55,7 @@ function getDisplayName(login: string, personalDetail: Pick value?.login === userAccountIDOrLogin)?.[1]; - // It's possible for displayName to be empty string, so we must fallback to userAccountIDOrLogin. + // It's possible for displayName to be empty string, so we must use "||" to fallback to userAccountIDOrLogin. // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing return detailsByLogin?.displayName || userAccountIDOrLogin; } const detailsByAccountID = allPersonalDetails?.[accountID]; - // It's possible for displayName to be empty string, so we must fallback to login or defaultDisplayName. + // It's possible for displayName to be empty string, so we must use "||" to fallback to login or defaultDisplayName. // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing return detailsByAccountID?.displayName || detailsByAccountID?.login || defaultDisplayName; } @@ -89,7 +90,7 @@ function getDisplayNameForTypingIndicator(userAccountIDOrLogin: string, defaultD * so we return empty strings instead. */ function extractFirstAndLastNameFromAvailableDetails({login, displayName, firstName, lastName}: PersonalDetails): FirstAndLastName { - // It's possible for firstName to be empty string, so we must consider lastName instead. + // It's possible for firstName to be empty string, so we must use "||" to consider lastName instead. // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing if (firstName || lastName) { return {firstName: firstName ?? '', lastName: lastName ?? ''};