[$500] Workspace - Endless skeleton loader on the member page if you switch to it multiple times

[lanitochka17]

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.63.7
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Log in with a new expensifail account
2. Create a workspace
3. Navigate to the members page
4. Navigate back
5. Navigate to the members page again and wait 20-30 seconds
6. Navigate back
7. Navigate to the members page again

Expected Result:

Skeleton loader should only be visible for a very short time

Actual Result:

Endless skeleton loader appears on the member page if you switch to it multiple times

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

Bug6455086_1713542431121.MMUC4948.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e843a4238c37c0d9
• Upwork Job ID: 1781491296362684416
• Last Price Increase: 2024-05-15

[melvin-bot]

Triggered auto assignment to @puneetlath (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

@puneetlath 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]

We think that this bug might be related to #wave-collect - Release 1

[melvin-bot]

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

[melvin-bot]

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

[mericanAncer127]

Upon thorough analysis, it's evident that the endless skeleton loader arises from delays in data fetching and rendering when switching to the member page multiple times. To address this issue, I propose the following technical changes:

1. I'll implement efficient data fetching strategies aimed at reducing latency and improving loading times. This may involve optimizing network requests, employing caching mechanisms, or exploring asynchronous data loading techniques.
2. By revisiting the component rendering logic, I'll prioritize the display of essential content while asynchronously loading additional data in the background. This approach will mitigate the appearance of the endless skeleton loader and provide users with meaningful content promptly.
3. I'll introduce robust error handling mechanisms to gracefully manage data retrieval failures and network interruptions. Users will be presented with informative error messages, and detailed error logs will be generated for debugging purposes.
4. Integration of performance monitoring tools will be crucial for tracking page load times and identifying potential performance bottlenecks. Continuous monitoring will enable me to refine the member page loading experience and maintain optimal performance levels.

I hope this solution will help you with this issue.

Cooper Brown from Upwork.

[melvin-bot]

📣 @mericanAncer127! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[jitendraCredentia]

Issue Analysis and Proposal:

After reviewing the reported problem and examining the behavior described, it appears that the infinite scroll feature may be closely related to the issue at hand. The endless skeleton loader observed on the member page could potentially be triggered or exacerbated by the continuous loading mechanism implemented through infinite scroll.

Understanding the Impact:

Infinite scroll, while convenient for users in certain contexts, can sometimes lead to performance issues or unexpected behaviors, especially when dealing with complex data rendering and asynchronous loading processes. In this case, the endless skeleton loader suggests a failure in properly handling the continuous loading of member data.

Proposal for Resolution:

To address this issue effectively, I propose temporarily disabling the infinite scroll feature on the member page. By doing so, we can isolate the behavior and investigate any underlying causes more thoroughly. Additionally, this approach will allow us to assess the impact of infinite scroll on the reported problem and determine whether it is the primary contributing factor.

Next Steps:

Disable the infinite scroll feature on the member page.
Conduct comprehensive testing to observe any changes in behavior and performance.
Investigate the root cause of the endless skeleton loader issue in parallel.
Implement necessary fixes or optimizations based on the findings.
Re-enable infinite scroll once the underlying issues are resolved and thoroughly tested.

[melvin-bot]

📣 @jitendraCredentia! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[mountiny]

This feels like a niche issue so if by the time other blockers are resolved, I would demote it

[hungvu193]

I can't reproduce on latest main

[mountiny]

I will demote now and add Needs reproduction label

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[puneetlath]

I've asked applause to re-test.

[melvin-bot]

Upwork job price has been updated to $500

[puneetlath]

Bumping this to $500 to try and get a proposal.

[dominictb]

Proposal

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

Endless skeleton loader appears on the member page if you switch to it multiple times

What is the root cause of that problem?

I believe that there's only 1 scenario that lead to this issue, which is somehow API calls CreateWorkspace and OpenWorkspaceMembersPage return {jsonCode !== 200} and the getMissingOnyxUpdates call fails to pick up the missing updates from the server. I can partially simulate this locally by setting in the proxy:

   proxyRequest.on('response', (proxyResponse) => {
      if(requestPath?.includes('CreateWorkspace') || requestPath?.includes('OpenWorkspaceMembersPage')){
          response.writeHead(200, {'content-type': 'application/json'});
          response.write(Buffer.from(JSON.stringify({jsonCode: 429})))
          response.end()
          return;
      }
      response.writeHead(proxyResponse.statusCode ?? 0, proxyResponse.headers);
      proxyResponse.pipe(response);
  });

If the CreateWorkspace fails, it will apply the onyx update

const failureData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
            value: {employeeList: null},
        },
...

causing the policy.employeeList to be null, and since OpenWorkspaceMembersPage fails too, the employeeList is still null, causing isLoading value in the component WorkspaceMembersPage to be false. Below is the definition of isLoading

const isOfflineAndNoMemberDataAvailable = isEmptyObject(policy?.employeeList) && isOffline;

const isLoading = useMemo(
        () => !isOfflineAndNoMemberDataAvailable && (!OptionsListUtils.isPersonalDetailsReady(personalDetails) || isEmptyObject(policy?.employeeList)),
        [isOfflineAndNoMemberDataAvailable, personalDetails, policy?.employeeList],
    );

We can guarantee that OptionsListUtils.isPersonalDetailsReady(personalDetails) should always be true, and with isOffline = false and policy?.employeeList = null, the isLoading is always true in this case => the skeleton loader will be rendered in the SelectionList component

<SelectionList
                        ref={selectionListRef}
                        canSelectMultiple={isPolicyAdmin}
                        sections={[{data, isDisabled: false}]}
                        ListItem={TableListItem}
                        disableKeyboardShortcuts={removeMembersConfirmModalVisible}
                        headerMessage={getHeaderMessage()}
                        headerContent={getHeaderContent()}
                        onSelectRow={openMemberDetails}
                        onCheckboxPress={(item) => toggleUser(item.accountID)}
                        onSelectAll={() => toggleAllUsers(data)}
                        onDismissError={dismissError}
                        showLoadingPlaceholder={isLoading}
                        showScrollIndicator
                        shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()}
                        textInputRef={textInputRef}
                        customListHeader={getCustomListHeader()}
                        listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]}
                    />

You can see the video below to see the described behavior (until the getMissingOnyxUpdate call finish, which is the call I haven't been able to simulate yet)

web-9MB-5MB.mp4

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

First, there are no concrete evidence supporting my assumption about the failures of these 2 APIs in this bug (and the GetMissingOnyxUpdate failure to pick up missing update sequence as well), even though via elimination process, I can safely conclude that only under this circumstances that we can reproduce this bug.

Reasoning

We'll need to take a look at the isLoading definition

function isPersonalDetailsReady(personalDetails: OnyxEntry<PersonalDetailsList>): boolean {
    const personalDetailsKeys = Object.keys(personalDetails ?? {});
    return personalDetailsKeys.some((key) => personalDetails?.[key]?.accountID);
}

const isOfflineAndNoMemberDataAvailable = isEmptyObject(policy?.employeeList) && isOffline;

const isLoading = useMemo(
    () => !isOfflineAndNoMemberDataAvailable && (!OptionsListUtils.isPersonalDetailsReady(personalDetails) || isEmptyObject(policy?.employeeList)),
    [isOfflineAndNoMemberDataAvailable, personalDetails, policy?.employeeList],
);

The definition of the isPersonalDetailsReady shows that it is impossible for isPersonalDetailsReady(personalDetails) to be false, in order for isLoading=true, the isOffline must be false and isEmptyObject(policy?.employeeList) must be empty. By looking through all possible Onyx updates, we can assume that only when the CreateWorkspace call fails function createWorkspace and the failureData are applied, then the policy?.employeeList can be null.

Further investigation into the optimistic update of CreateWorkspace API call, and other related API calls like OpenWorkspaceMembersPage shows the exact behavior as described in the all of the reported videos here : At first opening the member page (and backing out many times), we can still see the member list (due to the optimistic data), but after roughly dozens of seconds (roughly the CreateWorkspace API call response time, the OpenWorkspaceMembersPage seems to be faster, but it could be slow in the reported video), it shows skeleton loader (onyx pick up failureData updates, causing the employeeList to be null).

If the assumption is true, the BE should take a look into the root cause of API failures. And on FE's side, to avoid the infinite loader in case of API failure, we can update the isLoading definition in the WorkspaceMembersPage component to exclude the check isEmptyObject for policy?.employeeList

const isOfflineAndNoMemberDataAvailable = isEmptyObject(policy?.employeeList) && isOffline;

const isLoading = useMemo(
    () => !isOfflineAndNoMemberDataAvailable && !OptionsListUtils.isPersonalDetailsReady(personalDetails)
    [isOfflineAndNoMemberDataAvailable, personalDetails],
);

and since the workspace member list cannot be empty (at least the owner is in the member list), we can show the message Cannot display the member list right now! Please try again! (this is subject to change) by modifying the getHeaderMessage function

 const getHeaderMessage = () => {
    if (isOfflineAndNoMemberDataAvailable) {
        return translate('workspace.common.mustBeOnlineToViewMembers');
    }

    return isEmptyObject(policy?.employeeList) ? 'Cannot display the member list right now! Please try again!': '';
};

What alternative solutions did you explore? (Optional)

N/A

[hungvu193]

@dominictb Do you have any consistent steps to reproduce this issue?

[dominictb]

@hungvu193 Currently no, as mentioned, I could only simulate (but not reproduce) the issue based on my assumption.

[hungvu193]

tbh, that's really hard to review it while we couldn't reproduce it but let me try to take another look today.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[dominictb]

@hungvu193 u got any chance to take a look at this?

[hungvu193]

and since the workspace member list cannot be empty (at least the owner is in the member list), we can show the message Cannot display the member list right now! Please try again! (this is subject to change) by modifying the getHeaderMessage function

 const getHeaderMessage = () => {
    if (isOfflineAndNoMemberDataAvailable) {
        return translate('workspace.common.mustBeOnlineToViewMembers');
    }

    return isEmptyObject(policy?.employeeList) ? 'Cannot display the member list right now! Please try again!': '';
};

Still can not reproduce this issue but I don't agree with you at this point, employeeList can be empty during it's fetching from server for the first time user open the app.

Since this issue was not reproduced anymore, I just wonder if we can close it?

[dominictb]

@hungvu193 the empty array value of employeeList comes from the OpenApp API, the mergeCollection for the policy_ collection operation. It's a separate issue and the BE should take a look into this?

However, if the OpenApp returns, the employeeList again cannot be empty, or at least it cannot be null unless the situation described in my proposal happens, thus my assumption is not totally inaccurate

[hungvu193]

the empty array value of employeeList comes from the OpenApp API,

I think this is intentional. So I still don't agree with you at that point.
cc @puneetlath for your thoughts at above discussion, I think we can close this issue.

[melvin-bot]

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

[dominictb]

@hungvu193 to be honest, I don't think it should be intentional that way. The onyx updates coming from the OpenApp API call should be atomic, which means FE should only care about the final state after apply all the Onyx operations at once. So, under no circumstances should the policy?.employeeList be promptly [] before becoming non-empty again, because OpenApp API call is just 1 single operation, which return the policy data with non-empty employeeList (even though it contains multiple Onyx sub-operations). @puneetlath can we have someone on the BE team to check this point, as I believe we shouldn't split into 2 operations mergeCollection and merge for policy_XXX just to update the employeeList from [] to {[email]: {....}}. It should live either in mergeCollection operation.

Again, it still confirms that the employeeList, by definition, cannot be empty.

[mvtglobally]

Issue not reproducible during KI retests. (Second week)

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@puneetlath, @hungvu193 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[puneetlath]

Ok, based on the discussion I agree that we can close this out. Thanks everyone.