From cb3f3d0bb0011e7531c90eee29dd4c0e5d05bc49 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Tue, 17 Oct 2023 16:17:44 +0200 Subject: [PATCH 01/23] add hasOutstandingChildRequest verification --- src/libs/ReportUtils.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 011907c2c88b..c97de623ad3a 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1253,6 +1253,11 @@ function isWaitingForIOUActionFromCurrentUser(report) { return true; } + // Child report that is awaiting for current user to Pay + if (report.hasOutstandingChildRequest && report.ownerAccountID === currentUserAccountID) { + return true; + } + return false; } From ddba0ece2d3b77764cc3af4b3e86635ebbe85876 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Wed, 18 Oct 2023 19:07:30 +0200 Subject: [PATCH 02/23] adjusted and removed unused parts --- src/components/LHNOptionsList/OptionRowLHN.js | 3 +- src/libs/ReportUtils.js | 31 +++---------------- src/libs/SidebarUtils.js | 2 +- tests/unit/ReportUtilsTest.js | 20 ++++++------ 4 files changed, 17 insertions(+), 39 deletions(-) diff --git a/src/components/LHNOptionsList/OptionRowLHN.js b/src/components/LHNOptionsList/OptionRowLHN.js index ba035c8b3baf..064c1c146163 100644 --- a/src/components/LHNOptionsList/OptionRowLHN.js +++ b/src/components/LHNOptionsList/OptionRowLHN.js @@ -116,8 +116,7 @@ function OptionRowLHN(props) { const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR; const defaultSubscriptSize = optionItem.isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT; - const shouldShowGreenDotIndicator = - !hasBrickError && (optionItem.isUnreadWithMention || optionItem.isWaitingForTaskCompleteFromAssignee || ReportUtils.isWaitingForIOUActionFromCurrentUser(optionItem)); + const shouldShowGreenDotIndicator = !hasBrickError && (optionItem.isUnreadWithMention || optionItem.isWaitingForTaskCompleteFromAssignee || ReportUtils.shouldShowGBR(optionItem)); /** * Show the ReportActionContextMenu modal popover. diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index c97de623ad3a..c8339810cb1f 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1213,12 +1213,12 @@ function getDisplayNamesWithTooltips(personalDetailsList, isMultipleParticipantR } /** - * 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) + * Determines if a report child has an outstanding request that is waiting for an action from the current user (either Pay or Add a credit bank account) * * @param {Object} report (chatReport or iouReport) * @returns {boolean} */ -function isWaitingForIOUActionFromCurrentUser(report) { +function shouldShowGBR(report) { if (!report) { return false; } @@ -1227,34 +1227,13 @@ function isWaitingForIOUActionFromCurrentUser(report) { return false; } - const policy = getPolicy(report.policyID); - if (policy.type === CONST.POLICY.TYPE.CORPORATE) { - // If the report is already settled, there's no action required from any user. - if (isSettled(report.reportID)) { - return false; - } - - // Report is pending approval and the current user is the manager - if (isReportManager(report) && !isReportApproved(report)) { - return true; - } - - // Current user is an admin and the report has been approved but not settled yet - return policy.role === CONST.POLICY.ROLE.ADMIN && isReportApproved(report); - } - // Money request waiting for current user to add their credit bank account if (report.hasOutstandingIOU && report.ownerAccountID === currentUserAccountID && report.isWaitingOnBankAccount) { 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; - } - // Child report that is awaiting for current user to Pay - if (report.hasOutstandingChildRequest && report.ownerAccountID === currentUserAccountID) { + if (report.hasOutstandingChildRequest) { return true; } @@ -3141,7 +3120,7 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, } // 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) || isWaitingForTaskCompleteFromAssignee(report)) { + if (report.hasDraft || shouldShowGBR(report) || isWaitingForTaskCompleteFromAssignee(report)) { return true; } const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID); @@ -3953,7 +3932,7 @@ export { isCurrentUserTheOnlyParticipant, hasAutomatedExpensifyAccountIDs, hasExpensifyGuidesEmails, - isWaitingForIOUActionFromCurrentUser, + shouldShowGBR, isIOUOwnedByCurrentUser, getMoneyRequestReimbursableTotal, getMoneyRequestSpendBreakdown, diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index dd6db33902fb..bc915c4b16fa 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -179,7 +179,7 @@ function getOrderedReportIDs(currentReportId, allReportsDict, betas, policies, p reportsToDisplay.forEach((report) => { if (report.isPinned) { pinnedReports.push(report); - } else if (ReportUtils.isWaitingForIOUActionFromCurrentUser(report)) { + } else if (ReportUtils.shouldShowGBR(report)) { outstandingIOUReports.push(report); } else if (report.hasDraft) { draftReports.push(report); diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index c6afde7d9161..10a1389d6cf9 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -246,9 +246,9 @@ describe('ReportUtils', () => { }); }); - describe('isWaitingForIOUActionFromCurrentUser', () => { + describe('shouldShowGBR', () => { it('returns false when there is no report', () => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser()).toBe(false); + expect(ReportUtils.shouldShowGBR()).toBe(false); }); it('returns false when the matched IOU report does not have an owner accountID', () => { const report = { @@ -256,7 +256,7 @@ describe('ReportUtils', () => { ownerAccountID: undefined, hasOutstandingIOU: true, }; - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); + expect(ReportUtils.shouldShowGBR(report)).toBe(false); }); it('returns false when the linked iou report has an oustanding IOU', () => { const report = { @@ -268,7 +268,7 @@ describe('ReportUtils', () => { ownerAccountID: 99, hasOutstandingIOU: true, }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); + expect(ReportUtils.shouldShowGBR(report)).toBe(false); }); }); it('returns false when the report has no oustanding IOU but is waiting for a bank account and the logged user is the report owner', () => { @@ -278,7 +278,7 @@ describe('ReportUtils', () => { ownerAccountID: currentUserAccountID, isWaitingOnBankAccount: true, }; - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); + expect(ReportUtils.shouldShowGBR(report)).toBe(false); }); it('returns true when the report has oustanding IOU and is waiting for a bank account and the logged user is the report owner', () => { const report = { @@ -287,7 +287,7 @@ describe('ReportUtils', () => { ownerAccountID: currentUserAccountID, isWaitingOnBankAccount: true, }; - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true); + expect(ReportUtils.shouldShowGBR(report)).toBe(true); }); it('returns false 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 = { @@ -296,16 +296,16 @@ describe('ReportUtils', () => { ownerAccountID: 97, isWaitingOnBankAccount: true, }; - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); + expect(ReportUtils.shouldShowGBR(report)).toBe(false); }); - it('returns true when the report has oustanding IOU', () => { + it('returns true when the report has oustanding child request', () => { const report = { ...LHNTestUtils.getFakeReport(), ownerAccountID: 99, - hasOutstandingIOU: true, + hasOutstandingChildRequest: true, isWaitingOnBankAccount: false, }; - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(true); + expect(ReportUtils.shouldShowGBR(report)).toBe(true); }); }); From 4fca0496676475b9c3725bd455364e850442a830 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Fri, 20 Oct 2023 14:13:05 +0200 Subject: [PATCH 03/23] clean up show GBR --- src/components/LHNOptionsList/OptionRowLHN.js | 2 +- src/libs/ReportUtils.js | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/LHNOptionsList/OptionRowLHN.js b/src/components/LHNOptionsList/OptionRowLHN.js index 064c1c146163..ae9a6da40c5d 100644 --- a/src/components/LHNOptionsList/OptionRowLHN.js +++ b/src/components/LHNOptionsList/OptionRowLHN.js @@ -116,7 +116,7 @@ function OptionRowLHN(props) { const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR; const defaultSubscriptSize = optionItem.isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT; - const shouldShowGreenDotIndicator = !hasBrickError && (optionItem.isUnreadWithMention || optionItem.isWaitingForTaskCompleteFromAssignee || ReportUtils.shouldShowGBR(optionItem)); + const shouldShowGreenDotIndicator = !hasBrickError && ReportUtils.shouldShowGBR(optionItem); /** * Show the ReportActionContextMenu modal popover. diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index c8339810cb1f..a6578a320f39 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1227,6 +1227,14 @@ function shouldShowGBR(report) { return false; } + if (report.isUnreadWithMention) { + return true; + } + + if (report.isWaitingForTaskCompleteFromAssignee) { + return true; + } + // Money request waiting for current user to add their credit bank account if (report.hasOutstandingIOU && report.ownerAccountID === currentUserAccountID && report.isWaitingOnBankAccount) { return true; From f6b79e6baf4a2f3d044c7c810c644177ada2ac5a Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Tue, 24 Oct 2023 02:39:35 +0200 Subject: [PATCH 04/23] tests --- src/pages/home/sidebar/SidebarLinksData.js | 1 + src/types/onyx/Report.ts | 3 +++ tests/unit/SidebarOrderTest.js | 28 +++++++++++----------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index 394f6c5ddc5a..2875b61256af 100644 --- a/src/pages/home/sidebar/SidebarLinksData.js +++ b/src/pages/home/sidebar/SidebarLinksData.js @@ -143,6 +143,7 @@ const chatReportSelector = (report) => total: report.total, nonReimbursableTotal: report.nonReimbursableTotal, hasOutstandingIOU: report.hasOutstandingIOU, + hasOutstandingChildRequest: report.hasOutstandingChildRequest, isWaitingOnBankAccount: report.isWaitingOnBankAccount, statusNum: report.statusNum, stateNum: report.stateNum, diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index 8587cf9b7cd5..c8b3b0752959 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -9,6 +9,9 @@ type Report = { /** Whether there is an outstanding amount in IOU */ hasOutstandingIOU?: boolean; + /** Whether child has an outstanding request */ + hasOutstandingChildRequest?: boolean; + /** List of icons for report participants */ icons?: OnyxCommon.Icon[]; diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index 4a693d679b86..64587466c942 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -385,7 +385,7 @@ describe('Sidebar', () => { }; const report3 = { ...LHNTestUtils.getFakeReport([5, 6], 1), - hasOutstandingIOU: false, + hasOutstandingChildRequest: false, // This has to be added after the IOU report is generated iouReportID: null, @@ -395,7 +395,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 2, - hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -404,7 +404,7 @@ describe('Sidebar', () => { const currentReportId = report2.reportID; const currentlyLoggedInUserAccountID = 9; LHNTestUtils.getDefaultRenderedSidebarLinks(currentReportId); - + console.log('iouReport :>> ', iouReport); return ( waitForBatchedUpdates() // When Onyx is updated with the data and the sidebar re-renders @@ -431,7 +431,7 @@ describe('Sidebar', () => { 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, [1, 'props', 'children'])).toBe('Email Two paid $100.00'); expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four'); }) ); @@ -733,7 +733,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 2, - hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -743,7 +743,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 3, - hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -753,7 +753,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 4, - hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 100000, currency: 'USD', chatReportID: report3.reportID, @@ -763,7 +763,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 5, - hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -773,7 +773,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 6, - hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -814,11 +814,11 @@ describe('Sidebar', () => { const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames'); const displayNames = screen.queryAllByLabelText(hintText); expect(displayNames).toHaveLength(5); - 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'); + expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Email Four paid $1,000.00'); + expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Five paid $100.00'); + expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Email Six paid $100.00'); + expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Email Three paid $100.00'); + expect(lodashGet(displayNames, [4, 'props', 'children'])).toBe('Email Two paid $100.00'); }) ); }); From f4cb0093f0981445a1bebe2709f00b84cd831ce7 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Tue, 24 Oct 2023 02:47:08 +0200 Subject: [PATCH 05/23] fix lint --- tests/unit/SidebarOrderTest.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index 64587466c942..a2b82dc1cb58 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -404,7 +404,6 @@ describe('Sidebar', () => { const currentReportId = report2.reportID; const currentlyLoggedInUserAccountID = 9; LHNTestUtils.getDefaultRenderedSidebarLinks(currentReportId); - console.log('iouReport :>> ', iouReport); return ( waitForBatchedUpdates() // When Onyx is updated with the data and the sidebar re-renders From 63f6bd88e0585ae9614ae5cb12a4f72a7bcfa66f Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Tue, 24 Oct 2023 13:24:18 +0200 Subject: [PATCH 06/23] tests fix --- tests/unit/ReportUtilsTest.js | 1 + tests/unit/SidebarOrderTest.js | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index 0c888b80b51b..81249214a4e8 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -302,6 +302,7 @@ describe('ReportUtils', () => { const report = { ...LHNTestUtils.getFakeReport(), ownerAccountID: 99, + hasOutstandingIOU: true, hasOutstandingChildRequest: true, isWaitingOnBankAccount: false, }; diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index a2b82dc1cb58..ea6fe49befc8 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -395,6 +395,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 2, + hasOutstandingIOU: true, hasOutstandingChildRequest: true, total: 10000, currency: 'USD', @@ -430,7 +431,7 @@ describe('Sidebar', () => { 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 paid $100.00'); + expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Two owes $100.00'); expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four'); }) ); @@ -732,6 +733,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 2, + hasOutstandingIOU: true, hasOutstandingChildRequest: true, total: 10000, currency: 'USD', @@ -742,6 +744,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 3, + hasOutstandingIOU: true, hasOutstandingChildRequest: true, total: 10000, currency: 'USD', @@ -752,6 +755,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 4, + hasOutstandingIOU: true, hasOutstandingChildRequest: true, total: 100000, currency: 'USD', @@ -762,6 +766,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 5, + hasOutstandingIOU: true, hasOutstandingChildRequest: true, total: 10000, currency: 'USD', @@ -772,6 +777,7 @@ describe('Sidebar', () => { type: CONST.REPORT.TYPE.IOU, ownerAccountID: 2, managerID: 6, + hasOutstandingIOU: true, hasOutstandingChildRequest: true, total: 10000, currency: 'USD', @@ -813,11 +819,11 @@ describe('Sidebar', () => { const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames'); const displayNames = screen.queryAllByLabelText(hintText); expect(displayNames).toHaveLength(5); - expect(lodashGet(displayNames, [0, 'props', 'children'])).toBe('Email Four paid $1,000.00'); - expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Five paid $100.00'); - expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Email Six paid $100.00'); - expect(lodashGet(displayNames, [3, 'props', 'children'])).toBe('Email Three paid $100.00'); - expect(lodashGet(displayNames, [4, 'props', 'children'])).toBe('Email Two paid $100.00'); + 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'); }) ); }); From 647efb8989ff39ed4a5b8da0d1fb82a5cf5c6286 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Wed, 25 Oct 2023 20:01:02 +0200 Subject: [PATCH 07/23] updated text --- src/libs/ReportUtils.js | 5 ++++- src/types/onyx/Report.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index d64490a6f5f8..1f585e7179bf 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1314,7 +1314,10 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) { } /** - * Determines if a report child has an outstanding request that is waiting for an action from the current user (either Pay or Add a credit bank account) + * Determines if a report should show a GBR (green dot) in the LHN. This can happen when the report: + - is unread and the user was mentioned in one of the unread comments + - is for an outstanding task waiting on the user + - has an outstanding child money request that is waiting for an action from the current user (e.g. pay, approve, add bank account) * * @param {Object} report (chatReport or iouReport) * @returns {boolean} diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index c8b3b0752959..9a23ab422719 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -9,7 +9,7 @@ type Report = { /** Whether there is an outstanding amount in IOU */ hasOutstandingIOU?: boolean; - /** Whether child has an outstanding request */ + /** Whether child has an outstanding money request that is awaiting action from the current user */ hasOutstandingChildRequest?: boolean; /** List of icons for report participants */ From de9e121fc599cc5dfdf39189d489eefe54d0fee9 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Fri, 27 Oct 2023 02:42:08 +0200 Subject: [PATCH 08/23] fix GBR display --- src/libs/SidebarUtils.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/SidebarUtils.js b/src/libs/SidebarUtils.js index 8948ce42a0c6..0ef7ccbc1355 100644 --- a/src/libs/SidebarUtils.js +++ b/src/libs/SidebarUtils.js @@ -257,6 +257,7 @@ function getOptionData(report, reportActions, personalDetails, preferredLocale, searchText: null, isPinned: false, hasOutstandingIOU: false, + hasOutstandingChildRequest: false, iouReportID: null, isIOUReportOwner: null, iouReportAmount: 0, @@ -301,6 +302,7 @@ function getOptionData(report, reportActions, personalDetails, preferredLocale, result.keyForList = String(report.reportID); result.tooltipText = ReportUtils.getReportParticipantsTitle(report.participantAccountIDs || []); result.hasOutstandingIOU = report.hasOutstandingIOU; + result.hasOutstandingChildRequest = report.hasOutstandingChildRequest; result.parentReportID = report.parentReportID || null; result.isWaitingOnBankAccount = report.isWaitingOnBankAccount; result.notificationPreference = report.notificationPreference || null; From bfb06ae3582ab18bd69132f46b8a53febecb216b Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Fri, 27 Oct 2023 15:47:57 +0200 Subject: [PATCH 09/23] changes --- src/libs/ReportUtils.js | 6 +----- src/types/onyx/Report.ts | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 5e47a798feed..7b0f35dbdd7b 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1335,17 +1335,13 @@ function shouldShowGBR(report) { return true; } - if (report.isWaitingForTaskCompleteFromAssignee) { - return true; - } - // Money request waiting for current user to add their credit bank account // hasOutstandingIOU will be false if the user paid, but isWaitingOnBankAccount will be true if user don't have a wallet or bank account setup if (!report.hasOutstandingIOU && report.isWaitingOnBankAccount && report.ownerAccountID === currentUserAccountID) { return true; } - // Child report that is awaiting for current user to Pay + // Has a child report that is awaiting action (e.g. approve, pay, add bank account) from current user if (report.hasOutstandingChildRequest) { return true; } diff --git a/src/types/onyx/Report.ts b/src/types/onyx/Report.ts index f882b401bb2e..839da2d6c548 100644 --- a/src/types/onyx/Report.ts +++ b/src/types/onyx/Report.ts @@ -9,7 +9,7 @@ type Report = { /** Whether there is an outstanding amount in IOU */ hasOutstandingIOU?: boolean; - /** Whether child has an outstanding money request that is awaiting action from the current user */ + /** Whether the report has a child that is an outstanding money request that is awaiting action from the current user */ hasOutstandingChildRequest?: boolean; /** List of icons for report participants */ From a9c9ef2cc30c7e5f817273e0e2586310639f5d27 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Mon, 30 Oct 2023 12:57:59 +0100 Subject: [PATCH 10/23] remove iou ordering --- src/libs/SidebarUtils.ts | 13 ++----------- tests/unit/SidebarOrderTest.js | 8 ++++---- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 6c4b893cb3e9..9ccdc85e9ad7 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -170,7 +170,6 @@ function getOrderedReportIDs( // 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 and settled IOUs // - Sorted by lastVisibleActionCreated in default (most recent) view mode @@ -179,15 +178,12 @@ function getOrderedReportIDs( // - Sorted by lastVisibleActionCreated in default (most recent) view mode // - Sorted by reportDisplayName in GSD (focus) view mode const pinnedReports: Report[] = []; - const outstandingIOUReports: Report[] = []; const draftReports: Report[] = []; const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; reportsToDisplay.forEach((report) => { - if (report.isPinned) { + if (report.isPinned || ReportUtils.shouldShowGBR(report)) { pinnedReports.push(report); - } else if (ReportUtils.shouldShowGBR(report)) { - outstandingIOUReports.push(report); } else if (report.hasDraft) { draftReports.push(report); } else if (ReportUtils.isArchivedRoom(report)) { @@ -199,11 +195,6 @@ function getOrderedReportIDs( // Sort each group of reports accordingly pinnedReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0)); - outstandingIOUReports.sort((a, b) => { - const compareAmounts = a?.iouReportAmount && b?.iouReportAmount ? b.iouReportAmount - a.iouReportAmount : 0; - const compareDisplayNames = a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0; - return compareAmounts || compareDisplayNames; - }); draftReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0)); if (isInDefaultMode) { @@ -221,7 +212,7 @@ function getOrderedReportIDs( // 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. - const LHNReports = [...pinnedReports, ...outstandingIOUReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); + const LHNReports = [...pinnedReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); setWithLimit(reportIDsCache, cachedReportsKey, LHNReports); return LHNReports; } diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index ea6fe49befc8..49785ead8b90 100644 --- a/tests/unit/SidebarOrderTest.js +++ b/tests/unit/SidebarOrderTest.js @@ -430,8 +430,8 @@ describe('Sidebar', () => { 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, [0, 'props', 'children'])).toBe('Email Two owes $100.00'); + expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('One, Two'); expect(lodashGet(displayNames, [2, 'props', 'children'])).toBe('Three, Four'); }) ); @@ -819,8 +819,8 @@ describe('Sidebar', () => { const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames'); const displayNames = screen.queryAllByLabelText(hintText); expect(displayNames).toHaveLength(5); - 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, [0, 'props', 'children'])).toBe('Email Five owes $100.00'); + expect(lodashGet(displayNames, [1, 'props', 'children'])).toBe('Email Four owes $1,000.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'); From 0ce08919121fafecf77cb665561ddcd8af723602 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Mon, 30 Oct 2023 16:04:20 +0100 Subject: [PATCH 11/23] lint fix --- src/libs/SidebarUtils.ts | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 9ccdc85e9ad7..e46523fd7e2c 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -1,23 +1,23 @@ /* eslint-disable rulesdir/prefer-underscore-method */ -import Onyx from 'react-native-onyx'; -import Str from 'expensify-common/lib/str'; -import {ValueOf} from 'type-fest'; -import ONYXKEYS from '../ONYXKEYS'; -import * as ReportUtils from './ReportUtils'; -import * as ReportActionsUtils from './ReportActionsUtils'; -import * as Localize from './Localize'; import CONST from '../CONST'; -import * as OptionsListUtils from './OptionsListUtils'; -import * as CollectionUtils from './CollectionUtils'; -import * as LocalePhoneNumber from './LocalePhoneNumber'; -import * as UserUtils from './UserUtils'; -import * as PersonalDetailsUtils from './PersonalDetailsUtils'; -import ReportAction, {ReportActions} from '../types/onyx/ReportAction'; +import ONYXKEYS from '../ONYXKEYS'; +import {PersonalDetails} from '../types/onyx'; import Beta from '../types/onyx/Beta'; +import * as OnyxCommon from '../types/onyx/OnyxCommon'; import Policy from '../types/onyx/Policy'; import Report from '../types/onyx/Report'; -import {PersonalDetails} from '../types/onyx'; -import * as OnyxCommon from '../types/onyx/OnyxCommon'; +import ReportAction, {ReportActions} from '../types/onyx/ReportAction'; +import * as CollectionUtils from './CollectionUtils'; +import * as LocalePhoneNumber from './LocalePhoneNumber'; +import * as Localize from './Localize'; +import * as OptionsListUtils from './OptionsListUtils'; +import * as PersonalDetailsUtils from './PersonalDetailsUtils'; +import * as ReportActionsUtils from './ReportActionsUtils'; +import * as ReportUtils from './ReportUtils'; +import * as UserUtils from './UserUtils'; +import Str from 'expensify-common/lib/str'; +import Onyx from 'react-native-onyx'; +import {ValueOf} from 'type-fest'; const visibleReportActionItems: ReportActions = {}; const lastReportActions: ReportActions = {}; @@ -182,7 +182,7 @@ function getOrderedReportIDs( const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; reportsToDisplay.forEach((report) => { - if (report.isPinned || ReportUtils.shouldShowGBR(report)) { + if (report.isPinned ?? ReportUtils.shouldShowGBR(report)) { pinnedReports.push(report); } else if (report.hasDraft) { draftReports.push(report); From c79c9f1cc9a9d359c3943d79db374f7daeca13fb Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 16:07:29 -0400 Subject: [PATCH 12/23] Add task logic to shouldShowGBR --- src/components/LHNOptionsList/OptionRowLHN.js | 2 +- src/libs/ReportUtils.js | 33 +++++++++---------- src/libs/SidebarUtils.ts | 5 ++- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/components/LHNOptionsList/OptionRowLHN.js b/src/components/LHNOptionsList/OptionRowLHN.js index 0c87e56ca87d..9257a5826ef0 100644 --- a/src/components/LHNOptionsList/OptionRowLHN.js +++ b/src/components/LHNOptionsList/OptionRowLHN.js @@ -116,7 +116,7 @@ function OptionRowLHN(props) { const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR; const defaultSubscriptSize = optionItem.isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT; - const shouldShowGreenDotIndicator = !hasBrickError && ReportUtils.shouldShowGBR(optionItem); + const shouldShowGreenDotIndicator = !hasBrickError && ReportUtils.shouldShowGBR(optionItem, optionItem.parentReportAction); /** * Show the ReportActionContextMenu modal popover. diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 6f01c41dfcd3..3293ccd1a659 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1323,6 +1323,17 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) { return ReportActionsUtils.getLastVisibleMessage(reportID, actionsToMerge); } +/** + * Checks if a report is an open task report assigned to current user. + * + * @param {Object} report + * @param {Object} parentReportAction - The parent report action of the report (Used to check if the task has been canceled) + * @returns {Boolean} + */ +function isWaitingForTaskCompleteFromAssignee(report, parentReportAction = {}) { + return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction); +} + /** * Determines if a report should show a GBR (green dot) in the LHN. This can happen when the report: - is unread and the user was mentioned in one of the unread comments @@ -1330,9 +1341,10 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) { - has an outstanding child money request that is waiting for an action from the current user (e.g. pay, approve, add bank account) * * @param {Object} report (chatReport or iouReport) + * @param {Object} parentReportAction (the report action the current report is a thread of) * @returns {boolean} */ -function shouldShowGBR(report) { +function shouldShowGBR(report, parentReportAction = {}) { if (!report) { return false; } @@ -1345,9 +1357,7 @@ function shouldShowGBR(report) { return true; } - // Money request waiting for current user to add their credit bank account - // hasOutstandingIOU will be false if the user paid, but isWaitingOnBankAccount will be true if user don't have a wallet or bank account setup - if (!report.hasOutstandingIOU && report.isWaitingOnBankAccount && report.ownerAccountID === currentUserAccountID) { + if (isWaitingForTaskCompleteFromAssignee(report, parentReportAction)) { return true; } @@ -1359,17 +1369,6 @@ function shouldShowGBR(report) { return false; } -/** - * Checks if a report is an open task report assigned to current user. - * - * @param {Object} report - * @param {Object} parentReportAction - The parent report action of the report (Used to check if the task has been canceled) - * @returns {Boolean} - */ -function isWaitingForTaskCompleteFromAssignee(report, parentReportAction = {}) { - return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction); -} - /** * Returns number of transactions that are nonReimbursable * @@ -3287,8 +3286,8 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, return true; } - // 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 || shouldShowGBR(report) || isWaitingForTaskCompleteFromAssignee(report)) { + // Include reports that are relevant to the user in any view mode. Criteria include having a draft or having a GBR showing. + if (report.hasDraft || shouldShowGBR(report)) { return true; } const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID); diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 091c40657710..c1bd45478882 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -261,6 +261,7 @@ type OptionData = { isTaskReport?: boolean | null; isWaitingForTaskCompleteFromAssignee?: boolean | null; parentReportID?: string | null; + parentReportAction?: ReportAction; notificationPreference?: string | number | null; displayNamesWithTooltips?: DisplayNamesWithTooltip[] | null; chatType?: ValueOf | null; @@ -350,9 +351,7 @@ function getOptionData( result.isThread = ReportUtils.isChatThread(report); result.isChatRoom = ReportUtils.isChatRoom(report); result.isTaskReport = ReportUtils.isTaskReport(report); - if (result.isTaskReport) { - result.isWaitingForTaskCompleteFromAssignee = ReportUtils.isWaitingForTaskCompleteFromAssignee(report, parentReportAction); - } + result.parentReportAction = parentReportAction; result.isArchivedRoom = ReportUtils.isArchivedRoom(report); result.isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report); result.isExpenseRequest = ReportUtils.isExpenseRequest(report); From c4979228b8a87b184aace418a9f60f51788983f4 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 16:08:59 -0400 Subject: [PATCH 13/23] Use or instead of null coalescing in getOrderedReportIDs --- src/libs/SidebarUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index c1bd45478882..c8e33970a318 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -182,7 +182,7 @@ function getOrderedReportIDs( const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; reportsToDisplay.forEach((report) => { - if (report.isPinned ?? ReportUtils.shouldShowGBR(report)) { + if (report.isPinned || ReportUtils.shouldShowGBR(report)) { pinnedReports.push(report); } else if (report.hasDraft) { draftReports.push(report); From 8e1ad6d0da4fe2fd5cacd365e095afd8b248200a Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 16:23:37 -0400 Subject: [PATCH 14/23] Update pinned logic for linter --- src/libs/SidebarUtils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index c8e33970a318..04a89c870eb2 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -182,7 +182,8 @@ function getOrderedReportIDs( const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; reportsToDisplay.forEach((report) => { - if (report.isPinned || ReportUtils.shouldShowGBR(report)) { + const isPinned = report.isPinned ?? false; + if (isPinned || ReportUtils.shouldShowGBR(report)) { pinnedReports.push(report); } else if (report.hasDraft) { draftReports.push(report); From 1195a835c2e0ba0dddaada3afaeecc0ae52e143c Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 16:37:19 -0400 Subject: [PATCH 15/23] Add tests for various shouldShowGBR cases --- tests/unit/ReportUtilsTest.js | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index ca40b9b11406..502b0d181418 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -5,6 +5,7 @@ import * as ReportUtils from '../../src/libs/ReportUtils'; import ONYXKEYS from '../../src/ONYXKEYS'; import * as LHNTestUtils from '../utils/LHNTestUtils'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +import DateUtils from '../../src/libs/DateUtils'; // Be sure to include the mocked permissions library or else the beta tests won't work jest.mock('../../src/libs/Permissions'); @@ -271,14 +272,14 @@ describe('ReportUtils', () => { expect(ReportUtils.shouldShowGBR(report)).toBe(false); }); }); - it('returns true when the report has no outstanding IOU but is waiting for a bank account and the logged user is the report owner', () => { + it('returns false when the report has no outstanding IOU but is waiting for a bank account and the logged user is the report owner', () => { const report = { ...LHNTestUtils.getFakeReport(), hasOutstandingIOU: false, ownerAccountID: currentUserAccountID, isWaitingOnBankAccount: true, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(true); + expect(ReportUtils.shouldShowGBR(report)).toBe(false); }); it('returns false when the report has outstanding IOU and is not waiting for a bank account and the logged user is the report owner', () => { const report = { @@ -298,6 +299,23 @@ describe('ReportUtils', () => { }; expect(ReportUtils.shouldShowGBR(report)).toBe(false); }); + it('returns true when the report has an unread mention', () => { + const report = { + ...LHNTestUtils.getFakeReport(), + isUnreadWithMention: true, + }; + expect(ReportUtils.shouldShowGBR(report)).toBe(true); + }); + it('returns true when the report is an outstanding task', () => { + const report = { + ...LHNTestUtils.getFakeReport(), + type: CONST.REPORT.TYPE.TASK, + managerID: currentUserAccountID, + stateNum: CONST.REPORT.STATE_NUM.OPEN, + statusNum: CONST.REPORT.STATUS.OPEN, + }; + expect(ReportUtils.shouldShowGBR(report)).toBe(true); + }); it('returns true when the report has oustanding child request', () => { const report = { ...LHNTestUtils.getFakeReport(), From 3e938c6c22353cc00de75e32b08bafc53f7b8c43 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 16:42:01 -0400 Subject: [PATCH 16/23] Remove unneeded import --- tests/unit/ReportUtilsTest.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index 502b0d181418..166a0983761b 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -5,7 +5,6 @@ import * as ReportUtils from '../../src/libs/ReportUtils'; import ONYXKEYS from '../../src/ONYXKEYS'; import * as LHNTestUtils from '../utils/LHNTestUtils'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; -import DateUtils from '../../src/libs/DateUtils'; // Be sure to include the mocked permissions library or else the beta tests won't work jest.mock('../../src/libs/Permissions'); From 056b7d5cb8894027434ba938db620fa157950373 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 17:13:21 -0400 Subject: [PATCH 17/23] Update comment about pinned/gbr reports --- src/libs/SidebarUtils.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 04a89c870eb2..c6a58aaac8bb 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -168,23 +168,23 @@ function getOrderedReportIDs( report.iouReportAmount = ReportUtils.getMoneyRequestReimbursableTotal(report, allReports); }); - // 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 - // 3. Drafts - Always sorted by reportDisplayName - // 4. Non-archived reports and settled IOUs + // The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order: + // 1. Pinned/GBR - Always sorted by reportDisplayName + // 2. Drafts - Always sorted by reportDisplayName + // 3. Non-archived reports and settled IOUs // - Sorted by lastVisibleActionCreated in default (most recent) view mode // - Sorted by reportDisplayName in GSD (focus) view mode - // 5. Archived reports + // 4. Archived reports // - Sorted by lastVisibleActionCreated in default (most recent) view mode // - Sorted by reportDisplayName in GSD (focus) view mode - const pinnedReports: Report[] = []; + const pinnedAndGBRReports: Report[] = []; const draftReports: Report[] = []; const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; reportsToDisplay.forEach((report) => { const isPinned = report.isPinned ?? false; if (isPinned || ReportUtils.shouldShowGBR(report)) { - pinnedReports.push(report); + pinnedAndGBRReports.push(report); } else if (report.hasDraft) { draftReports.push(report); } else if (ReportUtils.isArchivedRoom(report)) { @@ -195,7 +195,7 @@ function getOrderedReportIDs( }); // Sort each group of reports accordingly - pinnedReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0)); + pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0)); draftReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0)); if (isInDefaultMode) { @@ -213,7 +213,7 @@ function getOrderedReportIDs( // 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. - const LHNReports = [...pinnedReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); + const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); setWithLimit(reportIDsCache, cachedReportsKey, LHNReports); return LHNReports; } From 943bc802132a00884f42674bc90fe0e8cdb2674b Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 17:26:24 -0400 Subject: [PATCH 18/23] Rename functions to separate UI from function logic --- src/libs/ReportUtils.js | 14 +++++++------- src/libs/SidebarUtils.ts | 3 +-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 3293ccd1a659..1e9ac5ef0ad1 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1330,12 +1330,12 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) { * @param {Object} parentReportAction - The parent report action of the report (Used to check if the task has been canceled) * @returns {Boolean} */ -function isWaitingForTaskCompleteFromAssignee(report, parentReportAction = {}) { +function isWaitingForAssigneeToCompleteTask(report, parentReportAction = {}) { return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction); } /** - * Determines if a report should show a GBR (green dot) in the LHN. This can happen when the report: + * Determines if a report requires action from the current user. This can happen when the report: - is unread and the user was mentioned in one of the unread comments - is for an outstanding task waiting on the user - has an outstanding child money request that is waiting for an action from the current user (e.g. pay, approve, add bank account) @@ -1344,7 +1344,7 @@ function isWaitingForTaskCompleteFromAssignee(report, parentReportAction = {}) { * @param {Object} parentReportAction (the report action the current report is a thread of) * @returns {boolean} */ -function shouldShowGBR(report, parentReportAction = {}) { +function requiresAttentionFromCurrentUser(report, parentReportAction = {}) { if (!report) { return false; } @@ -1357,7 +1357,7 @@ function shouldShowGBR(report, parentReportAction = {}) { return true; } - if (isWaitingForTaskCompleteFromAssignee(report, parentReportAction)) { + if (isWaitingForAssigneeToCompleteTask(report, parentReportAction)) { return true; } @@ -3287,7 +3287,7 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas, } // Include reports that are relevant to the user in any view mode. Criteria include having a draft or having a GBR showing. - if (report.hasDraft || shouldShowGBR(report)) { + if (report.hasDraft || requiresAttentionFromCurrentUser(report)) { return true; } const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID); @@ -4145,7 +4145,7 @@ export { isCurrentUserTheOnlyParticipant, hasAutomatedExpensifyAccountIDs, hasExpensifyGuidesEmails, - shouldShowGBR, + requiresAttentionFromCurrentUser, isIOUOwnedByCurrentUser, getMoneyRequestReimbursableTotal, getMoneyRequestSpendBreakdown, @@ -4265,7 +4265,7 @@ export { hasNonReimbursableTransactions, hasMissingSmartscanFields, getIOUReportActionDisplayMessage, - isWaitingForTaskCompleteFromAssignee, + isWaitingForAssigneeToCompleteTask, isGroupChat, isReportDraft, shouldUseFullTitleToDisplay, diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index c6a58aaac8bb..ae468ab1aad2 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -183,7 +183,7 @@ function getOrderedReportIDs( const archivedReports: Report[] = []; reportsToDisplay.forEach((report) => { const isPinned = report.isPinned ?? false; - if (isPinned || ReportUtils.shouldShowGBR(report)) { + if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report)) { pinnedAndGBRReports.push(report); } else if (report.hasDraft) { draftReports.push(report); @@ -260,7 +260,6 @@ type OptionData = { isAllowedToComment?: boolean | null; isThread?: boolean | null; isTaskReport?: boolean | null; - isWaitingForTaskCompleteFromAssignee?: boolean | null; parentReportID?: string | null; parentReportAction?: ReportAction; notificationPreference?: string | number | null; From d6eff10375790ab10af0c86db33bc5b1ade8ab98 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 22:26:42 +0100 Subject: [PATCH 19/23] Update src/libs/ReportUtils.js Co-authored-by: Marc Glasser --- src/libs/ReportUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 1e9ac5ef0ad1..1d5361f0fe46 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1327,7 +1327,7 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) { * Checks if a report is an open task report assigned to current user. * * @param {Object} report - * @param {Object} parentReportAction - The parent report action of the report (Used to check if the task has been canceled) + * @param {Object} [parentReportAction] - The parent report action of the report (Used to check if the task has been canceled) * @returns {Boolean} */ function isWaitingForAssigneeToCompleteTask(report, parentReportAction = {}) { From fb9b40d06108a76b65b15852163fe8c358913897 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 17:34:02 -0400 Subject: [PATCH 20/23] Rename shouldShowGBR to requiresAttentionFromCurrentUser in tests --- tests/unit/ReportUtilsTest.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index 166a0983761b..a29b2727c847 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -246,9 +246,9 @@ describe('ReportUtils', () => { }); }); - describe('shouldShowGBR', () => { + describe('requiresAttentionFromCurrentUser', () => { it('returns false when there is no report', () => { - expect(ReportUtils.shouldShowGBR()).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser()).toBe(false); }); it('returns false when the matched IOU report does not have an owner accountID', () => { const report = { @@ -256,7 +256,7 @@ describe('ReportUtils', () => { ownerAccountID: undefined, hasOutstandingIOU: true, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false); }); it('returns false when the linked iou report has an oustanding IOU', () => { const report = { @@ -268,7 +268,7 @@ describe('ReportUtils', () => { ownerAccountID: 99, hasOutstandingIOU: true, }).then(() => { - expect(ReportUtils.shouldShowGBR(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false); }); }); it('returns false when the report has no outstanding IOU but is waiting for a bank account and the logged user is the report owner', () => { @@ -278,7 +278,7 @@ describe('ReportUtils', () => { ownerAccountID: currentUserAccountID, isWaitingOnBankAccount: true, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false); }); it('returns false when the report has outstanding IOU and is not waiting for a bank account and the logged user is the report owner', () => { const report = { @@ -287,7 +287,7 @@ describe('ReportUtils', () => { ownerAccountID: currentUserAccountID, isWaitingOnBankAccount: false, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false); }); it('returns false 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 = { @@ -296,14 +296,14 @@ describe('ReportUtils', () => { ownerAccountID: 97, isWaitingOnBankAccount: true, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false); }); it('returns true when the report has an unread mention', () => { const report = { ...LHNTestUtils.getFakeReport(), isUnreadWithMention: true, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(true); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(true); }); it('returns true when the report is an outstanding task', () => { const report = { @@ -313,7 +313,7 @@ describe('ReportUtils', () => { stateNum: CONST.REPORT.STATE_NUM.OPEN, statusNum: CONST.REPORT.STATUS.OPEN, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(true); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(true); }); it('returns true when the report has oustanding child request', () => { const report = { @@ -323,7 +323,7 @@ describe('ReportUtils', () => { hasOutstandingChildRequest: true, isWaitingOnBankAccount: false, }; - expect(ReportUtils.shouldShowGBR(report)).toBe(true); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(true); }); }); From cc748872d65b0069a2d8f06adb5e45b01540c258 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 18:01:46 -0400 Subject: [PATCH 21/23] Rename shouldShowGBR in one more place --- src/components/LHNOptionsList/OptionRowLHN.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/LHNOptionsList/OptionRowLHN.js b/src/components/LHNOptionsList/OptionRowLHN.js index 9257a5826ef0..8360fe58f1ef 100644 --- a/src/components/LHNOptionsList/OptionRowLHN.js +++ b/src/components/LHNOptionsList/OptionRowLHN.js @@ -116,7 +116,7 @@ function OptionRowLHN(props) { const hasBrickError = optionItem.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR; const defaultSubscriptSize = optionItem.isExpenseRequest ? CONST.AVATAR_SIZE.SMALL_NORMAL : CONST.AVATAR_SIZE.DEFAULT; - const shouldShowGreenDotIndicator = !hasBrickError && ReportUtils.shouldShowGBR(optionItem, optionItem.parentReportAction); + const shouldShowGreenDotIndicator = !hasBrickError && ReportUtils.requiresAttentionFromCurrentUser(optionItem, optionItem.parentReportAction); /** * Show the ReportActionContextMenu modal popover. From b35d1da5a4e40ace1fc3f4a0eeaa86d610ca3017 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 18:38:18 -0400 Subject: [PATCH 22/23] Rename report to option in requiresAttentionFromCurrentUser --- src/libs/ReportUtils.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 1d5361f0fe46..fd35d69b6c88 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1335,34 +1335,34 @@ function isWaitingForAssigneeToCompleteTask(report, parentReportAction = {}) { } /** - * Determines if a report requires action from the current user. This can happen when the report: + * Determines if the option requires action from the current user. This can happen when it: - is unread and the user was mentioned in one of the unread comments - is for an outstanding task waiting on the user - has an outstanding child money request that is waiting for an action from the current user (e.g. pay, approve, add bank account) * - * @param {Object} report (chatReport or iouReport) + * @param {Object} option (report or optionItem) * @param {Object} parentReportAction (the report action the current report is a thread of) * @returns {boolean} */ -function requiresAttentionFromCurrentUser(report, parentReportAction = {}) { - if (!report) { +function requiresAttentionFromCurrentUser(option, parentReportAction = {}) { + if (!option) { return false; } - if (isArchivedRoom(getReport(report.parentReportID))) { + if (isArchivedRoom(getReport(option.parentReportID))) { return false; } - if (report.isUnreadWithMention) { + if (option.isUnreadWithMention) { return true; } - if (isWaitingForAssigneeToCompleteTask(report, parentReportAction)) { + if (isWaitingForAssigneeToCompleteTask(option, parentReportAction)) { return true; } // Has a child report that is awaiting action (e.g. approve, pay, add bank account) from current user - if (report.hasOutstandingChildRequest) { + if (option.hasOutstandingChildRequest) { return true; } From efcefdbec52e9a8e2a5b25e49791da2f44352e90 Mon Sep 17 00:00:00 2001 From: Puneet Lath Date: Tue, 31 Oct 2023 18:58:22 -0400 Subject: [PATCH 23/23] Account for both options and reports in unread check --- src/libs/ReportUtils.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index fd35d69b6c88..7d47dee9096e 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1334,6 +1334,21 @@ function isWaitingForAssigneeToCompleteTask(report, parentReportAction = {}) { return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction); } +/** + * @param {Object} report + * @returns {Boolean} + */ +function isUnreadWithMention(report) { + if (!report) { + return false; + } + + // lastMentionedTime and lastReadTime are both datetime strings and can be compared directly + const lastMentionedTime = report.lastMentionedTime || ''; + const lastReadTime = report.lastReadTime || ''; + return lastReadTime < lastMentionedTime; +} + /** * Determines if the option requires action from the current user. This can happen when it: - is unread and the user was mentioned in one of the unread comments @@ -1353,7 +1368,7 @@ function requiresAttentionFromCurrentUser(option, parentReportAction = {}) { return false; } - if (option.isUnreadWithMention) { + if (option.isUnreadWithMention || isUnreadWithMention(option)) { return true; } @@ -3105,21 +3120,6 @@ function isUnread(report) { return lastReadTime < lastVisibleActionCreated; } -/** - * @param {Object} report - * @returns {Boolean} - */ -function isUnreadWithMention(report) { - if (!report) { - return false; - } - - // lastMentionedTime and lastReadTime are both datetime strings and can be compared directly - const lastMentionedTime = report.lastMentionedTime || ''; - const lastReadTime = report.lastReadTime || ''; - return lastReadTime < lastMentionedTime; -} - /** * @param {Object} report * @param {Object} allReportsDict