Expense - Report action shows "changed tax rate to x(previously x)" when there is no rate change

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.78-2
Reproducible in staging?: Yes
Reproducible in production?: N/A - new feature, doesn't exist in prod
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Mac 15.0 / Chrome
App Component: Money Requests

Action Performed:

Precondition:

• Rules and Taxes are enabled.
• There are two tax rates - A and B.
• Tax rate A belongs to Workspace and Foreign currency default.

1. Go to staging.new.expensify.com
2. Go to workspace settings > Categories.
3. Click Advertising category.
   4 Click Default tax rate.
4. Select Tax rate B.
5. Go to workspace chat.
6. Submit an expense with Tax rate B and without category (important).
7. Go to transaction thread.
8. Click Category and select Advertising.

Expected Result:

There should be no system message for tax rate change since the before and after tax rate is still the same (tax rate B) after selecting a category.

Actual Result:

There is system message for tax rate change when there is no changes to the tax rate.
It shows "changed the tax rate to 5% (previously 5%)".

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6701998_1735113525757.20241225_154605.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Triggered auto assignment to @youssef-lr (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[linhvovan29546]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Report action shows "changed tax rate to x(previously x)" when there is no rate change

What is the root cause of that problem?

When update category, we use this function to build optimisticData for report actions when setting a category

App/src/libs/ReportUtils.ts

Lines 3730 to 3736 in 12e0941

	function getModifiedExpenseOriginalMessage(
	oldTransaction: OnyxInputOrEntry<Transaction>,
	transactionChanges: TransactionChanges,
	isFromExpenseReport: boolean,
	policy: OnyxInputOrEntry<Policy>,
	updatedTransaction?: OnyxInputOrEntry<Transaction>,
	): OriginalMessageModifiedExpense {

However, this function does not compare transactionChanges with oldTransaction, which causes the system message to display incorrect text:

App/src/libs/ReportUtils.ts

Lines 3778 to 3779 in 12e0941

	const didTaxCodeChange = 'taxCode' in transactionChanges;
	if (didTaxCodeChange && !didAmountOrCurrencyChange) {

What changes do you think we should make in order to solve the problem?

We need to compare the taxCode of the new transaction with that of the old transaction.

 const didTaxCodeChange = 'taxCode' in transactionChanges && oldTransaction?.taxCode !== transactionChanges.taxCode;

and similar for taxAmount

const didTaxAmountChange = 'taxAmount' in transactionChanges && transactionChanges?.taxAmount !== TransactionUtils.getTaxAmount(oldTransaction, isFromExpenseReport)

POC

Screen.Recording.2024-12-25.at.9.09.20.PM.mp4

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can write automated tests for updateMoneyRequestCategory

What alternative solutions did you explore? (Optional)

We can modify the ModifiedExpenseMessage .buildMessageFragmentForValue function to compare newValue and oldValue. If they are the same, the fragment should not be pushed to changeFragments

App/src/libs/ModifiedExpenseMessage.ts

Lines 67 to 70 in 918488a

	} else {
	const fragment = Localize.translateLocal('iou.updatedTheRequest', {valueName: displayValueName, newValueToDisplay, oldValueToDisplay});
	changeFragments.push(fragment);
	}

[allgandalf]

@linhvovan29546 can you point the offending PR here ?

[linhvovan29546]

@linhvovan29546 can you point the offending PR here ?

@allgandalf The logic originates from PR #42128. And in the PR Apply tax rule when selecting category, we added taxCode and taxAmount based on the selected category, which caused the system message to display incorrectly.

[youssef-lr]

Dupe of #54559

[linhvovan29546]

Dupe of #54559

I think this is not a duplicate 🤔. The expected outcome for this issue is that no system message for the taxRate is displayed, whereas #54559 focuses on consistency between changing, removing, or setting Category and tax system messages