Merge pull request Expensify#30655 from Expensify/feat-29595-Update-G…

…BR-is-determined

Feat: Update how GBR is determined for IOU/expense reports

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.

src/libs/ReportUtils.js

@@ -1345,61 +1345,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
  *
@@ -3136,21 +3141,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
@@ -3294,8 +3284,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);
@@ -4153,7 +4143,7 @@ export {
     isCurrentUserTheOnlyParticipant,
     hasAutomatedExpensifyAccountIDs,
     hasExpensifyGuidesEmails,
-    isWaitingForIOUActionFromCurrentUser,
+    requiresAttentionFromCurrentUser,
     isIOUOwnedByCurrentUser,
     getMoneyRequestReimbursableTotal,
     getMoneyRequestSpendBreakdown,
@@ -4273,7 +4263,7 @@ export {
     hasNonReimbursableTransactions,
     hasMissingSmartscanFields,
     getIOUReportActionDisplayMessage,
-    isWaitingForTaskCompleteFromAssignee,
+    isWaitingForAssigneeToCompleteTask,
     isGroupChat,
     isReportDraft,
     shouldUseFullTitleToDisplay,

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<typeof CONST.REPORT.CHAT_TYPE> | 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;

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,

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[];
 

tests/unit/ReportUtilsTest.js

@@ -246,17 +246,17 @@ 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 = {
                 ...LHNTestUtils.getFakeReport(),
                 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);
         });
     });