Sort reportActions by created instead of sequenceNumber

src/libs/ReportActionsUtils.js

@@ -39,23 +39,46 @@ function isDeletedAction(reportAction) {
 }
 
 /**
- * Sorts the report actions by sequence number, filters out any that should not be shown and formats them for display.
+ * Sort an array of reportActions by their created timestamp first, and reportActionID second
+ * This gives us a stable order even in the case of multiple reportActions created on the same millisecond
  *
  * @param {Array} reportActions
+ * @param {Boolean} shouldSortInDescendingOrder
  * @returns {Array}
  */
-function getSortedReportActions(reportActions) {
-    return _.chain(reportActions)
-        .sortBy('sequenceNumber')
-        .filter(action => action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU
-
-            // All comment actions are shown unless they are deleted and non-pending
-            || (action.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT && (!isDeletedAction(action) || !_.isEmpty(action.pendingAction)))
-            || action.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED
-            || action.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED)
-        .map((item, index) => ({action: item, index}))
-        .value()
-        .reverse();
+function getSortedReportActions(reportActions, shouldSortInDescendingOrder = false) {
+    if (!_.isArray(reportActions)) {
+        throw new Error(`ReportActionsUtils.getSortedReportActions requires an array, received ${typeof reportActions}`);
+    }
+    const invertedMultiplier = shouldSortInDescendingOrder ? -1 : 1;
+    reportActions.sort((first, second) => {
+        if (first.created !== second.created) {
+            return (first.created < second.created ? -1 : 1) * invertedMultiplier;
+        }
+
+        return (first.reportActionID < second.reportActionID ? -1 : 1) * invertedMultiplier;
+    });
+    return reportActions;
+}
+
+/**
+ * Filter out any reportActions which should not be displayed.
+ *
+ * @param {Array} reportActions
+ * @returns {Array}
+ */
+function filterReportActionsForDisplay(reportActions) {
+    return _.filter(reportActions, (reportAction) => {
+        // Filter out any unsupported reportAction types
+        if (!_.has(CONST.REPORT.ACTIONS.TYPE, reportAction.actionName)) {
+            return false;
+        }
+
+        // All other actions are displayed except deleted, non-pending actions
+        const isDeleted = isDeletedAction(reportAction);
+        const isPending = !_.isEmpty(reportAction.pendingAction);
+        return !isDeleted || isPending;
+    });
 }
 
 /**
@@ -65,10 +88,13 @@ function getSortedReportActions(reportActions) {
  * @returns {String}
  */
 function getMostRecentIOUReportActionID(reportActions) {
-    return _.chain(reportActions)
-        .where({actionName: CONST.REPORT.ACTIONS.TYPE.IOU})
-        .max(action => action.sequenceNumber)
-        .value().reportActionID;
+    const iouActions = _.where(reportActions, {actionName: CONST.REPORT.ACTIONS.TYPE.IOU});
+    if (_.isEmpty(iouActions)) {
+        return null;
+    }
+
+    const sortedReportActions = getSortedReportActions(iouActions);
+    return _.last(sortedReportActions).reportActionID;
 }
 
 /**
@@ -82,7 +108,7 @@ function getMostRecentIOUReportActionID(reportActions) {
 function isConsecutiveActionMadeByPreviousActor(reportActions, actionIndex) {
     // Find the next non-pending deletion report action, as the pending delete action means that it is not displayed in the UI, but still is in the report actions list.
     // If we are offline, all actions are pending but shown in the UI, so we take the previous action, even if it is a delete.
-    const previousAction = _.find(_.drop(reportActions, actionIndex + 1), action => isNetworkOffline || (action.action.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE));
+    const previousAction = _.find(_.drop(reportActions, actionIndex + 1), action => isNetworkOffline || (action.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE));
     const currentAction = reportActions[actionIndex];
 
     // It's OK for there to be no previous action, and in that case, false will be returned
@@ -92,17 +118,17 @@ function isConsecutiveActionMadeByPreviousActor(reportActions, actionIndex) {
     }
 
     // Comments are only grouped if they happen within 5 minutes of each other
-    if (moment(currentAction.action.created).unix() - moment(previousAction.action.created).unix() > 300) {
+    if (moment(currentAction.created).unix() - moment(previousAction.created).unix() > 300) {
         return false;
     }
 
     // Do not group if previous or current action was a renamed action
-    if (previousAction.action.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED
-        || currentAction.action.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED) {
+    if (previousAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED
+        || currentAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED) {
         return false;
     }
 
-    return currentAction.action.actorEmail === previousAction.action.actorEmail;
+    return currentAction.actorEmail === previousAction.actorEmail;
 }
 
 /**
@@ -114,7 +140,7 @@ function isConsecutiveActionMadeByPreviousActor(reportActions, actionIndex) {
 function getLastVisibleMessageText(reportID, actionsToMerge = {}) {
     const parser = new ExpensiMark();
     const actions = _.toArray(lodashMerge({}, allReportActions[reportID], actionsToMerge));
-    const sortedActions = _.sortBy(actions, 'sequenceNumber');
+    const sortedActions = getSortedReportActions(actions);
     const lastMessageIndex = _.findLastIndex(sortedActions, action => (
         !isDeletedAction(action)
     ));
@@ -142,7 +168,7 @@ function getOptimisticLastReadSequenceNumberForDeletedAction(reportID, actionsTo
 
     // Otherwise, we must find the first previous index of an action that is not deleted and less than the lastReadSequenceNumber
     const actions = _.toArray(lodashMerge({}, allReportActions[reportID], actionsToMerge));
-    const sortedActions = _.sortBy(actions, 'sequenceNumber');
+    const sortedActions = getSortedReportActions(actions);
     const lastMessageIndex = _.findLastIndex(sortedActions, action => (
         !isDeletedAction(action) && action.sequenceNumber <= lastReadSequenceNumber
     ));
@@ -156,9 +182,10 @@ function getOptimisticLastReadSequenceNumberForDeletedAction(reportID, actionsTo
 }
 
 export {
+    getSortedReportActions,
+    filterReportActionsForDisplay,
     getOptimisticLastReadSequenceNumberForDeletedAction,
     getLastVisibleMessageText,
-    getSortedReportActions,
     getMostRecentIOUReportActionID,
     isDeletedAction,
     isConsecutiveActionMadeByPreviousActor,

src/pages/home/report/ReportActionsList.js

@@ -30,13 +30,7 @@ const propTypes = {
     report: reportPropTypes.isRequired,
 
     /** Sorted actions prepared for display */
-    sortedReportActions: PropTypes.arrayOf(PropTypes.shape({
-        /** Index of the action in the array */
-        index: PropTypes.number,
-
-        /** The action itself */
-        action: PropTypes.shape(reportActionPropTypes),
-    })).isRequired,
+    sortedReportActions: PropTypes.arrayOf(PropTypes.shape(reportActionPropTypes)).isRequired,
 
     /** The ID of the most recent IOU report action connected with the shown report */
     mostRecentIOUReportActionID: PropTypes.string,
@@ -108,7 +102,7 @@ class ReportActionsList extends React.Component {
      * @return {String}
      */
     keyExtractor(item) {
-        return item.action.reportActionID;
+        return item.reportActionID;
     }
 
     /**
@@ -118,27 +112,26 @@ class ReportActionsList extends React.Component {
      * See: https://reactnative.dev/docs/optimizing-flatlist-configuration#avoid-anonymous-function-on-renderitem
      *
      * @param {Object} args
-     * @param {Object} args.item
      * @param {Number} args.index
      *
      * @returns {React.Component}
      */
     renderItem({
-        item,
+        item: reportAction,
         index,
     }) {
         // When the new indicator should not be displayed we explicitly set it to 0. The marker should never be shown above the
         // created action (which will have sequenceNumber of 0) so we use 0 to indicate "hidden".
         const shouldDisplayNewIndicator = this.props.newMarkerSequenceNumber > 0
-            && item.action.sequenceNumber === this.props.newMarkerSequenceNumber
-            && !ReportActionsUtils.isDeletedAction(item.action);
+            && reportAction.sequenceNumber === this.props.newMarkerSequenceNumber
+            && !ReportActionsUtils.isDeletedAction(reportAction);
         return (
             <ReportActionItem
                 report={this.props.report}
-                action={item.action}
+                action={reportAction}
                 displayAsGroup={ReportActionsUtils.isConsecutiveActionMadeByPreviousActor(this.props.sortedReportActions, index)}
                 shouldDisplayNewIndicator={shouldDisplayNewIndicator}
-                isMostRecentIOUReportAction={item.action.reportActionID === this.props.mostRecentIOUReportActionID}
+                isMostRecentIOUReportAction={reportAction.reportActionID === this.props.mostRecentIOUReportActionID}
                 hasOutstandingIOU={this.props.report.hasOutstandingIOU}
                 index={index}
             />

src/pages/home/report/ReportActionsView.js

@@ -74,7 +74,7 @@ class ReportActionsView extends React.Component {
         };
 
         this.currentScrollOffset = 0;
-        this.sortedReportActions = ReportActionsUtils.getSortedReportActions(props.reportActions);
+        this.sortedReportActions = this.getSortedReportActionsForDisplay(props.reportActions);
         this.mostRecentIOUReportActionID = ReportActionsUtils.getMostRecentIOUReportActionID(props.reportActions);
         this.trackScroll = this.trackScroll.bind(this);
         this.toggleFloatingMessageCounter = this.toggleFloatingMessageCounter.bind(this);
@@ -130,7 +130,7 @@ class ReportActionsView extends React.Component {
 
     shouldComponentUpdate(nextProps, nextState) {
         if (!_.isEqual(nextProps.reportActions, this.props.reportActions)) {
-            this.sortedReportActions = ReportActionsUtils.getSortedReportActions(nextProps.reportActions);
+            this.sortedReportActions = this.getSortedReportActionsForDisplay(nextProps.reportActions);
             this.mostRecentIOUReportActionID = ReportActionsUtils.getMostRecentIOUReportActionID(nextProps.reportActions);
             return true;
         }
@@ -247,6 +247,15 @@ class ReportActionsView extends React.Component {
         Report.unsubscribeFromReportChannel(this.props.report.reportID);
     }
 
+    /**
+     * @param {Object} reportActions
+     * @returns {Array}
+     */
+    getSortedReportActionsForDisplay(reportActions) {
+        const sortedReportActions = ReportActionsUtils.getSortedReportActions(_.values(reportActions), true);
+        return ReportActionsUtils.filterReportActionsForDisplay(sortedReportActions);
+    }
+
     /**
      * @returns {Boolean}
      */

tests/ui/UnreadIndicatorsTest.js

@@ -145,6 +145,18 @@ function signInAndGetAppWithUnreadChat() {
                     sequenceNumber: 0,
                     created: MOMENT_TEN_MINUTES_AGO.format(MOMENT_FORMAT),
                     reportActionID: NumberUtils.rand64(),
+                    message: [
+                        {
+                            style: 'strong',
+                            text: '__FAKE__',
+                            type: 'TEXT',
+                        },
+                        {
+                            style: 'normal',
+                            text: 'created this report',
+                            type: 'TEXT',
+                        },
+                    ],
                 },
                 1: TestHelper.buildTestReportComment(USER_B_EMAIL, 1, MOMENT_TEN_MINUTES_AGO.add(10, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID),
                 2: TestHelper.buildTestReportComment(USER_B_EMAIL, 2, MOMENT_TEN_MINUTES_AGO.add(20, 'seconds').format(MOMENT_FORMAT), USER_B_ACCOUNT_ID),

tests/unit/ReportActionsUtilsTest.js

@@ -0,0 +1,123 @@
+import CONST from '../../src/CONST';
+import * as ReportActionsUtils from '../../src/libs/ReportActionsUtils';
+
+describe('ReportActionsUtils', () => {
+    describe('getSortedReportActions', () => {
+        const cases = [
+            [
+                [
+                    // This is the highest created timestamp, so should appear last
+                    {
+                        created: '2022-11-09 22:27:01.825',
+                        reportActionID: '8401445780099176',
+                    },
+                    {
+                        created: '2022-11-09 22:27:01.600',
+                        reportActionID: '6401435781022176',
+                    },
+
+                    // These reportActions were created in the same millisecond so should appear ordered by reportActionID
+                    {
+                        created: '2022-11-09 22:26:48.789',
+                        reportActionID: '2962390724708756',
+                    },
+                    {
+                        created: '2022-11-09 22:26:48.789',
+                        reportActionID: '1609646094152486',
+                    },
+                    {
+                        created: '2022-11-09 22:26:48.789',
+                        reportActionID: '1661970171066218',
+                    },
+                ],
+                [
+                    {
+                        created: '2022-11-09 22:26:48.789',
+                        reportActionID: '1609646094152486',
+                    },
+                    {
+                        created: '2022-11-09 22:26:48.789',
+                        reportActionID: '1661970171066218',
+                    },
+                    {
+                        created: '2022-11-09 22:26:48.789',
+                        reportActionID: '2962390724708756',
+                    },
+                    {
+                        created: '2022-11-09 22:27:01.600',
+                        reportActionID: '6401435781022176',
+                    },
+                    {
+                        created: '2022-11-09 22:27:01.825',
+                        reportActionID: '8401445780099176',
+                    },
+                ],
+            ],
+        ];
+
+        test.each(cases)('sorts by created, then actionName, then reportActionID', (input, expectedOutput) => {
+            const result = ReportActionsUtils.getSortedReportActions(input);
+            expect(result).toStrictEqual(expectedOutput);
+        });
+
+        test.each(cases)('in descending order', (input, expectedOutput) => {
+            const result = ReportActionsUtils.getSortedReportActions(input, true);
+            expect(result).toStrictEqual(expectedOutput.reverse());
+        });
+    });
+
+    describe('filterReportActionsForDisplay', () => {
+        it('should filter out non-whitelisted actions', () => {
+            const input = [
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
+                    message: [{html: 'Hello world'}],
+                },
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.CLOSED,
+                    message: [{html: 'Hello world'}],
+                },
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.CREATED,
+                    message: [{html: 'Hello world'}],
+                },
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
+                    message: [{html: 'Hello world'}],
+                },
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.RENAMED,
+                    message: [{html: 'Hello world'}],
+                },
+                {
+                    actionName: 'REIMBURSED',
+                    message: [{html: 'Hello world'}],
+                },
+            ];
+            const result = ReportActionsUtils.filterReportActionsForDisplay(input);
+            input.pop();
+            expect(result).toStrictEqual(input);
+        });
+
+        it('should filter out deleted, non-pending comments', () => {
+            const input = [
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
+                    message: [{html: 'Hello world'}],
+                },
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
+                    message: [{html: ''}],
+                    pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
+                },
+                {
+                    actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
+                    message: [{html: ''}],
+                },
+            ];
+            const result = ReportActionsUtils.filterReportActionsForDisplay(input);
+            input.pop();
+            expect(result).toStrictEqual(input);
+        });
+    });
+});