[HOLD for payment 2024-06-28] [$250] Tags - Enable tag option is no longer grayed out after renaming tag offline

[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: 1.4.78-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4587701
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go offline
3. Go to workspace settings
4. Go to Tags
5. Click on any tag
6. Disable or enable the tag
7. Note that the option grays out
8. Click on the tag name
9. Rename the tag

Expected Result:

Enable tag option will remain grayed out after renaming tag offline

Actual Result:

Enable tag option is no longer grayed out after renaming tag offline

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

Bug6497093_1717139350416.20240531_150625.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014348229109c8ccb2
• Upwork Job ID: 1798333089478942720
• Last Price Increase: 2024-06-05
• Automatic offers:
• Krishna2323 | Contributor | 102678138
• allgandalf | Contributor | 102763005

Issue Owner

Current Issue Owner: @garrettmknight

[melvin-bot]

Triggered auto assignment to @garrettmknight (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]

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

[lanitochka17]

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

[Krishna2323]

Proposal

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

Tags - Enable tag option is no longer grayed out after renaming tag offline

What is the root cause of that problem?

The current pending actions are replaced by new pending actions.

App/src/libs/actions/Policy/Tag.ts

Lines 380 to 382 in 81fedce

	pendingFields: {
	name: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
	},

App/src/libs/actions/Policy/Tag.ts

Lines 196 to 198 in 81fedce

	pendingFields: {
	enabled: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
	},

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

We need to spread the oldTag?.pendingFields in renamePolicyTag and ...(policyTag.tags[key]).pendingFields in setWorkspaceTagEnabled.

We will also need to give a type for oldTag in renamePolicyTag and then spread in pendingFields. For setWorkspaceTagEnabled we can directly spread ...policyTag.tags[key].pendingFields,.

const oldTag: PolicyTag | Record<string, never> = allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`]?.[tagListName]?.tags?.[oldTagName] ?? {};

pendingFields: {
    ...(oldTag.pendingFields ?? {}),
    name: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
},

We will also spread the previous pending field in success and failure data.

We will also check for similar bugs in policy pages or pages with similar functionality.

What alternative solutions did you explore? (Optional)

[Krishna2323]

Proposal Updated

• Minor updates in solution section

[garrettmknight]

I couldn't get the tag to disable offline when testing. @Krishna2323 are you getting a different result on Step 6?

[Krishna2323]

@garrettmknight, yup I can disable the tag, the updated styles (grayed out) is applied to switch until we edit the tag name. Both should be grayed out, but switch returns to normal state as if it wasn't updated offline.

Monosnap.screencast.2024-06-05.02-01-51.mp4

[garrettmknight]

Cool - just replicated, opening up.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014348229109c8ccb2

[melvin-bot]

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

[cretadn22]

Proposal

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

Enable tag option is no longer grayed out after renaming tag offline

What is the root cause of that problem?

When renaming the tag, we remove the old Tag and add the new tag. pendingFields of new tag will override the pendingFields of oldTag, so pendingFields.enable is lost

App/src/libs/actions/Policy/Tag.ts

Lines 375 to 382 in 90dee7a

	[oldTagName]: null,
	[newTagName]: {
	...oldTag,
	name: newTagName,
	pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
	pendingFields: {
	name: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
	},

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

In renamePolicyTag function, we need to make adjustments to these points:

• OptimisticData: Instead of completely overriding pendingFields, we need to perform a deep merge to combine the pending fields

pendingFields: merge(oldTag.pendingFields, {
   name: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
})

• SuccessData:

App/src/libs/actions/Policy/Tag.ts

Lines 397 to 401 in 90dee7a

	[newTagName]: {
	errors: null,
	pendingAction: null,
	pendingFields: {
	name: null,

We utilize oldTag.errors, errors.pendingAction, and oldTag.pendingFields.

errors: oldTag.errors, 
pendingAction: oldTag.pendingAction, 
pendingFields: oldTag.pendingFields

• FailureData: we need to deep merge the error object
  

  App/src/libs/actions/Policy/Tag.ts

  Lines 417 to 419 in 90dee7a

  	[oldTagName]: {
  	...oldTag,
  	errors: ErrorUtils.getMicroSecondOnyxError('workspace.tags.genericFailureMessage'),

errors: merge(oldTag.errors, {
    ErrorUtils.getMicroSecondOnyxError('workspace.tags.genericFailureMessage')
})

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[garrettmknight]

@thesahindia looks like we've got a proposal here!

[thesahindia]

@Krishna2323's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[Krishna2323]

@thesahindia, PR ready for review.

[allgandalf]

@garrettmknight , can you please assign me here(context), I’ll be reviewing the PR

[melvin-bot]

📣 @allgandalf 🎉 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 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.85-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: Tags - Enable tag option is no longer grayed out after renaming tag offline. #43635

If no regressions arise, payment will be issued on 2024-06-28. 🎊

For reference, here are some details about the assignees on this issue:

• @Krishna2323 requires payment automatic offer (Contributor)
• @allgandalf requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@allgandalf] The PR that introduced the bug has been identified. Link to the PR:
• [@allgandalf] 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:
• [@allgandalf] 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:
• [@allgandalf] Determine if we should create a regression test for this bug.
• [@allgandalf] 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.
• [@garrettmknight] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[garrettmknight]

Payment Summary:

• Contributor: @Krishna2323 $250 paid via upwork
• Reviewer: @allgandalf $250 paid via upwork

@allgandalf can you complete the checklist when you get a chance?

[garrettmknight]

Shouldn't be overdue?

[allgandalf]

completing the checklist @garrettmknight , should update this in sometime, sorry my K2 api token expired and this didn't update in my K2 at all whole week :)

[allgandalf]

• The PR that introduced the bug has been identified. Link to the PR:

  • Create Tag utils file #42559

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

  • https://github.com/Expensify/App/pull/42559/files#r1661887028

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

  • That was a new feature Implementation, so it would not have been possible to avoid this current bug, so N/A.

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

  • That was a new feature Implementation, so it would not have been possible to avoid this current bug, so N/A

• 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

---

• Precondition: Workspace must have few tags.

1. Go offline
2. Go to workspace settings
3. Go to Tags
4. Click on any tag
5. Disable or enable the tag
6. Note that the option grays out
7. Click on the tag name
8. Rename the tag
9. Verify both switch and name field is grayed out

Do we agree 👍 or 👎

[garrettmknight]

Done!