Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] Chat - Profile pic is not displayed for non communicated existing report in chat area #35378

Closed
1 of 6 tasks
kavimuru opened this issue Jan 30, 2024 · 14 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kavimuru
Copy link

kavimuru commented Jan 30, 2024

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: 1.4.33-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: Applause internal team
Slack conversation:

Action Performed:

  1. Open the app and login with gmail account
  2. Open search
  3. Search for user which already exists on app, has a profile pic but current logged in user has no conversation history with it (should be gmail account)
  4. Click on the result
  5. Observe that app displays profile pic in header but not in chat area

Expected Result:

App should either display profile pic in both chat area and header or should not display profile pic in either places

Actual Result:

App displays profile pic in header but does not display profile pic in chat area

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

Bug6360678_1706607798363.new_message_new_user_issue_profile.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017ddf238b8d0fa663
  • Upwork Job ID: 1752285750358851584
  • Last Price Increase: 2024-02-13
@kavimuru kavimuru added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 30, 2024
@melvin-bot melvin-bot bot changed the title Chat - Profile pic is not displayed for non communicated existing report in chat area [$500] Chat - Profile pic is not displayed for non communicated existing report in chat area Jan 30, 2024
Copy link

melvin-bot bot commented Jan 30, 2024

Job added to Upwork: https://www.upwork.com/jobs/~017ddf238b8d0fa663

Copy link

melvin-bot bot commented Jan 30, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 30, 2024
Copy link

melvin-bot bot commented Jan 30, 2024

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

@bernhardoj
Copy link
Contributor

bernhardoj commented Jan 30, 2024

Proposal

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

Profile pic in chat area doesn't show the custom avatar when starting a new chat with a new user.

What is the root cause of that problem?

When we start a new chat, the LHN and header avatar is updated after we receive the personal detail data, but not for the avatar in the chat area and that's because we memoize the component but didn't consider the personal details data.

personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
})(
memo(
ReportActionItemCreated,
(prevProps, nextProps) =>
prevProps.policy?.name === nextProps.policy?.name &&
prevProps.policy?.avatar === nextProps.policy?.avatar &&
prevProps.report?.stateNum === nextProps.report?.stateNum &&
prevProps.report?.statusNum === nextProps.report?.statusNum &&
prevProps.report?.lastReadTime === nextProps.report?.lastReadTime,
),

So, the component isn't updated when the personal detail is updated.

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

Add check for personal details too.

_.isEqual(prevProps.personalDetails, nextProps.personalDetails)

What alternative solutions did you explore? (Optional)

Or we can just use usePersonalDetails hook so we don't need to compare it in the memo.

I notice that we only check for some fields of report and policy which could potentially raise some issues too, perhaps we can remove the memo completely as it's really hard to maintain the list of attributes of each onyx data. We can totally do that, but it's just not worth the effort I guess

@ikevin127
Copy link
Contributor

ikevin127 commented Jan 30, 2024

Dupe, mentioned here: #35123
I posted the same proposal (rca, main solution) there 2 days ago as the one posted above here.

@s77rt
Copy link
Contributor

s77rt commented Jan 30, 2024

@ikevin127 Looks you reported a new (similar) bug. I think we can handle that there.

cc @sonialiap We can close this

@ikevin127
Copy link
Contributor

I can confirm that both issues are solved - not reproducible aymore 🚀
Must've been the big PR merged recently.

MacOS: mWeb Chrome
Screen.Recording.2024-02-02.at.13.42.53.mov

cc @sonialiap @s77rt

@melvin-bot melvin-bot bot added the Overdue label Feb 2, 2024
@s77rt
Copy link
Contributor

s77rt commented Feb 2, 2024

We can close this then

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Feb 2, 2024
@s77rt
Copy link
Contributor

s77rt commented Feb 5, 2024

To be closed

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2024
Copy link

melvin-bot bot commented Feb 6, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Feb 7, 2024
@s77rt
Copy link
Contributor

s77rt commented Feb 7, 2024

@sonialiap ^

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Feb 7, 2024
@s77rt
Copy link
Contributor

s77rt commented Feb 12, 2024

To be closed

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
Copy link

melvin-bot bot commented Feb 13, 2024

@s77rt @sonialiap this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Feb 13, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Feb 15, 2024
@melvin-bot melvin-bot bot removed the Overdue label Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

5 participants