diff --git a/src/libs/ReportActionsUtils.js b/src/libs/ReportActionsUtils.js index 9bcce5209ea2..7946b1cbb710 100644 --- a/src/libs/ReportActionsUtils.js +++ b/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, diff --git a/src/pages/home/report/ReportActionsList.js b/src/pages/home/report/ReportActionsList.js index d62d4e247c6f..15bb034251eb 100644 --- a/src/pages/home/report/ReportActionsList.js +++ b/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 ( diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 359b44c6546b..ff8304bcd281 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/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} */ diff --git a/tests/ui/UnreadIndicatorsTest.js b/tests/ui/UnreadIndicatorsTest.js index 81c954d6f3fd..b4ba16870cf7 100644 --- a/tests/ui/UnreadIndicatorsTest.js +++ b/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), diff --git a/tests/unit/ReportActionsUtilsTest.js b/tests/unit/ReportActionsUtilsTest.js new file mode 100644 index 000000000000..ccda75b52587 --- /dev/null +++ b/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); + }); + }); +});