diff --git a/src/components/LHNOptionsList/OptionRowLHN.js b/src/components/LHNOptionsList/OptionRowLHN.js index f83e0b834287..0c87e56ca87d 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 && ReportUtils.shouldShowGBR(optionItem); /** * Show the ReportActionContextMenu modal popover. diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 8cadc6dcc8ec..6f01c41dfcd3 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1324,12 +1324,15 @@ function getLastVisibleMessage(reportID, actionsToMerge = {}) { } /** - * 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 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} */ -function isWaitingForIOUActionFromCurrentUser(report) { +function shouldShowGBR(report) { if (!report) { return false; } @@ -1338,20 +1341,8 @@ 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); + if (report.isUnreadWithMention) { + return true; } // Money request waiting for current user to add their credit bank account @@ -1360,8 +1351,8 @@ 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)) { + // Has a child report that is awaiting action (e.g. approve, pay, add bank account) from current user + if (report.hasOutstandingChildRequest) { return true; } @@ -3297,7 +3288,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); @@ -4155,7 +4146,7 @@ export { isCurrentUserTheOnlyParticipant, hasAutomatedExpensifyAccountIDs, hasExpensifyGuidesEmails, - isWaitingForIOUActionFromCurrentUser, + shouldShowGBR, isIOUOwnedByCurrentUser, getMoneyRequestReimbursableTotal, getMoneyRequestSpendBreakdown, diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 79d3280e859e..091c40657710 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.isWaitingForIOUActionFromCurrentUser(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; } @@ -254,6 +245,7 @@ type OptionData = { searchText?: string | null; isPinned?: boolean | null; hasOutstandingIOU?: boolean | null; + hasOutstandingChildRequest?: boolean | null; iouReportID?: string | null; isIOUReportOwner?: boolean | null; iouReportAmount?: number | null; @@ -338,6 +330,7 @@ function getOptionData( searchText: null, isPinned: false, hasOutstandingIOU: false, + hasOutstandingChildRequest: false, iouReportID: null, isIOUReportOwner: null, iouReportAmount: 0, @@ -382,6 +375,7 @@ function getOptionData( 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; diff --git a/src/pages/home/sidebar/SidebarLinksData.js b/src/pages/home/sidebar/SidebarLinksData.js index a5d58768a95d..293dc3f5cd9d 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 6fe9b5fd5099..7721f3518181 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 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 */ icons?: OnyxCommon.Icon[]; diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index 72de874a631e..ca40b9b11406 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 true 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.isWaitingForIOUActionFromCurrentUser(report)).toBe(true); + expect(ReportUtils.shouldShowGBR(report)).toBe(true); }); 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.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 not the report owner', () => { const report = { @@ -296,16 +296,17 @@ 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); }); }); diff --git a/tests/unit/SidebarOrderTest.js b/tests/unit/SidebarOrderTest.js index 72b3a0c2f631..e9caed9f3dc7 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, @@ -396,6 +396,7 @@ describe('Sidebar', () => { ownerAccountID: 2, managerID: 2, hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -404,7 +405,6 @@ describe('Sidebar', () => { const currentReportId = report2.reportID; const currentlyLoggedInUserAccountID = 9; LHNTestUtils.getDefaultRenderedSidebarLinks(currentReportId); - return ( waitForBatchedUpdates() // When Onyx is updated with the data and the sidebar re-renders @@ -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'); }) ); @@ -734,6 +734,7 @@ describe('Sidebar', () => { ownerAccountID: 2, managerID: 2, hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -744,6 +745,7 @@ describe('Sidebar', () => { ownerAccountID: 2, managerID: 3, hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -754,6 +756,7 @@ describe('Sidebar', () => { ownerAccountID: 2, managerID: 4, hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 100000, currency: 'USD', chatReportID: report3.reportID, @@ -764,6 +767,7 @@ describe('Sidebar', () => { ownerAccountID: 2, managerID: 5, hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -774,6 +778,7 @@ describe('Sidebar', () => { ownerAccountID: 2, managerID: 6, hasOutstandingIOU: true, + hasOutstandingChildRequest: true, total: 10000, currency: 'USD', chatReportID: report3.reportID, @@ -814,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');