[$500] Incorrect avatar being displayed for expense report row in LHN

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.7.0
Reproducible in staging?: y
Reproducible in production?: y
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
Expensify/Expensify Issue URL:
Issue reported by: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1701391697330139

Action Performed:

1. Submit an expense on a workspace.
2. Then look at the expense report row that appears in the LHN

Expected Result:

My correct avatar should show up

Actual Result:

A default backup avatar shows in place of my own photo

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.288.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a355eb499fe4a8bb
• Upwork Job ID: 1731338946374533120
• Last Price Increase: 2023-12-03

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[shubham1206agra]

Proposal

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

Incorrect avatar being displayed for expense report row in LHN

What is the root cause of that problem?

This is due to a bug in getting participants correctly.
Line

App/src/components/LHNOptionsList/LHNOptionsList.js

Line 121 in 1682be4

const participants = [ReportUtils.getParticipantsIDs(itemFullReport), itemFullReport.ownerAccountID];

Here, it will create a list of lists instead of a list, as it didn't use the spread operator.

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

Add spread operator here. And maybe remove itemFullReport.ownerAccountID if it is 0.

const participants = [...ReportUtils.getParticipantsIDs(itemFullReport), itemFullReport.ownerAccountID];

What alternative solutions did you explore? (Optional)

We can also use Set along with the above to remove duplicates.

[chiItepin]

Proposal

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

A default avatar is being displayed for a user on the left navbar even though the app already has its profile image stored.

What is the root cause of that problem?

The participants variable in the renderItem callback in LHNOptionsList.js is creating a new array with missing participantIds, on top of that getParticipantsIDs is not being spread to add them as individual elements in the participants array.

That being said, I don't think there's a real need for spreading and creating a new array.

App/src/components/LHNOptionsList/LHNOptionsList.js

Line 121 in 1682be4

const participants = [ReportUtils.getParticipantsIDs(itemFullReport), itemFullReport.ownerAccountID];

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

Instead of creating a new array, we should provide the ReportUtils.getParticipantsIDs reference and even drop the itemFullReport.ownerAccountID property inside the renderItemcallback, this is because ReportUtils.getParticipantsIDs already comes with the full list of participantIds including the ownerId, so there's no need to create a new array and on top of that manually set the ownerId, it would make the code less maintainable.

Before

After

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[fedirjh]

This is a regression from #32113 , cc @bernhardoj @alitoshmatov

[situchan]

Note: the original issue (expense request main avatar) happening on production now fixed on staging. This was reported by @shawnborton 3 days ago.
The new issue (expense report main avatar) is not yet deployed to staging and it's just regression from #32113 which was merged into main 2 days ago.

[chiItepin]

Q: Have you guys considered using the ReportUtils.getParticipantsIDs reference instead of creating a new array? would make it more readable thus, more maintainable to prevent these regressions in the future

[situchan]

Q: Have you guys considered using the ReportUtils.getParticipantsIDs reference instead of creating a new array? would make it more readable thus, more maintainable to prevent these regressions in the future

The offending PR author/reviewer might have answer. It was introduced to fix #31372.

[bernhardoj]

Opened a PR to fix it

[shubham1206agra]

@mallenexpensify Can I get any compensation for the fix, as I was the one pointing towards the problem, and my solution is used in #32421?
Proposal: #32416 (comment)

[mallenexpensify]

That's a good question @shubham1206agra , situations like this rarely occur so we don't have concrete documentation to point too. Let me address internally.

[mallenexpensify]

Actually... before I raise the issue internally, @fedirjh , would you agree with @shubham1206agra that their proposal above

• was the one pointing towards the problem, and my solution is used in Fix custom avatar is not showing in LHN avatars #32421? ?

[fedirjh]

@mallenexpensify Yes, that's correct. Also, the offending PR is still in the regression period and the author should handle it. In similar situations, if this bug was a deploy blocker, I think @shubham1206agra would have been assigned to it. So I think it's fair to offer partial compensation in this case.

[mallenexpensify]

Thanks @fedirjh, our standard for payments for edge cases is 25, 50 and 100% of the issue price. I propose we compensate @shubham1206agra 50% here, do you agree @fedirjh and @shubham1206agra ?

[mallenexpensify]

@shubham1206agra can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01a355eb499fe4a8bb

[shubham1206agra]

Done

[mallenexpensify]

Contributor: @shubham1206agra paid $250 via Upwork

I belive that's the only payment due here, right?

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[fedirjh]

I belive that's the only payment due here, right?

Yes, it is. I think this can be closed.

[mvtglobally]

Issue not reproducible during KI retests. (Second week)