Skip to content

Commit

Permalink
Merge pull request Expensify#39229 from software-mansion-labs/kicu/38…
Browse files Browse the repository at this point in the history
…743-use-fallback-avatar

Use fallback user avatar in cases where the user is unknown to us
  • Loading branch information
grgia authored Apr 24, 2024
2 parents 390bf24 + ad3e041 commit 08b7ff4
Show file tree
Hide file tree
Showing 27 changed files with 102 additions and 117 deletions.
14 changes: 10 additions & 4 deletions src/components/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ReportUtils from '@libs/ReportUtils';
import type {AvatarSource} from '@libs/UserUtils';
import * as UserUtils from '@libs/UserUtils';
import type {AvatarSizeName} from '@styles/utils';
import CONST from '@src/CONST';
import type {AvatarType} from '@src/types/onyx/OnyxCommon';
Expand Down Expand Up @@ -49,10 +50,13 @@ type AvatarProps = {

/** Owner of the avatar. If user, displayName. If workspace, policy name */
name?: string;

/** Optional account id if it's user avatar */
accountID?: number;
};

function Avatar({
source,
source: originalSource,
imageStyles,
iconAdditionalStyles,
containerStyles,
Expand All @@ -62,6 +66,7 @@ function Avatar({
fallbackIconTestID = '',
type = CONST.ICON_TYPE_AVATAR,
name = '',
accountID,
}: AvatarProps) {
const theme = useTheme();
const styles = useThemeStyles();
Expand All @@ -72,16 +77,17 @@ function Avatar({

useEffect(() => {
setImageError(false);
}, [source]);
}, [originalSource]);

const isWorkspace = type === CONST.ICON_TYPE_WORKSPACE;
const iconSize = StyleUtils.getAvatarSize(size);

const iconSize = StyleUtils.getAvatarSize(size);
const imageStyle: StyleProp<ImageStyle> = [StyleUtils.getAvatarStyle(size), imageStyles, styles.noBorderRadius];
const iconStyle = imageStyles ? [StyleUtils.getAvatarStyle(size), styles.bgTransparent, imageStyles] : undefined;

// We pass the color styles down to the SVG for the workspace and fallback avatar.
const useFallBackAvatar = imageError || source === Expensicons.FallbackAvatar || !source;
const source = isWorkspace ? originalSource : UserUtils.getAvatar(originalSource, accountID);
const useFallBackAvatar = imageError || !source || source === Expensicons.FallbackAvatar;
const fallbackAvatar = isWorkspace ? ReportUtils.getDefaultWorkspaceAvatar(name) : fallbackIcon || Expensicons.FallbackAvatar;
const fallbackAvatarTestID = isWorkspace ? ReportUtils.getDefaultWorkspaceAvatarTestID(name) : fallbackIconTestID || 'SvgFallbackAvatar Icon';
const avatarSource = useFallBackAvatar ? fallbackAvatar : source;
Expand Down
9 changes: 6 additions & 3 deletions src/components/AvatarWithIndicator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import Tooltip from './Tooltip';

type AvatarWithIndicatorProps = {
/** URL for the avatar */
source: UserUtils.AvatarSource;
source?: UserUtils.AvatarSource;

/** account id if it's user avatar */
accountID?: number;

/** To show a tooltip on hover */
tooltipText?: string;
Expand All @@ -23,7 +26,7 @@ type AvatarWithIndicatorProps = {
isLoading?: boolean;
};

function AvatarWithIndicator({source, tooltipText = '', fallbackIcon = Expensicons.FallbackAvatar, isLoading = true}: AvatarWithIndicatorProps) {
function AvatarWithIndicator({source, accountID, tooltipText = '', fallbackIcon = Expensicons.FallbackAvatar, isLoading = true}: AvatarWithIndicatorProps) {
const styles = useThemeStyles();

return (
Expand All @@ -35,7 +38,7 @@ function AvatarWithIndicator({source, tooltipText = '', fallbackIcon = Expensico
<>
<Avatar
size={CONST.AVATAR_SIZE.SMALL}
source={UserUtils.getSmallSizeAvatar(source)}
source={UserUtils.getSmallSizeAvatar(source, accountID)}
fallbackIcon={fallbackIcon}
/>
<Indicator />
Expand Down
2 changes: 2 additions & 0 deletions src/components/MultipleAvatars.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ function MultipleAvatars({
name={icons[0].name}
type={icons[0].type}
fallbackIcon={icons[0].fallbackIcon}
accountID={icons[0].id}
/>
</View>
</UserDetailsTooltip>
Expand Down Expand Up @@ -207,6 +208,7 @@ function MultipleAvatars({
name={icon.name}
type={icon.type}
fallbackIcon={icon.fallbackIcon}
accountID={icon.id}
/>
</View>
</UserDetailsTooltip>
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/TaskView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function TaskView({report, shouldShowHorizontalRule, ...props}: TaskViewProps) {
<MenuItem
label={translate('task.assignee')}
title={ReportUtils.getDisplayNameForParticipant(report.managerID)}
icon={OptionsListUtils.getAvatarsForAccountIDs(report.managerID ? [report.managerID] : [], personalDetails)}
icon={OptionsListUtils.getAvatarsForAccountIDs([report.managerID], personalDetails)}
iconType={CONST.ICON_TYPE_AVATAR}
avatarSize={CONST.AVATAR_SIZE.SMALLER}
titleStyle={styles.assigneeTextStyle}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import * as LocalePhoneNumber from '@libs/LocalePhoneNumber';
import * as ReportUtils from '@libs/ReportUtils';
import * as UserUtils from '@libs/UserUtils';
import CONST from '@src/CONST';

function BaseUserDetailsTooltip({accountID, fallbackUserDetails, icon, delegateAccountID, shiftHorizontal, children}: UserDetailsTooltipProps) {
Expand Down Expand Up @@ -55,10 +54,11 @@ function BaseUserDetailsTooltip({accountID, fallbackUserDetails, icon, delegateA
<View style={styles.emptyAvatar}>
<Avatar
containerStyles={[styles.actionAvatar]}
source={icon?.source ?? UserUtils.getAvatar(userAvatar, userAccountID)}
source={icon?.source ?? userAvatar}
type={icon?.type ?? CONST.ICON_TYPE_AVATAR}
name={icon?.name ?? userLogin}
fallbackIcon={icon?.fallbackIcon}
accountID={userAccountID}
/>
</View>
<Text style={[styles.mt2, styles.textMicroBold, styles.textReactionSenders, styles.textAlignCenter]}>{title}</Text>
Expand Down
37 changes: 13 additions & 24 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import lodashSet from 'lodash/set';
import lodashSortBy from 'lodash/sortBy';
import Onyx from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import {FallbackAvatar} from '@components/Icon/Expensicons';
import type {SelectedTagOption} from '@components/TagPicker';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
Expand Down Expand Up @@ -304,13 +305,14 @@ function getAvatarsForAccountIDs(accountIDs: number[], personalDetails: OnyxEntr
Object.entries(defaultValues).forEach((item) => {
reversedDefaultValues[item[1]] = item[0];
});

return accountIDs.map((accountID) => {
const login = reversedDefaultValues[accountID] ?? '';
const userPersonalDetail = personalDetails?.[accountID] ?? {login, accountID, avatar: ''};
const userPersonalDetail = personalDetails?.[accountID] ?? {login, accountID};

return {
id: accountID,
source: UserUtils.getAvatar(userPersonalDetail.avatar, userPersonalDetail.accountID),
source: userPersonalDetail.avatar ?? FallbackAvatar,
type: CONST.ICON_TYPE_AVATAR,
name: userPersonalDetail.login ?? '',
};
Expand All @@ -333,9 +335,7 @@ function getPersonalDetailsForAccountIDs(accountIDs: number[] | undefined, perso
}
let personalDetail: OnyxEntry<PersonalDetails> = personalDetails[accountID];
if (!personalDetail) {
personalDetail = {
avatar: UserUtils.getDefaultAvatar(cleanAccountID),
} as PersonalDetails;
personalDetail = {} as PersonalDetails;
}

if (cleanAccountID === CONST.ACCOUNT_ID.CONCIERGE) {
Expand Down Expand Up @@ -364,6 +364,7 @@ function getParticipantsOption(participant: ReportUtils.OptionData | Participant
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const login = detail?.login || participant.login || '';
const displayName = PersonalDetailsUtils.getDisplayNameOrDefault(detail, LocalePhoneNumber.formatPhoneNumber(login));

return {
keyForList: String(detail?.accountID),
login,
Expand All @@ -374,7 +375,7 @@ function getParticipantsOption(participant: ReportUtils.OptionData | Participant
alternateText: LocalePhoneNumber.formatPhoneNumber(login) || displayName,
icons: [
{
source: UserUtils.getAvatar(detail?.avatar ?? '', detail?.accountID ?? -1),
source: detail?.avatar ?? FallbackAvatar,
name: login,
type: CONST.ICON_TYPE_AVATAR,
id: detail?.accountID,
Expand Down Expand Up @@ -758,13 +759,7 @@ function createOption(
// Disabling this line for safeness as nullish coalescing works only if the value is undefined or null
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
result.searchText = getSearchText(report, reportName, personalDetailList, !!result.isChatRoom || !!result.isPolicyExpenseChat, !!result.isThread);
result.icons = ReportUtils.getIcons(
report,
personalDetails,
UserUtils.getAvatar(personalDetail?.avatar ?? '', personalDetail?.accountID),
personalDetail?.login,
personalDetail?.accountID,
);
result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID);
result.subtitle = subtitle;

return result;
Expand Down Expand Up @@ -1864,7 +1859,6 @@ function getOptions(
[optimisticAccountID]: {
accountID: optimisticAccountID,
login: searchValue,
avatar: UserUtils.getDefaultAvatar(optimisticAccountID),
},
};
userToInvite = createOption([optimisticAccountID], personalDetailsExtended, null, reportActions, {
Expand All @@ -1877,10 +1871,10 @@ function getOptions(
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
userToInvite.alternateText = userToInvite.alternateText || searchValue;

// If user doesn't exist, use a default avatar
// If user doesn't exist, use a fallback avatar
userToInvite.icons = [
{
source: UserUtils.getAvatar('', optimisticAccountID),
source: FallbackAvatar,
name: searchValue,
type: CONST.ICON_TYPE_AVATAR,
},
Expand Down Expand Up @@ -1954,17 +1948,12 @@ function getShareLogOptions(options: OptionList, searchValue = '', betas: Beta[]
*/
function getIOUConfirmationOptionsFromPayeePersonalDetail(personalDetail: PersonalDetails | EmptyObject, amountText?: string): PayeePersonalDetails {
const formattedLogin = LocalePhoneNumber.formatPhoneNumber(personalDetail.login ?? '');
const icons = [{source: personalDetail.avatar ?? FallbackAvatar, name: personalDetail.login ?? '', type: CONST.ICON_TYPE_AVATAR, id: personalDetail.accountID}];

return {
text: PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, formattedLogin),
alternateText: formattedLogin || PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, '', false),
icons: [
{
source: UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID),
name: personalDetail.login ?? '',
type: CONST.ICON_TYPE_AVATAR,
id: personalDetail.accountID,
},
],
icons,
descriptiveText: amountText ?? '',
login: personalDetail.login ?? '',
accountID: personalDetail.accountID,
Expand Down
1 change: 0 additions & 1 deletion src/libs/PersonalDetailsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ function getPersonalDetailsOnyxDataForOptimisticUsers(newLogins: string[], newAc
personalDetailsNew[accountID] = {
login,
accountID,
avatar: UserUtils.getDefaultAvatarURL(accountID),
displayName: LocalePhoneNumber.formatPhoneNumber(login),
};

Expand Down
42 changes: 21 additions & 21 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {FileObject} from '@components/AttachmentModal';
import * as Expensicons from '@components/Icon/Expensicons';
import {FallbackAvatar} from '@components/Icon/Expensicons';
import * as defaultGroupAvatars from '@components/Icon/GroupDefaultAvatars';
import * as defaultWorkspaceAvatars from '@components/Icon/WorkspaceDefaultAvatars';
import type {IOUAction, IOUType} from '@src/CONST';
Expand Down Expand Up @@ -1629,7 +1629,7 @@ function getIconsForParticipants(participants: number[], personalDetails: OnyxCo
const participantsList = participants || [];

for (const accountID of participantsList) {
const avatarSource = UserUtils.getAvatar(personalDetails?.[accountID]?.avatar ?? '', accountID);
const avatarSource = personalDetails?.[accountID]?.avatar ?? FallbackAvatar;
const displayNameLogin = personalDetails?.[accountID]?.displayName ? personalDetails?.[accountID]?.displayName : personalDetails?.[accountID]?.login;
participantDetails.push([accountID, displayNameLogin ?? '', avatarSource, personalDetails?.[accountID]?.fallbackIcon ?? '']);
}
Expand Down Expand Up @@ -1690,12 +1690,12 @@ function getPersonalDetailsForAccountID(accountID: number): Partial<PersonalDeta
if (!accountID) {
return {};
}
return (
allPersonalDetails?.[accountID] ?? {
avatar: UserUtils.getDefaultAvatar(accountID),
isOptimisticPersonalDetail: true,
}
);

const defaultDetails = {
isOptimisticPersonalDetail: true,
};

return allPersonalDetails?.[accountID] ?? defaultDetails;
}

/**
Expand Down Expand Up @@ -1817,7 +1817,7 @@ function getIcons(
): Icon[] {
if (isEmptyObject(report)) {
const fallbackIcon: Icon = {
source: defaultIcon ?? Expensicons.FallbackAvatar,
source: defaultIcon ?? FallbackAvatar,
type: CONST.ICON_TYPE_AVATAR,
name: defaultName,
id: defaultAccountID,
Expand All @@ -1828,7 +1828,7 @@ function getIcons(
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: UserUtils.getAvatar(personalDetails?.[parentReportAction.actorAccountID ?? -1]?.avatar ?? '', parentReportAction.actorAccountID ?? -1),
source: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.avatar ?? FallbackAvatar,
id: parentReportAction.actorAccountID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.displayName ?? '',
Expand All @@ -1844,7 +1844,7 @@ function getIcons(
const actorDisplayName = PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[actorAccountID ?? -1], '', false);
const actorIcon = {
id: actorAccountID,
source: UserUtils.getAvatar(personalDetails?.[actorAccountID ?? -1]?.avatar ?? '', actorAccountID ?? -1),
source: personalDetails?.[actorAccountID ?? -1]?.avatar ?? FallbackAvatar,
name: actorDisplayName,
type: CONST.ICON_TYPE_AVATAR,
fallbackIcon: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.fallbackIcon,
Expand All @@ -1859,7 +1859,7 @@ function getIcons(
if (isTaskReport(report)) {
const ownerIcon = {
id: report?.ownerAccountID,
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.ownerAccountID ?? -1]?.fallbackIcon,
Expand Down Expand Up @@ -1891,7 +1891,7 @@ function getIcons(
if (isPolicyExpenseChat(report) || isExpenseReport(report)) {
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
id: report?.ownerAccountID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
Expand All @@ -1901,15 +1901,15 @@ function getIcons(
}
if (isIOUReport(report)) {
const managerIcon = {
source: UserUtils.getAvatar(personalDetails?.[report?.managerID ?? -1]?.avatar ?? '', report?.managerID ?? -1),
source: personalDetails?.[report?.managerID ?? -1]?.avatar ?? FallbackAvatar,
id: report?.managerID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.managerID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.managerID ?? -1]?.fallbackIcon,
};
const ownerIcon = {
id: report?.ownerAccountID,
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.ownerAccountID ?? -1]?.fallbackIcon,
Expand Down Expand Up @@ -1955,7 +1955,7 @@ function getDisplayNamesWithTooltips(
const accountID = Number(user?.accountID);
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const displayName = getDisplayNameForParticipant(accountID, isMultipleParticipantReport, shouldFallbackToHidden, shouldAddCurrentUserPostfix) || user?.login || '';
const avatar = UserUtils.getDefaultAvatar(accountID);
const avatar = user && 'avatar' in user ? user.avatar : FallbackAvatar;

let pronouns = user?.pronouns ?? undefined;
if (pronouns?.startsWith(CONST.PRONOUNS.PREFIX)) {
Expand Down Expand Up @@ -3241,7 +3241,7 @@ function buildOptimisticAddCommentReportAction(text?: string, file?: FileObject,
},
],
automatic: false,
avatar: allPersonalDetails?.[accountID ?? -1]?.avatar ?? UserUtils.getDefaultAvatarURL(accountID),
avatar: allPersonalDetails?.[accountID ?? -1]?.avatar,
created: DateUtils.getDBTimeWithSkew(Date.now() + createdOffset),
message: [
{
Expand Down Expand Up @@ -6054,10 +6054,10 @@ function hasMissingPaymentMethod(userWallet: OnyxEntry<UserWallet>, iouReportID:

/**
* Used from expense actions to decide if we need to build an optimistic expense report.
Create a new report if:
- we don't have an iouReport set in the chatReport
- we have one, but it's waiting on the payee adding a bank account
- we have one but we can't add more transactions to it due to: report is approved or settled, or report is processing and policy isn't on Instant submit reporting frequency
Create a new report if:
- we don't have an iouReport set in the chatReport
- we have one, but it's waiting on the payee adding a bank account
- we have one but we can't add more transactions to it due to: report is approved or settled, or report is processing and policy isn't on Instant submit reporting frequency
*/
function shouldCreateNewMoneyRequestReport(existingIOUReport: OnyxEntry<Report> | undefined | null, chatReport: OnyxEntry<Report> | null): boolean {
return !existingIOUReport || hasIOUWaitingOnCurrentUserBankAccount(chatReport) || !canAddOrDeleteTransactions(existingIOUReport);
Expand Down
Loading

0 comments on commit 08b7ff4

Please sign in to comment.