Skip to content

perf: Refactor sortReportsToDisplayInLHN by splitting into smaller parts #68005

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
JS00001 merged 4 commits into from
Aug 14, 2025
Merged

perf: Refactor sortReportsToDisplayInLHN by splitting into smaller parts #68005

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
318e86c
perf: Refactor sortReportsToDisplayInLHN by splitting into smaller parts
OlimpiaZurek Aug 6, 2025
94b7eb5
add comments
OlimpiaZurek Aug 6, 2025
da052cb
tweaks after code review
OlimpiaZurek Aug 13, 2025
9474c20
remove coalesce for isPinned
OlimpiaZurek Aug 14, 2025
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
164 changes: 125 additions & 39 deletions src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@

const visibleReportActionItems: ReportActions = {};
let allPersonalDetails: OnyxEntry<PersonalDetailsList>;
Onyx.connect({

Check warning on line 135 in src/libs/SidebarUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand All @@ -140,7 +140,7 @@
});

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 143 in src/libs/SidebarUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -148,7 +148,7 @@
},
});

Onyx.connect({

Check warning on line 151 in src/libs/SidebarUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
callback: (actions, key) => {
if (!actions || !key) {
Expand Down Expand Up @@ -336,68 +336,90 @@

return displayedReportsCopy;
}

/**
* @returns An array of reportIDs sorted in the proper order
* Categorizes reports into their respective LHN groups
*/
function sortReportsToDisplayInLHN(
function categorizeReportsForLHN(
reportsToDisplay: ReportsToDisplayInLHN,
priorityMode: OnyxEntry<PriorityMode>,
localeCompare: LocaleContextProps['localeCompare'],
reportNameValuePairs?: OnyxCollection<ReportNameValuePairs>,
reportAttributes?: ReportAttributesDerivedValue['reports'],
): string[] {
Performance.markStart(CONST.TIMING.GET_ORDERED_REPORT_IDS);
const isInFocusMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInFocusMode;
// The LHN is split into five distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
// 1. Pinned/GBR - Always sorted by reportDisplayName
// 2. Error reports - Always sorted by reportDisplayName
// 3. Drafts - Always sorted by reportDisplayName
// 4. Non-archived reports and settled IOUs
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
// 5. Archived reports
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode

) {
const pinnedAndGBRReports: MiniReport[] = [];
const errorReports: MiniReport[] = [];
const draftReports: MiniReport[] = [];
const nonArchivedReports: MiniReport[] = [];
const archivedReports: MiniReport[] = [];

// There are a few properties that need to be calculated for the report which are used when sorting reports.
Object.values(reportsToDisplay).forEach((reportToDisplay) => {
const report = reportToDisplay;
for (const report of Object.values(reportsToDisplay)) {
const reportID = report.reportID;
const isPinned = !!report.isPinned;
const hasErrors = !!report.hasErrorsOtherThanFailedReceipt;
const hasDraft = reportID ? hasValidDraftComment(reportID) : false;
const reportNameValuePairsKey = `${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportID}`;
const rNVPs = reportNameValuePairs?.[reportNameValuePairsKey];

const miniReport: MiniReport = {
reportID: report?.reportID,
reportID,
displayName: getReportName(report),
lastVisibleActionCreated: report?.lastVisibleActionCreated,
lastVisibleActionCreated: report.lastVisibleActionCreated,
};

const isPinned = report?.isPinned ?? false;
const rNVPs = reportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID}`];
if (isPinned || reportAttributes?.[report?.reportID]?.requiresAttention) {
if (isPinned || (reportID && reportAttributes?.[reportID]?.requiresAttention)) {
pinnedAndGBRReports.push(miniReport);
} else if (report?.hasErrorsOtherThanFailedReceipt) {
} else if (hasErrors) {
errorReports.push(miniReport);
} else if (hasValidDraftComment(report?.reportID)) {
} else if (hasDraft) {
draftReports.push(miniReport);
} else if (isArchivedNonExpenseReport(report, !!rNVPs?.private_isArchived)) {
archivedReports.push(miniReport);
} else {
nonArchivedReports.push(miniReport);
}
});
}

return {
pinnedAndGBRReports,
errorReports,
draftReports,
nonArchivedReports,
archivedReports,
};
}

/**
* Sorts categorized reports and returns new sorted arrays (pure function).
* This function does not mutate the input and returns new arrays for better testability.
*/
function sortCategorizedReports(
categories: {
pinnedAndGBRReports: MiniReport[];
errorReports: MiniReport[];
draftReports: MiniReport[];
nonArchivedReports: MiniReport[];
archivedReports: MiniReport[];
},
isInDefaultMode: boolean,
localeCompare: LocaleContextProps['localeCompare'],
): {
pinnedAndGBRReports: MiniReport[];
errorReports: MiniReport[];
draftReports: MiniReport[];
nonArchivedReports: MiniReport[];
archivedReports: MiniReport[];
} {
const {pinnedAndGBRReports, errorReports, draftReports, nonArchivedReports, archivedReports} = categories;

// Sort each group of reports accordingly
pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
errorReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
draftReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
const sortedPinnedAndGBRReports = [...pinnedAndGBRReports].sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
const sortedErrorReports = [...errorReports].sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
const sortedDraftReports = [...draftReports].sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
Comment thread
JS00001 marked this conversation as resolved.

let sortedNonArchivedReports: MiniReport[];
let sortedArchivedReports: MiniReport[];

if (isInDefaultMode) {
nonArchivedReports.sort((a, b) => {
sortedNonArchivedReports = [...nonArchivedReports].sort((a, b) => {
const compareDates = a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0;
if (compareDates) {
return compareDates;
Expand All @@ -406,19 +428,80 @@
return compareDisplayNames;
});
// For archived reports ensure that most recent reports are at the top by reversing the order
archivedReports.sort((a, b) => (a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0));
sortedArchivedReports = [...archivedReports].sort((a, b) =>
a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0,
);
} else {
nonArchivedReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
archivedReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
sortedNonArchivedReports = [...nonArchivedReports].sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
sortedArchivedReports = [...archivedReports].sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
}

return {
pinnedAndGBRReports: sortedPinnedAndGBRReports,
errorReports: sortedErrorReports,
draftReports: sortedDraftReports,
nonArchivedReports: sortedNonArchivedReports,
archivedReports: sortedArchivedReports,
};
}

/**
* Combines sorted report categories and extracts report IDs
*/
function combineReportCategories(
pinnedAndGBRReports: MiniReport[],
errorReports: MiniReport[],
draftReports: MiniReport[],
nonArchivedReports: MiniReport[],
archivedReports: MiniReport[],
): string[] {
// Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID.
// The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar.
return [...pinnedAndGBRReports, ...errorReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report?.reportID).filter(Boolean) as string[];
}

const LHNReports = [...pinnedAndGBRReports, ...errorReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report?.reportID).filter(Boolean) as string[];
/**
* @returns An array of reportIDs sorted in the proper order
*/
function sortReportsToDisplayInLHN(
Comment thread
OlimpiaZurek marked this conversation as resolved.
Outdated
reportsToDisplay: ReportsToDisplayInLHN,
priorityMode: OnyxEntry<PriorityMode>,
localeCompare: LocaleContextProps['localeCompare'],
reportNameValuePairs?: OnyxCollection<ReportNameValuePairs>,
reportAttributes?: ReportAttributesDerivedValue['reports'],
): string[] {
Performance.markStart(CONST.TIMING.GET_ORDERED_REPORT_IDS);

const isInFocusMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInFocusMode;
// The LHN is split into five distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
// 1. Pinned/GBR - Always sorted by reportDisplayName
// 2. Error reports - Always sorted by reportDisplayName
// 3. Drafts - Always sorted by reportDisplayName
// 4. Non-archived reports and settled IOUs
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
// 5. Archived reports
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode

// Step 1: Categorize reports
const categories = categorizeReportsForLHN(reportsToDisplay, reportNameValuePairs, reportAttributes);

// Step 2: Sort each category
const sortedCategories = sortCategorizedReports(categories, isInDefaultMode, localeCompare);

// Step 3: Combine and extract IDs
const result = combineReportCategories(
sortedCategories.pinnedAndGBRReports,
sortedCategories.errorReports,
sortedCategories.draftReports,
sortedCategories.nonArchivedReports,
sortedCategories.archivedReports,
);

Performance.markEnd(CONST.TIMING.GET_ORDERED_REPORT_IDS);
return LHNReports;
return result;
}

type ReasonAndReportActionThatHasRedBrickRoad = {
Expand Down Expand Up @@ -961,6 +1044,9 @@
export default {
getOptionData,
sortReportsToDisplayInLHN,
categorizeReportsForLHN,
sortCategorizedReports,
combineReportCategories,
getWelcomeMessage,
getReasonAndReportActionThatHasRedBrickRoad,
shouldShowRedBrickRoad,
Expand Down
Loading
Loading