From 4bec51471eb177f4c34824f8fafb8ab22d3c8d32 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 8 Jul 2024 11:47:14 +0200 Subject: [PATCH 1/3] perf: remove sorting from findLastAccessedReport --- src/libs/ReportUtils.ts | 45 +++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 342e2439ed66..2f7e3823203a 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -5,6 +5,7 @@ import lodashEscape from 'lodash/escape'; import lodashFindLastIndex from 'lodash/findLastIndex'; import lodashIntersection from 'lodash/intersection'; import lodashIsEqual from 'lodash/isEqual'; +import lodashMaxBy from 'lodash/maxBy'; import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import type {OriginalMessageModifiedExpense} from 'src/types/onyx/OriginalMessage'; @@ -1085,20 +1086,6 @@ function filterReportsByPolicyIDAndMemberAccountIDs(reports: Array !!report && doesReportBelongToWorkspace(report, policyMemberAccountIDs, policyID)); } -/** - * Given an array of reports, return them sorted by the last read timestamp. - */ -function sortReportsByLastRead(reports: Array>, reportMetadata: OnyxCollection): Array> { - return reports - .filter((report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime)) - .sort((a, b) => { - const aTime = new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? ''); - const bTime = new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${b?.reportID}`]?.lastVisitTime ?? b?.lastReadTime ?? ''); - - return aTime.valueOf() - bTime.valueOf(); - }); -} - /** * Returns true if report is still being processed */ @@ -1182,11 +1169,13 @@ function findLastAccessedReport( reportsValues = filterReportsByPolicyIDAndMemberAccountIDs(reportsValues, policyMemberAccountIDs, policyID); } - let sortedReports = sortReportsByLastRead(reportsValues, reportMetadata); + let filteredReports = reportsValues.filter( + (report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime), + ); let adminReport: OnyxEntry; if (openOnAdminRoom) { - adminReport = sortedReports.find((report) => { + adminReport = filteredReports.find((report) => { const chatType = getChatType(report); return chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS; }); @@ -1195,7 +1184,7 @@ function findLastAccessedReport( // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const shouldFilter = excludeReportID || ignoreDomainRooms; if (shouldFilter) { - sortedReports = sortedReports.filter((report) => { + filteredReports = filteredReports.filter((report) => { if (excludeReportID && report?.reportID === excludeReportID) { return false; } @@ -1218,22 +1207,27 @@ function findLastAccessedReport( if (isFirstTimeNewExpensifyUser) { // Filter out the systemChat report from the reports list, as we don't want to drop the user into that report over Concierge when they first log in - sortedReports = sortedReports.filter((report) => !isSystemChat(report)) ?? []; - if (sortedReports.length === 1) { - return sortedReports[0]; + filteredReports = filteredReports.filter((report) => !isSystemChat(report)) ?? []; + if (filteredReports.length === 1) { + return filteredReports[0]; } - return adminReport ?? sortedReports.find((report) => !isConciergeChatReport(report)); + return adminReport ?? filteredReports.find((report) => !isConciergeChatReport(report)); } // If we only have two reports and one of them is the system chat, filter it out so we don't // overwrite showing the concierge chat - const hasSystemChat = sortedReports.find((report) => isSystemChat(report)) ?? false; - if (sortedReports.length === 2 && hasSystemChat) { - sortedReports = sortedReports.filter((report) => !isSystemChat(report)) ?? []; + const hasSystemChat = filteredReports.find((report) => isSystemChat(report)) ?? false; + if (filteredReports.length === 2 && hasSystemChat) { + filteredReports = filteredReports.filter((report) => !isSystemChat(report)) ?? []; } - return adminReport ?? sortedReports.at(-1); + // We are getting the last read report from the metadata of the report. + const lastRead = lodashMaxBy(filteredReports, (a) => + new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? '').valueOf(), + ); + + return adminReport ?? lastRead; } /** @@ -7295,7 +7289,6 @@ export { shouldShowFlagComment, shouldShowRBRForMissingSmartscanFields, shouldUseFullTitleToDisplay, - sortReportsByLastRead, updateOptimisticParentReportAction, updateReportPreview, temporary_getMoneyRequestOptions, From 87c521d03d9bdf9d493f5a1393e5e32281ede9ca Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Mon, 8 Jul 2024 12:22:24 +0200 Subject: [PATCH 2/3] fix tests --- src/libs/ReportUtils.ts | 34 ++++++++++++++++++---------------- tests/unit/ReportUtilsTest.ts | 12 ++++-------- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 2f7e3823203a..fd5657f5ca0c 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -1146,6 +1146,13 @@ function hasExpensifyGuidesEmails(accountIDs: number[]): boolean { return accountIDs.some((accountID) => Str.extractEmailDomain(allPersonalDetails?.[accountID]?.login ?? '') === CONST.EMAIL.GUIDES_DOMAIN); } +function getLatestReport(reports: Array>, reportMetadata: OnyxCollection): OnyxEntry { + const filteredReports = reports.filter( + (report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime), + ); + return lodashMaxBy(filteredReports, (a) => new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? '').valueOf()); +} + function findLastAccessedReport( reports: OnyxCollection, ignoreDomainRooms: boolean, @@ -1169,13 +1176,9 @@ function findLastAccessedReport( reportsValues = filterReportsByPolicyIDAndMemberAccountIDs(reportsValues, policyMemberAccountIDs, policyID); } - let filteredReports = reportsValues.filter( - (report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime), - ); - let adminReport: OnyxEntry; if (openOnAdminRoom) { - adminReport = filteredReports.find((report) => { + adminReport = reportsValues.find((report) => { const chatType = getChatType(report); return chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS; }); @@ -1184,7 +1187,7 @@ function findLastAccessedReport( // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const shouldFilter = excludeReportID || ignoreDomainRooms; if (shouldFilter) { - filteredReports = filteredReports.filter((report) => { + reportsValues = reportsValues.filter((report) => { if (excludeReportID && report?.reportID === excludeReportID) { return false; } @@ -1207,25 +1210,23 @@ function findLastAccessedReport( if (isFirstTimeNewExpensifyUser) { // Filter out the systemChat report from the reports list, as we don't want to drop the user into that report over Concierge when they first log in - filteredReports = filteredReports.filter((report) => !isSystemChat(report)) ?? []; - if (filteredReports.length === 1) { - return filteredReports[0]; + reportsValues = reportsValues.filter((report) => !isSystemChat(report)) ?? []; + if (reportsValues.length === 1) { + return reportsValues[0]; } - return adminReport ?? filteredReports.find((report) => !isConciergeChatReport(report)); + return adminReport ?? reportsValues.find((report) => !isConciergeChatReport(report)); } // If we only have two reports and one of them is the system chat, filter it out so we don't // overwrite showing the concierge chat - const hasSystemChat = filteredReports.find((report) => isSystemChat(report)) ?? false; - if (filteredReports.length === 2 && hasSystemChat) { - filteredReports = filteredReports.filter((report) => !isSystemChat(report)) ?? []; + const hasSystemChat = reportsValues.find((report) => isSystemChat(report)) ?? false; + if (reportsValues.length === 2 && hasSystemChat) { + reportsValues = reportsValues.filter((report) => !isSystemChat(report)) ?? []; } // We are getting the last read report from the metadata of the report. - const lastRead = lodashMaxBy(filteredReports, (a) => - new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? '').valueOf(), - ); + const lastRead = getLatestReport(reportsValues, reportMetadata); return adminReport ?? lastRead; } @@ -7306,6 +7307,7 @@ export { getChatUsedForOnboarding, findPolicyExpenseChatByPolicyID, hasOnlyNonReimbursableTransactions, + getLatestReport, }; export type { diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 4ec530036ba5..189f285e4834 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -767,8 +767,8 @@ describe('ReportUtils', () => { }); }); - describe('sortReportsByLastRead', () => { - it('should filter out report without reportID & lastReadTime and sort lastReadTime in ascending order', () => { + describe('getLatestReport', () => { + it('should filter out report without reportID & lastReadTime and return the report with the latest lastReadTime', () => { const reports: Array> = [ {reportID: '1', lastReadTime: '2023-07-08 07:15:44.030'}, {reportID: '2', lastReadTime: undefined}, @@ -778,12 +778,8 @@ describe('ReportUtils', () => { {reportID: '6'}, undefined, ]; - const sortedReports: Array> = [ - {reportID: '3', lastReadTime: '2023-07-06 07:15:44.030'}, - {reportID: '4', lastReadTime: '2023-07-07 07:15:44.030', type: CONST.REPORT.TYPE.IOU}, - {reportID: '1', lastReadTime: '2023-07-08 07:15:44.030'}, - ]; - expect(ReportUtils.sortReportsByLastRead(reports, undefined)).toEqual(sortedReports); + const latestReport: OnyxEntry = {reportID: '1', lastReadTime: '2023-07-08 07:15:44.030'}; + expect(ReportUtils.getLatestReport(reports, undefined)).toEqual(latestReport); }); }); From a09c3221e4f85da69e9ae0ef43367d400517df59 Mon Sep 17 00:00:00 2001 From: Jakub Kosmydel <104823336+kosmydel@users.noreply.github.com> Date: Wed, 10 Jul 2024 09:17:49 +0200 Subject: [PATCH 3/3] rename to getMostRecentlyVisitedReport --- src/libs/ReportUtils.ts | 6 +++--- tests/unit/ReportUtilsTest.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 7e4d7718c200..fb3bebd75274 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -1157,7 +1157,7 @@ function hasExpensifyGuidesEmails(accountIDs: number[]): boolean { return accountIDs.some((accountID) => Str.extractEmailDomain(allPersonalDetails?.[accountID]?.login ?? '') === CONST.EMAIL.GUIDES_DOMAIN); } -function getLatestReport(reports: Array>, reportMetadata: OnyxCollection): OnyxEntry { +function getMostRecentlyVisitedReport(reports: Array>, reportMetadata: OnyxCollection): OnyxEntry { const filteredReports = reports.filter( (report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime), ); @@ -1230,7 +1230,7 @@ function findLastAccessedReport(ignoreDomainRooms: boolean, openOnAdminRoom = fa } // We are getting the last read report from the metadata of the report. - const lastRead = getLatestReport(reportsValues, allReportMetadata); + const lastRead = getMostRecentlyVisitedReport(reportsValues, allReportMetadata); return adminReport ?? lastRead; } @@ -7356,7 +7356,7 @@ export { getChatUsedForOnboarding, findPolicyExpenseChatByPolicyID, hasOnlyNonReimbursableTransactions, - getLatestReport, + getMostRecentlyVisitedReport, }; export type { diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 189f285e4834..aa1a2a60628c 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -767,8 +767,8 @@ describe('ReportUtils', () => { }); }); - describe('getLatestReport', () => { - it('should filter out report without reportID & lastReadTime and return the report with the latest lastReadTime', () => { + describe('getMostRecentlyVisitedReport', () => { + it('should filter out report without reportID & lastReadTime and return the most recently visited report', () => { const reports: Array> = [ {reportID: '1', lastReadTime: '2023-07-08 07:15:44.030'}, {reportID: '2', lastReadTime: undefined}, @@ -779,7 +779,7 @@ describe('ReportUtils', () => { undefined, ]; const latestReport: OnyxEntry = {reportID: '1', lastReadTime: '2023-07-08 07:15:44.030'}; - expect(ReportUtils.getLatestReport(reports, undefined)).toEqual(latestReport); + expect(ReportUtils.getMostRecentlyVisitedReport(reports, undefined)).toEqual(latestReport); }); });