diff --git a/src/hooks/useUnreadMarker.ts b/src/hooks/useUnreadMarker.ts index bbd2018e9d72..46139d892e0b 100644 --- a/src/hooks/useUnreadMarker.ts +++ b/src/hooks/useUnreadMarker.ts @@ -53,6 +53,7 @@ type UseUnreadMarkerResult = { }; const lastReadTimeSelector = (report: OnyxTypes.Report | undefined) => report?.lastReadTime ?? ''; +const manuallyMarkedUnreadReportActionIDSelector = (report: OnyxTypes.Report | undefined) => report?.manuallyMarkedUnreadReportActionID ?? null; function useUnreadMarker({ reportID, @@ -74,6 +75,10 @@ function useUnreadMarker({ }); const reportLastReadTime = reportLastReadTimeValue ?? ''; + const [manuallyMarkedUnreadReportActionID] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + selector: manuallyMarkedUnreadReportActionIDSelector, + }); + const [unreadMarkerTime, setUnreadMarkerTime] = useState(reportLastReadTime); useEffect(() => { @@ -131,6 +136,7 @@ function useUnreadMarker({ isReversed, isAnonymousUser, prevUnreadMarkerReportActionID, + manuallyMarkedUnreadReportActionID, hasWindowFocus: Visibility.hasFocus(), newMessageBoundaryTime, }); diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index 1da9a991919b..615510be8cbc 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -456,6 +456,7 @@ function validateReportDraftProperty(key: keyof Report | keyof ReportNameValuePa case 'lastMessageText': case 'lastVisibleActionCreated': case 'lastReadTime': + case 'manuallyMarkedUnreadReportActionID': case 'lastMentionedTime': case 'policyAvatar': case 'policyName': @@ -639,6 +640,7 @@ function validateReportDraftProperty(key: keyof Report | keyof ReportNameValuePa lastMessageText: CONST.RED_BRICK_ROAD_PENDING_ACTION, lastVisibleActionCreated: CONST.RED_BRICK_ROAD_PENDING_ACTION, lastReadTime: CONST.RED_BRICK_ROAD_PENDING_ACTION, + manuallyMarkedUnreadReportActionID: CONST.RED_BRICK_ROAD_PENDING_ACTION, lastReadSequenceNumber: CONST.RED_BRICK_ROAD_PENDING_ACTION, lastMentionedTime: CONST.RED_BRICK_ROAD_PENDING_ACTION, policyAvatar: CONST.RED_BRICK_ROAD_PENDING_ACTION, diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 9ed3961d35c3..73a41bb027ac 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -379,6 +379,13 @@ type OpenReportActionParams = { hasReportActions: boolean | undefined; + /** + * Whether this report's actions loaded at least once this session (RAM-only, so falsy means a page refresh / + * cold start — when a manual unread marker is cleared). Only the report screen passes it; other callers omit + * it to leave the marker alone. + */ + hasOnceLoadedReportActions?: boolean; + /** Whether opening the report should update its read state. Set to false when fetching report data without the user actually viewing the conversation */ shouldMarkAsRead?: boolean; @@ -529,6 +536,19 @@ Onyx.connect({ }, }); +// RAM-only set of reportIDs the user navigated away from this session, so `openReport` can clear a manual +// unread marker on the return trip only. A blur uniquely identifies that trip: it doesn't fire on the repeated +// openReport calls of a single visit, and being RAM-only it is empty after a refresh. +const reportsNavigatedAwayFrom = new Set(); + +/** Records that the user navigated away from the report, so the next `openReport` clears its manual unread marker. */ +function flagReportNavigatedAway(reportID: string | undefined) { + if (!reportID) { + return; + } + reportsNavigatedAwayFrom.add(reportID); +} + let allPersonalDetails: OnyxEntry = {}; Onyx.connect({ key: ONYXKEYS.PERSONAL_DETAILS_LIST, @@ -1706,6 +1726,8 @@ function openReport(params: OpenReportActionParams) { isSelfTourViewed, hasCompletedGuidedSetupFlow, hasReportActions, + // Defaults to true so only the report screen, the one caller that passes it, can clear a manual unread marker. + hasOnceLoadedReportActions = true, shouldMarkAsRead = true, conciergeChat, } = params; @@ -1717,7 +1739,21 @@ function openReport(params: OpenReportActionParams) { const participantAccountIDList = participants.map((p) => p.accountID).filter((id): id is number => id !== undefined); const existingReportName = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportName; const isCreatingNewReport = !isEmptyObject(newReportObject); - const optimisticReport: Partial> = hasReportActions || !existingReportName ? {} : {reportName: existingReportName}; + // True only on a genuine return trip: `flagReportNavigatedAway` sets it on blur/unmount, so it is false on the + // first open, on the repeated openReport calls of a single visit, and after a refresh (the set is RAM-only). + const didNavigateBackToReport = reportsNavigatedAwayFrom.has(reportID); + reportsNavigatedAwayFrom.delete(reportID); + // A refresh resets the report screen's RAM-only `hasOnceLoadedReportActions`, which is how we detect one here. + // A genuine first open has no marker to clear, so this only affects a marker persisted from before the refresh. + const isFirstLoadAfterRefresh = !hasOnceLoadedReportActions; + const optimisticReport: Partial> = hasReportActions || !existingReportName ? {} : {reportName: existingReportName}; + + // A manual mark-as-unread keeps its marker anchored while the user stays in the report, and is cleared only on + // a return trip or a refresh. This is a client-side decision, so it goes in optimisticData to apply immediately + // and offline. It is deliberately not restored in failureData — that would resurrect a marker already moved past. + if (didNavigateBackToReport || isFirstLoadAfterRefresh) { + optimisticReport.manuallyMarkedUnreadReportActionID = null; + } const optimisticData: Array< OnyxUpdate< @@ -3192,6 +3228,8 @@ function readNewestAction(reportID: string | undefined, isReportActionsLoaded: b const lastReadTime = getDBTimeWithSkew(); + // Deliberately leaves `manuallyMarkedUnreadReportActionID` alone so an auto-read doesn't wipe a marker the + // user created. `openReport` clears it on a return trip or a refresh. const optimisticData: Array> = [ { onyxMethod: Onyx.METHOD.MERGE, @@ -3274,6 +3312,7 @@ function markCommentAsUnread(reportID: string | undefined, reportActions: OnyxEn const reportValue = { lastReadTime, + manuallyMarkedUnreadReportActionID: reportAction?.reportActionID ?? null, ...(lastActorAccountID && {lastActorAccountID}), }; @@ -3285,11 +3324,18 @@ function markCommentAsUnread(reportID: string | undefined, reportActions: OnyxEn }, ]; + // Deliberately omits `manuallyMarkedUnreadReportActionID`. If this request is still queued when `openReport` + // clears the marker (e.g. the mark happened offline), reasserting the id on reconnect would resurrect a marker + // the user has moved past. The optimistic value above persists on its own, since the server MERGE never + // carries this client-only field. const successData: Array> = [ { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, - value: reportValue, + value: { + lastReadTime, + ...(lastActorAccountID && {lastActorAccountID}), + }, }, ]; @@ -3300,6 +3346,7 @@ function markCommentAsUnread(reportID: string | undefined, reportActions: OnyxEn value: { lastReadTime: report?.lastReadTime ?? null, lastActorAccountID: report?.lastActorAccountID ?? null, + manuallyMarkedUnreadReportActionID: report?.manuallyMarkedUnreadReportActionID ?? null, }, }, ]; @@ -8934,6 +8981,7 @@ export { leaveRoom, markAsManuallyExported, markCommentAsUnread, + flagReportNavigatedAway, navigateToAndOpenChildReport, navigateToAndOpenReport, navigateToAndOpenReportWithAccountIDs, diff --git a/src/pages/inbox/ReportFetchHandler.tsx b/src/pages/inbox/ReportFetchHandler.tsx index 2c6956399d82..4c87a2b06996 100644 --- a/src/pages/inbox/ReportFetchHandler.tsx +++ b/src/pages/inbox/ReportFetchHandler.tsx @@ -41,6 +41,7 @@ import type {ReportsSplitNavigatorParamList, RightModalNavigatorParamList} from import { clearStaleDMRecoveryTargetByTargetReportID, createTransactionThreadReport, + flagReportNavigatedAway, joinReportViaSecureLink, markLocalReportActionsAsLoaded, openReport, @@ -235,6 +236,9 @@ function ReportFetchHandler() { betas, personalDetails, hasReportActions, + // Falsy means a page refresh / cold start, which is when openReport clears a manual unread marker. + // This screen opens the report the user is looking at, so it is the only caller that passes it. + hasOnceLoadedReportActions: reportLoadingState.hasOnceLoadedReportActions, currentUserAccountID, isSelfTourViewed, hasCompletedGuidedSetupFlow, @@ -432,6 +436,20 @@ function ReportFetchHandler() { }; }, []); + // Record navigating away so the next openReport can clear a manual unread marker on the return trip. We flag + // on blur (wide layout keeps the screen mounted) and on unmount / reportID change (narrow layout tears it + // down). Staying in the report never flags it, so the user's marker is not wiped mid-session. + useEffect(() => { + if (!prevIsFocused || isFocused) { + return; + } + flagReportNavigatedAway(reportIDFromRoute); + }, [isFocused, prevIsFocused, reportIDFromRoute]); + + useEffect(() => { + return () => flagReportNavigatedAway(reportIDFromRoute); + }, [reportIDFromRoute]); + // `isLoadingInitialReportActions` is memory-only and is not reset between navigations. A prior failed // fetch leaves a stale `false` that can make ReportNotFoundGuard show "not here" before the fetch below // re-runs. When opening a report whose actions were never successfully loaded, mark it as loading again so diff --git a/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts b/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts index 8ad0a9ea2a52..614c85116b00 100644 --- a/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts +++ b/src/pages/inbox/report/shouldDisplayNewMarkerOnReportAction.ts @@ -25,6 +25,12 @@ type ShouldDisplayNewMarkerOnReportActionParams = { /** The reportActionID of the current unread marker, if one exists */ prevUnreadMarkerReportActionID?: string | null; + + /** Whether the action `prevUnreadMarkerReportActionID` points to is still present (not deleted/hidden) */ + isPrevUnreadMarkerReportActionPresent?: boolean; + + /** The reportActionID the user explicitly marked as unread, if any */ + manuallyMarkedUnreadReportActionID?: string | null; /** Whether the app window is focused */ hasWindowFocus?: boolean; @@ -47,9 +53,19 @@ const shouldDisplayNewMarkerOnReportAction = ({ isScrolledOverThreshold, isOffline, prevUnreadMarkerReportActionID, + isPrevUnreadMarkerReportActionPresent = false, + manuallyMarkedUnreadReportActionID, hasWindowFocus = true, newMessageBoundaryTime, }: ShouldDisplayNewMarkerOnReportActionParams): boolean => { + // While a manual mark is active, the marked action is the sole anchor: every other action is suppressed. + // We anchor by reportActionID rather than timestamp because `created` shifts on the optimistic->confirmed + // transition and would wrongly read as already-read. The marked action is the oldest unread by construction + // (markCommentAsUnread sets lastReadTime = its created - 1ms), so it stays correct as newer messages arrive. + if (manuallyMarkedUnreadReportActionID) { + return message.reportActionID === manuallyMarkedUnreadReportActionID && !shouldHideNewMarker(message, isOffline); + } + const isNextMessageUnread = !!nextMessage && isReportActionUnread(nextMessage, unreadMarkerTime); // If the current message is the earliest message received while offline, we want to display the unread marker above this message. @@ -84,12 +100,16 @@ const shouldDisplayNewMarkerOnReportAction = ({ const isPreviouslyOptimistic = (isPendingAdd(prevSortedVisibleReportActionsObjects[message.reportActionID]) && !isPendingAdd(message)) || (!!prevSortedVisibleReportActionsObjects[message.reportActionID]?.isOptimisticAction && !message.isOptimisticAction); - const shouldIgnoreUnreadForCurrentUserMessage = isNewMessage || isPreviouslyOptimistic; + const prevMarkedReportAction = prevUnreadMarkerReportActionID ? prevSortedVisibleReportActionsObjects[prevUnreadMarkerReportActionID] : undefined; + const isPreviouslyUnreadFromCurrentUser = currentUserAccountID === prevMarkedReportAction?.actorAccountID; + // Once a self-authored action holds the marker, don't let a different self-authored action steal it (the + // Expensify/App#91940 hop). Only while that anchor is still present — if it was deleted, the marker must relocate. + const isDifferentUnread = isPrevUnreadMarkerReportActionPresent && isPreviouslyUnreadFromCurrentUser && prevMarkedReportAction?.reportActionID !== message.reportActionID; + const shouldIgnoreUnreadForCurrentUserMessage = isNewMessage || isPreviouslyOptimistic || isDifferentUnread; if (isFromCurrentUser) { - // When an existing marker is being relocated (e.g. after the original unread message is deleted), - // allow the marker to land on a self-authored action. - // Otherwise, never anchor the "New" marker above a self-authored action on first open/re-entry. + // Only move/keep the marker on a self-authored action when one already exists in this session. + // An explicit mark-as-unread bypasses this guard via the early return at the top of the function. if (prevUnreadMarkerReportActionID) { return !shouldIgnoreUnreadForCurrentUserMessage; } @@ -134,6 +154,9 @@ type GetUnreadMarkerReportActionParams = { /** The reportActionID of the current unread marker, if one exists */ prevUnreadMarkerReportActionID?: string | null; + + /** The reportActionID the user explicitly marked as unread, if any */ + manuallyMarkedUnreadReportActionID?: string | null; /** Whether the app window is focused */ hasWindowFocus?: boolean; @@ -157,6 +180,7 @@ const getUnreadMarkerReportAction = ({ isReversed, isAnonymousUser = false, prevUnreadMarkerReportActionID, + manuallyMarkedUnreadReportActionID, hasWindowFocus = true, newMessageBoundaryTime, }: GetUnreadMarkerReportActionParams): [string | null, number] => { @@ -164,6 +188,20 @@ const getUnreadMarkerReportAction = ({ return [null, -1]; } + // Drop the manual anchor once the marked action is deleted, otherwise no action would match it and the + // marker would vanish instead of relocating via the timestamp scan below. + const manuallyMarkedUnreadReportAction = manuallyMarkedUnreadReportActionID + ? visibleReportActions.find((action) => action.reportActionID === manuallyMarkedUnreadReportActionID) + : undefined; + const activeManuallyMarkedUnreadReportActionID = + manuallyMarkedUnreadReportAction && !shouldHideNewMarker(manuallyMarkedUnreadReportAction, isOffline) ? manuallyMarkedUnreadReportActionID : null; + + // Lets the caller tell "the anchor was deleted, so relocate the marker" apart from "the anchor is still + // around, so another self-authored action must not steal it". + const isPrevUnreadMarkerReportActionPresent = prevUnreadMarkerReportActionID + ? visibleReportActions.some((action) => action.reportActionID === prevUnreadMarkerReportActionID && !shouldHideNewMarker(action, isOffline)) + : false; + const startIndex = isReversed ? visibleReportActions.length - 1 : (earliestReceivedOfflineMessageIndex ?? 0); const endIndex = isReversed ? (earliestReceivedOfflineMessageIndex ?? 0) : visibleReportActions.length; const step = isReversed ? -1 : 1; @@ -199,6 +237,8 @@ const getUnreadMarkerReportAction = ({ isScrolledOverThreshold, isOffline, prevUnreadMarkerReportActionID, + isPrevUnreadMarkerReportActionPresent, + manuallyMarkedUnreadReportActionID: activeManuallyMarkedUnreadReportActionID, hasWindowFocus, newMessageBoundaryTime, }); diff --git a/src/selectors/Report.ts b/src/selectors/Report.ts index 9cb7714a62a7..798db21b733e 100644 --- a/src/selectors/Report.ts +++ b/src/selectors/Report.ts @@ -206,6 +206,7 @@ type ExcludedFields = ValidReportKeys< 'lastMessageText', 'lastVisibleActionCreated', 'lastReadTime', + 'manuallyMarkedUnreadReportActionID', 'lastReadSequenceNumber', 'lastMentionedTime', 'lastVisibleActionLastModified', diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index f8e2ba7d7372..639cdb4faa6a 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -132,6 +132,10 @@ type Report = OnyxCommon.OnyxValueWithOfflineFeedback< /** The time when user read the last message */ lastReadTime?: string; + /** reportActionID the user explicitly marked as unread. Unlike lastReadTime it is stable across the + * optimistic→confirmed transition, so the "New" marker can anchor on a self-authored action. */ + manuallyMarkedUnreadReportActionID?: string | null; + /** The sequence number of the last report visit */ lastReadSequenceNumber?: number; diff --git a/src/types/utils/whitelistedReportKeys.ts b/src/types/utils/whitelistedReportKeys.ts index 6aee1edc8a70..f77cc9927b81 100644 --- a/src/types/utils/whitelistedReportKeys.ts +++ b/src/types/utils/whitelistedReportKeys.ts @@ -15,6 +15,7 @@ type WhitelistedReport = OnyxCommon.OnyxValueWithOfflineFeedback< lastMessageText: unknown; lastVisibleActionCreated: unknown; lastReadTime: unknown; + manuallyMarkedUnreadReportActionID: unknown; lastReadSequenceNumber: unknown; lastMentionedTime: unknown; policyAvatar: unknown; diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 5689931fd9b6..4a565194806f 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -5445,6 +5445,75 @@ describe('actions/Report', () => { }); }); + describe('openReport with hasOnceLoadedReportActions', () => { + /** Puts a manual unread mark on the report, the way markCommentAsUnread does. */ + async function givenAManualUnreadMark(reportID: string) { + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {reportID, manuallyMarkedUnreadReportActionID: 'marked-action-id'}); + await waitForBatchedUpdates(); + } + + async function getManualUnreadMark(reportID: string) { + const report = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`); + return report?.manuallyMarkedUnreadReportActionID; + } + + it('should clear the manual unread mark when false, because a falsy value means a page refresh / cold start', async () => { + global.fetch = TestHelper.createGlobalFetchMock(); + const REPORT_ID = 'unreadMarkRefresh'; + await givenAManualUnreadMark(REPORT_ID); + + Report.openReport({ + conciergeChat: undefined, + reportID: REPORT_ID, + introSelected: undefined, + betas: undefined, + hasReportActions: true, + currentUserAccountID: 1, + hasOnceLoadedReportActions: false, + }); + await waitForBatchedUpdates(); + + expect(await getManualUnreadMark(REPORT_ID)).toBeFalsy(); + }); + + it('should keep the manual unread mark when true, so the marker survives the repeated openReport calls of a single visit', async () => { + global.fetch = TestHelper.createGlobalFetchMock(); + const REPORT_ID = 'unreadMarkSameVisit'; + await givenAManualUnreadMark(REPORT_ID); + + Report.openReport({ + conciergeChat: undefined, + reportID: REPORT_ID, + introSelected: undefined, + betas: undefined, + hasReportActions: true, + currentUserAccountID: 1, + hasOnceLoadedReportActions: true, + }); + await waitForBatchedUpdates(); + + expect(await getManualUnreadMark(REPORT_ID)).toBe('marked-action-id'); + }); + + it('should keep the manual unread mark when omitted, so callers other than the report screen never clear it', async () => { + global.fetch = TestHelper.createGlobalFetchMock(); + const REPORT_ID = 'unreadMarkOtherCaller'; + await givenAManualUnreadMark(REPORT_ID); + + Report.openReport({ + conciergeChat: undefined, + reportID: REPORT_ID, + introSelected: undefined, + betas: undefined, + hasReportActions: true, + currentUserAccountID: 1, + }); + await waitForBatchedUpdates(); + + expect(await getManualUnreadMark(REPORT_ID)).toBe('marked-action-id'); + }); + }); + describe('openReport with participants', () => { it('should send passed participants as emailList/accountIDList so the server can resolve a stale optimistic reportID', async () => { global.fetch = TestHelper.createGlobalFetchMock(); diff --git a/tests/unit/ReportActionsUtilsTest.ts b/tests/unit/ReportActionsUtilsTest.ts index 346fddd54022..bb8394298b45 100644 --- a/tests/unit/ReportActionsUtilsTest.ts +++ b/tests/unit/ReportActionsUtilsTest.ts @@ -6367,7 +6367,9 @@ describe('ReportActionsUtils', () => { ).toBe(false); }); - it('returns false when message is from current user and is already present (not new, not optimistic) and no existing marker', () => { + it('returns false for a self-authored already-present action on a cold open when no marker exists and it was not explicitly marked unread (Expensify/App#91940 guard)', () => { + // A persisted self-authored action (e.g. a reimbursable toggle) that reads as unread must not anchor + // the marker on a cold open. An explicit mark-as-unread is handled separately, by the tests below. const message = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'existing-action-id'}); const prevSortedVisibleReportActionsObjects = { [message.reportActionID]: makeAction({actorAccountID: currentUserAccountID, reportActionID: 'existing-action-id'}), @@ -6399,6 +6401,65 @@ describe('ReportActionsUtils', () => { ).toBe(true); }); + it('does not move the marker from one self-authored action to a different self-authored action while the previous anchor is still present', () => { + // The previous anchor is still present, so `isDifferentUnread` stops this action stealing the marker + // off it (the Expensify/App#91940 hop) even though it reads as unread. + const message = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-b'}); + const prevMarkedAction = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-a'}); + const prevSortedVisibleReportActionsObjects = { + [prevMarkedAction.reportActionID]: prevMarkedAction, + [message.reportActionID]: makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-b'}), + }; + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + prevSortedVisibleReportActionsObjects, + prevUnreadMarkerReportActionID: 'self-action-a', + isPrevUnreadMarkerReportActionPresent: true, + isOffline: false, + }), + ).toBe(false); + }); + + it('moves the marker to another self-authored action once the previous anchor has been deleted', () => { + // The previous anchor was deleted, so the marker must be free to relocate to the next unread message. + const message = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-b'}); + const prevMarkedAction = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-a'}); + const prevSortedVisibleReportActionsObjects = { + [prevMarkedAction.reportActionID]: prevMarkedAction, + [message.reportActionID]: makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-b'}), + }; + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + prevSortedVisibleReportActionsObjects, + prevUnreadMarkerReportActionID: 'self-action-a', + isPrevUnreadMarkerReportActionPresent: false, + isOffline: false, + }), + ).toBe(true); + }); + + it('keeps the marker on the same self-authored action it was previously anchored on', () => { + // The action being evaluated is the previous anchor, so `isDifferentUnread` is false and it keeps the marker. + const message = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-a'}); + const prevSortedVisibleReportActionsObjects = { + [message.reportActionID]: makeAction({actorAccountID: currentUserAccountID, reportActionID: 'self-action-a'}), + }; + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + prevSortedVisibleReportActionsObjects, + prevUnreadMarkerReportActionID: 'self-action-a', + isPrevUnreadMarkerReportActionPresent: true, + isOffline: false, + }), + ).toBe(true); + }); + it('returns true when an unread message from another user is new and the list is scrolled over the threshold', () => { const message = makeAction({reportActionID: 'other-new-id'}); expect( @@ -6440,6 +6501,93 @@ describe('ReportActionsUtils', () => { }), ).toBe(true); }); + + it('anchors the marker on the explicitly marked-unread action even after its confirmed created drifts before unreadMarkerTime', () => { + // The offline→online case: the confirmed `created` lands before unreadMarkerTime, so the timestamp + // check reads the action as "read" and only the stable id can still anchor the marker. + const message = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'marked-action-id', pendingAction: null, created: '2023-01-01 09:00:00.000'}); + const prevSortedVisibleReportActionsObjects = { + [message.reportActionID]: makeAction({ + actorAccountID: currentUserAccountID, + reportActionID: 'marked-action-id', + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }), + }; + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + prevSortedVisibleReportActionsObjects, + manuallyMarkedUnreadReportActionID: 'marked-action-id', + isOffline: false, + }), + ).toBe(true); + }); + + it('does not anchor the marker on a just-sent self-message when no action is marked unread', () => { + // Same confirmed self-message, but with nothing marked unread the just-sent suppression still applies, + // keeping the #91443 fix intact. + const message = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'confirmed-action-id', pendingAction: null}); + const prevSortedVisibleReportActionsObjects = { + [message.reportActionID]: makeAction({ + actorAccountID: currentUserAccountID, + reportActionID: 'confirmed-action-id', + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }), + }; + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + prevSortedVisibleReportActionsObjects, + manuallyMarkedUnreadReportActionID: null, + isOffline: false, + }), + ).toBe(false); + }); + + it('keeps the marker on the explicitly marked-unread action even when a newer message is present', () => { + // The marked action is the oldest unread by construction (lastReadTime = its created - 1ms), so a + // newer message arriving after the mark must not steal the marker off it. + const message = makeAction({actorAccountID: currentUserAccountID, reportActionID: 'marked-action-id', created: '2023-01-01 11:00:00.000'}); + const nextMessage = makeAction({created: '2023-01-01 11:30:00.000'}); + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + nextMessage, + manuallyMarkedUnreadReportActionID: 'marked-action-id', + isOffline: false, + }), + ).toBe(true); + }); + + it('returns false for any action that is not the marked one while a manual mark is active (sole anchor)', () => { + // The marked action is the sole anchor, so even an unread message from another user is suppressed. + const message = makeAction({actorAccountID: 99, reportActionID: 'other-action-id', created: '2023-01-01 11:00:00.000'}); + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + manuallyMarkedUnreadReportActionID: 'marked-action-id', + isOffline: false, + }), + ).toBe(false); + }); + + it('returns false for the earliest-received-offline message while a different action is marked unread', () => { + // The manual mark takes precedence over the earliest-received-offline branch. + const message = makeAction({actorAccountID: 99, reportActionID: 'offline-action-id', created: '2023-01-01 11:00:00.000'}); + expect( + shouldDisplayNewMarkerOnReportAction({ + ...baseParams, + message, + isEarliestReceivedOfflineMessage: true, + manuallyMarkedUnreadReportActionID: 'marked-action-id', + isOffline: false, + }), + ).toBe(false); + }); }); describe('getUnreadMarkerReportAction', () => { diff --git a/tests/unit/useUnreadMarkerTest.ts b/tests/unit/useUnreadMarkerTest.ts index bc332d42e694..fb7c8894173b 100644 --- a/tests/unit/useUnreadMarkerTest.ts +++ b/tests/unit/useUnreadMarkerTest.ts @@ -29,13 +29,15 @@ jest.mock('@hooks/useIsAnonymousUser', () => ({ default: () => mockIsAnonymousUser, })); -// The hook subscribes to `${ONYXKEYS.COLLECTION.REPORT}${reportID}` with a selector that returns -// `lastReadTime`. The implementation is set in beforeEach so it can use ONYXKEYS freely (a jest.mock -// factory cannot reference out-of-scope variables). -const mockUseOnyx = jest.fn<[string], [string]>(); +// The hook subscribes to `${ONYXKEYS.COLLECTION.REPORT}${reportID}` twice with different selectors, so the mock +// applies the passed selector to a fake report. The implementation is set in beforeEach so it can use ONYXKEYS +// freely (a jest.mock factory cannot reference out-of-scope variables). +type FakeReport = Pick; +type UseOnyxOptions = {selector?: (value: FakeReport | undefined) => unknown}; +const mockUseOnyx = jest.fn<[unknown], [string, UseOnyxOptions?]>(); jest.mock('@hooks/useOnyx', () => ({ __esModule: true, - default: (key: string) => mockUseOnyx(key), + default: (key: string, options?: UseOnyxOptions) => mockUseOnyx(key, options), })); function makeAction(reportActionID: string, overrides: Partial = {}): OnyxTypes.ReportAction { @@ -67,9 +69,10 @@ describe('useUnreadMarker', () => { mockIsAnonymousUser = false; mockLastReadTime = LAST_READ_TIME; mockLastReadTimeByReportID = {}; - mockUseOnyx.mockImplementation((key) => { + mockUseOnyx.mockImplementation((key, options) => { const reportID = key.replace(ONYXKEYS.COLLECTION.REPORT, ''); - return [mockLastReadTimeByReportID[reportID] ?? mockLastReadTime]; + const report: FakeReport = {lastReadTime: mockLastReadTimeByReportID[reportID] ?? mockLastReadTime}; + return [options?.selector ? options.selector(report) : report]; }); });