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

LHN order improvements #12978

Merged
merged 12 commits into from
Dec 7, 2022
21 changes: 17 additions & 4 deletions src/libs/SidebarUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,27 @@ function getOrderedReportIDs(reportIDFromRoute) {

// Sort each group of reports accordingly
pinnedReports = _.sortBy(pinnedReports, report => report.displayName.toLowerCase());
outstandingIOUReports = _.sortBy(outstandingIOUReports, 'iouReportAmount').reverse();
outstandingIOUReports = outstandingIOUReports.sort((a, b) => {
if (a.iouReportAmount > b.iouReportAmount) {
return -1;
}
if (a.iouReportAmount < b.iouReportAmount) {
return 1;
}
return a.displayName.toLowerCase() > b.displayName.toLowerCase() ? 1 : -1;
chiragsalian marked this conversation as resolved.
Show resolved Hide resolved
});
draftReports = _.sortBy(draftReports, report => report.displayName.toLowerCase());
nonArchivedReports = _.sortBy(nonArchivedReports, report => (isInDefaultMode ? report.lastActionCreated : report.displayName.toLowerCase()));
nonArchivedReports = nonArchivedReports.sort((a, b) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i noticed that nonArchivedReports also showed up very differently between web and mobile. I made some improvements so that it sorts more consistently between mobile and web.

But its not perfect. There is a chance both messageTimestamp and displayNames are the same. For example i noticed for some #admin rooms the order would be off. We could further sort on policyID but i didn't want to go into a rabbit hole if it's not currently a problem and I'm content with the current improvement.

if (isInDefaultMode && (a.lastActionCreated || b.lastActionCreated) && a.lastActionCreated !== b.lastActionCreated) {
return a.lastActionCreated > b.lastActionCreated ? -1 : 1;
}

return a.displayName.toLowerCase() > b.displayName.toLowerCase() ? 1 : -1;
});
archivedReports = _.sortBy(archivedReports, report => (isInDefaultMode ? report.lastActionCreated : report.displayName.toLowerCase()));

// 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
// For 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();
}

Expand Down
117 changes: 117 additions & 0 deletions tests/unit/SidebarOrderTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -616,5 +616,122 @@ describe('Sidebar', () => {
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Report (archived)');
});
});

it('orders IOU reports by displayName if amounts are the same', () => {
// Given three IOU reports containing the same IOU amounts
const report1 = {
...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@test.com']),
hasOutstandingIOU: true,

// This has to be added after the IOU report is generated
iouReportID: null,
};
const report2 = {
...LHNTestUtils.getFakeReport(['email3@test.com', 'email4@test.com']),
hasOutstandingIOU: true,

// This has to be added after the IOU report is generated
iouReportID: null,
};
const report3 = {
...LHNTestUtils.getFakeReport(['email5@test.com', 'email6@test.com']),
hasOutstandingIOU: true,

// This has to be added after the IOU report is generated
iouReportID: null,
};
const iouReport1 = {
...LHNTestUtils.getFakeReport(['email7@test.com', 'email8@test.com']),
ownerEmail: 'email2@test.com',
hasOutstandingIOU: true,
total: 10000,
currency: 'USD',
chatReportID: report3.reportID,
};
const iouReport2 = {
...LHNTestUtils.getFakeReport(['email9@test.com', 'email10@test.com']),
ownerEmail: 'email2@test.com',
hasOutstandingIOU: true,
total: 10000,
currency: 'USD',
chatReportID: report3.reportID,
};
const iouReport3 = {
...LHNTestUtils.getFakeReport(['email11@test.com', 'email12@test.com']),
ownerEmail: 'email2@test.com',
hasOutstandingIOU: true,
total: 10000,
currency: 'USD',
chatReportID: report3.reportID,
};

report1.iouReportID = iouReport1.reportID;
report2.iouReportID = iouReport2.reportID;
report3.iouReportID = iouReport3.reportID;

const currentlyLoggedInUserEmail = 'email13@test.com';
const sidebarLinks = LHNTestUtils.getDefaultRenderedSidebarLinks('0');
return waitForPromisesToResolve()

// When Onyx is updated with the data and the sidebar re-renders
.then(() => Onyx.multiSet({
[ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT,
[ONYXKEYS.PERSONAL_DETAILS]: LHNTestUtils.fakePersonalDetails,
[ONYXKEYS.SESSION]: {email: currentlyLoggedInUserEmail},
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport1.reportID}`]: iouReport1,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport2.reportID}`]: iouReport2,
[`${ONYXKEYS.COLLECTION.REPORT}${iouReport3.reportID}`]: iouReport3,
}))

// Then the reports are ordered alphabetically since their amounts are the same
.then(() => {
const displayNames = sidebarLinks.queryAllByA11yLabel('Chat user display names');
expect(displayNames).toHaveLength(3);
expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('One, Two');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four');
});
});

it('orders nonArchived reports by displayName if created timestamps are the same', () => {
// Given three nonArchived reports created at the same time
const lastActionCreated = DateUtils.getDBTime();
const report1 = {
...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@test.com']),
lastActionCreated,
};
const report2 = {
...LHNTestUtils.getFakeReport(['email3@test.com', 'email4@test.com']),
lastActionCreated,
};
const report3 = {
...LHNTestUtils.getFakeReport(['email5@test.com', 'email6@test.com']),
lastActionCreated,
};

const sidebarLinks = LHNTestUtils.getDefaultRenderedSidebarLinks('0');
return waitForPromisesToResolve()

// When Onyx is updated with the data and the sidebar re-renders
.then(() => Onyx.multiSet({
[ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT,
[ONYXKEYS.PERSONAL_DETAILS]: LHNTestUtils.fakePersonalDetails,
[`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1,
[`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2,
[`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3,
}))

// Then the reports are ordered alphabetically since their lastActionCreated are the same
.then(() => {
const displayNames = sidebarLinks.queryAllByA11yLabel('Chat user display names');
expect(displayNames).toHaveLength(3);
expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Five, Six');
expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('One, Two');
expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four');
});
});
});
});