Web - Taxes - RBR shown on expense preview without any errors in the transaction thread

[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.64-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #52309
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Navigate to staging.new.expensify.com
2. Create a workspace
3. Enable tax feature
4. Add a tax
5. Create an expense with the added tax on step 4

Expected Result:

No RBR on the expense

Actual Result:

RBR shown without any errors in the transaction thread

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6669095_1731995117189.bandicam_2024-11-19_08-38-33-101.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @JmillsExpensify

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (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.

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-11-19 10:21:29 UTC.

Proposal


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

Web - Taxes - RBR shown on expense preview without any errors in the transaction thread

What is the root cause of that problem?

• In hasNoticeTypeViolation we aren't checking for ['taxAmountChanged', 'taxRateChanged'], these notice types shouldn't be used in the NewDot as mentioned here.
  

  App/src/libs/TransactionUtils/index.ts

  Lines 874 to 881 in f77087f

  	/**
  	* Checks if any violations for the provided transaction are of type 'notice'
  	*/
  	function hasNoticeTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>, showInReview?: boolean): boolean {
  	return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some(
  	(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.NOTICE && (showInReview === undefined || showInReview === (violation.showInReview ?? false)),
  	);
  	}

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


• We should update hasNoticeTypeViolation to check if the violation name is taxAmountChanged' or 'taxRateChanged' and only proceed to the next check if it is not equal to taxAmountChanged' or 'taxRateChanged'.

function hasNoticeTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>, showInReview?: boolean): boolean {
    // We don't want to show these violations on NewDot
    const excludedViolationsName = ['taxAmountChanged', 'taxRateChanged'];
    return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some(
        (violation: TransactionViolation) =>
            !excludedViolationsName.includes(violation.name) &&
            violation.type === CONST.VIOLATION_TYPES.NOTICE &&
            (showInReview === undefined || showInReview === (violation.showInReview ?? false)),
    );

• If needed, we can also add this check in other violation check util functions like hasWarningTypeViolation, hasViolation.

What alternative solutions did you explore? (Optional)

Result

[melvin-bot]

@JmillsExpensify Eep! 4 days overdue now. Issues have feelings too...

[JmillsExpensify]

I actually forget if this is by design. @MonilBhavsar can you clarify?

[Krishna2323]

@JmillsExpensify, it was decided to not show taxRateChanged & taxAmountChanged.

Please see these comments: #41401 (comment) #41649 (comment)

[MonilBhavsar]

#41401 (comment) #41649 (comment)

Reading through these comments.

Looks like we no longer display taxRateChanged and taxAmountChanged notice violation because we have system messages and reviewer/approver can know if it has changed.
That sounds good!

If so, we need to stop sending these violations from server at all for newDot requests and not add any conditional logic on the client side. Also remove the code we have currently

[Krishna2323]

Also remove the code we have currently

@MonilBhavsar, I can work on removing the conditional logic from the frontend if needed.

[JmillsExpensify]

Nice, thanks both for the refresher. Agree with those comments.

[MonilBhavsar]

Working on this today

[melvin-bot]

@JmillsExpensify @MonilBhavsar this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MonilBhavsar]

Submitted PR's for review

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.72-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Cleanup tax violations logic and migrate it to server #53459

If no regressions arise, payment will be issued on 2024-12-16. 🎊

[melvin-bot]

@MonilBhavsar @JmillsExpensify @MonilBhavsar The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[MonilBhavsar]

no payment is due here. Closing