Improve the performance of report name generation

src/components/ReportWelcomeText.js

@@ -91,7 +91,7 @@ const ReportWelcomeText = (props) => {
                         {roomWelcomeMessage.phrase1}
                     </Text>
                     <Text style={[styles.textStrong]} onPress={() => Navigation.navigate(ROUTES.getReportDetailsRoute(props.report.reportID))}>
-                        {ReportUtils.getReportName(props.report, props.personalDetails, props.policies)}
+                        {ReportUtils.getReportName(props.report, props.policies)}
                     </Text>
                     <Text>
                         {roomWelcomeMessage.phrase2}

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);

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,
 };

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;

src/pages/ReportDetailsPage.js

@@ -127,7 +127,7 @@ class ReportDetailsPage extends Component {
                             <View style={[styles.reportDetailsRoomInfo, styles.mw100]}>
                                 <View style={[styles.alignSelfCenter, styles.w100]}>
                                     <DisplayNames
-                                        fullTitle={ReportUtils.getReportName(this.props.report, this.props.personalDetails, this.props.policies)}
+                                        fullTitle={ReportUtils.getReportName(this.props.report, this.props.policies)}
                                         displayNamesWithTooltips={displayNamesWithTooltips}
                                         tooltipEnabled
                                         numberOfLines={1}

src/pages/home/HeaderView.js

@@ -61,7 +61,7 @@ const HeaderView = (props) => {
     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);

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()', () => {

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)'));
                 });
             });
         });