Skip to content

[No QA] update previousTaxCode in setPolicyTaxCode successData #94286

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.

Sign up for GitHub

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

Merged
MonilBhavsar merged 3 commits into from
Jul 9, 2026
Merged

[No QA] update previousTaxCode in setPolicyTaxCode successData #94286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
996e18e
update previousTaxCode in setPolicyTaxCode successData
dukenv0307 Jun 23, 2026
3b4b3ca
use optimisticPreviousTaxCode in getCurrentTaxID
dukenv0307 Jun 28, 2026
68fdb73
Merge branch 'main' into fix/94003
dukenv0307 Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libs/PolicyUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2456,7 +2456,7 @@ function hasOnlyPersonalPolicies(policies: OnyxCollection<Policy>) {
}

function getCurrentTaxID(policy: OnyxEntry<Policy>, taxID: string): string | undefined {
return Object.keys(policy?.taxRates?.taxes ?? {}).find((taxIDKey) => policy?.taxRates?.taxes?.[taxIDKey].previousTaxCode === taxID || taxIDKey === taxID);
return Object.keys(policy?.taxRates?.taxes ?? {}).find((taxIDKey) => policy?.taxRates?.taxes?.[taxIDKey].optimisticPreviousTaxCode === taxID || taxIDKey === taxID);
}

function getTagApproverRule(policy: OnyxEntry<Policy>, tagName: string) {
Expand Down
3 changes: 3 additions & 0 deletions src/libs/actions/TaxRate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ function setPolicyTaxCode(
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
errorFields: {code: null},
previousTaxCode: oldTaxCode,
optimisticPreviousTaxCode: oldTaxCode,
},
},
},
Expand All @@ -612,6 +613,8 @@ function setPolicyTaxCode(
pendingFields: {...originalTaxRate.pendingFields, code: null},
pendingAction: null,
errorFields: {code: null},
previousTaxCode: oldTaxCode,

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale tax-code aliases after success

Keeping previousTaxCode in the confirmed Onyx state makes the old code a permanent alias for this rate. If the user later reuses that old code for another tax, validateTaxCode/isExistingTaxCode only reject existing object keys, and the old key was removed; then WorkspaceEditTaxPage calls getCurrentTaxID(), which can return this renamed rate via previousTaxCode before it reaches the real reused-code key. In that scenario the edit route for the newly reused code redirects to this renamed tax, so the new tax cannot be edited or deleted correctly.

Useful? React with 👍 / 👎.

@dukenv0307 dukenv0307 Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced optimisticPreviousTaxCode to fix this issue cc @truph01

@dmkt9 dmkt9 Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dukenv0307 I think we should update the previousTaxCode value for the policy in snapshot_ as well.

@dukenv0307 dukenv0307 Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmkt9 Do you mean update it optimistically or in BE?

@dmkt9 dmkt9 Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I mean we should update both.

@dukenv0307 dukenv0307 Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give me the test steps to reproduce the bug (if we don't update snapshot)? I'll update it in the follow-up PR

@dmkt9 dmkt9 Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. You can use my PR (#94007) for testing. Please refer to Test 4:

  1. Create a workspace.
  2. Enable Taxes and navigate to the Taxes page.
  3. Create two new tax rates (e.g., Tax A and Tax B).
  4. Open Tax A.
  5. Verify that the Tax Code field is empty.
  6. Click the Tax Code field, enter any value, and save the changes.
  7. Verify that the Tax Code field now contains a value.
  8. Create a couple of expenses using Tax A and Tax B.
  9. Navigate to Spend > Expenses.
  10. Enable the Tax Code column.
  11. Note that the Tax Code column displays empty values for those expenses

Expected Result:
Verify that the Tax Code column displays a value for expenses using Tax A, and is empty for expenses using Tax B.

@dukenv0307 dukenv0307 Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MelvinBot can you please create the new issue in E/E repo?

@dukenv0307 dukenv0307 Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmkt9 I can't reproduce, can you please check again? previousTaxCode is already in snapshots

Screen.Recording.2026-06-30.at.4.25.05.PM.mov

@dmkt9 dmkt9 Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmkt9 I can't reproduce, can you please check again? previousTaxCode is already in snapshots

Screen.Recording.2026-06-30.at.4.25.05.PM.mov

Ah, thanks! It works well for me.

optimisticPreviousTaxCode: null,
},
},
},
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@ type TaxRate = OnyxCommon.OnyxValueWithOfflineFeedback<{
/** The old tax code of the tax rate when we edit the tax code */
previousTaxCode?: string;

/** The old tax code kept only while a tax code edit is in flight, used to resolve the rate from the old code; cleared once the API resolves */
optimisticPreviousTaxCode?: string;

/** An error message to display to the user */
errors?: OnyxCommon.Errors;

Expand Down
Loading