Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NoQA] fix: do not use last actor details for LHN if last action is for closing chat #16606

Merged
88 changes: 56 additions & 32 deletions src/libs/ReportActionsUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,26 +158,64 @@ function getLastVisibleMessageText(reportID, actionsToMerge = {}) {
}

/**
* A helper method to filter out report actions keyed by sequenceNumbers.
* Checks if a reportAction is deprecated.
*
* @param {Object} reportActions
* @returns {Array}
* @param {Object} reportAction
* @param {String} key
* @returns {Boolean}
*/
function filterOutDeprecatedReportActions(reportActions) {
function isReportActionDeprecated(reportAction, key) {
if (!reportAction) {
return true;
}

// HACK ALERT: We're temporarily filtering out any reportActions keyed by sequenceNumber
// to prevent bugs during the migration from sequenceNumber -> reportActionID
return _.filter(reportActions, (reportAction, key) => {
if (!reportAction) {
return false;
}
if (String(reportAction.sequenceNumber) === key) {
Log.info('Front-end filtered out reportAction keyed by sequenceNumber!', false, reportAction);
return true;
}

if (String(reportAction.sequenceNumber) === key) {
Log.info('Front-end filtered out reportAction keyed by sequenceNumber!', false, reportAction);
return false;
}
return false;
}

return true;
});
/**
* Checks if a reportAction is fit for display, meaning that it's not deprecated, is of a valid
* and supported type, it's not deleted and also not closed.
*
* @param {Object} reportAction
* @param {String} key
* @returns {Boolean}
*/
function shouldReportActionBeVisible(reportAction, key) {
if (isReportActionDeprecated(reportAction, key)) {
return false;
}

// Filter out any unsupported reportAction types
if (!_.has(CONST.REPORT.ACTIONS.TYPE, reportAction.actionName) && !_.contains(_.values(CONST.REPORT.ACTIONS.TYPE.POLICYCHANGELOG), reportAction.actionName)) {
return false;
}

// Ignore closed action here since we're already displaying a footer that explains why the report was closed
if (reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED) {
return false;
}

// All other actions are displayed except deleted, non-pending actions
const isDeleted = isDeletedAction(reportAction);
const isPending = !_.isEmpty(reportAction.pendingAction);
return !isDeleted || isPending;
}

/**
* A helper method to filter out report actions keyed by sequenceNumbers.
*
* @param {Object} reportActions
* @returns {Array}
*/
function filterOutDeprecatedReportActions(reportActions) {
return _.filter(reportActions, (reportAction, key) => !isReportActionDeprecated(reportAction, key));
}
Comment on lines +210 to 219
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being used anywhere?

Copy link
Contributor Author

@BeeMargarida BeeMargarida Apr 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in the same file, in getLastClosedReportAction (used for archive report footer) and getSortedReportActionsForDisplay (report screen and IOU transaction).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thanks I can see now!


/**
Expand All @@ -190,24 +228,8 @@ function filterOutDeprecatedReportActions(reportActions) {
* @returns {Array}
*/
function getSortedReportActionsForDisplay(reportActions) {
const filteredReportActions = filterOutDeprecatedReportActions(reportActions);
const sortedReportActions = getSortedReportActions(filteredReportActions, true);
return _.filter(sortedReportActions, (reportAction) => {
// Filter out any unsupported reportAction types
if (!_.has(CONST.REPORT.ACTIONS.TYPE, reportAction.actionName) && !_.contains(_.values(CONST.REPORT.ACTIONS.TYPE.POLICYCHANGELOG), reportAction.actionName)) {
return false;
}

// Ignore closed action here since we're already displaying a footer that explains why the report was closed
if (reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED) {
return false;
}

// All other actions are displayed except deleted, non-pending actions
const isDeleted = isDeletedAction(reportAction);
const isPending = !_.isEmpty(reportAction.pendingAction);
return !isDeleted || isPending;
});
const filteredReportActions = _.filter(reportActions, (reportAction, key) => shouldReportActionBeVisible(reportAction, key));
return getSortedReportActions(filteredReportActions, true);
}

/**
Expand Down Expand Up @@ -250,6 +272,8 @@ export {
getLastVisibleMessageText,
getMostRecentIOURequestActionID,
isDeletedAction,
shouldReportActionBeVisible,
isReportActionDeprecated,
isConsecutiveActionMadeByPreviousActor,
getSortedReportActionsForDisplay,
getLastClosedReportAction,
Expand Down
20 changes: 16 additions & 4 deletions src/libs/SidebarUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ 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 @@ -52,6 +53,7 @@ Onyx.connect({
callback: val => betas = val,
});

const visibleReportActionItems = {};
const lastReportActions = {};
const reportActions = {};
Onyx.connect({
Expand All @@ -61,7 +63,16 @@ Onyx.connect({
return;
}
const reportID = CollectionUtils.extractCollectionItemID(key);
lastReportActions[reportID] = _.last(_.toArray(actions));

const actionsArray = _.toArray(actions);
lastReportActions[reportID] = _.last(actionsArray);

// The report is only visible if it is the last action not deleted that
// does not match a closed or created state.
const reportActionsForDisplay = _.filter(actionsArray, (reportAction, actionKey) => (ReportActionsUtils.shouldReportActionBeVisible(reportAction, actionKey)
&& (reportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither a regression nor in the scope of this PR, but we should also filter delete pending action items to correctly get the last visible item.

#24231

visibleReportActionItems[reportID] = _.first(ReportActionsUtils.getSortedReportActions(reportActionsForDisplay, true));

reportActions[key] = actions;
},
});
Expand Down Expand Up @@ -251,10 +262,11 @@ function getOptionData(reportID) {
}

// If the last actor's details are not currently saved in Onyx Collection,
// then try to get that from the last report action.
// then try to get that from the last report action if that action is valid
// to get data from.
let lastActorDetails = personalDetails[report.lastActorEmail] || null;
if (!lastActorDetails && lastReportActions[report.reportID] && lastReportActions[report.reportID].actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED) {
const lastActorDisplayName = lodashGet(lastReportActions[report.reportID], 'person[0].text');
if (!lastActorDetails && visibleReportActionItems[report.reportID]) {
const lastActorDisplayName = lodashGet(visibleReportActionItems[report.reportID], 'person[0].text');
lastActorDetails = lastActorDisplayName ? {
displayName: lastActorDisplayName,
login: report.lastActorEmail,
Expand Down