Skip to content
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.

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

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

Closed
6 tasks done
m-natarajan opened this issue May 13, 2024 · 55 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@m-natarajan
Copy link

m-natarajan commented May 13, 2024

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?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6479872_1715632444443.20240514_042952.mp4

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
@m-natarajan m-natarajan added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels May 13, 2024
Copy link

melvin-bot bot commented May 13, 2024

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

Copy link

melvin-bot bot commented May 13, 2024

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.

Copy link

melvin-bot bot commented May 13, 2024

👋 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.

@melvin-bot melvin-bot bot added Engineering Hourly KSv2 and removed Hourly KSv2 labels May 13, 2024
@github-actions github-actions bot added Hourly KSv2 and removed Daily KSv2 labels May 13, 2024
Copy link
Contributor

👋 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
Copy link
Author

@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
Copy link
Author

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

@aldo-expensify
Copy link
Contributor

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

image

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

image

@aldo-expensify
Copy link
Contributor

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

@aldo-expensify
Copy link
Contributor

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

@aldo-expensify aldo-expensify mentioned this issue May 14, 2024
50 tasks
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels May 14, 2024
@aldo-expensify
Copy link
Contributor

aldo-expensify commented May 14, 2024

Created a PR: #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.

@aldo-expensify aldo-expensify added Hourly KSv2 and removed Weekly KSv2 labels May 14, 2024
@MonilBhavsar MonilBhavsar removed the DeployBlocker Indicates it should block deploying the API label May 14, 2024
@aldo-expensify
Copy link
Contributor

@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
Copy link
Contributor

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

@MonilBhavsar
Copy link
Contributor

I'm back and can work on backend issues

@aldo-expensify
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

@aldo-expensify
Copy link
Contributor

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

@sonialiap sonialiap added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

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.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 16, 2024
@sonialiap
Copy link
Contributor

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

@Christinadobrzyn
Copy link
Contributor

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

@Christinadobrzyn Christinadobrzyn added Weekly KSv2 and removed Daily KSv2 labels Aug 16, 2024
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Aug 20, 2024

looks like the PR is in staging so getting close

@Christinadobrzyn Christinadobrzyn changed the title [Track Tax] Support tax tracking when categorizing tracked expense [payment due 8-27][Track Tax] Support tax tracking when categorizing tracked expense Aug 27, 2024
@Christinadobrzyn Christinadobrzyn added Daily KSv2 and removed Weekly KSv2 labels Aug 27, 2024
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Aug 27, 2024

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

Payouts due:

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

@Christinadobrzyn
Copy link
Contributor

nuged @eh2077 can you confirm - #42114 (comment)

@eh2077
Copy link
Contributor

eh2077 commented Aug 29, 2024

@Christinadobrzyn Thanks for the ping. I'd like to pay via upwork. Yes, I think we need to add regression test for this issue.

@eh2077
Copy link
Contributor

eh2077 commented Aug 29, 2024

I'm away from keyboard and I'll complete the checklist asap, thanks 🙏

@Christinadobrzyn
Copy link
Contributor

Thanks @eh2077 I just send you an Upwork offer too.

@sonialiap
Copy link
Contributor

sonialiap commented Sep 2, 2024

Thanks @Christinadobrzyn ! I'm back from OOO and can take this back, feel free to keep on or unassign yourself

@eh2077 I've approved the payment ✔️
Please complete the checklist once you get a chance :D

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@eh2077] The PR that introduced the bug has been identified. Link to the PR:
  • [@eh2077] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@eh2077] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@eh2077] Determine if we should create a regression test for this bug.
  • [@eh2077] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@eh2077
Copy link
Contributor

eh2077 commented Sep 2, 2024

Checklist

  • [@eh2077] The PR that introduced the bug has been identified. Link to the PR: I can't a PR directly caused this issue. The task ended up a larger scope than the original issue.
  • [@eh2077] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
  • [@eh2077] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
  • [@eh2077] Determine if we should create a regression test for this bug. Yes

Regression test

==== Prepare ====

  1. Create a workspace
  2. Set the workspace default currency to USD if it is not already that
  3. Go to "More features" and enable "Distance rates" and "Taxes"
  4. Go to "Taxes" and define 3 tax rates:
    • Tax USD40: 40%
    • Tax other currency10: 10%
    • Tax distance25: 25%
  5. Still in "Taxes", click in "Settings" in the top and choose the following defaults:
  1. Verify you end up with a configuration like this:
  1. Go "Distance rates" and enable "Track tax" in the top right settings
  2. Create a distance rate, assign a tax rate to it and input some amount smaller than the rate in "Tax reclaimable on":

==== case 1 ====

  1. In the DM chat with yourself, click "+ > Track expense" and create an expense with the same currency as the workspace currency
  1. Wait for the concierge message with actions and click "Categorize it"
  1. The RHN should open with the workspace as the only alternative, select the workspace
  2. Choose any category from the list
  3. In the expense form, click "Show more"
  4. Verify that the tax code is set to the default tax code for expenses with the currency of the workspace
  5. Verify that the tax amount looks correct
  6. Change the tax code manually
  7. Verify that the tax amount is updated

==== case 2 ====

  1. Dismiss the RHN clicking outside
  2. In the DM chat with yourself, click "+ > Track expense" and create an expense with a different currency
  3. Wait for the concierge message with actions and click "Categorize it"
  4. The RHN should open with the workspace as the only alternative, select the workspace
  5. Choose any category from the list
  6. In the expense form, click "Show more"
  7. Verify that the tax code is set to the default tax code for expenses with a different currency of the workspace

==== case 3 ====

  1. Dismiss the RHN clicking outside
  2. In the DM chat with yourself, click "+ > Track expense" and create a distance request
  3. Wait for the concierge message with actions and click "Categorize it"
  4. The RHN should open with the workspace as the only alternative, select the workspace
  5. Choose any category from the list
  6. Verify that the tax code is set to the default tax code for the distance rate selected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
No open projects
Status: Done
Development

No branches or pull requests

7 participants