Skip to content

Commit

Permalink
Merge pull request #12558 from Expensify/tgolen-improving-grouping-so…
Browse files Browse the repository at this point in the history 
…rting-reports

Improve the readability and maintainability of the code which groups and sorts the reports in the sidebar
  • Loading branch information
@tgolen
tgolen authored Nov 18, 2022
2 parents 46c1c7b + dc9bc97 commit 1ca61d8
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 76 deletions.
9 changes: 2 additions & 7 deletions src/libs/OptionsListUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,13 +357,8 @@ function createOption(logins, personalDetails, report, reportActions = {}, {
result.alternateText = Str.removeSMSDomain(personalDetail.login);
}

if (result.hasOutstandingIOU) {
const iouReport = iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`] || null;
if (iouReport) {
result.isIOUReportOwner = iouReport.ownerEmail === currentUserLogin;
result.iouReportAmount = iouReport.total;
}
}
result.isIOUReportOwner = ReportUtils.isIOUOwnedByCurrentUser(result, currentUserLogin, iouReports);
result.iouReportAmount = ReportUtils.getIOUTotal(result, iouReports);

if (!hasMultipleParticipants) {
result.login = personalDetail.login;
Expand Down
36 changes: 35 additions & 1 deletion src/libs/ReportUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,6 @@ function isUnread(report) {
* @param {Object} iouReports
* @returns {boolean}
*/

function hasOutstandingIOU(report, currentUserLogin, iouReports) {
if (!report || !report.iouReportID || _.isUndefined(report.hasOutstandingIOU)) {
return false;
Expand All @@ -985,6 +984,39 @@ function hasOutstandingIOU(report, currentUserLogin, iouReports) {
return report.hasOutstandingIOU;
}

/**
* @param {Object} report
* @param {String} report.iouReportID
* @param {Object} iouReports
* @returns {Number}
*/
function getIOUTotal(report, iouReports = {}) {
if (report.hasOutstandingIOU) {
const iouReport = iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`];
if (iouReport) {
return iouReport.total;
}
}
return 0;
}

/**
* @param {Object} report
* @param {String} report.iouReportID
* @param {String} currentUserLogin
* @param {Object} iouReports
* @returns {Boolean}
*/
function isIOUOwnedByCurrentUser(report, currentUserLogin, iouReports = {}) {
if (report.hasOutstandingIOU) {
const iouReport = iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`];
if (iouReport) {
return iouReport.ownerEmail === currentUserLogin;
}
}
return false;
}

/**
* Takes several pieces of data from Onyx and evaluates if a report should be shown in the option list (either when searching
* for reports or the reports shown in the LHN).
Expand Down Expand Up @@ -1098,6 +1130,8 @@ export {
hasExpensifyEmails,
hasExpensifyGuidesEmails,
hasOutstandingIOU,
isIOUOwnedByCurrentUser,
getIOUTotal,
canShowReportRecipientLocalTime,
formatReportLastMessageText,
chatIncludesConcierge,
Expand Down
136 changes: 68 additions & 68 deletions src/libs/SidebarUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,16 @@ Onyx.connect({
callback: val => policies = val,
});

let iouReports;
let iouReports = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_IOUS,
waitForCollectionCallback: true,
callback: val => iouReports = val,
callback: (val) => {
if (!val) {
return;
}
iouReports = val;
},
});

let currentUserLogin;
Expand All @@ -86,85 +91,85 @@ 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) => {
// There are a few properties that need to be calculated for the report which are used when sorting reports.
_.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
report.displayName = ReportUtils.getReportName(report, policies);

// -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
// eslint-disable-next-line no-param-reassign
report.iouReportAmount = ReportUtils.getIOUTotal(report, iouReports);
});

// 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
let pinnedReports = [];
let outstandingIOUReports = [];
let draftReports = [];
let nonArchivedReports = [];
let archivedReports = [];

_.each(reportsToDisplay, (report) => {
if (report.isPinned) {
pinnedReports.push(report);
return;
}

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) {
outstandingIOUReports.push(report);
return;
}

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

// 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)) {
archivedReports.push(report);
return;
}
}

// 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);
nonArchivedReports.push(report);
});

// 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);
// Sort each group of reports accordingly
pinnedReports = _.sortBy(pinnedReports, report => report.displayName.toLowerCase());
outstandingIOUReports = _.sortBy(outstandingIOUReports, 'iouReportAmount').reverse();
draftReports = _.sortBy(draftReports, report => report.displayName.toLowerCase());
nonArchivedReports = _.sortBy(nonArchivedReports, report => (isInDefaultMode ? report.lastMessageTimestamp : report.displayName.toLowerCase()));
archivedReports = _.sortBy(archivedReports, report => (isInDefaultMode ? report.lastMessageTimestamp : report.displayName.toLowerCase()));

// 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);
// For archived and non-archived reports, ensure that most recent reports are at the top by reversing the order of the arrays because underscore will only sort them in ascending order
if (isInDefaultMode) {
nonArchivedReports.reverse();
archivedReports.reverse();
}

return _.pluck(recentReportOptions, 'reportID');
// 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 _.pluck([]
.concat(pinnedReports)
.concat(outstandingIOUReports)
.concat(draftReports)
.concat(nonArchivedReports)
.concat(archivedReports), 'reportID');
}

/**
Expand Down Expand Up @@ -280,13 +285,8 @@ function getOptionData(reportID) {
result.alternateText = lastMessageText || Str.removeSMSDomain(personalDetail.login);
}

if (result.hasOutstandingIOU) {
const iouReport = (iouReports && iouReports[`${ONYXKEYS.COLLECTION.REPORT_IOUS}${report.iouReportID}`]) || null;
if (iouReport) {
result.isIOUReportOwner = iouReport.ownerEmail === currentUserLogin;
result.iouReportAmount = iouReport.total;
}
}
result.isIOUReportOwner = ReportUtils.isIOUOwnedByCurrentUser(result, currentUserLogin, iouReports);
result.iouReportAmount = ReportUtils.getIOUTotal(result, iouReports);

if (!hasMultipleParticipants) {
result.login = personalDetail.login;
Expand Down
0 tests/unit/LHNFilterTest.js → tests/unit/SidebarFilterTest.js
View file
File renamed without changes.
0 tests/unit/LHNOrderTest.js → tests/unit/SidebarOrderTest.js
View file
File renamed without changes.

0 comments on commit 1ca61d8

Please sign in to comment.