[$250] Tags - System message for disabling, adding tax code and deleting X: Y tag shows X\: Y tag

[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.23-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: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• Workspace is under Control plan.

1. Go to staging.new.expensify.com
2. Go to workspace settings > Tags
3. Add a tag in X: Y format
4. Disable, add tax code and delete the tag in X:Y format
5. Go to #admins

Expected Result:

The system message for disabling, adding tax code and deleting tag in X: Y format will display the tag correctly

Actual Result:

The system message for disabling, adding tax code and deleting tag in X: Y format displays the tag in X: Y format

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

Bug6578196_1724248326940.bandicam_2024-08-21_21-43-35-544.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e7a0584538b48c5f
• Upwork Job ID: 1826803032920988282
• Last Price Increase: 2024-08-23

[melvin-bot]

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

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

[abzokhattab]

Edited by proposal-police: This proposal was edited at 2024-08-21 23:08:16 UTC.

Proposal

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

The system message for disabling, adding, and deleting the X: Y tag displays as "X: Y tag."

What is the root cause of that problem?

The comment message is only cleaned and parsed when the tag is added, but not when it is updated or deleted.

App/src/pages/home/report/ReportActionItem.tsx

Lines 652 to 654 in f80d19b

	} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG) {
	children = <ReportActionItemBasicMessage message={PolicyUtils.getCleanedTagName(ReportActionsUtils.getReportActionMessage(action)?.text ?? '')} />;
	} else if (

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

We should add conditions for the remove, disable, enable, and update actions, as well as any other actions that display the tag name in the comment.

} else if (
            action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG ||
            action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_TAG_ENABLED ||
            action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_TAG_NAME ||
            action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.DELETE_TAG ||
            action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_TAG
        ) {

we can also add other actions that show the tag name in the comment

And as daledah mentioned we need to do the same in the sideBar as well since the same problem also shown in the LHN data ... so the same fix would be applied here as well
App/src/libs/SidebarUtils.ts
Result:

What alternative solutions did you explore? (Optional)

N/A

[daledah]

Proposal

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

• In LHN and report page, the system message for disabling, adding tax code and deleting tag in X: Y format displays the tag in X: Y format.

What is the root cause of that problem?

• In LHN, we do not convert X: Y to X\: Y in case of system message for disabling, updating tag:
  

  App/src/libs/SidebarUtils.ts

  Lines 428 to 430 in ad7c1d5

  	} else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG) {
  	result.alternateText = PolicyUtils.getCleanedTagName(ReportActionsUtils.getReportActionMessage(lastAction)?.text ?? '');
  	} else if (lastAction && ReportActionsUtils.isOldDotReportAction(lastAction)) {

• The same issue when displaying report actions:
  

  App/src/pages/home/report/ReportActionItem.tsx

  Lines 657 to 659 in ad7c1d5

  	} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG) {
  	children = <ReportActionItemBasicMessage message={PolicyUtils.getCleanedTagName(ReportActionsUtils.getReportActionMessage(action)?.text ?? '')} />;
  	} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_NAME) {

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

• We should create a function to check whether the action is needed to clean the tag name:

const isActionNeedToCleanup(actionName: string) {
    return [CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_TAG, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_TAG_ENABLED, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.DELETE_TAG, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_TAG_NAME].includes(actionName)
}

• Then:
  

  App/src/libs/SidebarUtils.ts

  Line 428 in ad7c1d5

  } else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG) {

  
  will be:

        } else if (isActionNeedToCleanup(lastAction?.actionName)) {

• And:
  

  App/src/pages/home/report/ReportActionItem.tsx

  Line 657 in ad7c1d5

  } else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG) {

  
  will be:

        } else if (isActionNeedToCleanup(action?.actionName)) {

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[sakluger]

@CortneyOfstad could you please help manage this issue while I'm OOO through next Friday, 8/30? Thanks!

[daledah]

@abzokhattab Please note the contributor guidelines:

If you want to make an entirely new proposal or update an existing proposal, please go back and edit your original proposal, then post a new comment to the issue in this format to alert everyone that it has been updated:

[abzokhattab]

Thank you for the note, Daledah.

The concept remains consistent across both files, which is why I didn't add a separate proposal update comment. The core issue we are addressing here focuses on the chat comments, while the LHN behavior, although related, can be seen as a sub-issue within the broader context as the result matches the expected output of the issue. Since the change in the other file is an extension of the same logic, I believe it doesn't fundamentally alter the original proposal.

[nabeel-tariq]

Proposal

Proposal: System Message for Disabling, Adding Tax Code, and Deleting Tag Shows X: Y Tag

Issue:
The system currently displays messages related to actions such as disabling, adding tax codes, and deleting tags with the X: Y tag format.

Proposed Solution:
To resolve this issue, we propose implementing a getCleanedTagName() function. This function will standardize the display of tags in system messages by removing unwanted escape characters and formatting tags properly.

Technical Explanation:
Currently, system messages for actions such as adding, updating, enabling, and deleting tags display the tag names with the escape character () resulting in the format X: Y. We will introduce a getCleanedTagName()` function that will clean and format tag names for display in system messages. This function will remove any escape characters and return the cleaned tag name in a user-friendly format. The addition of this function will require updates to the parts of the code responsible for generating system messages. Any existing code that directly displays tag names will be modified to use getCleanedTagName() to ensure consistency in tag display.

Implementation Plan:
*Implement the getCleanedTagName() function in the relevant utility.
*Refactor existing code that generates system messages to use getCleanedTagName() instead of directly referencing tag
names.
*Test the updated message formatting to ensure tags are displayed correctly.

Conclusion:
Implementing the getCleanedTagName() function will standardize the display of tag names in system messages, removing unnecessary escape characters and enhancing readability. This approach will address the current formatting issue and ensure that system messages are clear and user-friendly o n all the platforms like Android, IOS, MAC and Web.

[melvin-bot]

📣 @nabeel-tariq! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[nabeel-tariq]

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

[melvin-bot]

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

[melvin-bot]

@sakluger, @CortneyOfstad, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[abdulrahuman5196]

Checking now

[abdulrahuman5196]

@abzokhattab 's proposal here #47804 (comment) looks good and works well. Other proposal seems improvised version of core issue to fix system, so going with the original proposal.

🎀 👀 🎀
C+ Reviewed

[melvin-bot]

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

[melvin-bot]

❌ There was an error making the offer to @abzokhattab for the Contributor role. The BZ member will need to manually hire the contributor.

[sakluger]

Here's the offer for @abzokhattab: https://www.upwork.com/nx/wm/offer/103753777

[melvin-bot]

This issue has not been updated in over 15 days. @francoisl, @sakluger, @abdulrahuman5196, @abzokhattab 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!

[abdulrahuman5196]

Hi @francoisl / @sakluger I think the automation failed here.

The change was pushed to production 2 weeks before. And should be up for payment.

[francoisl]

Changing to Daily so we can issue payments then @sakluger

[sakluger]

Sorry about that delay!

Summarizing payment on this issue:

Contributor: @abzokhattab $250, paid via Upwork
Contributor+: @abdulrahuman5196 $250, please request on Newdot

[sakluger]

@abdulrahuman5196 please complete the BZ checklist:

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:

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

[sakluger]

We can close this as soon as the BZ checklist is completed.

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
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:
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:

Not a regression

Determine if we should create a regression test for this bug.

Yes.

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.

1. Workspace is under Control plan.
2. Go to workspace settings > Tags
3. Add a tag in X: Y format
4. Disable, add tax code and delete the tag in X:Y format
5. Go to #admins
6. The system message for disabling, adding tax code and deleting tag in X: Y format will display the tag correctly

[sakluger]

Thanks!

[JmillsExpensify]

$250 approved for @abdulrahuman5196