[HOLD for payment 2024-06-06] [$250] Tags - Opening tag with certain text shows hhm not here while opening for the first time

[izarutskaya]

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.66-2
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Tap profile -- Workspaces-- Workspace
3. Tap more features
4. Enable tags
5. Tap tags
6. Tap add tags
7. Enter ![https://hatrabbits.com/wp-content/uploads/2017/01/random.jpg]
   (https://hatrabbits.com/wp-content/uploads/2017/01/random.jpg)
8. Tap save
9. Immediately open the tag created

Expected Result:

Opening tag with certain text must not show hhm not here while opening for the first time

Actual Result:

Opening tag with certain text shows hhm not here while opening for the first time

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

Bug6462637_1714123382618.tag.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0178b10f7b05e6e2ba
• Upwork Job ID: 1785427210570833921
• Last Price Increase: 2024-04-30
• Automatic offers:
• DylanDylann | Reviewer | 0
• bernhardoj | Contributor | 0

Issue Owner

Current Issue Owner: @johncschuster

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #collect project.

[bernhardoj]

Proposal

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

Creating a new tag with a specific text and then open it quickly will show a not found page after a few seconds.

What is the root cause of that problem?

The issue can be reproduced if the text name contains a colon (:). When we create a tag with a name with a colon, the BE response will give us a different tag name. All the colon is replaced with \:, so if we set the name as a:b, the BE will return the name as a\:b. Based on this comment of a function that cleans it up before showing the name, it's for multi-level tag.

App/src/libs/PolicyUtils.ts

Lines 236 to 241 in 30bfabc

	/**
	* Cleans up escaping of colons (used to create multi-level tags, e.g. "Parent: Child") in the tag name we receive from the backend
	*/
	function getCleanedTagName(tag: string) {
	return tag?.replace(/\\{1,2}:/g, CONST.COLON);
	}

The policy tag is accessed by its name,

App/src/pages/workspace/tags/TagSettingsPage.tsx

Lines 49 to 53 in 30bfabc

	const currentPolicyTag = policyTag.tags[decodeURIComponent(route.params.tagName)];
	if (!currentPolicyTag) {
	return <NotFoundPage />;
	}

and because the name from the FE doesn't match the new name from the BE, the data is now not found, thus the not found page is shown.

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

We should match the optimistic data with the BE data by replacing the colon with \: when creating a new tag or editing an existing tag.

For example,

App/src/libs/actions/Policy.ts

Lines 3242 to 3253 in 30bfabc

	function createPolicyTag(policyID: string, tagName: string) {
	const policyTag = PolicyUtils.getTagLists(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})?.[0] ?? {};
	const onyxData: OnyxData = {
	optimisticData: [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`,
	value: {
	[policyTag.name]: {
	tags: {
	[tagName]: {

const newTagName = tagName.replaceAll(CONST.COLON, '\\:');

// then replace all tagName with newTagName

do the same for renaming in renamePolicyTag for the old and new tag name

[melvin-bot]

@johncschuster Whoops! This issue is 2 days overdue. Let's get this updated quick!

[johncschuster]

I don't think this is a blocker, but I wonder if entering a URL as the tag name is part of the reason for this behavior.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0178b10f7b05e6e2ba

[melvin-bot]

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

[DylanDylann]

@bernhardoj Your proposal looks promising. But we need to understand why the BE changed : to \: and whether are there any rules on the BE side to change tag names.

🎀 👀 🎀 C+ reviewed

Add this label to get more information about the BE logic from the internal engineer

[melvin-bot]

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

[DylanDylann]

But we need to understand why the BE changed : to \: and whether are there any rules on the BE side to change tag names.

@techievivek Before moving forward with this issue, could you help to check the logic in BE and give an answer to the above question

[melvin-bot]

@johncschuster, @techievivek, @DylanDylann Eep! 4 days overdue now. Issues have feelings too...

[johncschuster]

@techievivek bump on the above question!

[melvin-bot]

@johncschuster @techievivek @DylanDylann this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[johncschuster]

Bumped in ND!

[johncschuster]

Bumped in Slack as well!

[trjExpensify]

Yes, we allow colon in category name in NewDot and that achieves the sub-category nesting I highlighted above.

For tags, I think you should be able to use a colon in the name, it just doesn't do anything special - and it certainly doesn't create a multiTag list when you do. (Just like OldDot).

[nkdengineer]

Proposal

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

As confirmed here, we will allow user use a colon in the name

What is the root cause of that problem?

Currently if the tag name include a colon the BE will replace it to \: In the FE, we have a function to convert again \: to :

App/src/libs/PolicyUtils.ts

Lines 236 to 241 in 30bfabc

	/**
	* Cleans up escaping of colons (used to create multi-level tags, e.g. "Parent: Child") in the tag name we receive from the backend
	*/
	function getCleanedTagName(tag: string) {
	return tag?.replace(/\\{1,2}:/g, CONST.COLON);
	}

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

With the new confirmation, we need a change from BE to avoid replacing the tag name with a colon. In the FE we also don't need this function to convert the tag name back

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.

[DylanDylann]

As this comment, @nkdengineer's proposal looks promising. We don't need to convert anything in the tag name anymore.

@techievivek What do you think about their proposal?

[bernhardoj]

I don't think we can fix it from the BE based on this comment. As @amyevans mentioned from that comment, we need to escape the colon when sending it to the BE which is what this issue is about.

[DylanDylann]

I see we have some conflicts in decisions. @amyevans @techievivek Could you help to confirm the final expectation for this issue? Should we convert the colon in the tag name?

[amyevans]

We should match the optimistic data to what is returned from the server as @bernhardoj suggested. I think going down the road of a backend fix is inadvisable because under the hood the OldDot and NewDot API commands for creating a tag are calling the same functions, so the logic needs to be fully compatible and not create bugs on OldDot.

Multi-level tags are sent to/received from the server as a string separated by unescaped colons. So if I have tag lists Department, Location, and Project set up on a workspace, and I create an expense with
Department: Accounting
Location: Africa
Project: Project 1
that is sent/received as Accounting:Africa:Project 1

If we want to create a tag with a colon in its name, the colon needs to be escaped.

AFAIK, independent multi-level tags are implemented in NewDot (#32828), it is only dependent multi-level tags at this point that are not.

[DylanDylann]

@amyevans Thanks for your comment

[DylanDylann]

As this comment, @techievivek Let's go with @bernhardoj's proposal

[melvin-bot]

📣 @DylanDylann 🎉 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]

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

[bernhardoj]

PR is ready

cc: @DylanDylann

[DylanDylann]

Thanks for the quick PR

[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.77-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix opening tag with colon shows not found page while opening for the first time #42381

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

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

• @bernhardoj requires payment automatic offer (Contributor)
• @DylanDylann requires payment automatic offer (Reviewer)

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

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

[johncschuster]

Sweet! I'll issue payment for this next Thursday.

@DylanDylann, can you complete the BZ Checklist before then? Thank you!

[DylanDylann]

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
[@DylanDylann] 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: NA
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] 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 page
2. Enable tag in More features if not enabled yet
3. Create a new tag with a colon in the name
4. Open the tag quickly after creating it
5. Wait for a few seconds for the request to complete
6. Verify the not found page doesn't show show
7. Edit the tag name
8. Verify the tag name is updated
   Repeat step 5-6

Do we agree 👍 or 👎

[johncschuster]

Payment has been issued! Thanks for your contributions! 🎉