diff --git a/src/components/LHNOptionsList/OptionRowLHN.js b/src/components/LHNOptionsList/OptionRowLHN.js index f83e0b834287..8360fe58f1ef 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.requiresAttentionFromCurrentUser(optionItem, optionItem.parentReportAction); /** * Show the ReportActionContextMenu modal popover. diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 8cadc6dcc8ec..7d47dee9096e 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1324,61 +1324,66 @@ 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) + * Checks if a report is an open task report assigned to current user. * - * @param {Object} report (chatReport or iouReport) - * @returns {boolean} + * @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 isWaitingForAssigneeToCompleteTask(report, parentReportAction = {}) { + return isTaskReport(report) && isReportManager(report) && isOpenTaskReport(report, parentReportAction); +} + +/** + * @param {Object} report + * @returns {Boolean} */ -function isWaitingForIOUActionFromCurrentUser(report) { +function isUnreadWithMention(report) { if (!report) { return false; } - if (isArchivedRoom(getReport(report.parentReportID))) { + // 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 + - 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} option (report or optionItem) + * @param {Object} parentReportAction (the report action the current report is a thread of) + * @returns {boolean} + */ +function requiresAttentionFromCurrentUser(option, parentReportAction = {}) { + if (!option) { 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; - } + if (isArchivedRoom(getReport(option.parentReportID))) { + return false; + } - // 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 (option.isUnreadWithMention || isUnreadWithMention(option)) { + 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 (isWaitingForAssigneeToCompleteTask(option, parentReportAction)) { 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 (option.hasOutstandingChildRequest) { return true; } 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 * @@ -3115,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 @@ -3296,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 || isWaitingForIOUActionFromCurrentUser(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 || requiresAttentionFromCurrentUser(report)) { return true; } const lastVisibleMessage = ReportActionsUtils.getLastVisibleMessage(report.reportID); @@ -4155,7 +4145,7 @@ export { isCurrentUserTheOnlyParticipant, hasAutomatedExpensifyAccountIDs, hasExpensifyGuidesEmails, - isWaitingForIOUActionFromCurrentUser, + requiresAttentionFromCurrentUser, isIOUOwnedByCurrentUser, getMoneyRequestReimbursableTotal, getMoneyRequestSpendBreakdown, @@ -4275,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 79d3280e859e..ae468ab1aad2 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -168,26 +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 - // 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 + // 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 outstandingIOUReports: Report[] = []; + const pinnedAndGBRReports: Report[] = []; const draftReports: Report[] = []; const nonArchivedReports: Report[] = []; const archivedReports: Report[] = []; reportsToDisplay.forEach((report) => { - if (report.isPinned) { - pinnedReports.push(report); - } else if (ReportUtils.isWaitingForIOUActionFromCurrentUser(report)) { - outstandingIOUReports.push(report); + const isPinned = report.isPinned ?? false; + if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report)) { + pinnedAndGBRReports.push(report); } else if (report.hasDraft) { draftReports.push(report); } else if (ReportUtils.isArchivedRoom(report)) { @@ -198,12 +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)); - 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; - }); + 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) { @@ -221,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, ...outstandingIOUReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); + const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID); setWithLimit(reportIDsCache, cachedReportsKey, LHNReports); return LHNReports; } @@ -254,6 +246,7 @@ type OptionData = { searchText?: string | null; isPinned?: boolean | null; hasOutstandingIOU?: boolean | null; + hasOutstandingChildRequest?: boolean | null; iouReportID?: string | null; isIOUReportOwner?: boolean | null; iouReportAmount?: number | null; @@ -267,8 +260,8 @@ type OptionData = { isAllowedToComment?: boolean | null; isThread?: boolean | null; isTaskReport?: boolean | null; - isWaitingForTaskCompleteFromAssignee?: boolean | null; parentReportID?: string | null; + parentReportAction?: ReportAction; notificationPreference?: string | number | null; displayNamesWithTooltips?: DisplayNamesWithTooltip[] | null; chatType?: ValueOf | null; @@ -338,6 +331,7 @@ function getOptionData( searchText: null, isPinned: false, hasOutstandingIOU: false, + hasOutstandingChildRequest: false, iouReportID: null, isIOUReportOwner: null, iouReportAmount: 0, @@ -357,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); @@ -382,6 +374,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..a29b2727c847 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -246,9 +246,9 @@ describe('ReportUtils', () => { }); }); - describe('isWaitingForIOUActionFromCurrentUser', () => { + describe('requiresAttentionFromCurrentUser', () => { it('returns false when there is no report', () => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser()).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.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false); }); it('returns false when the linked iou report has an oustanding IOU', () => { const report = { @@ -268,17 +268,17 @@ describe('ReportUtils', () => { ownerAccountID: 99, hasOutstandingIOU: true, }).then(() => { - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(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.isWaitingForIOUActionFromCurrentUser(report)).toBe(true); + 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.isWaitingForIOUActionFromCurrentUser(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,16 +296,34 @@ describe('ReportUtils', () => { ownerAccountID: 97, isWaitingOnBankAccount: true, }; - expect(ReportUtils.isWaitingForIOUActionFromCurrentUser(report)).toBe(false); + expect(ReportUtils.requiresAttentionFromCurrentUser(report)).toBe(false); }); - it('returns true when the report has oustanding IOU', () => { + it('returns true when the report has an unread mention', () => { + const report = { + ...LHNTestUtils.getFakeReport(), + isUnreadWithMention: true, + }; + expect(ReportUtils.requiresAttentionFromCurrentUser(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.requiresAttentionFromCurrentUser(report)).toBe(true); + }); + 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.requiresAttentionFromCurrentUser(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');