diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 3c9be156c740..80214996756d 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -1,6 +1,7 @@ import FlatListWithScrollKey from '@components/FlatList/FlatListWithScrollKey'; import ScrollView from '@components/ScrollView'; +import useAppFocusEvent from '@hooks/useAppFocusEvent'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useIsReportActionsLoaded from '@hooks/useIsReportActionsLoaded'; import useLoadReportActions from '@hooks/useLoadReportActions'; @@ -355,6 +356,11 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) return unsubscribe; }, []); + // A visible browser window can regain OS focus without any visibility change, and nothing else re-runs the + // read catch-up in that case, so bump a counter on app focus to re-run it. + const [appFocusCount, setAppFocusCount] = useState(0); + useAppFocusEvent(useCallback(() => setAppFocusCount((count) => count + 1), [])); + useEffect(() => { if (!isFocused) { return; @@ -412,7 +418,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) // marker for the chat messages received while the user wasn't focused on the report or on another browser tab for web. // This effect should only run when app visibility/focus changes; the helper reads the latest report/action values without making every action update mark the report as read. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isFocused, isVisible]); + }, [isFocused, isVisible, appFocusCount]); /** * The index of the earliest message that was received while offline diff --git a/src/hooks/useMarkAsRead.ts b/src/hooks/useMarkAsRead.ts index 4408ff3a1b6e..58842a847a56 100644 --- a/src/hooks/useMarkAsRead.ts +++ b/src/hooks/useMarkAsRead.ts @@ -16,9 +16,10 @@ import type * as OnyxTypes from '@src/types/onyx'; import type {OnyxEntry} from 'react-native-onyx'; import {useIsFocused, useRoute} from '@react-navigation/native'; -import {useEffect, useEffectEvent, useRef, useState} from 'react'; +import {useCallback, useEffect, useEffectEvent, useRef, useState} from 'react'; import {DeviceEventEmitter} from 'react-native'; +import useAppFocusEvent from './useAppFocusEvent'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; import useIsAnonymousUser from './useIsAnonymousUser'; import useIsReportActionsLoaded from './useIsReportActionsLoaded'; @@ -61,6 +62,11 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible return unsubscribe; }, []); + // A visible browser window can regain OS focus without any visibility change, and nothing else re-runs the + // read catch-up in that case, so bump a counter on app focus to re-run it. + const [appFocusCount, setAppFocusCount] = useState(0); + useAppFocusEvent(useCallback(() => setAppFocusCount((count) => count + 1), [])); + const readActionSkippedRef = useRef(false); const userActiveSince = useRef(DateUtils.getDBTime()); const lastMessageTime = useRef(null); @@ -146,6 +152,12 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible return; } + // readNewestAction marks everything up to now as read, so newer actions outside the loaded slice would be + // consumed without ever being seen. Mirrors the !hasNewerActions guard on the report-change path. + if (hasNewerActions) { + return; + } + const newMessageTimeReference = lastMessageTime.current && report?.lastReadTime && lastMessageTime.current > report.lastReadTime ? userActiveSince.current : report?.lastReadTime; lastMessageTime.current = null; @@ -169,7 +181,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible // Only re-run when app visibility/focus changes, so action updates don't keep marking the report as read. useEffect(() => { handleAppVisibilityMarkAsRead(isFocused); - }, [isVisible, isFocused]); + }, [isVisible, isFocused, appFocusCount]); const markNewestActionAsRead = () => { readActionSkippedRef.current = false; diff --git a/tests/unit/useMarkAsReadTest.ts b/tests/unit/useMarkAsReadTest.ts index 19bc0238994b..be93ef83d88d 100644 --- a/tests/unit/useMarkAsReadTest.ts +++ b/tests/unit/useMarkAsReadTest.ts @@ -10,6 +10,8 @@ import type * as OnyxTypes from '@src/types/onyx'; import type {OnyxEntry} from 'react-native-onyx'; +import createRandomReportAction from '../utils/collections/reportActions'; + const REPORT_ID = '1'; let mockIsUnread = true; @@ -27,6 +29,14 @@ jest.mock('@libs/Visibility', () => ({ }, })); +let mockTriggerAppFocus: (() => void) | undefined; +jest.mock('@hooks/useAppFocusEvent', () => ({ + __esModule: true, + default: (callback: () => void) => { + mockTriggerAppFocus = callback; + }, +})); + jest.mock('@libs/ReportUtils', () => { const actual = jest.requireActual('@libs/ReportUtils'); return { @@ -137,4 +147,72 @@ describe('useMarkAsRead', () => { expect(readNewestAction).toHaveBeenCalledTimes(1); expect(readNewestAction).toHaveBeenCalledWith(REPORT_ID, false); }); + + it('marks the report as read when the window regains focus after a message arrived while it was unfocused', () => { + const readReport = {reportID: REPORT_ID, lastReadTime: '2023-01-01 10:00:00.000', lastVisibleActionCreated: '2023-01-01 10:00:00.000'} as OnyxTypes.Report; + const reportWithNewMessage = {...readReport, lastVisibleActionCreated: '2023-01-01 11:00:00.000'} as OnyxTypes.Report; + const incomingAction: OnyxTypes.ReportAction = {...createRandomReportAction(2), created: '2023-01-01 11:00:00.000', actorAccountID: 2}; + + // The user is viewing the newest message of an already read report. + mockIsUnread = false; + const {rerender} = renderHook( + (props: {report: OnyxTypes.Report; actions: OnyxTypes.ReportAction[]}) => + useMarkAsRead({ + reportID: REPORT_ID, + report: props.report as OnyxEntry, + transactionThreadReport: undefined, + sortedVisibleReportActions: props.actions, + isScrolledToEnd: true, + hasNewerActions: false, + }), + {initialProps: {report: readReport, actions: [] as OnyxTypes.ReportAction[]}}, + ); + readNewestAction.mockClear(); + + // The user clicks into another desktop app, so the still-visible window loses focus without a visibility change. + mockHasFocus = false; + + // A message from somebody else arrives while the window is unfocused, so the mark-as-read is skipped. + mockIsUnread = true; + rerender({report: reportWithNewMessage, actions: [incomingAction]}); + expect(readNewestAction).not.toHaveBeenCalled(); + + // The user clicks back into the window, which regains focus without any visibility change. + mockHasFocus = true; + act(() => mockTriggerAppFocus?.()); + + expect(readNewestAction).toHaveBeenCalledWith(REPORT_ID, true); + }); + + it('does not mark the report as read when the window regains focus while newer actions are still unloaded', () => { + const readReport = {reportID: REPORT_ID, lastReadTime: '2023-01-01 10:00:00.000', lastVisibleActionCreated: '2023-01-01 10:00:00.000'} as OnyxTypes.Report; + const reportWithNewMessage = {...readReport, lastVisibleActionCreated: '2023-01-01 11:00:00.000'} as OnyxTypes.Report; + const incomingAction: OnyxTypes.ReportAction = {...createRandomReportAction(2), created: '2023-01-01 11:00:00.000', actorAccountID: 2}; + + // The user is at the end of an older paginated slice, so newer actions exist but are not loaded yet. + mockIsUnread = false; + const {rerender} = renderHook( + (props: {report: OnyxTypes.Report; actions: OnyxTypes.ReportAction[]}) => + useMarkAsRead({ + reportID: REPORT_ID, + report: props.report as OnyxEntry, + transactionThreadReport: undefined, + sortedVisibleReportActions: props.actions, + isScrolledToEnd: true, + hasNewerActions: true, + }), + {initialProps: {report: readReport, actions: [] as OnyxTypes.ReportAction[]}}, + ); + readNewestAction.mockClear(); + + mockHasFocus = false; + mockIsUnread = true; + rerender({report: reportWithNewMessage, actions: [incomingAction]}); + + // Regaining focus must not consume the unread state of the newer actions the user has never seen. + mockHasFocus = true; + act(() => mockTriggerAppFocus?.()); + + expect(readNewestAction).not.toHaveBeenCalled(); + }); });