Remove allReportActions usage from readNewestAction

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) => {

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<ReportActions>) {
+    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;

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<PlatformStackRouteProp<ReportsSplitNavigatorParamList, typeof SCREENS.REPORT>>();
     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 = () => {

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();

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;

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<OnyxTypes.Report>;
-            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();

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);
+    });
+});

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', () => {