Block system user from Inviting into Workspace #4781

parasharrajat · 2021-08-23T01:08:34Z

Details

Block system users from inviting into Workspace.
Fix a silly mistake where users can invite existing multiple users (stop adding duplicate members #4510).
Fix users can invite existing users with phone number login.
Fix copy to show that multiple users can be invited.

Fixed Issues

$ #4646

Tests | QA Steps

Open any workspaces Settings on NewDot.
Open Invite People page.

Test 1

Try to invite System users (chronos@, receipts@, and concierge@).

Test 2

try to invite multiple users(more than one) who already exist in workspaces.

Test 3

Check the copy on the page, it should show that you can invite multiple users.

Tested On

Screenshots

Web | Desktop | Mobile Web

iOS

Android

parasharrajat · 2021-08-23T01:09:33Z

Please check the copy text. Let me know what would be the correct one and their translations.

roryabraham

Mostly looks good, but I had a few comments and want to get this copy + these translations vetted before we move forward.

roryabraham · 2021-08-23T18:18:57Z

src/libs/reportUtils.js

+ * @param {String} login - user email
+ * @return {Boolean}
+ */
+function isSystemUser(login) {


NAB but reportUtils feels like a weird place for this. Not sure I have a better suggestion for now unless we create userUtils or something?

roryabraham · 2021-08-23T18:20:21Z

src/pages/workspace/WorkspaceInvitePage.js

@@ -88,9 +90,16 @@ class WorkspaceInvitePage extends React.Component {
            Growl.error(this.props.translate('workspace.invite.pleaseEnterValidLogin'), 5000);
            return;
        }
+
+        const isEnteredLoginSystemLogin = _.some(logins, login => isSystemUser(login));


Suggested change

const isEnteredLoginSystemLogin = _.some(logins, login => isSystemUser(login));

const didEnterSystemLogin = _.some(logins, login => isSystemUser(login));

roryabraham · 2021-08-23T18:37:31Z

Getting some copy reviewed in the linked issue. However, in it's current state I don't think we'll want User to be capitalized.

roryabraham · 2021-08-23T20:18:46Z

src/languages/en.js

            pleaseEnterValidLogin: 'Please ensure the email or phone number is valid (e.g. +15005550006).',
            pleaseEnterUniqueLogin: 'That user is already a member of this workspace.',
            genericFailureMessage: 'An error occurred inviting the user to the workspace, please try again.',
+            systemUserError: 'Please ensure email or phone number is not a system User.',


Coming back from the linked issue, the copy we landed on here is:

Sorry, you cannot invite chronos@expensify.com to a workspace.

Using whichever invalid email was found first.

parasharrajat · 2021-08-23T23:40:29Z

Do you think the label and placeholder are correct for the email input?

roryabraham · 2021-08-23T23:46:43Z

Do you think the label and placeholder are correct for the email input?

I think Emails or Phones should be Emails or phone numbers

roryabraham

Mostly LGTM, but we should have someone buddy-check these translations after you make the next few changes.

roryabraham · 2021-08-23T23:55:13Z

src/languages/en.js

            personalMessagePrompt: 'Add a Personal Message (Optional)',
-            enterEmailOrPhone: 'Email or Phone',
+            enterEmailOrPhone: 'Emails or Phones',


Suggested change

enterEmailOrPhone: 'Emails or Phones',

enterEmailOrPhone: 'Emails or phone numbers',

src/languages/en.js

parasharrajat · 2021-08-24T03:42:52Z

OK. It will look like this with numberoflines 2 after #4649 is merged.

roryabraham · 2021-08-24T03:52:05Z

Okay. This PR is not urgent so let's HOLD on #4649

parasharrajat · 2021-08-24T16:26:51Z

@shawnborton Before we commit, I would like your feedback on the Changes to the Phone & email input here. #4781 (comment)

shawnborton · 2021-08-24T21:28:41Z

Looks good to me. It seems like with this issue here, we might end up changing this page a bit but I think we still need to go through the design phase first.

The only other feedback is that we should be consistent with how we capitalize the labels used in each textarea on this page. Let's just stick with sentence case?

parasharrajat · 2021-08-24T22:34:38Z

@roryabraham #4649 is merged. Let's move with this now. Thanks.

roryabraham · 2021-08-24T22:35:48Z

Okay, let me get the translations checked

parasharrajat · 2021-08-26T10:23:38Z

Translations are updated https://expensify.slack.com/archives/C01GTK53T8Q/p1629953043008900

OSBotify · 2021-08-26T20:37:21Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-08-30T17:32:48Z

🚀 Deployed to staging by @roryabraham in version: 1.0.88-3 🚀

platform	result
🤖 android 🤖	cancelled 🔪
🖥 desktop 🖥	cancelled 🔪
🍎 iOS 🍎	cancelled 🔪
🕸 web 🕸	success ✅

OSBotify · 2021-09-01T01:43:32Z

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

parasharrajat added 3 commits August 23, 2021 06:16

block system user from Workspaces

5bc0f4f

fix silly mistake and correct variable name

8c6c736

correct copy for multiple users

0896cc7

parasharrajat requested a review from a team as a code owner August 23, 2021 01:08

MelvinBot requested review from roryabraham and removed request for a team August 23, 2021 01:08

fix: match Phone number users correctly

242734b

roryabraham assigned parasharrajat Aug 23, 2021

roryabraham requested changes Aug 23, 2021

View reviewed changes

roryabraham added Waiting for copy User facing verbiage needs polishing and removed Waiting for copy User facing verbiage needs polishing labels Aug 23, 2021

roryabraham requested changes Aug 23, 2021

View reviewed changes

updated the error message

f238c73

roryabraham requested changes Aug 23, 2021

View reviewed changes

change copy

b710d97

roryabraham mentioned this pull request Aug 24, 2021

[Hold of Payment Sept 7] User can invite chronos@, receipts@ and concierge@ in Workspaces #4646

Closed

fix: label

aabda01

transaltion fix

32f41f9

roryabraham approved these changes Aug 26, 2021

View reviewed changes

roryabraham merged commit ae38f1b into Expensify:main Aug 26, 2021

OSBotify mentioned this pull request Aug 27, 2021

Deploy Checklist: New Expensify 2021-08-25 #4828

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block system user from Inviting into Workspace #4781

Block system user from Inviting into Workspace #4781

parasharrajat commented Aug 23, 2021 •

edited

Loading

parasharrajat commented Aug 23, 2021

roryabraham left a comment

roryabraham Aug 23, 2021

roryabraham Aug 23, 2021

roryabraham commented Aug 23, 2021

roryabraham Aug 23, 2021

parasharrajat commented Aug 23, 2021

roryabraham commented Aug 23, 2021

roryabraham left a comment

roryabraham Aug 23, 2021

parasharrajat commented Aug 24, 2021

roryabraham commented Aug 24, 2021

parasharrajat commented Aug 24, 2021

shawnborton commented Aug 24, 2021

parasharrajat commented Aug 24, 2021

roryabraham commented Aug 24, 2021

parasharrajat commented Aug 26, 2021

OSBotify commented Aug 26, 2021

OSBotify commented Aug 30, 2021

OSBotify commented Sep 1, 2021

	const isEnteredLoginSystemLogin = _.some(logins, login => isSystemUser(login));
	const didEnterSystemLogin = _.some(logins, login => isSystemUser(login));

	enterEmailOrPhone: 'Emails or Phones',
	enterEmailOrPhone: 'Emails or phone numbers',

Block system user from Inviting into Workspace #4781

Block system user from Inviting into Workspace #4781

Conversation

parasharrajat commented Aug 23, 2021 • edited Loading

Details

Fixed Issues

Tests | QA Steps

Test 1

Test 2

Test 3

Tested On

Screenshots

Web | Desktop | Mobile Web

iOS

Android

parasharrajat commented Aug 23, 2021

roryabraham left a comment

Choose a reason for hiding this comment

roryabraham Aug 23, 2021

Choose a reason for hiding this comment

roryabraham Aug 23, 2021

Choose a reason for hiding this comment

roryabraham commented Aug 23, 2021

roryabraham Aug 23, 2021

Choose a reason for hiding this comment

parasharrajat commented Aug 23, 2021

roryabraham commented Aug 23, 2021

roryabraham left a comment

Choose a reason for hiding this comment

roryabraham Aug 23, 2021

Choose a reason for hiding this comment

parasharrajat commented Aug 24, 2021

roryabraham commented Aug 24, 2021

parasharrajat commented Aug 24, 2021

shawnborton commented Aug 24, 2021

parasharrajat commented Aug 24, 2021

roryabraham commented Aug 24, 2021

parasharrajat commented Aug 26, 2021

OSBotify commented Aug 26, 2021

OSBotify commented Aug 30, 2021

OSBotify commented Sep 1, 2021

parasharrajat commented Aug 23, 2021 •

edited

Loading