Search-Search result shows fallback avatar and email instead of custom avatar and display name

[IuliiaHerets]

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: 9.0.51-1
Reproducible in staging?: Y
Reproducible in production?: N
Issue was found when executing this PR: #48652
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Click on the search icon in any report.
3. Enter an email address that has custom avatar and display name.

Expected Result:

The search result will show user with custom avatar and display name.

Actual Result:

The search result shows user with fallback avatar and email address as display name instead of custom avatar and custom display name.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6639700_1729346685510.bandicam_2024-10-19_22-01-20-118.mp4

View all open jobs on GitHub

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @MarioExpensify (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[hannojg]

Hey, I was responsible for the accused PR. Feel free to assign this to me I can look into it today!

[hannojg]

Hm, i can neither reproduce this on staging or latest dev commit:

Screen.Recording.2024-10-21.at.10.18.47.mov

The PR hasn't changed anything about how we display search results, it just changed how we filter for them. So i am not really sure if the PR could've caused such behaviour.

[MarioExpensify]

I also cannot reproduce here, either from the Desktop or the WebApp, both react to the search the same as Production App.
Maybe we could get this retested?

[MarioExpensify]

Retest requested. Just making sure before moving forward.

[izarutskaya]

Still reproducible on staging
Build v9.0.51-2
And different behavior in production

Recording.2867.mp4

Recording.2868.mp4

[MarioExpensify]

Hey @hannojg, mind taking another look? I looked through other PRs and did not find any other that may be the cause for this issue. Feel free to reach me out if you think it is unrelated to your latest changes.

[marcaaron]

That PR was reverted it seems. @MarioExpensify assigning myself since I merged that PR, but feel free to stay assigned.

[melvin-bot]

@hannojg @isabelastisser @marcaaron this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[hannojg]

Guys sorry, this completely slipped past me! I will take another look at that one this week! (If this is related to the mentioned PR, it currently shouldn't be reproducible on either staging on prod, as the PR has been reverted. I am working on a new PR, and will keep this bug in mind this time.)

[hannojg]

Okay, so after investigating this, this isn't a problem with the PR originally mentioned:

• Search suffix tree implementation #48652

The problem seems to be that sometimes you get a 407 error for the search query, thus we are not getting any data from the backend:

We can see from the screenshot that the network command for the search was fired before the user got re-authenticated.

I originally got assigned to this issue because it was suspected that my PR caused this issue - which it didn't. Our team (margelo) can look into the issue if you like, or you can make it external i think.

[isabelastisser]

@hannojg, thanks for the update! Can you please continue to work on this issue? Thanks!

[hannojg]

@chrispader can you comment here? Chris from the Margelo team will work on a fix for this ticket (as i am very busy with other tasks rn).

[chrispader]

commenting for assignment :)

[isabelastisser]

hey @chrispader, I just assigned you. Thanks!

[marcaaron]

@chrispader do you have an update for this one?

My understanding is that the Reviewing label should not have been applied and nobody is working on this yet.

[isabelastisser]

Hey @chrispader, can you please follow up on the questions above? Thanks!

[chrispader]

Hey so sorry for the massive delay! I was totally busy on another PR and should have looked into this way earlier! 🫠

I investigated the problem and can clearly reproduce the problem for a particular set of accounts/email addresses.

Problem

The problem essentially is that the response received from the backend for SearchForReports sometimes returns a login (email) property in the fetched (to-be merged) personalDetails, whereas sometimes not.

If no login property is provided in the response from the backend, the personal details option received from useOptionsList() will not have a login property and therefore the logic in OptionsListUtils.filterOptions() is not able to match it to the email that the user types in the search field.

Because the logic couldn't find any personal details item, we will just add a "blank" options list item to the options list and therefore the default avatar is used.

Solution

Because of this i would argue that this is a backend issue and the SearchForReports should always return a login property for the Onyx key personalDetailsList_. Looking into the local Onyx DB, we also don't really have any other data to match the (typed in) email address to the personal details, except for the name (could be ambiguous) or the account ID, where we would have to do a very complex and costly lookup.

Backend responses for different accounts/email addresses and Onyx DB screenshot

• [email protected]: (same as in the repro video)

• [email protected]:

• Onyx DB personalDetailsList_ key:

[chrispader]

I don't think it is on purpose, that the backend doesn't send a login property for some accounts, right?

Or am i missing something here?

cc @marcaaron @isabelastisser

[marcaaron]

AFAICT there's really only one case where we would not return a login in the personalDetail.

There is a property called isKnownUser and if it's false then no login is returned (actually a few other fields will not be returned either pronouns, timezone, phoneNumber and validated). Internal code here.

I think this feature is working as intended. @puneetlath worked on this and can give a second opinion. But my guess for this issue based on the evidence provided is that we didn't make the connection to isKnownUser feature.

[puneetlath]

That sounds correct to me. You should get the full personal details, including login, when you "know" someone. You should get the partial personal details when you don't.

[chrispader]

That sounds correct to me. You should get the full personal details, including login, when you "know" someone. You should get the partial personal details when you don't.

Ok i understand. Does that mean, that since we don't receive these infos, we also don't want to show things like the avatar?

Without the email, address we cannot really match it with the search input... e.g. when i'm searching for an email address of a user i "don't know", e.g. "[email protected]", we receive the missing data like avatar and name from the SearchForReports query, but cannot match it with the mail address.

[marcaaron]

I think what is being reported is probably fine and we should close this out (but seems like it could reopen again as a suspected bug). We could maybe improve the UX so that "not known" users show up with a ? avatar or something, but I don't feel strongly about prioritizing this right now.

[isabelastisser]

Closing this!