From 6da86d579f67f1f5338befe07bf45dcee5e190b3 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 11 Aug 2023 11:15:09 -0700 Subject: [PATCH 1/5] Ignore hasOutstandingIOU from child iouReport --- src/libs/OptionsListUtils.js | 2 +- src/libs/ReportUtils.js | 21 +++++---------------- src/libs/SidebarUtils.js | 4 ++-- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index b574bfd3d00e..33f693ef1de3 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -638,7 +638,7 @@ function getOptions( const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); // Filter out all the reports that shouldn't be displayed - const filteredReports = _.filter(reports, (report) => ReportUtils.shouldReportBeInOptionList(report, Navigation.getTopmostReportId(), false, null, betas, policies)); + const filteredReports = _.filter(reports, (report) => ReportUtils.shouldReportBeInOptionList(report, Navigation.getTopmostReportId(), false, betas, policies)); // Sorting the reports works like this: // - Order everything by the last message timestamp (descending) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index e8ef18a3ca27..6ffcc44c19ee 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1134,12 +1134,10 @@ function getMoneyRequestAction(reportAction = {}) { * Determines if a report has an IOU that is waiting for an action from the current user (either Pay or Add a credit bank account) * * @param {Object} report (chatReport or iouReport) - * @param {Object} allReportsDict * @returns {boolean} */ -function isWaitingForIOUActionFromCurrentUser(report, allReportsDict = null) { - const allAvailableReports = allReportsDict || allReports; - if (!report || !allAvailableReports) { +function isWaitingForIOUActionFromCurrentUser(report) { + if (!report) { return false; } @@ -1148,15 +1146,7 @@ function isWaitingForIOUActionFromCurrentUser(report, allReportsDict = null) { return true; } - let reportToLook = report; - if (report.iouReportID) { - const iouReport = allAvailableReports[`${ONYXKEYS.COLLECTION.REPORT}${report.iouReportID}`]; - if (iouReport) { - reportToLook = iouReport; - } - } - // Money request waiting for current user to Pay (from chat or from iou report) - if (reportToLook.ownerAccountID && (reportToLook.ownerAccountID !== currentUserAccountID || currentUserAccountID === reportToLook.managerID) && reportToLook.hasOutstandingIOU) { + if (report.hasOutstandingIOU && report.ownerAccountID && (report.ownerAccountID !== currentUserAccountID || currentUserAccountID === report.managerID)) { return true; } @@ -2337,13 +2327,12 @@ function canAccessReport(report, policies, betas, allReportActions) { * @param {Object} report * @param {String} currentReportId * @param {Boolean} isInGSDMode - * @param {Object} iouReports * @param {String[]} betas * @param {Object} policies * @param {Object} allReportActions * @returns {boolean} */ -function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouReports, betas, policies, allReportActions) { +function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, policies, allReportActions) { const isInDefaultMode = !isInGSDMode; // Exclude reports that have no data because there wouldn't be anything to show in the option item. @@ -2369,7 +2358,7 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, iouRep } // Include reports that are relevant to the user in any view mode. Criteria include having a draft, having an outstanding IOU, or being assigned to an open task. - if (report.hasDraft || isWaitingForIOUActionFromCurrentUser(report, iouReports) || isWaitingForTaskCompleteFromAssignee(report)) { + if (report.hasDraft || isWaitingForIOUActionFromCurrentUser(report) || isWaitingForTaskCompleteFromAssignee(report)) { return true; } diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 561d91a63a1f..d97ad15f86c7 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -87,7 +87,7 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p // Filter out all the reports that shouldn't be displayed const reportsToDisplay = _.filter(allReportsDict, (report) => - ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, allReportsDict, betas, policies, allReportActions), + ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, policies, allReportActions), ); if (_.isEmpty(reportsToDisplay)) { @@ -131,7 +131,7 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p return; } - if (ReportUtils.isWaitingForIOUActionFromCurrentUser(report, allReportsDict)) { + if (ReportUtils.isWaitingForIOUActionFromCurrentUser(report)) { outstandingIOUReports.push(report); return; } From c2abbe3fd3668fe4e895a0c5a2ddf8f3628962bc Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 11 Aug 2023 12:06:22 -0700 Subject: [PATCH 2/5] Fix isWaitingForIOUActionFromCurrentUser tests --- src/libs/SidebarUtils.js | 4 +- tests/unit/ReportUtilsTest.js | 82 ++++++++--------------------------- 2 files changed, 20 insertions(+), 66 deletions(-) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index d97ad15f86c7..5a7d7f02a216 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -86,9 +86,7 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p const isInDefaultMode = !isInGSDMode; // Filter out all the reports that shouldn't be displayed - const reportsToDisplay = _.filter(allReportsDict, (report) => - ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, policies, allReportActions), - ); + const reportsToDisplay = _.filter(allReportsDict, (report) => ReportUtils.shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, policies, allReportActions)); if (_.isEmpty(reportsToDisplay)) { // Display Concierge chat report when there is no report to be displayed diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index e0b50ca54cca..c72162f82dbb 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -288,44 +288,15 @@ describe('ReportUtils', () => { it('returns false when there is no report', () => { expect(ReportUtils.isWaitingForIOUActionFromCurrentUser()).toBe(false); }); - it('returns false when there is no reports collection', () => { + it('returns false when the matched IOU report does not have an owner accountID', () => { const report = { ...LHNTestUtils.getFakeReport(), - iouReportID: '1', + ownerAccountID: undefined, + hasOutstandingIOU: true, }; expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); }); - it('returns false when the report has no iouReportID', () => { - const report = LHNTestUtils.getFakeReport(); - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}2`, { - reportID: '2', - }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); - }); - }); - it('returns false when there is no matching IOU report', () => { - const report = { - ...LHNTestUtils.getFakeReport(), - iouReportID: '1', - }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}2`, { - reportID: '2', - }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); - }); - }); - it('returns false when the matched IOU report does not have an owner email', () => { - const report = { - ...LHNTestUtils.getFakeReport(), - iouReportID: '1', - }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, { - reportID: '1', - }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); - }); - }); - it('returns false when the matched IOU report does not have an owner email', () => { + it('returns false when the linked iou report has an oustanding IOU', () => { const report = { ...LHNTestUtils.getFakeReport(), iouReportID: '1', @@ -333,52 +304,37 @@ describe('ReportUtils', () => { Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, { reportID: '1', ownerAccountID: 99, + hasOutstandingIOU: true, }).then(() => { expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); }); }); - it('returns true when the report has an oustanding IOU', () => { + it('returns true when the report has no oustanding IOU but is waiting for a bank account and the logged user is the report owner', () => { const report = { ...LHNTestUtils.getFakeReport(), - iouReportID: '1', - hasOutstandingIOU: true, + hasOutstandingIOU: false, + ownerAccountID: currentUserAccountID, + isWaitingOnBankAccount: true, }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, { - reportID: '1', - ownerAccountID: 99, - hasOutstandingIOU: true, - }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true); - }); + expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true); }); - it('returns false when the report has no oustanding IOU', () => { + it('returns true when the report has no oustanding IOU but is waiting for a bank account and the logged user is not the report owner', () => { const report = { ...LHNTestUtils.getFakeReport(), - iouReportID: '1', hasOutstandingIOU: false, + ownerAccountID: 97, + isWaitingOnBankAccount: true, }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, { - reportID: '1', - ownerAccountID: 99, - hasOutstandingIOU: false, - }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); - }); + expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); }); - it('returns true when the report has no oustanding IOU but is waiting for a bank account', () => { + it('returns true when the report has oustanding IOU', () => { const report = { ...LHNTestUtils.getFakeReport(), - iouReportID: '1', - hasOutstandingIOU: false, + ownerAccountID: 99, + hasOutstandingIOU: true, + isWaitingOnBankAccount: false, }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}1`, { - reportID: '1', - ownerAccountID: currentUserEmail, - hasOutstandingIOU: false, - isWaitingOnBankAccount: true, - }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); - }); + expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true); }); }); From 5a0a2699f0a510c02de2decb596cdae619c07a68 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 11 Aug 2023 12:34:59 -0700 Subject: [PATCH 3/5] Fix order tests --- tests/unit/SidebarOrderTest.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index ffc619e7d578..51ff8fafdfa2 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -357,7 +357,7 @@ describe('Sidebar', () => { }; const report3 = { ...LHNTestUtils.getFakeReport([5, 6], 1), - hasOutstandingIOU: true, + hasOutstandingIOU: false, // This has to be added after the IOU report is generated iouReportID: null, @@ -403,8 +403,8 @@ describe('Sidebar', () => { expect(screen.queryAllByTestId('Pencil Icon')).toHaveLength(1); expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('One, Two'); expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Two owes $100.00'); - expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Five, Six'); - expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Three, Four'); + expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four'); + expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Five, Six'); }) ); }); From ea944e7db3f2efb2b5b7b68d86f40f0a8895334d Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Mon, 14 Aug 2023 10:49:48 -0700 Subject: [PATCH 4/5] Add comment --- src/libs/ReportUtils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index ac5ab7732e4e..f1b6b78830e9 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1148,6 +1148,7 @@ function isWaitingForIOUActionFromCurrentUser(report) { return true; } + // Money request waiting for current user to Pay (from expense or iou report) if (report.hasOutstandingIOU && report.ownerAccountID && (report.ownerAccountID !== currentUserAccountID || currentUserAccountID === report.managerID)) { return true; } From 43f38fb81d366fe083ce54b1000176112331f18e Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Mon, 14 Aug 2023 12:57:33 -0700 Subject: [PATCH 5/5] Fix tests --- tests/unit/SidebarOrderTest.js | 61 ++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index 72b4a7e4818a..ef56fa8783b8 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -427,13 +427,12 @@ describe('Sidebar', () => { .then(() => { const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames'); const displayNames = screen.queryAllByLabelText(hintText); - expect(displayNames).toHaveLength(4); + expect(displayNames).toHaveLength(3); expect(screen.queryAllByTestId('Pin Icon')).toHaveLength(1); expect(screen.queryAllByTestId('Pencil Icon')).toHaveLength(1); expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('One, Two'); expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Two owes $100.00'); expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four'); - expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Five, Six'); }) ); }); @@ -700,21 +699,31 @@ describe('Sidebar', () => { // Given three IOU reports containing the same IOU amounts const report1 = { ...LHNTestUtils.getFakeReport([1, 2]), - hasOutstandingIOU: true, // This has to be added after the IOU report is generated iouReportID: null, }; const report2 = { ...LHNTestUtils.getFakeReport([3, 4]), - hasOutstandingIOU: true, // This has to be added after the IOU report is generated iouReportID: null, }; const report3 = { ...LHNTestUtils.getFakeReport([5, 6]), - hasOutstandingIOU: true, + hasOutstandingIOU: false, + + // This has to be added after the IOU report is generated + iouReportID: null, + }; + const report4 = { + ...LHNTestUtils.getFakeReport([5, 6]), + + // This has to be added after the IOU report is generated + iouReportID: null, + }; + const report5 = { + ...LHNTestUtils.getFakeReport([5, 6]), // This has to be added after the IOU report is generated iouReportID: null, @@ -733,7 +742,7 @@ describe('Sidebar', () => { ...LHNTestUtils.getFakeReport([9, 10]), type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, - managerID: 2, + managerID: 3, hasOutstandingIOU: true, total: 10000, currency: 'USD', @@ -743,7 +752,27 @@ describe('Sidebar', () => { ...LHNTestUtils.getFakeReport([11, 12]), type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, - managerID: 2, + managerID: 4, + hasOutstandingIOU: true, + total: 100000, + currency: 'USD', + chatReportID: report3.reportID, + }; + const iouReport4 = { + ...LHNTestUtils.getFakeReport([11, 12]), + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: 2, + managerID: 5, + hasOutstandingIOU: true, + total: 10000, + currency: 'USD', + chatReportID: report3.reportID, + }; + const iouReport5 = { + ...LHNTestUtils.getFakeReport([11, 12]), + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: 2, + managerID: 6, hasOutstandingIOU: true, total: 10000, currency: 'USD', @@ -753,6 +782,8 @@ describe('Sidebar', () => { report1.iouReportID = iouReport1.reportID; report2.iouReportID = iouReport2.reportID; report3.iouReportID = iouReport3.reportID; + report4.iouReportID = iouReport4.reportID; + report5.iouReportID = iouReport5.reportID; const currentlyLoggedInUserAccountID = 13; LHNTestUtils.getDefaultRenderedSidebarLinks('0'); @@ -768,22 +799,26 @@ describe('Sidebar', () => { [`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1, [`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2, [`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3, + [`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`]: report4, + [`${ONYXKEYS.COLLECTION.REPORT}${report5.reportID}`]: report5, [`${ONYXKEYS.COLLECTION.REPORT}${iouReport1.reportID}`]: iouReport1, [`${ONYXKEYS.COLLECTION.REPORT}${iouReport2.reportID}`]: iouReport2, [`${ONYXKEYS.COLLECTION.REPORT}${iouReport3.reportID}`]: iouReport3, + [`${ONYXKEYS.COLLECTION.REPORT}${iouReport4.reportID}`]: iouReport4, + [`${ONYXKEYS.COLLECTION.REPORT}${iouReport5.reportID}`]: iouReport5, }), ) - // Then the reports are ordered alphabetically since their amounts are the same + // Then the reports with the same amount are ordered alphabetically .then(() => { const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames'); const displayNames = screen.queryAllByLabelText(hintText); expect(displayNames).toHaveLength(5); - expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Email Two owes $100.00'); - expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Two owes $100.00'); - expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Email Two owes $100.00'); - expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Five, Six'); - expect(lodashGet(displayNames, [4, 'props', 'children'])).toBe('One, Two'); + expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Email Four owes $1,000.00'); + expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Five owes $100.00'); + expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Email Six owes $100.00'); + expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Email Three owes $100.00'); + expect(lodashGet(displayNames, [4, 'props', 'children'])).toBe('Email Two owes $100.00'); }) ); });