Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Plaid Link for web / Fix logic breaking Plaid flow on web #5029

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Sep 2, 2021

cc @aldo-expensify this should fix the bug you ran into on main

Details

We discovered while testing Plaid Link on the web side that the flow is broken after the changes here. Taking this opportunity to fix and update Plaid.

Fixed Issues (Comment)

$ #4785 (comment)

Tests AND QA Steps

  1. Start with a non-public domain account that has not yet set up a workspace or bank account
  2. Create a new workspace by tapping on the green FAB button
  3. Navigate to /bank-account by selecting the workspace and tapping the "Get Started" button
  4. Tap button to add account via Plaid (Note: This step may take time to complete)
  5. Enter test credentials user_good pass_good
  6. Press "Continue"
  7. Verify that the account selection dropdown appears

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

2021-09-02_10-52-17
2021-09-02_10-52-27

Mobile Web

Skip because they use the same code as web + there is no UI change.

Desktop

Skip because they use the same code as web + there is no UI change.

iOS

2021-09-02_11-35-35
2021-09-02_11-35-42

Android

2021-09-02_11-09-44
2021-09-02_11-09-37

@marcaaron marcaaron self-assigned this Sep 2, 2021
@marcaaron marcaaron marked this pull request as ready for review September 2, 2021 21:36
@marcaaron marcaaron requested a review from a team as a code owner September 2, 2021 21:36
@MelvinBot MelvinBot requested review from yuwenmemon and removed request for a team September 2, 2021 21:36
@aldo-expensify aldo-expensify self-requested a review September 2, 2021 22:56
Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, worked for me on web.

@marcaaron marcaaron merged commit 4283835 into main Sep 2, 2021
@marcaaron marcaaron deleted the marcaaron-fixPlaid2 branch September 2, 2021 23:14
@OSBotify
Copy link
Contributor

OSBotify commented Sep 2, 2021

✋ 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
Copy link
Contributor

OSBotify commented Sep 3, 2021

🚀 Deployed to staging by @marcaaron in version: 1.0.92-1 🚀

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

@isagoico
Copy link

isagoico commented Sep 3, 2021

@marcaaron Hello! Can we test this by navigating to /Bank-account/new? We currently don't have a domain that meets these requirements Start with a non-public domain account that has not yet set up a workspace or bank account

@marcaaron
Copy link
Contributor Author

I think this requirement can be worked around by using a deep link (sent via gmail) and then opening in New Expensify app.

https://staging.new.expensify.com/bank-account

If it doesn't work let us know and we'll find another way to test

@isagoico
Copy link

isagoico commented Sep 3, 2021

@marcaaron It worked on Android but not on iOS. On iOS it opens mWeb directly

@marcaaron
Copy link
Contributor Author

I'll look into that. I think I know why.

@isagoico
Copy link

isagoico commented Sep 3, 2021

This issue is failing this PR #5075

@isagoico
Copy link

isagoico commented Sep 3, 2021

I'll look into that. I think I know why.

Let me know if I should open a separate issue for this.

@marcaaron
Copy link
Contributor Author

Can you try new-expensify://bank-account to see if that will open the deep link?

@marcaaron marcaaron mentioned this pull request Sep 3, 2021
5 tasks
@isagoico
Copy link

isagoico commented Sep 3, 2021

Can you try new-expensify://bank-account to see if that will open the deep link?

This worked but this issue was also reproducible there #5075

@marcaaron
Copy link
Contributor Author

Ok cool thanks! Glad the deep link worked. I'm unsure what could be causing this issue, but asking for help now and re-testing on dev to make sure things still work locally.

@OSBotify
Copy link
Contributor

OSBotify commented Sep 4, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.93-1 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants