[Awaiting bug report payment][$1000] Workspace - No loader for workspace members page

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the app in a new incognito window. Don't open in an existing incognito window.
2. Create a workspace if you don't already have one.
3. Go to Settings > Workspaces > Members.
4. Notice that there is no loader for the members and the message "Member not found. To invite a new member to the workspace, please use the Invite button above." is shown before members are loaded.

Expected Result:

A loader should be shown before members are loaded.

Actual Result:

There is no loader before members are loaded and the message "Member not found. To invite a new member to the workspace, please use the Invite button above." is shown which might be confusing for users if they have slow connection.

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.29-11
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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-06-22.at.2.06.40.PM.1.mov

Recording.887.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Nikhil-Vats
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687423087741759

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010e98a1022376a0f5
• Upwork Job ID: 1673721545937694720
• Last Price Increase: 2023-07-04

[melvin-bot]

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

[Nikhil-Vats]

Proposal

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

Loader is not shown on workspace members page before members are loaded. Instead an error message is shown - "Member not found. To invite a new member to the workspace, please use the Invite button above."

What is the root cause of that problem?

We are showing an error message if the length is 0 for members. When members are not loaded, length of members is 0 and we show this message instead of a loader. See line 471.

App/src/pages/workspace/WorkspaceMembersPage.js

Lines 448 to 472 in 1e308ca

	{data.length > 0 ? (
	<View style={[styles.w100, styles.mt4, styles.flex1]}>
	<View style={[styles.peopleRow, styles.ph5, styles.pb3]}>
	<Checkbox
	isChecked={!_.isEmpty(removableMembers) && _.every(_.keys(removableMembers), (accountID) => _.contains(selectedEmployees, Number(accountID)))}
	onPress={() => toggleAllUsers(removableMembers)}
	/>
	<View style={[styles.flex1]}>
	<Text style={[styles.textStrong, styles.ph5]}>{props.translate('workspace.people.selectAll')}</Text>
	</View>
	</View>
	<KeyboardDismissingFlatList
	renderItem={renderItem}
	data={data}
	keyExtractor={(item) => item.login}
	showsVerticalScrollIndicator
	style={[styles.ph5, styles.pb5]}
	contentContainerStyle={safeAreaPaddingBottomStyle}
	keyboardShouldPersistTaps="handled"
	/>
	</View>
	) : (
	<View style={[styles.ph5]}>
	<Text style={[styles.textLabel, styles.colorMuted]}>{props.translate('workspace.common.memberNotFound')}</Text>
	</View>

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

Check if props.policyMembers is empty or props.personalDetails is empty using _.isEmpty(props.policyMembers) || !OptionsListUtils.isPersonalDetailsReady(props.personalDetails) and show Skeleton using OptionsListSkeletonView component if it is indeed empty.

We need to check props.personalDetails is also loaded because this page uses personalDetails prop to get details of policyMembers.

{_.isEmpty(props.policyMembers) || !OptionsListUtils.isPersonalDetailsReady(props.personalDetails) ? (
    <OptionsListSkeletonView shouldAnimate />
) : (
  // existing code to render members, buttons, etc. when all data is loaded
)}

Alternatively, we can also make a function for this in the WorkspaceMembersPage or in PolicyUtils (which has other utilities related to workspace) -

function shouldShowPolicyMembers() {
  return !_.isEmpty(props.policyMembers) && OptionsListUtils.isPersonalDetailsReady(personalDetails));
}

Then, we can use this function in WorkspaceMembersPage. If this function returns true then we can render members, buttons, etc. otherwise we render OptionsListSkeletonView.

What alternative solutions did you explore? (Optional)

NA

Result -

Screen.Recording.2023-07-20.at.11.11.29.PM.mov

[trjExpensify]

I think this is a dupe of this issue and should be fixed by this PR when we implement the skeleton UI on this page when it's loading. Going to close it out!

[Nikhil-Vats]

@trjExpensify this is not a dupe of #19236, that issue is trying to fix the 'Hmm.. it's not here page' that appears when we open the members or invite page using deeplink in desktop app.

Screenshot of that issue -

Video of that issue -

members.mov

This issue appears on all platforms and is not related to deeplink at all. This issue is simply the lack of loader on Workspace Members page which shows this message until members are not loaded -
"Member not found. To invite a new member to the workspace, please use the Invite button above."

You can see the issue description for more details.

[trjExpensify]

Hm, but isn't the root cause the same? We don't currently have the skeleton UI implemented on the workspace members page? By adding it, this won't be a problem.

Happy to reopen and get a second opinion however! CC: @kowczarz @aimane-chnaif

[Nikhil-Vats]

@trjExpensify I tested that PR and it doesn't fix this issue because the state of policyMembers (whether they are loaded or not) is not being checked anywhere in existing code and that PR. That PR is showing the skeleton view when the workspace details have not loaded which includes details like name, owner, currency, etc. but doesn't include policyMembers since there is a separate API call for that.

Screen recording using the PR's code locally checked out -

Screen.Recording.2023-06-27.at.3.02.50.AM.mov

The HOC that is being changed in that PR - WithPolicyAndFullscreenLoading is also used in the initial workspace page (see screenshot below) so we cannot check for policyMembers in it since that API is not even called for this page so it would keep on showing loading indicator if we check for policyMembers in that page -

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010e98a1022376a0f5

[melvin-bot]

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

[melvin-bot]

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

[trjExpensify]

Cool, thanks for the added detail. I'm moving this on external to officially have a C+ assigned. @allroundexperts can you chime in here please?

[aimane-chnaif]

I suggest to put this on hold until PR is merged.

[trjExpensify]

Hehe, that's the same PR I referenced here

[allroundexperts]

@Nikhil-Vats Thanks for your proposal. With what you have suggested, wouldn't the error message never show up (even if the members are actually 0)?

[Nikhil-Vats]

@allroundexperts that code can be removed since there is always at least 1 member in the workspace - the workspace owner which cannot be removed by anyone.

I updated my proposal to reflect it.

[trjExpensify]

How are we looking here now @allroundexperts?

[allroundexperts]

How are we looking here now @allroundexperts?

@tjferriss We still don't have a good enough proposal.

[allroundexperts]

@Nikhil-Vats With what you've suggested in your proposal, if someone types something in the search bar, and if not results are found, he'll just end up with a spinner. Also I don't think that admin always shows up in the members page. I was able to produce an instance in which the workspace members page did not show any user at all.

[Nikhil-Vats]

Thanks @allroundexperts . I unnecessarily updated my proposal and like you said, it won't work correctly if we search for members. I have reverted the changes back as the initial proposal works fine. Please see the video below. It is also attached in my updated proposal.

Screen.Recording.2023-07-04.at.4.18.00.PM.mov

As for a workspace without an admin, can you tell me how were you able to produce an instance with no members so that I can better understand the edge case and propose an updated solution? As no one can remove the admin, not even the admin themselves.

[roryabraham]

Skeleton will be added in #21387

Looks like a loading spinner, not a skeleton UI?

@roryabraham Lets put this on hold then since #21387 is doing something similar?

Agreed, seems like this is a dupe of #19236, so let's put this on HOLD to see if @amyevans agrees. If so, we should just close this. @amyevans what do you think?

[amyevans]

Agreed, seems like this is a dupe of #19236, so let's put this on HOLD to see if @amyevans agrees. If so, we should just close this. @amyevans what do you think?

I don't think it's strictly a dupe, because different data missing is resulting in different views during the load phase. That said, per @aimane-chnaif's comment in the PR, we'll ensure both cases are covered in the PR and a skeleton UI is displayed. So I think a HOLD makes sense for this GH, with a reporter bonus payout once #21387 is live?

[roryabraham]

Thanks @amyevans!!!

[trjExpensify]

Nice, makes sense to me. Added the issue title in the header for the hold!

[roryabraham]

Still on HOLD. Making this a weekly in the meantime

[trjExpensify]

Linked PR is in review!

[roryabraham]

on HOLD

[trjExpensify]

Linked PR is still in review.

[trjExpensify]

Deployed to staging 15 hours ago! Retested and the skeleton UI circumvents the problem:

teuNJJiq9K.mp4

With that, I've sent @Nikhil-Vats an offer for $250 for the bug report. Once settled, we can close this out! :)

[Nikhil-Vats]

Offer accepted @trjExpensify.

[trjExpensify]

Settled!