Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 40 additions & 7 deletions src/components/ReportActionAvatars/useReportPreviewSenderID.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import useTransactionsAndViolationsForReport from '@hooks/useTransactionsAndViol
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, isMoneyRequestAction, isSentMoneyReportAction} from '@libs/ReportActionsUtils';
import {isDM, isIOUReport} from '@libs/ReportUtils';
import {isScanRequest} from '@libs/TransactionUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Policy, Report, ReportAction, ReportActions, Transaction} from '@src/types/onyx';
Expand Down Expand Up @@ -38,6 +39,22 @@ const getSplitsSelector = (actions: OnyxEntry<ReportActions>): Array<ReportActio
.filter((act) => getOriginalMessage(act)?.type === CONST.IOU.REPORT_ACTION_TYPE.SPLIT);
};

function getTransactionDirectionSign(transaction: Transaction): number | undefined {
if (transaction.amount !== 0) {
return Math.sign(transaction.amount);
}

if (isScanRequest(transaction)) {
const modifiedAmount = Number(transaction.modifiedAmount);

if (Number.isFinite(modifiedAmount) && modifiedAmount !== 0) {
return Math.sign(modifiedAmount);
}
}

return undefined;
}

type GetReportPreviewSenderIDParams = {
iouReport: OnyxEntry<Report>;
action: OnyxEntry<ReportAction>;
Expand All @@ -56,14 +73,30 @@ function getReportPreviewSenderID({iouReport, action, chatReport, iouActions, tr
return currentUserAccountID;
}

// 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 transactionActorAccountIDs = transactions?.map((transaction) => getIOUActionForTransactionID(iouActions ?? [], transaction.transactionID)?.actorAccountID);
const hasActorAccountIDForEachTransaction =
!!iouActions?.length && !!transactionActorAccountIDs && transactionActorAccountIDs.length > 0 && transactionActorAccountIDs.every((accountID) => accountID !== undefined);

const areAmountsSignsTheSame = new Set(transactions?.map((tr) => Math.sign(tr.amount))).size < 2;
// 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 (!areAmountsSignsTheSame) {
return undefined;
if (!areAllTransactionsCreatedByOneActor) {
return undefined;
Comment on lines +89 to +92
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid treating delegate actors as distinct report senders

This new actor-based gate can hide the single-avatar state for valid single-sender previews when requests are submitted by different delegates (or by a delegate plus the owner). In those flows, actorAccountID varies per action even though the preview sender should still resolve to the same childOwnerAccountID/childManagerAccountID, so the early return undefined incorrectly forces multi-avatar rendering. This regression is introduced by using raw actor IDs as the primary sameness check.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this case in the codebase and I don't think this concern applies here.

For IOU actions, delegate/copilot identity is stored separately in delegateAccountID, while the IOU action itself still uses actorAccountID for the main account:

actorAccountID: deprecatedCurrentUserAccountID,

/** The accountID of the copilot who took this action on behalf of the user */
delegateAccountID?: number;

The preview logic in ReportActionItemSingle also already treats delegate separately from the main sender account:

const delegateAccountID = getDelegateAccountIDFromReportAction(action);

So I don’t see evidence that delegate-created IOU actions would cause different actorAccountIDs for the same visible sender in this flow.

}
} else {
const transactionSigns = transactions?.map((transaction) => getTransactionDirectionSign(transaction)) ?? [];
const hasUnknownDirection = transactionSigns.some((sign) => sign === undefined);

if (hasUnknownDirection) {
return undefined;
}

const areAmountsSignsTheSame = new Set(transactionSigns).size < 2;

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.
Expand Down
Loading