Skip to content

Commit

Permalink
Merge pull request #23240 from margelo/fix/23189-regression-of-21022
Browse files Browse the repository at this point in the history 
[CP Staging] Fix regression #23189

(cherry picked from commit 45c4def)
  • Loading branch information
@mountiny @OSBotify
mountiny authored and OSBotify committed Jul 20, 2023
1 parent ef4b5fa commit 808b271
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 29 deletions.
24 changes: 21 additions & 3 deletions src/components/LHNOptionsList/OptionRowLHNData.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ const propTypes = {
// eslint-disable-next-line react/forbid-prop-types
fullReport: PropTypes.object,

/** The policies which the user has access to and which the report could be tied to */
policies: PropTypes.objectOf(
PropTypes.shape({
/** The ID of the policy */
id: PropTypes.string,
/** Name of the policy */
name: PropTypes.string,
/** Avatar of the policy */
avatar: PropTypes.string,
}),
),

...withCurrentReportIDPropTypes,
...basePropTypes,
};
Expand All @@ -37,6 +49,7 @@ const defaultProps = {
shouldDisableFocusOptions: false,
personalDetails: {},
fullReport: {},
policies: {},
preferredLocale: CONST.LOCALES.DEFAULT,
...withCurrentReportIDDefaultProps,
...baseDefaultProps,
Expand All @@ -48,22 +61,24 @@ const defaultProps = {
* The OptionRowLHN component is memoized, so it will only
* re-render if the data really changed.
*/
function OptionRowLHNData({shouldDisableFocusOptions, currentReportID, fullReport, personalDetails, preferredLocale, comment, ...propsToForward}) {
function OptionRowLHNData({shouldDisableFocusOptions, currentReportID, fullReport, personalDetails, preferredLocale, comment, policies, ...propsToForward}) {
const reportID = propsToForward.reportID;
// We only want to pass a boolean to the memoized component,
// instead of a changing number (so we prevent unnecessary re-renders).
const isFocused = !shouldDisableFocusOptions && currentReportID === reportID;

const policy = lodashGet(policies, [`${ONYXKEYS.COLLECTION.POLICY}${fullReport.policyID}`], '');

const optionItemRef = useRef();
const optionItem = useMemo(() => {
// Note: ideally we'd have this as a dependent selector in onyx!
const item = SidebarUtils.getOptionData(fullReport, personalDetails, preferredLocale);
const item = SidebarUtils.getOptionData(fullReport, personalDetails, preferredLocale, policy);
if (deepEqual(item, optionItemRef.current)) {
return optionItemRef.current;
}
optionItemRef.current = item;
return item;
}, [fullReport, preferredLocale, personalDetails]);
}, [fullReport, personalDetails, preferredLocale, policy]);

useEffect(() => {
if (!optionItem || optionItem.hasDraftComment || !comment || comment.length <= 0 || isFocused) {
Expand Down Expand Up @@ -137,6 +152,9 @@ export default React.memo(
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
},
policies: {
key: ONYXKEYS.COLLECTION.POLICY,
},
}),
)(OptionRowLHNData),
);
48 changes: 27 additions & 21 deletions src/libs/ReportUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,9 +505,10 @@ function isArchivedRoom(report) {
* @param {String} report.oldPolicyName
* @param {String} report.policyName
* @param {Boolean} [returnEmptyIfNotFound]
* @param {Object} [policy]
* @returns {String}
*/
function getPolicyName(report, returnEmptyIfNotFound = false) {
function getPolicyName(report, returnEmptyIfNotFound = false, policy = undefined) {
const noPolicyFound = returnEmptyIfNotFound ? '' : Localize.translateLocal('workspace.common.unavailable');
if (_.isEmpty(report)) {
return noPolicyFound;
Expand All @@ -516,12 +517,12 @@ function getPolicyName(report, returnEmptyIfNotFound = false) {
if ((!allPolicies || _.size(allPolicies) === 0) && !report.policyName) {
return Localize.translateLocal('workspace.common.unavailable');
}
const policy = _.get(allPolicies, `${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`);
const finalPolicy = policy || _.get(allPolicies, `${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`);

// // Public rooms send back the policy name with the reportSummary,
// // since they can also be accessed by people who aren't in the workspace
// Public rooms send back the policy name with the reportSummary,
// since they can also be accessed by people who aren't in the workspace

return lodashGet(policy, 'name') || report.policyName || report.oldPolicyName || noPolicyFound;
return lodashGet(finalPolicy, 'name') || report.policyName || report.oldPolicyName || noPolicyFound;
}

/**
Expand Down Expand Up @@ -785,10 +786,11 @@ function getIconsForParticipants(participants, personalDetails) {
* Given a report, return the associated workspace icon.
*
* @param {Object} report
* @param {Object} [policy]
* @returns {Object}
*/
function getWorkspaceIcon(report) {
const workspaceName = getPolicyName(report);
function getWorkspaceIcon(report, policy = undefined) {
const workspaceName = getPolicyName(report, false, policy);
const policyExpenseChatAvatarSource = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, 'avatar']) || getDefaultWorkspaceAvatar(workspaceName);
const workspaceIcon = {
source: policyExpenseChatAvatarSource,
Expand All @@ -809,9 +811,10 @@ function getWorkspaceIcon(report) {
* @param {Boolean} [isPayer]
* @param {String} [defaultName]
* @param {Number} [defaultAccountID]
* @param {Object} [policy]
* @returns {Array<*>}
*/
function getIcons(report, personalDetails, defaultIcon = null, isPayer = false, defaultName = '', defaultAccountID = -1) {
function getIcons(report, personalDetails, defaultIcon = null, isPayer = false, defaultName = '', defaultAccountID = -1, policy = undefined) {
if (_.isEmpty(report)) {
const fallbackIcon = {
source: defaultIcon || Expensicons.FallbackAvatar,
Expand All @@ -823,7 +826,7 @@ function getIcons(report, personalDetails, defaultIcon = null, isPayer = false,
}
if (isExpenseRequest(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
const workspaceIcon = getWorkspaceIcon(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: UserUtils.getAvatar(lodashGet(personalDetails, [parentReportAction.actorAccountID, 'avatar']), parentReportAction.actorAccountID),
id: parentReportAction.actorAccountID,
Expand All @@ -846,7 +849,7 @@ function getIcons(report, personalDetails, defaultIcon = null, isPayer = false,
};

if (isWorkspaceThread(report)) {
const workspaceIcon = getWorkspaceIcon(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
return [actorIcon, workspaceIcon];
}
return [actorIcon];
Expand All @@ -860,7 +863,7 @@ function getIcons(report, personalDetails, defaultIcon = null, isPayer = false,
};

if (isWorkspaceTaskReport(report)) {
const workspaceIcon = getWorkspaceIcon(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
return [ownerIcon, workspaceIcon];
}

Expand All @@ -879,11 +882,11 @@ function getIcons(report, personalDetails, defaultIcon = null, isPayer = false,
return [domainIcon];
}
if (isAdminRoom(report) || isAnnounceRoom(report) || isChatRoom(report) || isArchivedRoom(report)) {
const workspaceIcon = getWorkspaceIcon(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
return [workspaceIcon];
}
if (isPolicyExpenseChat(report) || isExpenseReport(report)) {
const workspaceIcon = getWorkspaceIcon(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: UserUtils.getAvatar(lodashGet(personalDetails, [report.ownerAccountID, 'avatar']), report.ownerAccountID),
id: report.ownerAccountID,
Expand Down Expand Up @@ -1032,14 +1035,15 @@ function getMoneyRequestTotal(report, moneyRequestReports = {}) {
* Get the title for a policy expense chat which depends on the role of the policy member seeing this report
*
* @param {Object} report
* @param {Object} [policy]
* @returns {String}
*/
function getPolicyExpenseChatName(report) {
function getPolicyExpenseChatName(report, policy = undefined) {
const reportOwnerDisplayName = getDisplayNameForParticipant(report.ownerAccountID) || lodashGet(allPersonalDetails, [report.ownerAccountID, 'login']) || report.reportName;

// If the policy expense chat is owned by this user, use the name of the policy as the report name.
if (report.isOwnPolicyExpenseChat) {
return getPolicyName(report);
return getPolicyName(report, false, policy);
}

const policyExpenseChatRole = lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, 'role']) || 'user';
Expand All @@ -1050,7 +1054,7 @@ function getPolicyExpenseChatName(report) {
const lastAction = ReportActionsUtils.getLastVisibleAction(report.reportID);
const archiveReason = (lastAction && lastAction.originalMessage && lastAction.originalMessage.reason) || CONST.REPORT.ARCHIVE_REASON.DEFAULT;
if (archiveReason === CONST.REPORT.ARCHIVE_REASON.ACCOUNT_MERGED && policyExpenseChatRole !== CONST.POLICY.ROLE.ADMIN) {
return getPolicyName(report);
return getPolicyName(report, false, policy);
}
}

Expand All @@ -1062,11 +1066,12 @@ function getPolicyExpenseChatName(report) {
* Get the title for a IOU or expense chat which will be showing the payer and the amount
*
* @param {Object} report
* @param {Object} [policy]
* @returns {String}
*/
function getMoneyRequestReportName(report) {
function getMoneyRequestReportName(report, policy = undefined) {
const formattedAmount = CurrencyUtils.convertToDisplayString(getMoneyRequestTotal(report), report.currency);
const payerName = isExpenseReport(report) ? getPolicyName(report) : getDisplayNameForParticipant(report.managerID);
const payerName = isExpenseReport(report) ? getPolicyName(report, false, policy) : getDisplayNameForParticipant(report.managerID);

return Localize.translateLocal(report.hasOutstandingIOU ? 'iou.payerOwesAmount' : 'iou.payerPaidAmount', {payer: payerName, amount: formattedAmount});
}
Expand Down Expand Up @@ -1117,9 +1122,10 @@ function getReportPreviewMessage(report, reportAction = {}) {
* Get the title for a report.
*
* @param {Object} report
* @param {Object} [policy]
* @returns {String}
*/
function getReportName(report) {
function getReportName(report, policy = undefined) {
let formattedName;
if (isChatThread(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
Expand All @@ -1145,11 +1151,11 @@ function getReportName(report) {
}

if (isPolicyExpenseChat(report)) {
formattedName = getPolicyExpenseChatName(report);
formattedName = getPolicyExpenseChatName(report, policy);
}

if (isMoneyRequestReport(report)) {
formattedName = getMoneyRequestReportName(report);
formattedName = getMoneyRequestReportName(report, policy);
}

if (isArchivedRoom(report)) {
Expand Down
9 changes: 5 additions & 4 deletions src/libs/SidebarUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,10 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p
* @param {Object} report
* @param {Object} personalDetails
* @param {String} preferredLocale
* @param {Object} [policy]
* @returns {Object}
*/
function getOptionData(report, personalDetails, preferredLocale) {
function getOptionData(report, personalDetails, preferredLocale, policy) {
// When a user signs out, Onyx is cleared. Due to the lazy rendering with a virtual list, it's possible for
// this method to be called after the Onyx data has been cleared out. In that case, it's fine to do
// a null check here and return early.
Expand Down Expand Up @@ -305,7 +306,7 @@ function getOptionData(report, personalDetails, preferredLocale) {
CONST.REPORT.ARCHIVE_REASON.DEFAULT;
lastMessageText = Localize.translate(preferredLocale, `reportArchiveReasons.${archiveReason}`, {
displayName: archiveReason.displayName || PersonalDetailsUtils.getDisplayNameOrDefault(lastActorDetails, 'displayName'),
policyName: ReportUtils.getPolicyName(report),
policyName: ReportUtils.getPolicyName(report, false, policy),
});
}

Expand Down Expand Up @@ -355,13 +356,13 @@ function getOptionData(report, personalDetails, preferredLocale) {
result.payPalMeAddress = personalDetail.payPalMeAddress;
}

const reportName = ReportUtils.getReportName(report);
const reportName = ReportUtils.getReportName(report, policy);

result.text = reportName;
result.subtitle = subtitle;
result.participantsList = participantPersonalDetailList;

result.icons = ReportUtils.getIcons(report, personalDetails, UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID), true);
result.icons = ReportUtils.getIcons(report, personalDetails, UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID), true, '', -1, policy);
result.searchText = OptionsListUtils.getSearchText(report, reportName, participantPersonalDetailList, result.isChatRoom || result.isPolicyExpenseChat, result.isThread);
result.displayNamesWithTooltips = displayNamesWithTooltips;
return result;
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/sidebar/SidebarLinks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const propTypes = {

currentUserPersonalDetails: personalDetailsPropType,

priorityMode: PropTypes.oneOf(_.values(CONST.OPTION_MODE)),
priorityMode: PropTypes.oneOf(_.values(CONST.PRIORITY_MODE)),

...withLocalizePropTypes,
};
Expand Down

0 comments on commit 808b271

Please sign in to comment.