[User Settings] Allow users to add secondary logins #2169

Maftalion · 2021-03-31T02:58:53Z

Details

Updates to the profile page to handle adding/verifying a secondary login
New page created for adding a new email/phone login

Fixed Issues

Fixes #2029

Tests

For a user without a secondary login:

Go to the Profile page, click Add, enter the phone/email & password on the form
Test that button is disabled until form is correctly filled and errors are shown for invalid entries
If the login exists but is unverified, confirm that clicking the "Resend" button resends the verification code
If the login is verified then it just appears in plain-text

To test specific scenarios, you can also stub the data in getLogins:

Unverified phone: [{ "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "" }, { "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "2021-02-22 03:54:15" }]  Unverified email: [{ "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "2021-02-22 03:54:15" }, { "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "" }]

Multiple phone (verified): [{ "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "" }, { "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "2021-02-22 03:54:15" }, { "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "2021-02-22 03:54:15" }]

No Phone: [{ "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "2021-02-22 03:54:15" }]

No Email: [{ "partnerName": "expensify.com", "partnerUserID": "[email protected]", "validatedDate": "2021-02-22 03:54:15" }]

QA Steps

same as above

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

src/pages/settings/AddLoginPage.js

NikkiWines

Had a couple of comments but overall this is really shaping up 🚀

src/libs/actions/User.js

src/pages/settings/AddLoginPage.js

src/pages/settings/ProfilePage/index.js

src/pages/settings/ProfilePage/LoginField.js

src/libs/Navigation/AppNavigator/ModalStackNavigators.js

src/pages/settings/ProfilePage/LoginField.js

# Conflicts: # src/pages/settings/ProfilePage/index.js

NikkiWines

This is looking super good.

I just realized we're routing users to e.com (through the validation link) instead of to e.cash, which is a little unfortunate. That'll require some changes outside of the E.cash repository to resolve. We'll likely want to pass something like isFromExpensifyCash in setSecondaryLogin and resendValidateCode. I'll take a look at that implementation today/this weekend/monday.

src/pages/settings/ProfilePage/LoginField.js

src/pages/settings/ProfilePage/index.js

NikkiWines

Looks great! Added some nabby comments but let's

I have a couple PRs up to route the validation links back to E.cash instead of E.com so no need to hold this one.

src/libs/actions/User.js

src/pages/settings/AddSecondaryLoginPage.js

src/pages/settings/ProfilePage/LoginField.js

NikkiWines

tgolen

Please submit a new PR to fix these

tgolen · 2021-04-06T20:52:12Z

src/pages/settings/AddSecondaryLoginPage.js

+
+    // Route object from navigation
+    route: PropTypes.shape({
+        params: PropTypes.shape({


These are missing inline comments, please add them

tgolen · 2021-04-06T20:52:21Z

src/pages/settings/AddSecondaryLoginPage.js

+        Onyx.merge(ONYXKEYS.USER, {error: ''});
+    }
+
+    submitForm() {


Missing method docs

tgolen · 2021-04-06T20:52:28Z

src/pages/settings/AddSecondaryLoginPage.js

+            });
+    }
+
+    // Determines whether form is valid


Not valid method docs

tgolen · 2021-04-06T20:52:39Z

src/pages/settings/ProfilePage/LoginField.js

+
+    // Login associated with the user
+    login: PropTypes.shape({
+        partnerUserID: PropTypes.string,


Missing inline comments

tgolen · 2021-04-06T20:52:46Z

src/pages/settings/ProfilePage/LoginField.js

+        this.onResendClicked = this.onResendClicked.bind(this);
+    }
+
+    onResendClicked() {


Missing method docs

tgolen · 2021-04-06T20:53:04Z

src/pages/settings/ProfilePage/index.js

    setAutomaticTimezone(isAutomaticTimezone) {
        this.setState(({selectedTimezone}) => ({
            isAutomaticTimezone,
            selectedTimezone: isAutomaticTimezone ? moment.tz.guess() : selectedTimezone,
        }));
    }

+    // Get the most validated login of each type


Invalid method docs

isagoico · 2021-04-07T00:33:46Z

Title:
iOS - 1.0.13-0 - Profile - Both email and phone number options are displayed for secondary login

Expected Result:

Only Phone number field displayed to add as secondary login

Actual Result:

Both email and phone number fields are displayed for secondary login

Action Performed:

Launch the app and login with email account
Tap Profile > Add

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web
iOS ✔️
Android
Desktop App
Mobile Web

**Version Number:**1.0.13-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:
We were able to reproduce this only in the iOS app.

Expensify/Expensify Issue URL:

NikkiWines · 2021-04-07T00:36:22Z

@isagoico what is the primary login for this user

isagoico · 2021-04-07T00:47:18Z

I was able to reproduce this on my test account with the primary log in of "[email protected] / 123456Isa"

NikkiWines · 2021-04-07T00:55:35Z

~~Huh, I can't reproduce this at all on a fresh account~~ Oops, didn't read this. I see this is only happening on iOS

NikkiWines · 2021-04-07T01:08:42Z

Probably related: #2253

NikkiWines · 2021-04-07T01:19:52Z

Ah @Maftalion correctly identified the root cause, which we discussed earlier on but never added into this PR. We need to modify returnValueList: ['account', 'loginList', 'nameValuePairs'] to be returnValueList: account,loginList, nameValuePairs, here otherwise mobile doesn't return the correct values.

I was able to repro on dev and confirm the fix resolved the issue (for iOS, looking to see if the android whitescreen is related)

Old	New

isagoico · 2021-04-07T01:27:33Z

Forgot to mention above. We were unable execute the steps of this PR in the Android app because of this issue #2253

NikkiWines · 2021-04-07T01:34:58Z

I am not able to reproduce that android whitescreen issue locally

Edit: whitescreen issue is resolved

Allow secondary logins

a307e3d

Maftalion requested a review from a team as a code owner March 31, 2021 02:58

MelvinBot requested review from timszot and removed request for a team March 31, 2021 02:59

Maftalion commented Mar 31, 2021

View reviewed changes

src/pages/settings/AddLoginPage.js Outdated Show resolved Hide resolved

Maftalion added 3 commits March 30, 2021 21:04

update error message

fd83a87

Add note to login fields

3387536

refactor to use LOGIN_TYPE enum

07b67c1

NikkiWines requested changes Mar 31, 2021

View reviewed changes

Maftalion added 7 commits March 31, 2021 19:05

always use primary login

7d19d4a

update error message

20f4049

rename file, style changes

79278fe

Merge branch 'master' into matt-2029-secondary-logins

a627289

# Conflicts: # src/pages/settings/ProfilePage/index.js

update to dynamic route

3366fb7

add more feedback on code sent

86c1356

fix export

86ec587

NikkiWines requested changes Apr 2, 2021

View reviewed changes

small fixes

640e64b

NikkiWines mentioned this pull request Apr 5, 2021

[No QA] Validate secondary logins in E.cash #2231

Merged

5 tasks

NikkiWines previously approved these changes Apr 5, 2021

View reviewed changes

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

src/pages/settings/AddSecondaryLoginPage.js Outdated Show resolved Hide resolved

src/pages/settings/ProfilePage/LoginField.js Outdated Show resolved Hide resolved

Maftalion added 2 commits April 5, 2021 15:46

use consts for keyboard types

e54a315

small fixes

4ce0892

Maftalion dismissed NikkiWines’s stale review via 4ce0892 April 5, 2021 22:47

NikkiWines approved these changes Apr 5, 2021

View reviewed changes

timszot approved these changes Apr 6, 2021

View reviewed changes

timszot merged commit 5ab2683 into Expensify:master Apr 6, 2021

OSBotify mentioned this pull request Apr 6, 2021

Deploy Checklist: Expensify.cash 2021-03-19 #1926

Closed

85 tasks

tgolen reviewed Apr 6, 2021

View reviewed changes

tgolen mentioned this pull request Apr 6, 2021

Refactor picker to be a cross-platform component #2242

Merged

5 tasks

Maftalion mentioned this pull request Apr 7, 2021

[User Settings] Allow users to add secondary logins (followup) #2251

Merged

NikkiWines mentioned this pull request Apr 7, 2021

Update User.fetch to get a valid login list on mobile #2255

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[User Settings] Allow users to add secondary logins #2169

[User Settings] Allow users to add secondary logins #2169

Maftalion commented Mar 31, 2021 •

edited

Loading

NikkiWines left a comment

NikkiWines left a comment

NikkiWines left a comment

NikkiWines left a comment

tgolen left a comment

tgolen Apr 6, 2021

tgolen Apr 6, 2021

tgolen Apr 6, 2021

tgolen Apr 6, 2021

tgolen Apr 6, 2021

tgolen Apr 6, 2021

isagoico commented Apr 7, 2021

NikkiWines commented Apr 7, 2021

isagoico commented Apr 7, 2021

NikkiWines commented Apr 7, 2021 •

edited

Loading

NikkiWines commented Apr 7, 2021

NikkiWines commented Apr 7, 2021

isagoico commented Apr 7, 2021 •

edited

Loading

NikkiWines commented Apr 7, 2021 •

edited

Loading

[User Settings] Allow users to add secondary logins #2169

[User Settings] Allow users to add secondary logins #2169

Conversation

Maftalion commented Mar 31, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

NikkiWines left a comment

Choose a reason for hiding this comment

NikkiWines left a comment

Choose a reason for hiding this comment

NikkiWines left a comment

Choose a reason for hiding this comment

NikkiWines left a comment

Choose a reason for hiding this comment

tgolen left a comment

Choose a reason for hiding this comment

tgolen Apr 6, 2021

Choose a reason for hiding this comment

tgolen Apr 6, 2021

Choose a reason for hiding this comment

tgolen Apr 6, 2021

Choose a reason for hiding this comment

tgolen Apr 6, 2021

Choose a reason for hiding this comment

tgolen Apr 6, 2021

Choose a reason for hiding this comment

tgolen Apr 6, 2021

Choose a reason for hiding this comment

isagoico commented Apr 7, 2021

Expected Result:

Actual Result:

Action Performed:

Workaround:

Platform:

NikkiWines commented Apr 7, 2021

isagoico commented Apr 7, 2021

NikkiWines commented Apr 7, 2021 • edited Loading

NikkiWines commented Apr 7, 2021

NikkiWines commented Apr 7, 2021

isagoico commented Apr 7, 2021 • edited Loading

NikkiWines commented Apr 7, 2021 • edited Loading

Maftalion commented Mar 31, 2021 •

edited

Loading

NikkiWines commented Apr 7, 2021 •

edited

Loading

isagoico commented Apr 7, 2021 •

edited

Loading

NikkiWines commented Apr 7, 2021 •

edited

Loading