[HOLD for payment 2024-07-10] [$250] Chat - System message about adding tag in Parent: Child format shows Parent\: Child

[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.81-4
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
Issue reported by: Applause - Internal Team

Issue found when executing PR #43392

Action Performed:

1. Go to URL https://staging.new.expensify.com/
2. Log with any account
3. Go to account settings -> Workspaces -> Create WS if needed
4. Enabled Tags
5. Add a tag in Parent: Child format
6. Go to #admins room and check the system message

Expected Result:

System message about adding tag in Parent: Child format shows as Parent: Child, not Parent\: Child

Actual Result:

System message about adding tag in Parent: Child format shows Parent\: Child

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014a67ec2b09731f24
• Upwork Job ID: 1800543159428158713
• Last Price Increase: 2024-06-11
• Automatic offers:
• eh2077 | Reviewer | 102720454

Issue Owner

Current Issue Owner: @eh2077

[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

[sakluger]

FYI @lanitochka17 - GitHub was removing the \ from your comment because it's a Markdown formatting character in GH - it's an escape character used to ignore other markdown formatting (more details here).

I updated your comment to add that character back. 😄

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014a67ec2b09731f24

[melvin-bot]

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

[devguest07]

Proposal

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

System message about adding tag in Parent: Child format shows Parent: Child

What is the root cause of that problem?

When displaying messages inside ReportActionItemFragment and TextCommentFragment, we do not clean the tag message escaping of colons (used to create multi-level tags, e.g., "Parent: Child") in the tag name received from the backend.

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

We need to clean up the message from the colons before displaying it. This can be done at multiple levels before being displayed. One way is by editing the fragment inside ReportActionItemFragment.

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

Lines 77 to 78 in 4acd9d2

	actionName,
	fragment,

We can use if (actionName === 'POLICYCHANGELOG_ADD_TAG') to check if it's a tag message. If so, we can use the utility PolicyUtils.getCleanedTagName to return a clean tag message.

if (actionName === 'POLICYCHANGELOG_ADD_TAG') {
  tagFragment = PolicyUtils.getCleanedTagName(fragment.html); // we can use fragment.text too
}

If we have a new value, we can display it and pass it to the component TextCommentFragment.

[truph01]

Proposal

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

• System message about adding tag in Parent: Child format shows Parent: Child

What is the root cause of that problem?

• With the system message about adding tag, BE returns a message like:

{
    "actionName": "POLICYCHANGELOG_ADD_TAG",
    "message": [
        {
            "html": "added the tag \"name\\: age\" to the list \"Tag\"",
            "text": "added the tag \"name\\: age\" to the list \"Tag\""
        }
    ]
}

• And we display this message by using ReportActionItemMessage:
  

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

  Line 647 in 8d11d0b

  <ReportActionItemMessage

  
  that will display a message base on raw reportAction.message data.
• So that leads to the bug.

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

• We need to add an additional else logic to this:
  

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

  Line 638 in 8d11d0b

  } else {

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

• Then, we add an additional else logic to this:
  

  App/src/libs/SidebarUtils.ts

  Line 378 in 8d11d0b

  } else {

else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG) {
            result.alternateText = PolicyUtils.getCleanedTagName(lastAction?.message?.[0]?.text ?? '');

to fix the LHN issue.

What alternative solutions did you explore? (Optional)

• There are a few component where we have the same issue, for example, copy to clipboard action and we need to address them as well.

[eh2077]

Thank you for your proposals!

@devguest07 Your proposal is close to success. You dived into the code and found a line to fix the effect, though that's not the best way to fix it. No worries, as you seem new to the project. Just a small tip for you: try to find out if there are similar existing coding patterns in the codebase when you're going to make changes. I believe you will soon land on a successful proposal.

@truph01 's proposal looks good to me. Their solution fixes both report and LHN message. More detail changes can be discussed in PR.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[devguest07]

Thank you @eh2077

[rlinoz]

Looks good, thanks!

[melvin-bot]

📣 @eh2077 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @truph01 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[eh2077]

@truph01 Kindly let us know when we can expect a PR to be ready for review, thanks!

[truph01]

@eh2077 I created PR #43765. It can be reviewed now.

[rlinoz]

@sakluger this issue was fixed by some other PR and is not reproducible anymore, but we had a working PR and all, can you proceed with payment to @truph01 and @eh2077 ?

[sakluger]

Summarizing payment on this issue:

Contributor: @truph01 $250, payable via Upwork
Contributor+: @eh2077 $250, paid via Upwork

[rlinoz]

Not sure why Melvin think this is overdue

[sakluger]

@truph01 could you please apply to the job on Upwork (https://www.upwork.com/jobs/~014a67ec2b09731f24) or share your Upwork profile link so that we can pay you for your work and close out the GH issue? Thanks!

[sakluger]

@truph01 could you please apply to the job on Upwork (https://www.upwork.com/jobs/~014a67ec2b09731f24) or share your Upwork profile link so that we can pay you for your work and close out the GH issue? Thanks!

[rlinoz]

@sakluger I actually messed up and got things wrong, the main issue is still reproducible, and we are fixing it in the PR, I am really sorry about that.

[melvin-bot]

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

• Fix: System message about adding tag in Parent: Child #43765

If no regressions arise, payment will be issued on 2024-07-10. 🎊

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

• @eh2077 requires payment automatic offer (Reviewer)
• @truph01 requires payment (Needs manual offer from BZ)

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

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR:
• [@eh2077] 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:
• [@eh2077] 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:
• [@eh2077] Determine if we should create a regression test for this bug.
• [@eh2077] 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]

I sent an offer to @truph01 via Upwork: https://www.upwork.com/nx/wm/offer/103034403.

@eh2077 can you please complete the BZ checklist?

[truph01]

@sakluger I accepted that offer, TY

[sakluger]

Thanks, all paid!

@eh2077 we're just holding for the BZ checklist before we can close this issue.

[sakluger]

Bump @eh2077

[eh2077]

Checklist

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR: There's not such PR that causes this bug because the system message is from backend.
• [@eh2077] 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: N/A
• [@eh2077] 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: N/A
• [@eh2077] Determine if we should create a regression test for this bug. No

cc @sakluger