Skip to content
New issue

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

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

Already on GitHub? # to your account

[$1000] Search - User custom avatar is not shown in search suggestion #24282

Closed
2 of 6 tasks
lanitochka17 opened this issue Aug 8, 2023 · 47 comments
Closed
2 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 8, 2023

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


Issue found when executing PR #24019

Action Performed:

  1. Open staging.new.expensify.com or start Desktop App
  2. Login with Expensifail.account
  3. Create several chats with gmail and expensifail users with custom avatars
  4. Go to Search
  5. Check user's avatars in search suggestions
  6. Go to FAB > Assign Task > Input any Title > Next > Share somewhere (This issue also impacts search on Assignee)
  7. Check user's avatars in share suggestions

Expected Result:

Step 5 and 7 > All users have the same avatars in the list

Actual Result:

Step 5 and 7 >
Gmail users have a default avatar in the list and a custom avatar in report when opened (e.g. Chat report of a task)

Workaround:

Unknown

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: 1.3.51.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6158037_Search-Result-Task-Share-No-Avatar.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0187747fcfdb75601f
  • Upwork Job ID: 1692552438131240960
  • Last Price Increase: 2023-09-01
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 8, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 8, 2023

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

@daordonez11
Copy link
Contributor

daordonez11 commented Aug 8, 2023

Proposal

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

The user avatar is shown in the tooltip but not in the search result

What is the root cause of that problem?

Inconsistency occurs because in the method getOptions the personalDetails list is filtered by the elements that contain login. Since there is no interaction with the gmail accounts they do have avatar but no login. Since UserDetailsTooltip searches in the personalDetails list without the filter it does find the avatar and the account information. Also, MultipleAvatars behaves differently than DisplayNames because in the first we are sending the icon as a prop.

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

Here are some ways of solving this since "unknown" users is a complex topic:

  1. Since we already have the filter to avoid issues and interactions we can include the same filter in UserDetailsTooltip so the icon will not appear in the tooltip. (But as it can be seen even in LHN the avatar is already shown, so it does not make so much sense to "hide it" here)

function UserDetailsTooltip(props) {
const {translate} = useLocalize();
const userDetails = lodashGet(props.personalDetailsList, props.accountID, props.fallbackUserDetails);
let userDisplayName = userDetails.displayName ? userDetails.displayName.trim() : '';

Here include the same filter of personalDetails = _.pick(personalDetails, (detail) => Boolean(detail.login));

  1. Instead of doing the filter we can map the list and bring only the "public" attributes so some information can be shown like avatar and displayName, filter by login is a filter that should be done in the backend not in the frontend because we can see this type of inconsistency.

So we can replace personalDetails = _.pick(personalDetails, (detail) => Boolean(detail.login)); with something like

_.each(personalDetails, (detail) => {
        if (Boolean(detail.login)) {
            personalDetails[detail.accountID] = {
                accountID: detail.accountID,
                avatar: detail.avatar,
                displayName: detail.displayName,
            }
        }
    });

Update: After reading some history option 2 does not work because it avoids creation of groups and other actions. Maybe we could only use this solution when in SearchPage
cc @Beamanator @puneetlath that have worked with "unknown" users to get their perspective on how to solve it.

Or a 3rd option
Same as UserDetailsTooltip load the avatars in MultipleAvatars directly from personalDetails instead of props.

What alternative solutions did you explore? (Optional)

@tienifr
Copy link
Contributor

tienifr commented Aug 9, 2023

Proposal

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

Search - User custom avatar is not shown in search suggestion but is shown in tooltip

What is the root cause of that problem?

When we open report with someone who does not interact with, we will create the optimistic data for personalDetails within login value and default avatar. But BE returns the different personalDetail without login value and actual avatar.

In SearchPage we show the list report, report contains the participants (accountIDs returned from BE)

In

personalDetails = _.pick(personalDetails, (detail) => Boolean(detail.login));

We're filtering out the personalDetail that don't have login value => the personalDetail returned from BE is filtered out. We're using this personalDetails to get the icons.

result.icons = ReportUtils.getIcons(report, personalDetails, UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID), false, personalDetail.login, personalDetail.accountID);

=> We can't find report.participants in personalDetails => option.icons is the default icon

We use this icon to show in option

<MultipleAvatars
icons={this.props.option.icons}
size={CONST.AVATAR_SIZE.DEFAULT}
secondAvatarStyle={[
StyleUtils.getBackgroundAndBorderStyle(themeColors.appBG),
this.props.optionIsFocused ? StyleUtils.getBackgroundAndBorderStyle(focusedBackgroundColor) : undefined,
hovered && !this.props.optionIsFocused ? StyleUtils.getBackgroundAndBorderStyle(hoveredBackgroundColor) : undefined,
]}
shouldShowTooltip={this.props.showTitleTooltip && OptionsListUtils.shouldOptionShowTooltip(this.props.option)}
/>

We're using UserDetailTooltip to show the avatar when user hovering on the title

<UserDetailsTooltip
key={index}
accountID={accountID}
fallbackUserDetails={{

it takes the accountID as the props and use it to find the avatar url => At that time we use personalDetails from onyx => the actual avatar is found here

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

Because the personalDetails in 2 places are different. So we need to fix here.

  1. We can remove these lines, because it's just the temporary fix

// We're only picking personal details that have logins set
// This is a temporary fix for all the logic that's been breaking because of the new privacy changes
// See https://github.com/Expensify/Expensify/issues/293465 for more context
// eslint-disable-next-line no-param-reassign
personalDetails = _.pick(personalDetails, (detail) => Boolean(detail.login));

  1. Remove the optimistic personal detail when BE returns the actual one in

const failurePersonalDetails = {};

        const failurePersonalDetails = {};
+        const successPersonalDetails = {};
        _.map(participantLoginList, (login, index) => {
          ...
            failurePersonalDetails[accountID] = allPersonalDetails[accountID] || null;
+            successPersonalDetails[accountID] = allPersonalDetails[accountID] || null
        });

...

+        onyxData.successData.push({
+           onyxMethod: Onyx.METHOD.MERGE,
+            key: ONYXKEYS.PERSONAL_DETAILS_LIST,
+            value: successPersonalDetails,
+       });

Result

Screen.Recording.2023-08-09.at.14.55.12.mov

@melvin-bot melvin-bot bot added the Overdue label Aug 10, 2023
@MitchExpensify MitchExpensify added the Needs Reproduction Reproducible steps needed label Aug 10, 2023
@MitchExpensify
Copy link
Contributor

I am unable to reproduce this so far adding the label

@melvin-bot melvin-bot bot removed the Overdue label Aug 10, 2023
@daordonez11
Copy link
Contributor

@MitchExpensify To reproduce this one you have to create a chat with a user that does have image and you havent interacted with previously, you can use my account dloz1996@gmail.com just remember not to send a message, if you interact with the account backend will send the login and the issue is "fixed".

@MitchExpensify
Copy link
Contributor

Thanks for the direction! This is what I saw when I tried that:

image

@melvin-bot melvin-bot bot added the Overdue label Aug 14, 2023
@MitchExpensify
Copy link
Contributor

Kind of weird but I can find dloz1996@gmail.com in Assignee but not Share somewhere:

image image

@melvin-bot melvin-bot bot removed the Overdue label Aug 14, 2023
@daordonez11
Copy link
Contributor

I think this is desired due to the issue we mentioned of the filter in both proposals @MitchExpensify but precisely the first image you sent is the error reported in this issue.

@MitchExpensify
Copy link
Contributor

@daordonez11 Are you referring to this image? I don't see the bug in that image, it seems to be the correct picture showing. Or are you referring to this image which happens in Assignee versus Share somewhere as reported in the issue?

@daordonez11
Copy link
Contributor

daordonez11 commented Aug 17, 2023

Let me include a video! @MitchExpensify

Icon.Error.-.8_17_2023.2_56_41.PM.webm

Error occurs in both search and share somewhere:

Share.somewhere.-.8_17_2023.2_59_16.PM.webm

@MitchExpensify
Copy link
Contributor

I finally reproduced this!

@MitchExpensify MitchExpensify changed the title Search - User custom avatar is not shown in search suggestion but is shown in tooltip Search - User custom avatar is not shown in search suggestion Aug 18, 2023
@MitchExpensify MitchExpensify added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Aug 18, 2023
@melvin-bot melvin-bot bot changed the title Search - User custom avatar is not shown in search suggestion [$1000] Search - User custom avatar is not shown in search suggestion Aug 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 18, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0187747fcfdb75601f

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 18, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 18, 2023

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

@MitchExpensify
Copy link
Contributor

I'm unsure if this needs internal backend work so curious what @situchan thinks 👍

@melvin-bot
Copy link

melvin-bot bot commented Sep 18, 2023

@MitchExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

@situchan
Copy link
Contributor

@daordonez11 @tienifr are you able to reproduce on latest main?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

@MitchExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@MitchExpensify
Copy link
Contributor

@daordonez11 @tienifr are you able to reproduce on latest main?

Are you still waiting on this @situchan ?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 22, 2023
@MitchExpensify
Copy link
Contributor

Bump @situchan

@melvin-bot melvin-bot bot removed the Overdue label Sep 26, 2023
@situchan
Copy link
Contributor

yes waiting for updated proposal based on latest codebase, along with clear repro step

@melvin-bot melvin-bot bot added the Overdue label Sep 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 29, 2023

@MitchExpensify, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

@MitchExpensify
Copy link
Contributor

Just to be clear @situchan, are you saying the current repro steps are not producing the current Actual Result?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 30, 2023
@MitchExpensify
Copy link
Contributor

Friendly bump @situchan

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

@MitchExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

@situchan
Copy link
Contributor

situchan commented Oct 9, 2023

@daordonez11 @tienifr can you please update proposals based on latest codebase?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 9, 2023
@MitchExpensify
Copy link
Contributor

Friendly bump @daordonez11 @tienifr

@melvin-bot melvin-bot bot removed the Overdue label Oct 11, 2023
@tienifr
Copy link
Contributor

tienifr commented Oct 16, 2023

I can't reproduce with the latest main

@melvin-bot melvin-bot bot added the Overdue label Oct 16, 2023
@MitchExpensify
Copy link
Contributor

Ok lets close unless someone can reliably reproduce

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

5 participants