From 6901cf1b230f975c145f41dacaa3a7060da588a0 Mon Sep 17 00:00:00 2001 From: Georgia Monahan Date: Mon, 30 Sep 2024 17:47:44 +0100 Subject: [PATCH] Revert "Merge pull request #49172 from Expensify/georgia-fixIOUpreview" This reverts commit b215aa8d8910864eedc38a08f838c7536fe506c8, reversing changes made to bc106fab216fbf60cd55004a94223cacf5afa8d9. --- src/CONST.ts | 2 +- src/libs/ReportActionsUtils.ts | 47 +--- src/libs/ReportUtils.ts | 27 +-- .../home/report/ReportActionItemSingle.tsx | 223 ++++++------------ .../report/ReportActionsListItemRenderer.tsx | 2 +- 5 files changed, 86 insertions(+), 215 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 4ca9b45f13df..8dd40862be71 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -171,7 +171,7 @@ const CONST = { }, // Note: Group and Self-DM excluded as these are not tied to a Workspace - WORKSPACE_ROOM_TYPES: [chatTypes.POLICY_ADMINS, chatTypes.POLICY_ANNOUNCE, chatTypes.DOMAIN_ALL, chatTypes.POLICY_ROOM, chatTypes.POLICY_EXPENSE_CHAT, chatTypes.INVOICE], + WORKSPACE_ROOM_TYPES: [chatTypes.POLICY_ADMINS, chatTypes.POLICY_ANNOUNCE, chatTypes.DOMAIN_ALL, chatTypes.POLICY_ROOM, chatTypes.POLICY_EXPENSE_CHAT], ANDROID_PACKAGE_NAME, WORKSPACE_ENABLE_FEATURE_REDIRECT_DELAY: 100, ANIMATED_HIGHLIGHT_ENTRY_DELAY: 50, diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 15d7728ba35a..eac0ac4fe438 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -1009,20 +1009,20 @@ const iouRequestTypes = new Set>([ CONST.IOU.REPORT_ACTION_TYPE.TRACK, ]); -function getMoneyRequestActions( - reportID: string, - reportActions: OnyxEntry | ReportAction[], - isOffline: boolean | undefined = undefined, -): Array> { - // If the report is not an IOU, Expense report, or Invoice, it shouldn't have money request actions. +/** + * Gets the reportID for the transaction thread associated with a report by iterating over the reportActions and identifying the IOU report actions. + * Returns a reportID if there is exactly one transaction thread for the report, and null otherwise. + */ +function getOneTransactionThreadReportID(reportID: string, reportActions: OnyxEntry | ReportAction[], isOffline: boolean | undefined = undefined): string | undefined { + // If the report is not an IOU, Expense report, or Invoice, it shouldn't be treated as one-transaction report. const report = ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; if (report?.type !== CONST.REPORT.TYPE.IOU && report?.type !== CONST.REPORT.TYPE.EXPENSE && report?.type !== CONST.REPORT.TYPE.INVOICE) { - return []; + return; } const reportActionsArray = Array.isArray(reportActions) ? reportActions : Object.values(reportActions ?? {}); if (!reportActionsArray.length) { - return []; + return; } const iouRequestActions = []; @@ -1050,15 +1050,6 @@ function getMoneyRequestActions( iouRequestActions.push(action); } } - return iouRequestActions; -} - -/** - * Gets the reportID for the transaction thread associated with a report by iterating over the reportActions and identifying the IOU report actions. - * Returns a reportID if there is exactly one transaction thread for the report, and null otherwise. - */ -function getOneTransactionThreadReportID(reportID: string, reportActions: OnyxEntry | ReportAction[], isOffline: boolean | undefined = undefined): string | undefined { - const iouRequestActions = getMoneyRequestActions(reportID, reportActions, isOffline); // If we don't have any IOU request actions, or we have more than one IOU request actions, this isn't a oneTransaction report if (!iouRequestActions.length || iouRequestActions.length > 1) { @@ -1078,27 +1069,6 @@ function getOneTransactionThreadReportID(reportID: string, reportActions: OnyxEn return singleAction.childReportID; } -/** - * Returns true if all transactions on the report have the same ownerID - */ -function hasSameActorForAllTransactions(reportID: string, reportActions: OnyxEntry | ReportAction[], isOffline: boolean | undefined = undefined): boolean { - const iouRequestActions = getMoneyRequestActions(reportID, reportActions, isOffline); - if (!iouRequestActions.length) { - return true; - } - - let actorID: number | undefined; - - for (const action of iouRequestActions) { - if (actorID !== undefined && actorID !== action?.actorAccountID) { - return false; - } - actorID = action?.actorAccountID; - } - - return true; -} - /** * When we delete certain reports, we want to check whether there are any visible actions left to display. * If there are no visible actions left (including system messages), we can hide the report from view entirely @@ -1881,7 +1851,6 @@ export { getRenamedAction, isCardIssuedAction, getCardIssuedMessage, - hasSameActorForAllTransactions, }; export type {LastVisibleMessage}; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 78ebdd92751e..71ab125cd367 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -1637,20 +1637,12 @@ function hasOnlyNonReimbursableTransactions(iouReportID: string | undefined): bo return transactions.every((transaction) => !TransactionUtils.getReimbursable(transaction)); } -/** - * Checks if a report has only transactions with same ownerID - */ -function isSingleActorMoneyReport(reportID: string): boolean { - const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? ([] as ReportAction[]); - return !!ReportActionsUtils.hasSameActorForAllTransactions(reportID, reportActions); -} - /** * Checks if a report has only one transaction associated with it */ function isOneTransactionReport(reportID: string): boolean { const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? ([] as ReportAction[]); - return !!ReportActionsUtils.getOneTransactionThreadReportID(reportID, reportActions); + return ReportActionsUtils.getOneTransactionThreadReportID(reportID, reportActions) !== null; } /* @@ -2283,7 +2275,7 @@ function getIcons( if (isChatThread(report)) { const parentReportAction = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`]?.[report.parentReportActionID]; - const actorAccountID = getReportActionActorAccountID(parentReportAction); + const actorAccountID = getReportActionActorAccountID(parentReportAction, report); const actorDisplayName = PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[actorAccountID ?? -1], '', false); const actorIcon = { id: actorAccountID, @@ -2378,7 +2370,7 @@ function getIcons( const isManager = currentUserAccountID === report?.managerID; // For one transaction IOUs, display a simplified report icon - if (isOneTransactionReport(report?.reportID ?? '-1') || isSingleActorMoneyReport(report?.reportID ?? '-1')) { + if (isOneTransactionReport(report?.reportID ?? '-1')) { return [ownerIcon]; } @@ -4910,9 +4902,9 @@ function buildOptimisticReportPreview( }, ], created, - accountID: iouReport?.ownerAccountID ?? -1, + accountID: iouReport?.managerID ?? -1, // The preview is initially whispered if created with a receipt, so the actor is the current user as well - actorAccountID: hasReceipt ? currentUserAccountID : iouReport?.ownerAccountID ?? -1, + actorAccountID: hasReceipt ? currentUserAccountID : iouReport?.managerID ?? -1, childReportID: childReportID ?? iouReport?.reportID, childMoneyRequestCount: 1, childLastMoneyRequestComment: comment, @@ -6832,7 +6824,7 @@ function shouldReportShowSubscript(report: OnyxEntry): boolean { return true; } - if (isExpenseReport(report)) { + if (isExpenseReport(report) && isOneTransactionReport(report?.reportID ?? '-1')) { return true; } @@ -6844,7 +6836,7 @@ function shouldReportShowSubscript(report: OnyxEntry): boolean { return true; } - if (isInvoiceRoom(report) || isInvoiceReport(report)) { + if (isInvoiceRoom(report)) { return true; } @@ -7726,10 +7718,10 @@ function canLeaveChat(report: OnyxEntry, policy: OnyxEntry): boo return (isChatThread(report) && !!getReportNotificationPreference(report)) || isUserCreatedPolicyRoom(report) || isNonAdminOrOwnerOfPolicyExpenseChat(report, policy); } -function getReportActionActorAccountID(reportAction: OnyxInputOrEntry): number | undefined { +function getReportActionActorAccountID(reportAction: OnyxInputOrEntry, iouReport: OnyxInputOrEntry | undefined): number | undefined { switch (reportAction?.actionName) { case CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW: - return reportAction?.childOwnerAccountID ?? reportAction?.actorAccountID; + return !isEmptyObject(iouReport) ? iouReport.managerID : reportAction?.childManagerAccountID; case CONST.REPORT.ACTIONS.TYPE.SUBMITTED: return reportAction?.adminAccountID ?? reportAction?.actorAccountID; @@ -8312,7 +8304,6 @@ export { isIndividualInvoiceRoom, isAuditor, hasMissingInvoiceBankAccount, - isSingleActorMoneyReport, }; export type { diff --git a/src/pages/home/report/ReportActionItemSingle.tsx b/src/pages/home/report/ReportActionItemSingle.tsx index 91f12339ee07..ef1a7765e996 100644 --- a/src/pages/home/report/ReportActionItemSingle.tsx +++ b/src/pages/home/report/ReportActionItemSingle.tsx @@ -85,21 +85,20 @@ function ReportActionItemSingle({ const personalDetails = usePersonalDetails() ?? CONST.EMPTY_OBJECT; const policy = usePolicy(report?.policyID); const delegatePersonalDetails = personalDetails[action?.delegateAccountID ?? '']; - const actorAccountID = ReportUtils.getReportActionActorAccountID(action); + const actorAccountID = ReportUtils.getReportActionActorAccountID(action, iouReport); const [invoiceReceiverPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : -1}`); + let displayName = ReportUtils.getDisplayNameForParticipant(actorAccountID); - const icons = ReportUtils.getIcons(iouReport ?? null, personalDetails); const {avatar, login, pendingFields, status, fallbackIcon} = personalDetails[actorAccountID ?? -1] ?? {}; const accountOwnerDetails = getPersonalDetailByEmail(login ?? ''); // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing let actorHint = (login || (displayName ?? '')).replace(CONST.REGEX.MERGED_ACCOUNT_PREFIX, ''); const isTripRoom = ReportUtils.isTripRoom(report); const isReportPreviewAction = action?.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW; - const displayAllActors = isReportPreviewAction && !isTripRoom && ReportUtils.isIOUReport(iouReport ?? null) && icons.length > 1; + const displayAllActors = isReportPreviewAction && !isTripRoom; const isInvoiceReport = ReportUtils.isInvoiceReport(iouReport ?? null); const isWorkspaceActor = isInvoiceReport || (ReportUtils.isPolicyExpenseChat(report) && (!actorAccountID || displayAllActors)); const ownerAccountID = iouReport?.ownerAccountID ?? action?.childOwnerAccountID; - const managerID = iouReport?.managerID ?? action?.childManagerAccountID; let avatarSource = avatar; let avatarId: number | string | undefined = actorAccountID; @@ -131,9 +130,10 @@ function ReportActionItemSingle({ }; } else { // The ownerAccountID and actorAccountID can be the same if a user submits an expense back from the IOU's original creator, in that case we need to use managerID to avoid displaying the same user twice - const secondaryAccountId = ownerAccountID === actorAccountID || isInvoiceReport ? managerID : ownerAccountID; + const secondaryAccountId = ownerAccountID === actorAccountID || isInvoiceReport ? actorAccountID : ownerAccountID; const secondaryUserAvatar = personalDetails?.[secondaryAccountId ?? -1]?.avatar ?? FallbackAvatar; const secondaryDisplayName = ReportUtils.getDisplayNameForParticipant(secondaryAccountId); + secondaryAvatar = { source: secondaryUserAvatar, type: CONST.ICON_TYPE_AVATAR, @@ -150,41 +150,24 @@ function ReportActionItemSingle({ } else { secondaryAvatar = {name: '', source: '', type: 'avatar'}; } - - const icon = useMemo( - () => ({ - source: avatarSource ?? FallbackAvatar, - type: isWorkspaceActor ? CONST.ICON_TYPE_WORKSPACE : CONST.ICON_TYPE_AVATAR, - name: primaryDisplayName ?? '', - id: avatarId, - }), - [avatarSource, isWorkspaceActor, primaryDisplayName, avatarId], - ); + const icon = { + source: avatarSource ?? FallbackAvatar, + type: isWorkspaceActor ? CONST.ICON_TYPE_WORKSPACE : CONST.ICON_TYPE_AVATAR, + name: primaryDisplayName ?? '', + id: avatarId, + }; // Since the display name for a report action message is delivered with the report history as an array of fragments // we'll need to take the displayName from personal details and have it be in the same format for now. Eventually, // we should stop referring to the report history items entirely for this information. - const personArray = useMemo(() => { - const baseArray = displayName - ? [ - { - type: 'TEXT', - text: displayName, - }, - ] - : action?.person ?? []; - - if (displayAllActors) { - return [ - ...baseArray, - { - type: 'TEXT', - text: secondaryAvatar.name ?? '', - }, - ]; - } - return baseArray; - }, [displayName, action?.person, displayAllActors, secondaryAvatar?.name]); + const personArray = displayName + ? [ + { + type: 'TEXT', + text: displayName, + }, + ] + : action?.person; const reportID = report?.reportID; const iouReportID = iouReport?.reportID; @@ -209,130 +192,45 @@ function ReportActionItemSingle({ [action, isWorkspaceActor, actorAccountID], ); - const getAvatar = useMemo(() => { - return () => { - if (displayAllActors) { - return ( - - ); - } - if (shouldShowSubscriptAvatar) { - return ( - - ); - } + const getAvatar = () => { + if (displayAllActors) { return ( - - - - - + ); - }; - }, [ - displayAllActors, - shouldShowSubscriptAvatar, - actorAccountID, - action?.delegateAccountID, - icon, - styles.actionAvatar, - fallbackIcon, - icons, - StyleUtils, - theme.appBG, - theme.hoverComponentBG, - theme.componentBG, - isHovered, - secondaryAvatar, - ]); - - const getHeading = useMemo(() => { - return () => { - if (displayAllActors && secondaryAvatar.name && isReportPreviewAction) { - return ( - - - - {` & `} - - - - ); - } + } + if (shouldShowSubscriptAvatar) { return ( + + ); + } + return ( + - {personArray?.map((fragment) => ( - - ))} + - ); - }; - }, [ - displayAllActors, - secondaryAvatar, - isReportPreviewAction, - personArray, - styles.flexRow, - styles.flex1, - styles.chatItemMessageHeaderSender, - styles.pre, - action, - actorAccountID, - displayName, - icon, - ]); - + + ); + }; const hasEmojiStatus = !displayAllActors && status?.emojiCode; const formattedDate = DateUtils.getStatusUntilDate(status?.clearAfter ?? ''); const statusText = status?.text ?? ''; @@ -363,7 +261,18 @@ function ReportActionItemSingle({ accessibilityLabel={actorHint} role={CONST.ROLE.BUTTON} > - {getHeading()} + {personArray?.map((fragment, index) => ( + + ))} {!!hasEmojiStatus && ( @@ -384,5 +293,7 @@ function ReportActionItemSingle({ ); } + ReportActionItemSingle.displayName = 'ReportActionItemSingle'; + export default ReportActionItemSingle; diff --git a/src/pages/home/report/ReportActionsListItemRenderer.tsx b/src/pages/home/report/ReportActionsListItemRenderer.tsx index 63b2cb43d836..ff1c2431ca8b 100644 --- a/src/pages/home/report/ReportActionsListItemRenderer.tsx +++ b/src/pages/home/report/ReportActionsListItemRenderer.tsx @@ -171,7 +171,7 @@ function ReportActionsListItemRenderer({ displayAsGroup={displayAsGroup} shouldDisplayNewMarker={shouldDisplayNewMarker} shouldShowSubscriptAvatar={ - (ReportUtils.isPolicyExpenseChat(report) || ReportUtils.isInvoiceRoom(report)) && + ReportUtils.isPolicyExpenseChat(report) && [ CONST.REPORT.ACTIONS.TYPE.IOU, CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW,