diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 63cb094142f..3cc3062dac3 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -8,6 +8,7 @@ import {DeviceEventEmitter, View} from 'react-native'; import FlatListWithScrollKey from '@components/FlatList/FlatListWithScrollKey'; import ScrollView from '@components/ScrollView'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; +import useIsReportActionsLoaded from '@hooks/useIsReportActionsLoaded'; import useLoadReportActions from '@hooks/useLoadReportActions'; import useLocalize from '@hooks/useLocalize'; import useNetworkWithOfflineStatus from '@hooks/useNetworkWithOfflineStatus'; @@ -106,6 +107,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${getNonEmptyStringOnyxID(report?.policyID)}`); const [reportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportIDFromRoute}`); const [reportPaginationState] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_PAGINATION_STATE}${reportIDFromRoute}`); + const isReportActionsLoaded = useIsReportActionsLoaded(reportIDFromRoute); const reportID = report?.reportID; const {reportActions: unfilteredReportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID, route?.params?.reportActionID); @@ -358,7 +360,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) const shouldReadOnReportChange = ((isVisible && Visibility.hasFocus()) || isFromNotification) && isScrolledToEnd; if (shouldReadOnReportChange) { - readNewestAction(report?.reportID, !!reportLoadingState?.hasOnceLoadedReportActions); + readNewestAction(report?.reportID, isReportActionsLoaded); if (isFromNotification) { Navigation.setParams({referrer: undefined}); } @@ -368,7 +370,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) } // This effect should only run when the newest visible action changes, otherwise every action/report object update can prematurely consume unread state. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [report?.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, report?.reportID, isVisible, reportLoadingState?.hasOnceLoadedReportActions]); + }, [report?.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, report?.reportID, isVisible, isReportActionsLoaded]); useEffect(() => { if (!isVisible || !Visibility.hasFocus() || !isFocused) { @@ -392,7 +394,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) return; } - readNewestAction(report?.reportID, !!reportLoadingState?.hasOnceLoadedReportActions); + readNewestAction(report?.reportID, true); userActiveSince.current = DateUtils.getDBTime(); // This effect logic to `mark as read` will only run when the report focused has new messages and the App visibility @@ -401,7 +403,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, reportLoadingState?.hasOnceLoadedReportActions]); + }, [isFocused, isVisible]); /** * The index of the earliest message that was received while offline @@ -438,7 +440,7 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) return; } readActionSkipped.current = false; - readNewestAction(report?.reportID, !!reportLoadingState?.hasOnceLoadedReportActions); + readNewestAction(report?.reportID, isReportActionsLoaded); }, unreadMarkerReportActionIndex, isInverted: false, @@ -618,8 +620,8 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps) reportScrollManager.scrollToEnd(); readActionSkipped.current = false; - readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions); - }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, reportID, reportLoadingState?.hasOnceLoadedReportActions, introSelected, betas]); + readNewestAction(reportID, true); + }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, reportID, introSelected, betas]); const scrollToNewTransaction = useCallback( (pageY: number) => { diff --git a/src/hooks/useIsReportActionsLoaded.tsx b/src/hooks/useIsReportActionsLoaded.tsx new file mode 100644 index 00000000000..88a7fe88b4c --- /dev/null +++ b/src/hooks/useIsReportActionsLoaded.tsx @@ -0,0 +1,18 @@ +import type {OnyxEntry} from 'react-native-onyx'; +import ONYXKEYS from '@src/ONYXKEYS'; +import {hasOnceLoadedReportActionsSelector} from '@src/selectors/ReportMetaData'; +import type {ReportActions} from '@src/types/onyx'; +import {isEmptyObject} from '@src/types/utils/EmptyObject'; +import useOnyx from './useOnyx'; + +function hasReportActionsSelector(reportActions: OnyxEntry) { + return !isEmptyObject(reportActions); +} + +function useIsReportActionsLoaded(reportID: string | undefined) { + const [hasOnceLoadedReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportID}`, {selector: hasOnceLoadedReportActionsSelector}); + const [hasReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {selector: hasReportActionsSelector}); + return !!hasOnceLoadedReportActions || !!hasReportActions; +} + +export default useIsReportActionsLoaded; diff --git a/src/hooks/useMarkAsRead.ts b/src/hooks/useMarkAsRead.ts index 9e64cf0d9ae..b50c5ba893d 100644 --- a/src/hooks/useMarkAsRead.ts +++ b/src/hooks/useMarkAsRead.ts @@ -11,12 +11,11 @@ import Visibility from '@libs/Visibility'; import type {ReportsSplitNavigatorParamList} from '@navigation/types'; import {readNewestAction} from '@userActions/Report'; import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; import type SCREENS from '@src/SCREENS'; import type * as OnyxTypes from '@src/types/onyx'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; import useIsAnonymousUser from './useIsAnonymousUser'; -import useOnyx from './useOnyx'; +import useIsReportActionsLoaded from './useIsReportActionsLoaded'; import useReportIsArchived from './useReportIsArchived'; // useRef gets reset when the reportID changes (the list reuses the same instance per report), @@ -46,8 +45,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible const route = useRoute>(); const isFocused = useIsFocused(); const isReportArchived = useReportIsArchived(reportID); - - const [reportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportID}`); + const isReportActionsLoaded = useIsReportActionsLoaded(reportID); const [isVisible, setIsVisible] = useState(Visibility.isVisible); useEffect(() => { @@ -89,8 +87,8 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible } didMarkReportAsReadInitially.current = true; - readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions); - }, [isReportUnreadValue, reportID, reportLoadingState?.hasOnceLoadedReportActions]); + readNewestAction(reportID, isReportActionsLoaded); + }, [isReportUnreadValue, reportID, isReportActionsLoaded]); const didMarkOnReportChangeRef = useRef(false); @@ -108,7 +106,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible const shouldReadOnReportChange = ((isVisible && Visibility.hasFocus()) || isFromNotification) && !hasNewerActions && isScrolledToEnd; if (shouldReadOnReportChange) { - readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions); + readNewestAction(reportID, isReportActionsLoaded); if (isFromNotification) { Navigation.setParams({referrer: undefined}); } @@ -119,7 +117,7 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible readActionSkippedRef.current = true; // This effect should only run when the newest visible action changes, otherwise every action/report object update can prematurely consume unread state. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [report?.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, reportID, isVisible, reportLoadingState?.hasOnceLoadedReportActions]); + }, [report?.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, reportID, isVisible, isReportActionsLoaded]); useEffect(() => { if (didMarkOnReportChangeRef.current) { @@ -153,15 +151,15 @@ function useMarkAsRead({reportID, report, transactionThreadReport, sortedVisible return; } - readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions); + readNewestAction(reportID, true); userActiveSince.current = DateUtils.getDBTime(); // 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 - }, [isVisible, isFocused, reportLoadingState?.hasOnceLoadedReportActions]); + }, [isVisible, isFocused]); const markNewestActionAsRead = () => { readActionSkippedRef.current = false; - readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions); + readNewestAction(reportID, true); }; const completeSkippedMarkAsRead = () => { diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 797fe8686a6..4b2f3b62ce0 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -2730,20 +2730,14 @@ function expandURLPreview(reportID: string | undefined, reportActionID: string) * @param shouldResetUnreadMarker Indicates whether the unread indicator should be reset. * Currently, the unread indicator needs to be reset only when users mark a report as read. */ -function readNewestAction(reportID: string | undefined, hasOnceLoadedReportActions: boolean, shouldResetUnreadMarker = false) { +function readNewestAction(reportID: string | undefined, isReportActionsLoaded: boolean, shouldResetUnreadMarker = false) { if (!reportID) { return; } // Do not try to mark the report as read if the report has not been loaded and shared with the user. - // However, if report actions already exist in Onyx (e.g., delivered via Pusher), the report is - // clearly shared with the user and we can proceed with marking it as read. - if (!hasOnceLoadedReportActions) { - const reportActions = allReportActions?.[reportID]; - const hasReportActions = !!reportActions && Object.keys(reportActions).length > 0; - if (!hasReportActions) { - return; - } + if (!isReportActionsLoaded) { + return; } const lastReadTime = getDBTimeWithSkew(); diff --git a/src/pages/inbox/ReportFetchHandler.tsx b/src/pages/inbox/ReportFetchHandler.tsx index 54fee8dc52b..65c49964981 100644 --- a/src/pages/inbox/ReportFetchHandler.tsx +++ b/src/pages/inbox/ReportFetchHandler.tsx @@ -6,6 +6,7 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails' import useIsAnonymousUser from '@hooks/useIsAnonymousUser'; import useIsInSidePanel from '@hooks/useIsInSidePanel'; import useIsOwnWorkspaceChatRef from '@hooks/useIsOwnWorkspaceChatRef'; +import useIsReportActionsLoaded from '@hooks/useIsReportActionsLoaded'; import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; import usePaginatedReportActions from '@hooks/usePaginatedReportActions'; @@ -87,6 +88,7 @@ function ReportFetchHandler() { const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportOnyx?.chatReportID}`); const [reportMetadata = defaultReportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportIDFromRoute}`); const [reportLoadingState = defaultReportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportIDFromRoute}`); + const isReportActionsLoaded = useIsReportActionsLoaded(reportIDFromRoute); const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED); const [betas] = useOnyx(ONYXKEYS.BETAS); const [onboarding] = useOnyx(ONYXKEYS.NVP_ONBOARDING); @@ -341,8 +343,8 @@ function ReportFetchHandler() { return; } // After creating the task report then navigating to task detail we don't have any report actions and the last read time is empty so We need to update the initial last read time when opening the task report detail. - readNewestAction(report?.reportID, !!reportLoadingState?.hasOnceLoadedReportActions); - }, [report, reportLoadingState?.hasOnceLoadedReportActions]); + readNewestAction(report?.reportID, isReportActionsLoaded); + }, [report, isReportActionsLoaded]); useEffect(() => { hasCreatedLegacyThreadRef.current = false; diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 6cce37254f0..cda3f43ea3a 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -7804,71 +7804,6 @@ describe('actions/Report', () => { }); }); - describe('readNewestAction', () => { - it('should mark a report as read when hasOnceLoadedReportActions is false but report actions exist in Onyx', () => { - global.fetch = TestHelper.getGlobalFetchMock(); - const REPORT_ID = '1'; - const USER_1_LOGIN = 'user@test.com'; - const USER_1_ACCOUNT_ID = 1; - const USER_2_ACCOUNT_ID = 2; - - let report: OnyxEntry; - Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, - callback: (val) => (report = val), - }); - - const reportActionCreatedDate = DateUtils.getDBTime(); - - return TestHelper.signInWithTestUser(USER_1_ACCOUNT_ID, USER_1_LOGIN) - .then(waitForNetworkPromises) - .then(() => TestHelper.setPersonalDetails(USER_1_LOGIN, USER_1_ACCOUNT_ID)) - .then(() => { - // Set up a report with actions in Onyx (as if delivered via Pusher from a new user) - // but without hasOnceLoadedReportActions being set (simulating offline + new chat) - return Promise.all([ - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, { - reportID: REPORT_ID, - participants: { - [USER_1_ACCOUNT_ID]: {notificationPreference: 'always'}, - }, - lastMessageText: 'Hello from new user', - lastActorAccountID: USER_2_ACCOUNT_ID, - lastVisibleActionCreated: reportActionCreatedDate, - lastReadTime: DateUtils.subtractMillisecondsFromDateTime(reportActionCreatedDate, 1), - }), - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, { - 1: { - actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, - actorAccountID: USER_2_ACCOUNT_ID, - automatic: false, - avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png', - message: [{type: 'COMMENT', html: 'Hello from new user', text: 'Hello from new user'}], - person: [{type: 'TEXT', style: 'strong', text: 'New User'}], - shouldShow: true, - created: reportActionCreatedDate, - reportActionID: '1', - }, - }), - ]); - }) - .then(waitForBatchedUpdates) - .then(() => { - // Verify the report is currently unread - expect(ReportUtils.isUnread(report, undefined, undefined)).toBe(true); - - // Call readNewestAction with hasOnceLoadedReportActions = false - // This simulates the scenario where a chat from a new user is opened offline - Report.readNewestAction(REPORT_ID, false); - return waitForBatchedUpdates(); - }) - .then(() => { - // The report should now be read because report actions exist in Onyx - expect(ReportUtils.isUnread(report, undefined, undefined)).toBe(false); - }); - }); - }); - describe('resolveActionableMentionWhisper', () => { it('should optimistically add invited users to report.participants when resolution is INVITE', async () => { global.fetch = TestHelper.getGlobalFetchMock(); diff --git a/tests/unit/useIsReportActionsLoadedTest.ts b/tests/unit/useIsReportActionsLoadedTest.ts new file mode 100644 index 00000000000..5e0ca91246b --- /dev/null +++ b/tests/unit/useIsReportActionsLoadedTest.ts @@ -0,0 +1,61 @@ +import {act, renderHook} from '@testing-library/react-native'; +import Onyx from 'react-native-onyx'; +import useIsReportActionsLoaded from '@hooks/useIsReportActionsLoaded'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {ReportAction} from '@src/types/onyx'; + +const REPORT_ID = '1'; + +const REPORT_ACTION = { + reportActionID: '100', + actionName: 'ADDCOMMENT', + created: '2023-01-01 10:00:00.000', +} as ReportAction; + +describe('useIsReportActionsLoaded', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + afterEach(() => Onyx.clear()); + + it('returns false when neither the loading state nor report actions are loaded', async () => { + const {result} = renderHook(() => useIsReportActionsLoaded(REPORT_ID)); + expect(result.current).toBe(false); + }); + + it('returns true when hasOnceLoadedReportActions is true in the loading state', async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${REPORT_ID}`, {hasOnceLoadedReportActions: true}); + + const {result} = renderHook(() => useIsReportActionsLoaded(REPORT_ID)); + expect(result.current).toBe(true); + }); + + it('returns true when the report has report actions even if it has never finished loading', async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {[REPORT_ACTION.reportActionID]: REPORT_ACTION}); + + const {result} = renderHook(() => useIsReportActionsLoaded(REPORT_ID)); + expect(result.current).toBe(true); + }); + + it('returns false when the report actions object is empty', async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {}); + + const {result} = renderHook(() => useIsReportActionsLoaded(REPORT_ID)); + expect(result.current).toBe(false); + }); + + it('returns false when reportID is undefined', async () => { + const {result} = renderHook(() => useIsReportActionsLoaded(undefined)); + expect(result.current).toBe(false); + }); + + it('updates from false to true when report actions become available', async () => { + const {result} = renderHook(() => useIsReportActionsLoaded(REPORT_ID)); + expect(result.current).toBe(false); + + await act(async () => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {[REPORT_ACTION.reportActionID]: REPORT_ACTION})); + + expect(result.current).toBe(true); + }); +}); diff --git a/tests/unit/useMarkAsReadTest.ts b/tests/unit/useMarkAsReadTest.ts index b44cdc6e00b..804e9b48f5e 100644 --- a/tests/unit/useMarkAsReadTest.ts +++ b/tests/unit/useMarkAsReadTest.ts @@ -103,7 +103,7 @@ describe('useMarkAsRead', () => { act(() => result.current.completeSkippedMarkAsRead()); - expect(readNewestAction).toHaveBeenCalledWith(REPORT_ID, false); + expect(readNewestAction).toHaveBeenCalledWith(REPORT_ID, true); }); it('does not complete a mark-as-read when none was skipped', () => {