Skip to content

Commit

Permalink
Merge pull request #16559 from Expensify/revert-15942-Rory-FixOffline…
Browse files Browse the repository at this point in the history 
…MessageCycle

Revert "Make optimistic reportActions appear last"
  • Loading branch information
@roryabraham
roryabraham authored Mar 27, 2023
2 parents f3878b9 + d4b2444 commit eb27dbd
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 79 deletions.
20 changes: 2 additions & 18 deletions src/libs/ReportActionsUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,8 @@ function isDeletedAction(reportAction) {
}

/**
* @param {Object} reportAction
* @returns {Boolean}
*/
function isOptimisticAction(reportAction) {
return lodashGet(reportAction, 'pendingAction') === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD;
}

/**
* Sort an array of reportActions by:
*
* - Finalized actions always are "later" than optimistic actions
* - then sort by created timestamp
* - then sort by reportActionID. This gives us a stable order even in the case of multiple reportActions created on the same millisecond
* 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
Expand All @@ -68,11 +57,6 @@ function getSortedReportActions(reportActions, shouldSortInDescendingOrder = fal
return _.chain(reportActions)
.compact()
.sort((first, second) => {
// First, make sure that optimistic reportActions appear at the end
if (isOptimisticAction(second) && !isOptimisticAction(first)) {
return -1 * invertedMultiplier;
}

// First sort by timestamp
if (first.created !== second.created) {
return (first.created < second.created ? -1 : 1) * invertedMultiplier;
Expand Down
42 changes: 0 additions & 42 deletions src/libs/ReportUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import linkingConfig from './Navigation/linkingConfig';
import * as defaultAvatars from '../components/Icon/DefaultAvatars';
import isReportMessageAttachment from './isReportMessageAttachment';
import * as defaultWorkspaceAvatars from '../components/Icon/WorkspaceDefaultAvatars';
import * as CollectionUtils from './CollectionUtils';

let sessionEmail;
Onyx.connect({
Expand Down Expand Up @@ -72,21 +71,6 @@ Onyx.connect({
callback: val => allReports = val,
});

const lastReportActions = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
callback: (actions, key) => {
if (!key || !actions) {
return;
}
const reportID = CollectionUtils.extractCollectionItemID(key);
lastReportActions[reportID] = _.find(
ReportActionsUtils.getSortedReportActionsForDisplay(_.toArray(actions)),
reportAction => reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
);
},
});

let doesDomainHaveApprovedAccountant;
Onyx.connect({
key: ONYXKEYS.ACCOUNT,
Expand Down Expand Up @@ -450,31 +434,6 @@ function canShowReportRecipientLocalTime(personalDetails, report) {
&& isReportParticipantValidated);
}

/**
* Gets the last message text from the report.
* Looks at reportActions data as the "best source" for information, because the front-end may have optimistic reportActions that the server is not yet aware of.
* If reportActions are not loaded for the report, then there can't be any optimistic reportActions, and the lastMessageText rNVP will be accurate as a fallback.
*
* @param {Object} report
* @returns {String}
*/
function getLastMessageText(report) {
if (!report) {
return '';
}

const lastReportAction = lastReportActions[report.reportID];
let lastReportActionText = report.lastMessageText;
let lastReportActionHtml = report.lastMessageHtml;
if (lastReportAction && lastReportAction.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT) {
lastReportActionText = lodashGet(lastReportAction, 'message[0].text', report.lastMessageText);
lastReportActionHtml = lodashGet(lastReportAction, 'message[0].html', report.lastMessageHtml);
}
return isReportMessageAttachment({text: lastReportActionText, html: lastReportActionHtml})
? `[${Localize.translateLocal('common.attachment')}]`
: Str.htmlDecode(lastReportActionText);
}

/**
* Trim the last message text to a fixed limit.
* @param {String} lastMessageText
Expand Down Expand Up @@ -1715,7 +1674,6 @@ export {
isIOUOwnedByCurrentUser,
getIOUTotal,
canShowReportRecipientLocalTime,
getLastMessageText,
formatReportLastMessageText,
chatIncludesConcierge,
isPolicyExpenseChat,
Expand Down
12 changes: 8 additions & 4 deletions src/libs/SidebarUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import lodashOrderBy from 'lodash/orderBy';
import Str from 'expensify-common/lib/str';
import ONYXKEYS from '../ONYXKEYS';
import * as ReportUtils from './ReportUtils';
import * as ReportActionsUtils from './ReportActionsUtils';
import * as Localize from './Localize';
import CONST from '../CONST';
import * as OptionsListUtils from './OptionsListUtils';
Expand Down Expand Up @@ -62,7 +61,7 @@ Onyx.connect({
return;
}
const reportID = CollectionUtils.extractCollectionItemID(key);
lastReportActions[reportID] = _.first(ReportActionsUtils.getSortedReportActionsForDisplay(_.toArray(actions)));
lastReportActions[reportID] = _.last(_.toArray(actions));
reportActions[key] = actions;
},
});
Expand Down Expand Up @@ -244,7 +243,12 @@ function getOptionData(reportID) {
// We only create tooltips for the first 10 users or so since some reports have hundreds of users, causing performance to degrade.
const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips((participantPersonalDetailList || []).slice(0, 10), hasMultipleParticipants);

const lastMessageTextFromReport = ReportUtils.getLastMessageText(report);
let lastMessageTextFromReport = '';
if (ReportUtils.isReportMessageAttachment({text: report.lastMessageText, html: report.lastMessageHtml})) {
lastMessageTextFromReport = `[${Localize.translateLocal('common.attachment')}]`;
} else {
lastMessageTextFromReport = Str.htmlDecode(report ? report.lastMessageText : '');
}

// If the last actor's details are not currently saved in Onyx Collection,
// then try to get that from the last report action.
Expand All @@ -259,7 +263,7 @@ function getOptionData(reportID) {
let lastMessageText = hasMultipleParticipants && lastActorDetails && (lastActorDetails.login !== currentUserLogin.email)
? `${lastActorDetails.displayName}: `
: '';
lastMessageText += lastMessageTextFromReport;
lastMessageText += report ? lastMessageTextFromReport : '';

if (result.isPolicyExpenseChat && result.isArchivedRoom) {
const archiveReason = (lastReportActions[report.reportID] && lastReportActions[report.reportID].originalMessage && lastReportActions[report.reportID].originalMessage.reason)
Expand Down
16 changes: 1 addition & 15 deletions tests/unit/ReportActionsUtilsTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,7 @@ describe('ReportActionsUtils', () => {
const cases = [
[
[
// This is the lowest created timestamp, but because it's an optimistic action it should appear last
{
created: '2022-11-09 20:00:00.000',
reportActionID: '395268342',
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
},

// This is the highest created timestamp, so should appear 2nd-to-last
// This is the highest created timestamp, so should appear last
{
created: '2022-11-09 22:27:01.825',
reportActionID: '8401445780099176',
Expand Down Expand Up @@ -69,12 +61,6 @@ describe('ReportActionsUtils', () => {
reportActionID: '8401445780099176',
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
},
{
created: '2022-11-09 20:00:00.000',
reportActionID: '395268342',
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
},
],
],
[
Expand Down

0 comments on commit eb27dbd

Please sign in to comment.