Skip to content

check if report.icons and report.icons[0] exists#5381

Merged
tgolen merged 3 commits into
mainfrom
marco-missingAvatarLHN
Sep 23, 2021
Merged

check if report.icons and report.icons[0] exists#5381
tgolen merged 3 commits into
mainfrom
marco-missingAvatarLHN

Conversation

@marcochavezf

@marcochavezf marcochavezf commented Sep 20, 2021

Copy link
Copy Markdown
Contributor

Details

Context: #5232

Sometimes the avatars disappear from the LHN and I was only able to reproduce the issue after logging out and loging in again with a different account.

I found out the value of report.actions sometimes is undefined or has an array with an empty string (['']). This is because the links of the avatar thumbnails are fetched after inserting the simplifiedReports into Onyx:

Screen Shot 2021-09-20 at 16 03 26

here ^ PersonalDetails.getFromReportParticipants(Object.values(simplifiedReports)) is called to fetch the avatar thumbnails asynchronously (using API.PersonalDetails_GetForEmails), which resolves after calling
Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, simplifiedReports) which triggers a re-render synchronously and calls the function createOption before fetching the avatars:

Screen Shot 2021-09-20 at 16 17 18

My hypothesis is that the avatar won't appear if API.PersonalDetails_GetForEmails doesn't resolve because of a connection issue (i.e. internet goes offline exactly when the app is fetching the avatars). So I added more conditions to check not only if the report object exists but also the icons and icons[0] has a valid value; I found sometimes icons[0] is an empty string too: https://github.com/Expensify/App/blob/main/src/libs/OptionsListUtils.js#L760 (which is called in PersonalDetails.getFromReportParticipants).

Seems the issue was only reproducible on Desktop and Web.

Fixed Issues

$ #5232

Tests

  1. Log in
  2. Log out
  3. Log in with a different account

QA Steps

  1. Log in
  2. Log out
  3. Log in with a different account

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screen Shot 2021-09-20 at 18 35 51

Mobile Web

Desktop

Screen Shot 2021-09-20 at 18 38 32

iOS

Android

@marcochavezf marcochavezf requested a review from a team as a code owner September 20, 2021 23:08
@MelvinBot MelvinBot requested review from tgolen and removed request for a team September 20, 2021 23:08
@marcochavezf marcochavezf self-assigned this Sep 21, 2021
Comment thread src/libs/OptionsListUtils.js Outdated
? lastMessageText
: Str.removeSMSDomain(personalDetail.login);
}
const icons = (report && report.icons && report.icons[0]) ? report.icons : [personalDetail.avatar];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please use Lodash.get instead for this? I think it would make this more simple.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

@marcochavezf

Copy link
Copy Markdown
Contributor Author

Update: use lodashGet to retrieve either report.actions or [personalDetail.avatar]

@marcochavezf marcochavezf requested a review from tgolen September 23, 2021 20:27

@tgolen tgolen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that looks great. Thanks!

@tgolen tgolen merged commit dcedffb into main Sep 23, 2021
@tgolen tgolen deleted the marco-missingAvatarLHN branch September 23, 2021 22:26
@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @tgolen in version: 1.1.1-9 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @timszot in version: 1.1.2-7 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants