Skip to content

Commit 5d48c19

Browse files
iwizniaiwiznia
authored
Merge pull request #92941 from hungvu193/fix-92870
[Submit] Prevent submitter from approving their report in submit ws
2 parents b50623f + 2eca854 commit 5d48c19

11 files changed

Lines changed: 301 additions & 5 deletions

‎config/eslint/eslint.seatbelt.tsv‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@
400400
"../../src/components/Search/chartGroupByConfig.ts" "@typescript-eslint/no-unsafe-type-assertion" 20
401401
"../../src/components/Search/hooks/useOptimisticSearchTracking.ts" "@typescript-eslint/no-unsafe-type-assertion" 4
402402
"../../src/components/Search/hooks/useStableOptimisticSortedData.ts" "@typescript-eslint/no-unsafe-type-assertion" 1
403-
"../../src/components/Search/index.tsx" "@typescript-eslint/no-unsafe-type-assertion" 11
403+
"../../src/components/Search/index.tsx" "@typescript-eslint/no-unsafe-type-assertion" 14
404404
"../../src/components/Search/index.tsx" "react-hooks/preserve-manual-memoization" 2
405405
"../../src/components/Search/index.tsx" "react-hooks/refs" 5
406406
"../../src/components/Search/index.tsx" "react-hooks/set-state-in-effect" 1
@@ -1586,7 +1586,7 @@
15861586
"../../tests/actions/ReplaceOptimisticReportWithActualReportTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 3
15871587
"../../tests/actions/ReportFieldTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 1
15881588
"../../tests/actions/ReportFieldTest.ts" "no-restricted-imports" 1
1589-
"../../tests/actions/ReportPreviewActionUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 28
1589+
"../../tests/actions/ReportPreviewActionUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 30
15901590
"../../tests/actions/ReportPreviewActionUtilsTest.ts" "no-restricted-imports" 2
15911591
"../../tests/actions/ReportTest.ts" "@typescript-eslint/no-deprecated/buildNextStepNew" 1
15921592
"../../tests/actions/ReportTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 80
@@ -1825,7 +1825,7 @@
18251825
"../../tests/unit/ReportActionsUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 35
18261826
"../../tests/unit/ReportLayoutUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 1
18271827
"../../tests/unit/ReportNameUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 23
1828-
"../../tests/unit/ReportPrimaryActionUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 175
1828+
"../../tests/unit/ReportPrimaryActionUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 181
18291829
"../../tests/unit/ReportPrimaryActionUtilsTest.ts" "no-restricted-imports" 1
18301830
"../../tests/unit/ReportSecondaryActionUtilsTest.ts" "@typescript-eslint/no-unsafe-type-assertion" 466
18311831
"../../tests/unit/ReportSecondaryActionUtilsTest.ts" "no-restricted-imports" 2

‎src/libs/PolicyUtils.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,18 @@ function isSubmitPolicyByType(policyType: string | undefined): boolean {
11301130
return policyType === CONST.POLICY.TYPE.SUBMIT;
11311131
}
11321132

1133+
/**
1134+
* Checks if the submitter's approval is blocked on the submit workspace.
1135+
*
1136+
* @param policy - The policy to check
1137+
* @param reportOwnerAccountID - The account ID of the report owner
1138+
* @param approverAccountID - The account ID of the approver
1139+
* @returns True if the submitter's approval is blocked on the submit workspace, false otherwise
1140+
*/
1141+
function isSubmitterApproveBlockedOnSubmitWorkspace(policy: OnyxInputOrEntry<Policy>, reportOwnerAccountID: number | undefined, approverAccountID: number): boolean {
1142+
return isSubmitPolicy(policy) && reportOwnerAccountID === approverAccountID;
1143+
}
1144+
11331145
/**
11341146
* We only allow users to access Submit feature if they have the SUBMIT_2026 beta enabled.
11351147
*
@@ -2661,6 +2673,7 @@ export {
26612673
canAccessSubmitWorkspaceFeatures,
26622674
getRulesDocumentSourceURL,
26632675
isSubmitPolicy,
2676+
isSubmitterApproveBlockedOnSubmitWorkspace,
26642677
};
26652678

26662679
export type {MemberEmailsToAccountIDs, PolicyFeature, PolicyFeatureAccess};

‎src/libs/ReportPreviewActionUtils.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,15 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
22
import type {ValueOf} from 'type-fest';
33
import CONST from '@src/CONST';
44
import type {BankAccountList, Policy, Report, ReportMetadata, Transaction, TransactionViolation} from '@src/types/onyx';
5-
import {arePaymentsEnabled, getSubmitToAccountID, getValidConnectedIntegration, hasDynamicExternalWorkflow, hasIntegrationAutoSync, isPreferredExporter} from './PolicyUtils';
5+
import {
6+
arePaymentsEnabled,
7+
getSubmitToAccountID,
8+
getValidConnectedIntegration,
9+
hasDynamicExternalWorkflow,
10+
hasIntegrationAutoSync,
11+
isPreferredExporter,
12+
isSubmitterApproveBlockedOnSubmitWorkspace,
13+
} from './PolicyUtils';
614
import {hasPendingDEWApprove} from './ReportActionsUtils';
715
import {isAddExpenseAction} from './ReportPrimaryActionUtils';
816
import {
@@ -61,6 +69,10 @@ function canSubmit(
6169
}
6270

6371
function canApprove(report: Report, currentUserAccountID: number, reportMetadata: OnyxEntry<ReportMetadata>, policy?: Policy, transactions?: Transaction[]) {
72+
if (isSubmitterApproveBlockedOnSubmitWorkspace(policy, report.ownerAccountID, currentUserAccountID)) {
73+
return false;
74+
}
75+
6476
const isExpense = isExpenseReport(report);
6577
const isProcessing = isProcessingReport(report);
6678
const isApprovalEnabled = policy?.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL;

‎src/libs/ReportPrimaryActionUtils.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
isPolicyAdmin as isPolicyAdminPolicyUtils,
1515
isPolicyApprover,
1616
isPreferredExporter,
17+
isSubmitterApproveBlockedOnSubmitWorkspace,
1718
} from './PolicyUtils';
1819
import {
1920
getAllReportActions,
@@ -154,6 +155,10 @@ function isSubmitAction(
154155
}
155156

156157
function isApproveAction(report: Report, reportTransactions: Transaction[], currentUserAccountID: number, reportMetadata: OnyxEntry<ReportMetadata>, policy?: Policy) {
158+
if (isSubmitterApproveBlockedOnSubmitWorkspace(policy, report.ownerAccountID, currentUserAccountID)) {
159+
return false;
160+
}
161+
157162
const isAnyReceiptBeingScanned = reportTransactions?.some((transaction) => isScanning(transaction));
158163

159164
if (isAnyReceiptBeingScanned) {

‎src/libs/ReportSecondaryActionUtils.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
isPreferredExporter,
3232
isSubmitAndClose,
3333
isSubmitPolicy,
34+
isSubmitterApproveBlockedOnSubmitWorkspace,
3435
} from './PolicyUtils';
3536
import {
3637
getAllReportActions,
@@ -293,6 +294,10 @@ function isApproveAction(
293294
reportMetadata: OnyxEntry<ReportMetadata>,
294295
policy?: Policy,
295296
): boolean {
297+
if (isSubmitterApproveBlockedOnSubmitWorkspace(policy, report.ownerAccountID, currentUserAccountID)) {
298+
return false;
299+
}
300+
296301
const isAnyReceiptBeingScanned = reportTransactions?.some((transaction) => isReceiptBeingScanned(transaction));
297302

298303
if (isAnyReceiptBeingScanned) {

‎src/libs/ReportUtils.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ import {
156156
isPolicyAuditor,
157157
isPolicyOwner,
158158
isSubmitAndClose,
159+
isSubmitterApproveBlockedOnSubmitWorkspace,
159160
shouldShowPolicy,
160161
} from './PolicyUtils';
161162
import type {LastVisibleMessage} from './ReportActionsUtils';
@@ -10993,7 +10994,13 @@ function isReportOwner(report: OnyxInputOrEntry<Report>): boolean {
1099310994
function isAllowedToApproveExpenseReport(report: OnyxEntry<Report>, approverAccountID?: number, reportPolicy?: OnyxEntry<Policy>): boolean {
1099410995
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
1099510996
const policy = reportPolicy ?? getPolicy(report?.policyID);
10996-
const isOwner = (approverAccountID ?? deprecatedCurrentUserAccountID) === report?.ownerAccountID;
10997+
const accountID = approverAccountID ?? deprecatedCurrentUserAccountID;
10998+
const isOwner = accountID === report?.ownerAccountID;
10999+
11000+
if (isSubmitterApproveBlockedOnSubmitWorkspace(policy, report?.ownerAccountID, accountID ?? CONST.DEFAULT_NUMBER_ID)) {
11001+
return false;
11002+
}
11003+
1099711004
return !(policy?.preventSelfApproval && isOwner);
1099811005
}
1099911006

‎tests/actions/ReportPreviewActionUtilsTest.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,81 @@ describe('getReportPreviewAction', () => {
534534
}),
535535
).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
536536
});
537+
538+
it('should return VIEW instead of APPROVE when submitter is the manager on a Submit workspace', async () => {
539+
const report = {
540+
...createRandomReport(REPORT_ID, undefined),
541+
type: CONST.REPORT.TYPE.EXPENSE,
542+
ownerAccountID: CURRENT_USER_ACCOUNT_ID,
543+
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
544+
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
545+
managerID: CURRENT_USER_ACCOUNT_ID,
546+
isWaitingOnBankAccount: false,
547+
};
548+
549+
const policy = createRandomPolicy(0, CONST.POLICY.TYPE.SUBMIT);
550+
policy.approvalMode = CONST.POLICY.APPROVAL_MODE.ADVANCED;
551+
policy.preventSelfApproval = false;
552+
553+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
554+
const transaction = {
555+
reportID: `${REPORT_ID}`,
556+
amount: 100,
557+
merchant: 'Test Merchant',
558+
created: '2025-01-01',
559+
} as unknown as Transaction;
560+
561+
expect(
562+
getReportPreviewAction({
563+
isReportArchived: false,
564+
currentUserAccountID: CURRENT_USER_ACCOUNT_ID,
565+
currentUserLogin: CURRENT_USER_EMAIL,
566+
report,
567+
policy,
568+
transactions: [transaction],
569+
bankAccountList: {},
570+
reportMetadata: undefined,
571+
}),
572+
).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.VIEW);
573+
});
574+
575+
it('should return APPROVE when a different user is the manager on a Submit workspace', async () => {
576+
const approverAccountID = CURRENT_USER_ACCOUNT_ID + 1;
577+
const report = {
578+
...createRandomReport(REPORT_ID, undefined),
579+
type: CONST.REPORT.TYPE.EXPENSE,
580+
ownerAccountID: CURRENT_USER_ACCOUNT_ID,
581+
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
582+
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
583+
managerID: approverAccountID,
584+
isWaitingOnBankAccount: false,
585+
};
586+
587+
const policy = createRandomPolicy(0, CONST.POLICY.TYPE.SUBMIT);
588+
policy.approvalMode = CONST.POLICY.APPROVAL_MODE.ADVANCED;
589+
policy.preventSelfApproval = false;
590+
591+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
592+
const transaction = {
593+
reportID: `${REPORT_ID}`,
594+
amount: 100,
595+
merchant: 'Test Merchant',
596+
created: '2025-01-01',
597+
} as unknown as Transaction;
598+
599+
expect(
600+
getReportPreviewAction({
601+
isReportArchived: false,
602+
currentUserAccountID: approverAccountID,
603+
currentUserLogin: 'approver@mail.com',
604+
report,
605+
policy,
606+
transactions: [transaction],
607+
bankAccountList: {},
608+
reportMetadata: undefined,
609+
}),
610+
).toBe(CONST.REPORT.REPORT_PREVIEW_ACTIONS.APPROVE);
611+
});
537612
});
538613

539614
it("canApprove should return true for the current report manager regardless of whether they're in the current approval workflow", async () => {

‎tests/unit/PolicyUtilsTest.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
hasPolicyWithXeroConnection,
4444
hasVendorFeature,
4545
isPolicyMemberWithoutPendingDelete,
46+
isSubmitterApproveBlockedOnSubmitWorkspace,
4647
shouldShowPolicy,
4748
sortPoliciesByName,
4849
sortWorkspacesBySelected,
@@ -2981,6 +2982,28 @@ describe('PolicyUtils', () => {
29812982
expect(result['bob@acme.com']).toBeUndefined();
29822983
});
29832984
});
2985+
describe('isSubmitterApproveBlockedOnSubmitWorkspace', () => {
2986+
const submitPolicy: Policy = {...createRandomPolicy(99000, CONST.POLICY.TYPE.SUBMIT), id: 'policy-submit-approve-block-test'};
2987+
const teamPolicy: Policy = {...createRandomPolicy(99001, CONST.POLICY.TYPE.TEAM), id: 'policy-team-approve-block-test'};
2988+
const submitterAccountID = 100;
2989+
2990+
it('returns true when policy is Submit and the approver is the report owner', () => {
2991+
expect(isSubmitterApproveBlockedOnSubmitWorkspace(submitPolicy, submitterAccountID, submitterAccountID)).toBe(true);
2992+
});
2993+
2994+
it('returns false when policy is Submit and the approver is not the report owner', () => {
2995+
expect(isSubmitterApproveBlockedOnSubmitWorkspace(submitPolicy, submitterAccountID, approverAccountID)).toBe(false);
2996+
});
2997+
2998+
it('returns false when policy is not Submit even if the approver is the report owner', () => {
2999+
expect(isSubmitterApproveBlockedOnSubmitWorkspace(teamPolicy, submitterAccountID, submitterAccountID)).toBe(false);
3000+
});
3001+
3002+
it('returns false when report owner account ID is undefined', () => {
3003+
expect(isSubmitterApproveBlockedOnSubmitWorkspace(submitPolicy, undefined, submitterAccountID)).toBe(false);
3004+
});
3005+
});
3006+
29843007
describe('canAccessSubmitWorkspaceFeatures', () => {
29853008
const submitPolicyForAccessTest: Policy = {...createRandomPolicy(99001, CONST.POLICY.TYPE.SUBMIT), id: 'policy-submit-access-test'};
29863009
const teamPolicyForAccessTest: Policy = {...createRandomPolicy(99002, CONST.POLICY.TYPE.TEAM), id: 'policy-team-access-test'};

‎tests/unit/ReportPrimaryActionUtilsTest.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,55 @@ describe('getPrimaryAction', () => {
426426
expect(isApproveAction(report, [transaction], CURRENT_USER_ACCOUNT_ID, {}, policy)).toBe(true);
427427
});
428428

429+
it('should return false from isApproveAction when submitter views their own report on a Submit workspace', async () => {
430+
const report = {
431+
reportID: REPORT_ID,
432+
type: CONST.REPORT.TYPE.EXPENSE,
433+
ownerAccountID: CURRENT_USER_ACCOUNT_ID,
434+
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
435+
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
436+
managerID: CURRENT_USER_ACCOUNT_ID,
437+
} as unknown as Report;
438+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
439+
const policy = {
440+
type: CONST.POLICY.TYPE.SUBMIT,
441+
approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED,
442+
} as unknown as Policy;
443+
const transaction = {
444+
reportID: `${REPORT_ID}`,
445+
amount: 10,
446+
merchant: 'Merchant',
447+
date: '2025-01-01',
448+
} as unknown as Transaction;
449+
450+
expect(isApproveAction(report, [transaction], CURRENT_USER_ACCOUNT_ID, {}, policy)).toBe(false);
451+
});
452+
453+
it('should return true from isApproveAction when approver views a report on a Submit workspace', async () => {
454+
const approverAccountID = CURRENT_USER_ACCOUNT_ID + 1;
455+
const report = {
456+
reportID: REPORT_ID,
457+
type: CONST.REPORT.TYPE.EXPENSE,
458+
ownerAccountID: CURRENT_USER_ACCOUNT_ID,
459+
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
460+
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
461+
managerID: approverAccountID,
462+
} as unknown as Report;
463+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
464+
const policy = {
465+
type: CONST.POLICY.TYPE.SUBMIT,
466+
approvalMode: CONST.POLICY.APPROVAL_MODE.ADVANCED,
467+
} as unknown as Policy;
468+
const transaction = {
469+
reportID: `${REPORT_ID}`,
470+
amount: 10,
471+
merchant: 'Merchant',
472+
date: '2025-01-01',
473+
} as unknown as Transaction;
474+
475+
expect(isApproveAction(report, [transaction], approverAccountID, {}, policy)).toBe(true);
476+
});
477+
429478
it('should return false from isApproveAction for DEW policy report with pending approval', async () => {
430479
// Given a submitted expense report on a DEW policy with a pending approval action
431480
const report = {

0 commit comments

Comments
 (0)