From aaaa61ddbc5477ebdfccdd291e8af9580b7e7c3f Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Wed, 23 Nov 2022 16:30:25 -0800 Subject: [PATCH 01/11] outstandingIOUReports order improvement --- src/libs/SidebarUtils.js | 10 +++++++++- src/pages/home/sidebar/SidebarLinks.js | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 0a0f35531902..2d45b91ed2ed 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -151,7 +151,15 @@ 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().localeCompare(b.displayName.toLowerCase()); + }); 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())); diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 85c6de2fb2dd..4c37274bc611 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -125,6 +125,7 @@ class SidebarLinks extends React.Component { return null; } const optionListItems = SidebarUtils.getOrderedReportIDs(this.props.reportIDFromRoute); + console.log('over here 1' + optionListItems); return ( Date: Wed, 23 Nov 2022 16:33:09 -0800 Subject: [PATCH 02/11] nonArchivedReports improvments --- src/libs/SidebarUtils.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 2d45b91ed2ed..cfe0ba12a5c3 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -161,7 +161,13 @@ function getOrderedReportIDs(reportIDFromRoute) { return a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()); }); draftReports = _.sortBy(draftReports, report => report.displayName.toLowerCase()); - nonArchivedReports = _.sortBy(nonArchivedReports, report => (isInDefaultMode ? report.lastMessageTimestamp : report.displayName.toLowerCase())); + nonArchivedReports = nonArchivedReports.sort((a, b) => { + if (isInDefaultMode && (a.lastMessageTimestamp || b.lastMessageTimestamp) && a.lastMessageTimestamp !== b.lastMessageTimestamp) { + return a.lastMessageTimestamp > b.lastMessageTimestamp ? -1 : 1; + } + + return a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()); + }); archivedReports = _.sortBy(archivedReports, report => (isInDefaultMode ? report.lastMessageTimestamp : 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 From 235f534c35ce8ad47229637c85f8f594cfe6db45 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Wed, 23 Nov 2022 16:33:30 -0800 Subject: [PATCH 03/11] removing a test debug --- src/pages/home/sidebar/SidebarLinks.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index 4c37274bc611..85c6de2fb2dd 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -125,7 +125,6 @@ class SidebarLinks extends React.Component { return null; } const optionListItems = SidebarUtils.getOrderedReportIDs(this.props.reportIDFromRoute); - console.log('over here 1' + optionListItems); return ( Date: Wed, 23 Nov 2022 16:56:37 -0800 Subject: [PATCH 04/11] logic fix --- src/libs/SidebarUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index cfe0ba12a5c3..4842f47f119b 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -163,7 +163,7 @@ function getOrderedReportIDs(reportIDFromRoute) { draftReports = _.sortBy(draftReports, report => report.displayName.toLowerCase()); nonArchivedReports = nonArchivedReports.sort((a, b) => { if (isInDefaultMode && (a.lastMessageTimestamp || b.lastMessageTimestamp) && a.lastMessageTimestamp !== b.lastMessageTimestamp) { - return a.lastMessageTimestamp > b.lastMessageTimestamp ? -1 : 1; + return a.lastMessageTimestamp > b.lastMessageTimestamp ? 1 : -1; } return a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()); From e34f2d95878f9959dde6bdc04d42ba4429106f08 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Wed, 23 Nov 2022 17:44:37 -0800 Subject: [PATCH 05/11] logic fixes --- src/libs/SidebarUtils.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 4842f47f119b..e9965f9eb1dc 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -158,21 +158,20 @@ function getOrderedReportIDs(reportIDFromRoute) { if (a.iouReportAmount < b.iouReportAmount) { return 1; } - return a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()); + return a.displayName.toLowerCase() > b.displayName.toLowerCase() ? 1 : -1; }); draftReports = _.sortBy(draftReports, report => report.displayName.toLowerCase()); nonArchivedReports = nonArchivedReports.sort((a, b) => { if (isInDefaultMode && (a.lastMessageTimestamp || b.lastMessageTimestamp) && a.lastMessageTimestamp !== b.lastMessageTimestamp) { - return a.lastMessageTimestamp > b.lastMessageTimestamp ? 1 : -1; + return a.lastMessageTimestamp > b.lastMessageTimestamp ? -1 : 1; } - return a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()); + return a.displayName.toLowerCase() > b.displayName.toLowerCase() ? 1 : -1; }); archivedReports = _.sortBy(archivedReports, report => (isInDefaultMode ? report.lastMessageTimestamp : 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 if (isInDefaultMode) { - nonArchivedReports.reverse(); archivedReports.reverse(); } From 4648bf9144d50924c74e0cfd1072e9d15facca60 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Sat, 3 Dec 2022 12:40:28 -0800 Subject: [PATCH 06/11] adding a test --- tests/unit/SidebarOrderTest.js | 79 ++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index 56d6398c0423..700415fd5ed1 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -616,5 +616,84 @@ 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'); + }); + }); }); }); From 6b5bd58c771f38e74029b4e8e57ddfa05f96a5b9 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Sat, 3 Dec 2022 12:40:40 -0800 Subject: [PATCH 07/11] one more test --- tests/unit/SidebarOrderTest.js | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index 700415fd5ed1..6beee8c3c2c2 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -695,5 +695,43 @@ describe('Sidebar', () => { expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four'); }); }); + + it('orders nonArchived reports by displayName if created timestamps are the same', () => { + // Given three IOU reports containing the same IOU amounts + const lastActionCreated = DateUtils.getDBTime(); + const report1 = { + ...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@test.com']), + lastActionCreated, + }; + const report2 = { + ...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@test.com']), + lastActionCreated, + }; + const report3 = { + ...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@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 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'); + }); + }); }); }); From d01f834f9354395fa7f2e2f1852ce9648da93bbf Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Sat, 3 Dec 2022 12:40:59 -0800 Subject: [PATCH 08/11] test correction --- tests/unit/SidebarOrderTest.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index 6beee8c3c2c2..d7d1afbb999d 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -704,11 +704,11 @@ describe('Sidebar', () => { lastActionCreated, }; const report2 = { - ...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@test.com']), + ...LHNTestUtils.getFakeReport(['email3@test.com', 'email4@test.com']), lastActionCreated, }; const report3 = { - ...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@test.com']), + ...LHNTestUtils.getFakeReport(['email5@test.com', 'email6@test.com']), lastActionCreated, }; From a12be51764391aafa4de33368a34c299389fc6c4 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Sat, 3 Dec 2022 12:42:05 -0800 Subject: [PATCH 09/11] cleanup --- tests/unit/SidebarOrderTest.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index d7d1afbb999d..5414a9afdb5e 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -697,7 +697,7 @@ describe('Sidebar', () => { }); it('orders nonArchived reports by displayName if created timestamps are the same', () => { - // Given three IOU reports containing the same IOU amounts + // Given three nonArchived reports created at the same time const lastActionCreated = DateUtils.getDBTime(); const report1 = { ...LHNTestUtils.getFakeReport(['email1@test.com', 'email2@test.com']), @@ -724,7 +724,7 @@ describe('Sidebar', () => { [`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3, })) - // Then the reports are ordered alphabetically since their amounts are the same + // 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); From 4ec7052cee3c87a47e2fc803333b0414647cc076 Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Sat, 3 Dec 2022 12:52:03 -0800 Subject: [PATCH 10/11] comment cleanup --- src/libs/SidebarUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 0527e187411a..26dbb9e2c063 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -167,7 +167,7 @@ function getOrderedReportIDs(reportIDFromRoute) { }); 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) { archivedReports.reverse(); } From ae0a7b6a02c8743b98e4991c036cc4d169b16a8f Mon Sep 17 00:00:00 2001 From: chiragsalian Date: Tue, 6 Dec 2022 16:54:02 -0800 Subject: [PATCH 11/11] using lodashOrderBy to clean up the logic here --- src/libs/SidebarUtils.js | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 26dbb9e2c063..cd419b3b1a9a 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -1,5 +1,6 @@ import Onyx from 'react-native-onyx'; import _ from 'underscore'; +import lodashOrderBy from 'lodash/orderBy'; import Str from 'expensify-common/lib/str'; import ONYXKEYS from '../ONYXKEYS'; import * as ReportUtils from './ReportUtils'; @@ -148,23 +149,11 @@ function getOrderedReportIDs(reportIDFromRoute) { // Sort each group of reports accordingly pinnedReports = _.sortBy(pinnedReports, report => report.displayName.toLowerCase()); - 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; - }); + outstandingIOUReports = lodashOrderBy(outstandingIOUReports, ['iouReportAmount', report => report.displayName.toLowerCase()], ['desc', 'asc']); draftReports = _.sortBy(draftReports, report => report.displayName.toLowerCase()); - nonArchivedReports = nonArchivedReports.sort((a, b) => { - 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; - }); + nonArchivedReports = isInDefaultMode + ? lodashOrderBy(nonArchivedReports, ['lastActionCreated', report => report.displayName.toLowerCase()], ['desc', 'asc']) + : lodashOrderBy(nonArchivedReports, [report => report.displayName.toLowerCase()], ['asc']); archivedReports = _.sortBy(archivedReports, report => (isInDefaultMode ? report.lastActionCreated : report.displayName.toLowerCase())); // 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