[payment due 8-27][Track Tax] Support tax tracking when categorizing tracked expense

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.73-0 Reproducible in staging?: y Reproducible in production?: no If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause internal team Slack conversation:

Action Performed:

Precondition:

• Workspace has enabled taxes.

1. Go to staging.new.expensify.com
2. Go to FAB > Track expense.
3. Track a manual expense.
4. In 1:1 DM, click Categorize from the whisper.
5. Select a category.
6. On confirmation page, click Show more.

Expected Result:

Tax rate will be populated in the Tax rate row.

Actual Result:

Tax rate row is empty. After the expense is submitted, the Tax rate row in transaction thread shows the selected tax rate.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/576e9914-ae3d-4f59-a932-b416f9e8e1c1

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0163666547be770dfa
• Upwork Job ID: 1793250352331440128
• Last Price Increase: 2024-05-22

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to @sonialiap (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[bot]]

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

1. If you find which PR caused the issue/bug, you can reassign it to the person responsible for it.
   • If the author is OOO or won’t get online before the daily deploy is due, you are responsible for finding the best fix/path forward. Don’t hesitate to ask for help!
2. Try to reproduce the issue, if the bug is on production, remove the DeployBlocker label but stay assigned to fix it (or find out which PR broke it to get help from the author).
   • You can adjust the urgency of the issue to better represent the gravity of the bug.
   • If the issue is super low priority, feel free to un-assign yourself.
   • Be careful with PHP warnings, sometimes it is more complex than just adding a null coalescing operator as they might be uncovering some bigger bug.
   • If it was a one-off issue that requires no action (for example, Bedrock was down or it is a duplicated issue), you can close it.

Remember rule #2: Never un-assign yourself from a real DeployBlocker unless you are 100% sure someone else is assigned and will take care of it.

[github-actions[bot]]

:wave: 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.

[m-natarajan]

@sonialiap FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[m-natarajan]

We think that this bug might be related to #vip-vsb

[aldo-expensify]

In staging and dev, I see that the transaction created in the conversation with ourselves has taxAmount and taxCode:

These fields get copied to the transaction draft, which I guess then it makes it show no value selected for taxes

Meanwhile, in production, I see that the transaction is missing taxAmount and taxCode

[aldo-expensify]

If I log out and log back in, the problem is gone, which makes me think this is an optimistic data problem

[aldo-expensify]

Deploy blocker caused by: https://github.com/Expensify/App/pull/40443/files#r1599258680

[aldo-expensify]

Created a PR: https://github.com/Expensify/App/pull/42126 I'm not 100% sure if there is something more to do or not, if the default tax is not 0%, the transaction ends up with a negative amount calculated... is that correct?

I tried to test in production to see what is the expected result, but in production the transaction ends up without a tax amount/code even if in the form we showed a tax code as selected, so it is not a useful comparison.

[MonilBhavsar]

Thanks for the investigation Aldo! I'll co assign and take a look

[MonilBhavsar]

I agree with root cause. When creating track expense, we create transaction with taxCode as "" and taxAmount as 0 When MoneyRequestConfirmationList is rendered, we have a useEffect to initialize taxCode and taxAmound fields with value but since taxCode and taxAmount are set, it does not update them. We have two solutions -

1. When categorizing expense, when user selects a policy and if it is policy expense chat, at that moment we update taxCode and taxAmount with defaults, optimistically
2. Do not create track expense with taxRate and taxAmount

Looking into 1, since it aligns with user actions and workflow

[MonilBhavsar]

Removing blocker as this is not only issue with App. The fix also needs to be made in Auth and Web-E https://expensify.slack.com/archives/C01GTK53T8Q/p1715694935703919?thread_ts=1715689873.074619&cid=C01GTK53T8Q

[aldo-expensify]

Hey @MonilBhavsar I think you have a better understanding of how taxes work, so feel free to proceed with the solution you find best: https://github.com/Expensify/App/pull/42151 or https://github.com/Expensify/App/pull/42126. Did you test both? is there one that works better?

[MonilBhavsar]

https://github.com/Expensify/App/pull/42126 looks good to me. Do you want to test it and push it forward for review? We would also need Web-E and Auth updates to make this flow work completely

[MonilBhavsar]

I have added this project to wave collect and Xerocon release.

---

Communicating with Aldo, on how we're tackling this...

[aldo-expensify]

I took a look at the backend and I see now what @MonilBhavsar is saying we are missing.

I'll work tomorrow in completing the Auth PR to use the new params taxCode / taxAmount: https://github.com/Expensify/Auth/pull/10987

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0163666547be770dfa

[melvin-bot[bot]]

Current assignee @eh2077 is eligible for the Internal assigner, not assigning anyone new.

[aldo-expensify]

I'll have to come back to this next week, I've been focusing in higher priority issues

[melvin-bot[bot]]

@sonialiap @MonilBhavsar @aldo-expensify @eh2077 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!

[eh2077]

We're still working on the PR.

cc @aldo-expensify

[melvin-bot[bot]]

@sonialiap, @MonilBhavsar, @aldo-expensify, @eh2077 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[aldo-expensify]

I'll try to get to this tomorrow, was OOO on Monday and I focused today on SharePolicy improvements which is higher priority.

[melvin-bot[bot]]

@sonialiap, @MonilBhavsar, @aldo-expensify, @eh2077 Huh... This is 4 days overdue. Who can take care of this?

[aldo-expensify]

I did some work on the frontend on Friday, but there is still work to do for the tax amount calculated. I haven't worked more in the backend yet,

[aldo-expensify]

Aiming to get back to this tomorrow (Friday)

[melvin-bot[bot]]

@sonialiap, @MonilBhavsar, @aldo-expensify, @eh2077 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[aldo-expensify]

Worked a bit on this on Friday, will try to continue tomorrow

[aldo-expensify]

Ended up re-implementing the front end PR in https://github.com/Expensify/App/pull/43519. This should avoid the bugs of the old PR and does less changes.

[aldo-expensify]

I'll work on the backend tomorrow

[aldo-expensify]

@MonilBhavsar , with the changes from this Auth PR https://github.com/Expensify/Auth/pull/10987/files, I see that the backend returns the amount and taxAmount as negative values

And then in the UI they are displayed as positive values:

I think this is correct, right? Then, when I see the optimistic data in the transactionDraft in the frontend, these values are stored as positive numbers... which maybe is incorrect, is it incorrect?

This becomes visible if we follow the flow of categorizing while offline

cc @eh2077, I may need to do more work in the App PR

[MonilBhavsar]

I think this is correct, right? Then, when I see the optimistic data in the transactionDraft in the frontend, these values are stored as positive numbers... which maybe is incorrect, is it incorrect?

Ah yes, backend stores it as negative amount. We can make the amount as negative in optimistic data. We display absolute value in the UI

[MonilBhavsar]

@aldo-expensify any update here?

[aldo-expensify]

Sorry for the slowness, I'll get to this today or tomorrow. My plan is to fix the frontend so the optimistic tax is calculated using negative values, then make that App PR ready for review again, and then go to the backend.

[aldo-expensify]

Added a fix for the tax amount showing as negative: https://github.com/Expensify/App/pull/43519 and fixed the existing conflicts. I'll work on the backend tomorrow.

[aldo-expensify]

I barely worked after lunch because I'm not feeling well so I couldn't get to this today, I'll try again tomorrow (Thursday).

[aldo-expensify]

@MonilBhavsar do you mind taking over the backend changes (or find someone else to do them)? I just came back to work today and next week I'll be ooo I think the frond end should be ready

[zsgreenwald]

@aldo-expensify can we find someone else while Monil is OOO?

[MonilBhavsar]

I'm back and can work on backend issues

[aldo-expensify]

@aldo-expensify can we find someone else while Monil is OOO?

Ouch, I didn't notice @MonilBhavsar was OOO. I'm back too now.

[aldo-expensify]

I resumed working on this, I'll try to have the backend ready before making the App PR ready for review again to try to avoid having to resolve conflicts more times.

[aldo-expensify]

Made backend PRs ready for review:

• https://github.com/Expensify/Auth/pull/10987
• https://github.com/Expensify/Web-Expensify/pull/42115

[aldo-expensify]

App PR was approved by @eh2077, waiting on internal engineer approval now.

[melvin-bot[bot]]

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

[sonialiap]

@Christinadobrzyn I'm OOO Aug 19-30, adding leave buddy. Status: getting close to payment, waiting for reviews to be completed

[Christinadobrzyn]

awesome! moving this back to weekly to track for payment.

[Christinadobrzyn]

looks like the PR is in staging so getting close

[Christinadobrzyn]

The payment summary didn't trigger, I think this is due for payment today.

Payouts due:

• [ ] Contributor+: $250 @eh2077 (offer - https://www.upwork.com/nx/wm/offer/103755823)

@eh2077 are you paid via Upwork or NewDot? Do we need a regression test for this?

[Christinadobrzyn]

nuged @eh2077 can you confirm - https://github.com/Expensify/App/issues/42114#issuecomment-2313466286