Skip to content

Commit 4aa41e4

Browse files
authored
Merge pull request Expensify#88528 from callstack-internal/bulk-edit-bug-tax-offline
fix: bulk edit, recompute taxAmount on amount-only bulk edit offline
2 parents 996d334 + 9e0d16f commit 4aa41e4

2 files changed

Lines changed: 204 additions & 0 deletions

File tree

src/libs/actions/IOU/BulkEdit.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,19 @@ function updateMultipleMoneyRequests({
168168
if (changes.amount !== undefined && canEditField(CONST.EDIT_REQUEST_FIELD.AMOUNT)) {
169169
transactionChanges.amount = changes.amount;
170170
}
171+
// When bulk-editing amount on a taxed expense without also changing taxCode, recompute
172+
// taxAmount from the transaction's existing taxCode so offline optimistic data and the
173+
// queued payload stay in sync with the new amount. Skip when the rate can't be resolved
174+
// (e.g. cross-policy bulk edit where the transaction's own policy is missing from cache)
175+
// to avoid silently overwriting a non-zero taxAmount with 0.
176+
if (changes.amount !== undefined && !changes.taxCode && transaction.taxCode && supportsExpenseFields && canEditField(CONST.EDIT_REQUEST_FIELD.TAX_RATE)) {
177+
const taxValue = getTaxValue(transactionPolicy, transaction, transaction.taxCode);
178+
if (taxValue) {
179+
const decimals = getCurrencyDecimals(getCurrency(transaction));
180+
const taxAmount = calculateTaxAmount(taxValue, Math.abs(changes.amount), decimals);
181+
transactionChanges.taxAmount = convertToBackendAmount(taxAmount);
182+
}
183+
}
171184
if (changes.currency && canEditField(CONST.EDIT_REQUEST_FIELD.CURRENCY)) {
172185
transactionChanges.currency = changes.currency;
173186
}

tests/actions/IOUTest/BulkEditTest.ts

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,6 +1242,197 @@ describe('actions/IOU/BulkEdit', () => {
12421242
buildOptimisticSpy.mockRestore();
12431243
canEditFieldSpy.mockRestore();
12441244
});
1245+
1246+
it('recalculates taxAmount when bulk-editing only amount on a taxed expense', () => {
1247+
const transactionID = 'transaction-tax-amount-only';
1248+
const iouReportID = 'iou-tax-amount-only';
1249+
const taxCode = 'id_TAX_RATE_1';
1250+
const policy: Policy = {
1251+
...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM),
1252+
taxRates: CONST.DEFAULT_TAX,
1253+
};
1254+
1255+
const iouReport: Report = {
1256+
...createRandomReport(2, undefined),
1257+
reportID: iouReportID,
1258+
policyID: policy.id,
1259+
type: CONST.REPORT.TYPE.EXPENSE,
1260+
};
1261+
1262+
const reports = {
1263+
[`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`]: iouReport,
1264+
};
1265+
1266+
const transaction: Transaction = {
1267+
...createRandomTransaction(1),
1268+
transactionID,
1269+
reportID: iouReportID,
1270+
amount: -1000,
1271+
currency: CONST.CURRENCY.USD,
1272+
taxCode,
1273+
taxAmount: -48,
1274+
};
1275+
const transactions = {
1276+
[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]: transaction,
1277+
};
1278+
1279+
const canEditFieldSpy = jest.spyOn(require('@libs/ReportUtils'), 'canEditFieldOfMoneyRequest').mockReturnValue(true);
1280+
// eslint-disable-next-line rulesdir/no-multiple-api-calls
1281+
const writeSpy = jest.spyOn(API, 'write').mockImplementation(jest.fn());
1282+
1283+
updateMultipleMoneyRequests({
1284+
transactionIDs: [transactionID],
1285+
changes: {amount: -2000},
1286+
policy,
1287+
reports,
1288+
transactions,
1289+
reportActions: {},
1290+
policyCategories: undefined,
1291+
policyTags: {},
1292+
hash: undefined,
1293+
introSelected: undefined,
1294+
betas: undefined,
1295+
});
1296+
1297+
const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
1298+
const updates = JSON.parse(params.updates) as {amount: number; taxAmount?: number};
1299+
expect(updates.amount).toBe(-2000);
1300+
expect(updates.taxAmount).toBe(95);
1301+
1302+
// Optimistic transaction merge should store the flipped sign for expense reports.
1303+
const onyxData = writeSpy.mock.calls.at(0)?.[2] as {optimisticData: Array<{key: string; value: Partial<Transaction>}>} | undefined;
1304+
const transactionOnyxUpdate = onyxData?.optimisticData?.find((update) => update.key === `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
1305+
expect(transactionOnyxUpdate?.value?.taxAmount).toBe(-95);
1306+
1307+
writeSpy.mockRestore();
1308+
canEditFieldSpy.mockRestore();
1309+
});
1310+
1311+
it('does not inject taxAmount when bulk-editing amount on a non-tax expense', () => {
1312+
// A transaction that has no taxCode must not have a taxAmount clobbered to 0 just
1313+
// because the user bulk-edited the amount — that would flip a non-tax expense into
1314+
// a zero-tax expense offline.
1315+
const transactionID = 'transaction-no-tax';
1316+
const iouReportID = 'iou-no-tax';
1317+
const policy: Policy = {
1318+
...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM),
1319+
taxRates: CONST.DEFAULT_TAX,
1320+
};
1321+
1322+
const iouReport: Report = {
1323+
...createRandomReport(2, undefined),
1324+
reportID: iouReportID,
1325+
policyID: policy.id,
1326+
type: CONST.REPORT.TYPE.EXPENSE,
1327+
};
1328+
1329+
const reports = {
1330+
[`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`]: iouReport,
1331+
};
1332+
1333+
const transaction: Transaction = {
1334+
...createRandomTransaction(1),
1335+
transactionID,
1336+
reportID: iouReportID,
1337+
amount: -1000,
1338+
currency: CONST.CURRENCY.USD,
1339+
taxCode: '',
1340+
taxAmount: undefined,
1341+
};
1342+
const transactions = {
1343+
[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]: transaction,
1344+
};
1345+
1346+
const canEditFieldSpy = jest.spyOn(require('@libs/ReportUtils'), 'canEditFieldOfMoneyRequest').mockReturnValue(true);
1347+
// eslint-disable-next-line rulesdir/no-multiple-api-calls
1348+
const writeSpy = jest.spyOn(API, 'write').mockImplementation(jest.fn());
1349+
1350+
updateMultipleMoneyRequests({
1351+
transactionIDs: [transactionID],
1352+
changes: {amount: -2000},
1353+
policy,
1354+
reports,
1355+
transactions,
1356+
reportActions: {},
1357+
policyCategories: undefined,
1358+
policyTags: {},
1359+
hash: undefined,
1360+
introSelected: undefined,
1361+
betas: undefined,
1362+
});
1363+
1364+
const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
1365+
const updates = JSON.parse(params.updates) as {amount: number; taxAmount?: number};
1366+
expect(updates.amount).toBe(-2000);
1367+
expect(updates.taxAmount).toBeUndefined();
1368+
1369+
writeSpy.mockRestore();
1370+
canEditFieldSpy.mockRestore();
1371+
});
1372+
1373+
it('does not clobber taxAmount when the transaction taxCode is unknown to the policy', () => {
1374+
// Cross-policy bulk edit / stale cache: the transaction has a taxCode that doesn't
1375+
// exist in the resolved policy's taxRates. getTaxValue returns undefined and
1376+
// calculateTaxAmount short-circuits to 0 — we must not write that 0 over a non-zero
1377+
// existing taxAmount.
1378+
const transactionID = 'transaction-unresolvable-tax';
1379+
const iouReportID = 'iou-unresolvable-tax';
1380+
const policy: Policy = {
1381+
...createRandomPolicy(1, CONST.POLICY.TYPE.TEAM),
1382+
taxRates: CONST.DEFAULT_TAX,
1383+
};
1384+
1385+
const iouReport: Report = {
1386+
...createRandomReport(2, undefined),
1387+
reportID: iouReportID,
1388+
policyID: policy.id,
1389+
type: CONST.REPORT.TYPE.EXPENSE,
1390+
};
1391+
1392+
const reports = {
1393+
[`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`]: iouReport,
1394+
};
1395+
1396+
const transaction: Transaction = {
1397+
...createRandomTransaction(1),
1398+
transactionID,
1399+
reportID: iouReportID,
1400+
amount: -1000,
1401+
currency: CONST.CURRENCY.USD,
1402+
taxCode: 'id_TAX_RATE_NOT_IN_POLICY',
1403+
taxAmount: -48,
1404+
};
1405+
const transactions = {
1406+
[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]: transaction,
1407+
};
1408+
1409+
const canEditFieldSpy = jest.spyOn(require('@libs/ReportUtils'), 'canEditFieldOfMoneyRequest').mockReturnValue(true);
1410+
// eslint-disable-next-line rulesdir/no-multiple-api-calls
1411+
const writeSpy = jest.spyOn(API, 'write').mockImplementation(jest.fn());
1412+
1413+
updateMultipleMoneyRequests({
1414+
transactionIDs: [transactionID],
1415+
changes: {amount: -2000},
1416+
policy,
1417+
reports,
1418+
transactions,
1419+
reportActions: {},
1420+
policyCategories: undefined,
1421+
policyTags: {},
1422+
hash: undefined,
1423+
introSelected: undefined,
1424+
betas: undefined,
1425+
});
1426+
1427+
const params = writeSpy.mock.calls.at(0)?.[1] as {updates: string};
1428+
const updates = JSON.parse(params.updates) as {amount: number; taxAmount?: number};
1429+
expect(updates.amount).toBe(-2000);
1430+
// No taxAmount should be queued — we couldn't resolve a rate to recompute from.
1431+
expect(updates.taxAmount).toBeUndefined();
1432+
1433+
writeSpy.mockRestore();
1434+
canEditFieldSpy.mockRestore();
1435+
});
12451436
});
12461437

12471438
describe('bulk edit draft transaction', () => {

0 commit comments

Comments
 (0)