From 85514ccd13bf5d67e44c8807e90d9c57e49af7f2 Mon Sep 17 00:00:00 2001 From: "Rory Abraham (via MelvinBot)" Date: Fri, 10 Apr 2026 19:35:14 +0000 Subject: [PATCH 1/9] Return undefined from getTaxName when taxCode is explicitly empty When tax is deleted via "Delete tax", taxCode is set to ''. Previously, getTaxName used || which treated '' as falsy and fell back to the default tax code, causing the tax rate column to show the default rate instead of being empty. Co-authored-by: Rory Abraham --- src/libs/TransactionUtils/index.ts | 10 ++++++---- tests/unit/TransactionUtilsTest.ts | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 3d4d7f74045a..3366d4201656 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2148,11 +2148,13 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - const defaultTaxCode = getDefaultTaxCode(policy, transaction); + // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"), so return undefined + if (transaction?.taxCode === '') { + return undefined; + } - // transaction?.taxCode may be an empty string - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const taxRate = Object.values(transformedTaxRates(policy, transaction)).find((rate) => rate.code === (transaction?.taxCode || defaultTaxCode)); + const defaultTaxCode = getDefaultTaxCode(policy, transaction); + const taxRate = Object.values(transformedTaxRates(policy, transaction)).find((rate) => rate.code === (transaction?.taxCode ?? defaultTaxCode)); if (shouldFallbackToValue && transaction?.taxValue !== undefined && taxRate?.value !== transaction?.taxValue) { return transaction?.taxValue; diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index b2e480163dd9..8f10169365a1 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -1854,7 +1854,7 @@ describe('TransactionUtils', () => { expect(result).toBe('Tax Rate 1 (5%)'); }); - it('should return default tax name when transaction has empty taxCode', () => { + it('should return empty string when transaction has empty taxCode (tax was deleted)', () => { const transaction = generateTransaction({ taxCode: '', taxValue: undefined, @@ -1862,7 +1862,7 @@ describe('TransactionUtils', () => { const result = TransactionUtils.getTaxRateTitle(policy, transaction, false, undefined); - expect(result).toBe('Tax exempt (0%) • Default'); + expect(result).toBe(''); }); it('should return empty string when policy is undefined', () => { From 32f3689d1ec850436a076b5989d9841a532c8882 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Tue, 21 Apr 2026 09:07:46 +0000 Subject: [PATCH 2/9] Fix: handle undefined/null taxCode after Onyx clears API null response When tax is deleted, the API returns taxCode as null. Onyx removes null values during merge, leaving taxCode as undefined. The previous check only handled taxCode === '' (optimistic), missing the post-API state. Broadened the check to handle all falsy taxCode values. Co-authored-by: Vinh Hoang --- src/libs/TransactionUtils/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 3366d4201656..569ce78b6893 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2148,8 +2148,10 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"), so return undefined - if (transaction?.taxCode === '') { + // A falsy taxCode means tax was explicitly cleared (e.g. via "Delete tax") or was never set. + // After clearing, the API returns null which Onyx removes, leaving taxCode as undefined. + // In all these cases, don't fall back to the default tax rate. + if (!transaction?.taxCode) { return undefined; } From 91a99fd55a78c23ce953e055cdf66c6a5284e2a5 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Tue, 21 Apr 2026 11:07:57 +0000 Subject: [PATCH 3/9] Fix: revert to taxCode === '' check to avoid TaxPicker deselection regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The broad !transaction?.taxCode check also caught undefined taxCode (after API response clears it), which broke the TaxPicker deselection flow — it showed empty instead of the default tax rate. Reverted to checking only taxCode === '' which correctly handles the optimistic state after "Delete tax" without affecting TaxPicker behavior. Co-authored-by: Vinh Hoang --- src/libs/TransactionUtils/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 9ba3eee3b427..c5573b41a596 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2162,10 +2162,10 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - // A falsy taxCode means tax was explicitly cleared (e.g. via "Delete tax") or was never set. - // After clearing, the API returns null which Onyx removes, leaving taxCode as undefined. - // In all these cases, don't fall back to the default tax rate. - if (!transaction?.taxCode) { + // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). + // The ?? operator on line below ensures only null/undefined trigger the default fallback, + // so '' won't match any tax rate and the function returns undefined. + if (transaction?.taxCode === '') { return undefined; } From 92397c29d119a454e683cce6714772fb5cfef050 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Wed, 22 Apr 2026 08:45:37 +0000 Subject: [PATCH 4/9] Revert: restore broad !transaction?.taxCode check Reverts the change from 91a99fd that narrowed the check back to taxCode === ''. The broader !transaction?.taxCode check correctly handles both the optimistic empty-string state and the post-API undefined state after Onyx clears null. Co-authored-by: Vinh Hoang --- src/libs/TransactionUtils/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index c5573b41a596..9ba3eee3b427 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2162,10 +2162,10 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). - // The ?? operator on line below ensures only null/undefined trigger the default fallback, - // so '' won't match any tax rate and the function returns undefined. - if (transaction?.taxCode === '') { + // A falsy taxCode means tax was explicitly cleared (e.g. via "Delete tax") or was never set. + // After clearing, the API returns null which Onyx removes, leaving taxCode as undefined. + // In all these cases, don't fall back to the default tax rate. + if (!transaction?.taxCode) { return undefined; } From f43d9fc0b4f6e4e9bed92838982ebcbd7e7a60b0 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Wed, 22 Apr 2026 14:24:58 +0000 Subject: [PATCH 5/9] Fix: Only treat empty string taxCode as deleted, not undefined MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous guard `!transaction?.taxCode` treated both '' (explicitly deleted) and undefined (never set, e.g. expense created before taxes enabled) the same way — skipping the default tax fallback. This would re-introduce the deploy blocker from Expensify/App#85729 where pre-tax expenses show an empty tax field instead of the default rate. Changed to `transaction?.taxCode === ''` so only explicitly deleted tax codes skip the fallback. Added a regression test for the undefined-taxCode scenario. Co-authored-by: Vinh Hoang --- src/libs/TransactionUtils/index.ts | 7 +++---- tests/unit/TransactionUtilsTest.ts | 11 +++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 1c813b4812c7..2ff4c21c2514 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2167,10 +2167,9 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - // A falsy taxCode means tax was explicitly cleared (e.g. via "Delete tax") or was never set. - // After clearing, the API returns null which Onyx removes, leaving taxCode as undefined. - // In all these cases, don't fall back to the default tax rate. - if (!transaction?.taxCode) { + // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). + // Don't fall back to the default tax rate in that case. + if (transaction?.taxCode === '') { return undefined; } diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index 5cdf6cfff7d2..bd10cfcc5fd1 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -1892,6 +1892,17 @@ describe('TransactionUtils', () => { expect(result).toBe(''); }); + it('should return default tax name when transaction has no taxCode (pre-tax expense)', () => { + const transaction = generateTransaction({ + taxCode: undefined, + taxValue: undefined, + }); + + const result = TransactionUtils.getTaxRateTitle(policy, transaction, false, undefined); + + expect(result).toBe('Tax exempt (0%) • Default'); + }); + it('should return empty string when policy is undefined', () => { const transaction = generateTransaction({ taxCode: 'id_TAX_RATE_1', From 37b9253bde8d1c4c4581e7c11693859673d63807 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Wed, 22 Apr 2026 15:12:34 +0000 Subject: [PATCH 6/9] Fix: also handle null taxCode from API response after tax deletion The UpdateMoneyRequestTaxRate API returns taxCode as null (not empty string) after tax deletion. The previous fix only handled empty string. Now both null and empty string are treated as explicitly cleared tax. Co-authored-by: Vinh Hoang --- src/libs/TransactionUtils/index.ts | 7 ++++--- tests/unit/TransactionUtilsTest.ts | 12 ++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 2ff4c21c2514..8bec793fc531 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2167,9 +2167,10 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). - // Don't fall back to the default tax rate in that case. - if (transaction?.taxCode === '') { + // A null or empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). + // The optimistic update sets taxCode to '', and the API response sets it to null. + // Don't fall back to the default tax rate in either case. + if (transaction?.taxCode === '' || transaction?.taxCode === null) { return undefined; } diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index bd10cfcc5fd1..5515b80ac22b 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -1892,6 +1892,18 @@ describe('TransactionUtils', () => { expect(result).toBe(''); }); + it('should return empty string when transaction has null taxCode (API response after tax deletion)', () => { + const transaction = generateTransaction({ + // The API returns null for taxCode after tax deletion, even though the TS type is string | undefined + taxCode: null as unknown as string, + taxValue: undefined, + }); + + const result = TransactionUtils.getTaxRateTitle(policy, transaction, false, undefined); + + expect(result).toBe(''); + }); + it('should return default tax name when transaction has no taxCode (pre-tax expense)', () => { const transaction = generateTransaction({ taxCode: undefined, From 4e8b6d8a91a0e8350c093ab984839f705560aa41 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Thu, 23 Apr 2026 07:39:53 +0000 Subject: [PATCH 7/9] Remove unnecessary null check for taxCode Onyx removes null values during merge (shouldRemoveNestedNulls), so taxCode will be undefined (not null) after the API response. Only the empty string check is needed for the optimistic update. Co-authored-by: Vinh Hoang --- src/libs/TransactionUtils/index.ts | 8 ++++---- tests/unit/TransactionUtilsTest.ts | 12 ------------ 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 8bec793fc531..ee76374bb40f 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2167,10 +2167,10 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - // A null or empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). - // The optimistic update sets taxCode to '', and the API response sets it to null. - // Don't fall back to the default tax rate in either case. - if (transaction?.taxCode === '' || transaction?.taxCode === null) { + // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). + // Onyx removes null values during merge, so the API response (null) becomes undefined, + // which falls through to the default tax code via ?? below. Only '' needs an early return. + if (transaction?.taxCode === '') { return undefined; } diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index 5515b80ac22b..bd10cfcc5fd1 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -1892,18 +1892,6 @@ describe('TransactionUtils', () => { expect(result).toBe(''); }); - it('should return empty string when transaction has null taxCode (API response after tax deletion)', () => { - const transaction = generateTransaction({ - // The API returns null for taxCode after tax deletion, even though the TS type is string | undefined - taxCode: null as unknown as string, - taxValue: undefined, - }); - - const result = TransactionUtils.getTaxRateTitle(policy, transaction, false, undefined); - - expect(result).toBe(''); - }); - it('should return default tax name when transaction has no taxCode (pre-tax expense)', () => { const transaction = generateTransaction({ taxCode: undefined, From ed3e2442c2b8fa9a32628123f6e6eca50e193090 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Thu, 23 Apr 2026 08:08:54 +0000 Subject: [PATCH 8/9] Preserve taxCode/taxValue in successData after tax deletion The API returns taxCode: null after tax deletion, and Onyx strips null values during merge (shouldRemoveNestedNulls), making taxCode undefined. This is indistinguishable from a pre-tax expense that never had taxCode. Fix: include the optimistic taxCode/taxValue in successData so they persist through the API response merge. This ensures taxCode stays as '' (deleted) rather than becoming undefined (default fallback). Co-authored-by: Vinh Hoang --- src/libs/actions/IOU/UpdateMoneyRequest.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/IOU/UpdateMoneyRequest.ts b/src/libs/actions/IOU/UpdateMoneyRequest.ts index d295003a9b11..7fc5ba388a1e 100644 --- a/src/libs/actions/IOU/UpdateMoneyRequest.ts +++ b/src/libs/actions/IOU/UpdateMoneyRequest.ts @@ -1235,7 +1235,10 @@ function getUpdateMoneyRequestParams(params: GetUpdateMoneyRequestParamsType): U apiParams.attendees = JSON.stringify(apiParams?.attendees); } - // Clear out the error fields and loading states on success + // Clear out the error fields and loading states on success. + // Preserve taxCode/taxValue from the optimistic update because the API may return null for + // cleared tax fields, and Onyx strips null values during merge (shouldRemoveNestedNulls). + // Without this, taxCode would become undefined — indistinguishable from a pre-tax expense. successData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, @@ -1244,6 +1247,8 @@ function getUpdateMoneyRequestParams(params: GetUpdateMoneyRequestParamsType): U isLoading: false, errorFields: null, routes: null, + ...(Object.hasOwn(transactionChanges, 'taxCode') && {taxCode: updatedTransaction?.taxCode}), + ...(Object.hasOwn(transactionChanges, 'taxValue') && {taxValue: updatedTransaction?.taxValue}), }, }); From 759230ac379b19ad0e0b03be1b3639e9f48f9649 Mon Sep 17 00:00:00 2001 From: "Vinh Hoang (via MelvinBot)" Date: Thu, 23 Apr 2026 09:22:33 +0000 Subject: [PATCH 9/9] Remove default taxCode fallback in getTaxName MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of trying to preserve taxCode='' through API round-trips (which fails because Onyx strips null values from the server response), remove the fallback to defaultTaxCode entirely in getTaxName(). Expense creation already sets taxCode to the default explicitly (MoneyRequest.ts:217), so a missing taxCode reliably means tax was deleted or never existed — showing empty is correct in both cases. This also reverts the successData taxCode/taxValue preservation in UpdateMoneyRequest.ts, which is no longer needed. Co-authored-by: Vinh Hoang --- src/libs/TransactionUtils/index.ts | 11 +++++------ src/libs/actions/IOU/UpdateMoneyRequest.ts | 5 ----- tests/unit/TransactionUtilsTest.ts | 4 ++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index ee76374bb40f..a0266a3728fc 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -2167,15 +2167,14 @@ function getWorkspaceTaxesSettingsName(policy: OnyxEntry, taxCode: strin * Gets the name corresponding to the taxCode that is displayed to the user */ function getTaxName(policy: OnyxEntry, transaction: OnyxEntry, shouldFallbackToValue = false) { - // An empty string taxCode means tax was explicitly cleared (e.g. via "Delete tax"). - // Onyx removes null values during merge, so the API response (null) becomes undefined, - // which falls through to the default tax code via ?? below. Only '' needs an early return. - if (transaction?.taxCode === '') { + // Don't fall back to the default tax code when taxCode is empty/undefined. + // Expense creation explicitly sets taxCode to the default (MoneyRequest.ts), + // so a missing taxCode means tax was deleted or never existed — not "use default". + if (!transaction?.taxCode) { return undefined; } - const defaultTaxCode = getDefaultTaxCode(policy, transaction); - const taxRate = Object.values(transformedTaxRates(policy, transaction)).find((rate) => rate.code === (transaction?.taxCode ?? defaultTaxCode)); + const taxRate = Object.values(transformedTaxRates(policy, transaction)).find((rate) => rate.code === transaction.taxCode); if (shouldFallbackToValue && transaction?.taxValue !== undefined && taxRate?.value !== transaction?.taxValue) { return transaction?.taxValue; diff --git a/src/libs/actions/IOU/UpdateMoneyRequest.ts b/src/libs/actions/IOU/UpdateMoneyRequest.ts index 7fc5ba388a1e..52f696d818e5 100644 --- a/src/libs/actions/IOU/UpdateMoneyRequest.ts +++ b/src/libs/actions/IOU/UpdateMoneyRequest.ts @@ -1236,9 +1236,6 @@ function getUpdateMoneyRequestParams(params: GetUpdateMoneyRequestParamsType): U } // Clear out the error fields and loading states on success. - // Preserve taxCode/taxValue from the optimistic update because the API may return null for - // cleared tax fields, and Onyx strips null values during merge (shouldRemoveNestedNulls). - // Without this, taxCode would become undefined — indistinguishable from a pre-tax expense. successData.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, @@ -1247,8 +1244,6 @@ function getUpdateMoneyRequestParams(params: GetUpdateMoneyRequestParamsType): U isLoading: false, errorFields: null, routes: null, - ...(Object.hasOwn(transactionChanges, 'taxCode') && {taxCode: updatedTransaction?.taxCode}), - ...(Object.hasOwn(transactionChanges, 'taxValue') && {taxValue: updatedTransaction?.taxValue}), }, }); diff --git a/tests/unit/TransactionUtilsTest.ts b/tests/unit/TransactionUtilsTest.ts index bd10cfcc5fd1..76c59a912584 100644 --- a/tests/unit/TransactionUtilsTest.ts +++ b/tests/unit/TransactionUtilsTest.ts @@ -1892,7 +1892,7 @@ describe('TransactionUtils', () => { expect(result).toBe(''); }); - it('should return default tax name when transaction has no taxCode (pre-tax expense)', () => { + it('should return empty string when transaction has no taxCode (tax deleted or never set)', () => { const transaction = generateTransaction({ taxCode: undefined, taxValue: undefined, @@ -1900,7 +1900,7 @@ describe('TransactionUtils', () => { const result = TransactionUtils.getTaxRateTitle(policy, transaction, false, undefined); - expect(result).toBe('Tax exempt (0%) • Default'); + expect(result).toBe(''); }); it('should return empty string when policy is undefined', () => {