[$1000] Default profile avatar is different in tooltip

[Beamanator]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to any chat with default tooltip
2. hover over profile pic

Note: This happens in multiple places in the app

Expected Result:

Profile should not be different in tooltip

Actual Result:

Profile is different in tooltip

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: main (latest)
Reproducible in staging?: N
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686828292914429

• https://expensify.slack.com/archives/C049HHMV9SM/p1686830645725319

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cd1cc4ea3187cd31
• Upwork Job ID: 1669689115344900096
• Last Price Increase: 2023-06-16

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[s-alves10]

I have done similar issue before(#20713) and I think I have most of the context regarding the issue. Can I work on this?

[Beamanator]

@s-alves10 please do, but please provide a proposal how you think this can be fixed first 🙏

[spcheema]

Proposed solution

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

Default profile avatar is different in tooltip

What is the root cause of that problem?

Invalid value being passed to the UserUtils.getAvatar method. Based on the recent change accountID is expected not login key

App/src/libs/UserUtils.js

Line 150 in d396430

function getAvatar(avatarURL, accountID) {

    const avatarSource = UserUtils.getAvatar(avatar, actorAccountID);

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

The similar inconsistency found on multioplaces:

• src/libs/OptionsListUtils.js line no. 840

App/src/libs/OptionsListUtils.js

Line 840 in d396430

source: UserUtils.getAvatar(personalDetail.avatar, personalDetail.login),

• src/pages/home/report/ReactionList/BaseReactionList.js line no. 90

App/src/pages/home/report/ReactionList/BaseReactionList.js

Line 90 in d396430

source: UserUtils.getAvatar(item.avatar, item.login),

• src/components/UserDetailsTooltip/index.js line no. 22

App/src/components/UserDetailsTooltip/index.js

Line 22 in d396430

source={UserUtils.getAvatar(userDetails.avatar, userDetails.login)}

Replace login key with accountID on all three files above listed.

What alternative solutions did you explore? (Optional)

NA

[grgia]

Avatar needs name, source, and type props for the default avatar to work. I also reported that the workspace avatars are incorrect for the same reason. I can potentially also take this one

[BhuvaneshPatil]

Proposal

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

Default profile avatar is different in tooltip

What is the root cause of that problem?

We are using login here in UserDetailsTooltip

App/src/components/UserDetailsTooltip/index.js

Lines 18 to 33 in d396430

	<View style={[styles.alignItemsCenter, styles.ph2, styles.pv2]}>
	<View style={styles.emptyAvatar}>
	<Avatar
	containerStyles={[styles.actionAvatar]}
	source={UserUtils.getAvatar(userDetails.avatar, userDetails.login)}
	/>
	</View>
	<Text style={[styles.mt2, styles.textMicroBold, styles.textReactionSenders, styles.textAlignCenter]}>
	{String(userDetails.displayName).trim() ? userDetails.displayName : ''}
	</Text>
	<Text style={[styles.textMicro, styles.fontColorReactionLabel]}>
	{String(userDetails.login).trim() && !_.isEqual(userDetails.login, userDetails.displayName) ? Str.removeSMSDomain(userDetails.login) : ''}
	</Text>
	</View>

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

We shall use accountID instead

What alternative solutions did you explore? (Optional)

[s-alves10]

I think @BhuvaneshPatil 's proposal is correct. During the last merge, UserUtils.getAvatar function changes, its 2nd parameter from login to accountID

[spcheema]

@s-alves10 Can you please review my proposal. It requires changes on multiple files?

[spcheema]

@grgia Yeah there are some places where changes were left unintentionally but the root cause it the same login is used instead of accountID

[twisterdotcom]

@spcheema - can I ask about why you post comments like this on issues to begin with?

Just intrigued about why and whether we can improve our processes at all? Are you only able to see notifications after you comment for some reason?

It's worth noting we don't filter by the oldest proposal, only the best and if two are the same, the least recently edited, so there isn't a benefit to posting like that I'm aware of.

[twisterdotcom]

@alex-mechler is this resolved by #20818 as well?

[alex-mechler]

This is not @twisterdotcom

[s-alves10]

Is this still reproducible on staging? I can't reproduce

[gadhiyamanan]

@s-alves10 this is reproducible in dev(main branch) not in staging

[BhuvaneshPatil]

@s-alves10 @gadhiyamanan
I just took a fresh pull of master, the bug is there (v1.3.28-4)

5.New.Expensify.Mozilla.Firefox.2023-06-15.22-54-11.mp4

[spcheema]

@twisterdotcom Sure I will make sure not to repeats this. I was supposed to type I solution but pulled off by somethings. But agree with you.

And also at some other places I put empty comment to subscribe updates. But will make sure not to cause any kind interruptions. Thanks for highlighting

[twisterdotcom]

No worries @spcheema! I generally use the Subscribe option to stay up to date on issues I'm not yet participating in:

[spcheema]

Noted 🖋️ thank you @twisterdotcom

[twisterdotcom]

I can't really reproduce until it's on staging, and I still can't reproduce now:

2023-06-16_10-24-17.mp4

[Beamanator]

Yeah sadly the code that potentially broke this is still in main / only the dev environment. I'll test real quick

[twisterdotcom]

I think if it is, let's just throw External on it and assign @BhuvaneshPatil if the C+ assigned agrees.

[Beamanator]

I can reproduce in main 👍

(Note: There's no email shown in my environment b/c I'm testing some other changes)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01cd1cc4ea3187cd31

[melvin-bot]

Current assignee @twisterdotcom is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[twisterdotcom]

@eVoloshchak - what do you think of the proposals here?

[twisterdotcom]

Bump @eVoloshchak to review these proposals.

[eVoloshchak]

@BhuvaneshPatil's proposal looks good to me, I think we should proceed with it

🎀👀🎀 C+ reviewed!
cc: @twisterdotcom

[situchan]

This was fixed already (#21026 (comment))

[situchan]

Latest codebase:

App/src/components/UserDetailsTooltip/index.js

Lines 20 to 23 in e9a7a54

	<Avatar
	containerStyles={[styles.actionAvatar]}
	source={UserUtils.getAvatar(userDetails.avatar, userDetails.accountID)}
	/>

[eVoloshchak]

Awesome, thanks @situchan!

[BhuvaneshPatil]

Yeah, this was fixed by @Beamanator - 2a67b3e

[twisterdotcom]

Okay, let's close then. Dupe of #21026 (comment)