diff --git a/src/components/ReportWelcomeText.js b/src/components/ReportWelcomeText.js index 8ee9718ab1d1..1109d7682901 100644 --- a/src/components/ReportWelcomeText.js +++ b/src/components/ReportWelcomeText.js @@ -91,7 +91,7 @@ const ReportWelcomeText = (props) => { {roomWelcomeMessage.phrase1} Navigation.navigate(ROUTES.getReportDetailsRoute(props.report.reportID))}> - {ReportUtils.getReportName(props.report, props.personalDetails, props.policies)} + {ReportUtils.getReportName(props.report, props.policies)} {roomWelcomeMessage.phrase2} diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 6308c4745a45..d23a3e119ce5 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -293,6 +293,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, { const personalDetail = personalDetailList[0] || {}; let hasMultipleParticipants = personalDetailList.length > 1; let subtitle; + let reportName; result.participantsList = personalDetailList; @@ -349,7 +350,9 @@ function createOption(logins, personalDetails, report, reportActions = {}, { ? lastMessageText : Str.removeSMSDomain(personalDetail.login); } + reportName = ReportUtils.getReportName(report, policies); } else { + reportName = ReportUtils.getDisplayNameForParticipant(logins[0]); result.keyForList = personalDetail.login; result.alternateText = Str.removeSMSDomain(personalDetail.login); } @@ -368,7 +371,6 @@ function createOption(logins, personalDetails, report, reportActions = {}, { result.payPalMeAddress = personalDetail.payPalMeAddress; } - const reportName = ReportUtils.getReportName(report, personalDetailMap, policies); result.text = reportName; result.searchText = getSearchText(report, reportName, personalDetailList, result.isChatRoom || result.isPolicyExpenseChat); result.icons = ReportUtils.getIcons(report, personalDetails, policies, personalDetail.avatar); diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 06f46c774419..6270e4c5ec61 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -465,27 +465,42 @@ function getIcons(report, personalDetails, policies, defaultIcon = null) { return _.map(sortedParticipants, item => item.avatar); } +/** + * Gets the personal details for a login by looking in the ONYXKEYS.PERSONAL_DETAILS Onyx key (stored in the local variable, allPersonalDetails). If it doesn't exist in Onyx, + * then a default object is constructed. + * @param {String} login + * @returns {Object} + */ +function getPersonalDetailsForLogin(login) { + if (!login) { + return {}; + } + return (allPersonalDetails && allPersonalDetails[login]) || { + login, + displayName: Str.removeSMSDomain(login), + avatar: getDefaultAvatar(login), + }; +} + /** * Get the displayName for a single report participant. * - * @param {Object} participant - * @param {String} participant.displayName - * @param {String} participant.firstName - * @param {String} participant.login + * @param {String} login * @param {Boolean} [shouldUseShortForm] * @returns {String} */ -function getDisplayNameForParticipant(participant, shouldUseShortForm = false) { - if (!participant) { +function getDisplayNameForParticipant(login, shouldUseShortForm = false) { + if (!login) { return ''; } + const personalDetails = getPersonalDetailsForLogin(login); - const loginWithoutSMSDomain = Str.removeSMSDomain(participant.login); - let longName = participant.displayName || loginWithoutSMSDomain; + const loginWithoutSMSDomain = Str.removeSMSDomain(personalDetails.login); + let longName = personalDetails.displayName || loginWithoutSMSDomain; if (longName === loginWithoutSMSDomain && Str.isSMSLogin(longName)) { longName = LocalePhoneNumber.toLocalPhone(preferredLocale, longName); } - const shortName = participant.firstName || longName; + const shortName = personalDetails.firstName || longName; return shouldUseShortForm ? shortName : longName; } @@ -497,7 +512,7 @@ function getDisplayNameForParticipant(participant, shouldUseShortForm = false) { */ function getDisplayNamesWithTooltips(participants, isMultipleParticipantReport) { return _.map(participants, (participant) => { - const displayName = getDisplayNameForParticipant(participant, isMultipleParticipantReport); + const displayName = getDisplayNameForParticipant(participant.login, isMultipleParticipantReport); const tooltip = Str.removeSMSDomain(participant.login); let pronouns = participant.pronouns; @@ -518,19 +533,17 @@ function getDisplayNamesWithTooltips(participants, isMultipleParticipantReport) * Get the title for a report. * * @param {Object} report - * @param {Object} [personalDetailsForParticipants] * @param {Object} [policies] * @returns {String} */ -function getReportName(report, personalDetailsForParticipants = {}, policies = {}) { +function getReportName(report, policies = {}) { let formattedName; if (isChatRoom(report)) { formattedName = report.reportName; } if (isPolicyExpenseChat(report)) { - const reportOwnerPersonalDetails = personalDetailsForParticipants[report.ownerEmail]; - const reportOwnerDisplayName = getDisplayNameForParticipant(reportOwnerPersonalDetails) || report.ownerEmail || report.reportName; + const reportOwnerDisplayName = getDisplayNameForParticipant(report.ownerEmail) || report.ownerEmail || report.reportName; formattedName = report.isOwnPolicyExpenseChat ? getPolicyName(report, policies) : reportOwnerDisplayName; } @@ -543,10 +556,10 @@ function getReportName(report, personalDetailsForParticipants = {}, policies = { } // Not a room or PolicyExpenseChat, generate title from participants - const participants = _.without(lodashGet(report, 'participants', []), sessionEmail); - const isMultipleParticipantReport = participants.length > 1; - const participantsToGetTheNamesOf = _.isEmpty(personalDetailsForParticipants) ? participants : personalDetailsForParticipants; - return _.map(participantsToGetTheNamesOf, participant => getDisplayNameForParticipant(participant, isMultipleParticipantReport)).join(', '); + const participants = (report && report.participants) || []; + const participantsWithoutCurrentUser = _.without(participants, sessionEmail); + const isMultipleParticipantReport = participantsWithoutCurrentUser.length > 1; + return _.map(participantsWithoutCurrentUser, login => getDisplayNameForParticipant(login, isMultipleParticipantReport)).join(', '); } /** @@ -690,7 +703,7 @@ function buildOptimisticIOUReport(ownerEmail, userEmail, total, chatReportID, cu function getIOUReportActionMessage(type, total, participants, comment, currency) { const amount = NumberFormatUtils.format(preferredLocale, total / 100, {style: 'currency', currency}); const isMultipleParticipantReport = participants.length > 1; - const displayNames = _.map(participants, participant => getDisplayNameForParticipant(allPersonalDetails[participant.login], isMultipleParticipantReport) || participant.login); + const displayNames = _.map(participants, participant => getDisplayNameForParticipant(participant.login, isMultipleParticipantReport) || participant.login); const from = displayNames.length < 3 ? displayNames.join(' and ') : `${displayNames.slice(0, -1).join(', ')}, and ${_.last(displayNames)}`; @@ -1083,7 +1096,6 @@ export { isPolicyExpenseChat, getDefaultAvatar, getIcons, - getDisplayNameForParticipant, getRoomWelcomeMessage, getDisplayNamesWithTooltips, getReportName, @@ -1100,4 +1112,5 @@ export { shouldReportBeInOptionList, getChatByParticipants, getIOUReportActionMessage, + getDisplayNameForParticipant, }; diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 095864ad2fff..d81b35fa891f 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -100,11 +100,12 @@ function getOrderedReportIDs(reportIDFromRoute) { // Get all the display names for our reports in an easy to access property so we don't have to keep // re-running the logic const filteredReportsWithReportName = _.map(filteredReports, (report) => { - const personalDetailMap = OptionsListUtils.getPersonalDetailsForLogins(report.participants, personalDetails); - return { - ...report, - reportDisplayName: ReportUtils.getReportName(report, personalDetailMap, policies), - }; + // Normally, the spread operator would be used here to clone the report and prevent the need to reassign the params. + // However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add + // the reportDisplayName property to the report object directly. + // eslint-disable-next-line no-param-reassign + report.reportDisplayName = ReportUtils.getReportName(report, policies); + return report; }); // Sorting the reports works like this: @@ -296,7 +297,7 @@ function getOptionData(reportID) { result.payPalMeAddress = personalDetail.payPalMeAddress; } - const reportName = ReportUtils.getReportName(report, personalDetailMap, policies); + const reportName = ReportUtils.getReportName(report, policies); result.text = reportName; result.subtitle = subtitle; result.participantsList = personalDetailList; diff --git a/src/pages/ReportDetailsPage.js b/src/pages/ReportDetailsPage.js index 622cb777aa5d..6efd4bf724c4 100644 --- a/src/pages/ReportDetailsPage.js +++ b/src/pages/ReportDetailsPage.js @@ -127,7 +127,7 @@ class ReportDetailsPage extends Component { { const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips(participantPersonalDetails, isMultipleParticipant); const isChatRoom = ReportUtils.isChatRoom(props.report); const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(props.report); - const title = ReportUtils.getReportName(props.report, participantPersonalDetails, props.policies); + const title = ReportUtils.getReportName(props.report, props.policies); const subtitle = ReportUtils.getChatRoomSubtitle(props.report, props.policies); const isConcierge = participants.length === 1 && _.contains(participants, CONST.EMAIL.CONCIERGE); diff --git a/tests/unit/OptionsListUtilsTest.js b/tests/unit/OptionsListUtilsTest.js index 37cb6235cbeb..c9680467b81f 100644 --- a/tests/unit/OptionsListUtilsTest.js +++ b/tests/unit/OptionsListUtilsTest.js @@ -273,7 +273,8 @@ describe('OptionsListUtils', () => { }, }); Onyx.registerLogger(() => {}); - return waitForPromisesToResolve(); + return waitForPromisesToResolve() + .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS, PERSONAL_DETAILS)); }); it('getSearchOptions()', () => { @@ -299,14 +300,18 @@ describe('OptionsListUtils', () => { // Value with latest lastMessageTimestamp should be at the top. expect(results.recentReports.length).toBe(2); expect(results.recentReports[0].text).toBe('Mister Fantastic'); - expect(results.recentReports[1].text).toBe('Iron Man, Mister Fantastic'); - - // When we filter again but provide a searchValue that should match with periods - results = OptionsListUtils.getSearchOptions(REPORTS, PERSONAL_DETAILS_WITH_PERIODS, 'barryallen@expensify.com'); - - // Then we expect to have the personal detail with period filtered - expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].text).toBe('The Flash'); + expect(results.recentReports[1].text).toBe('Mister Fantastic'); + + return waitForPromisesToResolve() + .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS, PERSONAL_DETAILS_WITH_PERIODS)) + .then(() => { + // When we filter again but provide a searchValue that should match with periods + results = OptionsListUtils.getSearchOptions(REPORTS, PERSONAL_DETAILS_WITH_PERIODS, 'barryallen@expensify.com'); + + // Then we expect to have the personal detail with period filtered + expect(results.recentReports.length).toBe(1); + expect(results.recentReports[0].text).toBe('The Flash'); + }); }); it('getNewChatOptions()', () => { diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index 53a1553aa951..30c2ed9b65b0 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -1,4 +1,3 @@ -import _ from 'underscore'; import Onyx from 'react-native-onyx'; import CONST from '../../src/CONST'; import ONYXKEYS from '../../src/ONYXKEYS'; @@ -36,7 +35,9 @@ const policies = { Onyx.init({keys: ONYXKEYS}); -beforeAll(() => Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail})); +beforeAll(() => waitForPromisesToResolve() + .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS, participantsPersonalDetails)) + .then(() => Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail}))); beforeEach(() => Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.DEFAULT_LOCALE).then(waitForPromisesToResolve)); describe('ReportUtils', () => { @@ -97,26 +98,26 @@ describe('ReportUtils', () => { test('with displayName', () => { expect(ReportUtils.getReportName({ participants: [currentUserEmail, 'ragnar@vikings.net'], - }, _.pick(participantsPersonalDetails, 'ragnar@vikings.net'))).toBe('Ragnar Lothbrok'); + })).toBe('Ragnar Lothbrok'); }); test('no displayName', () => { expect(ReportUtils.getReportName({ participants: [currentUserEmail, 'floki@vikings.net'], - }, _.pick(participantsPersonalDetails, 'floki@vikings.net'))).toBe('floki@vikings.net'); + })).toBe('floki@vikings.net'); }); test('SMS', () => { expect(ReportUtils.getReportName({ participants: [currentUserEmail, '+12223334444@expensify.sms'], - }, _.pick(participantsPersonalDetails, '+12223334444@expensify.sms'))).toBe('2223334444'); + })).toBe('2223334444'); }); }); test('Group DM', () => { expect(ReportUtils.getReportName({ participants: [currentUserEmail, 'ragnar@vikings.net', 'floki@vikings.net', 'lagertha@vikings.net', '+12223334444@expensify.sms'], - }, participantsPersonalDetails)).toBe('Ragnar, floki@vikings.net, Lagertha, 2223334444'); + })).toBe('Ragnar, floki@vikings.net, Lagertha, 2223334444'); }); describe('Default Policy Room', () => { @@ -175,7 +176,7 @@ describe('ReportUtils', () => { policyID: policy.policyID, isOwnPolicyExpenseChat: true, ownerEmail: 'ragnar@vikings.net', - }, participantsPersonalDetails, policies)).toBe('Vikings Policy'); + }, policies)).toBe('Vikings Policy'); }); test('as admin', () => { @@ -184,7 +185,7 @@ describe('ReportUtils', () => { policyID: policy.policyID, isOwnPolicyExpenseChat: false, ownerEmail: 'ragnar@vikings.net', - }, participantsPersonalDetails, policies)).toBe('Ragnar Lothbrok'); + }, policies)).toBe('Ragnar Lothbrok'); }); }); @@ -204,10 +205,10 @@ describe('ReportUtils', () => { isOwnPolicyExpenseChat: true, }; - expect(ReportUtils.getReportName(memberArchivedPolicyExpenseChat, {}, policies)).toBe('Vikings Policy (archived)'); + expect(ReportUtils.getReportName(memberArchivedPolicyExpenseChat, policies)).toBe('Vikings Policy (archived)'); return Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, 'es') - .then(() => expect(ReportUtils.getReportName(memberArchivedPolicyExpenseChat, {}, policies)).toBe('Vikings Policy (archivado)')); + .then(() => expect(ReportUtils.getReportName(memberArchivedPolicyExpenseChat, policies)).toBe('Vikings Policy (archivado)')); }); test('as admin', () => { @@ -216,10 +217,10 @@ describe('ReportUtils', () => { isOwnPolicyExpenseChat: false, }; - expect(ReportUtils.getReportName(adminArchivedPolicyExpenseChat, participantsPersonalDetails)).toBe('Ragnar Lothbrok (archived)'); + expect(ReportUtils.getReportName(adminArchivedPolicyExpenseChat)).toBe('Ragnar Lothbrok (archived)'); return Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, 'es') - .then(() => expect(ReportUtils.getReportName(adminArchivedPolicyExpenseChat, participantsPersonalDetails)).toBe('Ragnar Lothbrok (archivado)')); + .then(() => expect(ReportUtils.getReportName(adminArchivedPolicyExpenseChat)).toBe('Ragnar Lothbrok (archivado)')); }); }); });