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

Update LHN/chat header to handle displayName not being set #27547

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ function createOption(accountIDs, personalDetails, report, reportActions = {}, {
}
reportName = ReportUtils.getReportName(report);
} else {
reportName = ReportUtils.getDisplayNameForParticipant(accountIDs[0]);
reportName = ReportUtils.getDisplayNameForParticipant(accountIDs[0], false);
result.keyForList = String(accountIDs[0]);
result.alternateText = LocalePhoneNumber.formatPhoneNumber(lodashGet(personalDetails, [accountIDs[0], 'login'], ''));
}
Expand Down
18 changes: 15 additions & 3 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,7 @@ function getPersonalDetailsForAccountID(accountID) {
return (
(allPersonalDetails && allPersonalDetails[accountID]) || {
avatar: UserUtils.getDefaultAvatar(accountID),
isOptimisticPersonalDetail: true,
}
);
}
Expand All @@ -1173,27 +1174,38 @@ function getPersonalDetailsForAccountID(accountID) {
*
* @param {Number} accountID
* @param {Boolean} [shouldUseShortForm]
* @param {Boolean} shouldFallbackToHidden
* @returns {String}
*/
function getDisplayNameForParticipant(accountID, shouldUseShortForm = false) {
function getDisplayNameForParticipant(accountID, shouldUseShortForm = false, shouldFallbackToHidden = true) {
if (!accountID) {
return '';
}
const personalDetails = getPersonalDetailsForAccountID(accountID);
// this is to check if account is an invite/optimistically created one
// and prevent from falling back to 'Hidden', so a correct value is shown
// when searching for a new user
if (lodashGet(personalDetails, 'isOptimisticPersonalDetail') === true) {
return personalDetails.login || '';
}
const longName = personalDetails.displayName;
const shortName = personalDetails.firstName || longName;
if (!longName && !personalDetails.login && shouldFallbackToHidden) {
return Localize.translateLocal('common.hidden');
}
return shouldUseShortForm ? shortName : longName;
}

/**
* @param {Object} personalDetailsList
* @param {Boolean} isMultipleParticipantReport
* @param {Boolean} shouldFallbackToHidden
* @returns {Array}
*/
function getDisplayNamesWithTooltips(personalDetailsList, isMultipleParticipantReport) {
function getDisplayNamesWithTooltips(personalDetailsList, isMultipleParticipantReport, shouldFallbackToHidden) {
return _.map(personalDetailsList, (user) => {
const accountID = Number(user.accountID);
const displayName = getDisplayNameForParticipant(accountID, isMultipleParticipantReport) || user.login || '';
const displayName = getDisplayNameForParticipant(accountID, isMultipleParticipantReport, shouldFallbackToHidden) || user.login || '';
const avatar = UserUtils.getDefaultAvatar(accountID);

let pronouns = user.pronouns;
Expand Down
4 changes: 4 additions & 0 deletions src/pages/home/report/ParticipantLocalTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ function ParticipantLocalTime(props) {

const reportRecipientDisplayName = lodashGet(props, 'participant.firstName') || lodashGet(props, 'participant.displayName');

if (!reportRecipientDisplayName) {
return null;
}

return (
<View style={[styles.chatItemComposeSecondaryRow]}>
<Text
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItemSingle.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ function ReportActionItemSingle(props) {

// If this is a report preview, display names and avatars of both people involved
let secondaryAvatar = {};
const primaryDisplayName = displayName;
const primaryDisplayName = ReportUtils.getDisplayNameForParticipant(actorAccountID);
if (displayAllActors) {
// The ownerAccountID and actorAccountID can be the same if the a user requests money back from the IOU's original creator, in that case we need to use managerID to avoid displaying the same user twice
const secondaryAccountId = props.iouReport.ownerAccountID === actorAccountID ? props.iouReport.managerID : props.iouReport.ownerAccountID;
const secondaryUserDetails = props.personalDetailsList[secondaryAccountId] || {};
const secondaryDisplayName = lodashGet(secondaryUserDetails, 'displayName', '');
const secondaryDisplayName = ReportUtils.getDisplayNameForParticipant(secondaryAccountId);
displayName = `${primaryDisplayName} & ${secondaryDisplayName}`;
secondaryAvatar = {
source: UserUtils.getAvatar(secondaryUserDetails.avatar, secondaryAccountId),
Expand Down
11 changes: 7 additions & 4 deletions src/pages/iou/steps/MoneyRequestConfirmPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,13 @@ function MoneyRequestConfirmPage(props) {
const [receiptFile, setReceiptFile] = useState();
const participants = useMemo(
() =>
_.map(props.iou.participants, (participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? OptionsListUtils.getPolicyExpenseReportOption(participant) : OptionsListUtils.getParticipantsOption(participant, props.personalDetails);
}),
_.chain(props.iou.participants)
.map((participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? OptionsListUtils.getPolicyExpenseReportOption(participant) : OptionsListUtils.getParticipantsOption(participant, props.personalDetails);
})
.filter((participant) => !!participant.login)
.value(),
Comment on lines -73 to +79
Copy link
Contributor

@s77rt s77rt Nov 24, 2023

Choose a reason for hiding this comment

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

@lukemorawski Can you please clarify why we need to filter participants on login? cc @mananjadhav

(Coming from #31792)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s77rt You probably meant filtering "by login". It' there to filter out user without login so the would not appear in the money request as "Hidden". Also the process of creating a money request requires that field and allowing "Hidden" users to make a money request caused an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lukemorawski Thanks for your reply. It looks like making money requests to "Hidden" accounts works and it's actually a wanted functionality. Since this filter was only done to avoid an error that no longer occurs I think it's safe to remove it. Let us know if disagree.

[props.iou.participants, props.personalDetails],
);
const isPolicyExpenseChat = useMemo(() => ReportUtils.isPolicyExpenseChat(ReportUtils.getRootParentReport(props.report)), [props.report]);
Expand Down
Loading