Skip to content

[PR 5 of 16] Remove global Onyx reference from getOriginalReportID #82864

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.

Sign up for GitHub

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

Merged
tgolen merged 8 commits into from
Feb 20, 2026
Merged

[PR 5 of 16] Remove global Onyx reference from getOriginalReportID #82864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
188bae8
Left reference up one level
tgolen Feb 18, 2026
e31ea59
Lift up originalReportID higher
tgolen Feb 18, 2026
388d9a9
Revert a bunch of random changes
tgolen Feb 18, 2026
f1257f4
Pass original report ID
tgolen Feb 18, 2026
04cd555
Fix errors
tgolen Feb 18, 2026
7c5d0a1
Style
tgolen Feb 18, 2026
22f435e
fix test
tgolen Feb 18, 2026
a4060a6
Merge branch 'main' into tgolen-refactorOnyxConnection-reportactions-…
tgolen Feb 19, 2026
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
18 changes: 15 additions & 3 deletions src/components/ReportActionItem/MoneyRequestReceiptView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import ROUTES from '@src/ROUTES';
import type * as OnyxTypes from '@src/types/onyx';
import type {TransactionPendingFieldsKey} from '@src/types/onyx/Transaction';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import useOriginalReportID from '@hooks/useOriginalReportID';
import useCardFeedErrors from '@hooks/useCardFeedErrors';
import {getBrokenConnectionUrlToFixPersonalCard} from '@libs/CardUtils';
import ReportActionItemImage from './ReportActionItemImage';
Expand Down Expand Up @@ -117,6 +118,7 @@ function MoneyRequestReceiptView({

const [isLoading, setIsLoading] = useState(true);
const parentReportAction = report?.parentReportActionID ? parentReportActions?.[report.parentReportActionID] : undefined;
const originalReportID = useOriginalReportID(report?.reportID, parentReportAction);

@situchan situchan Feb 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the logic of this hook exactly same as getOriginalReportID function?
It's possible that this hook might return undefined for some reason, while getOriginalReportID always returns reportID as fallback.

App/src/hooks/useOriginalReportID.ts

Lines 73 to 74 in 9a0afa0

// If we reach here, we couldn't find the original reportID
return undefined;

The bot raised the same concern here.

@tgolen tgolen Feb 19, 2026

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 had the exact same question! I found the original author of the hook, and we discussed that the logic in the hook is probably more accurate than the method. We discussed a plan to try to unify the method and hook more (by just having the server return the reportAction.reportID and being able to reference that directly without as much of the extra logic), but I don't know when we'll be able to do that.

I'm not too worried about this returning undefined because I think it will only happen in theory and never in practice. If we are really worried about it, then I could add a log line so that we are at least aware if it is returning undefined.

const {iouReport, chatReport: chatIOUReport, isChatIOUReportArchived} = useGetIOUReportFromReportAction(parentReportAction);
const isTrackExpense = !mergeTransactionID && isTrackExpenseReportNew(report, parentReport, parentReportAction);
const moneyRequestReport = parentReport;
Expand Down Expand Up @@ -279,24 +281,34 @@ function MoneyRequestReceiptView({
return;
}
if (parentReportAction) {
cleanUpMoneyRequest(transaction?.transactionID ?? linkedTransactionID, parentReportAction, report.reportID, iouReport, chatIOUReport, isChatIOUReportArchived, true);
cleanUpMoneyRequest(
transaction?.transactionID ?? linkedTransactionID,
parentReportAction,
report.reportID,
iouReport,
chatIOUReport,
isChatIOUReportArchived,
originalReportID,
true,
);
return;
}
}

if (!transaction?.transactionID) {
if (!linkedTransactionID) {
return;
}
clearError(linkedTransactionID);
clearAllRelatedReportActionErrors(report.reportID, parentReportAction);
clearAllRelatedReportActionErrors(report.reportID, parentReportAction, originalReportID);
return;
}
if (!isEmptyObject(transactionAndReportActionErrors)) {
revert(transaction, getLastModifiedExpense(report?.reportID));
}
if (!isEmptyObject(errorsWithoutReportCreation)) {
clearError(transaction.transactionID);
clearAllRelatedReportActionErrors(report.reportID, parentReportAction);
clearAllRelatedReportActionErrors(report.reportID, parentReportAction, originalReportID);
}
if (!isEmptyObject(reportCreationError)) {
if (isInNarrowPaneModal) {
Expand Down
5 changes: 3 additions & 2 deletions src/libs/actions/IOU/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@
};

let allPersonalDetails: OnyxTypes.PersonalDetailsList = {};
Onyx.connect({

Check warning on line 804 in src/libs/actions/IOU/index.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
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand Down Expand Up @@ -894,7 +894,7 @@
};

let allTransactions: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({

Check warning on line 897 in src/libs/actions/IOU/index.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
key: ONYXKEYS.COLLECTION.TRANSACTION,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -908,7 +908,7 @@
});

let allTransactionDrafts: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({

Check warning on line 911 in src/libs/actions/IOU/index.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
key: ONYXKEYS.COLLECTION.TRANSACTION_DRAFT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -917,7 +917,7 @@
});

let allTransactionViolations: NonNullable<OnyxCollection<OnyxTypes.TransactionViolations>> = {};
Onyx.connect({

Check warning on line 920 in src/libs/actions/IOU/index.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
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -931,7 +931,7 @@
});

let allPolicyTags: OnyxCollection<OnyxTypes.PolicyTagLists> = {};
Onyx.connect({

Check warning on line 934 in src/libs/actions/IOU/index.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
key: ONYXKEYS.COLLECTION.POLICY_TAGS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -944,7 +944,7 @@
});

let allReports: OnyxCollection<OnyxTypes.Report>;
Onyx.connect({

Check warning on line 947 in src/libs/actions/IOU/index.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
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -953,7 +953,7 @@
});

let allReportNameValuePairs: OnyxCollection<OnyxTypes.ReportNameValuePairs>;
Onyx.connect({

Check warning on line 956 in src/libs/actions/IOU/index.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
key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -963,7 +963,7 @@

let userAccountID = -1;
let currentUserEmail = '';
Onyx.connect({

Check warning on line 966 in src/libs/actions/IOU/index.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
key: ONYXKEYS.SESSION,
callback: (value) => {
currentUserEmail = value?.email ?? '';
Expand All @@ -972,7 +972,7 @@
});

let deprecatedCurrentUserPersonalDetails: OnyxEntry<OnyxTypes.PersonalDetails>;
Onyx.connect({

Check warning on line 975 in src/libs/actions/IOU/index.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
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
deprecatedCurrentUserPersonalDetails = value?.[userAccountID] ?? undefined;
Expand All @@ -980,7 +980,7 @@
});

let allReportActions: OnyxCollection<OnyxTypes.ReportActions>;
Onyx.connect({

Check warning on line 983 in src/libs/actions/IOU/index.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
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,
callback: (actions) => {
Expand Down Expand Up @@ -8842,6 +8842,7 @@
iouReport: OnyxEntry<OnyxTypes.Report>,
chatReport: OnyxEntry<OnyxTypes.Report>,
isChatIOUReportArchived: boolean | undefined,
originalReportID: string | undefined,
isSingleTransactionView = false,
) {
const {shouldDeleteTransactionThread, shouldDeleteIOUReport, updatedReportAction, updatedIOUReport, updatedReportPreviewAction, transactionThreadID, reportPreviewAction} =
Expand Down Expand Up @@ -8999,7 +9000,7 @@
}

if (!shouldDeleteIOUReport) {
clearAllRelatedReportActionErrors(reportID, reportAction);
clearAllRelatedReportActionErrors(reportID, reportAction, originalReportID);
}

// First, update the reportActions to ensure related actions are not displayed.
Expand All @@ -9008,7 +9009,7 @@
// eslint-disable-next-line @typescript-eslint/no-deprecated
InteractionManager.runAfterInteractions(() => {
if (shouldDeleteIOUReport) {
clearAllRelatedReportActionErrors(reportID, reportAction);
clearAllRelatedReportActionErrors(reportID, reportAction, originalReportID);
}
// After navigation, update the remaining data.
Onyx.update(onyxUpdates);
Expand Down
21 changes: 14 additions & 7 deletions src/libs/actions/ReportActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ Onyx.connect({
},
});

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

@tgolen tgolen Feb 18, 2026

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.

This is the call I was able to remove in this PR


function clearReportActionErrors(reportID: string, reportAction: ReportAction, originalReportID: string | undefined, keys?: string[]) {
if (!reportAction?.reportActionID) {
return;
}
Expand Down Expand Up @@ -80,27 +78,36 @@ function clearReportActionErrors(reportID: string, reportAction: ReportAction, k
ignore: `undefined` means we want to check both parent and children report actions
ignore: `parent` or `child` means we want to ignore checking parent or child report actions because they've been previously checked
*/
function clearAllRelatedReportActionErrors(reportID: string | undefined, reportAction: ReportAction | null | undefined, ignore?: IgnoreDirection, keys?: string[]) {
function clearAllRelatedReportActionErrors(
reportID: string | undefined,
reportAction: ReportAction | null | undefined,
originalReportID: string | undefined,
ignore?: IgnoreDirection,
keys?: string[],
) {
const errorKeys = keys ?? Object.keys(reportAction?.errors ?? {});
if (!reportAction || errorKeys.length === 0 || !reportID) {
return;
}

clearReportActionErrors(reportID, reportAction, keys);
clearReportActionErrors(reportID, reportAction, originalReportID, keys);

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 19, 2026

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 Fallback to reportID when original report ID is unresolved

clearAllRelatedReportActionErrors() now forwards the caller-provided originalReportID directly, but new callers source it from useOriginalReportID(), which explicitly returns undefined when it cannot resolve the action yet (src/hooks/useOriginalReportID.ts). In that case this path can write to an invalid key (REPORT_ACTIONSundefined/sentinel values) instead of the real report’s actions, so dismissing errors does not clear the actual red-brick-road state. Before this change, getOriginalReportID() was recomputed internally and fell back from reportID, so this regression appears only after this refactor.

Useful? React with 👍 / 👎.


const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
if (report?.parentReportID && report?.parentReportActionID && ignore !== 'parent') {
const parentReportAction = getReportAction(report.parentReportID, report.parentReportActionID);
const parentErrorKeys = Object.keys(parentReportAction?.errors ?? {}).filter((err) => errorKeys.includes(err));
const parentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`] ?? {};

@tgolen tgolen Feb 18, 2026

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 know it stinks to have to add this (and the one a few lines below). I fully intend to pass these as params in the next PR. I felt it was just too much to do it in this same PR.

const parentOriginalReportID = getOriginalReportID(report.parentReportID, parentReportAction, parentReportActions);

clearAllRelatedReportActionErrors(report.parentReportID, parentReportAction, 'child', parentErrorKeys);
clearAllRelatedReportActionErrors(report.parentReportID, parentReportAction, parentOriginalReportID, 'child', parentErrorKeys);
}

if (reportAction.childReportID && ignore !== 'child') {
const childActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportAction.childReportID}`] ?? {};
for (const action of Object.values(childActions)) {
const childErrorKeys = Object.keys(action.errors ?? {}).filter((err) => errorKeys.includes(err));
clearAllRelatedReportActionErrors(reportAction.childReportID, action, 'parent', childErrorKeys);
const childOriginalReportID = getOriginalReportID(reportAction.childReportID, action, childActions);
clearAllRelatedReportActionErrors(reportAction.childReportID, action, childOriginalReportID, 'parent', childErrorKeys);
}
}
}
Expand Down
14 changes: 10 additions & 4 deletions src/pages/inbox/report/PureReportActionItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,13 @@ type PureReportActionItemProps = {
clearError?: (transactionID: string) => void;

/** Function to clear all errors from a report action */
clearAllRelatedReportActionErrors?: (reportID: string | undefined, reportAction: OnyxTypes.ReportAction | null | undefined, ignore?: IgnoreDirection, keys?: string[]) => void;
clearAllRelatedReportActionErrors?: (
reportID: string | undefined,
reportAction: OnyxTypes.ReportAction | null | undefined,
originalReportID: string | undefined,
ignore?: IgnoreDirection,
keys?: string[],
) => void;

/** Function to dismiss the actionable whisper for tracking expenses */
dismissTrackExpenseActionableWhisper?: (reportID: string | undefined, reportAction: OnyxEntry<OnyxTypes.ReportAction>) => void;
Expand Down Expand Up @@ -621,14 +627,14 @@ function PureReportActionItem({
const transactionID = isMoneyRequestAction(action) ? getOriginalMessage(action)?.IOUTransactionID : undefined;
if (isSendingMoney && transactionID && reportID) {
const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`];
cleanUpMoneyRequest(transactionID, action, reportID, report, chatReport, undefined, true);
cleanUpMoneyRequest(transactionID, action, reportID, report, chatReport, undefined, originalReportID, true);
return;
}
if (transactionID) {
clearError(transactionID);
}
clearAllRelatedReportActionErrors(reportID, action);
}, [action, isSendingMoney, clearAllRelatedReportActionErrors, reportID, allReports, report, clearError]);
clearAllRelatedReportActionErrors(reportID, action, originalReportID);

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 18, 2026

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 Default unresolved originalReportID to the current report

This call now forwards originalReportID directly, but useOriginalReportID() can return undefined when the action is not found in the current Onyx slice (and this component also defaults missing values to '-1'). In those cases clearAllRelatedReportActionErrors() ultimately writes to REPORT_ACTIONSundefined/REPORT_ACTIONS-1 instead of the real report action collection, so dismissing an error can leave brick-road errors and optimistic actions uncleared. Please fall back to reportID (or recompute via getOriginalReportID) before invoking the clear/cleanup paths.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 19, 2026

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 Fall back to reportID when clearing action errors

dismissError now forwards originalReportID directly, but useOriginalReportID() can legitimately return undefined when the action cannot be resolved yet (e.g., merged/single-transaction threads before related actions are loaded). In that case this path calls clearAllRelatedReportActionErrors() with an unknown origin, and clearReportActionErrors() writes to a non-report key instead of the action’s real report, so dismissing a failed money request can leave the red-brick error/optimistic action uncleared. Before this change, clearAllRelatedReportActionErrors() computed the origin internally and fell back to reportID.

Useful? React with 👍 / 👎.

@situchan situchan Feb 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as #82864 (comment)

}, [action, isSendingMoney, clearAllRelatedReportActionErrors, reportID, allReports, report, clearError, originalReportID]);

const showDismissReceiptErrorModal = useCallback(async () => {
const result = await showConfirmModal({
Expand Down
2 changes: 1 addition & 1 deletion tests/actions/IOUTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2552,7 +2552,7 @@ describe('actions/IOU', () => {
() =>
new Promise<void>((resolve) => {
if (iouReportID) {
clearAllRelatedReportActionErrors(iouReportID, iouAction ?? null);
clearAllRelatedReportActionErrors(iouReportID, iouAction ?? null, iouReportID);

@grgia grgia Feb 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tgolen should this new arg be the same iouReportID?

@tgolen tgolen Feb 20, 2026

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.

Yep, it's the same value that getOriginalReportID() would return, so it's fine to just short-cut the test and use the value. I could add a call to getOriginalReportID() in this test, but I don't think there is too much value in it since that method already has unit tests covering it.

}
resolve();
}),
Expand Down
Loading