Look into making report display name generation more efficient

[tgolen]

Coming from #11226 (comment)

Problem

There is code that generates the display name of a report. This code may be non-performant at scale and turn into a bottle neck.

Solution

Consider some solutions:

1. memoizing the report name generation
2. generating the display name on the server
3. storing the displayName in a map instead of cloning the reports in a map

@marcaaron I'm particularly interested in your thoughts on solution 2. The report name will only change when:

1. The participants change (I don't think we ever change participants... We either archive the report or create a new report when adding participants
2. The type of chat report changes (I don't think it's possible to change the type of a chat report)
3. The chat is archived (valid reason)
4. The details of the participants change (valid reason)

Because of point 4, maybe that's the best reason to keep the display name generation on the client and to use solution 1 or 3 above?

[melvin-bot]

Triggered auto assignment to @michaelhaxhiu (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[michaelhaxhiu]

@tgolen would it be best to close this for and make an #expensify-open-source discussion/proposal? And then depending on what we discover there, we create a GH to handle next steps?

Just asking because as a triager, I'm struggling to assess my next step until this is definitively framed as a bug or newfeaure.

[michaelhaxhiu]

An alternative is to un-assign triage until it's ready for being pushed through the assignment flow. I'll leave it assigned for a few days till you have a moment to consider.

[marcaaron]

@tgolen report names can also be changed manually (in the case of user created rooms) so I'd add that case to the list

I like the idea of sending this from the backend as a new reportDisplayName field. Since we do not yet notify people in realtime when personal details change I don't think we can really do much about that point (until we have realtime updating of details and then should also update the report names when they change).

Overall, I think it is reasonable to send an updated reportDisplayName whenever one of the other things on your list happens.

[tgolen]

@michaelhaxhiu I think this is just a cleanup task that I'll work on internally. Sorry, I usually remember to omit the AutoAssignerTriage label when I do that :D

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

The top for items here are N/A since this issue was not a bug and therefore would not have any regression linked to it.

• N/A A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• N/A The PR that introduced the bug has been identified. Link to the PR:
• N/A 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 A discussion in #contributor-plus 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:
• Payment has been made to the issue reporter (if applicable)
• Payment has been made to the contributor that fixed the issue (if applicable)
• Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[tgolen]

Yeah, I confirmed that the PR from this caused the regression. It was a spot I missed in the code when I changed a method signature.

[melvin-bot]

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

[tgolen]

I'm reassigning the bug label to have someone from the BZ team close this out.

[NicMendonca]

The PR that introduced the bug has been identified. Link to the PR:

@tgolen is there a PR that would have introduced this GH?

[melvin-bot]

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

[tgolen]

Sorry @NicMendonca I missed the updates to this. I think this issue is a little confusing. For the purposes of closing this out, this issue was not a bug and so there could be no regression. It was just a code cleanup. I think for the BugZero checklist, the remaining checkboxes are all "N/A" and it's OK to close this.