[PAY 8/20] You can invite someone who's already a member of the workspace

[aman-atg]

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

---

Action Performed:

1. Create a workspace
2. Choose people tabs
3. Invite yourself by adding your email address
4. It'll show two entries

Expected Result:

An error message

Actual Result:

Workaround:

Refreshing the browser make the 2nd entry disappear after a few seconds.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @strepanier03 (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[aman-atg]

Proposal

App/src/pages/workspace/WorkspaceInvitePage.js

Lines 74 to 85 in 9f15c14

	inviteUser() {
	const logins = _.map(_.compact(this.state.userLogins.split(',')), login => login.trim());
	const isEnteredLoginsvalid = _.every(logins, login => Str.isValidEmail(login) || Str.isValidPhone(login));
	if (!isEnteredLoginsvalid) {
	Growl.error(this.props.translate('workspace.invite.pleaseEnterValidLogin'), 5000);
	return;
	}
	invite(logins, this.state.welcomeNote || this.getWelcomeNotePlaceholder(),
	this.props.route.params.policyID);
	Navigation.goBack();
	}

• Adding the below code after line 81 will fix the issue

        const policyEmployeeList = lodashGet(this.props, 'policy.employeeList', []);
        const AreLoginsDuplicate = _.every(logins, login => _.contains(policyEmployeeList, login));
        if (AreLoginsDuplicate) {
            Growl.error(this.props.translate('workspace.invite.someGenericMessage'), 5000);
            return;
        }

• workspace.invite.someGenericMessage - need suggestions for a generic message
  • Please invite users that hasn't been invited.
  • Please ensure that the users are not already a member of the workspace
  • Please ensure that the users are not already invited.
  • Can't invite existing Users

[strepanier03]

Unique — Looks to be unique to me,
Issue hygiene — Looks go here as well,
Clarity of problem — Problem is stated clearly,
Value — Looks like something we should fix,

Passing to Eng.

[MelvinBot]

Triggered auto assignment to @francoisl (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[francoisl]

Hi @aman-atg, your proposal would work for the very specific issue where someone can invite themselves, but the issue can be generalized and rephrased as: you can invite someone who's already a member of the workspace (e.g. invite the same account twice in a row).

With that in mind, can you please update your proposal? For the error message, I suggest something like <login> is already a member of this workspace.
Thanks!

[aman-atg]

@francoisl Thanks for the input. I will update the Proposal with better logic that will cover the above mentioned scenario.

I have one question, though. Since we're supporting multiple logins.. what should be the error message when 2 or more logins are duplicates.

App/src/pages/workspace/WorkspaceInvitePage.js

Lines 74 to 85 in 9f15c14

	inviteUser() {
	const logins = _.map(_.compact(this.state.userLogins.split(',')), login => login.trim());
	const isEnteredLoginsvalid = _.every(logins, login => Str.isValidEmail(login) || Str.isValidPhone(login));
	if (!isEnteredLoginsvalid) {
	Growl.error(this.props.translate('workspace.invite.pleaseEnterValidLogin'), 5000);
	return;
	}
	invite(logins, this.state.welcomeNote || this.getWelcomeNotePlaceholder(),
	this.props.route.params.policyID);
	Navigation.goBack();
	}

One more thing to note is that currently we're validating all the logins and even if one is wrong, We're showing the error and returning

So, what should happen when some of the logins are duplicates?
I think we can follow the same convention and show a generic message (maybe?) and then return.

[francoisl]

I agree that following the same convention and returning early would make sense. In this case, we could use ${login} is already a member of this workspace. for the error message.

Although thinking about it more, I'm now thinking that in terms of providing a better UX, maybe we could just silently ignore and filter out the logins that are already in the workspace (and if all the logins are already in the workspace, we don't call invite() at all)? This way, the user doesn't see an error message, and they still "accomplished" what they were trying to do.

[aman-atg]

Although thinking about it more, I'm now thinking that in terms of providing a better UX, maybe we could just silently ignore and filter out the logins that are already in the workspace

• IMO that's a valid option for us.

(and if all the logins are already in the workspace, we don't call invite() at all)? This way, the user doesn't see an error message, and they still "accomplished" what they were trying to do.

• Here, however I feel that if all the logins entered by the User are duplicate then User should get some feedback like

     // `Please ensure that the user is not already a member of the workspace` or
     // `Please ensure that the user is not already invited.`

[francoisl]

I'm not passionate about either solution, I'm going to relabel this to get another opinion.

[MelvinBot]

Triggered auto assignment to @laurenreidexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[mananjadhav]

@francoisl I feel the input itself is not user-friendly. There is no way to know that you can invite more than one person by comma separating values.

Proposal

My approach would be to change the input to a tagged input such as above. I can see Workspace is in beta so there's a lot we can do to improve the UX here.

This will handle the cases for multiple inputs as well as filtering/showing problematic tags such as already existing participants.

btw the current flow should also check if the phone number entered is of the currently logged-in user.

[aman-atg]

This will handle the cases for multiple inputs

@mananjadhav this is being dealt with on a separate issue #4029

[MelvinBot]

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

[deetergp]

Apologies, I've a bit bogged down, looking at it now…

[deetergp]

The code portion of @aman-atg's proposal is solid. As far as the message goes, how about something like That user is already a member of this workspace?

[aman-atg]

As far as the message goes, how about something like That user is already a member of this workspace?

Sounds good to me!

[laurenreidexpensify]

Nice so @deetergp to confirm - this can move to upwork and we are going with @aman-atg's proposal to fix it?

[deetergp]

@laurenreidexpensify Yes, let's do it!

[deetergp]

Waiting on a PR. Do you think this should stay a Daily, @laurenreidexpensify, or is setting to Weekly more appropriate?

[aman-atg]

Added a PR. Not yet hired on upwork though.

[deetergp]

Setting this back to Planning

[mallenexpensify]

Created job and invited you @aman-atg, please confirm in a comment here once accepted. https://www.upwork.com/jobs/~012d85aaf070515c46

[aman-atg]

Accepted. (Proposal)

[deetergp]

The job has been offered & accepted and the PR has been merged. I think we're good here 👍

[mallenexpensify]

Officially hired on Upwork. Will pay on 8/20 if no regressions. Bumped to Weekly

[mallenexpensify]

Paid in Upwork with bonus for reporting issues. Deployed to production 3 days ago, closing