Skip to content
Open
Show file tree
Hide file tree
Changes from 11 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
6 changes: 3 additions & 3 deletions src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,8 @@ const ONYXKEYS = {
/** The end date (epoch timestamp) of the workspace owner’s grace period for an overdue Consolidated Travel Billing invoice. */
NVP_PRIVATE_OWNER_TRAVEL_BILLING_GRACE_PERIOD_END: 'nvp_private_travelBillingGracePeriodEnd',

/** The NVP containing the target url to navigate to when deleting a transaction */
NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL: 'nvp_deleteTransactionNavigateBackURL',
/** Set while a deleted transaction's screen is navigating away, holds the route it goes back to. Not found guards stay quiet while it's set */
RAM_ONLY_PENDING_TRANSACTION_DELETE_URL: 'pendingTransactionDeleteURL',

/**
* Cutoff time for a full reconnect. If this client's last full reconnect
Expand Down Expand Up @@ -2019,7 +2019,7 @@ type OnyxValuesMapping = {
[ONYXKEYS.NVP_PRIVATE_AMOUNT_OWED]: number;
[ONYXKEYS.NVP_PRIVATE_OWNER_BILLING_GRACE_PERIOD_END]: number;
[ONYXKEYS.NVP_PRIVATE_OWNER_TRAVEL_BILLING_GRACE_PERIOD_END]: number;
[ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL]: string | undefined;
[ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL]: string | undefined;
[ONYXKEYS.NVP_RECONNECT_APP_IF_FULL_RECONNECT_BEFORE]: string;
[ONYXKEYS.NVP_PRIVATE_FIRST_POLICY_CREATED_DATE]: string;
[ONYXKEYS.NVP_PRIVATE_MANUAL_TEAM_2025_PRICING]: string;
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportOrNotFoundGuard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function ReportOrNotFoundGuard({reportID, shouldRequireReportID = true, children
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
const [reportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportID}`);
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL);
const [guideAccountIDs] = useOnyx(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS);
const hasGuidesEmails = hasExpensifyGuidesEmails(Object.keys(report?.participants ?? {}).map(Number), guideAccountIDs);
const isFocused = useIsFocused();
Expand Down
29 changes: 29 additions & 0 deletions src/hooks/useClearDeleteTransactionNavigateBackUrlOnUnmount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import {doesDeleteNavigateBackUrlIncludeDuplicatesReview} from '@libs/TransactionNavigationUtils';

import {clearDeleteTransactionNavigateBackUrl, clearDeleteTransactionNavigateBackUrlAfterTransitions} from '@userActions/Report';

import ONYXKEYS from '@src/ONYXKEYS';

import {useEffect} from 'react';
import Onyx from 'react-native-onyx';

/** Clears the delete navigate back URL on unmount, since native stack never renders a popped screen unfocused. */
function useClearDeleteTransactionNavigateBackUrlOnUnmount(shouldClear: boolean) {
useEffect(() => {
if (!shouldClear) {
return;
}
return () =>
clearDeleteTransactionNavigateBackUrlAfterTransitions(() => {
// Read when the clear fires, the unmounted screen may never have received the URL.
Onyx.get(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL).then((url) => {
if (!url || doesDeleteNavigateBackUrlIncludeDuplicatesReview(url)) {
return;
}
requestAnimationFrame(clearDeleteTransactionNavigateBackUrl);

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.

Let's also cancel the queued animation frame when a new deletion starts. Once the transition callback fires, there's no pending handle left to cancel. If another deletion sets its URL before this frame runs, the old callback clears that new URL and can show Not Found while the screen is closing.

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.

done, the cancel handle now covers the whole chain: the transition, the Onyx.get result and the queued frame (cancelAnimationFrame). so a new deletion cancels it even after the transition callback already ran. added a test for that case too

});
});
}, [shouldClear]);
}

export default useClearDeleteTransactionNavigateBackUrlOnUnmount;
4 changes: 2 additions & 2 deletions src/libs/ExportOnyxState/masking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,6 @@ const onyxKeysToMaskFragileData = new Set<string>([
ONYXKEYS.NEW_GROUP_CHAT_DRAFT,
ONYXKEYS.NVP_BILLING_FUND_ID,
ONYXKEYS.NVP_CSV_EXPORT_LAYOUTS,
// Holds a raw route, and a search route can carry an unencoded email in its "name" param
ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL,
ONYXKEYS.NVP_EXPENSE_RULES,
ONYXKEYS.NVP_EXPENSIFY_COMPANY_CARDS_CUSTOM_NAMES,
ONYXKEYS.NVP_FIRST_DAY_FREE_TRIAL,
Expand Down Expand Up @@ -550,6 +548,8 @@ const onyxKeysToMaskFragileData = new Set<string>([
ONYXKEYS.RAM_ONLY_HAS_DISMISSED_CONCIERGE_NOTIFICATION_BANNER,
ONYXKEYS.RAM_ONLY_HAS_FRESH_WALLET_DATA,
ONYXKEYS.RAM_ONLY_IS_LOADING_SEARCH_FILTERS_CATEGORY_DATA,
// Holds a raw route, and a search route can carry an unencoded email in its "name" param
ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL,
ONYXKEYS.RAM_ONLY_WALLET_ONFIDO,
ONYXKEYS.RAM_ONLY_WORKSPACE_MEMBERS_SELECTED_FOR_BULK_EDIT,
ONYXKEYS.RAM_ONLY_WORKSPACE_MEMBERS_SELECTED_FOR_ROLE_CHANGE,
Expand Down
22 changes: 20 additions & 2 deletions src/libs/actions/Report/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTop
import type {LinkToOptions} from '@libs/Navigation/helpers/linkTo/types';
import Navigation from '@libs/Navigation/Navigation';
import REPORT_LINK_ROUTE_PARAMS from '@libs/Navigation/reportLinkRouteParams';
import TransitionTracker from '@libs/Navigation/TransitionTracker';
import enhanceParameters from '@libs/Network/enhanceParameters';
import {getDBTimeWithSkew, getIsOffline as isOfflineNetwork, getServerAnchoredDBTime} from '@libs/NetworkState';
import {buildOptimisticNextStep} from '@libs/NextStepUtils';
Expand Down Expand Up @@ -7432,12 +7433,28 @@ function downloadReportPDF(fileName: string, reportName: string, translate: Loca
fileDownload(translate, addEncryptedAuthTokenToURL(pdfURL, encryptedAuthToken, true), downloadFileName, '', Browser.isMobileSafari()).then(() => setDownload(fileName, false));
}

// A newer deletion cancels this so a stale clear can't wipe its URL.
let pendingDeleteTransactionNavigateBackUrlClear: {cancel: () => void} | undefined;

function setDeleteTransactionNavigateBackUrl(url: string) {
Onyx.set(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, url);
pendingDeleteTransactionNavigateBackUrlClear?.cancel();
pendingDeleteTransactionNavigateBackUrlClear = undefined;
Onyx.set(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL, url);
}

function clearDeleteTransactionNavigateBackUrl() {
Onyx.merge(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, null);
Onyx.merge(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL, null);
}

function clearDeleteTransactionNavigateBackUrlAfterTransitions(onTransitionsEnd: () => void) {
pendingDeleteTransactionNavigateBackUrlClear?.cancel();
pendingDeleteTransactionNavigateBackUrlClear = TransitionTracker.runAfterTransitions({
callback: () => {
pendingDeleteTransactionNavigateBackUrlClear = undefined;
onTransitionsEnd();
},
waitForUpcomingTransition: true,
Comment thread
David-2002065 marked this conversation as resolved.
Resolved
});
}

type DeleteAppReportProps = {
Expand Down Expand Up @@ -9539,6 +9556,7 @@ export {
clearAddRoomMemberError,
clearAvatarErrors,
clearDeleteTransactionNavigateBackUrl,
clearDeleteTransactionNavigateBackUrlAfterTransitions,
clearIOUError,
clearReportErrors,
clearNewRoomFormError,
Expand Down
5 changes: 4 additions & 1 deletion src/pages/Search/SearchMoneyRequestReportPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {useSearchResultsContext} from '@components/Search/SearchContext';
import useRHPWidth from '@components/WideRHPContextProvider/useRHPWidth';
import WideRHPOverlayWrapper from '@components/WideRHPOverlayWrapper';

import useClearDeleteTransactionNavigateBackUrlOnUnmount from '@hooks/useClearDeleteTransactionNavigateBackUrlOnUnmount';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useDismissOnMoneyRequestReportRemoval from '@hooks/useDismissOnMoneyRequestReportRemoval';
import useDocumentTitle from '@hooks/useDocumentTitle';
Expand Down Expand Up @@ -85,7 +86,7 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {
selector: fullPolicySnapshotLastModifiedSelector,
});
const [hasReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportIDFromRoute}`, {selector: Boolean});
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL);

const parentReportAction = useParentReportAction(report);

Expand Down Expand Up @@ -128,6 +129,8 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {
return () => handle.cancel();
}, [isFocused, deleteTransactionNavigateBackUrl]);

useClearDeleteTransactionNavigateBackUrlOnUnmount(true);

const [reportLoadingState = defaultReportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportIDFromRoute}`);
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
const isReportArchived = useReportIsArchived(report?.reportID);
Expand Down
2 changes: 1 addition & 1 deletion src/pages/TransactionDuplicate/DynamicReviewPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function DynamicReviewPage() {
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`);
const [hasReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${route.params.reportID}`, {selector: Boolean});
const [parentReportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${report?.parentReportID}`);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL);
const [reportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${route.params.reportID}`);
const [expenseReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`);
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`);
Expand Down
17 changes: 14 additions & 3 deletions src/pages/inbox/DeleteTransactionNavigateBackHandler.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import useClearDeleteTransactionNavigateBackUrlOnUnmount from '@hooks/useClearDeleteTransactionNavigateBackUrlOnUnmount';
import useOnyx from '@hooks/useOnyx';

import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import TransitionTracker from '@libs/Navigation/TransitionTracker';
import {isReportTransactionThread} from '@libs/ReportUtils';
import {doesDeleteNavigateBackUrlIncludeDuplicatesReview} from '@libs/TransactionNavigationUtils';

import {clearDeleteTransactionNavigateBackUrl} from '@userActions/Report';
Expand All @@ -10,14 +13,20 @@ import ONYXKEYS from '@src/ONYXKEYS';
import {useIsFocused} from '@react-navigation/native';
import {useEffect} from 'react';

type DeleteTransactionNavigateBackHandlerProps = {
/** The reportID of the report screen this handler is mounted in */
reportID: string | undefined;
};

/**
* Component that does not render anything but isolates the NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL
* Component that does not render anything but isolates the RAM_ONLY_PENDING_TRANSACTION_DELETE_URL
* subscription from ReportScreen. Clears the URL after interactions complete
* when the report is no longer focused.
*/
function DeleteTransactionNavigateBackHandler() {
function DeleteTransactionNavigateBackHandler({reportID}: DeleteTransactionNavigateBackHandlerProps) {
const isFocused = useIsFocused();
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL);
const [isTransactionThread = false] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(reportID)}`, {selector: isReportTransactionThread});

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.

Could we subscribe to the parent report and action too? isReportTransactionThread reads both, but this selector only reruns when the thread changes. If the parent data loads later, this stays false and we still skip clearing the URL on native unmount.

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.

good point, updated. the handler now subscribes to the parent report and parent action too and passes them into a new pure helper isReportTransactionThreadNew, so it rechecks once the parent data loads. added a test where the parent arrives after the thread mounts


useEffect(() => {
if (isFocused || !deleteTransactionNavigateBackUrl) {
Expand All @@ -36,6 +45,8 @@ function DeleteTransactionNavigateBackHandler() {
return () => handle.cancel();
}, [isFocused, deleteTransactionNavigateBackUrl]);

useClearDeleteTransactionNavigateBackUrlOnUnmount(isTransactionThread);

return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/pages/inbox/ReportNotFoundGuard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function ReportNotFoundGuard({children}: ReportNotFoundGuardProps) {
});
const [isLoadingReportData = true] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL);

const reportID = report?.reportID;
const isOptimisticDelete = report?.statusNum === CONST.REPORT.STATUS_NUM.CLOSED;
Expand Down
2 changes: 1 addition & 1 deletion src/pages/inbox/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ function ReportScreen({route, navigation, shouldDeferReportActions = false}: Rep
testID={`report-screen-${reportIDFromRoute}`}
>
<ReportNonEssentialsDeferredMount reportID={reportIDFromRoute}>
<DeleteTransactionNavigateBackHandler />
<DeleteTransactionNavigateBackHandler reportID={reportIDFromRoute} />
<ReportRouteParamHandler />
<OneTransactionThreadRedirectHandler />
<ReportFetchHandler />
Expand Down
2 changes: 1 addition & 1 deletion src/pages/inbox/report/withReportOrNotFound.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export default function (shouldRequireReportID = true): <TProps extends WithRepo
const [reportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`);
const [reportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportID}`);
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL);
const [guideAccountIDs] = useOnyx(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS);
const hasGuidesEmails = hasExpensifyGuidesEmails(Object.keys(report?.participants ?? {}).map(Number), guideAccountIDs);
const isFocused = useIsFocused();
Expand Down
1 change: 1 addition & 0 deletions src/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export default function () {
ONYXKEYS.RAM_ONLY_ARE_TRANSLATIONS_LOADING,
ONYXKEYS.RAM_ONLY_MOBILE_SELECTION_MODE,
ONYXKEYS.RAM_ONLY_IS_SIDEBAR_LOADED,
ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL,
ONYXKEYS.RAM_ONLY_IS_PRODUCT_MARKETING_WINDOW_COVERED,
ONYXKEYS.DERIVED.RAM_ONLY_SORTED_REPORT_ACTIONS,
ONYXKEYS.RAM_ONLY_IS_CHECKING_PUBLIC_ROOM,
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/DynamicReportDetailsPageTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ describe('DynamicReportDetailsPage', () => {
await waitForBatchedUpdatesWithAct();

// The page's own fallback must have produced the invoice-room back URL.
const backUrl = await getOnyxValue(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
const backUrl = await getOnyxValue(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL);
expect(backUrl).toBe(ROUTES.REPORT_WITH_ID.getRoute(invoiceRoomID));
});

Expand Down Expand Up @@ -413,7 +413,7 @@ describe('DynamicReportDetailsPage', () => {
// Simulate the invoice delete: set the delete-back URL to the invoice room, then null the invoice report
// (the deferred optimistic delete) while this focused RHP is still mounted.
await act(async () => {
await Onyx.set(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, ROUTES.REPORT_WITH_ID.getRoute(invoiceRoomID));
await Onyx.set(ONYXKEYS.RAM_ONLY_PENDING_TRANSACTION_DELETE_URL, ROUTES.REPORT_WITH_ID.getRoute(invoiceRoomID));
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${invoiceReportID}`, null);
});
await waitForBatchedUpdatesWithAct();
Expand Down
1 change: 1 addition & 0 deletions tests/ui/SearchMoneyRequestReportPageTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ jest.mock('@userActions/Report', () => ({
openReport: jest.fn(),
updateLastVisitTime: jest.fn(),
clearDeleteTransactionNavigateBackUrl: jest.fn(),
clearDeleteTransactionNavigateBackUrlAfterTransitions: jest.fn(),
}));

// All child components are mocked to null: none of them need to actually render for these tests,
Expand Down
Loading
Loading