[HOLD] 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.shouldShowGBR(optionItem);
 
     /**
      * Show the ReportActionContextMenu modal popover.

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,

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;

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('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 = {
                 ...LHNTestUtils.getFakeReport(),
                 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);
         });
     });
 

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');