Add modal for inviting users to a workspace #3463

Jag96 · 2021-06-09T05:13:44Z

cc @mountiny

Details

This PR adds the ability to invite a user to a workspace.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/166040

Tests

Go to expensify.com.dev (or expensify.com if testing on prod) and get the policyID of your testing account. You can find this by going to https://www.expensify.com.dev/admin_policies and clicking on one of the policies and you'll see it in the URL. For example, the one I'm using to test is 4A19DC10B34DC505.
Since the workspace people list page isn't done yet, the easiest way to remove users while testing is to go to the URL mentioned above, choose a policy, choose People and remove that way.
Update InitialPage.js and add the following object to menuItems, this will allow us to easily navigate to the modal on all platforms while testing (replace the policyID with your policyID)
1. {translationKey: 'common.invite', icon: Profile, action: () => { Navigation.navigate(ROUTES.getWorkspaceInviteRoute('4A19DC10B34DC505')); },},
Run the app and go to settings, and click on the bottom option Invite (the one we added in step 2), and confirm that the modal opens with the Invite button disabled. Confirm that the welcomeNote placeholder value contains the policy name.
Open the Application tab of the Chrome inspector (web is the easiest way to test this) and type in policy_4A19DC10B34DC505 in the filter bar (replace 4A19DC10B34DC505 with your policyID), confirm you see the policy object w/ the employeeList
Add a new user via the invite modal, confirm that after hitting the Invite button the modal disappears.
Refresh the value in the Application tab of the Chrome inspector, confirm that the new email is added
In the Chrome inspector, filter using personalDetails, confirm the invited user's personal details show up
To test the failure case, you can take one of two routes:
Internal to Expensify - update the dev code of the API to throw an Exception inside the Policy_Employees_Merge command of the PHP api
External - update the .then handler of Policy_Employees_Merge in Policy.js so that the data.jsonCode === 200 check is now data.jsonCode !== 200
Invite another user via the modal, confirm that the modal closes and you see an error message
Check the Application tab of the Chrome inspector, confirm that the invited email doesn't show up in the list

QA Steps

N/A, this will be more easily testable by QA in https://github.com/Expensify/Expensify/issues/166468

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

src/libs/actions/Policy.js

Jag96 · 2021-06-15T04:14:20Z

This is ready for review!

marcaaron

LGTM and overall fine work! Really only had NAB comments.

src/languages/es.js

marcaaron · 2021-06-15T05:41:54Z

src/libs/actions/Policy.js

+            translateLocal = (phrase, variables) => translate(preferredLocale, phrase, variables);
+        }
+    },
+});


NAB but maybe a good follow up. I noticed that we are doing this in every file that needs to use translate(). Perhaps we should just subscribe to the preferred locale in '../translate.js' and then we could export the method that already knows what the locale is.

That sounds like a good follow-up to me 👍

Created a follow-up PR here: #3716

marcaaron · 2021-06-15T05:46:49Z

src/libs/actions/Policy.js

+            if (data.jsonCode === 200) {
+                const policyDataToStore = _.reduce(data.policyList, (memo, policy) => ({
+                    ...memo,
+                    [`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: getSimplifiedMemberList(policy),


NAB, I wonder if it would be more clear that this is doing a merge on an object if we instead did this...

[`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`]: {employeeList: getSimplifiedMemberList(policy)},

also seems like getSimplifiedMemberList() would return a list.

I updated the name of the function to getSimplifiedEmployeeListObject, let me know if that's more clear or if you think it'd still be better to return just the array from that function instead.

src/libs/actions/Policy.js

src/pages/workspace/WorkspaceInvitePage.js

kidroca · 2021-06-15T07:40:17Z

src/libs/actions/Policy.js

+    const policy = _.clone(allPolicies[key]);
+    policy.employeeList.push(login);


If it's a shallow copy - you are pushing to the original allPolicies[key].employeeList
It' probably won't make a difference here, but mutating your source data is bad

See: #3463 (comment)

agree it doesn't really matter in this case, but should be avoided.

OSBotify · 2021-06-15T23:24:08Z

✋ 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-06-16T18:18:00Z

🚀 Deployed to staging in version: 1.0.69-1🚀

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

OSBotify · 2021-06-25T18:32:03Z

🚀 Deployed to production in version: 1.0.73-3🚀

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

Jag96 added 11 commits June 8, 2021 20:19

update routes

6e72ad1

update linkingConfig

2a60ac7

add workspaceInvite page

e01059d

add placeholder values for spanish translations

c255c9e

import withLocalize

ba6e9ce

add new screen and navigator

b87c128

update text, fix broken state

89dd12f

style welcome message box

41d45d3

get policy name showing up in welcome text

39d1b51

add Policy_Employees_Merge api command

0bde5fc

style

8be51d8

Jag96 self-assigned this Jun 9, 2021

Jag96 added 6 commits June 10, 2021 17:21

add policyID to params

44f095a

add API call for policyList

04382bb

fix empty employeeList

b92ebaf

pass data to invite, optimisitcally add user

5604e30

insert personal details into onyx

4023ff8

test mergeCollection bug

686db89

kidroca reviewed Jun 11, 2021

View reviewed changes

src/libs/actions/Policy.js Outdated Show resolved Hide resolved

Jag96 added 6 commits June 11, 2021 14:13

merge main

6836fe0

get optimistic addition and fallback remove working

d8e8120

insert formatted personalDetails

2901551

pass default welcome if none is filled in

25d16b8

add privacy policy link

77cc488

style

7cee97a

marcaaron self-requested a review June 11, 2021 23:11

Jag96 added 4 commits June 14, 2021 18:19

merge main

04f28fe

add welcomeNote label

cc62bd2

match styles from mockup

a6d715b

move page into workspace folder

7e5937f

Jag96 added 4 commits June 14, 2021 19:50

show error on invite failure

59ab488

more style updates

ff4f095

fix textInput spacing on mobile

f34c715

simplify privacy link

409f66a

Jag96 marked this pull request as ready for review June 15, 2021 04:10

Jag96 requested a review from a team as a code owner June 15, 2021 04:10

MelvinBot requested review from HorusGoul and removed request for a team June 15, 2021 04:10

Jag96 added 2 commits June 14, 2021 21:43

improve error messages

466a9bc

fix style

c787e01

marcaaron previously approved these changes Jun 15, 2021

View reviewed changes

Jag96 added 2 commits June 14, 2021 23:30

translate to spanish

4fe596a

address PR review comments

5be04c0

Jag96 dismissed marcaaron’s stale review via 5be04c0 June 15, 2021 06:49

remove commented out line

ae3fc4a

kidroca reviewed Jun 15, 2021

View reviewed changes

Jag96 added 2 commits June 15, 2021 14:52

don't modify original data

2b0e575

merge main

cdbc397

marcaaron approved these changes Jun 15, 2021

View reviewed changes

marcaaron merged commit 2863305 into main Jun 15, 2021

marcaaron deleted the joe-invite-to-workspace branch June 15, 2021 23:23

Jag96 mentioned this pull request Jun 15, 2021

Update readme to clarify the expected behavior for Onyx.merge Expensify/react-native-onyx#83

Closed

OSBotify mentioned this pull request Jun 17, 2021

Deploy Checklist: Expensify.cash 2021-06-15 #3593

Closed

Jag96 mentioned this pull request Jun 22, 2021

Clean up multiple subscriptions of preferredLocale #3716

Closed

5 tasks

sobitneupane mentioned this pull request Apr 18, 2023

[$1000] Trying to send message within angle brackets in the invite new members page doesn’t display in email #15700

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add modal for inviting users to a workspace #3463

Add modal for inviting users to a workspace #3463

Jag96 commented Jun 9, 2021 •

edited

Loading

Jag96 commented Jun 15, 2021

marcaaron left a comment

marcaaron Jun 15, 2021

Jag96 Jun 15, 2021

Jag96 Jun 22, 2021

marcaaron Jun 15, 2021

Jag96 Jun 15, 2021

kidroca Jun 15, 2021 •

edited

Loading

marcaaron Jun 15, 2021

OSBotify commented Jun 15, 2021

OSBotify commented Jun 16, 2021

OSBotify commented Jun 25, 2021

		const policy = _.clone(allPolicies[key]);
		policy.employeeList.push(login);

Add modal for inviting users to a workspace #3463

Add modal for inviting users to a workspace #3463

Conversation

Jag96 commented Jun 9, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Jag96 commented Jun 15, 2021

marcaaron left a comment

Choose a reason for hiding this comment

marcaaron Jun 15, 2021

Choose a reason for hiding this comment

Jag96 Jun 15, 2021

Choose a reason for hiding this comment

Jag96 Jun 22, 2021

Choose a reason for hiding this comment

marcaaron Jun 15, 2021

Choose a reason for hiding this comment

Jag96 Jun 15, 2021

Choose a reason for hiding this comment

kidroca Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

marcaaron Jun 15, 2021

Choose a reason for hiding this comment

OSBotify commented Jun 15, 2021

OSBotify commented Jun 16, 2021

OSBotify commented Jun 25, 2021

Jag96 commented Jun 9, 2021 •

edited

Loading

kidroca Jun 15, 2021 •

edited

Loading