Skip to content
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.

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

Improve the readability and maintainability of the code which groups and sorts the reports in the sidebar #12558

Merged
merged 16 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
117 changes: 61 additions & 56 deletions src/libs/SidebarUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,85 +86,90 @@ Onyx.connect({
* @returns {String[]} An array of reportIDs sorted in the proper order
*/
function getOrderedReportIDs(reportIDFromRoute) {
let recentReportOptions = [];
const pinnedReportOptions = [];
const iouDebtReportOptions = [];
const draftReportOptions = [];

const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInGSDMode;

// Filter out all the reports that shouldn't be displayed
const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, reportIDFromRoute, isInGSDMode, currentUserLogin, iouReports, betas, policies));
const reportsToDisplay = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, reportIDFromRoute, isInGSDMode, currentUserLogin, iouReports, betas, policies));

// 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) => {
_.each(reportsToDisplay, (report) => {
// 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:
// - When in default mode, reports will be ordered by most recently updated (in descending order) so that the most recently updated are at the top
// - When in GSD mode, reports are ordered by their display name so they are alphabetical (in ascending order)
// - Regardless of mode, all archived reports should remain at the bottom
const orderedReports = _.sortBy(filteredReportsWithReportName, (report) => {
if (ReportUtils.isArchivedRoom(report)) {
return isInDefaultMode

// -Infinity is used here because there is no chance that a report will ever have an older timestamp than -Infinity and it ensures that archived reports
// will always be listed last
? -Infinity

// Similar logic is used for 'ZZZZZZZZZZZZZ' to reasonably assume that no report will ever have a report name that will be listed alphabetically after this, ensuring that
// archived reports will be listed last
: 'ZZZZZZZZZZZZZ';
// 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 - Always sorted by reportDisplayName
// 2. Outstanding IOUs - Always sorted by iouReportAmount with the largest amounts at the top of the group
// 3. Drafts - Always sorted by reportDisplayName
// 4. Non-archived reports
// - Sorted by lastMessageTimestamp in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
// 5. Archived reports
// - Sorted by lastMessageTimestamp in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
const reportGroups = _.groupBy(reportsToDisplay, (report) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Grouping these into an object keyed with these strings is less clear than something like setting up a bunch of empty arrays:

const pinnedReports = [];
const outstandingIOUReports = [];

I forgot what groupBy does, wasn't really sure what was happening here and had to look it up.

Could we just set up the arrays, then sort them, then concat them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is a good idea and hopefully it will be more clear that way. I've update the PR for that.

if (report.isPinned) {
return 'isPinned';
}

return isInDefaultMode ? report.lastMessageTimestamp : report.reportDisplayName;
});

// Apply the decsending order to reports when in default mode
if (isInDefaultMode) {
orderedReports.reverse();
}
if (report.hasOutstandingIOU && !report.isIOUReportOwner) {
return 'hasOutstandingIOU';
}

// Put all the reports into the different buckets
for (let i = 0; i < orderedReports.length; i++) {
const report = orderedReports[i];
if (report.hasDraft) {
return 'hasDraft';
}

// If the report is pinned and we are using the option to display pinned reports on top then we need to
// collect the pinned reports so we can sort them alphabetically once they are collected
if (report.isPinned) {
pinnedReportOptions.push(report);
} else if (report.hasOutstandingIOU && !report.isIOUReportOwner) {
iouDebtReportOptions.push(report);
} else if (report.hasDraft) {
draftReportOptions.push(report);
} else {
recentReportOptions.push(report);
if (ReportUtils.isArchivedRoom(report)) {
return 'archived';
}
}

// Prioritizing reports with draft comments, add them before the normal recent report options
// and sort them by report name.
const sortedDraftReports = _.sortBy(draftReportOptions, 'reportDisplayName');
recentReportOptions = sortedDraftReports.concat(recentReportOptions);
return 'nonArchived';
});

// Prioritizing IOUs the user owes, add them before the normal recent report options and reports
// with draft comments.
const sortedIOUReports = _.sortBy(iouDebtReportOptions, 'iouReportAmount').reverse();
recentReportOptions = sortedIOUReports.concat(recentReportOptions);
// Now each group can be sorted accordingly
const sortedGroups = {};
_.each(reportGroups, (reportGroup, groupName) => {
let sortedGroup;
switch (groupName) {
case 'hasOutstandingIOU':
sortedGroup = _.sortBy(reportGroup, 'iouReportAmount').reverse();
break;

case 'isPinned':
case 'hasDraft':
sortedGroup = _.sortBy(reportGroup, 'reportDisplayName');
break;

case 'archived':
case 'nonArchived':
sortedGroup = _.sortBy(reportGroup, isInDefaultMode ? 'lastMessageTimestamp' : 'reportDisplayName');
if (isInDefaultMode) {
sortedGroup.reverse();
}
break;

default:
break;
}

// If we are prioritizing our pinned reports then shift them to the front and sort them by report name
const sortedPinnedReports = _.sortBy(pinnedReportOptions, 'reportDisplayName');
recentReportOptions = sortedPinnedReports.concat(recentReportOptions);
// The sorted groups only need to contain the reportID because that's all that needs returned from getOrderedReportIDs()
sortedGroups[groupName] = _.pluck(sortedGroup, 'reportID');
});

return _.pluck(recentReportOptions, 'reportID');
// Now that we have all the report IDs grouped and sorted, they must be flattened into an array to be returned.
// The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar.
return []
.concat(sortedGroups.isPinned || [])
.concat(sortedGroups.hasOutstandingIOU || [])
.concat(sortedGroups.hasDraft || [])
.concat(sortedGroups.nonArchived || [])
.concat(sortedGroups.archived || []);
}

/**
Expand Down
File renamed without changes.
File renamed without changes.