[Payment due @Krishna2323] Exclude reports with all expenses on hold from Submit/Approve/Pay to-dos

config/eslint/eslint.seatbelt.tsv

@@ -1846,7 +1846,7 @@
 "../../tests/unit/NextStepUtilsTest.ts"	"@typescript-eslint/no-deprecated/buildNextStepNew"	23
 "../../tests/unit/NextStepUtilsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	2
 "../../tests/unit/OnboardingSelectorsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	10
-"../../tests/unit/OnyxDerivedTest.tsx"	"@typescript-eslint/no-unsafe-type-assertion"	7
+"../../tests/unit/OnyxDerivedTest.tsx"	"@typescript-eslint/no-unsafe-type-assertion"	8
 "../../tests/unit/OnyxUpdateManagerTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	4
 "../../tests/unit/OptionListContextProviderTest.tsx"	"@typescript-eslint/no-unsafe-type-assertion"	10
 "../../tests/unit/OptionsListUtilsTest.tsx"	"@typescript-eslint/no-unsafe-type-assertion"	50
@@ -1870,7 +1870,7 @@
 "../../tests/unit/ReportActionsUtilsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	35
 "../../tests/unit/ReportLayoutUtilsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	1
 "../../tests/unit/ReportNameUtilsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	23
-"../../tests/unit/ReportPrimaryActionUtilsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	168
+"../../tests/unit/ReportPrimaryActionUtilsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	175
 "../../tests/unit/ReportPrimaryActionUtilsTest.ts"	"no-restricted-imports"	1
 "../../tests/unit/ReportSecondaryActionUtilsTest.ts"	"@typescript-eslint/no-unsafe-type-assertion"	466
 "../../tests/unit/ReportSecondaryActionUtilsTest.ts"	"no-restricted-imports"	2

src/libs/ReportPrimaryActionUtils.ts

@@ -469,6 +469,7 @@ function getReportPrimaryAction(params: GetReportPrimaryActionParams): ValueOf<t
         return '';
     }
 
+    const allExpensesHeld = hasOnlyHeldExpenses(reportTransactions);
     const isPayActionWithAllExpensesHeld =
         isPrimaryPayAction({
             report,
@@ -481,7 +482,7 @@ function getReportPrimaryAction(params: GetReportPrimaryActionParams): ValueOf<t
             isChatReportArchived,
             invoiceReceiverPolicy,
             reportActions,
-        }) && hasOnlyHeldExpenses(reportTransactions);
+        }) && allExpensesHeld;
     const expensesToHold = getAllExpensesToHoldIfApplicable(report, reportActions, reportTransactions, policy, currentUserAccountID);
 
     if (isMarkAsCashAction(currentUserLogin, currentUserAccountID, report, reportTransactions, violations, policy)) {
@@ -492,7 +493,7 @@ function getReportPrimaryAction(params: GetReportPrimaryActionParams): ValueOf<t
         return CONST.REPORT.PRIMARY_ACTIONS.REVIEW_DUPLICATES;
     }
 
-    if (isApproveAction(report, reportTransactions, currentUserAccountID, reportMetadata, policy)) {
+    if (isApproveAction(report, reportTransactions, currentUserAccountID, reportMetadata, policy) && !allExpensesHeld) {
         return CONST.REPORT.PRIMARY_ACTIONS.APPROVE;
     }
 
@@ -504,7 +505,7 @@ function getReportPrimaryAction(params: GetReportPrimaryActionParams): ValueOf<t
         return CONST.REPORT.PRIMARY_ACTIONS.MARK_AS_RESOLVED;
     }
 
-    if (isSubmitAction(report, reportTransactions, reportMetadata, policy, reportNameValuePairs, violations, currentUserLogin, currentUserAccountID)) {
+    if (isSubmitAction(report, reportTransactions, reportMetadata, policy, reportNameValuePairs, violations, currentUserLogin, currentUserAccountID) && !allExpensesHeld) {
         return CONST.REPORT.PRIMARY_ACTIONS.SUBMIT;
     }
 
@@ -520,7 +521,8 @@ function getReportPrimaryAction(params: GetReportPrimaryActionParams): ValueOf<t
             isChatReportArchived,
             invoiceReceiverPolicy,
             reportActions,
-        })
+        }) &&
+        !allExpensesHeld
     ) {
         return CONST.REPORT.PRIMARY_ACTIONS.PAY;
     }

src/libs/actions/OnyxDerived/configs/todos.ts

@@ -1,6 +1,6 @@
 import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
 import {isApproveAction, isExportAction, isPrimaryPayAction, isSubmitAction} from '@libs/ReportPrimaryActionUtils';
-import {hasOnlyNonReimbursableTransactions} from '@libs/ReportUtils';
+import {hasOnlyHeldExpenses, hasOnlyNonReimbursableTransactions} from '@libs/ReportUtils';
 import createOnyxDerivedValueConfig from '@userActions/OnyxDerived/createOnyxDerivedValueConfig';
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
@@ -58,10 +58,11 @@ const createTodosReportsAndTransactions = ({
         const reportActions = Object.values(allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`] ?? []);
         const reportTransactions = transactionsByReportID[report.reportID] ?? [];
         const reportMetadata = allReportMetadata?.[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${report.reportID}`];
-        if (isSubmitAction(report, reportTransactions, reportMetadata, policy, reportNameValuePair, undefined, login, currentUserAccountID)) {
+        const allExpensesHeld = hasOnlyHeldExpenses(reportTransactions);
+        if (isSubmitAction(report, reportTransactions, reportMetadata, policy, reportNameValuePair, undefined, login, currentUserAccountID) && !allExpensesHeld) {
             reportsToSubmit.push(report);
         }
-        if (isApproveAction(report, reportTransactions, currentUserAccountID, reportMetadata, policy)) {
+        if (isApproveAction(report, reportTransactions, currentUserAccountID, reportMetadata, policy) && !allExpensesHeld) {
             reportsToApprove.push(report);
         }
         if (
@@ -74,7 +75,8 @@ const createTodosReportsAndTransactions = ({
                 policy,
                 reportNameValuePairs: reportNameValuePair,
             }) &&
-            !hasOnlyNonReimbursableTransactions(report.reportID, reportTransactions)
+            !hasOnlyNonReimbursableTransactions(report.reportID, reportTransactions) &&
+            !allExpensesHeld
         ) {
             reportsToPay.push(report);
         }

tests/unit/OnyxDerivedTest.tsx

@@ -660,6 +660,73 @@ describe('OnyxDerived', () => {
             });
         });
 
+        describe('excludes reports with all expenses on hold', () => {
+            const HELD_SUBMIT_REPORT_ID = 'held_submit_1';
+            const HELD_APPROVE_REPORT_ID = 'held_approve_1';
+            const HELD_PAY_REPORT_ID = 'held_pay_1';
+
+            beforeEach(async () => {
+                const submitReport = createMockReport(HELD_SUBMIT_REPORT_ID, {
+                    stateNum: CONST.REPORT.STATE_NUM.OPEN,
+                    statusNum: CONST.REPORT.STATUS_NUM.OPEN,
+                    ownerAccountID: CURRENT_USER_ACCOUNT_ID,
+                });
+                const approveReport = createMockReport(HELD_APPROVE_REPORT_ID, {
+                    stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
+                    statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
+                    ownerAccountID: OTHER_USER_ACCOUNT_ID,
+                    managerID: CURRENT_USER_ACCOUNT_ID,
+                });
+                const payReport = createMockReport(HELD_PAY_REPORT_ID, {
+                    stateNum: CONST.REPORT.STATE_NUM.APPROVED,
+                    statusNum: CONST.REPORT.STATUS_NUM.APPROVED,
+                    ownerAccountID: OTHER_USER_ACCOUNT_ID,
+                    managerID: CURRENT_USER_ACCOUNT_ID,
+                    total: -100,
+                });
+
+                const policy = createMockPolicy(POLICY_ID, {
+                    approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
+                    role: CONST.POLICY.ROLE.ADMIN,
+                    ownerAccountID: CURRENT_USER_ACCOUNT_ID,
+                    reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES,
+                });
+
+                const heldOverride: Partial<Transaction> = {comment: {hold: 'HOLD_ACTION_ID'}};
+
+                await Onyx.multiSet({
+                    [ONYXKEYS.SESSION]: {
+                        email: CURRENT_USER_EMAIL,
+                        accountID: CURRENT_USER_ACCOUNT_ID,
+                    },
+                    [`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`]: policy,
+                    [`${ONYXKEYS.COLLECTION.REPORT}${HELD_SUBMIT_REPORT_ID}`]: submitReport,
+                    [`${ONYXKEYS.COLLECTION.REPORT}${HELD_APPROVE_REPORT_ID}`]: approveReport,
+                    [`${ONYXKEYS.COLLECTION.REPORT}${HELD_PAY_REPORT_ID}`]: payReport,
+                    [`${ONYXKEYS.COLLECTION.TRANSACTION}trans_${HELD_SUBMIT_REPORT_ID}`]: createMockTransaction(`trans_${HELD_SUBMIT_REPORT_ID}`, HELD_SUBMIT_REPORT_ID, heldOverride),
+                    [`${ONYXKEYS.COLLECTION.TRANSACTION}trans_${HELD_APPROVE_REPORT_ID}`]: createMockTransaction(`trans_${HELD_APPROVE_REPORT_ID}`, HELD_APPROVE_REPORT_ID, heldOverride),
+                    [`${ONYXKEYS.COLLECTION.TRANSACTION}trans_${HELD_PAY_REPORT_ID}`]: createMockTransaction(`trans_${HELD_PAY_REPORT_ID}`, HELD_PAY_REPORT_ID, heldOverride),
+                } as OnyxMultiSetInput);
+
+                await waitForBatchedUpdates();
+            });
+
+            it('excludes an all-held report from reportsToSubmit', async () => {
+                const todos = await OnyxUtils.get(ONYXKEYS.DERIVED.TODOS);
+                expect(todos?.reportsToSubmit.map((r) => r.reportID)).not.toContain(HELD_SUBMIT_REPORT_ID);
+            });
+
+            it('excludes an all-held report from reportsToApprove', async () => {
+                const todos = await OnyxUtils.get(ONYXKEYS.DERIVED.TODOS);
+                expect(todos?.reportsToApprove.map((r) => r.reportID)).not.toContain(HELD_APPROVE_REPORT_ID);
+            });
+
+            it('excludes an all-held report from reportsToPay', async () => {
+                const todos = await OnyxUtils.get(ONYXKEYS.DERIVED.TODOS);
+                expect(todos?.reportsToPay.map((r) => r.reportID)).not.toContain(HELD_PAY_REPORT_ID);
+            });
+        });
+
         describe('categorizes reports correctly', () => {
             const SUBMIT_REPORT_IDS = ['submit_1', 'submit_2', 'submit_3', 'submit_4'];
             const APPROVE_REPORT_IDS = ['approve_1', 'approve_2', 'approve_3'];

tests/unit/ReportPrimaryActionUtilsTest.ts

@@ -306,9 +306,6 @@ describe('getPrimaryAction', () => {
         };
         const transaction = {
             reportID: `${REPORT_ID}`,
-            comment: {
-                hold: 'Hold',
-            },
         } as unknown as Transaction;
 
         expect(
@@ -573,6 +570,40 @@ describe('getPrimaryAction', () => {
         ).toBe(CONST.REPORT.PRIMARY_ACTIONS.PAY);
     });
 
+    it('should not return PAY for an expense report when every expense is held', async () => {
+        const report = {
+            reportID: REPORT_ID,
+            type: CONST.REPORT.TYPE.EXPENSE,
+            ownerAccountID: CURRENT_USER_ACCOUNT_ID,
+            statusNum: CONST.REPORT.STATUS_NUM.CLOSED,
+            total: -300,
+        } as unknown as Report;
+        await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
+        const policy = {
+            role: CONST.POLICY.ROLE.ADMIN,
+        };
+        const transaction = {
+            reportID: `${REPORT_ID}`,
+            comment: {
+                hold: 'Hold',
+            },
+        } as unknown as Transaction;
+
+        expect(
+            getReportPrimaryAction({
+                currentUserLogin: CURRENT_USER_EMAIL,
+                currentUserAccountID: CURRENT_USER_ACCOUNT_ID,
+                report,
+                chatReport,
+                reportTransactions: [transaction],
+                violations: {},
+                bankAccountList: {},
+                policy: policy as Policy,
+                isChatReportArchived: false,
+            }),
+        ).not.toBe(CONST.REPORT.PRIMARY_ACTIONS.PAY);
+    });
+
     it('should not return PAY for expense report with only non-reimbursable transactions when total is 0', async () => {
         const report = {
             reportID: REPORT_ID,
@@ -888,6 +919,80 @@ describe('getPrimaryAction', () => {
         ).toBe(CONST.REPORT.PRIMARY_ACTIONS.REMOVE_HOLD);
     });
 
+    it('should return REMOVE HOLD over APPROVE when all expenses are held and the manager can unhold', async () => {
+        const MEMBER_ACCOUNT_ID = 2;
+        const HOLD_ACTION_ID = 'HOLD_ACTION_ID';
+        const REPORT_ACTION_ID = 'REPORT_ACTION_ID';
+        const TRANSACTION_ID = 'TRANSACTION_ID';
+        const CHILD_REPORT_ID = 'CHILD_REPORT_ID';
+
+        const report = {
+            reportID: REPORT_ID,
+            type: CONST.REPORT.TYPE.EXPENSE,
+            ownerAccountID: MEMBER_ACCOUNT_ID,
+            managerID: CURRENT_USER_ACCOUNT_ID,
+            stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
+            statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
+        } as unknown as Report;
+        await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
+
+        const policy = {
+            approver: CURRENT_USER_EMAIL,
+            approvalMode: CONST.POLICY.APPROVAL_MODE.BASIC,
+        } as unknown as Policy;
+
+        const transaction = {
+            transactionID: TRANSACTION_ID,
+            reportID: `${REPORT_ID}`,
+            comment: {
+                hold: HOLD_ACTION_ID,
+            },
+        } as unknown as Transaction;
+
+        const reportAction = {
+            actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
+            type: CONST.REPORT.ACTIONS.TYPE.IOU,
+            reportActionID: REPORT_ACTION_ID,
+            actorAccountID: MEMBER_ACCOUNT_ID,
+            childReportID: CHILD_REPORT_ID,
+            childType: CONST.REPORT.TYPE.CHAT,
+            message: [
+                {
+                    html: 'html',
+                },
+            ],
+            originalMessage: {
+                type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
+                IOUTransactionID: TRANSACTION_ID,
+            },
+        } as unknown as ReportAction;
+
+        // The member created the hold, not the approver, so isRemoveHoldAction is false and the result depends on the all-held redirect
+        const holdAction = {
+            reportActionID: HOLD_ACTION_ID,
+            reportID: CHILD_REPORT_ID,
+            actorAccountID: MEMBER_ACCOUNT_ID,
+        };
+
+        await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {[REPORT_ACTION_ID]: reportAction});
+        await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${CHILD_REPORT_ID}`, {[HOLD_ACTION_ID]: holdAction});
+
+        expect(
+            getReportPrimaryAction({
+                currentUserLogin: CURRENT_USER_EMAIL,
+                currentUserAccountID: CURRENT_USER_ACCOUNT_ID,
+                report,
+                chatReport,
+                reportTransactions: [transaction],
+                violations: {},
+                bankAccountList: {},
+                policy,
+                reportActions: [reportAction],
+                isChatReportArchived: false,
+            }),
+        ).toBe(CONST.REPORT.PRIMARY_ACTIONS.REMOVE_HOLD);
+    });
+
     it('should not return REMOVE HOLD for closed reports with transactions on hold', async () => {
         const report = {
             reportID: REPORT_ID,