Skip to content

perf: remove sorting from findLastAccessedReport #44948

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 10, 2024
Merged

perf: remove sorting from findLastAccessedReport #44948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4bec514
perf: remove sorting from findLastAccessedReport
kosmydel Jul 8, 2024
87c521d
fix tests
kosmydel Jul 8, 2024
fa901f2
Merge branch 'main' into @kosmydel/perf/fix-useLastAccessedReportID
kosmydel Jul 10, 2024
a09c322
rename to getMostRecentlyVisitedReport
kosmydel Jul 10, 2024
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
49 changes: 22 additions & 27 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -1096,20 +1097,6 @@ function filterReportsByPolicyIDAndMemberAccountIDs(reports: Array<OnyxEntry<Rep
return reports.filter((report) => !!report && doesReportBelongToWorkspace(report, policyMemberAccountIDs, policyID));
}

/**
* Given an array of reports, return them sorted by the last read timestamp.
*/
function sortReportsByLastRead(reports: Array<OnyxEntry<Report>>, reportMetadata: OnyxCollection<ReportMetadata>): Array<OnyxEntry<Report>> {
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
*/
Expand Down Expand Up @@ -1170,6 +1157,13 @@ function hasExpensifyGuidesEmails(accountIDs: number[]): boolean {
return accountIDs.some((accountID) => Str.extractEmailDomain(allPersonalDetails?.[accountID]?.login ?? '') === CONST.EMAIL.GUIDES_DOMAIN);
}

function getMostRecentlyVisitedReport(reports: Array<OnyxEntry<Report>>, reportMetadata: OnyxCollection<ReportMetadata>): OnyxEntry<Report> {
const filteredReports = reports.filter(
(report) => !!report?.reportID && !!(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`]?.lastVisitTime ?? report?.lastReadTime),
Comment on lines +1161 to +1162
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have added a filter to exclude the left chat reports as well. This later caused #46085

);
return lodashMaxBy(filteredReports, (a) => new Date(reportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${a?.reportID}`]?.lastVisitTime ?? a?.lastReadTime ?? '').valueOf());
}

function findLastAccessedReport(ignoreDomainRooms: boolean, openOnAdminRoom = false, policyID?: string, excludeReportID?: string): OnyxEntry<Report> {
// If it's the user's first time using New Expensify, then they could either have:
// - just a Concierge report, if so we'll return that
Expand All @@ -1186,11 +1180,9 @@ function findLastAccessedReport(ignoreDomainRooms: boolean, openOnAdminRoom = fa
reportsValues = filterReportsByPolicyIDAndMemberAccountIDs(reportsValues, policyMemberAccountIDs, policyID);
}

let sortedReports = sortReportsByLastRead(reportsValues, allReportMetadata);

let adminReport: OnyxEntry<Report>;
if (openOnAdminRoom) {
adminReport = sortedReports.find((report) => {
adminReport = reportsValues.find((report) => {
const chatType = getChatType(report);
return chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS;
});
Expand All @@ -1199,7 +1191,7 @@ function findLastAccessedReport(ignoreDomainRooms: boolean, openOnAdminRoom = fa
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const shouldFilter = excludeReportID || ignoreDomainRooms;
if (shouldFilter) {
sortedReports = sortedReports.filter((report) => {
reportsValues = reportsValues.filter((report) => {
if (excludeReportID && report?.reportID === excludeReportID) {
return false;
}
Expand All @@ -1222,22 +1214,25 @@ function findLastAccessedReport(ignoreDomainRooms: boolean, openOnAdminRoom = fa

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];
reportsValues = reportsValues.filter((report) => !isSystemChat(report)) ?? [];
if (reportsValues.length === 1) {
return reportsValues[0];
}

return adminReport ?? sortedReports.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 = sortedReports.find((report) => isSystemChat(report)) ?? false;
if (sortedReports.length === 2 && hasSystemChat) {
sortedReports = sortedReports.filter((report) => !isSystemChat(report)) ?? [];
const hasSystemChat = reportsValues.find((report) => isSystemChat(report)) ?? false;
if (reportsValues.length === 2 && hasSystemChat) {
reportsValues = reportsValues.filter((report) => !isSystemChat(report)) ?? [];
}

return adminReport ?? sortedReports.at(-1);
// We are getting the last read report from the metadata of the report.
const lastRead = getMostRecentlyVisitedReport(reportsValues, allReportMetadata);

return adminReport ?? lastRead;
}

/**
Expand Down Expand Up @@ -7344,7 +7339,6 @@ export {
shouldShowFlagComment,
shouldShowRBRForMissingSmartscanFields,
shouldUseFullTitleToDisplay,
sortReportsByLastRead,
updateOptimisticParentReportAction,
updateReportPreview,
temporary_getMoneyRequestOptions,
Expand All @@ -7362,6 +7356,7 @@ export {
getChatUsedForOnboarding,
findPolicyExpenseChatByPolicyID,
hasOnlyNonReimbursableTransactions,
getMostRecentlyVisitedReport,
};

export type {
Expand Down
12 changes: 4 additions & 8 deletions tests/unit/ReportUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,8 +767,8 @@ describe('ReportUtils', () => {
});
});

describe('sortReportsByLastRead', () => {
it('should filter out report without reportID & lastReadTime and sort lastReadTime in ascending order', () => {
describe('getMostRecentlyVisitedReport', () => {
it('should filter out report without reportID & lastReadTime and return the most recently visited report', () => {
const reports: Array<OnyxEntry<Report>> = [
{reportID: '1', lastReadTime: '2023-07-08 07:15:44.030'},
{reportID: '2', lastReadTime: undefined},
Expand All @@ -778,12 +778,8 @@ describe('ReportUtils', () => {
{reportID: '6'},
undefined,
];
const sortedReports: Array<OnyxEntry<Report>> = [
{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<Report> = {reportID: '1', lastReadTime: '2023-07-08 07:15:44.030'};
expect(ReportUtils.getMostRecentlyVisitedReport(reports, undefined)).toEqual(latestReport);
});
});

Expand Down
Loading