From df12d41a22c07fcdde69c4b2a938595dcd09278a Mon Sep 17 00:00:00 2001 From: Sobit Neupane <073bct543.sobit@pcampus.edu.np> Date: Mon, 24 Jan 2022 13:07:07 +0545 Subject: [PATCH 1/4] Separate report name and icon configuration from personal details --- src/libs/OptionsListUtils.js | 2 +- src/libs/actions/PersonalDetails.js | 31 --------------------- src/libs/actions/Report.js | 42 +++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 4dc9af374968..9c89b974b644 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -782,7 +782,7 @@ function getReportIcons(report, personalDetails) { } const sortedParticipants = _.map(report.participants, dmParticipant => ({ firstName: lodashGet(personalDetails, [dmParticipant, 'firstName'], ''), - avatar: lodashGet(personalDetails, [dmParticipant, 'avatarThumbnail'], '') + avatar: lodashGet(personalDetails, [dmParticipant, 'avatar'], '') || getDefaultAvatar(dmParticipant), })) .sort((first, second) => first.firstName - second.firstName); diff --git a/src/libs/actions/PersonalDetails.js b/src/libs/actions/PersonalDetails.js index 22453fcd4ee9..6bbaf0a52ade 100644 --- a/src/libs/actions/PersonalDetails.js +++ b/src/libs/actions/PersonalDetails.js @@ -8,7 +8,6 @@ import CONST from '../../CONST'; import NetworkConnection from '../NetworkConnection'; import * as API from '../API'; import NameValuePair from './NameValuePair'; -import * as ReportUtils from '../reportUtils'; import * as OptionsListUtils from '../OptionsListUtils'; import Growl from '../Growl'; import * as Localize from '../Localize'; @@ -173,36 +172,6 @@ function getFromReportParticipants(reports) { const formattedPersonalDetails = formatPersonalDetails(details); Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, formattedPersonalDetails); - - // The personalDetails of the participants contain their avatar images. Here we'll go over each - // report and based on the participants we'll link up their avatars to report icons. This will - // skip over default rooms which aren't named by participants. - const reportsToUpdate = {}; - _.each(reports, (report) => { - if (report.participants.length <= 0 && !ReportUtils.isChatRoom(report)) { - return; - } - - const avatars = OptionsListUtils.getReportIcons(report, details); - const reportName = ReportUtils.isChatRoom(report) - ? report.reportName - : _.chain(report.participants) - .filter(participant => participant !== currentUserEmail) - .map(participant => lodashGet( - formattedPersonalDetails, - [participant, 'displayName'], - participant, - )) - .value() - .join(', '); - - reportsToUpdate[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = {icons: avatars, reportName}; - }); - - // We use mergeCollection such that it updates ONYXKEYS.COLLECTION.REPORT in one go. - // Any withOnyx subscribers to this key will also receive the complete updated props just once - // than updating props for each report and re-rendering had merge been used. - Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, reportsToUpdate); }); } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 6800a7c4aecd..7b9eefe15d6b 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -25,6 +25,7 @@ import Timers from '../Timers'; import * as ReportActions from './ReportActions'; import Growl from '../Growl'; import * as Localize from '../Localize'; +import * as OptionsListUtils from '../OptionsListUtils'; let currentUserEmail; let currentUserAccountID; @@ -319,6 +320,44 @@ function fetchIOUReportID(debtorEmail) { }); } +function configureReportNameAndIcon(reports) { + // The personalDetails of the participants contain their avatar images. Here we'll go over each + // report and based on the participants we'll link up their avatars to report icons. This will + // skip over default rooms which aren't named by participants. + + Onyx.connect({ + key: ONYXKEYS.PERSONAL_DETAILS, + callback: (formattedPersonalDetails) => { + const reportsToUpdate = {}; + _.each(reports, (report) => { + if (report.participants.length <= 0 && !ReportUtils.isChatRoom(report)) { + return; + } + + const avatars = ReportUtils.isChatRoom(report) ? (['']) : OptionsListUtils.getReportIcons(report, formattedPersonalDetails); + const reportName = ReportUtils.isChatRoom(report) + ? report.reportName + : _.chain(report.participants) + .filter(participant => participant !== currentUserEmail) + .map(participant => lodashGet( + formattedPersonalDetails, + [participant, 'displayName'], + participant, + )) + .value() + .join(', '); + + reportsToUpdate[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = {icons: avatars, reportName}; + }); + + // We use mergeCollection such that it updates ONYXKEYS.COLLECTION.REPORT in one go. + // Any withOnyx subscribers to this key will also receive the complete updated props just once + // than updating props for each report and re-rendering had merge been used. + Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, reportsToUpdate); + }, + }); +} + /** * Fetches chat reports when provided a list of chat report IDs. * If the shouldRedirectIfInaccessible flag is set, we redirect to the Concierge chat @@ -401,6 +440,9 @@ function fetchChatReportsByIDs(chatList, shouldRedirectIfInaccessible = false) { // Fetch the personal details if there are any PersonalDetails.getFromReportParticipants(_.values(simplifiedReports)); + + // Configure Report Name and Report Icon + configureReportNameAndIcon(_.values(simplifiedReports)); return fetchedReports; }) .catch((err) => { From 57f3264ffc3d80b83ff7fd529eacc37e3ca7a954 Mon Sep 17 00:00:00 2001 From: Sobit Neupane <073bct543.sobit@pcampus.edu.np> Date: Tue, 25 Jan 2022 00:09:27 +0545 Subject: [PATCH 2/4] Resolve race condition in fetchChatReportsByIDs --- src/libs/OptionsListUtils.js | 2 +- src/libs/actions/PersonalDetails.js | 4 +- src/libs/actions/Report.js | 61 +++++++++++++---------------- 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 9c89b974b644..4dc9af374968 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -782,7 +782,7 @@ function getReportIcons(report, personalDetails) { } const sortedParticipants = _.map(report.participants, dmParticipant => ({ firstName: lodashGet(personalDetails, [dmParticipant, 'firstName'], ''), - avatar: lodashGet(personalDetails, [dmParticipant, 'avatar'], '') + avatar: lodashGet(personalDetails, [dmParticipant, 'avatarThumbnail'], '') || getDefaultAvatar(dmParticipant), })) .sort((first, second) => first.firstName - second.firstName); diff --git a/src/libs/actions/PersonalDetails.js b/src/libs/actions/PersonalDetails.js index 6bbaf0a52ade..49ab13bc19cc 100644 --- a/src/libs/actions/PersonalDetails.js +++ b/src/libs/actions/PersonalDetails.js @@ -145,6 +145,7 @@ function fetchPersonalDetails() { * Get personal details from report participants. * * @param {Object} reports + * @returns {Promise} */ function getFromReportParticipants(reports) { const participantEmails = _.chain(reports) @@ -157,7 +158,7 @@ function getFromReportParticipants(reports) { return; } - API.PersonalDetails_GetForEmails({emailList: participantEmails.join(',')}) + return API.PersonalDetails_GetForEmails({emailList: participantEmails.join(',')}) .then((data) => { const existingDetails = _.pick(data, participantEmails); @@ -172,6 +173,7 @@ function getFromReportParticipants(reports) { const formattedPersonalDetails = formatPersonalDetails(details); Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, formattedPersonalDetails); + return details; }); } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 7b9eefe15d6b..2d953dd7d26e 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -320,42 +320,37 @@ function fetchIOUReportID(debtorEmail) { }); } -function configureReportNameAndIcon(reports) { +function configureReportNameAndIcon(reports, details) { // The personalDetails of the participants contain their avatar images. Here we'll go over each // report and based on the participants we'll link up their avatars to report icons. This will // skip over default rooms which aren't named by participants. - Onyx.connect({ - key: ONYXKEYS.PERSONAL_DETAILS, - callback: (formattedPersonalDetails) => { - const reportsToUpdate = {}; - _.each(reports, (report) => { - if (report.participants.length <= 0 && !ReportUtils.isChatRoom(report)) { - return; - } - - const avatars = ReportUtils.isChatRoom(report) ? (['']) : OptionsListUtils.getReportIcons(report, formattedPersonalDetails); - const reportName = ReportUtils.isChatRoom(report) - ? report.reportName - : _.chain(report.participants) - .filter(participant => participant !== currentUserEmail) - .map(participant => lodashGet( - formattedPersonalDetails, - [participant, 'displayName'], - participant, - )) - .value() - .join(', '); - - reportsToUpdate[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = {icons: avatars, reportName}; - }); + const reportsToUpdate = {}; + _.each(reports, (report) => { + if (report.participants.length <= 0 && !ReportUtils.isChatRoom(report)) { + return; + } - // We use mergeCollection such that it updates ONYXKEYS.COLLECTION.REPORT in one go. - // Any withOnyx subscribers to this key will also receive the complete updated props just once - // than updating props for each report and re-rendering had merge been used. - Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, reportsToUpdate); - }, + const avatars = ReportUtils.isChatRoom(report) ? (['']) : OptionsListUtils.getReportIcons(report, details); + const reportName = ReportUtils.isChatRoom(report) + ? report.reportName + : _.chain(report.participants) + .filter(participant => participant !== currentUserEmail) + .map(participant => lodashGet( + details, + [participant, 'displayName'], + participant, + )) + .value() + .join(', '); + + reportsToUpdate[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = {icons: avatars, reportName}; }); + + // We use mergeCollection such that it updates ONYXKEYS.COLLECTION.REPORT in one go. + // Any withOnyx subscribers to this key will also receive the complete updated props just once + // than updating props for each report and re-rendering had merge been used. + Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, reportsToUpdate); } /** @@ -439,10 +434,10 @@ function fetchChatReportsByIDs(chatList, shouldRedirectIfInaccessible = false) { Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, simplifiedReports); // Fetch the personal details if there are any - PersonalDetails.getFromReportParticipants(_.values(simplifiedReports)); + Promise.resolve(PersonalDetails.getFromReportParticipants(_.values(simplifiedReports))).then( + details => configureReportNameAndIcon(_.values(simplifiedReports), details), + ); - // Configure Report Name and Report Icon - configureReportNameAndIcon(_.values(simplifiedReports)); return fetchedReports; }) .catch((err) => { From 753ad929618f3f859ef3077ff5dab3d0037eb150 Mon Sep 17 00:00:00 2001 From: Sobit Neupane <073bct543.sobit@pcampus.edu.np> Date: Tue, 25 Jan 2022 11:37:35 +0545 Subject: [PATCH 3/4] Make syntatic change in use of Promise --- src/libs/actions/PersonalDetails.js | 2 +- src/libs/actions/Report.js | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/PersonalDetails.js b/src/libs/actions/PersonalDetails.js index 49ab13bc19cc..a4f868acd6cb 100644 --- a/src/libs/actions/PersonalDetails.js +++ b/src/libs/actions/PersonalDetails.js @@ -155,7 +155,7 @@ function getFromReportParticipants(reports) { .value(); if (participantEmails.length === 0) { - return; + return Promise.resolve([]); } return API.PersonalDetails_GetForEmails({emailList: participantEmails.join(',')}) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 2d953dd7d26e..6f78ae6cdc83 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -433,10 +433,11 @@ function fetchChatReportsByIDs(chatList, shouldRedirectIfInaccessible = false) { Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT_IOUS, reportIOUData); Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, simplifiedReports); + const simplifiedReportsList = _.values(simplifiedReports); + // Fetch the personal details if there are any - Promise.resolve(PersonalDetails.getFromReportParticipants(_.values(simplifiedReports))).then( - details => configureReportNameAndIcon(_.values(simplifiedReports), details), - ); + PersonalDetails.getFromReportParticipants(simplifiedReportsList) + .then(details => configureReportNameAndIcon(simplifiedReportsList, details)); return fetchedReports; }) From 8e19bf3efc8891d9676c8e7fa9e381766ee5ecdd Mon Sep 17 00:00:00 2001 From: Sobit Neupane <073bct543.sobit@pcampus.edu.np> Date: Tue, 25 Jan 2022 12:02:22 +0545 Subject: [PATCH 4/4] Maintain consistent interface --- src/libs/actions/PersonalDetails.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/PersonalDetails.js b/src/libs/actions/PersonalDetails.js index a4f868acd6cb..aaeb85f953d9 100644 --- a/src/libs/actions/PersonalDetails.js +++ b/src/libs/actions/PersonalDetails.js @@ -155,7 +155,7 @@ function getFromReportParticipants(reports) { .value(); if (participantEmails.length === 0) { - return Promise.resolve([]); + return Promise.resolve({}); } return API.PersonalDetails_GetForEmails({emailList: participantEmails.join(',')})