Skip to content

[No QA] Remove unsafe type assertions from BulkEditTest.ts tests #96671

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 7 commits into from
Jul 29, 2026
Merged

[No QA] Remove unsafe type assertions from BulkEditTest.ts tests #96671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a04b8e9
fix: publish validated cleanup result
KJ21-ENG Jul 21, 2026
4bf034e
fix: address BulkEditTest review feedback
KJ21-ENG Jul 24, 2026
01ceba3
fix: remove unused test helper exports
KJ21-ENG Jul 24, 2026
2b6d919
fix: sync test helper with main
KJ21-ENG Jul 27, 2026
9dbea05
ci: rerun checks
KJ21-ENG Jul 27, 2026
2c0056b
fix: sync shared test utilities with main
KJ21-ENG Jul 28, 2026
0089b04
fix: inline API write spy type
KJ21-ENG Jul 29, 2026
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
114 changes: 58 additions & 56 deletions tests/actions/IOUTest/BulkEditTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import CONST from '@src/CONST';
import * as API from '@src/libs/API';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Policy, Report, ReportActions} from '@src/types/onyx';
import type {OnyxData} from '@src/types/onyx/Request';
import type Transaction from '@src/types/onyx/Transaction';

import type {OnyxCollection, OnyxKey} from 'react-native-onyx';
import type {OnyxCollection} from 'react-native-onyx';

import Onyx from 'react-native-onyx';

Expand All @@ -16,26 +15,31 @@ import {createRandomReport} from '../../utils/collections/reports';
import createRandomTransaction from '../../utils/collections/transaction';
import createMock from '../../utils/createMock';
import getOnyxValue from '../../utils/getOnyxValue';
import {getRequiredOnyxUpdates, getRequiredWriteCall} from '../../utils/TestHelper';
import {isObject, parseJSONRecord} from '../../utils/typeGuards';
import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates';

const RORY_ACCOUNT_ID = 3;

function isPartialReport(value: unknown): value is Partial<Report> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
return isObject(value) && !Array.isArray(value);
}

function isOnyxData(value: unknown): value is OnyxData<OnyxKey> {
return typeof value === 'object' && value !== null && 'optimisticData' in value;
function getBulkEditUpdates(writeSpy: jest.SpiedFunction<typeof API.write>, callIndex = 0): Record<string, unknown> {
const [, parameters] = getRequiredWriteCall(writeSpy.mock.calls, callIndex);
return parseJSONRecord(parameters.updates, 'bulk-edit updates');
}

function getOptimisticReportNamesFromWriteSpy(writeSpy: jest.SpyInstance, reportID: string): Array<string | undefined> {
function getOptimisticReportNamesFromWriteSpy(writeSpy: jest.SpiedFunction<typeof API.write>, reportID: string): Array<string | undefined> {
const targetKey = `${ONYXKEYS.COLLECTION.REPORT}${reportID}`;
return (writeSpy.mock.calls as ReadonlyArray<readonly unknown[]>).flatMap((call) => {
const onyxData = call.at(2);
if (!isOnyxData(onyxData)) {
return [];
}
return (onyxData.optimisticData ?? []).filter((update) => update.key === targetKey).map((update) => (isPartialReport(update.value) ? update.value.reportName : undefined));
return writeSpy.mock.calls.flatMap((_, callIndex) => {
const [, , onyxData] = getRequiredWriteCall(writeSpy.mock.calls, callIndex);
return getRequiredOnyxUpdates(onyxData, 'optimisticData').flatMap((update) => {
if (!isObject(update) || update.key !== targetKey) {
return [];
}
return [isPartialReport(update.value) ? update.value.reportName : undefined];
});
});
}

Expand Down Expand Up @@ -98,8 +102,7 @@ describe('actions/IOU/BulkEdit', () => {
delegateAccountID: undefined,
});

const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {amount: number};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.amount).toBe(1000);
expect(buildOptimisticSpy).toHaveBeenCalledWith(
transactionThread,
Expand Down Expand Up @@ -235,9 +238,9 @@ describe('actions/IOU/BulkEdit', () => {
});

const getOptimisticTotal = (callIndex: number) => {
const onyxData = writeSpy.mock.calls.at(callIndex)?.[2] as {optimisticData: Array<{key: string; value?: {total?: number}}>};
const reportUpdate = onyxData.optimisticData.find((update) => update.key === `${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`);
return reportUpdate?.value?.total;
const [, , onyxData] = getRequiredWriteCall(writeSpy.mock.calls, callIndex);
const reportUpdate = getRequiredOnyxUpdates(onyxData, 'optimisticData').find((update) => isObject(update) && update.key === `${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`);
return isObject(reportUpdate) && isPartialReport(reportUpdate.value) ? reportUpdate.value.total : undefined;
};

expect(getOptimisticTotal(0)).toBe(-2300);
Expand Down Expand Up @@ -305,8 +308,7 @@ describe('actions/IOU/BulkEdit', () => {
delegateAccountID: undefined,
});

const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {amount: number};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.amount).toBe(-1000);
expect(buildOptimisticSpy).toHaveBeenCalledWith(
transactionThread,
Expand Down Expand Up @@ -381,8 +383,7 @@ describe('actions/IOU/BulkEdit', () => {
delegateAccountID: undefined,
});

const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {billable: boolean; reimbursable: boolean};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.billable).toBe(true);
expect(updates.reimbursable).toBe(false);

Expand Down Expand Up @@ -447,8 +448,7 @@ describe('actions/IOU/BulkEdit', () => {
delegateAccountID: undefined,
});

const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {amount: number};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.amount).toBe(1000);
expect(buildOptimisticSpy).toHaveBeenCalledWith(
transactionThread,
Expand Down Expand Up @@ -513,8 +513,7 @@ describe('actions/IOU/BulkEdit', () => {
});

expect(writeSpy).toHaveBeenCalled();
const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {merchant: string};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.merchant).toBe('New merchant');

writeSpy.mockRestore();
Expand Down Expand Up @@ -1182,8 +1181,7 @@ describe('actions/IOU/BulkEdit', () => {
});

expect(writeSpy).toHaveBeenCalled();
const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as Record<string, unknown>;
const updates = getBulkEditUpdates(writeSpy);
expect(updates.category).toBe('Food');
expect(updates.amount).toBeUndefined();
expect(updates.currency).toBeUndefined();
Expand Down Expand Up @@ -1275,14 +1273,16 @@ describe('actions/IOU/BulkEdit', () => {
// Check the optimistic Onyx data passed to API.write (3rd argument) for the TRANSACTION merge.
const writeCall = writeSpy.mock.calls.at(0);
expect(writeCall).toBeDefined();

const onyxData = writeCall?.[2] as {optimisticData: Array<{key: string; value: Partial<Transaction>}>} | undefined;
const transactionOnyxUpdate = onyxData?.optimisticData?.find((update) => update.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
const [, , onyxData] = getRequiredWriteCall(writeSpy.mock.calls, 0);
const transactionOnyxUpdate = getRequiredOnyxUpdates(onyxData, 'optimisticData').find(
(update) => isObject(update) && update.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
);
expect(transactionOnyxUpdate).toBeDefined();

// The tax code should resolve from the transaction's policy (which has the expense rule),
// NOT from the shared bulk-edit policy (which has no expense rules)
expect(transactionOnyxUpdate?.value?.taxCode).toBe(expectedTaxCode);
const transactionOnyxValue = isObject(transactionOnyxUpdate) && isObject(transactionOnyxUpdate.value) ? transactionOnyxUpdate.value : undefined;
expect(transactionOnyxValue?.taxCode).toBe(expectedTaxCode);

writeSpy.mockRestore();
canEditFieldSpy.mockRestore();
Expand Down Expand Up @@ -1434,15 +1434,17 @@ describe('actions/IOU/BulkEdit', () => {
delegateAccountID: undefined,
});

const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {amount: number; taxAmount?: number};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.amount).toBe(-2000);
expect(updates.taxAmount).toBe(95);

// Optimistic transaction merge should store the flipped sign for expense reports.
const onyxData = writeSpy.mock.calls.at(0)?.[2] as {optimisticData: Array<{key: string; value: Partial<Transaction>}>} | undefined;
const transactionOnyxUpdate = onyxData?.optimisticData?.find((update) => update.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
expect(transactionOnyxUpdate?.value?.taxAmount).toBe(-95);
const [, , onyxData] = getRequiredWriteCall(writeSpy.mock.calls, 0);
const transactionOnyxUpdate = getRequiredOnyxUpdates(onyxData, 'optimisticData').find(
(update) => isObject(update) && update.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
);
const transactionOnyxValue = isObject(transactionOnyxUpdate) && isObject(transactionOnyxUpdate.value) ? transactionOnyxUpdate.value : undefined;
expect(transactionOnyxValue?.taxAmount).toBe(-95);

writeSpy.mockRestore();
canEditFieldSpy.mockRestore();
Expand Down Expand Up @@ -1503,8 +1505,7 @@ describe('actions/IOU/BulkEdit', () => {
delegateAccountID: undefined,
});

const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {amount: number; taxAmount?: number};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.amount).toBe(-2000);
expect(updates.taxAmount).toBeUndefined();

Expand Down Expand Up @@ -1568,8 +1569,7 @@ describe('actions/IOU/BulkEdit', () => {
delegateAccountID: undefined,
});

const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
const updates = JSON.parse(params.updates) as {amount: number; taxAmount?: number};
const updates = getBulkEditUpdates(writeSpy);
expect(updates.amount).toBe(-2000);
// No taxAmount should be queued — we couldn't resolve a rate to recompute from.
expect(updates.taxAmount).toBeUndefined();
Expand Down Expand Up @@ -1602,7 +1602,7 @@ describe('actions/IOU/BulkEdit', () => {
amount: 500,
currency: CONST.CURRENCY.USD,
};
delete (transaction as Partial<Transaction>).transactionThreadReportID;
delete transaction.transactionThreadReportID;

@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/actions/IOUTest/BulkEditTest.ts:1631: clean cast removal, confirmed

delete transaction.transactionThreadReportID; without the old as Partial<Transaction> is correct, transactionThreadReportID is optional on Transaction (src/types/onyx/Transaction.ts:592), so delete is valid here without the cast. Nothing to change, just noting I checked it ✅

const transactions = {
[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]: transaction,
};
Expand All @@ -1628,21 +1628,24 @@ describe('actions/IOU/BulkEdit', () => {
});

expect(writeSpy).toHaveBeenCalled();
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access */
const onyxData = writeSpy.mock.calls.at(0)?.[2] as any;
const optimisticData = onyxData?.optimisticData as any[];
const [, , onyxData] = getRequiredWriteCall(writeSpy.mock.calls, 0);
const optimisticData = getRequiredOnyxUpdates(onyxData, 'optimisticData');

// An optimistic thread report should be created via SET
const optimisticReportSet = optimisticData.find(
(entry: any) => String(entry.key).startsWith(ONYXKEYS.COLLECTION.REPORT) && entry.onyxMethod === 'set' && entry.key !== `${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`,
(entry) =>
isObject(entry) && String(entry.key).startsWith(ONYXKEYS.COLLECTION.REPORT) && entry.onyxMethod === 'set' && entry.key !== `${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`,
);
expect(optimisticReportSet).toBeDefined();
if (!isObject(optimisticReportSet)) {
throw new Error('Expected an optimistic thread report SET update');
}
const optimisticThreadReportID = String(optimisticReportSet.key).replace(ONYXKEYS.COLLECTION.REPORT, '');

// The transaction optimistic data should link back to the new thread via transactionThreadReportID
const transactionMerge = optimisticData.find((entry: any) => entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
expect(transactionMerge?.value?.transactionThreadReportID).toBe(optimisticThreadReportID);
/* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access */
const transactionMerge = optimisticData.find((entry) => isObject(entry) && entry.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
const transactionMergeValue = isObject(transactionMerge) && isObject(transactionMerge.value) ? transactionMerge.value : undefined;
expect(transactionMergeValue?.transactionThreadReportID).toBe(optimisticThreadReportID);

writeSpy.mockRestore();
canEditFieldSpy.mockRestore();
Expand Down Expand Up @@ -1731,24 +1734,23 @@ describe('actions/IOU/BulkEdit', () => {
});

expect(writeSpy).toHaveBeenCalled();
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access */
const onyxData = writeSpy.mock.calls.at(0)?.[2] as any;
const optimisticData = onyxData?.optimisticData as any[];
const [, , onyxData] = getRequiredWriteCall(writeSpy.mock.calls, 0);
const optimisticData = getRequiredOnyxUpdates(onyxData, 'optimisticData');

// No optimistic thread report should be created — the existing thread from childReportID should be used
const optimisticReportSet = optimisticData.find(
(entry: any) => String(entry.key).startsWith(ONYXKEYS.COLLECTION.REPORT) && entry.onyxMethod === 'set' && entry.key !== `${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`,
(entry) =>
isObject(entry) && String(entry.key).startsWith(ONYXKEYS.COLLECTION.REPORT) && entry.onyxMethod === 'set' && entry.key !== `${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`,
);
expect(optimisticReportSet).toBeUndefined();

// The MODIFIED_EXPENSE report action should be written to the childReportID thread, not the transactionThreadReportID thread
const reportActionMerge = optimisticData.find((entry: any) => entry.key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${childReportID}`);
const reportActionMerge = optimisticData.find((entry) => isObject(entry) && entry.key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${childReportID}`);
expect(reportActionMerge).toBeDefined();

// No report action should be written to the transactionThreadReportID thread
const wrongThreadReportAction = optimisticData.find((entry: any) => entry.key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadID}`);
const wrongThreadReportAction = optimisticData.find((entry) => isObject(entry) && entry.key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadID}`);
expect(wrongThreadReportAction).toBeUndefined();
/* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access */

writeSpy.mockRestore();
canEditFieldSpy.mockRestore();
Expand Down Expand Up @@ -2257,7 +2259,7 @@ describe('actions/IOU/BulkEdit', () => {
});

describe('bulk edit draft transaction', () => {
const draftKey = `${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${CONST.IOU.OPTIMISTIC_BULK_EDIT_TRANSACTION_ID}` as OnyxKey;
const draftKey: `${typeof ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${string}` = `${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${CONST.IOU.OPTIMISTIC_BULK_EDIT_TRANSACTION_ID}`;
Comment on lines -2260 to +2262

@ikevin127 ikevin127 Jul 23, 2026 •
edited
Loading

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/actions/IOUTest/BulkEditTest.ts:2284 and the removed eslint-disable blocks — nice

Replacing as OnyxKey with the explicit `${typeof ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${string}` annotation is the right way to do it.

And removing the two /* eslint-disable @typescript-eslint/no-explicit-any ... */ blocks with the as any casts is exactly the point of this issue. This is the strongest part of the PR ✅

@blimpich blimpich Jul 28, 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.

@ikevin127 I don't think this comment adds any value. Can you please keep your inline comments to things that you think should be changed, rather than just noting when something is done correctly? Or explain to me what the value of these comments are?

@ikevin127 ikevin127 Jul 28, 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.

Sure, I'll stop adding comments that don't require changes ✅


it('initializes the bulk edit draft transaction', async () => {
await Onyx.set(draftKey, {amount: 1000});
Expand Down
Loading
Loading