Deep link to add secondary login in OldDot #6319

luacmartins · 2021-11-16T03:25:24Z

This PR is a copy of #6176. I forgot to add a hold on the previous PR and it was merged before it should have been and we reverted it.

cc @mountiny

Details

Copy changes to Issue corporate cards and Connect bank account page and deep-linking to Add secondary login in OldDot.

Fixed Issues

https://github.com/Expensify/Expensify/issues/182071 (partial)

Tests

Create an account using a public domain, e.g @gmail.com
Follow the steps in this SO to add an "OPEN" bank account.
When you reach the last step, verify the following:

Copy

This bank account will be used to reimburse expenses, collect invoices, and pay bills all from the same account. Please add a work email address as a secondary login to enable the Expensify Card.

Button

Add work email address

Click the button and verify that it opens new browser tab on OldDot and directs user to Settings > Account Details > Add secondary login with the secondary login modal open.
Go back to NewDot and navigate to Settings > Select Workspace > Issue corporate cards.
Verify that the following:

Copy

Add a work email address to issue unlimited Expensify Cards for your workspace members, as well as all of these incredible benefits:

Up to 2% cash back
Digital and physical cards
No personal liability
Customizable limits

Button

Add work email address

Click the button and verify that it opens new browser tab on OldDot and directs user to Settings > Account Details > Add secondary login with the secondary login modal open.

QA Steps

Steps above.

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mountiny

Approved it once, gonna approve it again 😅

Jag96

A couple of questions, otherwise looks good!

Jag96 · 2021-11-16T18:56:46Z

src/libs/actions/User.js

+ * Subscribes to Expensify Card updates when checking loginList for private domains
+ */
+function subscribeToExpensifyCardUpdates() {
+    const pusherChannelName = `private-user-accountID-${currentUserAccountID}`;


It looks like we use the same in subscribeToUserEvents, can we consolidate those somehow? Also, is there a reason we don't need to check if currentUserAccountID exists in this function?

This is a one-time only subscription that happens when the user clicks the Add work email address link. Adding it to subscribeToUserEvents would subscribe users prematurely and unnecessarily.

I didn't add the check because when the user subscribes to this event we should have a currentUserAccountID. That is different from subscribeToUserEvents because subscribeToUserEvents is called upon login and race conditions could cause currentUserAccountID to be undefined. With that being said, I'll add the check just to be safe.

Sounds good, thanks for the explanation!

Jag96 · 2021-11-16T18:57:24Z

src/libs/actions/User.js

+    // Handle Expensify Card approval flow updates
+    Pusher.subscribe(pusherChannelName, Pusher.TYPE.EXPENSIFY_CARD_UPDATE, (pushJSON) => {
+        if (pushJSON.isUsingExpensifyCard) {
+            Onyx.merge(ONYXKEYS.USER, {isUsingExpensifyCard: pushJSON.isUsingExpensifyCard, isCheckingDomain: null});


Why do we set isCheckingDomain to null instead of false?

Setting it to null should delete the key from Onyx. Since this is a one-time only event, I felt like it was better to remove the key than leave it set as false.

luacmartins · 2021-11-19T17:12:00Z

No longer on hold! Self-merging since it was approved.

OSBotify · 2021-11-19T17:12:54Z

✋ 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-11-22T20:53:01Z

🚀 Deployed to staging by @luacmartins in version: 1.1.15-18 🚀

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

…eanUp Deep link to add secondary login in OldDot (cherry picked from commit ceba16f)

…ng-6319 🍒 Cherry pick PR #6319 to staging 🍒

OSBotify · 2021-11-23T23:54:42Z

🚀 Cherry-picked to staging by @roryabraham in version: 1.1.16-6 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

isagoico · 2021-11-24T13:56:02Z

We found this issue when retesting this on Android #6457. I don't think this PR is the culprit do so checking it off. Tested this on Android via adding the bank account through web and then logging into that same account in Android.

isagoico · 2021-11-24T14:00:30Z

@luacmartins @Jag96 Question about the OldDot redirection. This is expected to open this mWeb page right? I ask because OldDot is messy sometimes in mWeb (in this case it looks ok but I wanted to confirm if this was the expected behaviour)

luacmartins · 2021-11-24T19:30:07Z

Hi @isagoico! Yes, that is the expected behavior as of now. Thanks for confirming!

isagoico · 2021-11-25T13:00:41Z

@luacmartins Hello! This issue was found while executing this PR in iOS #6475 and it's currently failing QA for iOS - It's a different behaviour from what we have in Android. Can you give it a look 🙇‍♀️

luacmartins · 2021-11-25T18:36:17Z

Hi @isagoico! I put up a fix for it #6479. Thanks for bringing this up to my attention!

OSBotify · 2021-11-29T18:46:54Z

🚀 Deployed to production by @roryabraham in version: 1.1.16-10 🚀

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

OSBotify · 2021-11-29T18:48:36Z

🚀 Deployed to production by @roryabraham in version: 1.1.16-10 🚀

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

mananjadhav · 2023-04-27T09:46:12Z

I am tagging this PR to highlight an issue fixed here. All conditions in ternary expressions or left-hand operands on conditional renders, should be boolean. This PR is one of the PRs that uses conditional render with string operands, hence I am tagging it here for the contributors to check.

We've also updated the item in the checklist with this PR to avoid this issue in the future.

luacmartins added 2 commits November 15, 2021 20:19

change copy and create pusher event

677356a

change onPress

d51ef8f

luacmartins self-assigned this Nov 16, 2021

remove navigateToConciergeChat

97c71ad

luacmartins marked this pull request as ready for review November 16, 2021 03:34

luacmartins requested a review from a team as a code owner November 16, 2021 03:34

MelvinBot requested review from Jag96 and removed request for a team November 16, 2021 03:34

mountiny previously approved these changes Nov 16, 2021

View reviewed changes

Jag96 reviewed Nov 16, 2021

View reviewed changes

add accounID check

99536a4

luacmartins dismissed mountiny’s stale review via 99536a4 November 16, 2021 22:42

Jag96 approved these changes Nov 16, 2021

View reviewed changes

mountiny self-requested a review November 16, 2021 23:03

mountiny approved these changes Nov 16, 2021

View reviewed changes

luacmartins changed the title ~~[Hold Web-E #32333] Deep link to add secondary login in OldDot~~ Deep link to add secondary login in OldDot Nov 19, 2021

luacmartins merged commit ceba16f into main Nov 19, 2021

luacmartins deleted the cmartins-autoApproveTriggerCleanUp branch November 19, 2021 17:12

OSBotify mentioned this pull request Nov 22, 2021

Deploy Checklist: New Expensify 2021-11-22 #6399

Closed

isagoico mentioned this pull request Nov 23, 2021

"Add work email address" button doesn't show up and text copy is not the correct one #6439

Closed

github-actions bot pushed a commit that referenced this pull request Nov 23, 2021

Merge pull request #6319 from Expensify/cmartins-autoApproveTriggerCl…

deef03c

…eanUp Deep link to add secondary login in OldDot (cherry picked from commit ceba16f)

OSBotify mentioned this pull request Nov 23, 2021

🍒 Cherry pick PR #6319 to staging 🍒 #6452

Merged

luacmartins added a commit that referenced this pull request Nov 23, 2021

Merge pull request #6452 from Expensify/roryabraham-cherry-pick-stagi…

de8f909

…ng-6319 🍒 Cherry pick PR #6319 to staging 🍒

kavimuru mentioned this pull request Nov 24, 2021

Android - VBA - User could not add bank account using plaid option #6457

Closed

OSBotify mentioned this pull request Aug 16, 2022

Deploy Checklist: New Expensify 2022-08-04 #10253

Closed

alexxxwork mentioned this pull request Apr 21, 2023

[HOLD for payment 2023-05-05] [HOLD for payment 2023-04-20] [$1000] Text strings must be rendered within a <Text> component #16683

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deep link to add secondary login in OldDot #6319

Deep link to add secondary login in OldDot #6319

luacmartins commented Nov 16, 2021 •

edited

Loading

mountiny left a comment

Jag96 left a comment

Jag96 Nov 16, 2021

luacmartins Nov 16, 2021

Jag96 Nov 16, 2021

Jag96 Nov 16, 2021

luacmartins Nov 16, 2021

luacmartins commented Nov 19, 2021

OSBotify commented Nov 19, 2021

OSBotify commented Nov 22, 2021

OSBotify commented Nov 23, 2021

isagoico commented Nov 24, 2021 •

edited

Loading

isagoico commented Nov 24, 2021

luacmartins commented Nov 24, 2021

isagoico commented Nov 25, 2021 •

edited

Loading

luacmartins commented Nov 25, 2021

OSBotify commented Nov 29, 2021

OSBotify commented Nov 29, 2021

mananjadhav commented Apr 27, 2023

Deep link to add secondary login in OldDot #6319

Deep link to add secondary login in OldDot #6319

Conversation

luacmartins commented Nov 16, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mountiny left a comment

Choose a reason for hiding this comment

Jag96 left a comment

Choose a reason for hiding this comment

Jag96 Nov 16, 2021

Choose a reason for hiding this comment

luacmartins Nov 16, 2021

Choose a reason for hiding this comment

Jag96 Nov 16, 2021

Choose a reason for hiding this comment

Jag96 Nov 16, 2021

Choose a reason for hiding this comment

luacmartins Nov 16, 2021

Choose a reason for hiding this comment

luacmartins commented Nov 19, 2021

OSBotify commented Nov 19, 2021

OSBotify commented Nov 22, 2021

OSBotify commented Nov 23, 2021

isagoico commented Nov 24, 2021 • edited Loading

isagoico commented Nov 24, 2021

luacmartins commented Nov 24, 2021

isagoico commented Nov 25, 2021 • edited Loading

luacmartins commented Nov 25, 2021

OSBotify commented Nov 29, 2021

OSBotify commented Nov 29, 2021

mananjadhav commented Apr 27, 2023

luacmartins commented Nov 16, 2021 •

edited

Loading

isagoico commented Nov 24, 2021 •

edited

Loading

isagoico commented Nov 25, 2021 •

edited

Loading