[LOW] Tags: Support independent, multi-level tags on workspace requests

[puneetlath]

It is possible for Collect policies to be configured to have independent multi-level tags if they are connected to QBO. As such, let's add support for creating/editing requests with independent multi-level tags.

I'm guessing there will need to be both front-end and back-end components to this, but I'll leave it to @yuwenmemon to figure out the details.

Issue Owner

Current Issue Owner: @yuwenmemon

[melvin-bot]

Triggered auto assignment to @garrettmknight (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[yuwenmemon]

I think this will be relatively simple, we already import all levels of tags into NewDot when you have them turned on:

cc @amyevans in case you can think of any gotchas that might be out there.

[yuwenmemon]

Onyx updates from auth and PHP are already supported as well.

[yuwenmemon]

Yeah, it will be mostly if not exclusively changes in the front-end. Mostly around turning our current code, which assumes a single tag list, to juggle the potential for multiple tags.

For instance, one thing to consider will be how to manage routes. The current setup is agnostic to any sort of "level" - we just show ${iouType}/new/tag/${reportID} or create/${iouType}/tag/${transactionID}/${reportID}/ or r/:threadReportID/edit/:field. We'll need to reconfigure this to factor in the index of a tag (or maybe the tag name, not sure which would be better 🤔), so we can edit the tag properly on the expense as well as show the correct tag list.

For saving tags on an Expense, we can do what we do in OldDot, which is delimit them using colons:
"Ops:Asia:Project 3". Our API assumes this format anyway.

The policyRecentlyUsedTags_ collections are already keyed by the name of the tag level. However, there's a bug because we're using the tag name to key the recently used tags list. If you change the tag name, it will think it's a new tag entirely:

[yuwenmemon]

One backend change we'll need to make is to start sending the tag level in the MODIFIEDEXPENSE system message. Right now it ignores tag level, and we just assume the tag name is the first tag in the tagLists

[yuwenmemon]

I'll post some questions in the #wave6 room in the morning but so far this looks like it could be 3 GHs, with the main front-end one being able to be worked on by a Contributor.

[amyevans]

one thing to consider will be how to manage routes

Yeah this was the biggest "gotcha" we saw going into the initial tags implementation in NewDot. We initially wrote in the doc that we'd use the tag list name in the route, "slug-ifying" the name (spaces to dashes, lowercase all letters, etc), and then decided to punt that to be a future problem (now 😄). Your idea of using the index is interesting though, could definitely side step some of the edge cases I was dreaming up with weird tag list names!

Being careful around the splitting on colon and edge cases with that is another potential gotcha (for example this code probably needs to be ported to NewDot: https://github.com/Expensify/Web-Expensify/blob/main/site/lib/expenseUtils.jsx#L10-L39).

But yeah overall I agree the lift isn't too bad here really!

[yuwenmemon]

Discussion about the route here: https://expensify.slack.com/archives/C01GTK53T8Q/p1703100843219419

Seems like we're leaning towards using an index-based route. Going to create an issue to fix the renaming bug with policyRecentlyUsedTags_ and work on it, as that's a pre-existing bug that's a prerequisite to this feature anyway.

[melvin-bot]

@garrettmknight, @yuwenmemon Huh... This is 4 days overdue. Who can take care of this?

[garrettmknight]

Yuwen's out till 1/3

[yuwenmemon]

Created the front-end issue above and tagged @rezkiy37 and @waterim - are either of you two interested in working on this? It's an extension of the tags feature you helped plan/build before.

[rezkiy37]

Sounds interesting, I would like to work on the issue. Posting a comment...

[yuwenmemon]

@rezkiy37 asked a few questions in the issue linked above - answered them coming back from OOO

[greg-schroeder]

Where do we stand on this one? Are you still taking this @rezkiy37?

[rezkiy37]

Where do we stand on this one? Are you still taking this @rezkiy37?

I am actively working on this sub-issue: #33983.

[yuwenmemon]

@rezkiy37's PR is awaiting further review from @allroundexperts

[yuwenmemon]

This is done. One follow up bug to fix here: #37054