Skip to content

Commit

Permalink
Merge pull request #41087 from Expensify/revert-39229-kicu/38743-use-…
Browse files Browse the repository at this point in the history
…fallback-avatar

[CP Staging] Revert "Use fallback user avatar in cases where the user is unknown to us"

(cherry picked from commit 7885af4)
  • Loading branch information
pecanoro authored and OSBotify committed Apr 26, 2024
1 parent b2f6d55 commit 6cac408
Show file tree
Hide file tree
Showing 27 changed files with 117 additions and 102 deletions.
14 changes: 4 additions & 10 deletions src/components/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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 @@ -50,13 +49,10 @@ 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: originalSource,
source,
imageStyles,
iconAdditionalStyles,
containerStyles,
Expand All @@ -66,7 +62,6 @@ function Avatar({
fallbackIconTestID = '',
type = CONST.ICON_TYPE_AVATAR,
name = '',
accountID,
}: AvatarProps) {
const theme = useTheme();
const styles = useThemeStyles();
Expand All @@ -77,17 +72,16 @@ function Avatar({

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

const isWorkspace = type === CONST.ICON_TYPE_WORKSPACE;

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 source = isWorkspace ? originalSource : UserUtils.getAvatar(originalSource, accountID);
const useFallBackAvatar = imageError || !source || source === Expensicons.FallbackAvatar;
const useFallBackAvatar = imageError || source === Expensicons.FallbackAvatar || !source;
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: 3 additions & 6 deletions src/components/AvatarWithIndicator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ import Tooltip from './Tooltip';

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

/** account id if it's user avatar */
accountID?: number;
source: UserUtils.AvatarSource;

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

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

return (
Expand All @@ -38,7 +35,7 @@ function AvatarWithIndicator({source, accountID, tooltipText = '', fallbackIcon
<>
<Avatar
size={CONST.AVATAR_SIZE.SMALL}
source={UserUtils.getSmallSizeAvatar(source, accountID)}
source={UserUtils.getSmallSizeAvatar(source)}
fallbackIcon={fallbackIcon}
/>
<Indicator />
Expand Down
2 changes: 0 additions & 2 deletions src/components/MultipleAvatars.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ function MultipleAvatars({
name={icons[0].name}
type={icons[0].type}
fallbackIcon={icons[0].fallbackIcon}
accountID={icons[0].id}
/>
</View>
</UserDetailsTooltip>
Expand Down Expand Up @@ -208,7 +207,6 @@ 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], personalDetails)}
icon={OptionsListUtils.getAvatarsForAccountIDs(report.managerID ? [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,6 +10,7 @@ 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 @@ -54,11 +55,10 @@ function BaseUserDetailsTooltip({accountID, fallbackUserDetails, icon, delegateA
<View style={styles.emptyAvatar}>
<Avatar
containerStyles={[styles.actionAvatar]}
source={icon?.source ?? userAvatar}
source={icon?.source ?? UserUtils.getAvatar(userAvatar, userAccountID)}
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: 24 additions & 13 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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 @@ -305,14 +304,13 @@ 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};
const userPersonalDetail = personalDetails?.[accountID] ?? {login, accountID, avatar: ''};

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

if (cleanAccountID === CONST.ACCOUNT_ID.CONCIERGE) {
Expand Down Expand Up @@ -364,7 +364,6 @@ 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 @@ -375,7 +374,7 @@ function getParticipantsOption(participant: ReportUtils.OptionData | Participant
alternateText: LocalePhoneNumber.formatPhoneNumber(login) || displayName,
icons: [
{
source: detail?.avatar ?? FallbackAvatar,
source: UserUtils.getAvatar(detail?.avatar ?? '', detail?.accountID ?? -1),
name: login,
type: CONST.ICON_TYPE_AVATAR,
id: detail?.accountID,
Expand Down Expand Up @@ -759,7 +758,13 @@ 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, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID);
result.icons = ReportUtils.getIcons(
report,
personalDetails,
UserUtils.getAvatar(personalDetail?.avatar ?? '', personalDetail?.accountID),
personalDetail?.login,
personalDetail?.accountID,
);
result.subtitle = subtitle;

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

// If user doesn't exist, use a fallback avatar
// If user doesn't exist, use a default avatar
userToInvite.icons = [
{
source: FallbackAvatar,
source: UserUtils.getAvatar('', optimisticAccountID),
name: searchValue,
type: CONST.ICON_TYPE_AVATAR,
},
Expand Down Expand Up @@ -1949,12 +1955,17 @@ 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,
icons: [
{
source: UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID),
name: personalDetail.login ?? '',
type: CONST.ICON_TYPE_AVATAR,
id: personalDetail.accountID,
},
],
descriptiveText: amountText ?? '',
login: personalDetail.login ?? '',
accountID: personalDetail.accountID,
Expand Down
1 change: 1 addition & 0 deletions src/libs/PersonalDetailsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ 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 {FallbackAvatar} from '@components/Icon/Expensicons';
import * as Expensicons 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 @@ -1630,7 +1630,7 @@ function getIconsForParticipants(participants: number[], personalDetails: OnyxCo
const participantsList = participants || [];

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

const defaultDetails = {
isOptimisticPersonalDetail: true,
};

return allPersonalDetails?.[accountID] ?? defaultDetails;
return (
allPersonalDetails?.[accountID] ?? {
avatar: UserUtils.getDefaultAvatar(accountID),
isOptimisticPersonalDetail: true,
}
);
}

/**
Expand Down Expand Up @@ -1818,7 +1818,7 @@ function getIcons(
): Icon[] {
if (isEmptyObject(report)) {
const fallbackIcon: Icon = {
source: defaultIcon ?? FallbackAvatar,
source: defaultIcon ?? Expensicons.FallbackAvatar,
type: CONST.ICON_TYPE_AVATAR,
name: defaultName,
id: defaultAccountID,
Expand All @@ -1829,7 +1829,7 @@ function getIcons(
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.avatar ?? FallbackAvatar,
source: UserUtils.getAvatar(personalDetails?.[parentReportAction.actorAccountID ?? -1]?.avatar ?? '', parentReportAction.actorAccountID ?? -1),
id: parentReportAction.actorAccountID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.displayName ?? '',
Expand All @@ -1845,7 +1845,7 @@ function getIcons(
const actorDisplayName = PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[actorAccountID ?? -1], '', false);
const actorIcon = {
id: actorAccountID,
source: personalDetails?.[actorAccountID ?? -1]?.avatar ?? FallbackAvatar,
source: UserUtils.getAvatar(personalDetails?.[actorAccountID ?? -1]?.avatar ?? '', actorAccountID ?? -1),
name: actorDisplayName,
type: CONST.ICON_TYPE_AVATAR,
fallbackIcon: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.fallbackIcon,
Expand All @@ -1860,7 +1860,7 @@ function getIcons(
if (isTaskReport(report)) {
const ownerIcon = {
id: report?.ownerAccountID,
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.ownerAccountID ?? -1]?.fallbackIcon,
Expand Down Expand Up @@ -1892,7 +1892,7 @@ function getIcons(
if (isPolicyExpenseChat(report) || isExpenseReport(report)) {
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
id: report?.ownerAccountID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
Expand All @@ -1902,15 +1902,15 @@ function getIcons(
}
if (isIOUReport(report)) {
const managerIcon = {
source: personalDetails?.[report?.managerID ?? -1]?.avatar ?? FallbackAvatar,
source: UserUtils.getAvatar(personalDetails?.[report?.managerID ?? -1]?.avatar ?? '', report?.managerID ?? -1),
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: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.ownerAccountID ?? -1]?.fallbackIcon,
Expand Down Expand Up @@ -1956,7 +1956,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 = user && 'avatar' in user ? user.avatar : FallbackAvatar;
const avatar = UserUtils.getDefaultAvatar(accountID);

let pronouns = user?.pronouns ?? undefined;
if (pronouns?.startsWith(CONST.PRONOUNS.PREFIX)) {
Expand Down Expand Up @@ -3242,7 +3242,7 @@ function buildOptimisticAddCommentReportAction(text?: string, file?: FileObject,
},
],
automatic: false,
avatar: allPersonalDetails?.[accountID ?? -1]?.avatar,
avatar: allPersonalDetails?.[accountID ?? -1]?.avatar ?? UserUtils.getDefaultAvatarURL(accountID),
created: DateUtils.getDBTimeWithSkew(Date.now() + createdOffset),
message: [
{
Expand Down Expand Up @@ -6056,10 +6056,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 6cac408

Please sign in to comment.