diff --git a/src/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index 3ecddcc864ab..08402f4ce3a3 100644 --- a/src/hooks/usePaginatedReportActions.ts +++ b/src/hooks/usePaginatedReportActions.ts @@ -28,13 +28,27 @@ type UsePaginatedReportActionsOptions = { * anchor from ever resolving. Scoped to Concierge so regular inbox chat pagination keeps the first-render ref behavior. */ shouldSnapshotInitialLastReadTime?: boolean; + + /** + * When true, the linked `reportActionID` is known to live in the one-transaction thread that gets merged into this + * report, so we drop the pagination anchor and render the newest window (the merged view surfaces the linked action). + * This must NOT be set merely because the action is absent from this report's cache — an action that belongs to this + * report but hasn't been fetched yet still needs the anchor so the list scrolls to it once `OpenReport` hydrates it + * (https://github.com/Expensify/App/issues/86919). + */ + isLinkedActionInMergedTransactionThread?: boolean; }; /** * Get the longest continuous chunk of reportActions including the linked reportAction. If not linking to a specific action, returns the continuous chunk of newest reportActions. */ function usePaginatedReportActions(reportID: string | undefined, reportActionID?: string, options?: UsePaginatedReportActionsOptions) { - const {shouldLinkToOldestUnreadReportAction = false, treatAsNoPaginationAnchor = false, shouldSnapshotInitialLastReadTime = false} = options ?? {}; + const { + shouldLinkToOldestUnreadReportAction = false, + treatAsNoPaginationAnchor = false, + shouldSnapshotInitialLastReadTime = false, + isLinkedActionInMergedTransactionThread = false, + } = options ?? {}; const nonEmptyStringReportID = getNonEmptyStringOnyxID(reportID); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${nonEmptyStringReportID}`); @@ -67,7 +81,14 @@ function usePaginatedReportActions(reportID: string | undefined, reportActionID? } if (reportActionID) { - return reportActionID; + // Only drop the anchor when the linked action is known to live in the one-transaction thread merged into this + // report. There getContinuousChain would otherwise return an empty page (the action isn't in this report's own + // actions), hiding the parent-level "Submitted" system message; rendering the newest window instead lets the + // merged view render and ReportActionsList's initialScrollKey anchors to the linked message. We must NOT drop the + // anchor just because the action is absent from this report's cache — an action that belongs to this report but + // hasn't loaded yet still needs the anchor so the list scrolls to it once OpenReport hydrates it + // (https://github.com/Expensify/App/issues/86919). + return isLinkedActionInMergedTransactionThread ? undefined : reportActionID; } if (!shouldLinkToOldestUnreadReportAction) { @@ -81,7 +102,15 @@ function usePaginatedReportActions(reportID: string | undefined, reportActionID? return sortedAllReportActions.findLast((reportAction) => reportAction.created > initialLastReadTime)?.reportActionID; /* eslint-enable react-hooks/refs */ - }, [treatAsNoPaginationAnchor, reportActionID, shouldLinkToOldestUnreadReportAction, sortedAllReportActions, shouldSnapshotInitialLastReadTime, firstDefinedLastReadTime]); + }, [ + treatAsNoPaginationAnchor, + reportActionID, + isLinkedActionInMergedTransactionThread, + shouldLinkToOldestUnreadReportAction, + sortedAllReportActions, + shouldSnapshotInitialLastReadTime, + firstDefinedLastReadTime, + ]); const { data: reportActions, diff --git a/src/hooks/useReportActionsPagination.ts b/src/hooks/useReportActionsPagination.ts index 61b87c8f7318..b7b8a44f02e6 100644 --- a/src/hooks/useReportActionsPagination.ts +++ b/src/hooks/useReportActionsPagination.ts @@ -1,6 +1,6 @@ import {getReportPreviewReportAction} from '@libs/actions/IOU/MoneyRequestBuilder'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; -import {getCombinedReportActions, getFilteredReportActionsForReportView, isCreatedAction} from '@libs/ReportActionsUtils'; +import {getCombinedReportActions, getFilteredReportActionsForReportView, getOneTransactionThreadReportID, isCreatedAction} from '@libs/ReportActionsUtils'; import {isConciergeChatReport, isInvoiceReport, isMoneyRequestReport, isReportTransactionThread as isReportTransactionThreadUtil, shouldReportAlignToTop} from '@libs/ReportUtils'; import getReportActionsToDisplay from '@pages/inbox/report/getReportActionsToDisplay'; @@ -48,6 +48,16 @@ function useReportActionsPagination(reportID: string | undefined, reportActionID const shouldBeAlignedToTop = shouldReportAlignToTop(report, parentReportAction); + // Resolve whether the linked action lives in the one-transaction thread that gets merged into this report. Only in that + // case should usePaginatedReportActions drop the pagination anchor — dropping it merely because the action is absent from + // this report's cache would break the initial scroll-to for a valid linked action that simply hasn't been fetched into + // this report yet (e.g. an older message). A non-one-transaction report resolves to undefined here, so its anchor is kept. + const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.chatReportID)}`); + const [reportActionsForThreadCheck] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(reportID)}`); + const linkedActionTransactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActionsForThreadCheck ?? {}, isOffline); + const [linkedActionTransactionThreadActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(linkedActionTransactionThreadReportID)}`); + const isLinkedActionInMergedTransactionThread = !!reportActionIDFromRoute && !!linkedActionTransactionThreadActions?.[reportActionIDFromRoute]; + const { reportActions: unfilteredReportActions, hasOlderActions, @@ -60,6 +70,7 @@ function useReportActionsPagination(reportID: string | undefined, reportActionID // Scope the first-defined lastReadTime snapshot to Concierge so the cold-open unread anchor resolves // (https://github.com/Expensify/App/issues/93196) without changing regular inbox chat pagination. shouldSnapshotInitialLastReadTime: isConciergeChat, + isLinkedActionInMergedTransactionThread, }); const allReportActions = useMemo(() => getFilteredReportActionsForReportView(unfilteredReportActions), [unfilteredReportActions]); diff --git a/src/pages/inbox/LinkedActionNotFoundGuard.tsx b/src/pages/inbox/LinkedActionNotFoundGuard.tsx index d77c311a4068..499bc27670d5 100644 --- a/src/pages/inbox/LinkedActionNotFoundGuard.tsx +++ b/src/pages/inbox/LinkedActionNotFoundGuard.tsx @@ -1,10 +1,11 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; +import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; import useReportIsArchived from '@hooks/useReportIsArchived'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import Navigation from '@libs/Navigation/Navigation'; -import {isReportActionVisible, isWhisperAction} from '@libs/ReportActionsUtils'; +import {getOneTransactionThreadReportID, isReportActionVisible, isWhisperAction} from '@libs/ReportActionsUtils'; import {canUserPerformWriteAction} from '@libs/ReportUtils'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -57,14 +58,24 @@ function LinkedActionNotFoundGate({reportActionIDFromRoute, children}: LinkedAct const reportIDFromRoute = getNonEmptyStringOnyxID(routeParams?.reportID); const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); + const {isOffline} = useNetwork(); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportIDFromRoute}`); + const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.chatReportID)}`); const [isLoadingInitialReportActions = true] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportIDFromRoute}`, { selector: isLoadingInitialReportActionsSelector, }); - const [linkedAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportIDFromRoute}`, { + const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportIDFromRoute}`); + const [linkedActionInRoute] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportIDFromRoute}`, { selector: (actions: OnyxEntry) => getReportActionByIDSelector(actions, reportActionIDFromRoute), }); + + const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActions ?? {}, isOffline); + const [linkedActionInTransactionThread] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(transactionThreadReportID)}`, { + selector: (actions: OnyxEntry) => getReportActionByIDSelector(actions, reportActionIDFromRoute), + }); + + const linkedAction = linkedActionInRoute ?? linkedActionInTransactionThread; const [visibleReportActionsData] = useOnyx(ONYXKEYS.DERIVED.VISIBLE_REPORT_ACTIONS); const isReportArchived = useReportIsArchived(reportIDFromRoute); diff --git a/src/pages/inbox/ReportFetchHandler.tsx b/src/pages/inbox/ReportFetchHandler.tsx index 5486eb5c3ca8..020fd6d4b83f 100644 --- a/src/pages/inbox/ReportFetchHandler.tsx +++ b/src/pages/inbox/ReportFetchHandler.tsx @@ -8,6 +8,7 @@ import useIsReportActionsLoaded from '@hooks/useIsReportActionsLoaded'; import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; import usePaginatedReportActions from '@hooks/usePaginatedReportActions'; +import useParentReportAction from '@hooks/useParentReportAction'; import usePrevious from '@hooks/usePrevious'; import useReportTransactionsCollection from '@hooks/useReportTransactionsCollection'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; @@ -62,6 +63,8 @@ import {useIsFocused, useNavigation, useRoute} from '@react-navigation/native'; import {guidedSetupAndTourStatusSelector} from '@selectors/Onboarding'; import {useEffect, useEffectEvent, useRef} from 'react'; +import shouldRedirectLinkedActionToParentReport from './shouldRedirectLinkedActionToParentReport'; + type ReportScreenRoute = | PlatformStackRouteProp | PlatformStackRouteProp; @@ -148,6 +151,10 @@ function ReportFetchHandler() { const isTransactionThreadView = isReportTransactionThread(report); + const parentReportAction = useParentReportAction(report); + const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.parentReportID)}`); + const shouldRedirectToParentReport = shouldRedirectLinkedActionToParentReport({report, parentReport, parentReportAction, reportActionIDFromRoute, isOffline}); + // Track whether the current route is an own workspace chat. See issue #84248. const isCurrentRouteOwnWorkspaceChatRef = useIsOwnWorkspaceChatRef(report, reportIDFromRoute); @@ -432,6 +439,17 @@ function ReportFetchHandler() { Navigation.navigate(ROUTES.EXPENSE_REPORT_RHP.getRoute({reportID: reportIDFromRoute, backTo: route.params?.backTo}), {forceReplace: true}); }, [isFocused, report, reportIDFromRoute, route.params?.backTo, shouldReplaceWithExpenseReportRHP]); + // Redirect a linked action on a one-transaction thread to its parent expense report so the combined view (including the + // parent's "Submitted" system message) is what opens, and the list can anchor to the linked action. `forceReplace` keeps + // this out of the history stack so going back returns to wherever the link was opened from, not to the thread route. + // Bail while blurred for the same reason as the redirect above: this effect can fire late, after the user has moved on. + useEffect(() => { + if (!shouldRedirectToParentReport || !isFocused || !report?.parentReportID) { + return; + } + Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.parentReportID, reportActionIDFromRoute, undefined, route.params?.backTo), {forceReplace: true}); + }, [shouldRedirectToParentReport, isFocused, report?.parentReportID, reportActionIDFromRoute, route.params?.backTo]); + useEffect(() => { // This function is triggered when a user clicks on a link to navigate to a report. // For each link click, we retrieve the report data again, even though it may already be cached. diff --git a/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx b/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx index 63b8ebc6c3f5..d1695f8690f5 100644 --- a/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx +++ b/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx @@ -1453,6 +1453,9 @@ const ContextMenuActions: ContextMenuAction[] = [ onPress: (closePopover, {reportAction, originalReportID}) => { getEnvironmentURL().then((environmentURL) => { const reportActionID = reportAction?.reportActionID; + // Always link to the report that actually owns the action so the link can never go stale. For a one-transaction + // expense that owner is the transaction thread; ReportFetchHandler redirects to the parent expense report at open + // time while that is still the report's only transaction, which is what surfaces the parent's "Submitted" message. Clipboard.setString(`${environmentURL}/r/${originalReportID}/${reportActionID}`); }); hideContextMenu(true, ReportActionComposeFocusManager.focus); diff --git a/src/pages/inbox/shouldRedirectLinkedActionToParentReport.ts b/src/pages/inbox/shouldRedirectLinkedActionToParentReport.ts new file mode 100644 index 000000000000..e025cb783550 --- /dev/null +++ b/src/pages/inbox/shouldRedirectLinkedActionToParentReport.ts @@ -0,0 +1,41 @@ +import {isOneTransactionThread, isReportTransactionThread} from '@libs/ReportUtils'; + +import type {Report, ReportAction} from '@src/types/onyx'; + +import type {OnyxEntry} from 'react-native-onyx'; + +type ShouldRedirectLinkedActionToParentReportParams = { + /** The report the route currently points at */ + report: OnyxEntry; + + /** The parent of `report`, if it has one */ + parentReport: OnyxEntry; + + /** The action in the parent report that created `report` */ + parentReportAction: OnyxEntry; + + /** The linked action ID from the route, if the route is a message link */ + reportActionIDFromRoute: string | undefined; + + isOffline: boolean; +}; + +/** + * A copied message link always points at the report that owns the action, which for a one-transaction expense is the + * transaction thread. Opening that thread directly would hide the parent's system messages (e.g. "Submitted") and the + * expense report's action buttons, so while the thread is still its parent's only transaction we redirect to the parent + * and render the combined view instead. + * + * Evaluating this at open time rather than when the link is copied is what keeps previously copied links working: once the + * report gains a second expense this returns false, and the thread opens on its own — which is still where the action + * lives, so the link never breaks. See https://github.com/Expensify/App/issues/86919. + */ +function shouldRedirectLinkedActionToParentReport({report, parentReport, parentReportAction, reportActionIDFromRoute, isOffline}: ShouldRedirectLinkedActionToParentReportParams): boolean { + if (!reportActionIDFromRoute || !report?.parentReportID || !isReportTransactionThread(report)) { + return false; + } + + return isOneTransactionThread(report, parentReport, parentReportAction, isOffline); +} + +export default shouldRedirectLinkedActionToParentReport; diff --git a/tests/navigation/LinkedActionNotFoundGuardTest.tsx b/tests/navigation/LinkedActionNotFoundGuardTest.tsx index b464add1f92b..1c18c3cbd0b3 100644 --- a/tests/navigation/LinkedActionNotFoundGuardTest.tsx +++ b/tests/navigation/LinkedActionNotFoundGuardTest.tsx @@ -68,10 +68,16 @@ jest.mock('@hooks/useResponsiveLayout', () => ({ let mockIsReportActionVisible = true; +// `undefined` means "this report is not a one-transaction expense", so the guard falls back to looking the linked action +// up in the route's own report — the behaviour every case below exercises. The transaction-thread lookup itself is +// covered separately in tests/unit/shouldRedirectLinkedActionToParentReportTest.ts. +let mockTransactionThreadReportID: string | undefined; + jest.mock('@libs/ReportActionsUtils', () => ({ __esModule: true, isReportActionVisible: () => mockIsReportActionVisible, isWhisperAction: () => false, + getOneTransactionThreadReportID: () => mockTransactionThreadReportID, })); jest.mock('@libs/ReportUtils', () => ({ @@ -82,14 +88,21 @@ jest.mock('@libs/ReportUtils', () => ({ // Mock useOnyx to control linked action, report, metadata, and derived values type UseOnyxReturn = [unknown, {status: string}]; let mockLinkedAction: ReportAction | null | undefined; +let mockLinkedActionInTransactionThread: ReportAction | null | undefined; let mockIsLoadingInitialReportActions: boolean; jest.mock('@hooks/useOnyx', () => ({ __esModule: true, default: (key: string): UseOnyxReturn => { - if (key.startsWith('reportActions_')) { + // The route report's own actions. Note the guard subscribes to this key twice (raw actions and the linked-action + // selector); returning the same value for both is fine because the selector is bypassed by this mock. + if (key === 'reportActions_12345') { return [mockLinkedAction, {status: 'loaded'}]; } + // Any other report's actions is the transaction thread the guard falls back to. + if (key.startsWith('reportActions_')) { + return [mockLinkedActionInTransactionThread, {status: 'loaded'}]; + } if (key.startsWith('reportLoadingState_')) { return [mockIsLoadingInitialReportActions, {status: 'loaded'}]; } @@ -127,6 +140,8 @@ describe('LinkedActionNotFoundGuard', () => { mockLinkedAction = createReportAction(); mockIsLoadingInitialReportActions = false; mockIsReportActionVisible = true; + mockTransactionThreadReportID = undefined; + mockLinkedActionInTransactionThread = null; }); it('renders children when linked action exists', () => { @@ -140,6 +155,24 @@ describe('LinkedActionNotFoundGuard', () => { expect(mockSetParams).not.toHaveBeenCalled(); }); + it('renders children when the linked action lives in the merged transaction thread instead of the route report', () => { + // A message link for a one-transaction expense resolves to the parent expense report, but the message itself lives + // in the transaction thread. Without the thread lookup this rendered "the comment you are looking for cannot be + // found" for a perfectly valid link. See issue #86919. + mockLinkedAction = null; + mockLinkedActionInTransactionThread = createReportAction({reportID: '54321'}); + mockTransactionThreadReportID = '54321'; + + render( + + + , + ); + + expect(screen.getByTestId('test-children')).toBeTruthy(); + expect(mockSetParams).not.toHaveBeenCalled(); + }); + it('renders children and clears reportActionID once when the linked action is already deleted on mount', () => { mockIsReportActionVisible = false; diff --git a/tests/unit/ContextMenuActionsCopyLinkTest.ts b/tests/unit/ContextMenuActionsCopyLinkTest.ts new file mode 100644 index 000000000000..d3d6e2e180ff --- /dev/null +++ b/tests/unit/ContextMenuActionsCopyLinkTest.ts @@ -0,0 +1,82 @@ +import Clipboard from '@libs/Clipboard'; +import type * as EnvironmentModule from '@libs/Environment/Environment'; + +import ContextMenuActions from '@pages/inbox/report/ContextMenu/ContextMenuActions'; +import type {ContextMenuActionPayload} from '@pages/inbox/report/ContextMenu/ContextMenuActions'; + +import CONST from '@src/CONST'; + +import createRandomReportAction from '../utils/collections/reportActions'; + +// Guards the copied-link format for issue #86919. The link must point at the report that OWNS the action +// (`originalReportID` — for a one-transaction expense that is the transaction thread) and must not be rewritten to the +// parent expense report. Rewriting it at copy time made links go stale: once the report gained a second expense the +// parent no longer resolved the child thread and the link fell through to the not-found page. The parent redirect is +// instead decided at open time in ReportFetchHandler (see shouldRedirectLinkedActionToParentReport). + +jest.mock( + 'expo-web-browser', + () => ({ + openAuthSessionAsync: jest.fn(), + }), + {virtual: true}, +); + +jest.mock('@components/Reactions/MiniQuickEmojiReactions', () => 'MiniQuickEmojiReactions'); +jest.mock('@components/Reactions/QuickEmojiReactions', () => 'QuickEmojiReactions'); + +jest.mock('@libs/Clipboard', () => ({ + __esModule: true, + default: { + canSetHtml: jest.fn(), + setString: jest.fn(), + setHtml: jest.fn(), + }, +})); + +jest.mock('@libs/Environment/Environment', () => ({ + __esModule: true, + ...jest.requireActual('@libs/Environment/Environment'), + getEnvironmentURL: jest.fn(() => Promise.resolve('https://new.expensify.com')), +})); + +const mockClipboard = jest.mocked(Clipboard); + +// ContextMenuAction is a union; sentryLabel/onPress only exist on the icon variant, so narrow with `in`. +const copyLinkAction = ContextMenuActions.find((action) => 'sentryLabel' in action && action.sentryLabel === CONST.SENTRY_LABEL.CONTEXT_MENU.COPY_LINK); + +// Flush the microtasks queued by getEnvironmentURL().then(...) inside the onPress handler. +const flushPromises = () => + new Promise((resolve) => { + process.nextTick(resolve); + }); + +function createPayload(overrides: Partial): ContextMenuActionPayload { + // The copy-link handler only reads reportAction and originalReportID; the rest of the (large) payload type is + // irrelevant to this action, so we assert the minimal shape it needs. + const payload = { + reportAction: {...createRandomReportAction(1), reportActionID: 'action-1'}, + originalReportID: 'transaction-thread-1', + isOffline: false, + ...overrides, + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- test payload only needs the fields the copy-link handler reads + return payload as ContextMenuActionPayload; +} + +describe('ContextMenuActions copy link', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('copies a link pointing at the report that owns the action, not a rewritten parent report', async () => { + if (!copyLinkAction || !('onPress' in copyLinkAction)) { + throw new Error('Copy link context menu action was not found'); + } + + copyLinkAction.onPress(true, createPayload({originalReportID: 'transaction-thread-1'})); + await flushPromises(); + + expect(mockClipboard.setString).toHaveBeenCalledWith('https://new.expensify.com/r/transaction-thread-1/action-1'); + }); +}); diff --git a/tests/unit/hooks/usePaginatedReportActions.test.ts b/tests/unit/hooks/usePaginatedReportActions.test.ts new file mode 100644 index 000000000000..d65b2f4d36ba --- /dev/null +++ b/tests/unit/hooks/usePaginatedReportActions.test.ts @@ -0,0 +1,207 @@ +import {renderHook} from '@testing-library/react-native'; + +import useOnyx from '@hooks/useOnyx'; +import usePaginatedReportActions from '@hooks/usePaginatedReportActions'; +import useReportIsArchived from '@hooks/useReportIsArchived'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Report, ReportAction} from '@src/types/onyx'; +import type Pages from '@src/types/onyx/Pages'; + +import type {OnyxKey, UseOnyxResult} from 'react-native-onyx'; + +import createRandomReportAction from '../../utils/collections/reportActions'; + +// The behavior change under test lives in the `id` useMemo of usePaginatedReportActions: +// when a `reportActionID` is provided but does NOT exist in this report's own actions +// (e.g. a one-transaction expense link where the linked message lives in the merged-in +// transaction thread), the hook must fall back to the newest window instead of anchoring +// pagination to a missing action. Previously, getContinuousChain returned an empty array in +// that case (see tests/unit/PaginationUtilsTest.ts "given an input ID of 8 or 13 ... empty +// array"), which is what hid the parent-level "Submitted" system message. +// +// We deliberately use the REAL getContinuousChain (PaginationUtils is not mocked) so these +// tests exercise the true integration of the change with pagination, and only mock the Onyx +// subscriptions so we can control the report, its actions, and its pages. + +jest.mock('@hooks/useOnyx', () => ({ + __esModule: true, + default: jest.fn(), +})); + +jest.mock('@hooks/useReportIsArchived', () => ({ + __esModule: true, + default: jest.fn(() => false), +})); + +const mockUseOnyx = jest.mocked(useOnyx); + +const REPORT_ID = 'expense-report-1'; +const LINKED_ACTION_ID = 'action-in-this-report'; +const SIBLING_ACTION_ID = 'action-in-transaction-thread'; + +// `{status: 'loaded'}` alone satisfies ResultMetadata (its sourceValue is optional). A single +// broad UseOnyxResult type keeps each mocked subscription value assignable without casts. +type MockOnyxResult = UseOnyxResult; + +function makeReport(overrides: Partial = {}): Report { + return { + reportID: REPORT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + chatReportID: 'chat-report-1', + ...overrides, + } as Report; +} + +/** + * Minimal display-sorted actions (newest first). Ordering only needs to be internally + * consistent — getContinuousChain indexes by reportActionID, and the "newest window" + * result for empty pages returns the whole array regardless of order. + */ +function makeActions(reportActionIds: string[]): ReportAction[] { + return reportActionIds.map((reportActionID, index) => ({ + ...createRandomReportAction(index), + reportActionID, + created: `2024-01-01 10:0${reportActionIds.length - index}:00.000`, + })); +} + +/** + * Wire the three Onyx subscriptions usePaginatedReportActions makes: the report, its + * (already display-sorted) actions, and its pages. The selector on the actions key is + * bypassed by the mock, so we pass pre-sorted actions directly. + */ +function wireOnyx({report, actions, pages}: {report: Report | undefined; actions: ReportAction[] | undefined; pages: Pages | undefined}): void { + mockUseOnyx.mockImplementation((key: OnyxKey): MockOnyxResult => { + if (key === `${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`) { + return [report, {status: 'loaded'}]; + } + if (key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`) { + return [actions, {status: 'loaded'}]; + } + if (key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS_PAGES}${REPORT_ID}`) { + return [pages, {status: 'loaded'}]; + } + return [undefined, {status: 'loaded'}]; + }); +} + +function actionIds(actions: ReportAction[] | undefined): string[] { + return (actions ?? []).map((action) => action.reportActionID); +} + +describe('usePaginatedReportActions', () => { + beforeEach(() => { + mockUseOnyx.mockReset(); + jest.mocked(useReportIsArchived).mockReturnValue(false); + }); + + describe('when the linked action exists in this report (no behavior change)', () => { + it('anchors to the linked action and returns the report actions with pages absent', () => { + const actions = makeActions(['c', 'b', LINKED_ACTION_ID]); + wireOnyx({report: makeReport(), actions, pages: []}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, LINKED_ACTION_ID)); + + // Non-empty result and the linked action is surfaced — identical to pre-change behavior. + expect(actionIds(result.current.reportActions)).toEqual(['c', 'b', LINKED_ACTION_ID]); + expect(result.current.linkedAction?.reportActionID).toBe(LINKED_ACTION_ID); + }); + + it('anchors to the linked action within its page when pages are present', () => { + const actions = makeActions(['e', 'd', 'c', 'b', 'a']); + const pages: Pages = [['e', 'd', 'c', 'b', 'a']]; + wireOnyx({report: makeReport(), actions, pages}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, 'c')); + + expect(result.current.reportActions.length).toBeGreaterThan(0); + expect(result.current.linkedAction?.reportActionID).toBe('c'); + }); + }); + + describe('when the linked action lives in the merged transaction thread (the fix)', () => { + it('falls back to the newest window instead of returning an empty list, with pages absent', () => { + const actions = makeActions(['c', 'b', 'a']); + wireOnyx({report: makeReport(), actions, pages: []}); + + // SIBLING_ACTION_ID lives in the transaction thread; the caller confirms this via the flag. + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, SIBLING_ACTION_ID, {isLinkedActionInMergedTransactionThread: true})); + + // Without dropping the anchor this was [] (getContinuousChain empty-array behavior); now it is the newest window. + expect(actionIds(result.current.reportActions)).toEqual(['c', 'b', 'a']); + // No linked action is surfaced from this report — the host screen merges the thread separately. + expect(result.current.linkedAction).toBeUndefined(); + }); + + it('falls back to the newest page instead of returning an empty list, with pages present', () => { + const actions = makeActions(['e', 'd', 'c', 'b', 'a']); + const pages: Pages = [['e', 'd', 'c']]; + wireOnyx({report: makeReport(), actions, pages}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, SIBLING_ACTION_ID, {isLinkedActionInMergedTransactionThread: true})); + + expect(result.current.reportActions.length).toBeGreaterThan(0); + expect(result.current.linkedAction).toBeUndefined(); + }); + }); + + describe('when the linked action is absent but NOT confirmed in the merged thread (regression guard)', () => { + it('keeps the anchor so the not-yet-loaded action still positions the list once it hydrates', () => { + // Simulates a report whose older page holding SIBLING_ACTION_ID has not been fetched yet. Because the action is + // not confirmed to live in a merged transaction thread, the anchor is kept — getContinuousChain returns an empty + // window (loading state) rather than the newest window, preserving the initial scroll-to once OpenReport hydrates. + const actions = makeActions(['c', 'b', 'a']); + wireOnyx({report: makeReport(), actions, pages: []}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, SIBLING_ACTION_ID)); + + expect(result.current.reportActions).toEqual([]); + expect(result.current.linkedAction).toBeUndefined(); + }); + }); + + describe('unchanged paths', () => { + it('returns the newest window when no reportActionID is provided', () => { + const actions = makeActions(['c', 'b', 'a']); + wireOnyx({report: makeReport(), actions, pages: []}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID)); + + expect(actionIds(result.current.reportActions)).toEqual(['c', 'b', 'a']); + expect(result.current.linkedAction).toBeUndefined(); + }); + + it('ignores the anchor and never surfaces a linked action when treatAsNoPaginationAnchor is set, even if the action exists', () => { + const actions = makeActions(['c', 'b', LINKED_ACTION_ID]); + wireOnyx({report: makeReport(), actions, pages: []}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, LINKED_ACTION_ID, {treatAsNoPaginationAnchor: true})); + + expect(result.current.reportActions.length).toBeGreaterThan(0); + expect(result.current.linkedAction).toBeUndefined(); + }); + + it('resolves the oldest-unread anchor when shouldLinkToOldestUnreadReportAction is set and no reportActionID is provided', () => { + // lastReadTime is older than "b"/"c" but newer than "a", so the oldest unread action is "b". + const actions = makeActions(['c', 'b', 'a']); + const report = makeReport({lastReadTime: '2024-01-01 10:01:30.000'}); + wireOnyx({report, actions, pages: []}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, undefined, {shouldLinkToOldestUnreadReportAction: true})); + + expect(result.current.reportActions.length).toBeGreaterThan(0); + expect(result.current.oldestUnreadReportAction?.reportActionID).toBe('b'); + }); + + it('returns an empty list without crashing when the report has no actions', () => { + wireOnyx({report: makeReport(), actions: [], pages: []}); + + const {result} = renderHook(() => usePaginatedReportActions(REPORT_ID, SIBLING_ACTION_ID)); + + expect(result.current.reportActions).toEqual([]); + expect(result.current.linkedAction).toBeUndefined(); + }); + }); +}); diff --git a/tests/unit/shouldRedirectLinkedActionToParentReportTest.ts b/tests/unit/shouldRedirectLinkedActionToParentReportTest.ts new file mode 100644 index 000000000000..468f9fe7bc68 --- /dev/null +++ b/tests/unit/shouldRedirectLinkedActionToParentReportTest.ts @@ -0,0 +1,89 @@ +import {isOneTransactionThread, isReportTransactionThread} from '@libs/ReportUtils'; +// eslint-disable-next-line no-restricted-imports -- type-only namespace (erased at runtime) used solely for the requireActual generic; no restricted functions are actually imported +import type * as ReportUtilsModule from '@libs/ReportUtils'; + +import shouldRedirectLinkedActionToParentReport from '@pages/inbox/shouldRedirectLinkedActionToParentReport'; + +import type {Report, ReportAction} from '@src/types/onyx'; + +// Covers the open-time redirect decision for issue #86919. A copied link points at the report that owns the action (the +// transaction thread for a one-transaction expense). We redirect to the parent expense report ONLY while the thread is +// still the parent's only transaction, so the combined view with the parent's "Submitted" message opens. Once a second +// expense is added the redirect must stop firing, so the previously copied link still resolves on the thread itself. + +jest.mock('@libs/ReportUtils', () => ({ + __esModule: true, + ...jest.requireActual('@libs/ReportUtils'), + isReportTransactionThread: jest.fn(), + isOneTransactionThread: jest.fn(), +})); + +const mockIsReportTransactionThread = jest.mocked(isReportTransactionThread); +const mockIsOneTransactionThread = jest.mocked(isOneTransactionThread); + +const THREAD_REPORT_ID = 'transaction-thread-1'; +const PARENT_REPORT_ID = 'parent-expense-1'; +const LINKED_ACTION_ID = 'action-1'; + +const threadReport = {reportID: THREAD_REPORT_ID, parentReportID: PARENT_REPORT_ID} as Report; +const parentReport = {reportID: PARENT_REPORT_ID} as Report; +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- only passed through to isOneTransactionThread, which is mocked +const parentReportAction = {reportActionID: 'iou-action-1'} as ReportAction; + +const baseParams = { + report: threadReport, + parentReport, + parentReportAction, + reportActionIDFromRoute: LINKED_ACTION_ID, + isOffline: false, +}; + +describe('shouldRedirectLinkedActionToParentReport', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockIsReportTransactionThread.mockReturnValue(true); + mockIsOneTransactionThread.mockReturnValue(true); + }); + + it("redirects while the thread is still the parent report's only transaction", () => { + expect(shouldRedirectLinkedActionToParentReport(baseParams)).toBe(true); + expect(mockIsOneTransactionThread).toHaveBeenCalledWith(threadReport, parentReport, parentReportAction, false); + }); + + it('does NOT redirect once the parent report has more than one transaction, so the copied link still resolves on the thread', () => { + mockIsOneTransactionThread.mockReturnValue(false); + + expect(shouldRedirectLinkedActionToParentReport(baseParams)).toBe(false); + }); + + it('does NOT redirect when the route has no linked action (a plain thread visit)', () => { + expect(shouldRedirectLinkedActionToParentReport({...baseParams, reportActionIDFromRoute: undefined})).toBe(false); + // Short-circuits before the more expensive one-transaction check. + expect(mockIsOneTransactionThread).not.toHaveBeenCalled(); + }); + + it('does NOT redirect when the report is not a transaction thread (e.g. a regular chat deep link)', () => { + mockIsReportTransactionThread.mockReturnValue(false); + + expect(shouldRedirectLinkedActionToParentReport(baseParams)).toBe(false); + expect(mockIsOneTransactionThread).not.toHaveBeenCalled(); + }); + + it('does NOT redirect when the report has no parent to redirect to', () => { + // Deliberately missing parentReportID + const orphanReport = {reportID: THREAD_REPORT_ID} as Report; + + expect(shouldRedirectLinkedActionToParentReport({...baseParams, report: orphanReport})).toBe(false); + expect(mockIsOneTransactionThread).not.toHaveBeenCalled(); + }); + + it('does NOT redirect when the report is undefined', () => { + expect(shouldRedirectLinkedActionToParentReport({...baseParams, report: undefined})).toBe(false); + }); + + it('forwards the offline flag to the one-transaction check', () => { + shouldRedirectLinkedActionToParentReport({...baseParams, isOffline: true}); + + expect(mockIsOneTransactionThread).toHaveBeenCalledWith(threadReport, parentReport, parentReportAction, true); + }); +});