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

[$250] Taxes - "Default" label changes to "Workspace currency default" after changing tax code #48088

Closed
2 of 6 tasks
IuliiaHerets opened this issue Aug 27, 2024 · 23 comments
Closed
2 of 6 tasks
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

@IuliiaHerets
Copy link

IuliiaHerets commented Aug 27, 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: v9.0.25-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Taxes.
  3. Click on the default tax code.
  4. Click Tax code.
  5. Edit tax code and save it.
  6. Note that "Default" label in the default tax code in the background changes to "Workspace currency default" briefly.

Expected Result:

"Default" label in the default tax code in the background will not change after changing tax code.

Actual Result:

"Default" label in the default tax code in the background changes to "Workspace currency default" briefly.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6583912_1724739972875.20240827_142325.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e55f0f655d87efe9
  • Upwork Job ID: 1828439699555129867
  • Last Price Increase: 2024-08-27
  • Automatic offers:
    • etCoderDysto | Contributor | 103702547
    • shubham1206agra | Contributor | 103702642
Issue OwnerCurrent Issue Owner: @akinwale
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Aug 27, 2024
Copy link

melvin-bot bot commented Aug 27, 2024

Triggered auto assignment to @OfstadC (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 Aug 27, 2024

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Aug 27, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Aug 27, 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.

@IuliiaHerets
Copy link
Author

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

@etCoderDysto
Copy link
Contributor

Proposal

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

"Default" label changes to "Workspace currency default" after changing tax code

What is the root cause of that problem?

Here were are not setting foreignTaxDefault to the newTaxCode when editing tax code

function setPolicyTaxCode(policyID: string, oldTaxCode: string, newTaxCode: string) {
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
const originalTaxRate = {...policy?.taxRates?.taxes[oldTaxCode]};
const onyxData: OnyxData = {
optimisticData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
taxRates: {
defaultExternalID: oldTaxCode === policy?.taxRates?.defaultExternalID ? newTaxCode : policy?.taxRates?.defaultExternalID,

And here default badge is only displayed when taxID === defaultExternalID && taxID === foreignTaxDefault condition holds to be true. And it is not being true, since we are not updating foreignTaxDefault
const textForDefault = useCallback(
(taxID: string, taxRate: TaxRate): string => {
let suffix;
if (taxID === defaultExternalID && taxID === foreignTaxDefault) {
suffix = translate('common.default');

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

We should update foreignTaxDefault with the new tax code in optimistic data here

foreignTaxDefault: oldTaxCode === policy?.taxRates?.foreignTaxDefault ? newTaxCode : policy?.taxRates?.foreignTaxDefault,

And set foreignTaxDefault to the existing foreignTaxDefault in failure data

foreignTaxDefault: policy?.taxRates?.foreignTaxDefault,

Also we shouldn't update defaultExternalID in successData since BE returns the updated value

What alternative solutions did you explore? (Optional)

@etCoderDysto
Copy link
Contributor

Offending pr #47819

cc: @rushatgabhane @cead22

@MariaHCD
Copy link
Contributor

@etCoderDysto's proposal looks good to me 👍🏼

@MariaHCD MariaHCD added the External Added to denote the issue can be worked on by a contributor label Aug 27, 2024
@melvin-bot melvin-bot bot changed the title Taxes - "Default" label changes to "Workspace currency default" after changing tax code [$250] Taxes - "Default" label changes to "Workspace currency default" after changing tax code Aug 27, 2024
Copy link

melvin-bot bot commented Aug 27, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01e55f0f655d87efe9

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 27, 2024
Copy link

melvin-bot bot commented Aug 27, 2024

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

@MariaHCD MariaHCD assigned etCoderDysto and unassigned akinwale Aug 27, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 27, 2024
Copy link

melvin-bot bot commented Aug 27, 2024

📣 @etCoderDysto 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@etCoderDysto
Copy link
Contributor

Thanks! I will raise a PR in an hour.

Copy link

melvin-bot bot commented Aug 27, 2024

📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@MariaHCD
Copy link
Contributor

Assigning @shubham1206agra for C+ review

@MariaHCD
Copy link
Contributor

Reassigned the C+ since this was a regression:

https://expensify.slack.com/archives/C02NK2DQWUX/p1724769162552359

@MariaHCD
Copy link
Contributor

@etCoderDysto any updates on the PR just yet?

@etCoderDysto
Copy link
Contributor

@etCoderDysto any updates on the PR just yet?

Sorry, I was on a road. I am working on the PR now. It will be ready for review under 30 minutes.

@puneetlath
Copy link
Contributor

I confirmed this is fixed on staging. Thanks!

@puneetlath puneetlath removed the DeployBlockerCash This issue or pull request should block deployment label Aug 28, 2024
@OfstadC
Copy link
Contributor

OfstadC commented Sep 10, 2024

I think we are just waiting for this to be deployed to production in order to issue payment, but please correct me if I'm wrong.

@etCoderDysto
Copy link
Contributor

@OfstadC this PR was cped on Aug 28. I think the automation is failing. It should be ready for payment.

@OfstadC
Copy link
Contributor

OfstadC commented Sep 10, 2024

Thanks for clarifying @etCoderDysto ! Sorry for the confusion here 😅

Payment Summary

@shubham1206agra
Copy link
Contributor

@OfstadC I did not reviewed the PR. @rushatgabhane did that as it was a regression from his PR.

@rushatgabhane
Copy link
Member

no c+ pay due here 😄

@OfstadC
Copy link
Contributor

OfstadC commented Sep 11, 2024

Ah okay perfect! Thanks for letting me know @shubham1206agra!

@OfstadC OfstadC closed this as completed Sep 11, 2024
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

8 participants