Merge pull request Expensify#33162 from s-alves10/fix/issue-33118

fix: hide new marker for pending deleted action in online mode
bernhardoj · Jan 9, 2024 · 06c64de · 06c64de
2 parents 99c900e + a070efa
commit 06c64de
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 17 deletions.
diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts
@@ -378,10 +378,20 @@ function shouldReportActionBeVisible(reportAction: OnyxEntry<ReportAction>, key:
 
     // All other actions are displayed except thread parents, deleted, or non-pending actions
     const isDeleted = isDeletedAction(reportAction);
-    const isPending = !!reportAction.pendingAction && !(!isNetworkOffline && reportAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
+    const isPending = !!reportAction.pendingAction;
     return !isDeleted || isPending || isDeletedParentAction(reportAction) || isReversedTransaction(reportAction);
 }
 
+/**
+ * Checks if the new marker should be hidden for the report action.
+ */
+function shouldHideNewMarker(reportAction: OnyxEntry<ReportAction>): boolean {
+    if (!reportAction) {
+        return true;
+    }
+    return !isNetworkOffline && reportAction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
+}
+
 /**
  * Checks if a reportAction is fit for display as report last action, meaning that
  * it satisfies shouldReportActionBeVisible, it's not whisper action and not deleted.
@@ -841,6 +851,7 @@ export {
     isWhisperAction,
     isReimbursementQueuedAction,
     shouldReportActionBeVisible,
+    shouldHideNewMarker,
     shouldReportActionBeVisibleAsLastAction,
     hasRequestFromCurrentAccount,
     getFirstVisibleReportActionID,

diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js
@@ -18,7 +18,6 @@ import withCurrentReportID, {withCurrentReportIDDefaultProps, withCurrentReportI
 import withViewportOffsetTop from '@components/withViewportOffsetTop';
 import useAppFocusEvent from '@hooks/useAppFocusEvent';
 import useLocalize from '@hooks/useLocalize';
-import useNetwork from '@hooks/useNetwork';
 import usePrevious from '@hooks/usePrevious';
 import useThemeStyles from '@hooks/useThemeStyles';
 import useWindowDimensions from '@hooks/useWindowDimensions';
@@ -159,7 +158,6 @@ function ReportScreen({
     const styles = useThemeStyles();
     const {translate} = useLocalize();
     const {isSmallScreenWidth} = useWindowDimensions();
-    const {isOffline} = useNetwork();
 
     const firstRenderRef = useRef(true);
     const flatListRef = useRef();
@@ -180,11 +178,8 @@ function ReportScreen({
     const {addWorkspaceRoomOrChatPendingAction, addWorkspaceRoomOrChatErrors} = ReportUtils.getReportOfflinePendingActionAndErrors(report);
     const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}];
 
-    // eslint-disable-next-line react-hooks/exhaustive-deps -- need to re-filter the report actions when network status changes
-    const filteredReportActions = useMemo(() => ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true), [isOffline, reportActions]);
-
     // There are no reportActions at all to display and we are still in the process of loading the next set of actions.
-    const isLoadingInitialReportActions = _.isEmpty(filteredReportActions) && reportMetadata.isLoadingInitialReportActions;
+    const isLoadingInitialReportActions = _.isEmpty(reportActions) && reportMetadata.isLoadingInitialReportActions;
 
     const isOptimisticDelete = lodashGet(report, 'statusNum') === CONST.REPORT.STATUS.CLOSED;
 
@@ -491,7 +486,7 @@ function ReportScreen({
                             >
                                 {isReportReadyForDisplay && !isLoadingInitialReportActions && !isLoading && (
                                     <ReportActionsView
-                                        reportActions={filteredReportActions}
+                                        reportActions={reportActions}
                                         report={report}
                                         isLoadingInitialReportActions={reportMetadata.isLoadingInitialReportActions}
                                         isLoadingNewerReportActions={reportMetadata.isLoadingNewerReportActions}
@@ -509,7 +504,7 @@ function ReportScreen({
                                 {isReportReadyForDisplay ? (
                                     <ReportFooter
                                         pendingAction={addWorkspaceRoomOrChatPendingAction}
-                                        reportActions={filteredReportActions}
+                                        reportActions={reportActions}
                                         report={report}
                                         isComposerFullSize={isComposerFullSize}
                                         onSubmitComment={onSubmitComment}
@@ -544,6 +539,7 @@ export default compose(
             reportActions: {
                 key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getReportID(route)}`,
                 canEvict: false,
+                selector: (reportActions) => ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true),
             },
             report: {
                 key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`,

diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js
@@ -159,7 +159,10 @@ function ReportActionsList({
     const lastVisibleActionCreatedRef = useRef(report.lastVisibleActionCreated);
     const lastReadTimeRef = useRef(report.lastReadTime);
 
-    const sortedVisibleReportActions = _.filter(sortedReportActions, (s) => isOffline || s.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || s.errors);
+    const sortedVisibleReportActions = useMemo(
+        () => _.filter(sortedReportActions, (s) => isOffline || s.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || s.errors),
+        [sortedReportActions, isOffline],
+    );
     const lastActionIndex = lodashGet(sortedVisibleReportActions, [0, 'reportActionID']);
     const reportActionSize = useRef(sortedVisibleReportActions.length);
 
@@ -354,9 +357,9 @@ function ReportActionsList({
         (reportAction, index) => {
             let shouldDisplay = false;
             if (!currentUnreadMarker) {
-                const nextMessage = sortedReportActions[index + 1];
+                const nextMessage = sortedVisibleReportActions[index + 1];
                 const isCurrentMessageUnread = isMessageUnread(reportAction, lastReadTimeRef.current);
-                shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, lastReadTimeRef.current));
+                shouldDisplay = isCurrentMessageUnread && (!nextMessage || !isMessageUnread(nextMessage, lastReadTimeRef.current)) && !ReportActionsUtils.shouldHideNewMarker(reportAction);
                 if (shouldDisplay && !messageManuallyMarkedUnread) {
                     const isWithinVisibleThreshold = scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD ? reportAction.created < userActiveSince.current : true;
                     // Prevent displaying a new marker line when report action is of type "REPORTPREVIEW" and last actor is the current user
@@ -373,15 +376,15 @@ function ReportActionsList({
 
             return shouldDisplay;
         },
-        [currentUnreadMarker, sortedReportActions, report.reportID, messageManuallyMarkedUnread],
+        [currentUnreadMarker, sortedVisibleReportActions, report.reportID, messageManuallyMarkedUnread],
     );
 
     useEffect(() => {
         // Iterate through the report actions and set appropriate unread marker.
         // This is to avoid a warning of:
         // Cannot update a component (ReportActionsList) while rendering a different component (CellRenderer).
         let markerFound = false;
-        _.each(sortedReportActions, (reportAction, index) => {
+        _.each(sortedVisibleReportActions, (reportAction, index) => {
             if (!shouldDisplayNewMarker(reportAction, index)) {
                 return;
             }
@@ -394,7 +397,7 @@ function ReportActionsList({
         if (!markerFound) {
             setCurrentUnreadMarker(null);
         }
-    }, [sortedReportActions, report.lastReadTime, report.reportID, messageManuallyMarkedUnread, shouldDisplayNewMarker, currentUnreadMarker]);
+    }, [sortedVisibleReportActions, report.lastReadTime, report.reportID, messageManuallyMarkedUnread, shouldDisplayNewMarker, currentUnreadMarker]);
 
     const renderItem = useCallback(
         ({item: reportAction, index}) => (

diff --git a/tests/unit/ReportActionsUtilsTest.js b/tests/unit/ReportActionsUtilsTest.js
@@ -288,7 +288,7 @@ describe('ReportActionsUtils', () => {
             expect(result).toStrictEqual(input);
         });
 
-        it('should filter out deleted and delete-pending comments', () => {
+        it('should filter out deleted, non-pending comments', () => {
             const input = [
                 {
                     created: '2022-11-13 22:27:01.825',
@@ -312,7 +312,6 @@ describe('ReportActionsUtils', () => {
             ];
             const result = ReportActionsUtils.getSortedReportActionsForDisplay(input);
             input.pop();
-            input.pop();
             expect(result).toStrictEqual(input);
         });
     });