[HOLD for payment 2024-03-25] [$500] [Wave Collect] [Categories] Categories - Not here page shows up when opening Parent/ and Parent/: Child category

[kbecciv]

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.48-0
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• User is an admin of Collect workspace.

1. Go to staging.new.expensify.com
2. Go to Workspace settings > Collect workspace.
3. Go to Categories.
4. Click Add category.
5. Add category in Parent/ and Parent/: Child format. And other special characters
6. Click on the created category.

Expected Result:

Category RHP opens without issue.

Actual Result:

Not here page shows up when opening category with Parent/ and Parent/: Child style.

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

Bug6405006_1709792482718.20240307_141255.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01425421b068a188c5
• Upwork Job ID: 1767873021149028352
• Last Price Increase: 2024-03-13

[melvin-bot]

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[kbecciv]

We think that this bug might be related to #wave8-collect-admins
CC @zanyrenney

[kbecciv]

@jliexpensify 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.

[tienifr]

Proposal

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

Not here page shows up when opening category with Parent/ and Parent/: Child style.

What is the root cause of that problem?

We use encodeURI to encode the category name before passing it to route params:

App/src/ROUTES.ts

Line 543 in 27b1aa8

getRoute: (policyID: string, categoryName: string) => `workspace/${policyID}/categories/${encodeURI(categoryName)}` as const,

However, encodeURI escapes all characters except /.

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

Should use encodeURIComponent here and decodeURIComponent here as we've been using widely in App.

More info about the differences between encodeURI and encodeURIComponent here.

Note: List of characters that are supported in category/tag name after the change:

A–Z a–z 0–9 - _ . ! ~ * ' ( )

; / ? : @ & = + $ , #

What alternative solutions did you explore? (Optional)

I see there're several places where we use encodeURI, we should investigate if it is the correct usage there as well.

[jliexpensify]

Posted here

I have reproduced the issue.

[melvin-bot]

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

[tienifr]

@jliexpensify Can you update the status?

[jliexpensify]

@tienifr - I haven't gotten any responses from my Slack post: basically, I'm not sure if we use/are planning to use Parent/Child styles in New Expensify.

[tienifr]

@jliexpensify The problem is not only with / but with other special characters as well. So the basic question is whether we want category/tag name to include special charaters.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01425421b068a188c5

[melvin-bot]

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

[mountiny]

@tienifr and others, we would like to make sure this works with various special characters in the name of the category

[hungvu193]

Reviewing shortly

[hungvu193]

Thanks for your proposal.
@tienifr 's proposal looks good to me. I checked other places that we used encodeURI inside url, most of them are static value.
There are 2 places that we're using encodeURI to encode the source but all of these are query param so it won't be a problem.

App/src/ROUTES.ts

Line 207 in 37a2dbb

getRoute: (reportID: string, source: string) => `r/${reportID}/attachment?source=${encodeURI(source)}` as const,

App/src/ROUTES.ts

Line 159 in 37a2dbb

getRoute: (source: string) => `settings/troubleshoot/console/share-log?source=${encodeURI(source)}` as const,

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

Current assignee @jliexpensify is eligible for the Bug assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[jliexpensify]

Hi @mallenexpensify - I am OOO from the 21st to 31st March. Here is what needs to be done:

PAYMENT SUMMARY (25TH)

• C: @tienifr $500 (please accept offer)
• C+: @hungvu193 $500 (please accept offer + complete checklist)

UPWORKS JOB

[hungvu193]

• The PR that introduced the bug has been identified. Link to the PR: feat: category settings page #37209
• 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/37209/files#r1529785049
• 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
• Determine if we should create a regression test for this bug.

Regression test:
Precondition:
User is an admin of Collect workspace.

1. Go to staging.new.expensify.com
2. Go to Workspace settings > Collect workspace.
3. Go to Categories.
4. Click Add category.
5. Add category in Parent/ and Parent/: Child format. And other special characters
6. Click on the created category.
7. Verify that Category RHP opens without issue.

Do we 👍 or 👎

[luacmartins]

No need for regression tests. We'll add them as part of the design doc once the feature is fully implemented.

[mallenexpensify]

@tienifr @hungvu193 can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01425421b068a188c5

[hungvu193]

@tienifr @hungvu193 can you please accept the job and reply here once you have?

https://www.upwork.com/jobs/~01425421b068a188c5

Accepted! Thank you

[tienifr]

@mallenexpensify I’ve accepted the offer, thanks!

[mallenexpensify]

Contributor: @tienifr paid $500 via Upwork
Contributor+: @hungvu193 paid $500 via Upwork.

TestRail GH created - https://github.com/Expensify/Expensify/issues/383897

Thanks!

[jliexpensify]

Thanks Matt, appreciate the assist!