-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fix IOU avatar overlap when sharing manual and scanned expenses #87961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ad5a635
d706e24
1dd2588
5b168b7
141b874
c9ed1ab
dbc5e98
306729f
99f96c5
cfc43d2
c5d45a8
f2f1465
9af8483
97fa714
c96f90e
da5ff2a
e598e53
2522c9f
c897d0f
5e87640
cf0a7c4
6bfc2ab
f11d00b
3f8d962
926ab7b
0ee0bb0
3a9837c
9a3d762
246db86
ee5287b
05cca00
c52150d
d25d9e7
b239a7a
ba06bc9
66131c2
921fb48
c75f048
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,12 @@ import {convertAttendeesToArray} from '@libs/AttendeeUtils'; | |
| import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; | ||
| import {getAllNonDeletedTransactions} from '@libs/MoneyRequestReportUtils'; | ||
| import {getPersonalDetailByEmail} from '@libs/PersonalDetailsUtils'; | ||
| import {getOriginalMessage, isMoneyRequestAction, isSentMoneyReportAction} from '@libs/ReportActionsUtils'; | ||
| import {getIOUActionForTransactionID, getOriginalMessage, isDeletedParentAction, isMoneyRequestAction, isSentMoneyReportAction} from '@libs/ReportActionsUtils'; | ||
| import {isDM, isIOUReport} from '@libs/ReportUtils'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import type {Policy, Report, ReportAction, ReportActions, Transaction} from '@src/types/onyx'; | ||
| import {hasOnceLoadedReportActionsSelector, isLoadingInitialReportActionsSelector} from '@src/selectors/ReportMetaData'; | ||
| import type {OriginalMessageIOU, Policy, Report, ReportAction, ReportActions, Transaction} from '@src/types/onyx'; | ||
|
|
||
| function getSplitAuthor(transaction: Transaction, splits?: Array<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>>) { | ||
| const {originalTransactionID, source} = transaction.comment ?? {}; | ||
|
|
@@ -39,6 +40,94 @@ const getSplitsSelector = (actions: OnyxEntry<ReportActions>): Array<ReportActio | |
| .filter((act) => getOriginalMessage(act)?.type === CONST.IOU.REPORT_ACTION_TYPE.SPLIT); | ||
| }; | ||
|
|
||
| function getTransactionDirectionSign(transaction: Transaction): number | undefined { | ||
| const modifiedAmount = Number(transaction.modifiedAmount); | ||
|
|
||
| if (Number.isFinite(modifiedAmount) && modifiedAmount !== 0) { | ||
| return Math.sign(modifiedAmount); | ||
| } | ||
|
|
||
| if (transaction.amount !== 0) { | ||
| return Math.sign(transaction.amount); | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| function hasPendingScanStateAndUnknownDirection(transaction: Transaction): boolean { | ||
| if (transaction.receipt?.source === undefined) { | ||
| return false; | ||
| } | ||
|
|
||
| if (transaction.receipt?.state !== CONST.IOU.RECEIPT_STATE.SCAN_READY && transaction.receipt?.state !== CONST.IOU.RECEIPT_STATE.SCANNING) { | ||
| return false; | ||
| } | ||
|
|
||
| if (getTransactionDirectionSign(transaction) !== undefined) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| function getPendingScanActorAccountID(transaction: Transaction, action: OnyxEntry<ReportAction>, iouReport: OnyxEntry<Report>, chatReport: OnyxEntry<Report>): number | undefined { | ||
| if (!hasPendingScanStateAndUnknownDirection(transaction)) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const chatLastActorAccountID = Number(chatReport?.lastActorAccountID); | ||
| const validPendingScanActorAccountIDs = new Set([action?.childOwnerAccountID, action?.childManagerAccountID, iouReport?.ownerAccountID, iouReport?.managerID].filter(Boolean)); | ||
|
|
||
| if ( | ||
| Number.isFinite(chatLastActorAccountID) && | ||
| chatLastActorAccountID > 0 && | ||
| chatLastActorAccountID !== CONST.ACCOUNT_ID.CONCIERGE && | ||
| validPendingScanActorAccountIDs.has(chatLastActorAccountID) | ||
| ) { | ||
| return chatLastActorAccountID; | ||
| } | ||
|
|
||
| const previewLastActorAccountID = Number(action?.childLastActorAccountID); | ||
|
|
||
| if (!Number.isFinite(previewLastActorAccountID) || previewLastActorAccountID <= 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return previewLastActorAccountID; | ||
| } | ||
|
|
||
| function getAccountIDFromTransactionDirection(transaction: Transaction, action: OnyxEntry<ReportAction>, iouReport: OnyxEntry<Report>): number | undefined { | ||
| const directionSign = getTransactionDirectionSign(transaction); | ||
|
|
||
| if (directionSign === undefined) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const accountID = Number(directionSign > 0 ? (action?.childOwnerAccountID ?? iouReport?.ownerAccountID) : (action?.childManagerAccountID ?? iouReport?.managerID)); | ||
|
|
||
| if (!Number.isFinite(accountID) || accountID <= 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return accountID; | ||
| } | ||
|
|
||
| function isExplicitlyDeletedIOUAction(iouAction: ReportAction): boolean { | ||
| const originalMessage = getOriginalMessage(iouAction) as OriginalMessageIOU | undefined; | ||
|
|
||
| if (originalMessage?.deleted || isDeletedParentAction(iouAction) || iouAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { | ||
| return true; | ||
| } | ||
|
|
||
| const message = iouAction.message; | ||
|
|
||
| if (Array.isArray(message)) { | ||
| return message.some((fragment) => !!fragment?.deleted); | ||
| } | ||
|
Comment on lines
+124
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated the deleted-action handling so IOU actions with |
||
|
|
||
| return !!message?.deleted; | ||
| } | ||
|
|
||
| type GetReportPreviewSenderIDParams = { | ||
| iouReport: OnyxEntry<Report>; | ||
| action: OnyxEntry<ReportAction>; | ||
|
|
@@ -48,25 +137,107 @@ type GetReportPreviewSenderIDParams = { | |
| splits: Array<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>> | undefined; | ||
| policy: OnyxEntry<Policy>; | ||
| currentUserAccountID: number; | ||
| hasFinishedInitialReportActionsLoad?: boolean; | ||
| }; | ||
|
|
||
| function getReportPreviewSenderID({iouReport, action, chatReport, iouActions, transactions, splits, policy, currentUserAccountID}: GetReportPreviewSenderIDParams): number | undefined { | ||
| function getReportPreviewSenderID({ | ||
| iouReport, | ||
| action, | ||
| chatReport, | ||
| iouActions, | ||
| transactions, | ||
| splits, | ||
| policy, | ||
| currentUserAccountID, | ||
| hasFinishedInitialReportActionsLoad, | ||
| }: GetReportPreviewSenderIDParams): number | undefined { | ||
| const isOptimisticReportPreview = action?.isOptimisticAction && action?.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW && isIOUReport(iouReport); | ||
|
|
||
| if (isOptimisticReportPreview) { | ||
| return currentUserAccountID; | ||
| } | ||
| const loadedTransactionCount = transactions?.length ?? 0; | ||
| const childMoneyRequestCount = action?.childMoneyRequestCount ?? 0; | ||
| const activeMoneyRequestCount = iouReport?.transactionCount ?? childMoneyRequestCount; | ||
| const activeIOUActions = iouActions?.filter((iouAction) => !isExplicitlyDeletedIOUAction(iouAction)) ?? []; | ||
| const uniqueIOUActionActorMap = new Map<string, number>(); | ||
|
|
||
| // 1. If all amounts have the same sign - either all amounts are positive or all amounts are negative. | ||
| // We have to do it this way because there can be a case when actions are not available | ||
| // See: https://github.com/Expensify/App/pull/64802#issuecomment-3008944401 | ||
| for (const iouAction of activeIOUActions) { | ||
| const iouTransactionID = (getOriginalMessage(iouAction) as OriginalMessageIOU | undefined)?.IOUTransactionID; | ||
|
|
||
| const areAmountsSignsTheSame = new Set(transactions?.map((tr) => Math.sign(tr.amount))).size < 2; | ||
| if (!iouTransactionID || iouAction.actorAccountID === undefined) { | ||
| continue; | ||
| } | ||
|
|
||
| uniqueIOUActionActorMap.set(iouTransactionID, iouAction.actorAccountID); | ||
| } | ||
|
|
||
| if (!areAmountsSignsTheSame) { | ||
| const hasCompleteActionCoverage = activeMoneyRequestCount > 0 && uniqueIOUActionActorMap.size >= activeMoneyRequestCount; | ||
| const areAllActiveChildRequestsCreatedByOneActor = uniqueIOUActionActorMap.size > 0 && new Set(uniqueIOUActionActorMap.values()).size < 2; | ||
| const canInferFromIOUActionsDuringPartialHydration = loadedTransactionCount > 0 && hasCompleteActionCoverage && activeIOUActions.length > 0 && areAllActiveChildRequestsCreatedByOneActor; | ||
|
|
||
| // After refresh, the preview action can hydrate before all active child transactions. | ||
| // Avoid collapsing to one avatar unless the available IOU actions already prove the remaining | ||
| // active requests all belong to the same sender. | ||
| if (!hasFinishedInitialReportActionsLoad && activeMoneyRequestCount > loadedTransactionCount && !canInferFromIOUActionsDuringPartialHydration) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const transactionActorAccountIDs = transactions?.map((transaction) => { | ||
| return getIOUActionForTransactionID(activeIOUActions, transaction.transactionID)?.actorAccountID ?? getPendingScanActorAccountID(transaction, action, iouReport, chatReport); | ||
| }); | ||
| const transactionSigns = transactions?.map((transaction) => getTransactionDirectionSign(transaction)) ?? []; | ||
|
|
||
| const hasActorAccountIDForEachTransaction = | ||
| activeIOUActions.length > 0 && !!transactionActorAccountIDs && transactionActorAccountIDs.length > 0 && transactionActorAccountIDs.every((accountID) => accountID !== undefined); | ||
|
|
||
| // 1. Use actorAccountID when it is available for every transaction. Otherwise, fall back to known transaction direction only. | ||
| if (hasActorAccountIDForEachTransaction) { | ||
| const areAllTransactionsCreatedByOneActor = new Set(transactionActorAccountIDs).size < 2; | ||
|
|
||
| if (!areAllTransactionsCreatedByOneActor) { | ||
| return undefined; | ||
| } | ||
| } else { | ||
| const transactionsWithUnknownDirection = (transactions ?? []).filter((transaction, index) => transactionSigns.at(index) === undefined); | ||
| const hasUnknownDirection = transactionSigns.some((sign) => sign === undefined); | ||
| const unknownDirectionComesOnlyFromPendingScans = transactionsWithUnknownDirection.length > 0 && transactionsWithUnknownDirection.every(hasPendingScanStateAndUnknownDirection); | ||
| const hasOnlyUnknownDirections = transactionSigns.length > 0 && transactionSigns.every((sign) => sign === undefined); | ||
| const hasOnlyUnknownNonPendingScanDirections = | ||
| hasOnlyUnknownDirections && transactionsWithUnknownDirection.every((transaction) => !hasPendingScanStateAndUnknownDirection(transaction)); | ||
|
|
||
| if (unknownDirectionComesOnlyFromPendingScans) { | ||
| const inferredActorAccountIDs = (transactions ?? []) | ||
| .map((transaction, index) => { | ||
| return transactionActorAccountIDs?.at(index) ?? getAccountIDFromTransactionDirection(transaction, action, iouReport); | ||
| }) | ||
| .filter((accountID): accountID is number => accountID !== undefined); | ||
|
|
||
| if (new Set(inferredActorAccountIDs).size > 1) { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| if (hasUnknownDirection && !unknownDirectionComesOnlyFromPendingScans && !hasOnlyUnknownNonPendingScanDirections) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const knownTransactionSigns = transactionSigns.filter((sign): sign is number => sign !== undefined); | ||
|
|
||
| if (knownTransactionSigns.length === 0 && !hasOnlyUnknownNonPendingScanDirections) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // 1. If all amounts have the same sign - either all amounts are positive or all amounts are negative. | ||
| // We have to do it this way because there can be a case when actions are not available. | ||
| // See: https://github.com/Expensify/App/pull/64802#issuecomment-3008944401 | ||
| const areAmountsSignsTheSame = hasOnlyUnknownNonPendingScanDirections || new Set(knownTransactionSigns).size < 2; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
|
|
||
| if (!areAmountsSignsTheSame) { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| // 2. If there is only one attendee - we check that by counting unique emails converted to account IDs in the attendees list. | ||
| // This is a fallback added because: https://github.com/Expensify/App/pull/64802#issuecomment-3007906310 | ||
|
|
||
|
|
@@ -86,13 +257,13 @@ function getReportPreviewSenderID({iouReport, action, chatReport, iouActions, tr | |
| } | ||
|
|
||
| // If the action is a 'Send Money' flow, it will only have one transaction, but the person who sent the money is the child manager account, not the child owner account. | ||
| const isSendMoneyFlowBasedOnActions = !!iouActions && iouActions.every(isSentMoneyReportAction); | ||
| const isSendMoneyFlowBasedOnActions = activeIOUActions.length > 0 && activeIOUActions.every(isSentMoneyReportAction); | ||
| // This is used only if there are no IOU actions in the Onyx | ||
| // eslint-disable-next-line rulesdir/no-negated-variables | ||
| const isSendMoneyFlowBasedOnTransactions = | ||
| !!action && action.childMoneyRequestCount === 0 && transactions?.length === 1 && (chatReport ? isDM(chatReport) : policy?.type === CONST.POLICY.TYPE.PERSONAL); | ||
|
|
||
| const isSendMoneyFlow = !!iouActions && iouActions?.length > 0 ? isSendMoneyFlowBasedOnActions : isSendMoneyFlowBasedOnTransactions; | ||
| const isSendMoneyFlow = activeIOUActions.length > 0 ? isSendMoneyFlowBasedOnActions : isSendMoneyFlowBasedOnTransactions; | ||
|
|
||
| const singleAvatarAccountID = isSendMoneyFlow ? action?.childManagerAccountID : action?.childOwnerAccountID; | ||
|
|
||
|
|
@@ -108,14 +279,33 @@ function useReportPreviewSenderID({iouReport, action, chatReport}: {action: Onyx | |
| const [iouActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(shouldFetchData ? iouReport?.reportID : undefined)}`, { | ||
| selector: getIOUActionsSelector, | ||
| }); | ||
| const [hasOnceLoadedReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${getNonEmptyStringOnyxID(shouldFetchData ? action?.childReportID : undefined)}`, { | ||
| selector: hasOnceLoadedReportActionsSelector, | ||
| }); | ||
| const [isLoadingInitialReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${getNonEmptyStringOnyxID(shouldFetchData ? action?.childReportID : undefined)}`, { | ||
| selector: isLoadingInitialReportActionsSelector, | ||
| }); | ||
| const hasFinishedInitialReportActionsLoad = hasOnceLoadedReportActions === true || isLoadingInitialReportActions === false; | ||
|
|
||
| const {transactions: reportTransactions} = useTransactionsAndViolationsForReport(shouldFetchData ? action?.childReportID : undefined); | ||
| const transactions = useMemo(() => { | ||
| if (!shouldFetchData) { | ||
| return undefined; | ||
| } | ||
| return getAllNonDeletedTransactions(reportTransactions, iouActions ?? []); | ||
| }, [reportTransactions, iouActions, shouldFetchData]); | ||
| const activeMoneyRequestCount = iouReport?.transactionCount ?? action?.childMoneyRequestCount ?? 0; | ||
| const allReportTransactions = Object.values(reportTransactions ?? {}).filter((transaction): transaction is Transaction => !!transaction); | ||
| const nonDeletedTransactionsIncludingOrphans = getAllNonDeletedTransactions(reportTransactions, iouActions ?? [], false, true); | ||
| const filteredTransactions = | ||
| nonDeletedTransactionsIncludingOrphans.length < allReportTransactions.length | ||
| ? getAllNonDeletedTransactions(reportTransactions, iouActions ?? []) | ||
| : nonDeletedTransactionsIncludingOrphans; | ||
|
|
||
| if (filteredTransactions.length < allReportTransactions.length && filteredTransactions.length < activeMoneyRequestCount) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add comment why this condition is needed |
||
| return nonDeletedTransactionsIncludingOrphans; | ||
| } | ||
|
|
||
| return filteredTransactions; | ||
| }, [reportTransactions, iouActions, shouldFetchData, iouReport?.transactionCount, action?.childMoneyRequestCount]); | ||
|
|
||
| const [splits] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(shouldFetchData ? chatReport?.reportID : undefined)}`, { | ||
| selector: getSplitsSelector, | ||
|
|
@@ -124,7 +314,7 @@ function useReportPreviewSenderID({iouReport, action, chatReport}: {action: Onyx | |
| const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${getNonEmptyStringOnyxID(shouldFetchData ? iouReport?.policyID : undefined)}`); | ||
| const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); | ||
|
|
||
| return getReportPreviewSenderID({iouReport, action, chatReport, iouActions, transactions, splits, policy, currentUserAccountID}); | ||
| return getReportPreviewSenderID({iouReport, action, chatReport, iouActions, transactions, splits, policy, currentUserAccountID, hasFinishedInitialReportActionsLoad}); | ||
| } | ||
|
|
||
| export default useReportPreviewSenderID; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should modifiedAmount take precedence over amount?