Skip to content

Hide tax fields for time and per diem expenses #87612

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
youssef-lr merged 3 commits into from
Apr 13, 2026
Merged

Hide tax fields for time and per diem expenses #87612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4aca2df
Hide tax fields for time and per diem expenses in MoneyRequestView
MelvinBot Apr 10, 2026
06fddc4
Move time/perDiem guard from shouldShowTax to shouldShowTaxDisabledAlert
MelvinBot Apr 10, 2026
5aa0800
Skip TAX_OUT_OF_POLICY violation for time and per diem expenses
MelvinBot Apr 11, 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
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ function MoneyRequestView({
const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => (pendingAction ? undefined : transaction?.pendingFields?.[fieldPath]);

const isTaxEnabled = isTaxTrackingEnabled(isPolicyExpenseChat || isExpenseUnreported, policy, isDistanceRequest, isPerDiemRequest, isTimeRequest);
const shouldShowTaxDisabledAlert = !isTaxEnabled && !!transaction?.taxCode;
const shouldShowTaxDisabledAlert = !isTaxEnabled && !!transaction?.taxCode && !isTimeRequest && !isPerDiemRequest;

@hoangzinh hoangzinh Apr 12, 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.

@MelvinBot Could you please add a comment to explain this condition?

@MelvinBot MelvinBot Apr 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

youssef-lr should I add this comment? The condition skips time and per diem expenses because isTaxTrackingEnabled returns false for those types, which would otherwise incorrectly trigger the tax-disabled alert when the transaction has a taxCode from a tax-enabled workspace.

I can only make code changes when directed by the PR author, an assignee, or an existing co-author. To have me implement this, the PR assignee or author should reply with a request tagging me.

const shouldShowTax = isTaxEnabled || !!transaction?.taxName || shouldShowTaxDisabledAlert;

let amountDescription = `${translate('iou.amount')}`;
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Violations/ViolationsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ const ViolationsUtils = {
}

const hasTransactionTaxData = !!updatedTransaction.taxCode || !!updatedTransaction.taxValue || !!updatedTransaction.taxAmount;
const shouldAddTaxOutOfPolicy = isPolicyTrackTaxEnabled ? !isTaxInPolicy : hasTransactionTaxData;
const shouldAddTaxOutOfPolicy = !isTimeRequest && !isPerDiemRequest && (isPolicyTrackTaxEnabled ? !isTaxInPolicy : hasTransactionTaxData);

@hoangzinh hoangzinh Apr 12, 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.

@MelvinBot Could you please add a comment to explain this condition?

@MelvinBot MelvinBot Apr 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

youssef-lr should I add this comment? The condition skips time and per diem expenses because isTaxTrackingEnabled returns false for those types — without this guard, the hasTransactionTaxData fallback would incorrectly generate a TAX_OUT_OF_POLICY violation whenever these expense types have a taxCode set from a tax-enabled workspace.

I can only make code changes when directed by the PR author, an assignee, or an existing co-author. To have me implement this, the PR assignee or author should reply with a request tagging me.


if (!hasTaxOutOfPolicyViolation && shouldAddTaxOutOfPolicy) {
newTransactionViolations.push({name: CONST.VIOLATIONS.TAX_OUT_OF_POLICY, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true});
Expand Down
28 changes: 28 additions & 0 deletions tests/ui/MoneyRequestViewTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,34 @@ describe('MoneyRequestView edit fields', () => {
});
});

it('should not show tax fields for time expenses even when transaction has tax data', async () => {
const threadReport = {
...LHNTestUtils.getFakeReport(),
parentReportID: expenseReportID,
parentReportActionID,
};

await setupTestData();

await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {
taxCode: 'TAX_10',
taxAmount: 500,
taxValue: '10%',
comment: {type: CONST.TRANSACTION.TYPE.TIME},
});
});
await waitForBatchedUpdatesWithAct();

renderMoneyRequestView(threadReport, {tax: {trackingEnabled: true}});
await waitForBatchedUpdatesWithAct();

await waitFor(() => {
expect(screen.queryByTestId('menu-item-common.tax')).not.toBeOnTheScreen();
expect(screen.queryByTestId('menu-item-iou.taxAmount')).not.toBeOnTheScreen();
});
});

it('should show amount and merchant as readonly when report is settled', async () => {
const threadReport = {
...LHNTestUtils.getFakeReport(),
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/ViolationUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1166,6 +1166,33 @@ describe('getViolationsOnyxData', () => {
expect(result.value).not.toContainEqual(taxOutOfPolicyViolation);
});
});

describe('time and per diem requests', () => {
it('should not add taxOutOfPolicy violation for time requests even with tax data and tax tracking disabled', () => {
policy.tax = {trackingEnabled: false};
transaction.taxCode = 'SOME_TAX';
transaction.comment = {...transaction.comment, type: CONST.TRANSACTION.TYPE.TIME};
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false, false);
expect(result.value).not.toContainEqual(taxOutOfPolicyViolation);
});

it('should not add taxOutOfPolicy violation for per diem requests even with tax data and tax tracking disabled', () => {
policy.tax = {trackingEnabled: false};
transaction.taxCode = 'SOME_TAX';
transaction.comment = {...transaction.comment, type: CONST.TRANSACTION.TYPE.CUSTOM_UNIT, customUnit: {name: CONST.CUSTOM_UNITS.NAME_PER_DIEM_INTERNATIONAL}};
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false, false);
expect(result.value).not.toContainEqual(taxOutOfPolicyViolation);
});

it('should not add taxOutOfPolicy violation for time requests even with invalid taxCode and tax tracking enabled', () => {
policy.tax = {trackingEnabled: true};
transaction.taxCode = 'UNKNOWN_TAX';
transaction.comment = {...transaction.comment, type: CONST.TRANSACTION.TYPE.TIME};
policy.taxRates = {name: 'Taxes', defaultExternalID: 'TAX_10', defaultValue: '10%', foreignTaxDefault: 'TAX_10', taxes: {TAX_10: {name: '10%', value: '10%'}}};
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false, false);
expect(result.value).not.toContainEqual(taxOutOfPolicyViolation);
});
});
});

describe('overTripLimit violation', () => {
Expand Down
Loading