Skip to content

[No QA] Remove unsafe type assertions from ReportSecondaryActionUtilsTest.ts #96777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blimpich merged 1 commit into from
Jul 24, 2026
Merged

[No QA] Remove unsafe type assertions from ReportSecondaryActionUtilsTest.ts #96777

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 65 additions & 62 deletions tests/unit/ReportSecondaryActionUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,14 @@ describe('getSecondaryAction', () => {
});

it('does not include PRINT option when the report is in OPEN state', () => {
const report = {
const report = createMock<Report>({

@ikevin127 ikevin127 Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 tests/unit/ReportSecondaryActionUtilsTest.ts:213: createMock<T> is the right call

Good use of createMock<Report>({...}) instead of {...} as unknown as Report. This actually type-checks the partial against Report (so a typo or wrong value type is caught at the call site) and keeps the single widening assertion isolated in the shared helper.

This is the reuse-a-shared-util direction we discussed on the earlier PRs in this series, so it is nice to see it applied here. Nothing to change ✅

reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
stateNum: CONST.REPORT.STATE_NUM.OPEN,
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
} as unknown as Report;
const policy = {} as unknown as Policy;
});
const policy = createMock<Policy>({});

const result = getSecondaryReportActions({
currentUserLogin: EMPLOYEE_EMAIL,
Expand All @@ -226,7 +226,7 @@ describe('getSecondaryAction', () => {
report,
chatReport,
reportTransactions: [],
originalTransaction: {} as Transaction,
originalTransaction: createMock<Transaction>({}),
violations: {},
bankAccountList: {},
policy,
Expand All @@ -240,14 +240,14 @@ describe('getSecondaryAction', () => {
});

it('includes PRINT option when the report is submitted', () => {
const report = {
const report = createMock<Report>({
reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
} as unknown as Report;
const policy = {} as unknown as Policy;
});
const policy = createMock<Policy>({});

const result = getSecondaryReportActions({
currentUserLogin: EMPLOYEE_EMAIL,
Expand All @@ -256,7 +256,7 @@ describe('getSecondaryAction', () => {
report,
chatReport,
reportTransactions: [],
originalTransaction: {} as Transaction,
originalTransaction: createMock<Transaction>({}),
violations: {},
bankAccountList: {},
policy,
Expand Down Expand Up @@ -2180,19 +2180,19 @@ describe('getSecondaryAction', () => {
});

it('includes RECEIVED_PAYMENT for submitter on Outstanding (Processing) report in Submit workspace', () => {
const report = {
const report = createMock<Report>({
reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
total: -100,
nonReimbursableTotal: 0,
} as unknown as Report;
const policy = {
});
const policy = createMock<Policy>({
type: CONST.POLICY.TYPE.SUBMIT,
role: CONST.POLICY.ROLE.EDITOR,
} as unknown as Policy;
});

const result = getSecondaryReportActions({
currentUserLogin: EMPLOYEE_EMAIL,
Expand All @@ -2201,7 +2201,7 @@ describe('getSecondaryAction', () => {
report,
chatReport,
reportTransactions: [],
originalTransaction: {} as Transaction,
originalTransaction: createMock<Transaction>({}),
violations: {},
bankAccountList: {},
policy,
Expand All @@ -2212,19 +2212,19 @@ describe('getSecondaryAction', () => {
});

it('does not include RECEIVED_PAYMENT when current user did not submit the report (Submit workspace)', () => {
const report = {
const report = createMock<Report>({
reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
ownerAccountID: MANAGER_ACCOUNT_ID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
total: -100,
nonReimbursableTotal: 0,
} as unknown as Report;
const policy = {
});
const policy = createMock<Policy>({
type: CONST.POLICY.TYPE.SUBMIT,
role: CONST.POLICY.ROLE.EDITOR,
} as unknown as Policy;
});

const result = getSecondaryReportActions({
currentUserLogin: EMPLOYEE_EMAIL,
Expand All @@ -2233,7 +2233,7 @@ describe('getSecondaryAction', () => {
report,
chatReport,
reportTransactions: [],
originalTransaction: {} as Transaction,
originalTransaction: createMock<Transaction>({}),
violations: {},
bankAccountList: {},
policy,
Expand All @@ -2244,7 +2244,7 @@ describe('getSecondaryAction', () => {
});

it('does not include RECEIVED_PAYMENT for submitter on Submit workspace report waiting on bank account', () => {
const report = {
const report = createMock<Report>({
reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
Expand All @@ -2253,11 +2253,11 @@ describe('getSecondaryAction', () => {
total: -100,
nonReimbursableTotal: 0,
isWaitingOnBankAccount: true,
} as unknown as Report;
const policy = {
});
const policy = createMock<Policy>({
type: CONST.POLICY.TYPE.SUBMIT,
role: CONST.POLICY.ROLE.EDITOR,
} as unknown as Policy;
});

const result = getSecondaryReportActions({
currentUserLogin: EMPLOYEE_EMAIL,
Expand All @@ -2266,7 +2266,7 @@ describe('getSecondaryAction', () => {
report,
chatReport,
reportTransactions: [],
originalTransaction: {} as Transaction,
originalTransaction: createMock<Transaction>({}),
violations: {},
bankAccountList: {},
policy,
Expand All @@ -2277,23 +2277,23 @@ describe('getSecondaryAction', () => {
});

it('does not include RECEIVED_PAYMENT for submitter on Submit workspace report with held expenses', () => {
const heldTransaction = {
const heldTransaction = createMock<Transaction>({
transactionID: '1',
comment: {hold: 'hold-id'},
} as unknown as Transaction;
const report = {
});
const report = createMock<Report>({
reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
total: -100,
nonReimbursableTotal: 0,
} as unknown as Report;
const policy = {
});
const policy = createMock<Policy>({
type: CONST.POLICY.TYPE.SUBMIT,
role: CONST.POLICY.ROLE.EDITOR,
} as unknown as Policy;
});

const result = getSecondaryReportActions({
currentUserLogin: EMPLOYEE_EMAIL,
Expand All @@ -2302,7 +2302,7 @@ describe('getSecondaryAction', () => {
report,
chatReport,
reportTransactions: [heldTransaction],
originalTransaction: {} as Transaction,
originalTransaction: createMock<Transaction>({}),
violations: {},
bankAccountList: {},
policy,
Expand All @@ -2313,19 +2313,19 @@ describe('getSecondaryAction', () => {
});

it('does not include RECEIVED_PAYMENT for submitter on Outstanding report in non-Submit workspace', () => {
const report = {
const report = createMock<Report>({
reportID: REPORT_ID,
type: CONST.REPORT.TYPE.EXPENSE,
ownerAccountID: EMPLOYEE_ACCOUNT_ID,
stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
total: -100,
nonReimbursableTotal: 0,
} as unknown as Report;
const policy = {
});
const policy = createMock<Policy>({
type: CONST.POLICY.TYPE.TEAM,
role: CONST.POLICY.ROLE.USER,
} as unknown as Policy;
});

const result = getSecondaryReportActions({
currentUserLogin: EMPLOYEE_EMAIL,
Expand All @@ -2334,7 +2334,7 @@ describe('getSecondaryAction', () => {
report,
chatReport,
reportTransactions: [],
originalTransaction: {} as Transaction,
originalTransaction: createMock<Transaction>({}),
violations: {},
bankAccountList: {},
policy,
Expand Down Expand Up @@ -5307,37 +5307,35 @@ describe('getSecondaryTransactionThreadActions', () => {
return acc;
}, createMock<Record<string, Policy>>({}));

type MockFunction = ((...args: unknown[]) => unknown) | boolean;
type IsWorkspaceEligibleForReportChangeMock = (
...args: Parameters<typeof ReportUtils.isWorkspaceEligibleForReportChange>
) => ReturnType<typeof ReportUtils.isWorkspaceEligibleForReportChange>;
type MockConfig = Partial<{
isIOUReport: boolean;
doesReportContainRequestsFromMultipleUsers: boolean;
isCurrentUserSubmitter: boolean;
isReportManager: boolean;
isWorkspaceEligibleForReportChange: MockFunction;
isWorkspaceEligibleForReportChange: boolean | IsWorkspaceEligibleForReportChangeMock;
canEditReportPolicy: boolean;
isExported: boolean;
isSettled: boolean;
}>;

const setupMocks = (mocks: MockConfig = {}) => {

@ikevin127 ikevin127 Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 tests/unit/ReportSecondaryActionUtilsTest.ts:5320-5338: setupMocks unroll looks correct

Unrolling the dynamic jest.spyOn(ReportUtils, method as keyof typeof ReportUtils) loop into explicit per-method spies is the right way to drop the as any and the eslint-disable, since the dynamic-key version could not be typed.

I checked the defaults and they all match the old ones (canEditReportPolicy defaults true, the rest false), and ?? correctly keeps an explicit false from a caller.

No behavior change. Nothing to change here ✅

const defaults = {
isIOUReport: false,
doesReportContainRequestsFromMultipleUsers: false,
isCurrentUserSubmitter: false,
isReportManager: false,
isWorkspaceEligibleForReportChange: true,
canEditReportPolicy: true,
isExported: false,
isSettled: false,
};

for (const [method, value] of Object.entries({...defaults, ...mocks})) {
if (typeof value === 'function') {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
jest.spyOn(ReportUtils, method as keyof typeof ReportUtils).mockImplementation(value as any);
} else {
jest.spyOn(ReportUtils, method as keyof typeof ReportUtils).mockReturnValue(value);
}
jest.spyOn(ReportUtils, 'isIOUReport').mockReturnValue(mocks.isIOUReport ?? false);
jest.spyOn(ReportUtils, 'doesReportContainRequestsFromMultipleUsers').mockReturnValue(mocks.doesReportContainRequestsFromMultipleUsers ?? false);
jest.spyOn(ReportUtils, 'isCurrentUserSubmitter').mockReturnValue(mocks.isCurrentUserSubmitter ?? false);
jest.spyOn(ReportUtils, 'isReportManager').mockReturnValue(mocks.isReportManager ?? false);
jest.spyOn(ReportUtils, 'canEditReportPolicy').mockReturnValue(mocks.canEditReportPolicy ?? true);
jest.spyOn(ReportUtils, 'isExported').mockReturnValue(mocks.isExported ?? false);
jest.spyOn(ReportUtils, 'isSettled').mockReturnValue(mocks.isSettled ?? false);

const workspaceEligibilityMock = jest.spyOn(ReportUtils, 'isWorkspaceEligibleForReportChange');
const workspaceEligibility = mocks.isWorkspaceEligibleForReportChange ?? true;
if (typeof workspaceEligibility === 'function') {
workspaceEligibilityMock.mockImplementation(workspaceEligibility);
} else {
workspaceEligibilityMock.mockReturnValue(workspaceEligibility);
}
};

Expand Down Expand Up @@ -5386,7 +5384,12 @@ describe('getSecondaryTransactionThreadActions', () => {
});

it('should return true when only one available policy and it is different from current report policy', () => {
setupMocks({isWorkspaceEligibleForReportChange: ((_, policy: Policy) => policy?.id === POLICY_ID) as MockFunction});
setupMocks({
isWorkspaceEligibleForReportChange: (
_submitterEmail: Parameters<typeof ReportUtils.isWorkspaceEligibleForReportChange>[0],
policy: Parameters<typeof ReportUtils.isWorkspaceEligibleForReportChange>[1],
Comment on lines +5389 to +5390

@ikevin127 ikevin127 Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 tests/unit/ReportSecondaryActionUtilsTest.ts:5388-5390: the Parameters<...> annotations here are redundant

Since MockConfig already types isWorkspaceEligibleForReportChange as boolean | IsWorkspaceEligibleForReportChangeMock, TypeScript contextually types the callback params when you pass it to setupMocks, so you do not need to annotate each param. This can just be:

setupMocks({
    isWorkspaceEligibleForReportChange: (_submitterEmail, policy) => policy?.id === POLICY_ID,
});

_submitterEmail and policy are still fully typed from the real isWorkspaceEligibleForReportChange signature through IsWorkspaceEligibleForReportChangeMock.

Same type safety, less noise. Minor, not blocking.

) => policy?.id === POLICY_ID,
});
const report = createReport({policyID: OLD_POLICY_ID});
const policies = createPolicies(POLICY_ID, OLD_POLICY_ID);

Expand Down Expand Up @@ -5435,7 +5438,7 @@ describe('getSecondaryTransactionThreadActions', () => {

it('should return true when report is settled and currentUserLogin is admin of available policies', () => {
setupMocks({isSettled: true});
const mockedIsPolicyAdmin = jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin as jest.Mock;
const mockedIsPolicyAdmin = jest.mocked(jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin);
mockedIsPolicyAdmin.mockReturnValue(true);

const report = createReport({policyID: OLD_POLICY_ID});
Expand All @@ -5446,7 +5449,7 @@ describe('getSecondaryTransactionThreadActions', () => {

it('should return false when report is settled and currentUserLogin is not admin of any policy', () => {
setupMocks({isSettled: true});
const mockedIsPolicyAdmin = jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin as jest.Mock;
const mockedIsPolicyAdmin = jest.mocked(jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin);
mockedIsPolicyAdmin.mockReturnValue(false);

const report = createReport({policyID: OLD_POLICY_ID});
Expand All @@ -5457,8 +5460,8 @@ describe('getSecondaryTransactionThreadActions', () => {

it('should filter policies by admin role using currentUserLogin when report is settled', () => {
setupMocks({isSettled: true});
const mockedIsPolicyAdmin = jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin as jest.Mock;
mockedIsPolicyAdmin.mockImplementation((policy: Policy, login?: string) => {
const mockedIsPolicyAdmin = jest.mocked(jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin);
mockedIsPolicyAdmin.mockImplementation((policy, login) => {
return login === ADMIN_EMAIL && policy?.id === POLICY_ID;
});

Expand All @@ -5473,7 +5476,7 @@ describe('getSecondaryTransactionThreadActions', () => {

it('should not filter policies by admin role when report is not settled', () => {
setupMocks({isSettled: false});
const mockedIsPolicyAdmin = jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin as jest.Mock;
const mockedIsPolicyAdmin = jest.mocked(jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin);
mockedIsPolicyAdmin.mockReturnValue(false);

const report = createReport({policyID: OLD_POLICY_ID});
Expand All @@ -5485,7 +5488,7 @@ describe('getSecondaryTransactionThreadActions', () => {

it('should pass currentUserLogin to isPolicyAdmin for each candidate policy when settled', () => {
setupMocks({isSettled: true});
const mockedIsPolicyAdmin = jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin as jest.Mock;
const mockedIsPolicyAdmin = jest.mocked(jest.requireMock<typeof PolicyUtils>('@libs/PolicyUtils').isPolicyAdmin);
mockedIsPolicyAdmin.mockReturnValue(true);

const report = createReport({policyID: OLD_POLICY_ID});
Expand Down
Loading