search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.54k stars 2.88k forks source link

Tax - Incorrect tax name in editor after creating a new rate with the old name of existing tax #47447

Closed lanitochka17 closed 2 months ago

lanitochka17 commented 2 months ago

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.20-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Create a new workspace
  3. Upgrade the workspace to Control by going to More features > Report fields > Enable > Upgrade
  4. Enable Taxes feature
  5. Go to Taxes
  6. Add a new tax rate with name A and save it
  7. Click on the tax rate with name A
  8. Change the name to name B and also tax code B (very important)
  9. Add another tax rate with name A and save it
  10. Click on the new tax rate added in Step 10 (the one with name A)
  11. Note that the tax rate editor shows the name of tax rate with name B when tax rate with name A is opened
  12. Click on Name field
  13. Note that the name is name A even though the editor shows name B on the header and Name field

Expected Result:

In Step 12, the correct tax rate name in editor should be displayed after creating a new tax rate with the old tax rate name

Actual Result:

In Step 12, the incorrect tax rate name in editor is displayed after creating a new tax rate with the old tax rate name In Step 14, when opening tax rate name editor, it displays the correct name even though the previous page shows the incorrect name

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/805a8b67-4fca-4dbf-a932-86491a2ac937

View all open jobs on GitHub

melvin-bot[bot] commented 2 months ago

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

lanitochka17 commented 2 months ago

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

lanitochka17 commented 2 months ago

@joekaufmanexpensify 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

nkdengineer commented 2 months ago

Proposal

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

In Step 12, the incorrect tax rate name in editor is displayed after creating a new tax rate with the old tax rate name In Step 14, when opening tax rate name editor, it displays the correct name even though the previous page shows the incorrect name

What is the root cause of that problem?

When we edit the tax code, the previousTaxCode is added to the optimistic data to store the previous tax code name and used to fix the not found page when we go back to the tax detail page. But we don't remove it in success data.

https://github.com/Expensify/App/blob/cfa84e21c1a5540f798f3fdfd76ecbd64a546704/src/libs/actions/TaxRate.ts#L503

Then when we get the currentTaxID here, it returns the first tax that we created and edited the tax code because it's the first item matches the condition in getCurrentTaxID function here

https://github.com/Expensify/App/blob/cfa84e21c1a5540f798f3fdfd76ecbd64a546704/src/pages/workspace/taxes/WorkspaceEditTaxPage.tsx#L38

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

We should reset the previousTaxCode in successData here

previousTaxCode: null

https://github.com/Expensify/App/blob/cfa84e21c1a5540f798f3fdfd76ecbd64a546704/src/libs/actions/TaxRate.ts#L523

Maybe the bug still happens if we do the same case in offline mode, so we also need to update getCurrentTaxID function to prioritize the tax that has taxIDKey is the taxID. If it doesn't exist then we will find the tax that has the previousTaxCode is taxID https://github.com/Expensify/App/blob/cfa84e21c1a5540f798f3fdfd76ecbd64a546704/src/libs/PolicyUtils.ts#L959-L960

What alternative solutions did you explore? (Optional)

joekaufmanexpensify commented 2 months ago

Hmm, I can reproduce this, but the reproduction steps feel very niche. I am not convinced this is a real case that a customer would experience...If someone adds a tax code and then changes that tax code AND tax rate, why would they then immediately go and add another separate tax code to replace the one they just changed away from?

They would just add a new tax code if they wanted to add one in addition to the one they've already created.

joekaufmanexpensify commented 2 months ago

Further, this only happens if you change both the tax rate and tax code. If it's only the code, there's no issue. And even if you do experience it, if you change the tax rate again, the issue disappears.

joekaufmanexpensify commented 2 months ago

Going to close this for now, as I don't think this is something we need to prioritize changing. We can revisit if any actual customers experience this. Let me know if anyone disagrees!