[HOLD for VSB] [$250] Public room - Fallback avatars displayed/avatars don't load when scrolling up as anon user

lanitochka17 commented 2 months ago

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.81-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4610474 Email or phone of affected tester (no customers): Issue reported by: Applause - Internal Team

Action Performed:

Navigate to a public room with long existing chat history, e. g. https://staging.new.expensify.com/r/2652281908731856 as anon user
Scroll to the top of the conversation

Expected Result:

Avatars of users should load along with the chat history

Actual Result:

Avatars of users do not load, fallback avatars are displayed instead Avatars do not load even after a few minutes waiting time

Workaround:

Unknown

Platforms:

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/5d548520-ec77-4650-9806-d870a6778ccc

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01897ff6965422a788
Upwork Job ID: 1800422322383265958
Last Price Increase: 2024-06-11

Issue Owner

Current Issue Owner: @Christinadobrzyn

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 commented 2 months ago

@Christinadobrzyn FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 commented 2 months ago

We think that this bug might be related to #vip-vsp

Christinadobrzyn commented 2 months ago

I can reproduce based on the steps in the OP - I think this can be external

melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01897ff6965422a788

melvin-bot[bot] commented 2 months ago

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

truph01 commented 2 months ago

Proposal

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

Avatars of users do not load, fallback avatars are displayed instead Avatars do not load even after a few minutes waiting time

What is the root cause of that problem?

In a public room there're a lot of actors because a lot of people send messages there.

When the report is initially loaded, it will only load a subset of the latest report actions, along with the personal details for the actors of those report actions.

When the user scrolls up, older report actions are loaded by GetOlderActions, but the personalDetails of the actors of those older actions are still lacking.

So there's no user avatar to show and there'll be the disabled cursor from this line https://github.com/Expensify/App/blob/65db65069390cdd9fd386e087b6d8eed94ee22b0/src/pages/home/report/ReportActionItemSingle.tsx#L172

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

When we try to open the profile of a user from deep link, and that profile doesn't exist in personal details, we'll call openPublicProfilePage to fetch more info on those users. Likewise for calling openReport when navigating to a report route that doesn't exist in Onyx data.

So for this case of personal details missing from Onyx (note: only if it doesn't exist in Onyx, if it's an isOptimisticPersonalDetail, there will be no fetching), if the case is detected, we should also call an endpoint to fetch data on the users. We can use openPublicProfilePage.

So in https://github.com/Expensify/App/blob/65db65069390cdd9fd386e087b6d8eed94ee22b0/src/libs/ReportUtils.ts#L991

function isPersonalDetailAvailable(accountID: number): boolean {
    return !!allPersonalDetails?.[accountID];
}

In https://github.com/Expensify/App/blob/65db65069390cdd9fd386e087b6d8eed94ee22b0/src/pages/home/report/ReportActionItemSingle.tsx#L171

useEffect(() => {
        if (!actorAccountID || ReportUtils.isPersonalDetailAvailable(actorAccountID)) {
            return;
        }

        PersonalDetailsActions.openPublicProfilePage(actorAccountID);
    }, [actorAccountID]);

(This can be made a useFetchPersonalDetailIfNotAvailable hook or a regular fetchPersonalDetailIfNotAvailable JS function, for reuse in similar page)

Also, if a personal detail is being loading (it will have isLoading: true in PERSONAL_DETAILS_METADATA), we will not load it, this can be checked in the useFetchPersonalDetailIfNotAvailable hook/function or in openPublicProfilePage

More ideally, if there's a back-end endpoints that allow fetching multiple public profile at once, something like openPublicProfiles, we can batch the accountIds that are missing personal details (for instance in a 3 seconds window) and fetch them in 1 API call.

Working well after fixing: https://github.com/Expensify/App/assets/170341821/1c54442a-f799-40eb-b4f4-cb4eff429edd

What alternative solutions did you explore? (Optional)

TheGithubDev commented 2 months ago

Proposal

Re-state the problem that we are trying to solve in this issue:

Avatars of users do not load when an anonymous (anon) user scrolls up in a public room with a long chat history. Instead, fallback avatars are displayed and remain even after waiting for a few minutes.

What is the root cause of that problem?

The root cause of this problem is due to issues with loading user avatars for anonymous users when fetching older messages in a chat room. This could be caused by:

Network Requests: The network requests for fetching user avatars aren't triggered correctly or might fail when the user is anonymous.
Caching: Issues with caching that prevent avatars from being loaded correctly for anonymous users.
Permission: Anonymous users not having the proper permissions to load user avatars.

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

To address this issue, we should:

Investigate Network Requests:
- Ensure that the network requests to load user avatars are being made correctly and handle failures gracefully.
```
useEffect(() => {
if (isAnonUser && isFetchingOlderMessages) {
   fetchUserAvatars();
}
}, [isAnonUser, isFetchingOlderMessages]);
```

Adjust Caching Mechanisms:

Review and adjust the caching mechanisms to ensure avatars can be fetched correctly even for anonymous users.

if (cache.hasAvatar(userId)) {
displayAvatar(cache.getAvatar(userId));
} else {
fetchAvatar(userId)
   .then(avatar => {
       cache.setAvatar(userId, avatar);
       displayAvatar(avatar);
   })
   .catch(() => {
       displayFallbackAvatar();
   });
}

Ensure Proper Permissions:
- Check and update permissions to make sure anonymous users can access user avatars.
```
if (isAnonUser) {
ensureAvatarPermissions(userId);
}
```

Update Fallback Logic:

Improve the fallback logic to retry fetching avatars or show a loading indicator instead of static fallback avatars.

if (!avatarLoaded) {
displayLoadingIndicator();
fetchAvatar(userId)
   .then(avatar => {
       displayAvatar(avatar);
   })
   .catch(() => {
       displayFallbackAvatar();
   });
}

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

Christinadobrzyn commented 2 months ago

Just a heads up - I'm going to be ooo until June 24th so going to assign a teammate to watch this while I'm away

@kadiealexander we are collecting proposals

@sobitneupane would you be able to review the proposals we have?

sobitneupane commented 2 months ago

Thanks for your proposal @truph01

Overall your proposal looks good to me. But I am concerned about the performance as we will be making api request for each ReportActionItem without personal details. Will it cause any impact on performance?

truph01 commented 2 months ago

Will it cause any impact on performance?

@sobitneupane From my testing, I don't see any performance impact.

There likely won't be performance issues because:

For each personal detail that's missing, we'll call the API only once (if the personal details is already isLoading, we won't call)
Each time the user scrolls up and fetch older actions, there'll like 6-7 new distinct actorAccountIds, this is low and shouldn't cause issues because the openPublicProfilePage API is fast and the API calls will be sequential

sobitneupane commented 2 months ago

Thanks for the update @truph01

The proposal from @truph01 looks good to me.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @cristipaval, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

cristipaval commented 2 months ago

I'm not happy about calling an API for each missing personal detail. We should fix the backend and make it return the personal details properly when loading a new batch of report actions with the GetOlderActions command. A new API command just for fetching a batch of missing personal details might be against our 1:1:1 pattern, which requires the App to make one single API call for a user action. Scroll Up user action should trigger the GetOlderActions command only.

cristipaval commented 2 months ago

I made this one Internal for now. Next week, I'll investigate the implementation of the GetOlderActions command and come back to this issue.

melvin-bot[bot] commented 2 months ago

@cristipaval @sobitneupane @Christinadobrzyn @kadiealexander this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Christinadobrzyn commented 2 months ago

This is just an update for Melvbot. @cristipaval, are you going to take a peek at this when you have the bandwidth? Let us know if you'd like us to try to find another volunteer!

cristipaval commented 2 months ago

I'm afraid I won't be able to take care of this one this week. I have my plate full and am also assigned to the API monitoring this week.

Christinadobrzyn commented 2 months ago

Sounds good @cristipaval! I will add a hot pick label to this to see if we can get a volunteer

Christinadobrzyn commented 2 months ago

still looking for a volunteer