Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ function MoneyRequestReportActionsList({
hasNextActionMadeBySameActor(visibleReportActions, index);

const actionEmojiReactions = emojiReactions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}${reportAction.reportActionID}`];
const originalReportID = getOriginalReportID(report.reportID, reportAction);
const originalReportID = getOriginalReportID(report.reportID, reportAction, undefined);
const reportDraftMessages = draftMessage?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`];
const matchingDraftMessage = reportDraftMessages?.[reportAction.reportActionID];
const matchingDraftMessageString = matchingDraftMessage?.message;
Expand Down
2 changes: 1 addition & 1 deletion src/components/ShowContextMenuContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function showContextMenuForReport(
contextMenuAnchor: anchor,
report: {
reportID,
originalReportID: reportID ? getOriginalReportID(reportID, action) : undefined,
originalReportID: reportID ? getOriginalReportID(reportID, action, undefined) : undefined,
isArchivedRoom,
},
reportAction: {
Expand Down
15 changes: 2 additions & 13 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@

let conciergeReportIDOnyxConnect: OnyxEntry<string>;
Onyx.connect({
key: ONYXKEYS.CONCIERGE_REPORT_ID,

Check warning on line 995 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
callback: (value) => {
conciergeReportIDOnyxConnect = value;
},
Expand All @@ -1000,7 +1000,7 @@

const defaultAvatarBuildingIconTestID = 'SvgDefaultAvatarBuilding Icon';
Onyx.connect({
key: ONYXKEYS.SESSION,

Check warning on line 1003 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
callback: (value) => {
// When signed out, val is undefined
if (!value) {
Expand All @@ -1018,7 +1018,7 @@
let allPersonalDetailLogins: string[];
let currentUserPersonalDetails: OnyxEntry<PersonalDetails>;
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,

Check warning on line 1021 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
callback: (value) => {
if (currentUserAccountID) {
currentUserPersonalDetails = value?.[currentUserAccountID] ?? undefined;
Expand All @@ -1030,7 +1030,7 @@

let allReportsDraft: OnyxCollection<Report>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_DRAFT,

Check warning on line 1033 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
waitForCollectionCallback: true,
callback: (value) => (allReportsDraft = value),
});
Expand All @@ -1038,7 +1038,7 @@
let allPolicies: OnyxCollection<Policy>;
let hasPolicies: boolean;
Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY,

Check warning on line 1041 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
waitForCollectionCallback: true,
callback: (value) => {
allPolicies = value;
Expand All @@ -1048,7 +1048,7 @@

let allPolicyDrafts: OnyxCollection<Policy>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY_DRAFTS,

Check warning on line 1051 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
waitForCollectionCallback: true,
callback: (value) => (allPolicyDrafts = value),
});
Expand All @@ -1056,7 +1056,7 @@
let allReports: OnyxCollection<Report>;
let reportsByPolicyID: ReportByPolicyMap;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,

Check warning on line 1059 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
waitForCollectionCallback: true,
callback: (value) => {
allReports = value;
Expand Down Expand Up @@ -1092,14 +1092,14 @@

let betaConfiguration: OnyxEntry<BetaConfiguration> = {};
Onyx.connect({
key: ONYXKEYS.BETA_CONFIGURATION,

Check warning on line 1095 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
callback: (value) => (betaConfiguration = value ?? {}),
});

let allTransactions: OnyxCollection<Transaction> = {};
let reportsTransactions: Record<string, Transaction[]> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION,

Check warning on line 1102 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
waitForCollectionCallback: true,
callback: (value) => {
if (!value) {
Expand All @@ -1125,7 +1125,7 @@

let allReportActions: OnyxCollection<ReportActions>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,

Check warning on line 1128 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
waitForCollectionCallback: true,
callback: (actions) => {
if (!actions) {
Expand Down Expand Up @@ -9960,11 +9960,11 @@
/**
* Returns ID of the original report from which the given reportAction is first created.
*/
function getOriginalReportID(reportID: string | undefined, reportAction: OnyxInputOrEntry<ReportAction>): string | undefined {
function getOriginalReportID(reportID: string | undefined, reportAction: OnyxInputOrEntry<ReportAction>, reportActionsParam: OnyxEntry<ReportActions> | undefined): string | undefined {
if (!reportID) {
return undefined;
}
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`];
const reportActions = reportActionsParam ?? allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`];
const currentReportAction = reportAction?.reportActionID ? reportActions?.[reportAction.reportActionID] : undefined;
const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`];
Expand Down Expand Up @@ -10807,16 +10807,6 @@
return !hasCreditBankAccount(bankAccountList) ? 'bankAccount' : undefined;
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was 100% dead code, so easy to remove.

* Checks if report chat contains missing payment method
*/
function hasMissingPaymentMethod(userWalletTierName: string | undefined, iouReportID: string | undefined, bankAccountList: OnyxEntry<BankAccountList>): boolean {
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportID}`] ?? {};
return Object.values(reportActions)
.filter(Boolean)
.some((action) => getIndicatedMissingPaymentMethod(userWalletTierName, iouReportID, action, bankAccountList) !== undefined);
}

/**
* Used from expense actions to decide if we need to build an optimistic expense report.
* Create a new report if:
Expand Down Expand Up @@ -12919,7 +12909,6 @@
hasExpensifyGuidesEmails,
hasHeldExpenses,
hasIOUWaitingOnCurrentUserBankAccount,
hasMissingPaymentMethod,
hasNonReimbursableTransactions,
hasOnlyHeldExpenses,
hasOnlyTransactionsWithPendingRoutes,
Expand Down
8 changes: 4 additions & 4 deletions src/libs/actions/Report/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2092,7 +2092,7 @@ function deleteReportComment(
currentEmail: string,
) {
const reportID = report?.reportID;
const originalReportID = getOriginalReportID(reportID, reportAction);
const originalReportID = getOriginalReportID(reportID, reportAction, undefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ [PERF-2] Return early before expensive work (docs)

The function calls getOriginalReportID() (line 2095) before checking if reportAction.reportActionID exists (line 2098). getOriginalReportID() performs expensive operations (Onyx reads, function calls) that will be wasted if the validation fails.

Fix: Extract simple checks first, then perform expensive work:

const reportID = report?.reportID;
const reportActionID = reportAction.reportActionID;

if (\!reportActionID || \!reportID) {
    return;
}

const originalReportID = getOriginalReportID(reportID, reportAction, undefined);

if (\!originalReportID) {
    return;
}

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is almost never a case where reportAction.reportActionID wouldn't exist, so this is not a valuable optimization.

const reportActionID = reportAction.reportActionID;

if (!reportActionID || !originalReportID || !reportID) {
Expand Down Expand Up @@ -2420,13 +2420,13 @@ function editReportComment(

/** Deletes the draft for a comment report action. */
function deleteReportActionDraft(reportID: string | undefined, reportAction: ReportAction) {
const originalReportID = getOriginalReportID(reportID, reportAction);
const originalReportID = getOriginalReportID(reportID, reportAction, undefined);
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`, {[reportAction.reportActionID]: null});
}

/** Saves the draft for a comment report action. This will put the comment into "edit mode" */
function saveReportActionDraft(reportID: string | undefined, reportAction: ReportAction, draftMessage: string) {
const originalReportID = getOriginalReportID(reportID, reportAction);
const originalReportID = getOriginalReportID(reportID, reportAction, undefined);
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`, {[reportAction.reportActionID]: {message: draftMessage}});
}

Expand Down Expand Up @@ -3761,7 +3761,7 @@ function toggleEmojiReaction(
currentUserAccountID: number,
ignoreSkinToneOnCompare = false,
) {
const originalReportID = getOriginalReportID(reportID, reportAction);
const originalReportID = getOriginalReportID(reportID, reportAction, undefined);

if (!originalReportID) {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/ReportActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Onyx.connect({
});

function clearReportActionErrors(reportID: string, reportAction: ReportAction, keys?: string[]) {
const originalReportID = getOriginalReportID(reportID, reportAction);
const originalReportID = getOriginalReportID(reportID, reportAction, undefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ [PERF-2] Return early before expensive work (docs)

The function calls getOriginalReportID() before checking if reportAction?.reportActionID exists. getOriginalReportID() performs expensive operations (Onyx reads, function calls) that will be wasted if reportActionID is missing.

Fix: Move the simple validation check before the expensive work:

function clearReportActionErrors(reportID: string, reportAction: ReportAction, keys?: string[]) {
    if (!reportAction?.reportActionID) {
        return;
    }

    const originalReportID = getOriginalReportID(reportID, reportAction, undefined);
    // ... rest of function
}

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is almost never a case where reportAction.reportActionID wouldn't exist, so this is not a valuable optimization.


if (!reportAction?.reportActionID) {
return;
Expand Down
3 changes: 2 additions & 1 deletion src/pages/FlagCommentPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ function FlagCommentPage({parentReportAction, route, report, parentReport, repor
if (isChatThread(report) && reportAction?.reportActionID === parentReportAction?.reportActionID) {
reportID = parentReport?.reportID;
}
const originalReportID = getOriginalReportID(reportID, reportAction);
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {canBeMissing: true});
const originalReportID = getOriginalReportID(reportID, reportAction, reportActions);
const [originalReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${originalReportID}`, {canBeMissing: true});
const isOriginalReportArchived = useReportIsArchived(originalReportID);

Expand Down
4 changes: 3 additions & 1 deletion src/pages/ReportDetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ function ReportDetailsPage({policy, report, route, reportMetadata}: ReportDetail
const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID, {canBeMissing: true});

const {reportActions} = usePaginatedReportActions(report.reportID);
const [reportActionsForOriginalReportID] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, {canBeMissing: true});

const {removeTransaction} = useSearchContext();

Expand Down Expand Up @@ -439,7 +440,7 @@ function ReportDetailsPage({policy, report, route, reportMetadata}: ReportDetail
}

if (isTrackExpenseReport && !isDeletedParentAction) {
const actionReportID = getOriginalReportID(report.reportID, parentReportAction);
const actionReportID = getOriginalReportID(report.reportID, parentReportAction, reportActionsForOriginalReportID);
const whisperAction = getTrackExpenseActionableWhisper(iouTransactionID, moneyRequestReport?.reportID);
Comment on lines 442 to 444

Choose a reason for hiding this comment

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

P2 Badge Include reportActionsForOriginalReportID in menuItems deps

The menuItems useMemo now uses reportActionsForOriginalReportID when calling getOriginalReportID, but the dependency list omits reportActionsForOriginalReportID. If the report actions collection changes after initial render, the memoized actionReportID used by the Track Expense actions can become stale, leading to drafts/navigations targeting the wrong report. Include reportActionsForOriginalReportID in the dependency array so the menu recomputes when actions load or change.

Useful? React with 👍 / 👎.

const actionableWhisperReportActionID = whisperAction?.reportActionID;
items.push({
Expand Down Expand Up @@ -621,6 +622,7 @@ function ReportDetailsPage({policy, report, route, reportMetadata}: ReportDetail
allTransactionDrafts,
activePolicy,
parentReport,
reportActionsForOriginalReportID,
]);

const displayNamesWithTooltips = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function DuplicateTransactionItem({transaction, index, allReports, policies, onP
return IOUTransactionID === transaction?.transactionID;
});

const originalReportID = getOriginalReportID(report?.reportID, action);
const originalReportID = getOriginalReportID(report?.reportID, action, reportActions);

const [draftMessage] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`, {
canBeMissing: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ function BaseReportActionContextMenu({
// eslint-disable-next-line @typescript-eslint/non-nullable-type-assertion-style
reportAction: (reportAction ?? null) as ReportAction,
reportID,
originalReportID,
report,
draftMessage,
selection,
Expand Down
8 changes: 3 additions & 5 deletions src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ import {
getIOUReportActionDisplayMessage,
getMovedActionMessage,
getMovedTransactionMessage,
getOriginalReportID,
getPolicyChangeMessage,
getReimbursementDeQueuedOrCanceledActionMessage,
getReimbursementQueuedActionMessage,
Expand Down Expand Up @@ -239,6 +238,7 @@ type ContextMenuActionPayload = {
reportAction: ReportAction;
transaction?: OnyxEntry<Transaction>;
reportID: string | undefined;
originalReportID: string | undefined;
currentUserAccountID: number;
report: OnyxEntry<ReportType>;
policy?: OnyxEntry<Policy>;
Expand Down Expand Up @@ -460,12 +460,11 @@ const ContextMenuActions: ContextMenuAction[] = [

return hasReasoning(reportAction);
},
onPress: (closePopover, {reportAction, reportID, translate, currentUserPersonalDetails}) => {
onPress: (closePopover, {reportAction, reportID, originalReportID, translate, currentUserPersonalDetails}) => {
if (!reportID) {
return;
}

const originalReportID = getOriginalReportID(reportID, reportAction);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In both of these cases here, I was able to remove the call to getOriginalReportID() because the originalReportID already existed in the base context, it could be passed down directly.

if (closePopover) {
hideContextMenu(false, () => {
KeyboardUtils.dismiss().then(() => {
Expand Down Expand Up @@ -1084,8 +1083,7 @@ const ContextMenuActions: ContextMenuAction[] = [
const isDynamicWorkflowRoutedAction = isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.DYNAMIC_EXTERNAL_WORKFLOW_ROUTED);
return type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && !isAttachmentTarget && !isMessageDeleted(reportAction) && !isDynamicWorkflowRoutedAction;
},
onPress: (closePopover, {reportAction, reportID}) => {
const originalReportID = getOriginalReportID(reportID, reportAction);
onPress: (closePopover, {reportAction, originalReportID}) => {
getEnvironmentURL().then((environmentURL) => {
const reportActionID = reportAction?.reportActionID;
Clipboard.setString(`${environmentURL}/r/${originalReportID}/${reportActionID}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ function PopoverReportActionContextMenu({ref}: PopoverReportActionContextMenuPro
const selectionRef = useRef('');
const reportActionDraftMessageRef = useRef<string | undefined>(undefined);
const isReportArchived = useReportIsArchived(reportIDRef.current);
const isOriginalReportArchived = useReportIsArchived(getOriginalReportID(reportIDRef.current, reportActionRef.current));
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportIDRef.current}`, {canBeMissing: true});
const isOriginalReportArchived = useReportIsArchived(getOriginalReportID(reportIDRef.current, reportActionRef.current, reportActions));
const {iouReport, chatReport, isChatIOUReportArchived} = useGetIOUReportFromReportAction(reportActionRef.current);
const {transitionActionSheetState} = useActionSheetAwareScrollViewActions();

Expand Down Expand Up @@ -332,7 +333,7 @@ function PopoverReportActionContextMenu({ref}: PopoverReportActionContextMenuPro
policy,
});

const [originalReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getOriginalReportID(reportIDRef.current, reportActionRef.current)}`, {
const [originalReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getOriginalReportID(reportIDRef.current, reportActionRef.current, reportActions)}`, {
canBeMissing: true,
});
const ancestorsRef = useRef<typeof ancestors>([]);
Expand Down
3 changes: 2 additions & 1 deletion src/pages/inbox/report/ReportActionItemMessageEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,9 @@ function ReportActionItemMessageEdit({
// The ref to check whether the comment saving is in progress
const isCommentPendingSaved = useRef(false);
const [originalReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${originalReportID}`, {canBeMissing: true});
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`, {canBeMissing: true});
const isOriginalReportArchived = useReportIsArchived(originalReportID);
const originalParentReportID = getOriginalReportID(originalReportID, action);
const originalParentReportID = getOriginalReportID(originalReportID, action, reportActions);
const isOriginalParentReportArchived = useReportIsArchived(originalParentReportID);
const ancestors = useAncestors(originalReport);
const icons = useMemoizedLazyExpensifyIcons(['Checkmark', 'Close']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import type {OnyxEntry} from 'react-native-onyx';
import RenderHTML from '@components/RenderHTML';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import {explain} from '@libs/actions/Report';
import {hasReasoning} from '@libs/ReportActionsUtils';
import {getOriginalReportID} from '@libs/ReportUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {ReportAction} from '@src/types/onyx';
import ReportActionItemBasicMessage from './ReportActionItemBasicMessage';

Expand All @@ -29,6 +31,7 @@ type ReportActionItemMessageWithExplainProps = {
function ReportActionItemMessageWithExplain({message, action, reportID}: ReportActionItemMessageWithExplainProps) {
const {translate} = useLocalize();
const personalDetail = useCurrentUserPersonalDetails();
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {canBeMissing: true});

const actionHasReasoning = hasReasoning(action);
const computedMessage = actionHasReasoning ? `${message}${translate('iou.AskToExplain')}` : message;
Expand All @@ -39,10 +42,10 @@ function ReportActionItemMessageWithExplain({message, action, reportID}: ReportA
return;
}

const actionOriginalReportID = getOriginalReportID(reportID, action);
const actionOriginalReportID = getOriginalReportID(reportID, action, reportActions);
explain(action, actionOriginalReportID, translate, personalDetail?.timezone);
},
[action, reportID, translate, personalDetail?.timezone],
[action, reportID, reportActions, translate, personalDetail?.timezone],
);

return (
Expand Down
2 changes: 1 addition & 1 deletion src/pages/inbox/report/ReportActionItemParentAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ function ReportActionItemParentAction({
const shouldDisplayThreadDivider = !isTripPreview(ancestorReportAction);
const isAncestorReportArchived = isArchivedReport(ancestorsReportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${ancestorReport.reportID}`]);

const originalReportID = getOriginalReportID(ancestorReport.reportID, ancestorReportAction);
const originalReportID = getOriginalReportID(ancestorReport.reportID, ancestorReportAction, undefined);
const reportDraftMessages = originalReportID ? allDraftMessages?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`] : undefined;
const matchingDraftMessage = reportDraftMessages?.[ancestorReportAction.reportActionID];
const matchingDraftMessageString = matchingDraftMessage?.message;
Expand Down
4 changes: 3 additions & 1 deletion src/pages/inbox/report/ReportActionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ function ReportActionsList({
const [isUserValidated] = useOnyx(ONYXKEYS.ACCOUNT, {selector: isUserValidatedSelector, canBeMissing: true});
const [draftMessage] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}`, {canBeMissing: true});
const [emojiReactions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_REACTIONS}`, {canBeMissing: true});
const [reportActionsFromOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, {canBeMissing: true});
const [userBillingFundID] = useOnyx(ONYXKEYS.NVP_BILLING_FUND_ID, {canBeMissing: true});
const [tryNewDot] = useOnyx(ONYXKEYS.NVP_TRY_NEW_DOT, {canBeMissing: false});
const isTryNewDotNVPDismissed = !!tryNewDot?.classicRedirect?.dismissed;
Expand Down Expand Up @@ -692,7 +693,7 @@ function ReportActionsList({

const renderItem = useCallback(
({item: reportAction, index}: ListRenderItemInfo<OnyxTypes.ReportAction>) => {
const originalReportID = getOriginalReportID(report.reportID, reportAction);
const originalReportID = getOriginalReportID(report.reportID, reportAction, reportActionsFromOnyx);
const reportDraftMessages = draftMessage?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${originalReportID}`];
Comment on lines 694 to 697

Choose a reason for hiding this comment

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

P2 Badge Recompute renderItem when reportActionsFromOnyx changes

The renderItem callback now depends on reportActionsFromOnyx to compute originalReportID, but it is not listed in the useCallback dependency array. When the report actions collection updates (e.g., Onyx loads/evicts actions after initial render), the callback will keep using a stale collection and can compute the wrong originalReportID, which then indexes drafts/emoji reactions for the wrong report. Add reportActionsFromOnyx to the dependency list (or derive originalReportID outside the callback) so it stays in sync with Onyx updates.

Useful? React with 👍 / 👎.

const matchingDraftMessage = reportDraftMessages?.[reportAction.reportActionID];
const matchingDraftMessageString = matchingDraftMessage?.message;
Expand Down Expand Up @@ -767,6 +768,7 @@ function ReportActionsList({
isReportArchived,
reportNameValuePairs?.origin,
reportNameValuePairs?.originalID,
reportActionsFromOnyx,
],
);

Expand Down
39 changes: 39 additions & 0 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import {
getIOUReportActionDisplayMessage,
getMoneyReportPreviewName,
getMostRecentlyVisitedReport,
getOriginalReportID,
getOutstandingChildRequest,
getParentNavigationSubtitle,
getParticipantsList,
Expand Down Expand Up @@ -12022,6 +12023,44 @@ describe('ReportUtils', () => {
});
});

describe('getOriginalReportID', () => {
it('should return undefined when reportID is undefined', () => {
const reportAction = createRandomReportAction(1);
const result = getOriginalReportID(undefined, reportAction, undefined);
expect(result).toBeUndefined();
});

it('should return reportID when currentReportAction exists in reportActions', () => {
const reportID = '123';
const reportAction = createRandomReportAction(1);
const reportActions = {
[reportAction.reportActionID]: reportAction,
};

const result = getOriginalReportID(reportID, reportAction, reportActions);
expect(result).toBe(reportID);
});

it('should return reportID when reportActions is undefined but reportAction has no reportActionID', () => {
const reportID = '123';
const reportAction = {
...createRandomReportAction(1),
reportActionID: undefined,
} as unknown as ReportAction;

const result = getOriginalReportID(reportID, reportAction, undefined);
expect(result).toBe(reportID);
});

it('should return reportID when reportActions is empty and no thread conditions apply', () => {
const reportID = '123';
const reportAction = createRandomReportAction(1);

const result = getOriginalReportID(reportID, reportAction, {});
expect(result).toBe(reportID);
});
});

describe('getBillableAndTaxTotal', () => {
const expenseReport: Report = {
...createExpenseReport(1),
Expand Down
Loading