[HOLD for payment 2024-09-07] [$250] Taxes - Name field is no longer grayed out after editing tax code offline

[IuliiaHerets]

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: v9.0.22-5
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp https://expensify.testrail.io/index.php?/tests/view/4874241
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Taxes.
3. Go offline.
4. Click on any tax rate.
5. Click Name.
6. Edit the name and save it.
7. Click Tax code.
8. Edit Tax code and save it.

Expected Result:

Name field will remain grayed out after editing tax code offline.

Actual Result:

Name field is no longer grayed out after editing tax code offline.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6577125_1724165547316.20240820_224656.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01da26942e57c47d6b
• Upwork Job ID: 1826075814524858613
• Last Price Increase: 2024-08-21

Issue Owner

Current Issue Owner: @kadiealexander

[melvin-bot]

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

[IuliiaHerets]

We think that this bug might be related to #wave-collect - Release 1

[IuliiaHerets]

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

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-08-20 20:40:06 UTC.

Proposal

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

Taxes - Name field is no longer grayed out after editing tax code offline

What is the root cause of that problem?

Current pending field are not spread when updating the tax code. When updating tax code we create a key/value pair with the new tax code name so the pending fields are not merged in this case like it does when updating name or value, that's why we don't need to spread the pending fields when updating name or value.

App/src/libs/actions/TaxRate.ts

Lines 498 to 503 in b2eb922

	[newTaxCode]: {
	...originalTaxRate,
	pendingFields: {code: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE},
	pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
	errorFields: {code: null},
	previousTaxCode: oldTaxCode,

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

Spread the pending fields. We should also check for other update actions for tax details edit.

EDIT: We don't need to do that when editing name or value.

What alternative solutions did you explore? (Optional)



Result

Monosnap.screencast.2024-08-21.01-59-36.mp4

[abzokhattab]

Edited by proposal-police: This proposal was edited at 2024-08-20 21:06:07 UTC.

Proposal

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

Taxes - Name field is no longer grayed out after editing tax code offline

What is the root cause of that problem?

when updating the tax code it overrides the existing pending fields: https://github.com/Expensify/App/blob/b2eb922823d491996faa5d50275bfce540e9b288/src/libs/actions/TaxRate.ts#L496C21-L525C27

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

we should add the existing pending field from the originalTaxRate.pendingFields into the pending fields object... same applies to the success and the failure data

To do that we should change the pending fields property in the optimistic values to:

pendingFields: {...(originalTaxRate?.pendingFields ?? {}), code: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE},

also, modify that in the success and failure objects

What alternative solutions did you explore? (Optional)

N.A

[Krishna2323]

Proposal Updated

• Added result video.

[Krishna2323]

Proposal Updated

• Updated RCA to include a better explanation, the solution remains the same.

[abzokhattab]

Proposal Updated

Added the code implementation

[melvin-bot]

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

[melvin-bot]

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

[c3024]

@Krishna2323 's first posted proposal comment shows the offending code block correctly in the RCA and the suggested solution is clear enough. I think we can proceed with his proposal because his is first.

🎀 👀 🎀 C+ Reviewed

[melvin-bot]

Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Krishna2323]

@mountiny, bump for assignment if you agree with @c3024's decision.

[mountiny]

Go ahead please @Krishna2323

[Krishna2323]

@c3024, PR ready for review ^

[c3024]

Automation failed here. This was deployed to production yesterday. Payment due on 7-Sep or 8-Sep.

cc: @kadiealexander

[mountiny]

Updated

$250 to @Krishna2323 and to @c3024

[kadiealexander]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@c3024] The PR that introduced the bug has been identified. Link to the PR: [GL] Tax - Add tax code #43156
• [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: bee9d44#r146398000
• [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: No discussion was started because this could not have identified earlier.
• [@c3024] Determine if we should create a regression test for this bug.
• [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/426788

[melvin-bot]

Payment Summary

Upwork Job

• Contributor: @Krishna2323 paid $250 via Upwork (https://www.upwork.com/nx/wm/offer/103873834)
• Reviewer: @c3024 paid $250 via Upwork (https://www.upwork.com/offer/103873822)

BugZero Checklist (@kadiealexander)

• 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/1826075814524858613/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[c3024]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@c3024] The PR that introduced the bug has been identified. Link to the PR: [GL] Tax - Add tax code #43156
• [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: bee9d44#r146398000
• [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: No discussion was started because this could not have identified earlier.
• [@c3024] Determine if we should create a regression test for this bug. Yes
• [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Go to workspace settings > Taxes.
2. Go offline.
3. Click on any tax rate.
4. Click on Name.
5. Edit the name and save it.
6. Click on Tax code.
7. Edit the Tax code and save it.
8. Verify that the name field remains greyed out.

[kadiealexander]

Not overdue, offers sent!

[Krishna2323]

Offer accepted.