[HOLD for payment 2024-06-06] [HOLD for payment 2024-05-03] Use Fallback Avatar Pattern for Unknown Users

[grgia]

Background

Slack convo: https://expensify.slack.com/archives/C036QM0SLJK/p1710942820060629

Colorful / Default Avatars

Fallback Avatar

Problem

We have a lot of issues surrounding avatar behavior. Some of this is due to how we originally designed the Default Avatar logic, which used a user's login to calculate. This meant that a default avatar could be correctly guessed for any user. However, due to email and privacy concerns, we changed this logic to use the accountID.

Now, any optimistic account ID that is later replaced results in a mismatch of avatars. This has caused a group of bugs all with the same RCA.

Solution

Using the color default avatars as a placeholder avatar causes confusion, so let's stop using them as defaults for users we don't know. From now on, let's always use the fall back avatar. We should be storing the default avatars the same way we store a user-uploaded avatar. So we should stop optimistically setting avatars for users that you don't have in ONYX.

Rules for Avatars

From now on, we want to treat avatars as the following:

• Colorful default avatars are considered assigned avatars and should be treated the same as when a user has a custom uploaded avatar
• Fallback avatars are used whenever a user’s accountID is unknown or when there’s a problem fetching the avatar (isOffline)
• You’ll only see avatars for users you have in ONYX

Issue Owner

Current Issue Owner: @puneetlath

[hayes102]

Hello @grgia is this issue open for proposals ?

[melvin-bot]

📣 @hayes102! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[grgia]

@hayes102 I'm waiting to hear back from someone at one of our agencies about this.

#38674

For context, I started working on this in this PR- I have the search bar working with this pattern, but there's some funkiness around money request search options or chatting a new user, so I am planning to take those changes out for now and have this PR only add the theme support and tackle those changes with this issue. That said, we will begin taking proposals by tomorrow if the issue is not taken by SWM.

[Kicu]

hey I work for SWM and would like to work on this one

[grgia]

@Kicu I removed the fallback changes I made in the PR- 44dc9fc

see this commit for the search, as well as this thread:

#38674 (comment)

[Kicu]

Thank you @grgia I will be working on this today

[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.

[melvin-bot]

This issue has not been updated in over 15 days. @Kicu, @s77rt, @grgia eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[rlinoz]

This didn't actually cause the blocker, sorry.

[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.

[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.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[puneetlath]

What's left to do here?

[Kicu]

Nothing to do, PR is open awaiting on review

[puneetlath]

Still being reviewed?

[s77rt]

Waiting for the engineer review and for the merge freeze I suppose

[Kicu]

yup. Let's ping either @mountiny or @grgia if you could take a look and review?
From what I hear the merge freeze might be off tomorrow.

This PR touches a lot of places in the code and so I would really like to merge it sooner rather than later.

[mountiny]

Yep just waiting for the merge freeze to be lifted now.

[puneetlath]

We're now in merge slush. Can we review?

[mountiny]

I have approved and requested a merge

[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.

[mountiny]

We have cleared that

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.77-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Use fallback user avatar in cases where the user is unknown to us #41846

If no regressions arise, payment will be issued on 2024-06-06. 🎊

For reference, here are some details about the assignees on this issue:

• @Kicu does not require payment (Contractor)
• @s77rt requires payment (Needs manual offer from BZ)

[melvin-bot]

Payment Summary

Upwork Job

• Contributor: @Kicu is from an agency-contributor and not due payment
• ROLE: @s77rt paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@puneetlath)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[puneetlath]

I just want to confirm that any regressions have been fixed and we are ready to pay here? And that @s77rt is the only person needing payment?

[s77rt]

Yes all has been handled

[puneetlath]

Ok offer here: https://www.upwork.com/nx/wm/offer/102641250

[puneetlath]

Please ping me on this issue when you've accepted.

[s77rt]

Accepted! Please make the payment $62.5 for the initial regression and for this regression #42750

[puneetlath]

That was surprisingly hard, but I figured it out.

All paid, thanks y'all!