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

[HOLD for payment 2024-09-09][$250] Track tax - Tax rate is no longer assigned to the distance rate after changing the tax code #45861

Closed
6 tasks done
lanitochka17 opened this issue Jul 21, 2024 · 47 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 21, 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: 9.0.10-2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

  • Workspace is under Control plan.
  • Distance rates and Taxes features are enabled.
  • Track tax is enabled.
  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Distance rates
  3. Click on any distance rate
  4. Click Tax rate
  5. Assign a tax rate
  6. Go to Taxes
  7. Click on the tax rate in Step 5
  8. Click Tax code
  9. Edit tax code and save it
  10. Go back to Distance rates
  11. Click on the distance rate from Step 3

Expected Result:

The tax rate will remain assigned to the distance rate after changing the tax code

Actual Result:

The tax rate is no longer assigned to the distance rate after changing the tax code

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

Bug6548331_1721504216281.20240721_033014.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01487f230385609e0a
  • Upwork Job ID: 1815359905732178734
  • Last Price Increase: 2024-07-22
Issue OwnerCurrent Issue Owner: @abdulrahuman5196
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jul 21, 2024
Copy link

melvin-bot bot commented Jul 21, 2024

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

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.

@lanitochka17
Copy link
Author

We think that this bug might be related to #wave-control

@mountiny
Copy link
Contributor

Does this work in production or in OldDot? We might have to optimistically change the tax rate code in the distance rates to make this work, but I wonder if we do this in the backend even.

AS this applies for Control policies, I think we could treat this as NAB

@mountiny mountiny added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 DeployBlocker Indicates it should block deploying the API labels Jul 22, 2024
@mountiny
Copy link
Contributor

I dont think this is a blocker as its quite a minor case, but we should explore it, marking as external

@mountiny mountiny added the External Added to denote the issue can be worked on by a contributor label Jul 22, 2024
Copy link

melvin-bot bot commented Jul 22, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01487f230385609e0a

@melvin-bot melvin-bot bot changed the title Track tax - Tax rate is no longer assigned to the distance rate after changing the tax code [$250] Track tax - Tax rate is no longer assigned to the distance rate after changing the tax code Jul 22, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 22, 2024
Copy link

melvin-bot bot commented Jul 22, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 (External)

@mountiny
Copy link
Contributor

Asked if this works well in prod or if this is a new feature

@daledah
Copy link
Contributor

daledah commented Jul 22, 2024

Proposal

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

The tax rate is no longer assigned to the distance rate after changing the tax code

What is the root cause of that problem?

When updating the tax code here

function setPolicyTaxCode(policyID: string, oldTaxCode: string, newTaxCode: string) {
, we're not updating the taxRateExternalID of the distance rates (example usage of taxRateExternalID here
const taxRateExternalID = rate.attributes?.taxRateExternalID;
), so the taxRateExternalID of those distance rates remain the old code, which no longer exists.

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

In

function setPolicyTaxCode(policyID: string, oldTaxCode: string, newTaxCode: string) {
, go through the list of policy distance rates, check if the taxRateExternalID of the distance rate is equal to the old tax code, if true we should create optimistic data to update that taxRateExternalID to the new tax code.

We also need to revert to the old taxRateExternalID in failureData

What alternative solutions did you explore? (Optional)

@mountiny mountiny added the DeployBlockerCash This issue or pull request should block deployment label Jul 22, 2024
Copy link

melvin-bot bot commented Jul 22, 2024

Current assignee @marcochavezf is eligible for the DeployBlockerCash assigner, not assigning anyone new.

@github-actions github-actions bot added Hourly KSv2 and removed Daily KSv2 labels Jul 22, 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.

@mountiny
Copy link
Contributor

@daledah Can you please confirm which PR has caused this regression?

@mountiny
Copy link
Contributor

QA confirmed this works well in production so given that (I thought its more of a new feature) we should fix this

@dangrous
Copy link
Contributor

dangrous commented Aug 5, 2024

@rushatgabhane can you review @daledah's proposal and we can go from there? Thanks!

@daledah
Copy link
Contributor

daledah commented Aug 12, 2024

@rushatgabhane friendly bump

@rushatgabhane
Copy link
Member

@dangrous do we need to do this optimistically?

@daledah what happens when taxCode update fails, do we reset the optimistic data for distance rate?

Can a race condition happen here?

@daledah
Copy link
Contributor

daledah commented Aug 12, 2024

@rushatgabhane I mentioned the case when taxCode update fails in my proposal

We also need to revert to the old taxRateExternalID in failureData.

@rushatgabhane
Copy link
Member

@dangrous let's hire @daledah for their proposal
🎀 👀 🎀

Copy link

melvin-bot bot commented Aug 12, 2024

Current assignee @dangrous is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@dangrous
Copy link
Contributor

Assigned!

Copy link

melvin-bot bot commented Aug 12, 2024

📣 @daledah You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

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

daledah commented Aug 15, 2024

@rushatgabhane This PR is ready for review.

@daledah
Copy link
Contributor

daledah commented Aug 19, 2024

friendly bump @rushatgabhane

@dangrous
Copy link
Contributor

dangrous commented Sep 4, 2024

I think this was deployed to prod on 2024-09-02 but we should confirm and adjust payment timing as needed

@dangrous dangrous added the Bug Something is broken. Auto assigns a BugZero manager. label Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

Triggered auto assignment to @garrettmknight (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 Sep 6, 2024
@dangrous dangrous changed the title [$250] Track tax - Tax rate is no longer assigned to the distance rate after changing the tax code [HOLD for payment 2024-09-09][$250] Track tax - Tax rate is no longer assigned to the distance rate after changing the tax code Sep 6, 2024
@dangrous dangrous added Weekly KSv2 and removed Daily KSv2 labels Sep 6, 2024
@dangrous
Copy link
Contributor

dangrous commented Sep 6, 2024

Automation didn't fire but this seems to have been deployed on 9/2, so payment on 9/9. Grabbed you @garrettmknight for when payment is ready!

Copy link

melvin-bot bot commented Sep 9, 2024

Payment Summary

Upwork Job

BugZero Checklist (@garrettmknight)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1815359905732178734/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@JmillsExpensify
Copy link

JmillsExpensify commented Sep 11, 2024

I went ahead and filled out the payment summary above, though repeating here for posterity:

@garrettmknight
Copy link
Contributor

@daledah offer sent in Upwork (linking here)

@garrettmknight
Copy link
Contributor

$250 approved for @rushatgabhane

@garrettmknight
Copy link
Contributor

All paid up

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. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants