diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index d3694c553ed3..728cc0a97b77 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -190,6 +190,7 @@ function MoneyRequestReportActionsList({ const [session] = useOnyx(ONYXKEYS.SESSION); const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${getNonEmptyStringOnyxID(reportID)}`); + const [reportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`); const shouldShowHarvestCreatedAction = isHarvestCreatedExpenseReport(reportNameValuePairs?.origin, reportNameValuePairs?.originalID); const [offlineModalVisible, setOfflineModalVisible] = useState(false); const [isDownloadErrorModalVisible, setIsDownloadErrorModalVisible] = useState(false); @@ -370,13 +371,14 @@ function MoneyRequestReportActionsList({ if (!isFocused) { return; } + if (isUnread(report, transactionThreadReport, isReportArchived) || (lastAction && isCurrentActionUnread(report, lastAction, visibleReportActions))) { // On desktop, when the notification center is displayed, isVisible will return false. // Currently, there's no programmatic way to dismiss the notification center panel. // To handle this, we use the 'referrer' parameter to check if the current navigation is triggered from a notification. const isFromNotification = route?.params?.referrer === CONST.REFERRER.NOTIFICATION; if ((isVisible || isFromNotification) && scrollingVerticalBottomOffset.current < CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD) { - readNewestAction(report.reportID); + readNewestAction(report.reportID, !!reportMetadata?.hasOnceLoadedReportActions); if (isFromNotification) { Navigation.setParams({referrer: undefined}); } @@ -385,7 +387,7 @@ function MoneyRequestReportActionsList({ } } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [report.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, report.reportID, isVisible]); + }, [report.lastVisibleActionCreated, transactionThreadReport?.lastVisibleActionCreated, report.reportID, isVisible, reportMetadata?.hasOnceLoadedReportActions]); useEffect(() => { if (!isVisible || !isFocused) { @@ -409,7 +411,7 @@ function MoneyRequestReportActionsList({ return; } - readNewestAction(report.reportID); + readNewestAction(report.reportID, !!reportMetadata?.hasOnceLoadedReportActions); 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 @@ -417,7 +419,7 @@ function MoneyRequestReportActionsList({ // We will mark the report as read in the above case which marks the LHN report item as read while showing the new message // marker for the chat messages received while the user wasn't focused on the report or on another browser tab for web. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isFocused, isVisible]); + }, [isFocused, isVisible, reportMetadata?.hasOnceLoadedReportActions]); /** * The index of the earliest message that was received while offline @@ -488,6 +490,7 @@ function MoneyRequestReportActionsList({ // We additionally track the top offset to be able to scroll to the new transaction when it's added scrollingVerticalTopOffset.current = contentOffset.y; }, + hasOnceLoadedReportActions: !!reportMetadata?.hasOnceLoadedReportActions, }); useEffect(() => { @@ -673,8 +676,8 @@ function MoneyRequestReportActionsList({ reportScrollManager.scrollToEnd(); readActionSkipped.current = false; - readNewestAction(report.reportID); - }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, report.reportID]); + readNewestAction(report.reportID, !!reportMetadata?.hasOnceLoadedReportActions); + }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, report.reportID, reportMetadata?.hasOnceLoadedReportActions]); const scrollToNewTransaction = useCallback( (pageY: number) => { diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index d57d5bb49541..0dd596311a83 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -1916,14 +1916,21 @@ function expandURLPreview(reportID: string | undefined, reportActionID: string) } /** Marks the new report actions as read + * @param hasOnceLoadedReportActions Whether the report actions have been loaded at least once. + * If false, the API call will be skipped to avoid 401 errors from reading reports not yet shared with the user. * @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, shouldResetUnreadMarker = false) { +function readNewestAction(reportID: string | undefined, hasOnceLoadedReportActions: 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 + if (!hasOnceLoadedReportActions) { + return; + } + const lastReadTime = NetworkConnection.getDBTimeWithSkew(); const optimisticData: Array> = [ diff --git a/src/pages/inbox/ReportScreen.tsx b/src/pages/inbox/ReportScreen.tsx index 8555b5de909e..139f6065a3d1 100644 --- a/src/pages/inbox/ReportScreen.tsx +++ b/src/pages/inbox/ReportScreen.tsx @@ -925,8 +925,8 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr 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); - }, [report]); + readNewestAction(report?.reportID, !!reportMetadata?.hasOnceLoadedReportActions); + }, [report, reportMetadata?.hasOnceLoadedReportActions]); // Reset the ref when navigating to a different report useEffect(() => { diff --git a/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx b/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx index 8317bcf38455..a3ed7f05cdc8 100644 --- a/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx +++ b/src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx @@ -504,7 +504,7 @@ const ContextMenuActions: ContextMenuAction[] = [ successIcon: 'Checkmark', shouldShow: ({type, isUnreadChat}) => type === CONST.CONTEXT_MENU_TYPES.REPORT && isUnreadChat, onPress: (closePopover, {reportID}) => { - readNewestAction(reportID, true); + readNewestAction(reportID, true, true); if (closePopover) { hideContextMenu(true, ReportActionComposeFocusManager.focus); } diff --git a/src/pages/inbox/report/ReportActionsList.tsx b/src/pages/inbox/report/ReportActionsList.tsx index a6920c430459..4585aae475fe 100644 --- a/src/pages/inbox/report/ReportActionsList.tsx +++ b/src/pages/inbox/report/ReportActionsList.tsx @@ -368,6 +368,7 @@ function ReportActionsList({ setShouldScrollToEndAfterLayout(false); } }, + hasOnceLoadedReportActions: !!reportMetadata?.hasOnceLoadedReportActions, }); useEffect(() => { @@ -402,18 +403,13 @@ function ReportActionsList({ return; } - // Do not try to mark the report as read if the report has not been loaded and shared with the user - if (!reportMetadata?.hasOnceLoadedReportActions) { - return; - } - if (isUnread(report, transactionThreadReport, isReportArchived) || (lastAction && isCurrentActionUnread(report, lastAction, sortedVisibleReportActions))) { // On desktop, when the notification center is displayed, isVisible will return false. // Currently, there's no programmatic way to dismiss the notification center panel. // To handle this, we use the 'referrer' parameter to check if the current navigation is triggered from a notification. const isFromNotification = route?.params?.referrer === CONST.REFERRER.NOTIFICATION; if ((isVisible || isFromNotification) && scrollOffsetRef.current < CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD) { - readNewestAction(report.reportID); + readNewestAction(report.reportID, !!reportMetadata?.hasOnceLoadedReportActions); if (isFromNotification) { Navigation.setParams({referrer: undefined}); } @@ -455,7 +451,7 @@ function ReportActionsList({ return; } - readNewestAction(report.reportID); + readNewestAction(report.reportID, !!reportMetadata?.hasOnceLoadedReportActions); userActiveSince.current = DateUtils.getDBTime(); return true; @@ -464,7 +460,7 @@ function ReportActionsList({ // We will mark the report as read in the above case which marks the LHN report item as read while showing the new message // marker for the chat messages received while the user wasn't focused on the report or on another browser tab for web. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isFocused, isVisible]); + }, [isFocused, isVisible, reportMetadata?.hasOnceLoadedReportActions]); const prevHandleReportChangeMarkAsRead = useRef<() => void>(null); const prevHandleAppVisibilityMarkAsRead = useRef<() => void>(null); @@ -631,8 +627,8 @@ function ReportActionsList({ } reportScrollManager.scrollToBottom(); readActionSkipped.current = false; - readNewestAction(report.reportID); - }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, report.reportID, backTo, introSelected]); + readNewestAction(report.reportID, !!reportMetadata?.hasOnceLoadedReportActions); + }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, report.reportID, backTo, introSelected, reportMetadata?.hasOnceLoadedReportActions]); /** * Calculates the ideal number of report actions to render in the first render, based on the screen height and on diff --git a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts index 12016b2b56d5..5d9948649ba9 100644 --- a/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts +++ b/src/pages/inbox/report/useReportUnreadMessageScrollTracking.ts @@ -23,6 +23,9 @@ type Args = { /** Callback to call on every scroll event */ onTrackScrolling: (event: NativeSyntheticEvent) => void; + + /** Whether the report actions have been loaded at least once */ + hasOnceLoadedReportActions: boolean; }; export default function useReportUnreadMessageScrollTracking({ @@ -32,14 +35,16 @@ export default function useReportUnreadMessageScrollTracking({ onTrackScrolling, unreadMarkerReportActionIndex, isInverted, + hasOnceLoadedReportActions, }: Args) { const [isFloatingMessageCounterVisible, setIsFloatingMessageCounterVisible] = useState(false); const isFocused = useIsFocused(); - const ref = useRef<{previousViewableItems: ViewToken[]; reportID: string; unreadMarkerReportActionIndex: number; isFocused: boolean}>({ + const ref = useRef<{previousViewableItems: ViewToken[]; reportID: string; unreadMarkerReportActionIndex: number; isFocused: boolean; hasOnceLoadedReportActions: boolean}>({ reportID, unreadMarkerReportActionIndex, previousViewableItems: [], isFocused: true, + hasOnceLoadedReportActions, }); // We want to save the updated value on ref to use it in onViewableItemsChanged // because FlatList requires the callback to be stable and we cannot add a dependency on the useCallback. @@ -52,6 +57,10 @@ export default function useReportUnreadMessageScrollTracking({ ref.current.isFocused = isFocused; }, [isFocused]); + useEffect(() => { + ref.current.hasOnceLoadedReportActions = hasOnceLoadedReportActions; + }, [hasOnceLoadedReportActions]); + /** * On every scroll event we want to: * Show/hide the latest message pill when user is scrolling back/forth in the history of messages. @@ -113,7 +122,7 @@ export default function useReportUnreadMessageScrollTracking({ if (unreadActionVisible && readActionSkippedRef.current) { // eslint-disable-next-line no-param-reassign readActionSkippedRef.current = false; - readNewestAction(ref.current.reportID); + readNewestAction(ref.current.reportID, ref.current.hasOnceLoadedReportActions); } // FlatList requires a stable onViewableItemsChanged callback for optimal performance. diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 7fb51a9cf157..ce8e696a78b9 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -452,7 +452,7 @@ describe('actions/Report', () => { // When the user visits the report currentTime = DateUtils.getDBTime(); Report.openReport(REPORT_ID, TEST_INTRO_SELECTED); - Report.readNewestAction(REPORT_ID); + Report.readNewestAction(REPORT_ID, true); waitForBatchedUpdates(); return waitForBatchedUpdates(); }) diff --git a/tests/ui/UnreadIndicatorsTest.tsx b/tests/ui/UnreadIndicatorsTest.tsx index f8c38a31bb85..c4fb2ce87997 100644 --- a/tests/ui/UnreadIndicatorsTest.tsx +++ b/tests/ui/UnreadIndicatorsTest.tsx @@ -672,7 +672,7 @@ describe('Unread Indicators', () => { // Given a read report await signInAndGetAppWithUnreadChat(); - readNewestAction(REPORT_ID, true); + readNewestAction(REPORT_ID, true, true); await waitForBatchedUpdates(); diff --git a/tests/unit/useReportUnreadMessageScrollTrackingTest.ts b/tests/unit/useReportUnreadMessageScrollTrackingTest.ts index 68ce4f0609ae..beda62ec194c 100644 --- a/tests/unit/useReportUnreadMessageScrollTrackingTest.ts +++ b/tests/unit/useReportUnreadMessageScrollTrackingTest.ts @@ -40,6 +40,7 @@ describe('useReportUnreadMessageScrollTracking', () => { unreadMarkerReportActionIndex: -1, isInverted: true, onTrackScrolling: onTrackScrollingMockFn, + hasOnceLoadedReportActions: true, }), ); @@ -69,6 +70,7 @@ describe('useReportUnreadMessageScrollTracking', () => { isInverted: true, unreadMarkerReportActionIndex: -1, onTrackScrolling: onTrackScrollingMockFn, + hasOnceLoadedReportActions: true, }), ); @@ -95,6 +97,7 @@ describe('useReportUnreadMessageScrollTracking', () => { isInverted: true, unreadMarkerReportActionIndex: 1, onTrackScrolling: onTrackScrollingMockFn, + hasOnceLoadedReportActions: true, }), ); @@ -126,6 +129,7 @@ describe('useReportUnreadMessageScrollTracking', () => { unreadMarkerReportActionIndex: -1, isInverted: true, onTrackScrolling: onTrackScrollingMockFn, + hasOnceLoadedReportActions: true, }), ); @@ -151,6 +155,7 @@ describe('useReportUnreadMessageScrollTracking', () => { unreadMarkerReportActionIndex: 1, isInverted: true, onTrackScrolling: onTrackScrollingMockFn, + hasOnceLoadedReportActions: true, }), ); @@ -180,6 +185,7 @@ describe('useReportUnreadMessageScrollTracking', () => { unreadMarkerReportActionIndex: 1, isInverted: true, onTrackScrolling: onTrackScrollingMockFn, + hasOnceLoadedReportActions: true, }), );