[$250] Tax - Toggle becomes unlocked, Delete button appears briefly after saving tax code of default rate

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] 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
• Taxes feature is enabled

1. Go to staging.new.expensify.com
2. Go to workspace settings > Taxes
3. Click on the default tax rate
4. Click Tax code
5. Edit the tax code and save it

Expected Result:

The toggle will remain locked and Delete button will not appear after saving the tax code of the default tax rate

Actual Result:

The toggle becomes unlocked and Delete button appears briefly after saving the tax code of the default 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

Bug6548319_1721502876398.20240721_031001.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017da022f8b49cd8bf
• Upwork Job ID: 1815084149483432900
• Last Price Increase: 2024-07-21
• Automatic offers:
• ahmedGaber93 | Reviewer | 103216881

Issue Owner

Current Issue Owner: @ahmedGaber93

[melvin-bot]

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

[github-actions]

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

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

[etCoderDysto]

Proposal

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

Tax - Toggle becomes unlocked, Delete button appears briefly after saving tax code of default rate

What is the root cause of that problem?

When updating tax code, defaultExternalID is storing the old value, while newTaxCode is updated optimistically to a new value.

App/src/libs/actions/TaxRate.ts

Lines 486 to 495 in f3a8f73

	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: {

Then canEditTaxRate returns true comparing the old defaultExternalID with the new tax code here

App/src/libs/PolicyUtils.ts

Lines 408 to 409 in f3a8f73

	function canEditTaxRate(policy: Policy, taxID: string): boolean {
	return policy.taxRates?.defaultExternalID !== taxID;

Then shouldShowDeleteMenuItem becomes true we display delete button

App/src/pages/workspace/taxes/WorkspaceEditTaxPage.tsx

Lines 42 to 44 in f3a8f73

	const canEditTaxCode = !PolicyUtils.isControlPolicy(policy);
	const shouldShowDeleteMenuItem = canEditTaxRate && !hasAccountingConnections;

Then BE udpates defaultExternalID with new value and delete button becomes hidden since the new defaultExternalID is equal to newTaxCode

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

We should set defaultExternalID to the new value in optimisticData

taxRates: {
  defaultExternalID: oldTaxCode === policy?.taxRates?.defaultExternalID ? newTaxCode : policy?.taxRates?.defaultExternalID,
                        

App/src/libs/actions/TaxRate.ts

Lines 494 to 496 in f3a8f73

	value: {
	taxRates: {
	taxes: {

And add for failureData to

defaultExternalID:  policy?.taxRates?.defaultExternalID;

Note: We should apply the same pattern on foreignTaxDefault

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

Applying the same pattern on foreignTaxDefault, makes sure that Default badge is displayed on the default tax rate rather than Workspace currency default

App/src/pages/workspace/taxes/WorkspaceTaxesPage.tsx

Lines 82 to 86 in f3a8f73

	const textForDefault = useCallback(
	(taxID: string, taxRate: TaxRate): string => {
	let suffix;
	if (taxID === defaultExternalID && taxID === foreignTaxDefault) {
	suffix = translate('common.default');

What alternative solutions did you explore? (Optional)

Result:

Screen.Recording.2024-07-21.at.2.35.03.at.night.mp4

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017da022f8b49cd8bf

[melvin-bot]

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

[NaveedShaikh78]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~0147b6419f14a69c1b

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[codeapexdev]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/akashvashishtha

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[ahmedGaber93]

@etCoderDysto's proposal LGTM!

When we update the default tax rate code, We missed updating defaultExternalID in optimisticData and this causes the issue briefly until the backend data received. We also should do the same for the default foreign tax.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[cead22]

@ahmedGaber93 @etCoderDysto what PR was this introduced in / was this a regression?

[rushatgabhane]

half way done.

[melvin-bot]

@cead22, @mollfpr, @rushatgabhane, @etCoderDysto Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@cead22, @mollfpr, @rushatgabhane Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@cead22, @mollfpr, @rushatgabhane Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[cead22]

@rushatgabhane do you have an ETA? Just curious

[melvin-bot]

@cead22, @mollfpr, @rushatgabhane Eep! 4 days overdue now. Issues have feelings too...

[cead22]

@rushatgabhane how that PR coming along?

[melvin-bot]

@cead22, @mollfpr, @rushatgabhane Huh... This is 4 days overdue. Who can take care of this?

[cead22]

DM'ed @rushatgabhane on slack to ask for an update

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

This issue has not been updated in over 15 days. @cead22, @mollfpr, @rushatgabhane eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[rushatgabhane]

this can be closed